open-telemetry / opentelemetry-java-instrumentation

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

Inject data in log4j2 that is also propagated via TextMapPropagator #6588

Closed TopoDiFogna closed 1 year ago

TopoDiFogna commented 1 year ago

Hello,

I currently have created an extension for the otel agent that uses a TextMapPropagator to propagate a custom header; I need to use this extension, together with the agent, of course, with a spring boot application.

My custom TextMapPropagator extracts the header, which value is a JSON, and creates a custom object that implements ImplicitContextKeyed; this TextMapPropagator is then registered using a ConfigurablePropagatorProvider

This object is saved inside the context using the <C> Context extract(Context context, @Nullable C carrier, TextMapGetter<C> getter); method inherited from the interface using as key a public static final ContextKey<LoggingContext> defined in this custom object, then injects the same header using the <C> void inject(Context context, @Nullable C carrier, TextMapSetter<C> setter); method in the downstream http calls using the same key (reference); this works like a charm 😄 .

Now I need to inject some data contained in this custom object (deserialized from JSON) in every log line printed while managing the incoming http call so I defined a new TypeInstrumentation that creates an @Advice.OnMethodExit that basically creates a new ContextDataInjector which first calls injectContextData() on the original ContextDataInjector and then executes its custom logic.

The main problem is that when I try to extract from the context my custom object which was inserted by my TextMapPropagator, and I know that is there from the debugger, cannot be extracted because the public static final ContextKey<LoggingContext> is a different object since it my custom object is loaded also from another classloader.

For the TextMapPropagator my custom object is loaded from the application classloader but for the ContextDataInjector my custom object is loaded from the agent classpath.

Is there something that I'm missing in my implementation?

I followed this readme to implement my custom Instrumentation module for log4j.

trask commented 1 year ago

hi @TopoDiFogna! have you seen https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/docs/logger-mdc-instrumentation.md#logger-mdc-auto-instrumentation already?

TopoDiFogna commented 1 year ago

Actually yes, and I also peeked how traceId and spanId are injected inside MDC for log4j2 from the classes present here.

The main difference is that when in my code I do: Context context = Java8BytecodeBridge.currentContext(); from the debugger I can see there the entry I need, but just after that line I do: LoggingContext loggingContext = context.get(LOGGING_CONTEXT_KEY); and here the LOGGING_CONTEXT_KEY, despite being declared static public final, it is loaded again from a different classloader; This means that I have 2 instances of LOGGING_CONTEXT_KEY and the matching inside context.get() is done by reference so I don't get the value associated to that key 😢

LoggingContext is my object deserialized from json contained in the incoming requests header, it is injected in the context by my custom TextMapPropagator and contains the previously mentioned public static final ContextKey<LoggingContext> LOGGING_CONTEXT_KEY

TopoDiFogna commented 1 year ago

I just noted that for log4j version 2.17 there's more code. I'll try to review my code following what is there and report asap

laurit commented 1 year ago

Agent code is isolated from application code. If you need to share a key between the application and agent code you'll have to define it in boot loader, or build an api in boot loader that bridges the calls between application and agent.

TopoDiFogna commented 1 year ago

I was just saying that after having updated the code I noticed that the common key is loaded inside the application classloader by the instrumentation module and it is loaded again by the TextMapPropagator in the agent classloader.

@laurit how can be achieved what you are suggesting ?

Main goal of this extension was to be plug and play to the SpringBoot application with the agent so it can be included only when needed, so modifying the application itself can be problematic.

laurit commented 1 year ago

See custom distro sample on how to add classes to boot loader. Currently there is no easy way to do the same with an extension, though if you really want to do it you could probably get access to Instrumentation through https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/javaagent-bootstrap/src/main/java/io/opentelemetry/javaagent/bootstrap/InstrumentationHolder.java and use https://docs.oracle.com/javase/7/docs/api/java/lang/instrument/Instrumentation.html#appendToBootstrapClassLoaderSearch(java.util.jar.JarFile)

TopoDiFogna commented 1 year ago

Thank you, I'll try the custom distro example first then if that's the "easy" way.

trask commented 1 year ago

hi @TopoDiFogna, I'm not super clear on your use case, but baggage might be a good option, this was recently implemented for logback, and could be implemented similarly for log4j2

github-actions[bot] commented 1 year ago

This has been automatically marked as stale because it has been marked as needing author feedback and has not had any activity for 7 days. It will be closed automatically if there is no response from the author within 7 additional days from this comment.