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 Usage regression #388

Open
pauljean opened this issue Jan 22, 2024 · 10 comments
Open

MDC Usage regression #388

pauljean opened this issue Jan 22, 2024 · 10 comments

Comments

@pauljean
Copy link

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
Copy link
Member

ceki commented Jan 22, 2024

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
Copy link
Author

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
Copy link
Member

ceki commented Jan 22, 2024

Which version of slf4j and logback are you upgrading from?

@pauljean
Copy link
Author

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
Copy link
Member

ceki commented Jan 23, 2024

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

@pauljean
Copy link
Author

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
Copy link
Member

ceki commented Jan 23, 2024

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
Copy link
Author

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

@ceki
Copy link
Member

ceki commented Jan 23, 2024

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
Copy link
Author

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

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

2 participants