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

Projects
None yet
6 participants
@kimchy
Copy link
Member

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.

Infra for deprecation logging
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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Member Author

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

This comment has been minimized.

Copy link
@jpountz

jpountz May 7, 2015

Contributor

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

This comment has been minimized.

Copy link
Member

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?

@spinscale

This comment has been minimized.

Copy link
Member

commented May 20, 2015

@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

This comment has been minimized.

Copy link
Member

commented May 22, 2015

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

Documentation: Fix elasticsearch documentation build
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

ResourceWatcher: Rename settings to prevent watcher clash
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

ResourceWatcher: Rename settings to prevent watcher clash
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

ResourceWatcher: Rename settings to prevent watcher clash
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
You can’t perform that action at this time.