-
- Notifications
You must be signed in to change notification settings - Fork 1.7k
Implement MessageFactory-namespaced logger registry #2936
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
| @rgoers, would you mind reviewing this, please? (Checked with @ppkarwasz, he also agrees with deprecating |
log4j-api/src/main/java/org/apache/logging/log4j/simple/SimpleLoggerContext.java Show resolved Hide resolved
AbstractLogger.checkMessageFactory()MessageFactory-namespaced logger registry 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.
Nice work Volkan!
This might be great for 2.25.0, but I am not sure the problem requires such a drastical changes. By this I mean the deprecations in LoggerRegistry.
Note also that all message factories should implement equals and hashCode (I think this is technically a minor version change).
log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java Show resolved Hide resolved
log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java Show resolved Hide resolved
log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java Outdated Show resolved Hide resolved
log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java Show resolved Hide resolved
log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java Outdated Show resolved Hide resolved
log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java Show resolved Hide resolved
I will merge these changes to
AFAICT, |
log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java Show resolved Hide resolved
log4j-api/src/main/java/org/apache/logging/log4j/spi/LoggerRegistry.java Outdated Show resolved Hide resolved
log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java Outdated Show resolved Hide resolved
log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java Outdated Show resolved Hide resolved
Co-authored-by: Piotr P. Karwasz <piotr.github@karwasz.org>
This feature will be released with `2.24.1` before `2.25.0` this branch is pointing to. Hence, no need to duplicate entries.
Fixes #2933 by deprecating
AbstractLogger.checkMessageFactory()and removing its usages.Rationale
In its current state, all
LoggerContext#getLogger(String, MessageFactory)methods of allLoggerContextimplementations delegate toLoggerRegistry, which in essence is aMap<MessageFactory, Map<String, Logger>>. That is, allLoggerRegistry-producedLoggers areMessageFactory-namespaced. This renders thecheckMessageFactory()check redundant.Put another way,
getLogger("foo", someMF)will create aLoggerassociated withsomeMF.getLogger("foo")will create aLoggerassociated with theMFconfigured for theLC. In either case, ifsomeMF != the MF configured for the LC,Loggers will differ anyway, hencecheckMessageFactory()will always succeed.