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

Config deprecations: allow to collect usage of deprecated (renamed) config keys #112633

Closed
pgayvallet opened this issue Sep 21, 2021 · 3 comments
Closed
Labels
discuss Feature:Configuration Settings in kibana.yml Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@pgayvallet
Copy link
Contributor

Currently, the recommended way to handle a change of name or path for a configuration property is to use the rename or renameFromRoot config deprecations.

However, those deprecations simply move the deprecated property to its new target, removing the deprecated entry from the config.

Effectively, this makes telemetry's config collector blind or the usages of the deprecated key. In other words there is no way, when using a rename deprecation, to collect telemetry info about the usage (value overrides) of the deprecated key.

As we want to preserve BWC by keeping the deprecated config path usable until a certain threshold is reached (as in #76385 for example), we need a way to have the config usage collector be aware of those deprecated (and removed keys).

One possibility would be to copy the deprecated key to the new path instead of moving it. However this has impacts on the owners schemas, as the schema then need to define both the old and new key, just for the sake of the collector to be able to collect this info, and doesn't seem like the correct approach (there are probably other side effects to this option).

Not sure what alternative we have though, as CoreUsageDataService.getNonDefaultKibanaConfigs directly relies on the configService's raw config, which already has the deprecations applied. Maybe we should have a way to have the config deprecations return the info of the potentially used-by-deleted keys when applying them, and then have the core usage data service leverage this info inside getNonDefaultKibanaConfigs?

@pgayvallet pgayvallet added discuss Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:Configuration Settings in kibana.yml labels Sep 21, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@lukeelmers
Copy link
Member

However this has impacts on the owners schemas, as the schema then need to define both the old and new key, just for the sake of the collector to be able to collect this info, and doesn't seem like the correct approach (there are probably other side effects to this option).

This is my concern with the copy approach as well. I feel like the whole point of the config deprecations is to make things dead simple so that users renaming config keys can write all of their code as if the new key is always being used.

Maybe we should have a way to have the config deprecations return the info of the potentially used-by-deleted keys when applying them, and then have the core usage data service leverage this info inside getNonDefaultKibanaConfigs?

This sounds like a pain but TBH I think would result in a better DX if it were feasible

@pgayvallet
Copy link
Contributor Author

From #112585 (comment)

We report the value (or [redacted]) of the non-deprecated config regardless which config was used.
lets say for example we have:

rename('whitelistedHosts', 'allowedHosts')
Our config collector will report the config key and value of allowedHosts regardless if whitelistedHosts or allowedHosts was set in the kibana configs.

To check if the deprecated renamed config whitelistedHosts was used you need to check another collector we have in telemetry in the path: core.config.deprecatedKeys.unset

To check the value of the config follow this usage path: kibana.plugins.kibana_config_usage

I believe a combination of both collectors should answer all the metrics you're after in this issue. We already have dashboards in the telemetry cluster for other plugins to answer similar questions. Happy to share offline

Feature is already implemented, therefor closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Feature:Configuration Settings in kibana.yml Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

No branches or pull requests

3 participants