-
Notifications
You must be signed in to change notification settings - Fork 850
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
Should instrumentation-api (and library instrumentation) continue using slf4j? #5053
Comments
Does anybody remember what was the original reason not to use jul? |
I suspect it's inherited from dd-trace. We would probably want to be consistent and use jul everywhere, not just instrumentation API. Because it's in the JDK it can't really cause dependency conflicts I guess - would it allow us to remove patch logger and even bridge into the user's app if they use a jul-slf4j or similar bridge? That seems like it could be a nice UX improvement. |
Using jul is problematic because applications, for example wildfly, may attempt to use a custom log manager by calling |
It sounds like there's agreement on using jul at least in the instrumentation-api and library instrumentation. I agree with @laurit that using jul in the javaagent has significant issues that we'd have to weigh carefully. I'll create a separate issue to track bridging agent logging to users app logging since that has come up a couple of times now (#3413 (comment)). |
Is it possible to use jul in instrumentation API but slf4j in agent without creating a logging API to pick one or the other? Or can we assume usage of instrumentation API would be late enough to use jul even from agent? Also recently I added a jul debug statement in the SDK when finishing autoconfiguration - do we expect that (will be released in 1.10) to break wildfly, if debug logging is enabled? We have many cool tricks in the agent for corner cases, such as hiding methods from reflection. It would be cool if there is a trick by instrumenting jul to make using jul safe :) |
this is what PatchLogger is for, we shade all of the agent's (transitive) usages of jul so that they really use PatchLogger -> slf4j |
Ah oops that makes sense. Does it mean we would be able to use jul in agent code too, though it doesn't magically allow bridging into the app? It would be nice to avoid using two different logging APIs in the same codebase. |
yeah, this should work |
Discussed in today's SIG meeting, and decided:
For now at least, the javaagent will continue to shade all jul usage to PatchLogger.java for the reasons @laurit mentioned above #5053 (comment), and we will explore redirecting logging to user application logging separately in #5059 Closing this discussion issue and will open a new issue with this plan. |
Currently
instrumentation-api
depends on slf4j, while OpenTelemetry API depends only on java.util.logging.The text was updated successfully, but these errors were encountered: