search

elastic / elasticsearch

Free and Open Source, Distributed, RESTful Search Engine
https://www.elastic.co/products/elasticsearch
Other
1.21k stars 24.85k forks source link

Improve deprecation message for settings #79666

Open jakelandis opened 3 years ago

jakelandis commented 3 years ago

Currently all settings if deprecated logs the following message

         String message = "[{}] setting was deprecated in Elasticsearch and will be removed in a future release! "
                    + "See the breaking changes documentation for the next major version.";

With https://github.com/elastic/elasticsearch/pull/79665 some settings will be reduced to warning severity and as such message may not make as much sense (why looking at breaking changes if nothing is breaking). Additionally, it would be nice we we could steer people directly to the documentation. Especially since as of https://github.com/elastic/elasticsearch/pull/79035 the deprecation logs will be indexed by default and exposed via the Upgrade Assistant. (@cjcenizal - do you know if http:// text in the deprecated logs will be rendered as clickable links ?)

This issue is to discuss changing the deprecation message to better allow for better wording for non-breaking changes (warnings) as well as any improvements we can make to the existing message for breaking changes. There are literally hundreds of deprecated settings in 7.16, so anything we can do to help the user make sense with what to do with these is helpful. (Note - many/most of these deprecated settings also have an entry in the deprecation info API too with a dedicated URL link).

cc: @debadair @lockewritesdocs @jrodewig @rjernst @masseyke @colings86

elasticmachine commented 3 years ago

Pinging @elastic/es-docs (Team:Docs)

elasticmachine commented 3 years ago

Pinging @elastic/es-core-infra (Team:Core/Infra)

cjcenizal commented 3 years ago

do you know if http:// text in the deprecated logs will be rendered as clickable links

I don't believe this functionality exists today, but it's a great idea. I opened https://github.com/elastic/kibana/issues/116027 so we won't lose track of it.

pgomulka commented 3 years ago

hey @jakelandis we were wondering if this issue is a follow-up/bug from https://github.com/elastic/elasticsearch/issues/78781 and https://github.com/elastic/elasticsearch/blob/1f0e85bae33f015667d51a9dfce00dfd68bffdc5/server/src/main/java/org/elasticsearch/common/settings/Setting.java#L579 ?

and do you think the URL part is related to https://github.com/elastic/elasticsearch/issues/73774 ?

We parked the issue and would like to discuss this in more detail to get a holistic approach to deprecations. I also wonder if these should improvements should be only for 8.0+ or should be considered for 7.16 series too (as you mentioned there is hundreds of deprecations and users sooner or later will benefit from these)

jakelandis commented 3 years ago

It is directly related to https://github.com/elastic/elasticsearch/issues/78781 in that we probably want to tweak the wording for warning messages since referencing the breaking changes documentation probably won't help.

It is tangentially related https://github.com/elastic/elasticsearch/issues/73774 in that if we get a better strategy thdeprecation message should improve (imo the deprecation info api messages are more meaningful).

In the short term, we should adjust "See the breaking changes documentation for the next major version." to maybe just "See the deprecation documentation for the next major version." -> I will submit a PR to discuss this specific aspect and we can park the rest of this issue.

pgomulka commented 2 years ago

@jakelandis re changing the message I created a PR here https://github.com/elastic/elasticsearch/pull/80900 but before we merge it, do we already have a deprecation documentation (warn level) ? the breaking changes documentation is https://www.elastic.co/guide/en/elasticsearch/reference/8.0/migrating-8.0.html#breaking-changes-8.0 We should apply this change (including the creation of deprecation documentation) for all v8.1, v8.0 and 7.16, right?