TSVB is retaining results even after indexpattern is removed from opt… #32003

alexwizp · 2019-02-26T10:27:10Z

Fix: #31951

Checklist

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-02-26T11:06:32Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-02-26T12:05:29Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-02-26T12:49:01Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-02-26T16:33:31Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

alexwizp · 2019-02-27T10:31:00Z

Current behavior:

when creating a new tsvb viz value for index pattern we take from the configuration (defaultIndex value);
when opening an existing tsvb viz with empty indexpattern value for index pattern we take from the configuration (defaultIndex value) like in previous case;
clearing the index-pattern field we set empty string as a value for index pattern (it's equivalent of * - all available indexes)

My main concern in points 2 and 3. I do not understand why, if the user clears the index pattern field, we show him data for all available indexes, but after saving and reopening saved visualization, we replace this index with the default index pattern? It's confusing and I think we should replace index pattern only for one case: it's a creating a new tsvb viz.

How to fix it: in the file src/legacy/core_plugins/metrics/public/components/vis_editor.js we should do:

Any ideas?
@AlonaNadler @bhavyarm @timroes @markov00

timroes · 2019-02-28T12:01:55Z

After syncing with @markov00 about that, we think we should use the behavior that you suggested, where we use undefined by default and replace an undefined (but not an empty string) to the default index pattern in the editor (in such a way, that the index pattern name will actually be stored when saving - which this PR does as far as I tested).

There is still some weird behavior around that empty string index pattern and what the user sees in the editor. So we came up with the following suggestion:

If the saved object has an empty string in there, we're treating that as the "default index" pattern for querying.
We will also NOT replace the "empty string" in the editor by the name of the default index pattern (since that is one of the large confusions around that feature). Instead we will just show the empty index pattern box.
We'll add a placeholder to the index pattern box that has the name of the default index pattern in it.
When the field is empty, we'll show a warning (below or some icon with a tooltip behind the box), that tells "If no index pattern is defined the default index pattern is used. To query all indexes use *."

That way we should be very explicit and not break any existing functionality. New visualizations will have the default index pattern shown in the editor (via the undefined check) and also saved in the saved object. Old existing visualizations (with empty index pattern) will still search for the default index pattern. And we're making that behavior clear to the user via placeholder and hints.

If we would e.g. start querying all indexes in case it's an empty string, the old saved TSVB visualizations will behave weird, because for the user it looked like the default index in the editor (since we're replacing it there), but then on dashboard it will load all indexes (the weird bug in the past). So we should stay with loading the default index pattern for all empty string index patterns. We also can't easily migrate those empty strings, since during the saved object migration phase Kibana isn't started completely, meaning we're not having access to the configuration and could see what the default index pattern is.

alexwizp · 2019-02-28T17:11:18Z

@timroes @bhavyarm Please review

elasticmachine · 2019-02-28T17:48:50Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-02-28T18:29:25Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

markov00 · 2019-03-01T09:27:37Z

@alexwizp With these changes I don't see the timestamp field populated with the right values for the default index pattern. The field I'm currently seeing seems to be related to the * pattern.
Can you please verify that and check also with @sulemanof that he is currently working on these fields?

…ions

… = defaultIndexPattern;

elasticmachine · 2019-03-01T13:46:37Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

markov00

Tested locally LGTM

flash1293

Tested and works, LGTM 👍 I added one small nitpick about a typo

flash1293 · 2019-03-01T14:22:11Z

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

@@ -62,11 +63,16 @@ export const IndexPattern = props => {
              id="tsvb.indexPatternLabel"
              defaultMessage="Index pattern"
            />)}
+            helpText={(model.default_index_pattern && !model[indexPatternName] && <FormattedMessage
+              id="tsvb.indexPattern.searchByDefaultIndex"
+              defaultMessage="Default index pattern is used. To query all indexes use *"


I think it should use indices as plural of index - but I wasn't sure and searched for it in the Kibana codebase - 2800 vs 141 in favor of indices, so I think we should go with it :)

bhavyarm

LGTM. Please backport to 6.7.0

sulemanof

LGTM

elastic#32003)

elastic#32003) # Conflicts: # src/legacy/core_plugins/metrics/server/lib/get_fields.js

#32003) (#32320) # Conflicts: # src/legacy/core_plugins/metrics/server/lib/get_fields.js

#32003) (#32319) # Conflicts: # src/legacy/core_plugins/metrics/server/lib/get_fields.js

#32003) (#32317)

alexwizp requested review from timroes, markov00, flash1293 and sulemanof February 26, 2019 10:27

markov00 requested a review from bhavyarm February 26, 2019 11:01

alexwizp force-pushed the 31951 branch from 71613b4 to b696a4a Compare February 26, 2019 11:26

alexwizp force-pushed the 31951 branch from 0ceb2e1 to 8788ff3 Compare February 28, 2019 16:54

alexwizp added 7 commits March 1, 2019 15:37

TSVB is retaining results even after indexpattern is removed from opt…

3c063ae

…ions

call this.handleChange instead of this.props.vis.params.index_pattern…

510cfbe

… = defaultIndexPattern;

fix error on empty string index pattern

937370f

set the default index pattern into placeholder

c2bc6c8

remove extra comment

2c1223a

revert undedined -> ''

97d956c

get right fields for default index pattern

1cfa927

alexwizp force-pushed the 31951 branch from e754fca to 1cfa927 Compare March 1, 2019 12:58

markov00 approved these changes Mar 1, 2019

View reviewed changes

flash1293 approved these changes Mar 1, 2019

View reviewed changes

bhavyarm approved these changes Mar 1, 2019

View reviewed changes

sulemanof reviewed Mar 1, 2019

View reviewed changes

sulemanof approved these changes Mar 1, 2019

View reviewed changes

alexwizp merged commit f733308 into elastic:master Mar 1, 2019

alexwizp added a commit to alexwizp/kibana that referenced this pull request Mar 1, 2019

TSVB is retaining results even after indexpattern is removed from opt… (

da50e2f

elastic#32003)

alexwizp mentioned this pull request Mar 1, 2019

[7.x] TSVB is retaining results even after indexpattern is removed from opt… (#32003) #32317

Merged

alexwizp added a commit to alexwizp/kibana that referenced this pull request Mar 1, 2019

TSVB is retaining results even after indexpattern is removed from opt… (

53c8207

elastic#32003) # Conflicts: # src/legacy/core_plugins/metrics/server/lib/get_fields.js

This was referenced Mar 1, 2019

[7.0] TSVB is retaining results even after indexpattern is removed from opt… (#32003) #32319

Merged

[6.7] TSVB is retaining results even after indexpattern is removed from opt… (#32003) #32320

Merged

alexwizp added a commit to alexwizp/kibana that referenced this pull request Mar 1, 2019

TSVB is retaining results even after indexpattern is removed from opt… (

aeac391

elastic#32003) # Conflicts: # src/legacy/core_plugins/metrics/server/lib/get_fields.js

alexwizp added a commit that referenced this pull request Mar 4, 2019

TSVB is retaining results even after indexpattern is removed from opt… (

e768a2e

#32003) (#32320) # Conflicts: # src/legacy/core_plugins/metrics/server/lib/get_fields.js

alexwizp added a commit that referenced this pull request Mar 4, 2019

TSVB is retaining results even after indexpattern is removed from opt… (

e054492

#32003) (#32319) # Conflicts: # src/legacy/core_plugins/metrics/server/lib/get_fields.js

alexwizp added a commit that referenced this pull request Mar 4, 2019

TSVB is retaining results even after indexpattern is removed from opt… (

68e03eb

#32003) (#32317)

alexwizp added release_note:fix v6.7.0 labels Mar 14, 2019

alexwizp deleted the 31951 branch January 4, 2020 08:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TSVB is retaining results even after indexpattern is removed from opt… #32003

TSVB is retaining results even after indexpattern is removed from opt… #32003

alexwizp commented Feb 26, 2019

elasticmachine commented Feb 26, 2019

elasticmachine commented Feb 26, 2019

elasticmachine commented Feb 26, 2019

elasticmachine commented Feb 26, 2019

alexwizp commented Feb 27, 2019 •

edited

Loading

timroes commented Feb 28, 2019

alexwizp commented Feb 28, 2019

elasticmachine commented Feb 28, 2019

elasticmachine commented Feb 28, 2019

markov00 commented Mar 1, 2019

elasticmachine commented Mar 1, 2019

markov00 left a comment

flash1293 left a comment

flash1293 Mar 1, 2019

bhavyarm left a comment

sulemanof left a comment

TSVB is retaining results even after indexpattern is removed from opt… #32003

TSVB is retaining results even after indexpattern is removed from opt… #32003

Conversation

alexwizp commented Feb 26, 2019

Checklist

For maintainers

elasticmachine commented Feb 26, 2019

💔 Build Failed

elasticmachine commented Feb 26, 2019

💔 Build Failed

elasticmachine commented Feb 26, 2019

💔 Build Failed

elasticmachine commented Feb 26, 2019

💚 Build Succeeded

alexwizp commented Feb 27, 2019 • edited Loading

timroes commented Feb 28, 2019

alexwizp commented Feb 28, 2019

elasticmachine commented Feb 28, 2019

💔 Build Failed

elasticmachine commented Feb 28, 2019

💚 Build Succeeded

markov00 commented Mar 1, 2019

elasticmachine commented Mar 1, 2019

💚 Build Succeeded

markov00 left a comment

Choose a reason for hiding this comment

flash1293 left a comment

Choose a reason for hiding this comment

flash1293 Mar 1, 2019

Choose a reason for hiding this comment

bhavyarm left a comment

Choose a reason for hiding this comment

sulemanof left a comment

Choose a reason for hiding this comment

alexwizp commented Feb 27, 2019 •

edited

Loading