Log correlation for JBoss LogManager

[tobiasstadler]

No description provided.

[eyalkoren]

Thanks for this addition!!

I wouldn't add it this way though. I would like to have only one JBoss logging module nested under the general logging plugin. I believe that the issue here is that you need to use a different (older) dependency in order to test that. What I would do then is just add the instrumentation to the existing apm-jboss-plugin module and add the test, but not run it directly, but as a TestClassWithDependencyRunner, similarly to how log4j 2.6 is tested.

Another alternative is to add submudules under a common JBoss logging module.

However you decide to do that, these would be the requirements:

• Have one JBoss logging module under apm-logging-plugin
• Make this addition as minimal as possible, including sharing as much code as possible between the two tests. They are pretty much identical, apart from how the way you get the log.

In addition, can't we share the instrumentation helper code, only updating org.jboss.logmanager.MDC instead of org.jboss.logging.MDC? If so, this logic should be shared as well.

[eyalkoren]

@tobiasstadler needless to say, if you prefer I'll take it from here- just let me know.
Your contribution is already very helpful!

[tobiasstadler]

@eyalkoren Sorry for the long delay. JBoss Logging and JBoss LogManager are two distinct things. JBoss Logging is mostly a logging facade similar to slf4j, while JBoss LogManager is an actual logging implementation which can act as a backend for JBoss Logging. But there are also adapters for e.g log4j1, log4j2 or slf4j. Maybe we should remove Instrumentation for JBoss logging and only keep one for JBoss LogManager?

[eyalkoren]

Sorry for the long delay.

Extremely long - one day! I started to get worried 😜

JBoss Logging is mostly a logging facade similar to slf4j, while JBoss LogManager is an actual logging implementation which can act as a backend for JBoss Logging.

Right, this use case should be covered through the instrumentation of org.jboss.logging.JBossLogManagerLogger. It's only the standalone jboss-logmanager usage that was left unsupported.

Maybe we should remove Instrumentation for JBoss logging and only keep one for JBoss LogManager?

Currently, this is our way to support JBoss-logging+JUL, which is the default jboss-logging setup. We may consider that when we add JUL log correlation.

JBoss Logging and JBoss LogManager are two distinct things.

OK, then it makes sense to be in a separate module, but please nest both modules under one JBoss logging module for now. Once we get to the JUL correlation task we will check whether we can remove the current instrumentation and if that would be the case, we'll eliminate the redundant plugin level from the module hierarchy.

[tobiasstadler]

JBoss Logging and JBoss LogManager are two distinct things.

OK, then it makes sense to be in a separate module, but please nest both modules under one JBoss logging module for now. Once we get to the JUL correlation task we will check whether we can remove the current instrumentation and if that would be the case, we'll eliminate the redundant plugin level from the module hierarchy.

Ok, so both Instrumentations should be in apm-jboss-logging-plugin? Should they also share the same instrumentation groups?

What do you think about dropping support for org.jboss.logging.JBossLogManagerLogger in JBossLoggingCorrelationInstrumentation, since this is handled in JBossLogManagerCorrelationInstrumentation?

[eyalkoren]

Ok, so both Instrumentations should be in apm-jboss-logging-plugin?

Yes

Should they also share the same instrumentation groups?

Yes, it would be good enough, even though theoretically they can be used together, but if the need arises, we can separate them later

[eyalkoren]

Even better - together at the same module.
Thanks!