Rewrite @Advice.Enter for indy advice (#9887)

laurit · web-flow · commit 6313391d71cf · 2023-11-17T07:49:19.000-08:00
diff --git a/instrumentation/jedis/jedis-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v1_4/JedisInstrumentationModule.java b/instrumentation/jedis/jedis-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v1_4/JedisInstrumentationModule.java
@@ -32,14 +32,4 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(new JedisConnectionInstrumentation(), new JedisInstrumentation());
   }
-
-  @Override
-  public boolean isIndyModule() {
-    // java.lang.NoClassDefFoundError:
-    //      io/opentelemetry/javaagent/instrumentation/jedis/JedisRequestContext
-    //  at redis.clients.jedis.Jedis.flushAll(Jedis.java:367)
-    //  at io.opentelemetry.javaagent.instrumentation.jedis.v1_4
-    //          .JedisClientTest.setup(JedisClientTest.java:49)
-    return false;
-  }
 }
diff --git a/instrumentation/jedis/jedis-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v3_0/JedisInstrumentationModule.java b/instrumentation/jedis/jedis-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v3_0/JedisInstrumentationModule.java
@@ -34,14 +34,4 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(new JedisConnectionInstrumentation(), new JedisInstrumentation());
   }
-
-  @Override
-  public boolean isIndyModule() {
-    // java.lang.NoClassDefFoundError:
-    //      io/opentelemetry/javaagent/instrumentation/jedis/JedisRequestContext
-    //  at redis.clients.jedis.BinaryJedis.flushAll(BinaryJedis.java:595)
-    //  at io.opentelemetry.javaagent.instrumentation.jedis.v3_0
-    //       .Jedis30ClientTest.setup(Jedis30ClientTest.java:50)
-    return false;
-  }
 }
diff --git a/instrumentation/jedis/jedis-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v4_0/JedisInstrumentationModule.java b/instrumentation/jedis/jedis-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jedis/v4_0/JedisInstrumentationModule.java
@@ -30,14 +30,4 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(new JedisConnectionInstrumentation(), new JedisInstrumentation());
   }
-
-  @Override
-  public boolean isIndyModule() {
-    // java.lang.NoClassDefFoundError:
-    //      io/opentelemetry/javaagent/instrumentation/jedis/JedisRequestContext
-    // at redis.clients.jedis.Jedis.set(Jedis.java:4613)
-    // at io.opentelemetry.javaagent.instrumentation.jedis
-    //     .v4_0.Jedis40ClientTest.getCommand(Jedis40ClientTest.java:78)
-    return false;
-  }
 }
diff --git a/instrumentation/kafka/kafka-clients/kafka-clients-0.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkaclients/v0_11/KafkaClientsInstrumentationModule.java b/instrumentation/kafka/kafka-clients/kafka-clients-0.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkaclients/v0_11/KafkaClientsInstrumentationModule.java
@@ -18,12 +18,6 @@ public KafkaClientsInstrumentationModule() {
     super("kafka-clients", "kafka-clients-0.11", "kafka");
   }
 
