open-telemetry / opentelemetry-java

OpenTelemetry Java SDK
https://opentelemetry.io
Apache License 2.0
1.95k stars 805 forks source link

Log messages from OpenTelemetry Exporter use JUL instead of SLF4J #5715

Closed sergeykad closed 12 months ago

sergeykad commented 1 year ago

Describe the bug At least some of the classes in io.opentelemetry.exporter use JUL logger instead of SLF4J despite https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/1328

Steps to reproduce Check sources of the io.opentelemetry.exporter.internal.grpc.GrpcExporterUtil class for example.

What did you expect to see? I expected it to use SLF4J which can easily be redirected to a logger that my application already uses

What did you see instead? I see that JUL is used, which requires an adapter and extra configuration to use with a different logger.

What version are you using? 1.29.0

Environment Compiler: Temurin 17.0.7 OS: Ubuntu 22.04

Additional context https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/1447

mateuszrzeszutek commented 1 year ago

At least some of the classes in io.opentelemetry.exporter use JUL logger instead of SLF4J despite open-telemetry/opentelemetry-java-instrumentation#1328

All of the SDK & javaagent logging is done through JUL; the issue you mentioned is very outdated. That is by design: we do not want to include an extra dependency for logging, we're aiming to have as little deps as possible.

If you're using the agent, check out this configuration setting -- it allows youto redirect all the javaagent logging to your app's slf4j instance.

sergeykad commented 1 year ago

That's a good point. I missed this setting since I use manual instrumentation and this is listed under the Automatic section.

Maybe it will make sense to move this configuration setting into a common configuration section?

Also, since most projects will want to use a different logger, this decision can actually increase the number of dependencies used by a project. It already will have SLF4J as a dependency, but might need to add jul-to-slf4j as well for redirecting OpenTelemetry logs.

trask commented 1 year ago

Transferring to opentelemetry-java repo where the exporters live

mateuszrzeszutek commented 1 year ago

Maybe it will make sense to move this configuration setting into a common configuration section?

That is unfortunately impossible, this feature depends on the javaagent's internal machinery, and requires bytecode instrumentation.

sergeykad commented 1 year ago

This is strange since I started getting messages through logger instead of stdout after adding otel.javaagent.logging=application and I do not use the Java agent.

trask commented 1 year ago

it seems something else may be going on in your setup, is it possible for you to reduce this to a small standalone repro in order to better understand what's going on?

andrebrait commented 12 months ago

If you happen to have a JUL-to-slf4j or JUL-to-whatever-logging-framework-you-use in your classpath, you'll see the log messages there, but I can confirm the code still uses JUL. I had to use JUL to get a reference to the logger in HttpSender in order to add filters, for example.

EDIT: perhaps you (author, etc.) have one of those bridges in your classpath and hence why you cannot reproduce it?

andrebrait commented 12 months ago

@sergeykad was something that fixed this merged just now?

sergeykad commented 12 months ago

No, but @mateuszrzeszutek this behavior is by design, unless I misunderstood him.

trask commented 12 months ago

@andrebrait are you facing a related issue?

andrebrait commented 12 months ago

@andrebrait are you facing a related issue?

@trask not a related issue but right now it's one of the few things I need to touch JUL for. But it's not specific to OTEL itself.

My application treats any logged SEVERE lines during startup as a deployment failure (don't ask, legacy stuff) and HttpSender logs that when it fails to export a Span.

Having to use JUL to touch that specific log filter is not a problem per se, but the fact it's an internal class is.

trask commented 11 months ago

My application treats any logged SEVERE lines during startup as a deployment failure (don't ask, legacy stuff) and HttpSender logs that when it fails to export a Span.

Having to use JUL to touch that specific log filter is not a problem per se, but the fact it's an internal class is.

@jack-berg what do you think of reducing failed exports from SEVERE to WARNING, under the assumption that networks and endpoints can be flaky?

jack-berg commented 11 months ago

Seems reasonable to me.

trask commented 11 months ago

@andrebrait if this would help, go ahead and open an issue and/or PR, thx!