Fix visualizations for rollups using fixed_interval or calendar_interval #39537

wylieconlon · 2019-06-24T20:54:30Z

Summary

Visualizations on rollup index patterns were failing due to improper use of the interval value. Fixes #36295

Steps to reproduce the bug:

Create a rollup with a time interval such as 5m or 24h. These kinds of rollups are stored as fixed_interval in the index pattern.
Create another rollup with a time interval such as 1h. This is stored as a calendar_interval in the index pattern.
Wait for the rollup data to come in
Go to visualize and create a Date Histogram aggregation for the new rollup index pattern, for both.
You should be able to select any interval that is a multiple of the rollup interval. For a fixed interval like 5m, it will multiply to 10m, 15m, etc. For a calendar interval like 1h, it will multiply to 1d, 1M, etc.

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-06-24T20:54:32Z

Pinging @elastic/kibana-app

wylieconlon · 2019-06-24T21:06:26Z

x-pack/legacy/plugins/rollup/public/visualize/editor_config.js

@@ -56,7 +56,7 @@ export function initEditorConfig() {

    // Set date histogram time zone based on rollup capabilities
    if (aggTypeName === 'date_histogram') {
-      const interval = fieldAgg.interval;
+      const interval = fieldAgg.fixed_interval ? fieldAgg.fixed_interval : fieldAgg.interval;


Should this actually use the logic fixed_interval || calendar_interval || interval? #36306

Yes, it should...

elasticmachine · 2019-06-24T21:56:36Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

wylieconlon · 2019-06-25T19:12:51Z

@timroes I'd appreciate any suggestion on who to reach out to for review

wylieconlon · 2019-06-26T20:12:44Z

Need to update this because rollups are also using calendar_interval #36310

elasticmachine · 2019-06-26T21:31:28Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

jen-huang

Code LGTM! I tested locally to make sure that both calendar and fixed rollup indices allow the correct interval inputs, their requests are sent correctly with either calendar_interval or fixed_interval, and that their visualizations render.

timroes · 2019-06-27T07:50:58Z

@jen-huang out of curiosity and so we can track this: Since 7.3 rollup jobs created by Kibana will have a calendar_interval and fixed_interval instead of an interval? But jobs created beforehand, or even if creating a job manually until 8.0 it could still use interval? With 8.0, will that be removed, will it only be removed to create new jobs with interval or can we be sure no job has interval in it anymore?

cc @alexwizp

elasticmachine · 2019-06-27T08:53:32Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

…ixed-interval

jen-huang · 2019-06-27T17:59:22Z

@timroes I checked with Zack (@polyfractal) and he will look into whether we will leave interval deprecated for longer than 8.0 (which was the original goal). He will leave a reply after investigating.

polyfractal · 2019-06-28T14:12:20Z

If you don't need an answer immediately, I'm going to bring this up at the weekly meeting and collect some feedback. The original plan was indeed 8.0... but considering the scope of the change (everyone who uses a date_histo, kibana or otherwise) it might be worth leaving this deprecated for a long time.

That said, we can probably "upgrade" rollup configs to fixed/calendar automatically. Internally it already upgrades to the correct type when it encounters a legacy interval, so we could easily just persist that (either when the job starts, or perhaps as an upgrade migration helper) So even if the date histo has a long deprecation period, we can probably simplify rollup since we are in full control.

wylieconlon · 2019-06-28T14:34:36Z

@polyfractal @jen-huang Based on that feedback, it seems like the rollup jobs could still contain interval until we do the automatic migration you mentioned. Assuming I'm reading that correctly I'd like to get this merged and backported for 7.3 with fallback to interval, but please let me know if I should make a change.

timroes · 2019-06-28T14:38:38Z

@wylieconlon I don't think there are any changes needed given that information. so feel free to merge it.

polyfractal · 2019-06-28T14:42:54Z

it seems like the rollup jobs could still contain interval until we do the automatic migration you mentioned

Yeah that's correct, jobs that were created pre-7.2 will still have interval in their config (both in the cluster state used by the running job, and in the _meta stored with the data)

elasticmachine · 2019-06-28T15:11:31Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

…nterval

elasticmachine · 2019-06-28T16:51:12Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

wylieconlon · 2019-06-28T17:03:15Z

Jenkins, test this

elasticmachine · 2019-06-28T18:13:43Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

wylieconlon · 2019-06-28T18:21:47Z

Jenkins, test this. More flakiness

elasticmachine · 2019-06-28T19:26:28Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

…val (elastic#39537) * Fix visualizations for rollups using fixed_interval * Update to support calendar_interval as well

…val (#39537) (#39941) * Fix visualizations for rollups using fixed_interval * Update to support calendar_interval as well

…val (#39537) (#39942) * Fix visualizations for rollups using fixed_interval * Update to support calendar_interval as well

Fix visualizations for rollups using fixed_interval

32b4e81

wylieconlon requested review from cjcenizal and jen-huang June 24, 2019 20:54

cjcenizal removed their request for review June 24, 2019 20:56

wylieconlon requested a review from timroes June 24, 2019 20:59

wylieconlon commented Jun 24, 2019

View reviewed changes

Update to support calendar_interval as well

94e05b7

wylieconlon changed the title ~~Fix visualizations for rollups using fixed_interval~~ Fix visualizations for rollups using fixed_interval or calendar_interval Jun 26, 2019

jen-huang approved these changes Jun 26, 2019

View reviewed changes

timroes closed this Jun 27, 2019

timroes reopened this Jun 27, 2019

Merge branch 'master' of github.com:elastic/kibana into fix-rollups-f…

0f1e62c

…ixed-interval

Merge remote-tracking branch 'origin/master' into fix-rollups-fixed-i…

3fea8c9

…nterval

wylieconlon merged commit ad04e54 into elastic:master Jun 28, 2019

wylieconlon deleted the fix-rollups-fixed-interval branch June 28, 2019 19:48

This was referenced Jun 28, 2019

[7.x] Fix visualizations for rollups using fixed_interval or calendar_interval (#39537) #39941

Merged

[7.2] Fix visualizations for rollups using fixed_interval or calendar_interval (#39537) #39942

Merged

wylieconlon pushed a commit that referenced this pull request Jun 28, 2019

Fix visualizations for rollups using fixed_interval or calendar_inter…

2c5ba49

…val (#39537) (#39941) * Fix visualizations for rollups using fixed_interval * Update to support calendar_interval as well

wylieconlon pushed a commit that referenced this pull request Jun 28, 2019

Fix visualizations for rollups using fixed_interval or calendar_inter…

65083b1

…val (#39537) (#39942) * Fix visualizations for rollups using fixed_interval * Update to support calendar_interval as well

timroes added the v7.2.1 label Jun 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix visualizations for rollups using fixed_interval or calendar_interval #39537

Fix visualizations for rollups using fixed_interval or calendar_interval #39537

wylieconlon commented Jun 24, 2019 •

edited

Loading

elasticmachine commented Jun 24, 2019

wylieconlon Jun 24, 2019

wylieconlon Jun 26, 2019

elasticmachine commented Jun 24, 2019

wylieconlon commented Jun 25, 2019

wylieconlon commented Jun 26, 2019

elasticmachine commented Jun 26, 2019

jen-huang left a comment

timroes commented Jun 27, 2019 •

edited

Loading

elasticmachine commented Jun 27, 2019

jen-huang commented Jun 27, 2019

polyfractal commented Jun 28, 2019

wylieconlon commented Jun 28, 2019

timroes commented Jun 28, 2019

polyfractal commented Jun 28, 2019

elasticmachine commented Jun 28, 2019

elasticmachine commented Jun 28, 2019

wylieconlon commented Jun 28, 2019

elasticmachine commented Jun 28, 2019

wylieconlon commented Jun 28, 2019

elasticmachine commented Jun 28, 2019

Fix visualizations for rollups using fixed_interval or calendar_interval #39537

Fix visualizations for rollups using fixed_interval or calendar_interval #39537

Conversation

wylieconlon commented Jun 24, 2019 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Jun 24, 2019

wylieconlon Jun 24, 2019

Choose a reason for hiding this comment

wylieconlon Jun 26, 2019

Choose a reason for hiding this comment

elasticmachine commented Jun 24, 2019

💚 Build Succeeded

wylieconlon commented Jun 25, 2019

wylieconlon commented Jun 26, 2019

elasticmachine commented Jun 26, 2019

💔 Build Failed

jen-huang left a comment

Choose a reason for hiding this comment

timroes commented Jun 27, 2019 • edited Loading

elasticmachine commented Jun 27, 2019

💚 Build Succeeded

jen-huang commented Jun 27, 2019

polyfractal commented Jun 28, 2019

wylieconlon commented Jun 28, 2019

timroes commented Jun 28, 2019

polyfractal commented Jun 28, 2019

elasticmachine commented Jun 28, 2019

💔 Build Failed

elasticmachine commented Jun 28, 2019

💔 Build Failed

wylieconlon commented Jun 28, 2019

elasticmachine commented Jun 28, 2019

💔 Build Failed

wylieconlon commented Jun 28, 2019

elasticmachine commented Jun 28, 2019

💚 Build Succeeded

wylieconlon commented Jun 24, 2019 •

edited

Loading

timroes commented Jun 27, 2019 •

edited

Loading