Skip to content
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

Remove Dashboard Import/Export API #41439

Open
4 of 5 tasks
rudolf opened this issue Jul 18, 2019 · 20 comments · Fixed by #111283
Open
4 of 5 tasks

Remove Dashboard Import/Export API #41439

rudolf opened this issue Jul 18, 2019 · 20 comments · Fixed by #111283
Assignees
Labels
Breaking Change Feature:Legacy Removal Issues related to removing legacy Kibana Feature:Saved Objects impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@rudolf
Copy link
Contributor

rudolf commented Jul 18, 2019

The dashboard import/export API has been superseded by the saved objects import/export API

Since this change introduces a new API endpoint and file format (json -> ndjson), dashboards will have to be re-exported using the new API.

This issue tracks the deprecation and removal of the API as well as the migration of any consumers.

Pending consumer migrations:

Note: this issue was previously conflating the Dashboard Import/Export API with the legacy json import files which are two separate features. This issue only tracks the Dashboard import/export API removal, the legacy import feature is being tracked in #103921

@rudolf rudolf added Feature:Saved Objects Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Jul 18, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform

@epixa epixa mentioned this issue Aug 1, 2019
20 tasks
@rudolf
Copy link
Contributor Author

rudolf commented Aug 6, 2019

@urso With which release would beats be able to migrate over to the new API, i.e. which release can we target for the deprecation?

@joshdover
Copy link
Contributor

Beats is tentatively planning to remove this feature completely. We should ensure this API continues to exist in all of 7.x, but can move forward with deprecating now.

@rudolf
Copy link
Contributor Author

rudolf commented Jun 23, 2020

We didn't want to mark this API as deprecated before 8.0 because that would mean that users using a completely up to date stack would receive deprecation warnings they can't do anything about.

@pgayvallet
Copy link
Contributor

Makes sense. Should we just remove it in 8.0 as a 'breaking' change then, or do we want to support it for a time after 8.0 is out?

@lukeelmers
Copy link
Member

Beats has moved away from this API in elastic/beats#27220, which looks like it will make the cut for 7.15 🤞

Should we just remove it in 8.0 as a 'breaking' change then, or do we want to support it for a time after 8.0 is out?

After further discussion, we've decided we will not remove this in 8.0, but rather we will:

  • Mark the API as deprecated
  • Add telemetry so we can track remaining usage of it now that Beats is no longer relying on it

We are okay with this tradeoff because of the relatively low cost of maintenance for the API. I will update the issue description here accordingly; feel free to chime in with any corrections @rudolf @pgayvallet @kobelb

@pgayvallet
Copy link
Contributor

Add telemetry so we can track remaining usage of it now that Beats is no longer relying on it

What kind of telemetry we ideally want to add here? Should we use an existing (or new) collector somehow? Do we want to use a usage counter?

@rudolf
Copy link
Contributor Author

rudolf commented Sep 6, 2021

I've added usage data in #111283

But thinking more about this, I think we need to keep pushing to remove it in 8.0.

Keeping the code around has a relatively low cost, but supporting it won't:

In this case, we won't be helping any users by keeping this API around but it's behaviour is badly tested and definitely not compatible with a multi namespace world.

So if usage data shows significant amount of users using this, we will have to invest time into improving this API if we don't remove it.

@rudolf
Copy link
Contributor Author

rudolf commented Sep 6, 2021

Or alternatively, we need to clearly document the impact/shortcomings of using this API

@pgayvallet
Copy link
Contributor

Keeping the code around has a relatively low cost, but supporting it won't:

Imho supporting the code is the same as keeping the code, so that means that keeping the code comes at a heavy cost.

In this case, we won't be helping any users by keeping this API around but it's behaviour is badly tested and definitely not compatible with a multi namespace world.

++

Or alternatively, we need to clearly document the impact/shortcomings of using this API

We need to at least do that. But it would feel safer to be allowed to just drop the API imho.

@lukeelmers
Copy link
Member

Keeping the code around has a relatively low cost, but supporting it won't

These are all good points, and I don't think the lack of support for these features was considered in the original discussion around not moving forward with the break.

@kobelb What are your thoughts on this?

  • The API doesn't require any active "maintenance", however it isn't as feature complete or well-tested as the regular import/export API, which could lead to unexpected bugs or confusion for users
  • We only just added telemetry so we don't yet have a good idea of how heavily it is used (we know that Beats, our main internal consumer, has now moved off it)

If we proceed with the plan of not making the breaking change, our only option for preventing possible confusion will be to document the caveats.

@kobelb
Copy link
Contributor

kobelb commented Sep 7, 2021

Keeping the code around has a relatively low cost, but supporting it won't:

Imho supporting the code is the same as keeping the code

Agreed.

Is my understanding correct that this API is undocumented? If so given the fact that there is a high-cost to keeping around the compatibility layer, I'm alright with us removing it in 8.0. However, I do think that we need to integrate it with the upgrade assistant deprecations in 7.16 so the user is aware that it's being removed and what HTTP API they should use instead.

@kobelb
Copy link
Contributor

kobelb commented Sep 7, 2021

I'm checking with Mark to ensure he's comfortable adding this to the list of 8.0 breaking changes, and I'll reply here with his response.

@kobelb
Copy link
Contributor

kobelb commented Sep 7, 2021

Mark is alright with us removing the HTTP API in 8.0. However, he'd like for us to add telemetry in 7.16. This will give us a window of time to measure whether our assumptions are correct that this HTTP API isn't used widely and potentially reverse coarse and reintroduce this HTTP API.

@rudolf
Copy link
Contributor Author

rudolf commented Oct 27, 2021

Re-opening since #111283 only deprecated the API but didn't remove it

@rudolf rudolf reopened this Oct 27, 2021
@rudolf rudolf changed the title Deprecate Dashboard Import/Export API Deprecate and remove Dashboard Import/Export API Oct 27, 2021
@exalate-issue-sync exalate-issue-sync bot added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort labels Nov 4, 2021
@pgayvallet
Copy link
Contributor

@rudolf do you know if we're safe to now remove the API in 8.1 or 8.2? Do we still have any consumers of the dashboard import/export in 8.x?

@rudolf
Copy link
Contributor Author

rudolf commented Feb 4, 2022

Percentage of clusters using this API:

7.17.0 0.6%
7.16.3 1.5%
7.16.2 1.2%
7.16.1 2.5%
7.16.0 0.2% 

I'm not sure what exactly our cutt-off would be, but these usage numbers seem pretty low to me. Given the cost and risks outlined above I don't think these justify keeping the API around.

@lukeelmers
Copy link
Member

I'm not sure what exactly our cutt-off would be

I think our rough rule of thumb is < 1%, but based on our earlier discussions on this thread, it sounds like we were already comfortable removing it. I'm also unclear on whether the fact that this didn't merge for 8.0 means we need to stick with the newer, longer deprecation timelines, or if simply meeting the telemetry thresholds is sufficient. cc @stacey-gammon WDYT?

@stacey-gammon
Copy link
Contributor

Unfortunately, since it didn't make it into 8.0, I think we need to stick with the new deprecation policy which is 18 months and getting breaking change approval. I think the process for getting approval is still TBD. Let me check and I'll loop back.

@TinaHeiligers
Copy link
Contributor

@rudolf rudolf changed the title Deprecate and remove Dashboard Import/Export API Remove Dashboard Import/Export API Sep 6, 2024
@TinaHeiligers TinaHeiligers self-assigned this Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Feature:Legacy Removal Issues related to removing legacy Kibana Feature:Saved Objects impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants