From b702a9d4d1005d2526fe73d3e566459222f63308 Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Tue, 19 Apr 2022 21:24:10 +0200 Subject: [PATCH] Fix logging instrumentation to pass Context instead of Span (#5881) --- .../log4j/v1_2/CategoryInstrumentation.java | 6 +- .../v1_2/LoggingEventInstrumentation.java | 56 +++---------------- .../mdc/v1_0/LoggerInstrumentation.java | 6 +- .../mdc/v1_0/LoggingEventInstrumentation.java | 13 +++-- 4 files changed, 24 insertions(+), 57 deletions(-) diff --git a/instrumentation/log4j/log4j-mdc-1.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/v1_2/CategoryInstrumentation.java b/instrumentation/log4j/log4j-mdc-1.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/v1_2/CategoryInstrumentation.java index bbe38fba06fa..f1dc4c6a432c 100644 --- a/instrumentation/log4j/log4j-mdc-1.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/v1_2/CategoryInstrumentation.java +++ b/instrumentation/log4j/log4j-mdc-1.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/v1_2/CategoryInstrumentation.java @@ -11,7 +11,7 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; -import io.opentelemetry.api.trace.Span; +import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.field.VirtualField; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; @@ -43,8 +43,8 @@ public static class CallAppendersAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) public static void onEnter(@Advice.Argument(0) LoggingEvent event) { - VirtualField.find(LoggingEvent.class, Span.class) - .set(event, Java8BytecodeBridge.currentSpan()); + VirtualField.find(LoggingEvent.class, Context.class) + .set(event, Java8BytecodeBridge.currentContext()); } } } diff --git a/instrumentation/log4j/log4j-mdc-1.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/v1_2/LoggingEventInstrumentation.java b/instrumentation/log4j/log4j-mdc-1.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/v1_2/LoggingEventInstrumentation.java index 68d58294b8bc..cc11efec6341 100644 --- a/instrumentation/log4j/log4j-mdc-1.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/v1_2/LoggingEventInstrumentation.java +++ b/instrumentation/log4j/log4j-mdc-1.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/log4j/v1_2/LoggingEventInstrumentation.java @@ -14,16 +14,15 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; -import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.field.VirtualField; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; -import java.util.Hashtable; +import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; -import org.apache.log4j.MDC; import org.apache.log4j.spi.LoggingEvent; public class LoggingEventInstrumentation implements TypeInstrumentation { @@ -41,10 +40,6 @@ public void transform(TypeTransformer transformer) { .and(takesArguments(1)) .and(takesArgument(0, String.class)), LoggingEventInstrumentation.class.getName() + "$GetMdcAdvice"); - - transformer.applyAdviceToMethod( - isMethod().and(isPublic()).and(named("getMDCCopy")).and(takesArguments(0)), - LoggingEventInstrumentation.class.getName() + "$GetMdcCopyAdvice"); } @SuppressWarnings("unused") @@ -61,12 +56,16 @@ public static void onExit( return; } - Span span = VirtualField.find(LoggingEvent.class, Span.class).get(event); - if (span == null || !span.getSpanContext().isValid()) { + Context context = VirtualField.find(LoggingEvent.class, Context.class).get(event); + if (context == null) { + return; + } + + SpanContext spanContext = Java8BytecodeBridge.spanFromContext(context).getSpanContext(); + if (!spanContext.isValid()) { return; } - SpanContext spanContext = span.getSpanContext(); switch (key) { case TRACE_ID: value = spanContext.getTraceId(); @@ -83,41 +82,4 @@ public static void onExit( } } } - - @SuppressWarnings("unused") - public static class GetMdcCopyAdvice { - - @SuppressWarnings({"unchecked", "rawtypes"}) - @Advice.OnMethodEnter(suppress = Throwable.class) - public static void onEnter( - @Advice.This LoggingEvent event, - @Advice.FieldValue(value = "mdcCopyLookupRequired", readOnly = false) boolean copyRequired, - @Advice.FieldValue(value = "mdcCopy", readOnly = false) Hashtable mdcCopy) { - // this advice basically replaces the original method - - if (copyRequired) { - copyRequired = false; - - Hashtable mdc = new Hashtable(); - - Hashtable originalMdc = MDC.getContext(); - if (originalMdc != null) { - mdc.putAll(originalMdc); - } - - // Assume already instrumented event if traceId is present. - if (!mdc.containsKey(TRACE_ID)) { - Span span = VirtualField.find(LoggingEvent.class, Span.class).get(event); - if (span != null && span.getSpanContext().isValid()) { - SpanContext spanContext = span.getSpanContext(); - mdc.put(TRACE_ID, spanContext.getTraceId()); - mdc.put(SPAN_ID, spanContext.getSpanId()); - mdc.put(TRACE_FLAGS, spanContext.getTraceFlags().asHex()); - } - } - - mdcCopy = mdc; - } - } - } } diff --git a/instrumentation/logback/logback-mdc-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/mdc/v1_0/LoggerInstrumentation.java b/instrumentation/logback/logback-mdc-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/mdc/v1_0/LoggerInstrumentation.java index 1e4f97ca1edd..3191ca47ce52 100644 --- a/instrumentation/logback/logback-mdc-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/mdc/v1_0/LoggerInstrumentation.java +++ b/instrumentation/logback/logback-mdc-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/mdc/v1_0/LoggerInstrumentation.java @@ -12,7 +12,7 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import ch.qos.logback.classic.spi.ILoggingEvent; -import io.opentelemetry.api.trace.Span; +import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.field.VirtualField; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; @@ -44,8 +44,8 @@ public static class CallAppendersAdvice { @Advice.OnMethodEnter public static void onEnter(@Advice.Argument(value = 0, readOnly = false) ILoggingEvent event) { - VirtualField.find(ILoggingEvent.class, Span.class) - .set(event, Java8BytecodeBridge.currentSpan()); + VirtualField.find(ILoggingEvent.class, Context.class) + .set(event, Java8BytecodeBridge.currentContext()); } } } diff --git a/instrumentation/logback/logback-mdc-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/mdc/v1_0/LoggingEventInstrumentation.java b/instrumentation/logback/logback-mdc-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/mdc/v1_0/LoggingEventInstrumentation.java index 73b11718463d..c1e543bf6277 100644 --- a/instrumentation/logback/logback-mdc-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/mdc/v1_0/LoggingEventInstrumentation.java +++ b/instrumentation/logback/logback-mdc-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/logback/mdc/v1_0/LoggingEventInstrumentation.java @@ -17,12 +17,13 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import ch.qos.logback.classic.spi.ILoggingEvent; -import io.opentelemetry.api.trace.Span; import io.opentelemetry.api.trace.SpanContext; +import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.field.VirtualField; import io.opentelemetry.instrumentation.logback.v1_0.internal.UnionMap; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; +import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; import java.util.HashMap; import java.util.Map; import net.bytebuddy.asm.Advice; @@ -63,13 +64,17 @@ public static void onExit( return; } - Span currentSpan = VirtualField.find(ILoggingEvent.class, Span.class).get(event); - if (currentSpan == null || !currentSpan.getSpanContext().isValid()) { + Context context = VirtualField.find(ILoggingEvent.class, Context.class).get(event); + if (context == null) { + return; + } + + SpanContext spanContext = Java8BytecodeBridge.spanFromContext(context).getSpanContext(); + if (!spanContext.isValid()) { return; } Map spanContextData = new HashMap<>(); - SpanContext spanContext = currentSpan.getSpanContext(); spanContextData.put(TRACE_ID, spanContext.getTraceId()); spanContextData.put(SPAN_ID, spanContext.getSpanId()); spanContextData.put(TRACE_FLAGS, spanContext.getTraceFlags().asHex());