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

[New Platform] 8.0 Refactor kibana.yml #46705

Closed
rudolf opened this issue Sep 26, 2019 · 26 comments
Closed

[New Platform] 8.0 Refactor kibana.yml #46705

rudolf opened this issue Sep 26, 2019 · 26 comments
Labels
Breaking Change Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.0.0

Comments

@rudolf
Copy link
Contributor

rudolf commented Sep 26, 2019

2019-11-19 Updated based on comments

Core's config service uses "paths" to split up kibana.yml into sub-config's with their own schema. We want sub-configs to be grouped by domain, but some of the existing config prefix paths don't correspond to any domains in Core.

For instance (not a complete list):

  1. There's no "migrations" domain, these config options belongs to the Saved Objects domain:
    • migrations.scrollDuration - The scroll value used to read batches of documents from the source index. Defaults to 15m.
    • migrations.batchSize - The number of documents to read / transform / write at a time during index migrations
    • migrations.pollInterval - How often, in milliseconds, secondary Kibana instances will poll to see if the primary Kibana instance has finished migrating the
  2. The "kibana" domain should be reserved for global configuration options:
    • kibana.enabled remove
    • kibana.defaultAppId deprecated
    • kibana.disableWelcomeScreen moved to home plugin
    • kibana.autocompleteTerminateAfter move to data plugin
    • kibana.autocompleteTimeout move to data plugin
    • path.data move to be under "kibana"
  3. some properties should be moved from their legacy namespace:
    • server.uuid should move outside of the server namespace

This refactoring would be a breaking change so it will have to be introduced in 8.0.0. We should audit all of our configuration options and coordinate this with all teams to ensure that we're satisfied with the end result for all of them.

@rudolf rudolf added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:New Platform labels Sep 26, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform

@legrego
Copy link
Member

legrego commented Sep 27, 2019

kibana.index move into saved objects domain as saved_objects.index

This one might get tricky, as there are other plugins which use this field to derive their own data which isn't strictly related to saved objects.

For example, the Security plugin uses the kibana.index value to represent the "tenant" when registering application privileges with elasticsearch. The application name that we register is kibana-${kibana.index}. This allows for multiple kibana installations to use the same ES cluster, while maintaining their own index and privilege/permission sets.

I think the alerting team is also planning on using this value to derive the name of one of their indices, in order to support multi-tenant setups, and perhaps other features.

I'm not sure what domain this would fall under...core? But core.index doesn't make a whole lot of sense for the typical Kibana administrator, who doesn't know/care about how the internals of Kibana are structured.

@joshdover
Copy link
Contributor

I'm not sure what domain this would fall under...core? But core.index doesn't make a whole lot of sense for the typical Kibana administrator, who doesn't know/care about how the internals of Kibana are structured.

You could definitely argue that kibana is actually a good namespace for "global config" values. If that's what we want to go with, there's other namespaces that maybe should be moved underneath this one or combined with this one. For instance path.data

@rudolf
Copy link
Contributor Author

rudolf commented Nov 1, 2019

As part of this refactor it might be worth switching from camel case to snake case convention for consistency across the stack #7444 (thanks @azasypkin)

@joshdover joshdover removed the blocker label Nov 12, 2019
@lukasolson
Copy link
Member

Throwing this out there: elasticsearch.shardTimeout should probably be removed from the Elasticsearch core service because it's not even used by that service. It should be renamed to something like kibana.searchTimeout and exposed in the same manner as other kibana.* settings.

@lukasolson
Copy link
Member

lukasolson commented Nov 18, 2019

kibana.autocompleteTerminateAfter move?
kibana.autocompleteTimeout move?

These should probably be moved into the data plugin/domain.

@joshdover
Copy link
Contributor

For clarity: we are not planning on working on this until after the Platform migration work has been completed (at the earliest). In the meantime, the workaround outlined in #46240 will be the near-term solution.

@lukasolson We'll make sure to include the config keys you listed above and it should be very easy to add additional options in the workaround mechanism in the future if we find more.

