[Discuss] Solution for multiple plugins using same migration type.

[lukeelmers]

In the process of reviewing the first PR to include a visualization migration (#26057), @timroes and I realized that the restriction of only allowing migrations for each type to be defined in a single place poses some challenges when it comes to situations where multiple plugins may use a single type (visualizations are the best example of this).

Conventional wisdom suggests simply moving all migrations to one shared location, however, this doesn't really work as the migrations get defined in a plugin's uiExports.

Right now the best solution we have would be to create a shared file of migrations for a single type that gets imported into one random vis plugin, but of course the choice of which plugin to use is totally arbitrary, and then logic for a migration that is specific to a single plugin gets spread out elsewhere in the codebase.

A more robust alternative could be creating some type of registry to serve as a wrapper for the migrations, where migrations could be registered for a particular type and gathered into one location.

@epixa or @elastic/kibana-platform: Have there been any prior discussions around this topic? We don't want to create some vis-specific workaround if there are already plans for addressing this use case more broadly.

For reference:
First PR to use a migration of type visualization: #26057
Original discussion on migrations: #15100

cc @timroes @chrisdavies

[lukeelmers]

Just adding a note here that for the time being, we are moving all migrations to the main Kibana plugin (see #28199 and #29478), so that they can be managed centrally until it is decided whether further work in this area is necessary.

[spalger]

Conventional wisdom suggests simply moving all migrations to one shared location, however, this doesn't really work as the migrations get defined in a plugin's uiExports.

The same is true with the mappings though, I think whichever plugin is responsible for the mappings of a saved object should also be responsible for its migrations.

[epixa]

I agree with Spencer's thoughts on the subject. I don't think extensibility of a specific saved object type should be a concern of the core migration system.

Visualizations themselves are/should be a platform in their own right, so it makes sense to me that the plugin responsible for the visualizations platform would provide extension points for managing the visualization schema and potentially some sort of discrete hook into migration logic.

[spalger]

the plugin responsible for the visualizations platform would provide extension points for managing the visualization schema and potentially some sort of discrete hook into migration logic

Possibly, but doing that is really risky as plugins come and go but the migrations defined by a plugin need to be static/never changing.

[lukeelmers]

plugins come and go but the migrations defined by a plugin need to be static/never changing.

So really, it comes back to this same issue, regardless of whether this is addressed in the core migration system or just within visualizations. The fact that plugins are by definition transient means somewhere along the line we'll be dealing with this issue, as long as there's more than 1 plugin ever using an object type.

(Possibly Unrelated) Question: Do migrations work on ui_settings as well? That would be another place I could think of where you have different plugins sharing one object type.

[stacey-gammon]

How will the migration system work for things that span across object types, like for instance a "Tagging" plugin that adds a tags attribute to all saved objects? Or security with RBAC? Or are we thinking those can be solved without modifying other plugin's saved object types (e.g. new document that maps saved object id to tag id, and add saved object metadata that way)?

cc @mikecote

[stacey-gammon]

I'm going to dump some thoughts into this, though it's late in the day and I'm about to hop off so hopefully they make some sense.

Essentially this is making me wonder about the structure of our visualization objects, and also how it related to Embeddable API. We always had this weird layering of types. Embeddable type and visualization "sub" type. Maybe we could flatten this, and solve the migration issue, at the same time.

UI

If we want to let the user create visualizations and embeddables inline, how will that look? Will they first select the embeddable type, options being: Saved Search, visualization, maybe eventually Map, canvas workpad, etc. If they select Visualization, then they once again have to select a type to add: TSVB, pie, area, data table.

The extra sub type (but still called type on the vis saved object) feels weird from a user perspective. They don’t care about embeddable vs visloader. It’s just different things to create and drop on a dashboard. I could instead envision one type, that is filled with everything: pie, area, tsvb, saved search, data table, map workpad, canvas workpad, etc.

Migrations

Because we are currently stuffing a lot of data into a single object type (‘vis’), we are hitting the common scenario of different plugins wanting to write migrations for the same object type. Trying to support that at the migration level is difficult. It might be easier if we just use practices that don’t encourage that. If you want to add metadata to an object, you can always use a mapping, or nest a reference to the original visualization object inside your new type.

For instance, if we wanted to pull out the different sub types of visualizations, it would be pretty easy to implement at the embeddable layer:

• Create an embeddable factory for each type, so it has:
  • factory.name: ‘tsvb'
  • factory.name: data_table
  • etc
• Each of those factories can be extremely slim, just forwarding on the logic to the VisualizeEmbeddableFactory
• Dashboard passes in type along with id into the EmbeddableFactories
• VisualizeEmbeddableFactory uses that type to open the saved object, maybe make some checks on the shape (e.g. verify there is an expression object), and then commences as usual.

I don’t know what the downsides are, but it seems like it would make both migrations and the UX easier.

We have already had discussions about how/whether to combine the visLoader and the Embeddable layer. Maybe hitting this just means we should push that forward a bit.

[elasticmachine]

Pinging @elastic/kibana-app

[ppisljar]

thinking about the future vis saved object .... it will just hold the expression. But each of the functions in that expression could need to be migrated ... We definitely don't want multiple saved objects on this level, we also don't want visualize plugin being responsible for migrating 3rd party functions.

So something in the direction @epixa was mentioning would make the most sense to me. Not sure whats meant with the plugins come and go ...

If you disabled a plugin, which provided esaggs function ... well, then you don't care about migrating this function to new syntax either, as you are obviously not using it, or if you try you will get 'function not found' exception.

[chrisdavies]

Regarding things like a "tagging" plugin that adds tags to all saved objects, the tagging plugin would contain migrations that target whatever property it owns (e.g. tags or whatever). Those migrations would get applied to any saved objects that have a tags property, so I think that scenario should be fine.

[lukeelmers]

My main objective in opening this issue was to understand whether this is something that we would ever consider adding as a platform-wide solution, or whether it is something the Kibana app team should take into consideration when thinking through the app & vis architecture.

It sounds as if we are converging on the idea that this should be an app-specific concern for now. Is that a fair summary? If so, I will close this issue and we will revisit it in more detail during Kibana app roadmap planning.

[chrisdavies]

RIght. This adds non-determinism to migrations, because ordering and enabling / disabling plugins really messes with things. I think this is a scenario we should never support. However, multiple plugins can end up migrating the same saved object, but they do so by targeting root properties of the object that they own. So, security might migrate the ACL property on a bunch of visualizations, and the visualization plugin might migrate the "y-axis" property on those same objects. But two plugins cannot target the same property.

[mikecote]

Just to add, we've had our first issue where a migration was defined in a different plugin than the mappings (#31644). It caused issues when importing the data in a system with a different license.

@stacey-gammon apologies, I somehow missed the cc, @chrisdavies answered the question.

[lukeelmers]

I'll go ahead and close this for now then; feel free to re-open if anybody has something else to add.

It looks like there is already a PR open to enforce migrations being defined in the same plugin as mappings, which should help ensure consistency at least: #31739

Moving forward, we'll assume any vis-specific concerns related to this should be handled on the app side.