make netty-* indy compatible (#11559)

Co-authored-by: Jonas Kunz <jonas.kunz@elastic.co> Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
open-telemetry · Aug 22, 2024 · d0d39b8 · d0d39b8
1 parent d47289a
commit d0d39b8
Show file tree

Hide file tree

Showing 22 changed files with 283 additions and 160 deletions.
diff --git a/...lemetry/javaagent/instrumentation/finaglehttp/v23_11/ChannelTransportInstrumentation.java b/...lemetry/javaagent/instrumentation/finaglehttp/v23_11/ChannelTransportInstrumentation.java
@@ -34,16 +34,16 @@ public void transform(TypeTransformer transformer) {
   public static class WriteAdvice {
 
     @Advice.OnMethodEnter(suppress = Throwable.class)
-    public static void methodEnter(@Advice.Local("otelScope") Scope scope) {
+    public static Scope methodEnter() {
       Option<Context> ref = Helpers.CONTEXT_LOCAL.apply();
       if (ref.isDefined()) {
-        scope = ref.get().makeCurrent();
+        return ref.get().makeCurrent();
       }
+      return null;
     }
 
     @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class)
-    public static void methodExit(
-        @Advice.Local("otelScope") Scope scope, @Advice.Thrown Throwable thrown) {
+    public static void methodExit(@Advice.Enter Scope scope, @Advice.Thrown Throwable thrown) {
       if (scope != null) {
         scope.close();
       }

diff --git a/...emetry/javaagent/instrumentation/finaglehttp/v23_11/FinagleHttpInstrumentationModule.java b/...emetry/javaagent/instrumentation/finaglehttp/v23_11/FinagleHttpInstrumentationModule.java
@@ -8,11 +8,13 @@
 import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
+import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
 import java.util.Arrays;
 import java.util.List;
 
 @AutoService(InstrumentationModule.class)
-public class FinagleHttpInstrumentationModule extends InstrumentationModule {
+public class FinagleHttpInstrumentationModule extends InstrumentationModule
+    implements ExperimentalInstrumentationModule {
 
   public FinagleHttpInstrumentationModule() {
     super("finagle-http", "finagle-http-23.11");
@@ -27,9 +29,17 @@ public List<TypeInstrumentation> typeInstrumentations() {
   }
 
   @Override
-  public boolean isIndyModule() {
-    // injects helpers to access package-private members
-    return false;
+  public String getModuleGroup() {
+    // relies on netty and needs access to common netty instrumentation classes
+    return "netty";
+  }
+
+  @Override
+  public List<String> injectedClassNames() {
+    // these are injected so that they can access package-private members
+    return Arrays.asList(
+        "com.twitter.finagle.ChannelTransportHelpers",
+        "io.netty.channel.OpenTelemetryChannelInitializerDelegate");
   }
 
   @Override

diff --git a/...etry/javaagent/instrumentation/finaglehttp/v23_11/H2StreamChannelInitInstrumentation.java b/...etry/javaagent/instrumentation/finaglehttp/v23_11/H2StreamChannelInitInstrumentation.java
@@ -42,19 +42,21 @@ public void transform(TypeTransformer transformer) {
   public static class InitServerAdvice {
 
     @Advice.OnMethodExit
-    public static void handleExit(
-        @Advice.Return(readOnly = false) ChannelInitializer<Channel> initializer) {
-      initializer = Helpers.wrapServer(initializer);
+    @Advice.AssignReturned.ToReturned
+    public static ChannelInitializer<Channel> handleExit(
+        @Advice.Return ChannelInitializer<Channel> initializer) {
+      return Helpers.wrapServer(initializer);
     }
   }
 
   @SuppressWarnings("unused")
   public static class InitClientAdvice {
 
     @Advice.OnMethodExit
-    public static void handleExit(
-        @Advice.Return(readOnly = false) ChannelInitializer<Channel> initializer) {
-      initializer = Helpers.wrapClient(initializer);
+    @Advice.AssignReturned.ToReturned
+    public static ChannelInitializer<Channel> handleExit(
+        @Advice.Return ChannelInitializer<Channel> initializer) {
+      return Helpers.wrapClient(initializer);
     }
   }
 }
diff --git a/...javaagent/instrumentation/finaglehttp/v23_11/LocalSchedulerActivationInstrumentation.java b/...javaagent/instrumentation/finaglehttp/v23_11/LocalSchedulerActivationInstrumentation.java
@@ -14,6 +14,7 @@
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
 import net.bytebuddy.asm.Advice;
+import net.bytebuddy.asm.Advice.AssignReturned.ToArguments.ToArgument;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
 
@@ -35,13 +36,14 @@ public void transform(TypeTransformer transformer) {
   public static class WrapRunnableAdvice {
 
     @Advice.OnMethodEnter(suppress = Throwable.class)
-    public static void wrap(@Advice.Argument(value = 0, readOnly = false) Runnable task) {
+    @Advice.AssignReturned.ToArguments(@ToArgument(0))
+    public static Runnable wrap(@Advice.Argument(value = 0) Runnable task) {
       if (task == null) {
-        return;
+        return null;
       }
 
       Context context = Java8BytecodeBridge.currentContext();
-      task = ContextPropagatingRunnable.propagateContext(task, context);
+      return ContextPropagatingRunnable.propagateContext(task, context);
     }
   }
 }
diff --git a/...lemetry/javaagent/instrumentation/finaglehttp/v23_11/PromiseMonitoredInstrumentation.java b/...lemetry/javaagent/instrumentation/finaglehttp/v23_11/PromiseMonitoredInstrumentation.java
@@ -12,6 +12,7 @@
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
 import net.bytebuddy.asm.Advice;
+import net.bytebuddy.asm.Advice.AssignReturned.ToArguments.ToArgument;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
 import scala.Function1;
@@ -34,13 +35,13 @@ public void transform(TypeTransformer transformer) {
   public static class WrapFunctionAdvice {
 
     @Advice.OnMethodEnter(suppress = Throwable.class)
-    public static void wrap(
-        @Advice.Argument(value = 1, readOnly = false) Function1<?, ?> function1) {
+    @Advice.AssignReturned.ToArguments(@ToArgument(1))
+    public static Function1<?, ?> wrap(@Advice.Argument(value = 1) Function1<?, ?> function1) {
       if (function1 == null) {
-        return;
+        return null;
       }
 
-      function1 = Function1Wrapper.wrap(function1);
+      return Function1Wrapper.wrap(function1);
     }
   }
 }
diff --git a/...ry/javaagent/instrumentation/finaglehttp/v23_11/TwitterUtilCoreInstrumentationModule.java b/...ry/javaagent/instrumentation/finaglehttp/v23_11/TwitterUtilCoreInstrumentationModule.java
@@ -10,15 +10,22 @@
 import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
+import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
 import java.util.List;
 
 @AutoService(InstrumentationModule.class)
-public class TwitterUtilCoreInstrumentationModule extends InstrumentationModule {
+public class TwitterUtilCoreInstrumentationModule extends InstrumentationModule
+    implements ExperimentalInstrumentationModule {
 
   public TwitterUtilCoreInstrumentationModule() {
     super("twitter-util-core");
   }
 
+  @Override
+  public String getModuleGroup() {
+    return "netty";
+  }
+
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(

diff --git a/...va/io/opentelemetry/javaagent/instrumentation/netty/v3_8/NettyChannelInstrumentation.java b/...va/io/opentelemetry/javaagent/instrumentation/netty/v3_8/NettyChannelInstrumentation.java
@@ -56,55 +56,54 @@ public void transform(TypeTransformer transformer) {
   public static class ChannelConnectAdvice {
 
     @Advice.OnMethodEnter(suppress = Throwable.class)
-    public static void onEnter(
-        @Advice.This Channel channel,
-        @Advice.Argument(0) SocketAddress remoteAddress,
-        @Advice.Local("otelParentContext") Context parentContext,
-        @Advice.Local("otelRequest") NettyConnectionRequest request,
-        @Advice.Local("otelTimer") Timer timer) {
-
-      parentContext = Java8BytecodeBridge.currentContext();
+    public static NettyScope onEnter(
+        @Advice.This Channel channel, @Advice.Argument(0) SocketAddress remoteAddress) {
+
+      Context parentContext = Java8BytecodeBridge.currentContext();
       Span span = Java8BytecodeBridge.spanFromContext(parentContext);
       if (!span.getSpanContext().isValid()) {
-        return;
+        return null;
       }
 
       VirtualField<Channel, NettyConnectionContext> virtualField =
           VirtualField.find(Channel.class, NettyConnectionContext.class);
       if (virtualField.get(channel) != null) {
-        return;
+        return null;
       }
       virtualField.set(channel, new NettyConnectionContext(parentContext));
 
-      request = NettyConnectionRequest.connect(remoteAddress);
-      timer = Timer.start();
+      NettyConnectionRequest request = NettyConnectionRequest.connect(remoteAddress);
+      Timer timer = Timer.start();
+
+      return new NettyScope(parentContext, request, timer);
     }
 
     @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class)
     public static void onExit(
         @Advice.Return ChannelFuture channelFuture,
         @Advice.Thrown Throwable error,
-        @Advice.Local("otelParentContext") Context parentContext,
-        @Advice.Local("otelRequest") NettyConnectionRequest request,
-        @Advice.Local("otelTimer") Timer timer) {
+        @Advice.Enter NettyScope nettyScope) {
 
-      if (request == null) {
+      if (nettyScope == null) {
         return;
       }
 
       if (error != null) {
-        if (connectionInstrumenter().shouldStart(parentContext, request)) {
+        if (connectionInstrumenter()
+            .shouldStart(nettyScope.getParentContext(), nettyScope.getRequest())) {
           InstrumenterUtil.startAndEnd(
               connectionInstrumenter(),
-              parentContext,
-              request,
+              nettyScope.getParentContext(),
+              nettyScope.getRequest(),
               null,
               error,
-              timer.startTime(),
-              timer.now());
+              nettyScope.getTimer().startTime(),
+              nettyScope.getTimer().now());
         }
       } else {
-        channelFuture.addListener(new ConnectionListener(parentContext, request, timer));
+        channelFuture.addListener(
+            new ConnectionListener(
+                nettyScope.getParentContext(), nettyScope.getRequest(), nettyScope.getTimer()));
       }
     }
   }

diff --git a/...entelemetry/javaagent/instrumentation/netty/v3_8/NettyChannelPipelineInstrumentation.java b/...entelemetry/javaagent/instrumentation/netty/v3_8/NettyChannelPipelineInstrumentation.java
@@ -51,25 +51,25 @@ public void transform(TypeTransformer transformer) {
   public static class ChannelPipelineAdd2ArgsAdvice {
 
     @Advice.OnMethodEnter
-    public static void checkDepth(
-        @Advice.This ChannelPipeline pipeline,
-        @Advice.Argument(1) ChannelHandler handler,
-        @Advice.Local("otelCallDepth") CallDepth callDepth) {
+    public static CallDepth checkDepth(
+        @Advice.This ChannelPipeline pipeline, @Advice.Argument(1) ChannelHandler handler) {
       // Pipelines are created once as a factory and then copied multiple times using the same add
       // methods as we are hooking. If our handler has already been added we need to remove it so we
       // don't end up with duplicates (this throws an exception)
       if (pipeline.get(handler.getClass().getName()) != null) {
         pipeline.remove(handler.getClass().getName());
       }
-      callDepth = CallDepth.forClass(ChannelPipeline.class);
+      CallDepth callDepth = CallDepth.forClass(ChannelPipeline.class);
       callDepth.getAndIncrement();
+      return callDepth;
     }
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void addHandler(
         @Advice.This ChannelPipeline pipeline,
         @Advice.Argument(1) ChannelHandler handler,
-        @Advice.Local("otelCallDepth") CallDepth callDepth) {
+        @Advice.Enter CallDepth callDepth) {
+
       if (callDepth.decrementAndGet() > 0) {
         return;
       }
@@ -82,29 +82,28 @@ public static void addHandler(
   public static class ChannelPipelineAdd3ArgsAdvice {
 
     @Advice.OnMethodEnter
-    public static void checkDepth(
-        @Advice.This ChannelPipeline pipeline,
-        @Advice.Argument(2) ChannelHandler handler,
-        @Advice.Local("otelCallDepth") CallDepth callDepth) {
+    public static CallDepth checkDepth(
+        @Advice.This ChannelPipeline pipeline, @Advice.Argument(2) ChannelHandler handler) {
       // Pipelines are created once as a factory and then copied multiple times using the same add
       // methods as we are hooking. If our handler has already been added we need to remove it so we
       // don't end up with duplicates (this throws an exception)
       if (pipeline.get(handler.getClass().getName()) != null) {
         pipeline.remove(handler.getClass().getName());
       }
-      callDepth = CallDepth.forClass(ChannelPipeline.class);
+      CallDepth callDepth = CallDepth.forClass(ChannelPipeline.class);
       callDepth.getAndIncrement();
+      return callDepth;
     }
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void addHandler(
         @Advice.This ChannelPipeline pipeline,
         @Advice.Argument(2) ChannelHandler handler,
-        @Advice.Local("otelCallDepth") CallDepth callDepth) {
+        @Advice.Enter CallDepth callDepth) {
+
       if (callDepth.decrementAndGet() > 0) {
         return;
       }
-
       ChannelPipelineUtil.wrapHandler(pipeline, handler);
     }
   }

diff --git a/...ava/io/opentelemetry/javaagent/instrumentation/netty/v3_8/NettyInstrumentationModule.java b/...ava/io/opentelemetry/javaagent/instrumentation/netty/v3_8/NettyInstrumentationModule.java
@@ -11,11 +11,13 @@
 import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
+import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
 import java.util.List;
 import net.bytebuddy.matcher.ElementMatcher;
 
 @AutoService(InstrumentationModule.class)
-public class NettyInstrumentationModule extends InstrumentationModule {
+public class NettyInstrumentationModule extends InstrumentationModule
+    implements ExperimentalInstrumentationModule {
   public NettyInstrumentationModule() {
     super("netty", "netty-3.8");
   }
@@ -25,6 +27,11 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
     return hasClassesNamed("org.jboss.netty.handler.codec.http.HttpMessage");
   }
 
+  @Override
+  public String getModuleGroup() {
+    return "netty";
+  }
+
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(

diff --git a/...agent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/NettyScope.java b/...agent/src/main/java/io/opentelemetry/javaagent/instrumentation/netty/v3_8/NettyScope.java
@@ -0,0 +1,36 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.netty.v3_8;
+
+import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.netty.common.internal.NettyConnectionRequest;
+import io.opentelemetry.instrumentation.netty.common.internal.Timer;
+
+/** Container used to carry state between enter and exit advices */
+public class NettyScope {
+
+  private final Context parentContext;
+  private final NettyConnectionRequest request;
+  private final Timer timer;
+
+  public NettyScope(Context parentContext, NettyConnectionRequest request, Timer timer) {
+    this.parentContext = parentContext;
+    this.request = request;
+    this.timer = timer;
+  }
+
+  public Context getParentContext() {
+    return parentContext;
+  }
+
+  public NettyConnectionRequest getRequest() {
+    return request;
+  }
+
+  public Timer getTimer() {
+    return timer;
+  }
+}