@pgayvallet
Copy link
Contributor

There's no "migrations" domain, these config options belongs to the Saved Objects domain:

  • migrations.scrollDuration - The scroll value used to read batches of documents from the source index. Defaults to 15m.
  • migrations.batchSize - The number of documents to read / transform / write at a time during index migrations
  • migrations.pollInterval - How often, in milliseconds, secondary Kibana instances will poll to see if the primary Kibana instance has finished migrating the

I also see a migrations.skip property in src/core/server/saved_objects/saved_objects_config.ts

Should the migrations namespace be renamed to saved_objects ? Or maybe moved under saved_objects?

migrations.scrollDuration -> saved_objects.migrations.scroll_duration ? (or camel case, not sure if camel to snake is in the scope of the issue)

kibana.enabled remove

Seems unused. should probably remove yes

kibana.defaultAppId move?

This seems used by multiple plugins. So I guess this should stay there?

kibana.disableWelcomeScreen move?

Should move to home plugin I think. home.disable_welcome_screen ?

kibana.autocompleteTerminateAfter move to data plugin
kibana.autocompleteTimeout move to data plugin

These are used in src/legacy/core_plugins/input_control_vis/public/control/list_control_factory.js and src/legacy/core_plugins/kibana/server/routes/api/suggestions/register_value_suggestions.js

should is really moves to data plugin?

path.data move to be under "kibana"

kibana.data_path then ? kibana.data_folder ?

some properties should be moved from their legacy namespace:

  • server.uuid should move outside of the server namespace

Unclear where this should move to, as it's quite an isolated property.

  • Could move to instance.uuid but that force to creates a namespace for a single property.
  • Could move to core.uuid (do we have other properties going under core / do we want a 'core' namespace?)
  • Could move to kibana.instance_uuid ?
  • Should we keep it there for now?

@jbudz
Copy link
Member

jbudz commented Dec 18, 2019

Some of these are stack globals, so that should be taken into consideration too. path.data

For settings that mean the same thing I'd push for consistency.

@mshustov
Copy link
Contributor

Throwing this out there: elasticsearch.shardTimeout should probably be removed from the Elasticsearch core service because it's not even used by that service. It should be renamed to something like kibana.searchTimeout and exposed in the same manner as other kibana.* settings.

@lukasolson shouldn't it be a part of data plugin as well?

@lukasolson
Copy link
Member

@restrry Yeah, it would make more sense there.

@flash1293
Copy link
Contributor

@pgayvallet I started a separate issue for Kibana app related configs: #54497

@nickofthyme Is working on some of those in #55937

kibana.defaultAppId move?

This seems used by multiple plugins. So I guess this should stay there?

defaultAppId is becoming obsolete once we migrated all of the apps currently bundled in the "kibana" app (Home, Discover, Visualize, Dashboard, Management, Dev Tools) - See also #54088 IMHO we should keep those with a deprecation warning till then.

kibana.disableWelcomeScreen move?

Should move to home plugin I think. home.disable_welcome_screen ?

👍

kibana.autocompleteTerminateAfter move to data plugin
kibana.autocompleteTimeout move to data plugin

These are used in src/legacy/core_plugins/input_control_vis/public/control/list_control_factory.js ?> and src/legacy/core_plugins/kibana/server/routes/api/suggestions/register_value_suggestions.js

should is really moves to data plugin?

I think it makes sense. Mid-term we can probably refactor input control vis to leave it up to the data fetching layer to set terminate after and timeout. Short-term we could expose it in the contract of the data plugin so other plugins can also make use of it but it's clear the data plugin owns it.

@flash1293
Copy link
Contributor

@lukeelmers Is the app arch team content with moving kibana.autocompleteTerminateAfter and kibana.autocompleteTimeout into the data plugin? If yes, is there already an issue for it? I had to implement a workaround in #63443 for this and would like to reference properly.

@lukeelmers
Copy link
Member

@flash1293 Thanks for the ping; yes moving those settings to the data plugin makes the most sense to me.

