Repository navigation
Conversation
In Spring Boot, LoggingApplicationListener can initialize the logging system multiple times. When the Java agent hooks into the initialization, this triggers multiple registration calls of Slf4jApplicationLoggerBridge, causing a warning log that multiple application logger implementations were provided. Track the class of the registered factory and return early if a subsequent registration is for the same factory class. A warning is still logged if a different factory class is provided. Fixes open-telemetry#14889 Signed-off-by: bhuvan-somisetty <somisettybhuvan5@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR suppresses a spurious WARN log emitted by the javaagent's application-logger bridge. In Spring Boot apps, LoggingApplicationListener#initialize (and/or the SLF4J LoggerFactory path) can drive multiple registration calls of the same Slf4jApplicationLoggerBridge into ApplicationLoggerFactory. Previously, every install after the first logged "Multiple application logger implementations were provided…", even when the registering factory was the identical class. The change records the class of the first successfully installed factory and short-circuits subsequent installs of the same class without warning, while still warning when a genuinely different factory class is provided. This addresses issue #14889.
Changes:
- Track the installed factory's class (
actualFactoryClass) and return early on repeat installs of the same class instead of logging a warning. - Preserve the existing warning behavior when a different factory class attempts to install after the first.
- Update/add unit tests to distinguish same-class repeated installs (no warning) from different-class installs (warning), using concrete
FirstFactory/SecondFactorymock targets.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
javaagent-internal-logging-application/src/main/java/io/opentelemetry/javaagent/logging/application/ApplicationLoggerFactory.java |
Adds actualFactoryClass field and same-class early-return in install(...) to suppress the duplicate warning. |
javaagent-internal-logging-application/src/test/java/io/opentelemetry/javaagent/logging/application/ApplicationLoggerFactoryTest.java |
Updates the first-bridge test to use distinct factory classes and adds a test asserting same-class re-install logs no warning; introduces FirstFactory/SecondFactory test helpers. |
Replace AtomicBoolean + separate volatile field with a single AtomicReference<Class<?>> so the winning factory class is stored atomically as part of the CAS gate. Any thread that loses the race now always reads a non-null value from installedFactoryClass.get(), closing the narrow window where a same-class re-install could still emit the spurious warning.
|
This PR has review comments. Review suggestions, whether from maintainers or automated reviewers, aren't always correct or required. Please evaluate each comment on its merits, then make sure each thread has a clear outcome. For example, link to the commit if you applied a suggestion, explain why it wasn't applied, or ask a follow-up question. Automation flags a PR for human review once every review thread has a reply or is marked as resolved. Status across open PRs is visible on the pull request dashboard. |
Suppressing the warning by comparing factory classes in ApplicationLoggerFactory was checking identity at the wrong layer -- Slf4jApplicationLoggerBridge.install() always constructs a fresh instance, so there's no meaningful "same instance" to compare there anyway. Instead, guard install() itself with a static AtomicBoolean so repeat calls from the slf4j LoggerFactory and Spring Boot LoggingApplicationListener instrumentation points only ever hand off one bridge instance downstream. ApplicationLoggerFactory goes back to its original shape, since a second install() call now always means a genuinely different implementation was provided.
|
@trask I've updated the PR description to accurately reflect the current implementation (the Copilot review flagged that it was stale, describing an earlier iteration). All CI checks are green. Ready for another look whenever you have a chance — thanks! |
|
@bhuvan-somisetty no need to continue merging main into your branch, we'll get this merged before the next release! |
|
Thanks for the update! I appreciate it. |
|
@bhuvan-somisetty please ask your AI to stop merging main, it's just wasting CI minutes, thanks! |
|
Got it, will stop, sorry for the noise on CI. Thanks for your patience @trask. |
|
Thank you for your contribution @bhuvan-somisetty! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey. |
Fixes #14889
Problem
When running a Spring Boot app with
-Dotel.javaagent.logging=application, the following warning appears spuriously at startup:Both the SLF4J
LoggerFactoryinstrumentation point and Spring Boot'sLoggingApplicationListenerinstrumentation point can independently callSlf4jApplicationLoggerBridge.install(), andLoggingApplicationListener#initialize()can itself run more than once during startup (e.g. forApplicationEnvironmentPreparedEventandApplicationStartedEvent). Each call toinstall()creates a newSlf4jApplicationLoggerBridgeinstance and hands it toApplicationLoggerBridge.installApplicationLogger(...), which only recognizes the first installed instance as legitimate — any subsequent install (even one backed by the exact same implementation) is treated as a conflicting bridge and logs the warning.Fix
Guard
Slf4jApplicationLoggerBridge.install()itself with a staticAtomicBoolean, so it only ever hands a single instance off toApplicationLoggerBridge.installApplicationLogger(...), no matter how many times SLF4J'sLoggerFactoryor Spring Boot'sLoggingApplicationListenertrigger it. This dedups at the source rather than askingApplicationLoggerBridge/ApplicationLoggerFactoryto recognize repeat installs of the same implementation —ApplicationLoggerFactory#install(...)is unchanged, so a second install there still correctly means a genuinely different implementation was provided and still logs the warning.Changes
Slf4jApplicationLoggerBridge.java: add a staticAtomicBoolean installedgate ininstall()soApplicationLoggerBridge.installApplicationLogger(...)is only ever called once, regardless of how many timesinstall()itself is invoked.ApplicationLoggerInstrumentationTest.java: assert that the "Multiple application logger implementations were provided" warning never appears in the fork-basedTestSpringAppoutput.