Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Enable KQL support for TSVB #26006
Enable KQL support for TSVB #26006
Changes from 17 commits
ac3532e
7f1464b
9db9e9d
15125bd
4bf31a3
b4a570f
eb27329
f007742
6093879
ce46be8
a61214e
c3ea045
85b81fc
0c431f3
edc0342
5e78ee9
9384a2d
940aba6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
There's a util function you can use to do this instead if you want
kibana/src/ui/public/index_patterns/static_utils/index.js
Lines 28 to 37 in b99c516
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.
I know, but reside on public and we need to use it on server side. I should move that function in a shared folder and reuse it.
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.
Oh duh, I missed the fact that one was server and one public. No sweat if you don't wanna move that around in this PR.
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.
Should the query bar really affect the annotations? I think annotations are likely to come from a different index, so queries in the global query bar will likely filter out all docs in the annotation index. Annotations also have options to ignore the global filter and the panel filter by which are enabled by default so if we did want to allow the query bar to affect annotations, I’d think we would want an option to ignore it as well. @alexfrancoeur or @AlonaNadler do you have any opinion here? I'm not super familiar with how annotations are generally used in the wild so I'm just guessing.
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.
Quite timely, I literally just closed two enhancement requests this week around annotations being filterable. They have been filterable for awhile now but it isn't too discoverable. https://github.com/elastic/enhancements/issues/4007, https://github.com/elastic/enhancements/issues/5141. I'd say it's important, we have controls in TSVB for it as of 5.5 #11260
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.
@alexfrancoeur Cool, so just to make sure I understand, you're saying we should allow the query bar to filter annotations, and add an additional boolean option called something like
Ignore global query bar
?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.
And what about the visualization, don't we need to add the
Ignore global query bar
also here? So you can opt-out from the query bar as it works before this PRThere 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.
I think KQL should filter out TSVB just as the existing query bar and filters does today. There are options to ignore the filters or to persist annotations there. This functionality is generally applied to metrics as well in TSVB, not just annotations.
These boolean options should already be available in the TSVB UI today. If it's easier to sync live, I'm happy to touch base over zoom.
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.
@alexfrancoeur yes please a zoom sync will help :)
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.
@alexfrancoeur @Bargs @lukasolson with this commit 9384a2d I've updated the PR to mimic exactly how TSVB works with lucene syntax:
yes
onIgnore global filters
on data, any filter added to a dashboard or any search written on the query bar will be ignored by the visualizationyes
onIgnore global filters
on annotations, any filter added to a dashboard or any search written on the query bar will be ignored by the annotationsno
onIgnore global filters
on data, any filter added to a dashboard or any search written on the query bar will filter the visualization datano
onIgnore global filters
on annotations, any filter added to a dashboard or any search written on the query bar will filter the annotationsThe current label
Ignore global filters
is, IMHO, a bit misleading because doesn't seems to include queries on the querybar, but I think we can change that on a different PR.If this is ok for you, we can merge this PR and backport it to 6.7