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

Infra for deprecation logging #11033

Closed
wants to merge 1 commit into from

Conversation

kimchy
Copy link
Member

@kimchy kimchy commented May 7, 2015

Add support for a specific deprecation logging that can be used to turn on in order to notify users of a specific feature, flag, setting, parameter, ... being deprecated.

The deprecation logger logs with a "deprecation." prefix logger (or "org.elasticsearch.deprecation." if full name is used), and outputs the logging to a dedicated deprecation log file.

Deprecation logging are logged under the DEBUG category. The idea is not to enabled them by default (under WARN or ERROR) when running embedded in another application.

By default they are turned off (INFO), in order to turn it on, the "deprecation" category need to be set to DEBUG. This can be set in the logging file or using the cluster update settings API.

Add support for a specific deprecation logging that can be used to turn on in order to notify users of a specific feature, flag, setting, parameter, ... being deprecated.

 The deprecation logger logs with a "deprecation." prefix logger (or "org.elasticsearch.deprecation." if full name is used), and outputs the logging to a dedicated deprecation log file.

Deprecation logging are logged under the DEBUG category. The idea is not to enabled them by default (under WARN or ERROR) when running embedded in another application.

By default they are turned off (INFO), in order to turn it on, the "deprecation" category need to be set to DEBUG. This can be set in the logging file or using the cluster update settings API.
@kimchy kimchy added the review label May 7, 2015
@jpountz
Copy link
Contributor

jpountz commented May 7, 2015

It looks good, my only concern is that this new code is not used anywhere. Maybe it wouldn't be too hard to make ParseField use it?

@kimchy
Copy link
Member Author

kimchy commented May 7, 2015

@jpountz ahh, yea, thats my next step, I just wanted to get feedback on the approach, I can create an example usage

@@ -4,6 +4,10 @@ rootLogger: ${es.logger.level}, console, file
logger:
# log action execution errors for easier debugging
action: DEBUG

# deprecation logging, turn to DEBUG to see them
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe be even more explicit that this logger only receives logs at the debug level, so any higher level will not log any message

@rjernst
Copy link
Member

rjernst commented May 7, 2015

Can we make this logger easier to grab/create? I'm thinking e.g. in meta field mappers (which are not components) and I dont think we should need to pass this down through all the layers form the mappings module (as we do not for other loggers right now). Perhaps something like ESLoggerFactory, or maybe just another method on that? Maybe too the logger could just fail to initialize if the log level is set incorrectly for it?

@clintongormley clintongormley added :Core/Infra/Logging Log management and logging utilities >feature labels May 15, 2015
@spinscale
Copy link
Contributor

@rjernst I dont think the logger should fail to initialize, because we can change the log levels using the cluster update settings API

Would it be sufficient in your use-case to just have another constructor for the DeprecatedLogger hat allows to be called like this?

DeprecationLogger deprecationLogger = new DeprecationLogger(this.getClass().getName());

@spinscale
Copy link
Contributor

created a follow up PR that adds documentation, tests and the possibility to use the ESLoggerFactory to obtain a logger. PR is at #11285

@spinscale spinscale closed this in 045f01c May 26, 2015
@kevinkluge kevinkluge removed the review label May 26, 2015
spinscale added a commit that referenced this pull request May 26, 2015
The commit for closing #11033 was not building the asciidoc
documentation.
spinscale added a commit to spinscale/elasticsearch that referenced this pull request May 26, 2015
The ResourceWatcher used settings prefixed `watcher.`, which
potentially could clash with the watcher plugin.

In order to prevent confusion, the settings have been renamed to
`resource.reload` prefixes.

This also uses the deprecation logging infrastructure introduced
in elastic#11033 to log deprecated settings and their alternative at
startup.

Closes elastic#11175
spinscale added a commit to spinscale/elasticsearch that referenced this pull request May 27, 2015
The ResourceWatcher used settings prefixed `watcher.`, which
potentially could clash with the watcher plugin.

In order to prevent confusion, the settings have been renamed to
`resource.reload` prefixes.

This also uses the deprecation logging infrastructure introduced
in elastic#11033 to log deprecated settings and their alternative at
startup.

Closes elastic#11175
spinscale added a commit to spinscale/elasticsearch that referenced this pull request Jun 9, 2015
The ResourceWatcher used settings prefixed `watcher.`, which
potentially could clash with the watcher plugin.

In order to prevent confusion, the settings have been renamed to
`resource.reload` prefixes.

This also uses the deprecation logging infrastructure introduced
in elastic#11033 to log deprecated settings and their alternative at
startup.

Closes elastic#11175
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Logging Log management and logging utilities >feature v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants