[TSVB] Runs the migration correctly for the drop last bucket #121734

stratoula · 2021-12-21T10:17:22Z

Summary

The problem

On 7.14 we changed the default behavior of the Drop last bucket setting from Yes to No. We also created another PR that adds a migration script (for < 7.14.0 SOs) #110782. What this migration does:

Checks if the global drop_last_bucket setting is set. If no, it adds the property with 1 (Drop last bucket ? Yes)
Checks if the chart uses the override dataview setting, and changes the series_drop_last_bucket to 1

This works fine for all charts / dashboards that migrate from a version prior to 7.14.0 (and for the metricbeats new installations)

In 7.15, beats changed their visualizations SOs (for example here https://github.com/elastic/beats/blob/master/metricbeat/module/system/_meta/kibana/7/visualization/83e12df0-1b91-11e7-bec4-a5e9ec5cab8b-ecs.json) and the new migration version became v7.14.0. This means that the above migration doesn't run for the newly created metricbeats dashboards (7.15++)

The fix

I created another migration (7.17). This is not the same migration that runs for 7.14.0 because we can't use it unfortunately if the override index pattern setting is used. This migration checks if the global drop_last_bucket setting is set. If no, it adds the property with 1 (Drop last bucket ? Yes)

This is safe as the drop_last_bucket is saved with the SO regardless the value from 7.14++. This means:

For a prior to 7.14 viz, the first migration will run and add the value if it doesn't exist.
For viz created for 7.14 and above, this value is saved in the SO and the second migration will run but won't affect the SO as the property exists.

Unfortunately this is not safe for the visualizations that are using the override index pattern setting and have been created for 7.14 +. The reason is that the if the default value of series_drop_last_bucket (default is No), this is not saved in the SO. What it means if I run the migration:

For a prior to 7.14 viz, the first migration will run and add the value if it doesn't exist. Then the second migration will run but the property has already be set so nothing will change to the SO. Everything good so far.
BUT for a viz that was created on 7.14++ that uses the override index pattern and uses the default value (drop last bucket No), if I run the migration that created on 7.14.0, it will set this value to Yes but this is not what the user wants. The user used the default value which is No.
The above cases can't be solved by us unfortunately. I think that they should be changed on the beats side. As far as I can see they are 4 beats charts that use the 'override_index_pattern : 1' property https://github.com/elastic/beats/search?p=1&q=override_index_pattern. I think that we should update them with adding series_drop_last_bucket : 1

In order this to never happen again, this PR except from adding the migration, also adds the default values for the override_index_pattern and series_drop_last_bucket for a newly created TSVB chart. With this change, the default values will always be stored in the SO.

How to test

Install the metricbeat dashboards and check the gauges visualizations. The drop_last_bucket setting should be set to Yes
v8.1 newly installed metricbeat dashboard

and the dashboard

Create a prior to 7.14 viz with different values of the drop last bucket and migrate it locally. Check the viz that they are working as expected. Test by value and by ref charts.
Do the same for a 7.14++ viz. Test by value and by ref charts.

Checklist

Unit or functional tests were updated or added to match the most common scenarios

stratoula · 2021-12-21T14:46:23Z

@flash1293 as I describe on the PR, I can partially fix it. The migration can run safely for the global option

but not for the series option (when the override data view is set to true)

I think that this should be fixed on the beats side. https://github.com/elastic/beats/search?p=2&q=override_index_pattern There are only 4 visualizations as far as I can see on the repo that need to be updated. Wdyt?
(From the results above, I have excluded the charts that have the override_index_pattern set to 0 and the ones that set the series_drop_last_bucket .

elasticmachine · 2021-12-21T18:04:27Z

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

alexwizp

LGTM!

dej611

Tested locally with Metricbeat dashboard and importing SOs.

dej611 · 2021-12-22T15:05:43Z

src/plugins/visualizations/server/migrations/visualization_common_migrations.ts

+      params: {
+        ...visState.params,
+        drop_last_bucket: visState.params.drop_last_bucket ?? 1,
+      },


nit: I think that if you invert the result is the same:

Suggested change

params: {

...visState.params,

drop_last_bucket: visState.params.drop_last_bucket ?? 1,

},

params: {

drop_last_bucket: 1,

...visState.params

},

stratoula · 2021-12-23T07:00:34Z

@elasticmachine merge upstream

kibana-ci · 2021-12-23T08:21:11Z

💛 Build succeeded, but was flaky

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`visTypeTimeseries`	447.6KB	447.6KB	+51.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`visTypeTimeseries`	14.6KB	14.7KB	+51.0B

History

💚 Build #14518 succeeded ff142e5
💚 Build #14464 succeeded f09172d

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…#121734) * [TSVB] Runs the migration correctly for the drop last bucket * [TSVB] Run the migration for drop_last_bucket Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

kibanamachine · 2021-12-23T08:27:49Z

💔 Backport failed

Status	Branch	Result
✅	8.0
❌	7.17	Commit could not be cherrypicked due to conflicts

Successful backport PRs will be merged automatically after passing CI.

To backport manually run:
node scripts/backport --pr 121734

…#121734) * [TSVB] Runs the migration correctly for the drop last bucket * [TSVB] Run the migration for drop_last_bucket Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # src/plugins/visualizations/server/embeddable/visualize_embeddable_factory.ts # src/plugins/visualizations/server/migrations/visualization_saved_object_migrations.test.ts # src/plugins/visualizations/server/migrations/visualization_saved_object_migrations.ts

#121939) * [TSVB] Runs the migration correctly for the drop last bucket * [TSVB] Run the migration for drop_last_bucket Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>

kibanamachine · 2021-12-27T08:33:04Z

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

kibanamachine · 2021-12-28T08:43:04Z

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

kibanamachine · 2021-12-29T08:48:45Z

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

…121734) (#121940) * [TSVB] Runs the migration correctly for the drop last bucket (#121734) * [TSVB] Runs the migration correctly for the drop last bucket * [TSVB] Run the migration for drop_last_bucket Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # src/plugins/visualizations/server/embeddable/visualize_embeddable_factory.ts # src/plugins/visualizations/server/migrations/visualization_saved_object_migrations.test.ts # src/plugins/visualizations/server/migrations/visualization_saved_object_migrations.ts * Commit test * Fix test Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

stratoula added 2 commits December 21, 2021 12:04

[TSVB] Runs the migration correctly for the drop last bucket

f09172d

[TSVB] Run the migration for drop_last_bucket

ff142e5

stratoula added Feature:TSVB TSVB (Time Series Visual Builder) release_note:fix v7.17.0 v8.0.0 v8.1.0 Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Dec 21, 2021

stratoula marked this pull request as ready for review December 21, 2021 18:04

stratoula requested a review from a team as a code owner December 21, 2021 18:04

alexwizp approved these changes Dec 22, 2021

View reviewed changes

dej611 approved these changes Dec 22, 2021

View reviewed changes

stratoula added the auto-backport Deprecated - use backport:version if exact versions are needed label Dec 23, 2021

Merge branch 'main' into fix-drop-last-bucket-migration

24cb15b

stratoula merged commit b4c2d2c into elastic:main Dec 23, 2021

kibanamachine mentioned this pull request Dec 23, 2021

[8.0] [TSVB] Runs the migration correctly for the drop last bucket (#121734) #121939

Merged

stratoula mentioned this pull request Dec 23, 2021

[7.17] [TSVB] Runs the migration correctly for the drop last bucket (#121734) #121940

Merged

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 27, 2021

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 29, 2021

hendry-lim mentioned this pull request Jan 4, 2022

[System] Issue with short time frame (5min) elastic/integrations#1437

Closed

crespocarlos mentioned this pull request Jun 23, 2022

[Metrics UI] Host overview shows 0 on individual metrics #135005

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TSVB] Runs the migration correctly for the drop last bucket #121734

[TSVB] Runs the migration correctly for the drop last bucket #121734

stratoula commented Dec 21, 2021 •

edited

Loading

stratoula commented Dec 21, 2021 •

edited

Loading

elasticmachine commented Dec 21, 2021

alexwizp left a comment

dej611 left a comment

dej611 Dec 22, 2021

stratoula commented Dec 23, 2021

kibana-ci commented Dec 23, 2021

kibanamachine commented Dec 23, 2021

kibanamachine commented Dec 27, 2021

kibanamachine commented Dec 28, 2021

kibanamachine commented Dec 29, 2021

[TSVB] Runs the migration correctly for the drop last bucket #121734

[TSVB] Runs the migration correctly for the drop last bucket #121734

Conversation

stratoula commented Dec 21, 2021 • edited Loading

Summary

The problem

The fix

How to test

Checklist

stratoula commented Dec 21, 2021 • edited Loading

elasticmachine commented Dec 21, 2021

alexwizp left a comment

Choose a reason for hiding this comment

dej611 left a comment

Choose a reason for hiding this comment

dej611 Dec 22, 2021

Choose a reason for hiding this comment

stratoula commented Dec 23, 2021

kibana-ci commented Dec 23, 2021

💛 Build succeeded, but was flaky

Metrics [docs]

Async chunks

Page load bundle

History

kibanamachine commented Dec 23, 2021

💔 Backport failed

kibanamachine commented Dec 27, 2021

kibanamachine commented Dec 28, 2021

kibanamachine commented Dec 29, 2021

stratoula commented Dec 21, 2021 •

edited

Loading

stratoula commented Dec 21, 2021 •

edited

Loading