Fix logging instrumentation to pass Context instead of Span (#5881)

open-telemetry · Apr 19, 2022 · b702a9d · b702a9d
1 parent 1ab1737
commit b702a9d
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 57 deletions.
diff --git a/...n/java/io/opentelemetry/javaagent/instrumentation/log4j/v1_2/CategoryInstrumentation.java b/...n/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/...va/io/opentelemetry/javaagent/instrumentation/log4j/v1_2/LoggingEventInstrumentation.java b/...va/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/...va/io/opentelemetry/javaagent/instrumentation/logback/mdc/v1_0/LoggerInstrumentation.java b/...va/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/...opentelemetry/javaagent/instrumentation/logback/mdc/v1_0/LoggingEventInstrumentation.java b/...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<String, String> 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());