-  @Override
-  public boolean isIndyModule() {
-    // OpenTelemetryMetricsReporter is not available in app class loader
-    return false;
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(
diff --git a/instrumentation/kafka/kafka-clients/kafka-clients-0.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkaclients/v0_11/metrics/KafkaMetricsInstrumentationModule.java b/instrumentation/kafka/kafka-clients/kafka-clients-0.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkaclients/v0_11/metrics/KafkaMetricsInstrumentationModule.java
@@ -10,10 +10,14 @@
 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 io.opentelemetry.javaagent.extension.instrumentation.internal.injection.ClassInjector;
+import io.opentelemetry.javaagent.extension.instrumentation.internal.injection.InjectionMode;
 import java.util.List;
 
 @AutoService(InstrumentationModule.class)
-public class KafkaMetricsInstrumentationModule extends InstrumentationModule {
+public class KafkaMetricsInstrumentationModule extends InstrumentationModule
+    implements ExperimentalInstrumentationModule {
   public KafkaMetricsInstrumentationModule() {
     super(
         "kafka-clients-metrics",
@@ -24,9 +28,11 @@ public KafkaMetricsInstrumentationModule() {
   }
 
   @Override
-  public boolean isIndyModule() {
-    // OpenTelemetryMetricsReporter is not available in app class loader
-    return false;
+  public void injectClasses(ClassInjector injector) {
+    injector
+        .proxyBuilder(
+            "io.opentelemetry.instrumentation.kafka.internal.OpenTelemetryMetricsReporter")
+        .inject(InjectionMode.CLASS_ONLY);
   }
 
   @Override
diff --git a/instrumentation/kafka/kafka-streams-0.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkastreams/KafkaStreamsInstrumentationModule.java b/instrumentation/kafka/kafka-streams-0.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kafkastreams/KafkaStreamsInstrumentationModule.java
@@ -18,13 +18,6 @@ public KafkaStreamsInstrumentationModule() {
     super("kafka-streams", "kafka-streams-0.11", "kafka");
   }
 
-  @Override
-  public boolean isIndyModule() {
-    // java.lang.NoClassDefFoundError:
-    // io/opentelemetry/javaagent/instrumentation/kafkastreams/StateHolder
-    return false;
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(
diff --git a/instrumentation/quarkus-resteasy-reactive/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/quarkus/resteasy/reactive/QuarkusResteasyReactiveInstrumentationModule.java b/instrumentation/quarkus-resteasy-reactive/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/quarkus/resteasy/reactive/QuarkusResteasyReactiveInstrumentationModule.java
@@ -19,12 +19,6 @@ public QuarkusResteasyReactiveInstrumentationModule() {
     super("quarkus", "jaxrs", "quarkus-resteasy-reactive", "quarkus-resteasy-reactive-3.0");
   }
 
-  @Override
-  public boolean isIndyModule() {
-    // RunAdvice returns OtelRequestContext that is not available to the application class loader
-    return false;
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(
diff --git a/instrumentation/ratpack/ratpack-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackInstrumentationModule.java b/instrumentation/ratpack/ratpack-1.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/ratpack/RatpackInstrumentationModule.java
@@ -20,8 +20,11 @@ public RatpackInstrumentationModule() {
 
   @Override
   public boolean isIndyModule() {
-    // StartAdvice uses both @Advice.Argument(readOnly = false) and return value from an
-    // @OnMethodEnter advice
+    // java.lang.ClassCastException: class
+    // io.opentelemetry.javaagent.shaded.instrumentation.netty.v4_1.internal.AutoValue_ServerContext
+    // cannot be cast to class
+    // io.opentelemetry.javaagent.shaded.instrumentation.netty.v4_1.internal.ServerContext
+    // (io.opentelemetry.javaagent.shaded.instrumentation.netty.v4_1.internal.AutoValue_ServerContext is in unnamed module of loader 'app'; io.opentelemetry.javaagent.shaded.instrumentation.netty.v4_1.internal.ServerContext is in unnamed module of loader io.opentelemetry.javaagent.tooling.instrumentation.indy.InstrumentationModuleClassLoader @7f088b5c)
     return false;
   }
 
diff --git a/instrumentation/reactor/reactor-kafka-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactor/kafka/v1_0/ReactorKafkaInstrumentationModule.java b/instrumentation/reactor/reactor-kafka-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactor/kafka/v1_0/ReactorKafkaInstrumentationModule.java
@@ -19,12 +19,6 @@ public ReactorKafkaInstrumentationModule() {
     super("reactor-kafka", "reactor-kafka-1.0");
   }
 
-  @Override
-  public boolean isIndyModule() {
-    // OpenTelemetryMetricsReporter is not available in app class loader
-    return false;
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(
diff --git a/instrumentation/reactor/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ReactorNettyInstrumentationModule.java b/instrumentation/reactor/reactor-netty/reactor-netty-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/reactornetty/v1_0/ReactorNettyInstrumentationModule.java
@@ -7,10 +7,12 @@
 
 import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
 import static java.util.Arrays.asList;
+import static java.util.Collections.singletonList;
 
 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 java.util.function.BiConsumer;
 import java.util.function.Function;
@@ -24,18 +26,13 @@
  * HttpClient#doOnRequest(BiConsumer)} to pass context from the caller to Reactor to Netty.
  */
 @AutoService(InstrumentationModule.class)
-public class ReactorNettyInstrumentationModule extends InstrumentationModule {
+public class ReactorNettyInstrumentationModule extends InstrumentationModule
+    implements ExperimentalInstrumentationModule {
 
   public ReactorNettyInstrumentationModule() {
     super("reactor-netty", "reactor-netty-1.0");
   }
 
-  @Override
-  public boolean isIndyModule() {
-    // ResponseMonoAdvice uses both @Advice.Local and return value from an @OnMethodEnter advice
-    return false;
-  }
-
   @Override
   public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
     // Introduced in 1.0.0
@@ -47,6 +44,11 @@ public boolean isHelperClass(String className) {
     return className.startsWith("reactor.netty.http.client.HttpClientConfigBuddy");
   }
 
+  @Override
+  public List<String> injectedClassNames() {
+    return singletonList("reactor.netty.http.client.HttpClientConfigBuddy");
+  }
+
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(
diff --git a/instrumentation/spring/spring-kafka-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/kafka/v2_7/SpringKafkaInstrumentationModule.java b/instrumentation/spring/spring-kafka-2.7/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/kafka/v2_7/SpringKafkaInstrumentationModule.java
@@ -18,12 +18,6 @@ public SpringKafkaInstrumentationModule() {
     super("spring-kafka", "spring-kafka-2.7");
   }
 
-  @Override
-  public boolean isIndyModule() {
-    // OpenTelemetryMetricsReporter is not available in app class loader
-    return false;
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(
diff --git a/instrumentation/spymemcached-2.12/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spymemcached/SpymemcachedInstrumentationModule.java b/instrumentation/spymemcached-2.12/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spymemcached/SpymemcachedInstrumentationModule.java
@@ -19,12 +19,6 @@ public SpymemcachedInstrumentationModule() {
     super("spymemcached", "spymemcached-2.12");
   }
 
-  @Override
-  public boolean isIndyModule() {
-    // SyncOperationAdvice uses both @Advice.Local and return value from an @OnMethodEnter advice
-    return false;
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return singletonList(new MemcachedClientInstrumentation());
diff --git a/instrumentation/spymemcached-2.12/javaagent/src/test/groovy/SpymemcachedTest.groovy b/instrumentation/spymemcached-2.12/javaagent/src/test/groovy/SpymemcachedTest.groovy
@@ -6,7 +6,6 @@
 import com.google.common.util.concurrent.MoreExecutors
 import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
 import io.opentelemetry.instrumentation.test.asserts.TraceAssert
-import io.opentelemetry.javaagent.instrumentation.spymemcached.CompletionListener
 import io.opentelemetry.semconv.SemanticAttributes
 import net.spy.memcached.CASResponse
 import net.spy.memcached.ConnectionFactory
@@ -627,15 +626,15 @@ class SpymemcachedTest extends AgentInstrumentationSpecification {
         "$SemanticAttributes.DB_OPERATION" operation
 
         if (error == "canceled") {
-          "${CompletionListener.DB_COMMAND_CANCELLED}" true
+          "spymemcached.command.cancelled" true
         }
 
         if (result == "hit") {
-          "${CompletionListener.MEMCACHED_RESULT}" CompletionListener.HIT
+          "spymemcached.result" "hit"
         }
 
         if (result == "miss") {
-          "${CompletionListener.MEMCACHED_RESULT}" CompletionListener.MISS
+          "spymemcached.result" "miss"
         }
       }
     }
diff --git a/instrumentation/vertx/vertx-kafka-client-3.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/kafka/v3_6/VertxKafkaInstrumentationModule.java b/instrumentation/vertx/vertx-kafka-client-3.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/kafka/v3_6/VertxKafkaInstrumentationModule.java
@@ -19,12 +19,6 @@ public VertxKafkaInstrumentationModule() {
     super("vertx-kafka-client", "vertx-kafka-client-3.6", "vertx");
   }
 
-  @Override
-  public boolean isIndyModule() {
-    // OpenTelemetryMetricsReporter is not available in app class loader
-    return false;
-  }
-
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
     return asList(
diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/AdviceTransformer.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/indy/AdviceTransformer.java
