Differentiate load-by-reference for embeddables

[jportner]

Overview

Embeddables (visualizations) can be loaded in two ways:

1. By value - using the target embeddable's attributes directly
2. By reference - using the target embeddable's saved object type and id

An example:
The primary consumer of embeddables today is the dashboard plugin. When you create a new dashboard, then you add a panel, it is a wrapper for an embeddable. You can add this by value (in which case, the embeddable's attributes are persisted in the dashboard's panelJson field), or you can add this by reference (in which case, the visualization is persisted as a separate saved object, and the object's type/id are added to the dashboard's references field.

If an embeddable is loaded by reference, the consumer (the dashboard plugin in this case) passes the reference to the embeddable, and the embeddable is responsible for fetching the saved object in question.

In the 8.0 release, we are implementing a breaking change that will regenerate all object IDs (#100489). Any saved object that can be loaded through a "deep link" URL must be retrieved using the SavedObjectsClient.resolve API.
When considering embeddables, there is another dimension to this: dashboards themselves can either be persisted as a saved object in Elasticsearch or as a "snapshot URL".

That means we have four different scenarios when loading embeddables:

1. Saved dashboard with embeddable by value
2. Snapshot URL dashboard with embeddable by value
3. Saved dashboard with embeddable by reference - currently retrieved with SavedObjectsClient.get
4. Snapshot URL dashboard with embeddable by reference - currently retrieved with SavedObjectsClient.get

Regarding the breaking change: only scenario (4) needs to be retrieved with SavedObjectsClient.resolve. However,
However, while implementing #110059 I discovered that embeddables can' tell the difference between scenarios (3) and (4) above. So in that PR, all embedded Lens visualizations loaded by reference were changed to be retrieved with SavedObjectsClient.resolve.

What's the problem?

As mentioned in the Sharing Saved Objects developer guide (FAQ 6), saved objects should only retrieved with SavedObjectsClient.resolve when loaded through a deep link URL. There are several reasons for this:

1. resolve is less performant
2. resolve is a temporary stop-gap to get us over this conversion hump without breaking existing deep link URLs, but we eventually want to get rid of it. To that end, we are trying to minimize usage of resolve, and we are collecting usage data on how often it's used / what the different outcomes are.
3. We want to measure how often users are running into aliasMatch and conflict outcomes versus the exactMatch outcome; this usage data gets diluted if plugins call resolve in a situation where the outcome will always be `exactMatch1.
4. Conversely, consumers can run into undesired aliasMatch and conflict outcomes more often than necessary if they use resolve in non-deep-link situations.

TL;DR

To minimize the usage of resolve, we need for embeddables to be able to differentiate between scenarios (3) and (4) above, so they can go back to using SavedObjectsClient.get for scenario (3).

This is not required for the 8.0 release, but would preferably be done in the early 8.x timeframe.

I expect this change will need to be made by @elastic/kibana-app-services, and then consumers of embeddables will need to change their implementation accordingly.

[elasticmachine]

Pinging @elastic/kibana-security (Team:Security)

[elasticmachine]

Pinging @elastic/kibana-presentation (Team:Presentation)

[vadimkibana]

IMO, embeddables should not know anything about Saved Objects or any other objects or any other way of loading the data. It should be up to each embeddable type to implement (or not) any persistence mechanism.

Ideally we would pull out any mentions of "saved objects" out of the Embeddable plugin. And then it is up to each embeddable where its data is stored.

[ThomThomson]

As Vadim said, there should be no concept of 'by value' or 'by reference' on the Embeddable API side, nor should there be any concept of 'loaded via saved object' vs 'loaded through snapshot URL' there.

That said, I think we can still differentiate between Scenario 3 and Scenario 4 on a case by case basis for each embeddable that loads a saved object.

One way to do this would be by interfacing with the Dashboard Container before loading the saved object:

1. When loading a dashboard, store whether an ID was passed in or not in a variable.
2. Pass that variable to the dashboard container through its input.
3. Write a method on the dashboard container that accesses that variable: dashboardContainer.isLoadedFromId()
4. In each embeddable type (lens, maps, visualize etc.) before loading the saved object use this kind of psuedocode

// if the embeddable is by reference...
if (this.getRoot().isContainer && 
    this.getRoot().type !== DASHBOARD_CONTAINER_TYPE &&
    !(this.getRoot() as DashboardContainer).isLoadedFromId()) {
    // load saved object via SavedObjectsClient.resolve
} else {
    // load saved object via SavedObjectsClient.get
}

[jportner]

As Vadim said, there should be no concept of 'by value' or 'by reference' on the Embeddable API side, nor should there be any concept of 'loaded via saved object' vs 'loaded through snapshot URL' there.

Sure, apologies if the issue description came off as prescriptive, I just meant that we should find a way to differentiate 👍

That said, I think we can still differentiate between Scenario 3 and Scenario 4 on a case by case basis for each embeddable that loads a saved object.

One way to do this would be by interfacing with the Dashboard Container before loading the saved object:

Nice idea!
From what I gather, from the container side of things, this is a dashboard-only problem, right?

[ThomThomson]

For now, this is only a dashboard problem yes! Canvas cannot be shared as a snapshot in the same way.

[ppisljar]

I have a feeling that this issue is wrongly stated, or i am not understanding it well.

1. as Vadim and Devon already stated embeddables have no concept of saved objects, and embeddables API does no loading of saved objects as far as i am aware. This is completely up to specific embeddable implementations on how they want to work.
2. its not clear about loading which saved objects this is all about ? is it about loading dashboard saved objects ? specific by reference embeddable saved objects ? or other saved objects that might be referenced deeply in any of the states ? (as index patterns mentioned inside filters etc)
3. you mention that by reference embeddables are fetched with savedObjects.get .... that is not the case, or maybe it is but it depends on embeddable and should be tracked as such imo. theoretically embeddables are free to load their state from anywhere they want (nothing is forcing them to use saved objects)
4. I don't think snapshot urls are the biggest problem as they are migratable (means we can also migrate old ids to new ones). the bigger problem are regular URLs (something user just bookmars or copy pastes from address bar). as we have no way to migrate those. (there is no plan for now to write migrations for snapshot urls as far as i am aware)

[ppisljar]

To me it looks like we need a general solution (not something embeddable, or worse, dashboard specific) that would allow us to identify whenever we are loading the saved objects if the object Ids are old or new.

Application should theoretically have that knowledge. So we could create a very simple service that every app can then use:

   core.compatibility.setLegacyIDMode(true);

and then internally in the savedObjectsClient.get we could do a check against this and figure our if we want to resolve or not. Saved objects client could internally also log all the partial matches to the service, and app could fetch this and notify the user about it.

[jportner]

1. as Vadim and Devon already stated embeddables have no concept of saved objects, and embeddables API does no loading of saved objects as far as i am aware. This is completely up to specific embeddable implementations on how they want to work.

As mentioned above, I noticed this behavior in the PR that changed Lens to use the resolve API (#110059).

Lens creates an Embeddable class (implementing AbstractEmbeddable) https://github.com/elastic/kibana/blob/90045710e54257a74faa7ee821ffdafaca31172b/x-pack/plugins/lens/public/embeddable/embeddable.tsx
The consumer passes the LensEmbeddableInput into the constructor (that type is the union LensByValueInput | LensByReferenceInput), and then the constructor calls the initializeSavedVis function which ultimately makes the API call to resolve the lens saved object, in the case the lens is being loaded by reference.

So, it seems pretty clear that the Lens embeddable is fetching the Lens saved object. Am I missing something?

2. its not clear about loading which saved objects this is all about ? is it about loading dashboard saved objects ? specific by reference embeddable saved objects ? or other saved objects that might be referenced deeply in any of the states ? (as index patterns mentioned inside filters etc)

Let me try to phrase, then! An Embeddable can be loaded by reference or by value. If it is loaded by reference, and it fetches the saved object, it really needs to know where the reference ID came from. Was it from a saved object (e.g., a dashboard saved object)? Or was it from a URL (e.g., a dashboard snapshot URL)?
If the reference ID came from a saved object, it is definitely up-to-date and the Embeddable can get the referenced saved object.
If the reference ID came from a URL, it might be an old URL, and the Embeddable needs to resolve the referenced saved object.

Does that make sense?

3. you mention that by reference embeddables are fetched with savedObjects.get .... that is not the case, or maybe it is but it depends on embeddable and should be tracked as such imo. theoretically embeddables are free to load their state from anywhere they want (nothing is forcing them to use saved objects)

Yes, and as I mentioned above in the Lens example, the Lens embeddable is fetching its own saved object. But in the case where it is loading the object by reference, it doesn't have enough context to know whether it should use get or resolve.

4. I don't think snapshot urls are the biggest problem as they are migratable (means we can also migrate old ids to new ones). the bigger problem are regular URLs (something user just bookmars or copy pastes from address bar). as we have no way to migrate those. (there is no plan for now to write migrations for snapshot urls as far as i am aware)

I'm sorry, I don't really follow. When you are looking at a dashboard, if you copy the snapshot URL, it dumps the contents of the saved object (including its references) to a URL. That can't be migrated, at least not today.

To me it looks like we need a general solution (not something embeddable, or worse, dashboard specific) that would allow us to identify whenever we are loading the saved objects if the object Ids are old or new.

Application should theoretically have that knowledge. So we could create a very simple service that every app can then use:

   core.compatibility.setLegacyIDMode(true);

and then internally in the savedObjectsClient.get we could do a check against this and figure our if we want to resolve or not. Saved objects client could internally also log all the partial matches to the service, and app could fetch this and notify the user about it.

That's an interesting idea. It's a harder problem when you think about transitive references... for example, a dashboard snapshot URL with a reference to a lens saved object that has a reference to an index pattern. In an ideal world we would:

1. load the dashboard attributes from the snapshot URL
2. resolve the embeddable
3. get the index pattern

but if the application sets a "context" when the page loads, then we don't have that nuance, all consumers would either get or resolve.

So it would be better if each saved object in the graph could set such a "context" individually for its child references, but it certainly doesn't sound like there's an easy solution for that...

[ThomThomson]

Closing this for now because sharing an entire dashboard snapshot URL with panels is now extremely unlikely, and we have a warning in place to prevent it. Additionally, most panels are now By Value.

IMO it would be alright at this point for the Embeddables which can be by reference to use only get instead of resolve, and in the extremely rare case where an unmigrated legacy ID from pre 8.0 is loaded into the dashboard panels via a bookmarked URL it would throw a 404.

@vadimkibana @ppisljar if you think it is still important to allow Embeddable implementations to differentiate when loading their saved objects, we can re-open this and look into prioritizing it.