-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
KQL in TSVB #36784
KQL in TSVB #36784
Conversation
💔 Build Failed |
jenkins, test this |
💚 Build Succeeded |
f5ffb4a
to
badc57d
Compare
jenkins, test this |
💚 Build Succeeded |
💚 Build Succeeded |
💚 Build Succeeded |
409410c
to
101523c
Compare
💔 Build Failed |
101523c
to
b285f7a
Compare
💔 Build Failed |
💚 Build Succeeded |
1f9ae36
to
6a91b0c
Compare
💔 Build Failed |
💔 Build Failed |
aa27c84
to
352e227
Compare
💚 Build Succeeded |
💔 Build Failed |
💚 Build Succeeded |
1a26770
to
9239571
Compare
💚 Build Succeeded |
906d6b6
to
4c11e28
Compare
💔 Build Failed |
💔 Build Failed |
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.
A couple other things in addition to the inline comments:
- I think we should update the saved objects in the sample data sets and change the queries to use KQL where possible
- The current use of
screenTitle
on the QueryBarInput creates non-sensical aria-labels like “You are on search box of TimeseriesPanelConfigQuery page. Start typing to search and filter the VisEditor”. My filters agg PR makes this prop optional. It also updates the query_bar_input to pass down the ID EUI creates to the underlying input element so that screenreaders will automatically read the correct label for the input (although passing the id explicitly won't be necessary if you end up passing all unknown props to the input element, as I suggested in one of the inline comments). You could cherry-pick the commit where I did that, or simply merge in master if my PR gets merged first.
I've mainly looked at the code so far. I'd like to get some time with you to go through the functionality. I'm having issues getting TSVB to display data, but this is an issue for me in master as well, so I think I'm doing something wrong.
src/legacy/core_plugins/metrics/public/components/splits/filter_items.js
Outdated
Show resolved
Hide resolved
src/legacy/core_plugins/metrics/public/components/vis_types/table/config.js
Show resolved
Hide resolved
src/legacy/core_plugins/metrics/public/components/vis_types/timeseries/config.js
Outdated
Show resolved
Hide resolved
💔 Build Failed |
…ved_objects data for ecommerce, flights and logs TSVB visualizations
💚 Build Succeeded |
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.
Changes look good, just a couple things that I noticed:
- One of the sample saved objects that has a filter string still needs updated: "[eCommerce] Sold Products per Day”
- Still need to remove
screenTitle
prop everywhere now that it's optional
💚 Build Succeeded |
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!
💚 Build Succeeded |
* Squashed commit of the following: commit 0cff8243a99ff23421c3cbb15d62b26332a31e21 Author: Christiane Heiligers <[email protected]> Date: Mon May 20 16:16:15 2019 -0700 Adds ignore_lobal_filter YesNo component back Deletes fixtures file commit e54e1690c9c46e2de5e747d14030a7e27e74c138 Author: Christiane Heiligers <[email protected]> Date: Mon May 20 14:39:21 2019 -0700 squash-merges adding KQL with the full QueryBar and SavedObject migrations Debugging Adds filter defaults Updates index pattern for the QueryBar on componentDidUpdate squash-merges adding KQL with the full QueryBar and SavedObject migrations required Adds ignore_lobal_filter YesNo component back Deletes fixtures file Updates migration test number * Changes QueryBar to QueryBarInput in TimeseriesPanelConfigUi Replaces QueryBar with QueryBarInput in Timeseries Changes QueryBar to QueryBarInput in TSVB Metric Changes QueryBar to QueryBarInput in other TSVB visualisation editors Removes async from componentWillMount changes indexPattern to indexPatternObject in series and table query request processors * Squashed commit of the following: commit 2c3107963cff20b2d24323fddfaf80c848b86848 Author: Christiane Heiligers <[email protected]> Date: Thu May 23 15:15:06 2019 -0700 Cleans up comments commit 943150d4b9b71183e9d5b26c95449ea4519aad4c Author: Christiane Heiligers <[email protected]> Date: Thu May 23 14:09:53 2019 -0700 Cleans up parser check more commit d31de88525595720556440ae71515f5772774eaa Author: Christiane Heiligers <[email protected]> Date: Thu May 23 14:01:02 2019 -0700 Cleans up parser check commit b592071b1d939ec2df9b9c921d57a22dac54e703 Author: Christiane Heiligers <[email protected]> Date: Thu May 23 13:16:55 2019 -0700 skips server call on invalid kuery queries Cleans up Deletes duplicate migration function Adds 's' to split_by_filters method Resolves issues after rebase * Removes quotes around index pattern strings passed into fetch_index_patterns, adds comments on reasoning * queryBar to queryBarInput Adds transformFilterStringToQueryObject back to visualizations migrations that was lost during rebase * Deletes unused code * Fixes migrations test * compiles visualizations migrations for version 7.3.0 * temporary change to query_bar_input fetch_index_patterns file formatting Includes code from PR#37413 Clean up * Formatting * Uses series index pattern when override_index_pattern is selected and an index pattern is provided * Removes empty object as alternative to localStorage * Removes handelQueryChange from where it is not needed * extracts retrieval of the default query language from uiSettings into a helper method * Resolves some PR comments * Handles Query Error from an invalid syntax query * Converts string queries into objects with kuery as the language in saved_objects data for ecommerce, flights and logs TSVB visualizations * Deletes unused translation items * Adds optional id prop to the query_bar_input and makes the screenTitle optional * Updates [eCommerce] Sold Products per Day * Removes screenTitle from component QueryBarInput * Wraps index pattern strings in a double quote to allow for names such as 'logstash-*'
* Squashed commit of the following: commit 0cff8243a99ff23421c3cbb15d62b26332a31e21 Author: Christiane Heiligers <[email protected]> Date: Mon May 20 16:16:15 2019 -0700 Adds ignore_lobal_filter YesNo component back Deletes fixtures file commit e54e1690c9c46e2de5e747d14030a7e27e74c138 Author: Christiane Heiligers <[email protected]> Date: Mon May 20 14:39:21 2019 -0700 squash-merges adding KQL with the full QueryBar and SavedObject migrations Debugging Adds filter defaults Updates index pattern for the QueryBar on componentDidUpdate squash-merges adding KQL with the full QueryBar and SavedObject migrations required Adds ignore_lobal_filter YesNo component back Deletes fixtures file Updates migration test number * Changes QueryBar to QueryBarInput in TimeseriesPanelConfigUi Replaces QueryBar with QueryBarInput in Timeseries Changes QueryBar to QueryBarInput in TSVB Metric Changes QueryBar to QueryBarInput in other TSVB visualisation editors Removes async from componentWillMount changes indexPattern to indexPatternObject in series and table query request processors * Squashed commit of the following: commit 2c3107963cff20b2d24323fddfaf80c848b86848 Author: Christiane Heiligers <[email protected]> Date: Thu May 23 15:15:06 2019 -0700 Cleans up comments commit 943150d4b9b71183e9d5b26c95449ea4519aad4c Author: Christiane Heiligers <[email protected]> Date: Thu May 23 14:09:53 2019 -0700 Cleans up parser check more commit d31de88525595720556440ae71515f5772774eaa Author: Christiane Heiligers <[email protected]> Date: Thu May 23 14:01:02 2019 -0700 Cleans up parser check commit b592071b1d939ec2df9b9c921d57a22dac54e703 Author: Christiane Heiligers <[email protected]> Date: Thu May 23 13:16:55 2019 -0700 skips server call on invalid kuery queries Cleans up Deletes duplicate migration function Adds 's' to split_by_filters method Resolves issues after rebase * Removes quotes around index pattern strings passed into fetch_index_patterns, adds comments on reasoning * queryBar to queryBarInput Adds transformFilterStringToQueryObject back to visualizations migrations that was lost during rebase * Deletes unused code * Fixes migrations test * compiles visualizations migrations for version 7.3.0 * temporary change to query_bar_input fetch_index_patterns file formatting Includes code from PR#37413 Clean up * Formatting * Uses series index pattern when override_index_pattern is selected and an index pattern is provided * Removes empty object as alternative to localStorage * Removes handelQueryChange from where it is not needed * extracts retrieval of the default query language from uiSettings into a helper method * Resolves some PR comments * Handles Query Error from an invalid syntax query * Converts string queries into objects with kuery as the language in saved_objects data for ecommerce, flights and logs TSVB visualizations * Deletes unused translation items * Adds optional id prop to the query_bar_input and makes the screenTitle optional * Updates [eCommerce] Sold Products per Day * Removes screenTitle from component QueryBarInput * Wraps index pattern strings in a double quote to allow for names such as 'logstash-*'
Summary
Using KQL with the autocomplete it offers makes it easier to create a query without the need to remember the exact field name and it assists with operators and values.
This PR replaces the text input fields for filters in the Visual Builder with QueryBarInput components that update as a query is entered with the option of using Kuery syntax with autocomplete or Lucene syntax.
A migration was needed to transform the filter strings in visualizations to filter query objects, containing the original string and the query language. We assume that if the query language is not present, it is Lucene syntax.
References #29790, #30177
PR in favor of #36175
References Support KQL in TSVB Filter aggregations
Adds KQL to Visual Builder (TSVB)
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.[including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support- [ ] Documentation was added for features that require explanation or tutorialsand screenreader accessibilityFor maintainers
- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately