Remove support for time-based interval index patterns with migration #35262

lukasolson · 2019-04-17T21:14:20Z

This is an intermediary step for 7.x to work towards #35173 without breaking existing users with time-based interval index patterns.

With this PR, when you load a time-based interval index pattern, you'll get a warning message:

The warning message explains that you are now querying the index, as converted to wildcards (which may include additional indices than the original index pattern). It also has a link to the index pattern in advanced settings, where you can actually easily migrate the index pattern:

elasticmachine · 2019-04-17T21:14:22Z

Pinging @elastic/kibana-app-arch

elasticmachine · 2019-04-17T22:08:40Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

AlonaNadler · 2019-04-17T22:17:35Z

Based on the message, I understand that the users don't have to click on the migrate they can continue with the way their current index set up interval-based, is that true?
If the users don't migrate, do Kibana query logstash-* or still query based in the interval?
If the users don't migrate do they get the error message each time? does the notification show up in every application?

I suggest we have a link in the message to the 7.0 breaking changes

@gchaps can you review the text in the notification and the index pattern page?

gchaps · 2019-04-17T23:31:31Z

Here are some suggestions.

Warning 1

Support for time intervals was removed

You are querying all indices that match logstash-*. For more information, view the index pattern in management.

Note: Can you add a space after the last sentence and the index pattern name?

Warning 2

Support for time-based index patterns was removed

These index patterns will not work in the next major version of Kibana. By clicking the Migrate button, you can migrate your index pattern to a wildcard pattern.

Migrate

Page text

This page lists every field in the index and its mapping type. To change a field type, use the Elasticsearch Mapping API.

lukasolson · 2019-04-18T18:28:28Z

Based on the message, I understand that the users don't have to click on the migrate they can continue with the way their current index set up interval-based, is that true?
If the users don't migrate, do Kibana query logstash-* or still query based in the interval?
If the users don't migrate do they get the error message each time? does the notification show up in every application?

That's correct, Kibana will query the wildcard pattern instead of the interval-based pattern, so in your example it would be logstash-*. Yes, they can continue without migrating, and yes, they will get the error each time they load Kibana and the index pattern is loaded.

elasticmachine · 2019-04-18T19:18:51Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-18T22:55:59Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-18T23:58:22Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-19T00:37:49Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-23T20:11:49Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-23T22:04:23Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

Bargs

This may be a pre-existing issue, but one thing I noticed is if I import an old interval based pattern via the management UI the interval name gets removed but the title is not migrated, so you end up with a pattern that does not work, but you also get no warnings (because the condition to show the warnings is based on whether the intervalName attribute exists).

Otherwise this LGTM. I did a fairly brief review of the code but nothing popped out at me. I tested the functionality with an old interval based pattern and everything I tried worked as expected.

Bargs · 2019-04-23T21:39:16Z

src/legacy/ui/public/index_patterns/_index_pattern.js

-            this.title, this.getInterval(), start, stop, sortDirection
-          );
-        }
+    getIndex() {


Could you add some tests for this method? This regex magic is complicated enough I just want to make sure it handles every edge case we might expect.

elasticmachine · 2019-04-23T22:57:39Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

lukeelmers

Code LGTM - tested & everything seems to work as intended. Added two minor UX notes on the toast.

lukeelmers · 2019-04-23T23:31:49Z

src/legacy/ui/public/index_patterns/_index_pattern.js

+      const warningText = i18n.translate('common.ui.indexPattern.warningText', {
+        defaultMessage: 'Instead of querying indices only matching the pattern {title}, you are now querying all indices ' +
+          'matching {index}. This index pattern should be migrated to a wildcard-based index pattern. To do so, edit the ' +
+          'index pattern in Management.',


UX nit: This message is hard to read before the notification disappears. I wonder if we could wordsmith it to make things shorter, e.g.

Support for time interval index patterns removed Currently querying all indices matching {index}. {title} should be migrated to a wildcard-based index pattern. Edit >

lukeelmers · 2019-04-23T23:35:47Z

src/legacy/ui/public/index_patterns/_index_pattern.js

+              <EuiFlexItem grow={false}>
+                <EuiButton size="s" href={kbnUrl.getRouteHref(indexPattern, 'edit')}>
+                  Edit index pattern
+                </EuiButton>


UX nit: Maybe this is something to add later, but I wonder if we should be integrating with feature controls here. That way we could hide the edit button if we know a user doesn't have access to management, rather than allowing them to click a button that will lead straight to an error page.

elasticmachine · 2019-04-23T23:42:41Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-25T21:04:46Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

…ration tooling

elasticmachine · 2019-05-01T21:00:37Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

alexwizp · 2019-05-02T17:23:54Z

src/legacy/ui/public/index_patterns/_index_pattern.js

+      }).then(({ attributes: { title, intervalName } }) => {
+        this.title = title;
+        this.intervalName = intervalName;
+      }).then(() => this);


Maybe it's a 'nip' from my side but I don't understand why we need it .then(() => this);
We can just return this from the previous then.

It may look a little worse, but we are not planning an additional Microtask.

Yeah, that's totally true. I think I was just following the same style as above, but it could definitely be changed.

lukasolson added Feature:Data Views Data Views code and UI - index patterns before 8.0 :AppArch v7.2.0 v7.0.1 labels Apr 17, 2019

lukasolson self-assigned this Apr 17, 2019

lukasolson added the release_note:breaking label Apr 18, 2019

lukasolson force-pushed the removeIntervalIndexPatterns7.x branch from 36ef2ca to 53d6548 Compare April 18, 2019 18:27

lukasolson requested review from lukeelmers and Bargs April 23, 2019 20:46

lukasolson removed the v7.0.1 label Apr 23, 2019

lukasolson force-pushed the removeIntervalIndexPatterns7.x branch from 7e1d940 to a65217c Compare April 23, 2019 22:20

Bargs approved these changes Apr 23, 2019

View reviewed changes

lukeelmers approved these changes Apr 23, 2019

View reviewed changes

lukasolson changed the base branch from master to 7.x May 1, 2019 19:45

lukasolson changed the base branch from 7.x to master May 1, 2019 19:45

Remove support for time-based interval index patterns but provide mig…

ef3b30d

…ration tooling

lukasolson added 9 commits May 1, 2019 12:54

Remove other unused translations

42db8f3

Fix setting index

7eae353

Fix index pattern stub

02d1e90

Check if index is undefined

994a687

Fix tests

8298816

Fix warning toast

8952913

Don't allow saving an empty title

b9dbf82

Add tests for getIndex

cae1f06

i18n-ify and simplify text

82f085f

lukasolson force-pushed the removeIntervalIndexPatterns7.x branch from cf4f550 to 82f085f Compare May 1, 2019 19:54

lukasolson requested review from a team as code owners May 1, 2019 19:54

lukasolson requested a review from a team May 1, 2019 19:54

lukasolson changed the base branch from master to 7.x May 1, 2019 19:54

alexwizp reviewed May 2, 2019

View reviewed changes

lukasolson merged commit 5c4ae11 into elastic:7.x May 2, 2019

lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Aug 1, 2019

Merge changes with updates from elastic#35262.

d50afb3

lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Aug 2, 2019

Merge changes with updates from elastic#35262.

dd5598e

lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Aug 2, 2019

Merge changes with updates from elastic#35262.

16f040b

lukeelmers added a commit to lukeelmers/kibana that referenced this pull request Aug 20, 2019

Updates to ensure changes from elastic#35262 are not wiped out.

3f131c4

lukeelmers mentioned this pull request Aug 20, 2019

[7.x] Move ui/index_patterns code to data plugin. (#42994) #43546

Merged

lukasolson deleted the removeIntervalIndexPatterns7.x branch December 2, 2019 22:35

lukasolson mentioned this pull request Feb 20, 2020

Remove unused indexPattern:fieldMapping:lookBack advanced setting #58147

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove support for time-based interval index patterns with migration #35262

Remove support for time-based interval index patterns with migration #35262

lukasolson commented Apr 17, 2019 •

edited

elasticmachine commented Apr 17, 2019

elasticmachine commented Apr 17, 2019

AlonaNadler commented Apr 17, 2019

gchaps commented Apr 17, 2019 •

edited

lukasolson commented Apr 18, 2019

elasticmachine commented Apr 18, 2019

elasticmachine commented Apr 18, 2019

elasticmachine commented Apr 18, 2019

elasticmachine commented Apr 19, 2019

elasticmachine commented Apr 23, 2019

elasticmachine commented Apr 23, 2019

Bargs left a comment

Bargs Apr 23, 2019

lukasolson Apr 23, 2019

elasticmachine commented Apr 23, 2019

lukeelmers left a comment

lukeelmers Apr 23, 2019

lukeelmers Apr 23, 2019

elasticmachine commented Apr 23, 2019

elasticmachine commented Apr 25, 2019

elasticmachine commented May 1, 2019

alexwizp May 2, 2019

lukasolson May 2, 2019

Remove support for time-based interval index patterns with migration #35262

Remove support for time-based interval index patterns with migration #35262

Conversation

lukasolson commented Apr 17, 2019 • edited

elasticmachine commented Apr 17, 2019

elasticmachine commented Apr 17, 2019

💔 Build Failed

AlonaNadler commented Apr 17, 2019

gchaps commented Apr 17, 2019 • edited

Warning 1

Warning 2

Page text

lukasolson commented Apr 18, 2019

elasticmachine commented Apr 18, 2019

💔 Build Failed

elasticmachine commented Apr 18, 2019

💔 Build Failed

elasticmachine commented Apr 18, 2019

💔 Build Failed

elasticmachine commented Apr 19, 2019

💔 Build Failed

elasticmachine commented Apr 23, 2019

💚 Build Succeeded

elasticmachine commented Apr 23, 2019

💔 Build Failed

Bargs left a comment

Choose a reason for hiding this comment

Bargs Apr 23, 2019

Choose a reason for hiding this comment

lukasolson Apr 23, 2019

Choose a reason for hiding this comment

elasticmachine commented Apr 23, 2019

💚 Build Succeeded

lukeelmers left a comment

Choose a reason for hiding this comment

lukeelmers Apr 23, 2019

Choose a reason for hiding this comment

lukeelmers Apr 23, 2019

Choose a reason for hiding this comment

elasticmachine commented Apr 23, 2019

💚 Build Succeeded

elasticmachine commented Apr 25, 2019

💚 Build Succeeded

elasticmachine commented May 1, 2019

💚 Build Succeeded

alexwizp May 2, 2019

Choose a reason for hiding this comment

lukasolson May 2, 2019

Choose a reason for hiding this comment

lukasolson commented Apr 17, 2019 •

edited

gchaps commented Apr 17, 2019 •

edited