-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LogsVolumePanel: support filtering logs from the selected level #474
Conversation
There's one scenario that we need to consider: the user previously had applied a Interacting with the Logs Volume panel adds
|
b29b605
to
3146893
Compare
When the user is |
745b5b1
to
12c3dff
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks @cyriltovena ! What does @grafana/observability-logs / @zizzpudding this of this? How does this behavior feels to you? |
@matyax what if we just ignored level/detected_level in the logs volume, although then we'd want to be able to set legend visibility when a level/detected_level filter is applied, but it looks like we can do that with series visibility overrides? 🤔
|
Pull request was converted to draft
I haven't follow the tech side of it but I love this feature. |
8b04be1
to
2ef107b
Compare
2ef107b
to
2edfb16
Compare
e11be15
to
c182f22
Compare
I've added a new variable, dedicated to hold the level, that is used everywhere but in the logs volume query. This should take this feature pretty much where we wanted. |
55e566f
to
0e0cafd
Compare
Not having to refresh logs volume seems to produce a bit more of responsiveness in the app. And it's one less query that we need to run. 💡 |
@grafana/observability-logs Mind taking another look? Thanks! |
It seems that we've been bitten by the buggy interpolation error we recently fixed, as the metric query it's producing for for fields and values is like: |
src/services/variables.ts
Outdated
@@ -12,7 +14,14 @@ export const VAR_LOGS_FORMAT = 'logsFormat'; | |||
export const VAR_LOGS_FORMAT_EXPR = '${logsFormat}'; | |||
export const VAR_LINE_FILTER = 'lineFilter'; | |||
export const VAR_LINE_FILTER_EXPR = '${lineFilter}'; | |||
export const LOG_STREAM_SELECTOR_EXPR = `${VAR_LABELS_EXPR} ${VAR_PATTERNS_EXPR} ${VAR_LOGS_FORMAT_EXPR} ${VAR_FIELDS_EXPR} ${VAR_LINE_FILTER_EXPR}`; | |||
let STREAM_SELECTOR_EXPR = `${VAR_LABELS_EXPR} ${VAR_PATTERNS_EXPR} ${VAR_LOGS_FORMAT_EXPR} ${VAR_LEVELS_EXPR} ${VAR_FIELDS_EXPR} ${VAR_LINE_FILTER_EXPR}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will wait for a new Grafana release before merging these changes. |
The whole thing is unmockable and it's not worth it
This reverts commit 13df68d.
7795455
to
6b6f988
Compare
Never stop believing. |
I think with that we should also upgrade the grafana versions in |
Definitely. |
Closes #422
This PR adds support to apply level filters from the Logs Volume panel.
Demo.mov