-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Ingest Manager] Modify ids of (some) saved object for installed assets #65035
Comments
Pinging @elastic/ingest-management (Team:Ingest Management) |
@ruflin Isn't the solution is to use a random ID and instead keep the identified as metadata field on the dashboard? Anything else seems fragile to me? |
@neptunian I suppose for updates we are currently expecting that the id is the same between revision? |
Currently, and I'm not sure it will stay this way or ought to be this way, when a package updates all the saved objects are removed and created again from the new package. The thinking was to get rid of of assets that may no longer exist. So there isn't any comparing or updating of ids during updating. |
Using unique ids is definitively one of the solutions. One reasons I like to prefix the ids for example with the package names is that it makes it easier for user to see where a saved object is coming from. Yes, having metadata would also be nice for this, but we don't at the moment. As @neptunian mentioned, ids do not have to be the same. This is important as we can't rely on the package creator to not change the ids. |
@neptunian Would adding the package name as a prefix would be a simple solution to implement? The package name are indeed unique in our world. If we do that are we at risk to hit a limit string lenght for the ids? @ruflin Do we have a string lenght limit for package name? If adding a prefix doesn't cause any short term problem lets do that in priority. |
We don't have any package name lengths limitation in place but I also can't see us creating packages with more then 20-30 chars. I'm also not aware of a Kibana id length max. Whatever we decide, we can change it later as we track the assets and always can remove the old ones. |
@ph This should be a simple change, though perhaps unexpected to package authors or people referencing the package if we make the change in Kibana instead of in the packages themselves? What's the reasoning it should be done in kibana and not the package? How does the user differentiate between these two assets when they both exist? |
This is a pretty good point you are bringing, we already have this problem today, let's assume that the user creates a custom dashboard title "foobar" with ID=123, and we have an integration package that also provides one with the title "foobar" but with ID=124. Outside of us, does ID also is used to link dashboard between them? |
Okay, maybe we can think about changing the Description field to differentiate at some point, assuming that is the link/title text that Kibana uses.
To clarify, by referencing I meant looking in the package in github at the ids for debugging, testing, or dev purposes and not finding those IDs in saved objects. Perhaps this is not a big deal. I don't think anyone is referencing our kibana package assets outside of us at the moment. Also, dashboards reference visualizations by ID: https://github.com/elastic/integrations/blob/master/packages/nginx/kibana/dashboard/023d2930-f1a5-11e7-a9ef-93c69af7b129-ecs.json#L96. So I assume these would also need to change if we plan to prefix all assets. |
Let me just clarify how rabbit hole this can be will add more in a few minutes. |
I had a chat with @neptunian today. Few notes:
|
Yes and so long as the type property is search, dashboard, or vis and not index pattern. There are id props in other places in the visualizations but they don't point to assets that we install/remove. |
At the moment AFAIK when we installed saved objects like dashboards or visualisations from package we take the ids given by the document and put them into Kibana. As some of these assets are imported from modules, this could cause some conflicts if the assets from the nginx module and nginx package are loaded. Because of this we should find a way how to prevent/minimise these conflicts.
TODO:
The text was updated successfully, but these errors were encountered: