-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Modernize and split log4j-jul
#2935
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
Conversation
0de1b92 to
ecb3565
Compare
vy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not JUL-savvy enough to provide a meaningful review, but skimmed through the changes and dropped some remarks anyway.
It fixes location detection, even if JUL Filters are used.
The last part probably could be improved: I don't see why we should supportj.u.l.Filters. We could drop the feature in3.x.
Then let's drop it? I guess we can later on add it in a backward compatible manner if needed, right?
I agree, I will drop the |
|
@vy, I did some further changes to this PR:
|
|
For backward compatibility with e.g.
Note: a better alternative to a hardcoded Logback users have the most problems with JUL-to-Logback redirection, since they can only use:
|
This splits `log4j-jul` into two artifacts: - `jul-to-log4j` that contains a `j.u.l.LogManager` implementation, but does not depend on Log4j Core. - `log4j-jul` that contains a `j.u.l.Handler` implementation and depends on Log4j Core. We also update the `j.u.l.LogManager` implementation to: - implement methods introduced in Java 9, - remove methods deprecated in Java 9, - remove the support for `j.u.l.Filter`.
fae3838 to
423876f
Compare
|
I am merging this first and then we can discuss where to put |
The current
log4j-julartifact contains four different features:j.u.l.LogManagerimplementation. This implementation is actually rather generic and can be adapted to any kind of logging backend by implementing ano.a.l.l.j.AbstractLoggerAdapter. The defaultAbstractLoggerAdapterforwards all logging calls to the Log4j API.o.a.l.l.j.AbstractLoggerAdaptercalledCoreLoggerAdapterthat forwards the JULLogger.setLeveland similar non-logging API method tolog4j-core.j.u.l.Handlerimplementation calledLog4jBridgeHandlerthat forwardsLogRecords to Log4j API. This handler can be used if the user can not change the implementation ofj.u.l.LogManager.Log4jBridgeHandleris very slow for log statements that are disabled in the Log4j API implementation, but not disabled in the default JULLogManagerimplementation. Thereforelog4j-juloffers a level propagator that modifies the level of JUL loggers, whenever the level of the corresponding Log4j Core loggers change. This feature of course depends onlog4j-core.This PR:
CoreLoggerAdapterand related classes. IMHO users should not usej.u.l.Loggerto modify the configuration of the logging backend.log4j-jul-propagator. Note that the level propagator idea comes from SLF4J/Logback wherejul-to-slf4jintroduces aLevelChangePropagatorinterface, but the implementation is in Logback.o.a.l.l.j.LogManager:sourceClassandsourceMethodparameters, by forwarding these parameters usingLogBuilder#withLocation().Filters are used.The last part probably could be improved: I don't see why we should support
j.u.l.Filters. We could drop the feature in 3.x.