Port MessageFactory-namespaced logger registry from 2.x - #4240
Open
vpelikh wants to merge 1 commit into
Open
Conversation
The warning was dead code: `LoggerRegistry.computeIfAbsent` always returns a logger matching the requested message factory, so the mismatch condition could never fire after the message factory-namespaced registry port (apache#4157). Remove it and pin the behavior with a regression test asserting no warning is emitted when a logger is re-requested with a different message factory.
MessageFactory-namespaced logger registry from 2.x (#2962)MessageFactory-namespaced logger registry from 2.x
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Completes the port of #2962 by removing the now-dead
checkMessageFactorymachinery fromLoggerContext.The MessageFactory-namespaced
InternalLoggerRegistryitself was already ported in #4157. This PR removes the leftover core code that only existed to warn about message-factory mismatches under the old name-keyed registry.Why
checkMessageFactorywas dead codeThe
LoggerRegistrynow keys loggers by (name, message factory). As a result:computeIfAbsent(name, mf, newLogger)always returns a logger whose message factory matches the requested one, so the warning branch!loggerMessageFactory.equals(messageFactory)could never fire.getLoggernormalizes anullmessage factory to the default before reaching the registry, so the null-mismatch branch was unreachable as well.The behavior this method guarded against — returning an existing logger created with a different message factory — no longer exists: a request with a different factory now returns a distinct, correctly-bound logger instead of the old logger plus a warning.
2.xremoved this dead method in PR #2936, the same change that introduced the namespaced registry there; this PR mirrors that change in the3.x, keeping it a faithful port.Note on registry placement: as suggested in #2962 (comment), the namespaced registry was kept out of
log4j-api; it lives inlog4j-core(mirroring2.x) rather thanlog4j-kit, since kit sits below core and can't reference the coreLoggertype.Changes
log4j-core—LoggerContextcheckMessageFactory(ExtendedLogger, MessageFactory)and its call site ingetLogger(String, MessageFactory).ExtendedLoggerandStatusLoggerimports.@Nullableto the message-factory parameter ofgetLogger(String, MessageFactory)andhasLogger(String, MessageFactory); refreshed thegetLoggerJavadoc to describe the message-factory-matching semantics.log4j-core-test—LoggerTestgetLogger_String_MessageFactoryMismatchProducesNoWarning: re-requests a logger with a different and with anullmessage factory, and asserts no "created with the message factory" status warning is emitted.builderhardcoded source-line constant in sync with the file layout.Closes #2962