Detect file collisions in SiftingAppender nested appenders (#1041) - #1056
Merged
ceki merged 1 commit intoJul 25, 2026
Merged
Conversation
FileCollisionAnalyser only inspects statically declared file/rolling-file
appenders during the DEPENDENCY_ANALYSIS phase. A SiftingAppender, however,
instantiates its nested appender once per discriminator value at runtime, so
that nested appender never passes through the static collision maps. As a
result two (or more) sifted appenders can silently write to the same file
when the nested file/fileNamePattern does not embed the discriminator key.
Add a dedicated check: when an AppenderModel contains a SiftModel, resolve
the discriminator key from the model tree and verify that the nested
appender's file/fileNamePattern textually references ${key}. If it does not,
every sifted appender resolves to the same target, so emit a warning naming
the appender, the discriminator key and the shared target file. The raw
(pre-substitution) body text is matched on purpose because the discriminator
key is only bound as a substitution property at runtime. When the key cannot
be determined from the configuration, or the nested appender writes to no
file, the check stays silent to avoid false positives.
Verification: new FileCollisionAnalyserTest cases siftShared.xml (nested
file does not reference the key -> warning) and siftDistinct.xml (nested
file embeds ${userId} -> no warning); the existing collision and sift test
suites still pass.
Signed-off-by: seonwoo_jung <79202163+seonwooj0810@users.noreply.github.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.
Fixes #1041
Root cause
FileCollisionAnalyserruns during theDEPENDENCY_ANALYSISphase and only inspects statically declaredFileAppender/RollingFileAppendermodels. ASiftingAppenderinstantiates its nested appender once per discriminator value at runtime, so the nested appender never passes through the static collision maps. When the nestedfile/fileNamePatterndoes not embed the discriminator key, every sifted instance resolves to the same target file and their output silently collides — exactly the case @ceki confirmed and scoped in the issue ("add an analyzer for the case the appender inside SiftingAppender does not make use of the discriminating key").Change
When an
AppenderModelcontains aSiftModel, the analyser now:<discriminator><Key>…</Key>),file/fileNamePatternraw body text (raw on purpose — the key is only bound as a substitution property at runtime), and${<key>}, emits a warning naming the appender, the discriminator key, and the shared target file.The check stays silent when the key cannot be determined from the configuration (e.g. a discriminator using a built-in default key) or when the nested appender writes to no file (e.g. a
ConsoleAppender), to avoid false positives. It is a warning rather than a hard error/skip because the colliding appenders only exist at runtime and cannot be skipped at model-analysis time.Change summary
FileCollisionAnalyser: detect the SiftingAppender variant and warn (SIFT_COLLISION_MESSAGE).Test evidence
Added to
FileCollisionAnalyserTest:siftingAppenderSharedFileIsDetected— nested<file>does not reference the discriminator key → warning is emitted (siftShared.xml, mirrors the issue's reproduction).siftingAppenderDistinctFileIsNotFlagged— nested<file>embeds${userId}→ no warning (siftDistinct.xml).Verification done: built
logback-core, ranFileCollisionAnalyserTest(7/7 pass, incl. the 5 pre-existing collision tests) and the full*Sift*suite (14 run, 0 failures) — no regressions. JDK 21.Automated tooling was used to help prepare this change; it has been reviewed and verified by a human.