[WIP] KQL in TSVB with full QueryBar #36175

TinaHeiligers · 2019-05-07T13:59:31Z

Note: #36784 has been opened in favor of this PR.

Summary

References Support KQL in TSVB Filter aggregations
Adds KQL to Visual Builder (TSVB)
Still TODO:

Replace the full QueryBar with the QueryBarInput.
Update components accordingly
Update this summary
Complete checks below

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
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 tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

elasticmachine · 2019-05-07T15:18:32Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-05-07T17:56:45Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

TinaHeiligers · 2019-05-07T20:00:17Z

jenkins, test this

elasticmachine · 2019-05-07T21:10:18Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-05-07T23:41:58Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-05-08T18:12:21Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-05-09T02:57:26Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-05-09T15:43:59Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-05-09T23:25:52Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

alexwizp · 2019-05-10T10:49:15Z

src/legacy/core_plugins/metrics/public/components/annotations_editor.js

+  async componentDidMount() {
+    await this.fetchIndexPatternsForQuery();
+  }
+  fetchIndexPatternsForQuery = async () => {


Can we reuse this method? I see you duplicated this method 7 times

The index pattern we use to fetch the index pattern from SavedObjects has a different reference in each method and it needs to set state in each component. Each method is slightly different and that is why it is redefined. If you have a recommendation on how to more easily set state in a component with a shared method, please let me know.

@TinaHeiligers I've checked that,
We have similar body for:

src/legacy/core_plugins/metrics/public/components/annotations_editor.js src/legacy/core_plugins/metrics/public/components/panel_config/gauge.js src/legacy/core_plugins/metrics/public/components/panel_config/markdown.js src/legacy/core_plugins/metrics/public/components/panel_config/metric.js src/legacy/core_plugins/metrics/public/components/panel_config/table.js src/legacy/core_plugins/metrics/public/components/panel_config/timeseries.js src/legacy/core_plugins/metrics/public/components/panel_config/top_n.js src/legacy/core_plugins/metrics/public/components/series.js

and only one different implementation for
src/legacy/core_plugins/metrics/public/components/split.js

I still think that we should somehow to reuse it

alexwizp · 2019-05-10T10:56:51Z

src/legacy/core_plugins/metrics/public/components/annotations_editor.js

+  }
+  fetchIndexPatternsForQuery = async () => {
+    const searchIndexPattern = this.props.model.index_pattern ?
+      this.props.model.index_pattern :


Probably we can save 2 lines using || operator
const searchIndexPattern = this.props.model.index_pattern || this.props.model.default_index_pattern

alexwizp · 2019-05-10T11:02:51Z

src/legacy/core_plugins/metrics/public/components/annotations_editor.js

-
+  handleSubmit = (model, query) => {
+    const part = { query_string: query.query };
+    collectionActions.handleChange(this.props, _.assign({}, model, part));


we are trying to remove all references to _. in our code and don't add the new one.

what do you think about:

collectionActions.handleChange(this.props, { ...model, ...part });`

alexwizp · 2019-05-10T11:04:27Z

src/legacy/core_plugins/metrics/public/components/annotations_editor.js

+                        : this.state.uiQueryLanguage,
+                      query: model.query_string.query,
+                    }}
+                    screenTitle={'AnnotationsEditor'}


Should we localize this value?

The value is localized internally within the QueryBarInputUI component.

Got it, thanks!

alexwizp · 2019-05-10T11:08:07Z

src/legacy/core_plugins/metrics/public/components/panel_config/top_n.js

+    this.state = {
+      selectedTab: 'data',
+      indexPatternForQuery: {},
+      uiQuerylanguage: uiSettingsQueryLanguage,


Do we have any cases when this value will be changed in the 'state'? If not probably we can remove it from state and do something like

<QueryBar query={{ language: model.filter.language || uiSettingsQueryLanguage , query: model.filter.query, }}

alexwizp · 2019-05-10T11:56:05Z

src/legacy/core_plugins/metrics/public/components/panel_config/metric.js

    const { model } = this.props;
-    if (!model.background_color_rules || (model.background_color_rules && model.background_color_rules.length === 0)) {


let's try to use isEmpty method from lodash here?

I didn't add that but can change it if preferred.

alexwizp · 2019-05-10T12:01:16Z

src/legacy/core_plugins/metrics/public/lib/fetch_index_patterns.js

+  let indexPatternObject;
+  const indexPatternsFromSavedObjects = await chrome.getSavedObjectsClient().find({
+    type: 'index-pattern',
+    fields: ['title', 'fields'],


Probably we can exclude'fields'from this array

alexwizp · 2019-05-10T12:13:36Z

src/legacy/core_plugins/metrics/public/components/annotations_editor.js

  constructor(props) {
    super(props);
+    this.state = {
+      indexPatternForQuery: {},


I see the 'QueryBar' works with the index pattern object but not sure that we should add one more field for storing index pattern value.

I offer you 2 different ways:

create a wrapper upper the 'QueryBar' component and move all this logic related to uiQueryLanguage and indexPatternForQuery to this component. It helps you to remove all code duplication

more complex way. TSVB should work with index pattern object instead of string value of index pattern. But I see a lot of changes here.

@timroes @markov00 what do you think?

alexwizp · 2019-05-10T12:16:52Z

src/legacy/core_plugins/metrics/public/lib/fetch_index_patterns.js

+    search_fields: ['title'],
+  });
+  const exactMatch = indexPatternsFromSavedObjects.savedObjects.find(
+    indexPattern => indexPattern.attributes.title === indexPatternString


TSVB allow us to work with wildcards e.g. kibana_*. Looks like for my example this method always return us false
Should we support this types of indexes?

The QueryBar needs an exact index pattern for autocomplete and suggestions. If we allow a wildcard then we cannot be certain that the index pattern returned from getFromSavedObject is the one that we need.

elasticmachine · 2019-05-10T20:58:54Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

alexwizp · 2019-05-11T08:51:11Z

src/legacy/core_plugins/metrics/public/components/annotations_editor.js

-                    onChange={this.handleChange(model, 'query_string')}
-                    value={model.query_string}
-                    fullWidth
+                  <QueryBar


@TinaHeiligers @Bargs Can we disable the refresh functionality for the QueryBar?

With this one we have 4 components which somehow related to the refresh functionality. Adding one more refresh button can confuse our users.
Also it's not working property with Auto-update. I expect that if user turn on this check-box next filter should be applying without pressing the refresh/update button.

Yep, that's already in progress.

elasticmachine · 2019-05-15T20:41:24Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-05-18T19:17:54Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

…Bar in TimeSeries

…uest processor for query

elasticmachine · 2019-05-20T21:31:15Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

TinaHeiligers · 2019-05-20T22:26:00Z

@alexwizp @Bargs would you mind if I address your comments in a new PR and close this one? I've had a nightmare time with rebasing all these commits against master. Since there's still a lot of work to be done, I've squash-merged the work to date into a new branch to continue with the work from there. It will make progress much quicker.

alexwizp · 2019-05-21T09:08:18Z

Issue:

Install kibana_sample_data_ecommerce (set as default index) and kibana_sample_data_logs sample data
Create new TSVB visualization
Go to TSVB panel options and type something in a Panel filter input.

Important:
We see fields related to default index pattern. It's correct
Change index pattern to kibana_sample_data_logs and try to change value for Panel filter

alexwizp · 2019-05-21T09:28:57Z

src/legacy/core_plugins/metrics/public/components/annotations_editor.js

+  async componentDidMount() {
+    await this.fetchIndexPatternsForQuery();
+  }
+  fetchIndexPatternsForQuery = async () => {


@TinaHeiligers I've checked that,
We have similar body for:

src/legacy/core_plugins/metrics/public/components/annotations_editor.js src/legacy/core_plugins/metrics/public/components/panel_config/gauge.js src/legacy/core_plugins/metrics/public/components/panel_config/markdown.js src/legacy/core_plugins/metrics/public/components/panel_config/metric.js src/legacy/core_plugins/metrics/public/components/panel_config/table.js src/legacy/core_plugins/metrics/public/components/panel_config/timeseries.js src/legacy/core_plugins/metrics/public/components/panel_config/top_n.js src/legacy/core_plugins/metrics/public/components/series.js

and only one different implementation for
src/legacy/core_plugins/metrics/public/components/split.js

I still think that we should somehow to reuse it

alexwizp · 2019-05-21T09:33:14Z

src/legacy/core_plugins/metrics/public/components/panel_config/timeseries.js

-                    fullWidth
+                  <QueryBar
+                    query={{
+                      language: model.filter.language ? model.filter.language : 'kuery',


I see that you use 'kuery' only here. In other places you use uiSettingsQueryLanguage. Is it correct?

I will change it.

alexwizp · 2019-05-21T09:39:01Z

src/legacy/core_plugins/metrics/public/components/series.js

+      return <Component {...params} />;
+    }
+    return (
+      <div>


Are you sure we need that extra div? I don't see any reason to refactor this this file

alexwizp · 2019-05-21T09:45:31Z

@TinaHeiligers @Bargs From my point of view we should continue to work in scope of this PR.
But if you want to continue with that PR and merge it please get approval from @timroes / @markov00

TinaHeiligers · 2019-05-21T14:13:30Z

@alexwizp Please hold off on reviewing while the WIP label is on this PR.
All comments will be addressed in #36784 opened in favor of this one.

The work is still in progress and not ready for review yet

TinaHeiligers requested review from a team and Bargs May 7, 2019 13:59

TinaHeiligers force-pushed the KQL_in_TSVB_rebased_May6_am branch from d0c7454 to 0d72b39 Compare May 7, 2019 22:31

TinaHeiligers force-pushed the KQL_in_TSVB_rebased_May6_am branch from 0d72b39 to 1a3ef1c Compare May 8, 2019 17:05

TinaHeiligers force-pushed the KQL_in_TSVB_rebased_May6_am branch from b6a4b7b to 8724d68 Compare May 9, 2019 14:34

alexwizp reviewed May 10, 2019

View reviewed changes

alexwizp reviewed May 11, 2019

View reviewed changes

TinaHeiligers force-pushed the KQL_in_TSVB_rebased_May6_am branch from 51620a4 to 061a01d Compare May 18, 2019 19:56

TinaHeiligers added 10 commits May 18, 2019 15:23

refactors Split component

91cc695

debugging

f9eb1d2

Fixes typo

79fccf7

Adds migration test for visualization migration 7.2.0

a181fc0

Fixes migration path to series and vis type check

732b314

Adds test for migrating markdown saved objects

858eb49

Migrates split filter items

44ec874

Adds comments

5c34602

moves json fixtures to another file until I can clean them up

03acbed

comments

f0b3a1a

TinaHeiligers added 20 commits May 20, 2019 13:03

fixed props types for index pattern

4c1dac2

deletes commented out refactored code

256f6f0

Adds blue border markers for components that need to use the Querybar

8e485fc

Adds QueryBar to TopN Panel Options Tab, renames screenName for Query…

db7fea0

…Bar in TimeSeries

Adds QueryBar to Gauge Panel Options Panel Filter

0cba967

Adds QueryBar to Markdown Panel Options Panel Filter

526ba78

Adds QueryBar to TSVB Table Panel Config Component, updates table req…

d50d37b

…uest processor for query

Refactors table request processors to handle queries

652e578

Adds todo's to modify default filter initialization in components

4aaa5a8

Deletes done TODO

e7042af

Changes default QueryBar query language from 'lucene' to 'kuery'

9e53eae

refactors Split component

1dede25

debugging

29fdbc1

Fixes typo

113340e

Adds migration test for visualization migration 7.2.0

3bed3b1

Migrates split filter items

76de27b

Adds comments

141b1ec

Visualization migration and tests update

f38aa1b

Fixes timeseries

7057264

Merge branch 'temp-rebase' into KQL_in_TSVB_rebased_May6_am

4aad263

alexwizp previously requested changes May 21, 2019

View reviewed changes

TinaHeiligers mentioned this pull request May 21, 2019

KQL in TSVB #36784

Merged

4 tasks

TinaHeiligers closed this May 21, 2019

TinaHeiligers deleted the KQL_in_TSVB_rebased_May6_am branch October 14, 2020 15:56

		const { model } = this.props;
		if (!model.background_color_rules \|\| (model.background_color_rules && model.background_color_rules.length === 0)) {

[WIP] KQL in TSVB with full QueryBar #36175

[WIP] KQL in TSVB with full QueryBar #36175

Conversation

TinaHeiligers commented May 7, 2019 • edited Loading

Note: #36784 has been opened in favor of this PR.

Summary

Checklist

For maintainers

elasticmachine commented May 7, 2019

💔 Build Failed

elasticmachine commented May 7, 2019

💔 Build Failed

TinaHeiligers commented May 7, 2019

elasticmachine commented May 7, 2019

💔 Build Failed

elasticmachine commented May 7, 2019

💔 Build Failed

elasticmachine commented May 8, 2019

💔 Build Failed

elasticmachine commented May 9, 2019

💔 Build Failed

elasticmachine commented May 9, 2019

💔 Build Failed

elasticmachine commented May 9, 2019

💔 Build Failed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented May 10, 2019

💔 Build Failed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented May 15, 2019

💔 Build Failed

elasticmachine commented May 18, 2019

💔 Build Failed

elasticmachine commented May 20, 2019

💚 Build Succeeded

TinaHeiligers commented May 20, 2019

alexwizp commented May 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexwizp commented May 21, 2019

TinaHeiligers commented May 21, 2019 • edited Loading

TinaHeiligers commented May 7, 2019 •

edited

Loading

TinaHeiligers commented May 21, 2019 •

edited

Loading