Add a ClassAndMethod class to Instrumentation API

docs/contributing/writing-instrumentation.md

@@ -384,3 +384,18 @@ dependencies {
   testImplementation(project(":instrumentation:yarpc-1.0:javaagent"))
 }
 ```
+
+## Why we don't use ByteBuddy @Advice.Origin Method
+
+Instead of
+```
+@Advice.Origin Method method
+```
+we prefer to use
+```
+@Advice.Origin("#t") Class<?> declaringClass,
+@Advice.Origin("#m") String methodName
+```
+because the former inserts a call to `Class.getMethod(...)` in transformed class. In contrast,
+getting the declaring class and method name is just loading constants from constant pool, which is
+a much simpler operation.

instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/SpanNames.java

@@ -5,6 +5,7 @@
 
 package io.opentelemetry.instrumentation.api.tracer;
 
+import io.opentelemetry.instrumentation.api.util.ClassAndMethod;
 import java.lang.reflect.Method;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
@@ -37,6 +38,14 @@ public static String fromMethod(Class<?> clazz, @Nullable Method method) {
     return fromMethod(clazz, method == null ? "<unknown>" : method.getName());
   }
 
+  /**
+   * This method is used to generate a span name based on a method. Anonymous classes are named
+   * based on their parent.
+   */
+  public static String fromMethod(ClassAndMethod classAndMethod) {
+    return fromMethod(classAndMethod.declaringClass(), classAndMethod.methodName());
+  }
+
   /**
    * This method is used to generate a span name based on a method. Anonymous classes are named
    * based on their parent.

instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/server/ClassAndMethod.java → instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/util/ClassAndMethod.java

@@ -3,7 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.javaagent.instrumentation.rmi.server;
+package io.opentelemetry.instrumentation.api.util;
 
 import com.google.auto.value.AutoValue;
 

instrumentation/external-annotations/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/extannotations/ExternalAnnotationAttributesExtractor.java

@@ -6,6 +6,7 @@
 package io.opentelemetry.javaagent.instrumentation.extannotations;
 
 import io.opentelemetry.instrumentation.api.instrumenter.code.CodeAttributesExtractor;
+import io.opentelemetry.instrumentation.api.util.ClassAndMethod;
 import javax.annotation.Nullable;
 
 final class ExternalAnnotationAttributesExtractor

instrumentation/external-annotations/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/extannotations/ExternalAnnotationInstrumentation.java

@@ -18,6 +18,7 @@
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.Scope;
 import io.opentelemetry.instrumentation.api.config.Config;
+import io.opentelemetry.instrumentation.api.util.ClassAndMethod;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
 import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;

instrumentation/external-annotations/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/extannotations/ExternalAnnotationSingletons.java

@@ -8,6 +8,7 @@
 import io.opentelemetry.api.GlobalOpenTelemetry;
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
 import io.opentelemetry.instrumentation.api.instrumenter.code.CodeSpanNameExtractor;
+import io.opentelemetry.instrumentation.api.util.ClassAndMethod;
 
 public final class ExternalAnnotationSingletons {
 

instrumentation/methods/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/methods/MethodInstrumentation.java

@@ -15,9 +15,9 @@
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.Scope;
 import io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationEndSupport;
+import io.opentelemetry.instrumentation.api.util.ClassAndMethod;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
-import java.lang.reflect.Method;
 import java.util.Set;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
@@ -55,30 +55,34 @@ public static class MethodAdvice {
 
     @Advice.OnMethodEnter(suppress = Throwable.class)
     public static void onEnter(
-        @Advice.Origin Method method,
+        @Advice.Origin("#t") Class<?> declaringClass,
+        @Advice.Origin("#m") String methodName,
+        @Advice.Local("otelMethod") ClassAndMethod classAndMethod,
         @Advice.Local("otelContext") Context context,
         @Advice.Local("otelScope") Scope scope) {
       Context parentContext = currentContext();
-      if (!instrumenter().shouldStart(parentContext, method)) {
+      classAndMethod = ClassAndMethod.create(declaringClass, methodName);
+      if (!instrumenter().shouldStart(parentContext, classAndMethod)) {
         return;
       }
 
-      context = instrumenter().start(parentContext, method);
+      context = instrumenter().start(parentContext, classAndMethod);
       scope = context.makeCurrent();
     }
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void stopSpan(
-        @Advice.Origin Method method,
+        @Advice.Origin("#r") Class<?> returnType,
+        @Advice.Local("otelMethod") ClassAndMethod classAndMethod,
         @Advice.Local("otelContext") Context context,
         @Advice.Local("otelScope") Scope scope,
         @Advice.Return(typing = Assigner.Typing.DYNAMIC, readOnly = false) Object returnValue,
         @Advice.Thrown Throwable throwable) {
       scope.close();
 
       returnValue =
-          AsyncOperationEndSupport.create(instrumenter(), Void.class, method.getReturnType())
-              .asyncEnd(context, method, returnValue, throwable);
+          AsyncOperationEndSupport.create(instrumenter(), Void.class, returnType)
+              .asyncEnd(context, classAndMethod, returnValue, throwable);
     }
   }
 }

instrumentation/methods/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/methods/MethodSingletons.java

@@ -10,23 +10,23 @@
 import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.SpanNameExtractor;
 import io.opentelemetry.instrumentation.api.tracer.SpanNames;
-import java.lang.reflect.Method;
+import io.opentelemetry.instrumentation.api.util.ClassAndMethod;
 
 public final class MethodSingletons {
   private static final String INSTRUMENTATION_NAME = "io.opentelemetry.methods";
 
-  private static final Instrumenter<Method, Void> INSTRUMENTER;
+  private static final Instrumenter<ClassAndMethod, Void> INSTRUMENTER;
 
   static {
-    SpanNameExtractor<Method> spanName = SpanNames::fromMethod;
+    SpanNameExtractor<ClassAndMethod> spanName = SpanNames::fromMethod;
 
     INSTRUMENTER =
-        Instrumenter.<Method, Void>builder(
+        Instrumenter.<ClassAndMethod, Void>builder(
                 GlobalOpenTelemetry.get(), INSTRUMENTATION_NAME, spanName)
             .newInstrumenter(SpanKindExtractor.alwaysInternal());
   }
 
-  public static Instrumenter<Method, Void> instrumenter() {
+  public static Instrumenter<ClassAndMethod, Void> instrumenter() {
     return INSTRUMENTER;
   }
 

instrumentation/opentelemetry-annotations-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/otelannotations/WithSpanInstrumentation.java

@@ -115,12 +115,17 @@ public static class WithSpanAdvice {
 
     @Advice.OnMethodEnter(suppress = Throwable.class)
     public static void onEnter(
-        @Advice.Origin Method method,
+        @Advice.Origin Method originMethod,
+        @Advice.Local("otelMethod") Method method,
         @Advice.Local("otelOperationEndSupport")
             AsyncOperationEndSupport<Method, Object> operationEndSupport,
         @Advice.Local("otelContext") Context context,
         @Advice.Local("otelScope") Scope scope) {
 
+      // Every usage of @Advice.Origin Method is replaced with a call to Class.getMethod, copy it
+      // to local variable so that there would be only one call to Class.getMethod.
+      method = originMethod;
+
       Instrumenter<Method, Object> instrumenter = instrumenter();
       Context current = Java8BytecodeBridge.currentContext();
 
@@ -134,7 +139,7 @@ public static void onEnter(
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void stopSpan(
-        @Advice.Origin Method method,
+        @Advice.Local("otelMethod") Method method,
         @Advice.Local("otelOperationEndSupport")
             AsyncOperationEndSupport<Method, Object> operationEndSupport,
         @Advice.Local("otelContext") Context context,
@@ -154,14 +159,19 @@ public static class WithSpanAttributesAdvice {
 
     @Advice.OnMethodEnter(suppress = Throwable.class)
     public static void onEnter(
-        @Advice.Origin Method method,
+        @Advice.Origin Method originMethod,
+        @Advice.Local("otelMethod") Method method,
         @Advice.AllArguments(typing = Assigner.Typing.DYNAMIC) Object[] args,
         @Advice.Local("otelOperationEndSupport")
             AsyncOperationEndSupport<MethodRequest, Object> operationEndSupport,
         @Advice.Local("otelRequest") MethodRequest request,
         @Advice.Local("otelContext") Context context,
         @Advice.Local("otelScope") Scope scope) {
 
+      // Every usage of @Advice.Origin Method is replaced with a call to Class.getMethod, copy it
+      // to local variable so that there would be only one call to Class.getMethod.
+      method = originMethod;
+
       Instrumenter<MethodRequest, Object> instrumenter = instrumenterWithAttributes();
       Context current = Java8BytecodeBridge.currentContext();
       request = new MethodRequest(method, args);
@@ -176,7 +186,7 @@ public static void onEnter(
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void stopSpan(
-        @Advice.Origin Method method,
+        @Advice.Local("otelMethod") Method method,
         @Advice.Local("otelOperationEndSupport")
             AsyncOperationEndSupport<MethodRequest, Object> operationEndSupport,
         @Advice.Local("otelRequest") MethodRequest request,

instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/server/RemoteServerInstrumentation.java

@@ -16,6 +16,7 @@
 
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.Scope;
+import io.opentelemetry.instrumentation.api.util.ClassAndMethod;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
 import io.opentelemetry.javaagent.instrumentation.api.CallDepth;

instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/server/RmiServerAttributesExtractor.java

@@ -6,6 +6,7 @@
 package io.opentelemetry.javaagent.instrumentation.rmi.server;
 
 import io.opentelemetry.instrumentation.api.instrumenter.rpc.RpcAttributesExtractor;
+import io.opentelemetry.instrumentation.api.util.ClassAndMethod;
 
 final class RmiServerAttributesExtractor extends RpcAttributesExtractor<ClassAndMethod, Void> {
 

instrumentation/rmi/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/rmi/server/RmiServerSingletons.java

@@ -9,6 +9,7 @@
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
 import io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor;
 import io.opentelemetry.instrumentation.api.instrumenter.rpc.RpcSpanNameExtractor;
+import io.opentelemetry.instrumentation.api.util.ClassAndMethod;
 
 public final class RmiServerSingletons {
 

instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/response/ResponseSendAdvice.java

@@ -9,19 +9,22 @@
 
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.Scope;
+import io.opentelemetry.instrumentation.api.util.ClassAndMethod;
 import io.opentelemetry.javaagent.instrumentation.api.CallDepth;
 import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
 import io.opentelemetry.javaagent.instrumentation.servlet.common.response.HttpServletResponseAdviceHelper;
 import jakarta.servlet.http.HttpServletResponse;
-import java.lang.reflect.Method;
 import net.bytebuddy.asm.Advice;
 
 @SuppressWarnings("unused")
 public class ResponseSendAdvice {
 
   @Advice.OnMethodEnter(suppress = Throwable.class)
   public static void start(
-      @Advice.Origin Method method,
+      @Advice.This Object response,
+      @Advice.Origin("#t") Class<?> declaringClass,
+      @Advice.Origin("#m") String methodName,
+      @Advice.Local("otelMethod") ClassAndMethod classAndMethod,
       @Advice.Local("otelContext") Context context,
       @Advice.Local("otelScope") Scope scope,
       @Advice.Local("otelCallDepth") CallDepth callDepth) {
@@ -33,23 +36,25 @@ public static void start(
     Context parentContext = Java8BytecodeBridge.currentContext();
     // Don't want to generate a new top-level span
     if (Java8BytecodeBridge.spanFromContext(parentContext).getSpanContext().isValid()) {
-      if (instrumenter().shouldStart(parentContext, method)) {
-        context = instrumenter().start(parentContext, method);
+      classAndMethod = ClassAndMethod.create(declaringClass, methodName);
+      if (instrumenter().shouldStart(parentContext, classAndMethod)) {
+        context = instrumenter().start(parentContext, classAndMethod);
         scope = context.makeCurrent();
       }
     }
   }
 
   @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
   public static void stopSpan(
-      @Advice.Origin Method method,
       @Advice.Thrown Throwable throwable,
+      @Advice.Local("otelMethod") ClassAndMethod classAndMethod,
       @Advice.Local("otelContext") Context context,
       @Advice.Local("otelScope") Scope scope,
       @Advice.Local("otelCallDepth") CallDepth callDepth) {
     if (callDepth.decrementAndGet() > 0) {
       return;
     }
-    HttpServletResponseAdviceHelper.stopSpan(instrumenter(), throwable, context, scope, method);
+    HttpServletResponseAdviceHelper.stopSpan(
+        instrumenter(), throwable, context, scope, classAndMethod);
   }
 }

instrumentation/servlet/servlet-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/v5_0/response/ResponseSingletons.java

@@ -8,20 +8,20 @@
 import io.opentelemetry.api.GlobalOpenTelemetry;
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
 import io.opentelemetry.instrumentation.api.tracer.SpanNames;
-import java.lang.reflect.Method;
+import io.opentelemetry.instrumentation.api.util.ClassAndMethod;
 
 public class ResponseSingletons {
 
-  private static final Instrumenter<Method, Void> INSTRUMENTER;
+  private static final Instrumenter<ClassAndMethod, Void> INSTRUMENTER;
 
   static {
     INSTRUMENTER =
-        Instrumenter.<Method, Void>builder(
+        Instrumenter.<ClassAndMethod, Void>builder(
                 GlobalOpenTelemetry.get(), "io.opentelemetry.servlet-5.0", SpanNames::fromMethod)
             .newInstrumenter();
   }
 
-  public static Instrumenter<Method, Void> instrumenter() {
+  public static Instrumenter<ClassAndMethod, Void> instrumenter() {
     return INSTRUMENTER;
   }
 }

instrumentation/servlet/servlet-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/common/response/HttpServletResponseAdviceHelper.java

@@ -8,15 +8,15 @@
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.Scope;
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
-import java.lang.reflect.Method;
+import io.opentelemetry.instrumentation.api.util.ClassAndMethod;
 
 public class HttpServletResponseAdviceHelper {
   public static void stopSpan(
-      Instrumenter<Method, Void> instrumenter,
+      Instrumenter<ClassAndMethod, Void> instrumenter,
       Throwable throwable,
       Context context,
       Scope scope,
-      Method request) {
+      ClassAndMethod request) {
     if (scope != null) {
       scope.close();