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
ramanathan1504
added a commit
to ramanathan1504/oss-cli
that referenced
this pull request
Aug 22, 2026
…163) Found by benchmarking rather than by review. Asked "which of my recorded pull request reviews are waiting on me", the loop searched the issue index three times, matched titles containing the word "review", and concluded -- carefully, and wrongly -- that the machine holds no such data: · recall pull request review waiting on me requested reviewer · recall review requested awaiting my review pending approval · recall ramanathanbscmca my pull requests authored by me "I can't find any pull request reviews assigned to you — and I don't think the index actually contains that kind of data." `oss hub` answers the same question correctly in eight seconds, from the review ledger. `recall` searches synced issues, and the ledger is not one. So this was not a missing index: it was the loop being blind to the tool it lives inside. Every answer oss already computes -- the ledger, follow-ups, the backlog, the picker -- was unreachable from inside it. The list of what may be asked is Askable's, not a new one. That table exists because the local board had the same need under the same constraint: everything on it reads, nothing on it writes, and a test in the serve package fails the build if a writing command is ever added. Reusing it means the page and the loop can never disagree about what is safe to run unattended, and this tool inherits that test instead of needing its own. Same question, same rung, after: · oss → Pull Request Intelligence Report · oss → apache/logging-log4j2 #4228 changes pushed,merged,reply:vy "#4218 — changes requested, author pushed and merged the base branch, but no reply, so there's new code to look at without an explanation attached. #4185 — a stale approval: you approved it, the author has since pushed ... a stale approval looks green to everyone else even though you haven't seen the current diff." Correct, and past what the command alone says: hub reports which are waiting, this reasoned about why each one is and which is riskiest. Run in this process rather than by spawning a second oss -- the store is already open here. Both streams are captured, because the first version let the inner command's own status line print through: asking `hub` put seventeen "3 of 17 — apache/logging-log4j2#4240" lines into the middle of a transcript belonging to a different question. mvn verify: 762 tests, 0 failures. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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