Backfill tabs for Discover Session saved object

[lukasolson]

Summary

Part of #219268.

Adds tabs field + sync code: Adds a new (optional) tabs field. Implements code to sync the fields between the existing top-level attributes and the new tabs field. (In other words, the tabs array will contain a single item that has the same data that is in the top-level attributes with the exception of title and description.) Uses a data_backfill function to eagerly migrate existing saved objects to populate this field. After this PR is merged, wait for the next serverless release before merging the next steps.

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

• 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
• If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
• This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
• Flaky Test Runner was used on any tests changed
• The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines
• Review the backport guidelines and apply applicable backport:* labels.

Identify risks

Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging.

• See some risk examples
• ...

[elasticmachine]

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

[davismcphee]

Code changes look good to me, and it seemed to work well overall when testing.

An existing Discover session created in main was correctly updated with the tabs array when opening it in this PR branch, and a new Discover session created in this branch included the tabs array as expected. Making changes in either case and saving caused the first tabs entry to be synced with the root attributes as expected.

I then switched back to main and was able to open, modify, and save both of the Discover sessions without issues. After doing this and again switching back to the PR branch, the modifications made in main were again synced to the first tabs array entry.

The only thing I noticed that I'm unsure about is the handling of by value Discover session panels in dashboards. I left some more details in a comment below.

[davismcphee]

I did some testing with Discover session dashboard panels and noticed this situation:

• Created a dashboard in main with a by reference Discover session panel, then duplicated it which created a separate by value panel. Saved the dashboard with both panels.
• Switched to this PR branch and opened the dashboard. Duplicated the existing by value panel, leaving me with one by reference panel and two by value panels. Saved the dashboard again.
• Navigated to saved objects management and inspected the dashboard. The newly duplicated by value panel was updated with a tabs array, but the existing by value panel created in main was not.

So it looks like existing by value panels will not get the tabs array, but new ones will. Does this seem ok for now, or could it cause issues for the followup steps we have planned?

[afharo]

nit: you should also test export/imports 😇

[afharo]

In any case: for the by values use case. Is there anything that we can do in Core to help migrate those?

It sounds like we should apply transforms to dashboards whenever an embeddable declares transforms.

Maybe we can work around it by defining a model version bump of type data_backfill that reapplies the same logic that's currently applied when re-saving the dashboard?

[davismcphee]

@afharo Good catch and suggestion. Tested again today and ensured import/export works as expected:

• Created a Discover session and a dashboard with a by reference and by value Discover panel in main. Exported both SOs.
• Switched to the PR branch and imported the Discover session and dashboard. Confirmed both imported as expected and were able to be opened without issues. Similar situation as above where the Discover session was updated with a tabs array, but the by value panel wasn't.
• Modified both in the PR branch and re-exported/imported, confirming there were still no issues.

Is there anything that we can do in Core to help migrate those?

Regarding the dashboard migration piece, I'm not really sure tbh. I'm under the impression the way panel maintainers are supposed to support backward compatibility currently is through the client side serialize/deserialize logic since there isn't real support for panel transforms yet. But I think there may be plans for something more complete on the Dashboard side. Maybe someone from @elastic/kibana-presentation can give some guidance?

[nreese]

embeddable transforms registry merged last week. This allows embeddables to register a function to transform embeddable state as part of dashboard CRUD read request prior to sending dashboard response.

This is part of dashboards as code project to allow embeddables to 1) inject references on read so that responses do not contain references. 2) transform embeddable state on read to provide better formated by-value state. Then on write, the transforms provide a way for embeddables to extract references prior to saving dashboard saved object.

Embeddable transforms can make any state changes so you could add tabs support with transforms.

[davismcphee]

Thanks for the info @nreese, great to know. @lukasolson and I chatted about this and we're going to try to use it in this PR to take care of the by value panels.

[davismcphee]

Tested these scenarios again and confirmed both are now working as expected: existing by value Discover session panels are now correctly updated with the tabs array on read, and the tabs array is persisted to the dashboard SO on save.

[davismcphee]

I assume at some point we'll ned to stabilize these IDs, but it doesn't really matter for now since we're not actually using the tabs yet?

[lukasolson]

Yes, it would definitely be good to stabilize these once we're using them.

[nreese]

Make sure to register transforms in public as well so that short link URL state gets transformed.

[davismcphee]

Thanks for the heads up 👍

[lukasolson]

Yes, thank you! Addressed in 3f6431b.

[davismcphee]

Thanks for the updates! Tested again after the latest changes and LGTM 👍 Seems like we just need to make one more minor change to run the same panel transform on the client, and this should be good to go.

[davismcphee]

Thanks for the heads up 👍

[davismcphee]

Tested these scenarios again and confirmed both are now working as expected: existing by value Discover session panels are now correctly updated with the tabs array on read, and the tabs array is persisted to the dashboard SO on save.

[davismcphee]

Code-only review, just a final check with the latest updates, LGTM 👍

[davismcphee]

Suggested change

	const { searchEmbeddableTransforms } = await import('../common/embeddable');
	const { searchEmbeddableTransforms } = await getEmbeddableServices();

Nit: this could probably be grouped with the embeddable_services bundle to limit async module imports:

kibana/src/platform/plugins/shared/discover/public/plugin.tsx

Line 458 in 9c18356

const getEmbeddableServices = () => import('./plugin_imports/embeddable_services');

[lukasolson]

/ci

[lukasolson]

/ci

[kertal]

/ci

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 2c9ba35

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
discover	1432	1434	+2
savedSearch	23	24	+1
total			+3

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
savedSearch	62	65	+3

Async chunks

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

id	before	after	diff
discover	1.1MB	1.1MB	+161.0B

Page load bundle

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

id	before	after	diff
discover	22.6KB	22.7KB	+106.0B
savedSearch	11.2KB	11.6KB	+326.0B
total			+432.0B

Unknown metric groups

API count

id	before	after	diff
savedSearch	63	68	+5

History

• 💔 Build #323301 failed 4cf4120
• 💔 Build #323143 failed fd0f1ad
• 💔 Build #323120 failed fd0f1ad
• 💔 Build #323096 failed a7db027
• 💔 Build #323065 failed 3f8913d

cc @lukasolson

[ThomThomson]

Copied changes from #228511 to make the transforms registry accessible LGTM! Nice work!