Skip to content

Port MessageFactory-namespaced logger registry from 2.x - #4240

Open
vpelikh wants to merge 1 commit into
apache:mainfrom
vpelikh:GH-2962
Open

Port MessageFactory-namespaced logger registry from 2.x#4240
vpelikh wants to merge 1 commit into
apache:mainfrom
vpelikh:GH-2962

Conversation

@vpelikh

@vpelikh vpelikh commented Aug 4, 2026

Copy link
Copy Markdown

Summary

Completes the port of #2962 by removing the now-dead checkMessageFactory machinery from LoggerContext.
The MessageFactory-namespaced InternalLoggerRegistry itself 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 checkMessageFactory was dead code

The LoggerRegistry now 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.
  • getLogger normalizes a null message 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.x removed this dead method in PR #2936, the same change that introduced the namespaced registry there; this PR mirrors that change in the 3.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 in log4j-core (mirroring 2.x) rather than log4j-kit, since kit sits below core and can't reference the core Logger type.

Changes

  1. log4j-coreLoggerContext
    • Removed private checkMessageFactory(ExtendedLogger, MessageFactory) and its call site in getLogger(String, MessageFactory).
    • Removed the now-unused ExtendedLogger and StatusLogger imports.
    • Added @Nullable to the message-factory parameter of getLogger(String, MessageFactory) and hasLogger(String, MessageFactory); refreshed the getLogger Javadoc to describe the message-factory-matching semantics.
  2. log4j-core-testLoggerTest
    • Added regression test getLogger_String_MessageFactoryMismatchProducesNoWarning: re-requests a logger with a different and with a null message factory, and asserts no "created with the message factory" status warning is emitted.
    • Kept the builder hardcoded source-line constant in sync with the file layout.

Closes #2962

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.
@vpelikh vpelikh changed the title Port MessageFactory-namespaced logger registry from 2.x (#2962) Port MessageFactory-namespaced logger registry from 2.x Aug 4, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

1 participant