Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MDC Properties Not Associated with Log Statements #709

Open
naser-ayat opened this issue Oct 5, 2023 · 3 comments
Open

MDC Properties Not Associated with Log Statements #709

naser-ayat opened this issue Oct 5, 2023 · 3 comments

Comments

@naser-ayat
Copy link

Starting from version 1.4.8 of Logback, an issue has been observed where MDC properties are no longer associated with log statements when a custom MDCAdapter, for instance com.twitter.inject.logging.FinagleMDCAdapter, is set in the application.

The likely root cause is that, beginning with version 1.4.8, Logback obtains the MDCAdapter from ch.qos.logback.classic.LoggerContext instead of directly getting it from MDC. Consequently, the application ends up using two separate and independent MDCAdapters concurrently.

You can find a simplified application created to reproduce this issue in the following repository:
https://github.com/naser-ayat/logback-bug-reproducer

@ceki
Copy link
Member

ceki commented Oct 6, 2023

@naser-ayat As of logback 1.4..8 (commit ca7fbc7 LoggingEvent will fetch MDC data from the MDCAdapter attached to its LoggingContext. Previously, the MDCAdapter instance was located statically through the MDC.MDC.getMDCAdapter() method.

You can solve this problem by adding the following line in your code:

ctx.setMDCAdapter(MDC.getMDCAdapter()) where ctx is the LoggerContext in use

FinagleMDCInitializer assumes calling MDC.mdcAdapter = new FinagleMDCAdapter is sufficient to set a custom MDCAdapter. This assumption no longer holds, and the following call is also necessary

ctx.setMDCAdapter(MDC.getMDCAdapter()) where ctx is the LoggerContext in use

See also the previous discussion of this issue at spring-projects/spring-boot#36177

@naser-ayat
Copy link
Author

@ceki Looking at the setMDCAdapter code (as shown below), it is important to note that only the first call succeeds, but the subsequent calls have no impact. Is there any way to make sure that my call is the first call?

if(this.mdcAdapter ==  null) {
   this.mdcAdapter = anAdapter;
} else {
   StatusManager sm = getStatusManager();
   sm.add(new ErrorStatus("mdcAdapter cannot be set multiple times", this, new IllegalStateException("mdcAdapter 
              already set")));
}

@MikeDombo
Copy link

Also take note that there will be a null pointer exception in https://github.com/qos-ch/logback/blame/8a746eb0eff709ac036fb4d6ab0719aaffbc0702/logback-classic/src/main/java/ch/qos/logback/classic/spi/LoggingEvent.java#L411 if no MDC is set. This unfortunately breaks any upgrade from 1.3.7 to 1.3.8 without making additional changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants