-
-
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
Outdated
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
Outdated
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 allLoggerContext
implementations delegate toLoggerRegistry
, which in essence is aMap<MessageFactory, Map<String, Logger>>
. That is, allLoggerRegistry
-producedLogger
s areMessageFactory
-namespaced. This renders thecheckMessageFactory()
check redundant.Put another way,
getLogger("foo", someMF)
will create aLogger
associated withsomeMF
.getLogger("foo")
will create aLogger
associated with theMF
configured for theLC
. In either case, ifsomeMF != the MF configured for the LC
,Logger
s will differ anyway, hencecheckMessageFactory()
will always succeed.