@rudolf Is there a plan yet for when the other items might be moved, so that we can coordinate? I've created #66085 to track this on the app arch side, but I'm thinking it might make sense to plan these for the same release, since it is a breaking change.

@rudolf
Copy link
Contributor Author

rudolf commented May 11, 2020

@lukeelmers We'll have to do this in 8.0 (I'll update the issue to make this more clear) and we will coordinate this with the wider Kibana team to ensure that we include all teams' changes.

@rudolf rudolf added the v8.0.0 label May 11, 2020
@rudolf rudolf changed the title [New Platform] Refactor kibana.yml [New Platform] 8.0 Refactor kibana.yml May 11, 2020
@joshdover
Copy link
Contributor

We'll have to do this in 8.0 (I'll update the issue to make this more clear) and we will coordinate this with the wider Kibana team to ensure that we include all teams' changes.

To be clear, we can start migrating this now and use the config deprecations feature to map the old name to the new name. We just can't remove that deprecation translation until 8.0. Ideally, we do these changes earlier than later so sysadmins have more heads up. I don't think it's critical that these deprecations are done at the same time.

@rudolf
Copy link
Contributor Author

rudolf commented Jun 23, 2020

We should move csp to server.csp

// TODO: Move this to server.csp using config deprecations

@pgayvallet
Copy link
Contributor

Ideally, we do these changes earlier than later so sysadmins have more heads up

Should this be moved to an earlier release then? It's currently on our 8.0 - Tentative board.

@joshdover
Copy link
Contributor

Ideally, we do these changes earlier than later so sysadmins have more heads up

Should this be moved to an earlier release then? It's currently on our 8.0 - Tentative board.

8.0 and 7.last are released at the same time, so they're sorta the same roadmap. We need to make sure the deprecation is in the last minor release though. I will move.

@pgayvallet
Copy link
Contributor

We need to make sure the deprecation is in the last minor release though. I will move.

I was more saying that, as we already know that these are going to be moved/deprecated, maybe we should add the deprecations sooner 😄 ?

I mean, as a user, having the info that these properties are going to be deprecated is better sooner than later?

@lukeelmers
Copy link
Member

As doing a Kibana-wide renaming of config keys is nowhere on our near-term horizon, we'll go ahead and close this issue for now. We can reopen later if it becomes necessary.

@TinaHeiligers
Copy link
Contributor

TinaHeiligers commented Nov 2, 2021

Throwing this out there: elasticsearch.shardTimeout should probably be removed from the Elasticsearch core service because it's not even used by that service. It should be renamed to something like kibana.searchTimeout and exposed in the same manner as other kibana.* settings.

@lukasolson shouldn't it be a part of data plugin as well?

@lukasolson The data plugin still uses elasticsearch.shardTimeout. are there still plans to move the config to the data plugin? If so, we'll need to start warning folks about the change since it's too late now to work that into #116296.

@lukasolson
Copy link
Member

Thanks for resurfacing this @TinaHeiligers... Yes, ideally this setting would be in the data plugin rather than elasticsearch, but I'm not really sure what our options are at this point. @lukeelmers Is it possible for us to rename this at this point, or what would you suggest?

@lukeelmers
Copy link
Member

@lukeelmers Is it possible for us to rename this at this point, or what would you suggest?

We'd need to follow the new deprecation guidelines before we could remove the old config, meaning the setting would probably linger for a couple years. However, I believe we could still move it to the data plugin starting in 8.1 (too late for 8.0), and just register a deprecation rename for it in the meantime so that users aren't affected.

@kobelb Do we need to discuss deprecations before they are added in the first place, or just need sign-off on the actual breaking changes before they happen?

@kobelb
Copy link
Contributor

kobelb commented Nov 10, 2021

@kobelb Do we need to discuss deprecations before they are added in the first place, or just need sign-off on the actual breaking changes before they happen?

Deprecations don't need approval. We just need the approval to make the breaking change itself.

@lukasolson
Copy link
Member

Created a new issue for tracking this last change here: #118250

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.0.0
Projects
None yet
Development

No branches or pull requests