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

OpenTelemetry MDC integration not quite working #26258

Closed
knutwannheden opened this issue Jun 21, 2022 · 8 comments · Fixed by #26298
Closed

OpenTelemetry MDC integration not quite working #26258

knutwannheden opened this issue Jun 21, 2022 · 8 comments · Fixed by #26298
Assignees
Labels
area/tracing kind/bug Something isn't working
Milestone

Comments

@knutwannheden
Copy link
Contributor

Describe the bug

While Quarkus appears to register the trace context fine in the MDC using VertxMDC, it appears that it cannot be accessed using the org.jboss.logging.MDC class. AFAICT the issue appears to be that the static initialization of org.jboss.logmanager.MDC#mdcProvider may not be able to find the io.quarkus.bootstrap.logging.LateBoundMDCProvider as registered in a corresponding META-INF/services file. This appears to be due to the fact that the mentioned field gets initialized too early and not explicitly using the TCCL. At least when I start my application in dev-mode and set a breakpoint in org.jboss.logmanager.MDC#getMDCProvider() the class initialization is triggered by the initialization of io.quarkus.deployment.dev.DevModeMain#log.

In my "ultimate" use case I simply want to use the quarkus-logging-json extension to log the log records (including MDC) in JSON and that should include the MDC. But when the logger attempts to get the MDC using org.jboss.logmanager.ExtLogRecord#getMdcCopy() (which calls org.jboss.logmanager.MDC.copyObject()) the returned result is always empty, since it ends up using the default org.jboss.logmanager.ThreadLocalMDC rather than the io.quarkus.bootstrap.logging.LateBoundMDCProvider implementation.

Expected behavior

The MDC data supplied by the OpenTelemetry integration should be available via the org.jboss.logging.MDC API.

Actual behavior

When using the org.jboss.logging.MDC API and also the quarkus-logging-json extension the MDC data supplied by the Quarkus OpenTelemetry integration is not available.

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.10.0.Final

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

@knutwannheden knutwannheden added the kind/bug Something isn't working label Jun 21, 2022
@quarkus-bot
Copy link

quarkus-bot bot commented Jun 21, 2022

/cc @radcortez

@luneo7
Copy link
Contributor

luneo7 commented Jun 21, 2022

It is actually that the META-INF/services/org.jboss.logmanager.MDCProvider is inside the vertx core extension and not in the independent-projects/bootstrap/runner project... The LateBoundMDCProvider is in the right place... the bootstrap is not being able to see the provider configuration file...

@luneo7
Copy link
Contributor

luneo7 commented Jun 21, 2022

Since LateBoundMDCProvider is only actually used by Vert.x Core (and the default implementation in LateBoundMDCProvider is a no op) maybe one approach is to create a lib containing the META-INF/services/org.jboss.logmanager.MDCProvider and in vertx core add the dependency to that lib and adding the parentFirstArtifacts and runnerParentFirstArtifacts configs so it is properly loaded during startup

@luneo7
Copy link
Contributor

luneo7 commented Jun 21, 2022

With 2.10.0.Final... if you run in native mode, or fat jar... it works though... just fast jar isn't working now...

@knutwannheden
Copy link
Contributor Author

@luneo7 Thanks for the analysis. In the issue I mentioned dev-mode only, but I can confirm that it also doesn't work in the normal launch mode with the default packaging (which I think is fast-jar as you mention).

@luneo7
Copy link
Contributor

luneo7 commented Jun 21, 2022

@geoand @gsmet any hint on how to better fix this?

@geoand
Copy link
Contributor

geoand commented Jun 22, 2022

I'll have a look

@geoand
Copy link
Contributor

geoand commented Jun 22, 2022

This is a little complicated.

As you have seen, implementations of org.jboss.logmanager.MDCProvider need to be loaded by the System ClassLoader (because LogManager is loaded from that ClassLoader - something which cannot change). This also means that the META-INF/org.jboss.logmanager.MDCProvider file is loaded by the System ClassLoader which does not know about the implementation in the Vert.x jar, because that jar is not part of the System ClassLoader, but part of the custom Quarkus ClassLoader used in fast-jar.

The solution here would be for us to enable adding Service Loader files to the root path of the fast-jar. That way the service will be discoverable by the System ClassLoader.

I can put something together next week (hopefully).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tracing kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants