Skip to content

test(spring-boot): Keep the Log4j2 LoggerContext reachable in appender auto-configuration tests - #6488

Merged
markushi merged 1 commit into
mainfrom
fix/log4j2-appender-test-loggercontext-gc
Nov 6, 2026
Merged

markushi merged 1 commit into
mainfrom
fix/log4j2-appender-test-loggercontext-gc

Conversation

@markushi

@markushi markushi commented Nov 6, 2026

Copy link
Copy Markdown
Member

📜 Description

SentryLog4j2AppenderAutoConfigurationTest (in sentry-spring-boot-jakarta and sentry-spring-boot-4) now keeps a strong reference to the Log4j2 LoggerContext for the whole test. Before, it read the context again with LogManager.getContext(false) on every access.

Test-only change.

💡 Motivation and Context

ClassLoaderContextSelector holds each LoggerContext only through a WeakReference. The test kept no strong reference. If a GC ran during the Spring context startup, Log4j2 collected the context and made a new one with a fresh DefaultConfiguration. Appenders that the test added before were then gone.

This made does not configure SentryAppender when appender is already configured fail: the root logger then had only SENTRY_APPENDER, not customAppender. The result depends on GC timing. It fails every time on my machine on main, and it failed twice in a row on the Build Job ubuntu-latest CI job of #6231, which does not touch Spring or Log4j2.

💚 How did you test it?

  • Before the fix: the test fails on main every time on my machine, also when run alone.
  • With temporary debug output, the LoggerContext identity inside the Spring context was different from the one before it. When the test held a strong reference, the identity stayed the same and the test passed.
  • After the fix: ./gradlew :sentry-spring-boot-jakarta:test :sentry-spring-boot-4:test passes.

📝 Checklist

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.
  • Public API changes reviewed by another Mobile SDK team member or implemented according to the develop docs spec.

🔮 Next steps

Rerun CI on #6231 after this merges.

🤖 Generated with Claude Code

#skip-changelog

@sentry

sentry Bot commented Nov 6, 2026

Copy link
Copy Markdown

📲 Install Builds

Android

🔗 App Name App ID Version Configuration
SDK Size io.sentry.tests.size 8.63.0 (1) release

⚙️ sentry-android Build Distribution Settings

@markushi
markushi marked this pull request as ready for review November 6, 2026 09:01
@markushi
markushi requested review from a team and adinauer as code owners November 6, 2026 09:01
@markushi
markushi requested review from romtsn and runningcode and removed request for a team November 6, 2026 09:01
@markushi markushi added the ship-it PR is ready to merge from a reviewer perspective label Nov 6, 2026

@runningcode runningcode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@markushi
markushi merged commit c1967f5 into main Nov 6, 2026
47 checks passed
@markushi
markushi deleted the fix/log4j2-appender-test-loggercontext-gc branch November 6, 2026 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ship-it PR is ready to merge from a reviewer perspective

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants