Deprecate kibana.index setting
#83988
Conversation
|
@elasticmachine merge upstream |
kobelb
force-pushed
the
deprecate-kibana-index
branch
from
7a6fe2f
to
e406786
Compare
src/core/server/kibana_config.ts
Outdated
| log( | ||
| `Setting [kibana.index] is deprecated. Multitenancy by changing 'kibana.index' will not be supported starting in 8.0. See https://ela.st/kbn-remove-legacy-multitenancy for more details` | ||
| ); |
There was a problem hiding this comment.
Remark: There is an inconsistency with the wording of our unused factory:
Setting [{config prop}] is deprecated vs {config prop} is deprecated.
I also see that rename is using double quote around the config prop (where unused does not).
This is minor, but we might want to be consistent in our deprecation messages.
Also thinking: we may want to adapt ConfigDeprecationFactory.unused (and others) to accept a new optional 'message' parameter. That would also to log custom messages in deprecations without having to recode a custom one just to log the message (as it is done here for kibana.index).
unused('kibana.index', "Multitenancy by changing 'kibana.index' will not be supported starting in 8.0. See https://ela.st/kbn-remove-legacy-multitenancy for more details")
This is probably out of the scope of this PR though. Should I create a separate issue?
There was a problem hiding this comment.
This is minor, but we might want to be consistent in our deprecation messages.
Agreed about being consistent here. I was originally replicating the format from the following deprecation log message, because I incorrectly assumed it used the "standard approach": [warning][config][deprecation] Setting [elasticsearch.username] to "kibana" is deprecated. You should use the "kibana_system" user instead.
Do we want to follow the precedent set by unused and not use double-quotes, or should we follow the precedent set by rename and use double-quotes?
Also thinking: we may want to adapt ConfigDeprecationFactory.unused (and others) to accept a new optional 'message' parameter. That would also to log custom messages in deprecations without having to recode a custom one just to log the message (as it is done here for kibana.index).
I think it's fine to support custom messages for unused/rename. However, I don't think we should be using the unused deprecation here, as this setting is still being used and has an effect, it's just deprecated. We use the unused deprecation elsewhere when settings don't have an effect.
There was a problem hiding this comment.
Do we want to follow the precedent set by unused and not use double-quotes, or should we follow the precedent set by rename and use double-quotes?
I think using quotes is slightly better, wdyt?
I think it's fine to support custom messages for unused/rename. However, I don't think we should be using the unused deprecation here, as this setting is still being used and has an effect, it's just deprecated.
Yea, you're right. I forgot we are still using the value of this property. Nevermind
There was a problem hiding this comment.
Double-quotes sound good to me! I just independently decided that also and used it in 9dc7532
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
* Deprecating `kibana.index` setting * Using ela.st service so this can be changed to the blog in the future * Adding unit tests * Revising deprecation log message * Changing the deprecation log message to be more consistent with others * Updating kibana.index docs also * Using rename deprecation as the "standard" for the deprecation messages * /s/'/`
cjcenizal
added
the
Team:Core
|
Pinging @elastic/kibana-core (Team:Core) |
Using the ela.st service for the deprecation message and the release-note, so we can redirect this to a blog-post with more information as we get closer to the 8.0 release.
Resolves #82521
"Release Note: Deprecating the
kibana.indexsetting. Multitenancy by changingkibana.indexwill not be supported starting in 8.0. See https://ela.st/kbn-remove-legacy-multitenancy for more details"