qos-ch / slf4j

Simple Logging Facade for Java
http://www.slf4j.org
MIT License
2.32k stars 980 forks source link

MDC Usage regression #388

Open pauljean opened 7 months ago

pauljean commented 7 months ago

Hello, Currently i'm using Slf4j version 2.0.11 and Logback version 1.4.14 I'm working on an old products with a lot of code and using a lot of thread, so I need to have MDC properties thread inheritance. So I force the usage of a BasicMDCAdapter by setting the MDC.mdcAdapter to new BasicMDCAdapter() when my program starts.

The problem is that, using Slf4j version 2.0.11 and Logback version 1.4.14 this doesn't work anymore. I don't have any MDC info in my logs. Could you please tell me what is the correct way to use your BasicMDCAdapter ? or tell me another way to have MDC properties thread inheritance ?

Many thanks

ceki commented 7 months ago

Off the top of my head, the idea is to get a copy of the MDC map from the parent thread, pass it to the child thread and have the child thread insert it into the MDC programmatically.

pauljean commented 7 months ago

Doing it programmatically is an extreme solution, since it is an old project with millions of lines of code. I am looking for a solution that will avoid modifying the entire project.

ceki commented 7 months ago

Which version of slf4j and logback are you upgrading from?

pauljean commented 7 months ago

I upgraded from logback 1.2.9 and slf4j 1.7.26. we made an adaptation to use the MdcAdapterReplacer like this MdcAdapterReplacer.replaceMdcAdapter(new BasicMDCAdapter()); and it worked fine. but with the upgraded version it no longer works.

ceki commented 7 months ago

Hello @pauljean I am unfamiliar with MdcAdapterReplacer. Do you have a reference?

pauljean commented 7 months ago

it's just an adaptation, which allowed us to update the context. with the upgrade it no longer works. Map<String, String> oldMap = MDC.getCopyOfContextMap(); MDC.mdcAdapter = new BasicMDCAdapter(); if (oldMap != null) { MDC.setContextMap(oldMap); }

ceki commented 7 months ago

In more recent versions of logback, the MdcAdapter that you wish to use needs to be attached to the LoggerContext as well, in addition to MDC.mdcAdapter.

See LogbackServiceProvider, lines 44 or so.

pauljean commented 7 months ago

Here is a test that allows to reproduce the problem. run ./compile.sh and ./run.sh java-slf4j-test.zip

ceki commented 7 months ago

If you set <configuration debug="true"/> you will see this printed:

22:43:07,129 |-ERROR in ch.qos.logback.classic.LoggerContext[default] - mdcAdapter cannot be set multiple times java.lang.IllegalStateException: mdcAdapter already set
        at java.lang.IllegalStateException: mdcAdapter already set
        at      at ch.qos.logback.classic.LoggerContext.setMDCAdapter(LoggerContext.java:418)
        at      at Main.main(Main.java:40)

So it looks like the code prevents the mdcAdapter from being set anew. I have to think about relaxing this hurdle.

pauljean commented 7 months ago

we don't understand why you locked the code to prevent a new definition of the MDCAdapter. It will be very useful if you relax the definition of the MDCAdapter. thank you for keeping us informed

Many thanks