open-telemetry / opentelemetry-java-instrumentation

OpenTelemetry auto-instrumentation and instrumentation libraries for Java
https://opentelemetry.io
Apache License 2.0
1.92k stars 839 forks source link

ContextStorage.addWrapper and javaagent incompatibility #7344

Closed oleborne closed 1 year ago

oleborne commented 1 year ago

When using the JavaAgent, ContextStorageWrappersInstrumentation adds a wrapper as the very last one. The purpose of this wrapper is the following:

We do this instead of using the normal service loader mechanism to make sure there is no dependency on a system property or possibility of a user overriding this since it's required for instrumentation in the agent to work properly. from the Javadoc of ContextStorageWrappersInstrumentation

By being the last wrapper and fully replacing the ContextStorage, this defeats the purpose of the wrapper: not replace the ContextStorage but "enrich" it. All wrappers that were previously installed on the ContextStorage are ignored.

Notably, Spring Boot 3 installs a wrapper on the ContextStorage for the purpose of reacting to scope changes and update trace and span id in slf4j MDC.

Steps to reproduce

  1. create a Spring Boot 3 project with Actuator, Lombok and Web dependencies
  2. Create a basic REST endpoint (hello world) that write logs when invoked
  3. Configure the logs to output trace and span ids: logging.pattern.level=%5p [%X{traceId:-},%X{spanId:-}]
  4. Build and start the service with Otel java agent enabled
  5. Call the REST endpoint

What did you expect to see? I would expect to see the trace and span id in the logs.

What did you see instead? Trace and span are not in the logs since the AgentContextStorage replaced the ContextStorage that was wrapped by Spring Boot.

What version are you using? v1.20.1

Environment Compiler: Coretto 17.0.4.1 OS: macOS 12.6.1

trask commented 1 year ago

hi @oleborne! can you post your reproducer to github? that make it a lot easier for us to take a quick look and make sure we're investigating the right thing

oleborne commented 1 year ago

Here you are: https://github.com/oleborne/java-instrumentation-issue7344

Thanks for your attention