Add a ClassAndMethod class to Instrumentation API (#4619)

* Add a ClassAndMethod class to Instrumentation API

* remove sentence

* Update docs/contributing/writing-instrumentation.md

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>

* address review comment

Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>

docs/contributing/writing-instrumentation-module.md

@@ -340,3 +340,18 @@ bytecode tweaks to optimize it. Because of this, retrieving a `VirtualField` ins
 limited: the `VirtualField#get()` method must receive class references as its parameters; it won't
 work with variables, method params, etc. Both the owner class and the field class must be known at
 compile time for it to work.
+
+### 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 method. 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 final class SpanNames {
     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-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/ClassAndMethod.java

@@ -1,20 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.javaagent.instrumentation.extannotations;
-
-import com.google.auto.value.AutoValue;
-
-@AutoValue
-public abstract class ClassAndMethod {
-
-  public static ClassAndMethod create(Class<?> declaringClass, String methodName) {
-    return new AutoValue_ClassAndMethod(declaringClass, methodName);
-  }
-
-  public abstract Class<?> declaringClass();
-
-  public abstract String methodName();
-}

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 static net.bytebuddy.matcher.ElementMatchers.not;
 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 @@ package io.opentelemetry.javaagent.instrumentation.extannotations;
 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 static net.bytebuddy.matcher.ElementMatchers.namedOneOf;
 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,21 +55,25 @@ public class MethodInstrumentation implements TypeInstrumentation {
 
     @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,
@@ -77,8 +81,8 @@ public class MethodInstrumentation implements TypeInstrumentation {
       scope.close();
 
       returnValue =
-          AsyncOperationEndSupport.create(instrumenter(), Void.class, method.getReturnType())
+          AsyncOperationEndSupport.create(instrumenter(), Void.class, returnType)
-              .asyncEnd(context, method, returnValue, throwable);
+              .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.Instrumenter;
 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 class WithSpanInstrumentation implements TypeInstrumentation {
 
     @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 class WithSpanInstrumentation implements TypeInstrumentation {
 
     @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,7 +159,8 @@ public class WithSpanInstrumentation implements TypeInstrumentation {
 
     @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,
@@ -162,6 +168,10 @@ public class WithSpanInstrumentation implements TypeInstrumentation {
         @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 class WithSpanInstrumentation implements TypeInstrumentation {
 
     @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 static net.bytebuddy.matcher.ElementMatchers.not;
 
 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.api.GlobalOpenTelemetry;
 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,11 +9,11 @@ import static io.opentelemetry.javaagent.instrumentation.servlet.v5_0.response.R
 
 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")
@@ -21,7 +21,10 @@ 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,8 +36,9 @@ public class ResponseSendAdvice {
     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)) {
+      classAndMethod = ClassAndMethod.create(declaringClass, methodName);
-        context = instrumenter().start(parentContext, method);
+      if (instrumenter().shouldStart(parentContext, classAndMethod)) {
+        context = instrumenter().start(parentContext, classAndMethod);
         scope = context.makeCurrent();
       }
     }
@@ -42,14 +46,15 @@ public class ResponseSendAdvice {
 
   @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 @@ package io.opentelemetry.javaagent.instrumentation.servlet.v5_0.response;
 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 @@ package io.opentelemetry.javaagent.instrumentation.servlet.common.response;
 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();
 

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

@@ -9,10 +9,10 @@ import static io.opentelemetry.javaagent.instrumentation.servlet.javax.response.
 
 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 java.lang.reflect.Method;
 import javax.servlet.http.HttpServletResponse;
 import net.bytebuddy.asm.Advice;
 
@@ -21,7 +21,9 @@ public class ResponseSendAdvice {
 
   @Advice.OnMethodEnter(suppress = Throwable.class)
   public static void start(
-      @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,
       @Advice.Local("otelCallDepth") CallDepth callDepth) {
@@ -32,8 +34,9 @@ public class ResponseSendAdvice {
     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)) {
+      classAndMethod = ClassAndMethod.create(declaringClass, methodName);
-        context = instrumenter().start(parentContext, method);
+      if (instrumenter().shouldStart(parentContext, classAndMethod)) {
+        context = instrumenter().start(parentContext, classAndMethod);
         scope = context.makeCurrent();
       }
     }
@@ -41,14 +44,15 @@ public class ResponseSendAdvice {
 
   @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-javax-common/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/servlet/javax/response/ResponseSingletons.java

@@ -8,22 +8,22 @@ package io.opentelemetry.javaagent.instrumentation.servlet.javax.response;
 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-javax-common",
                 SpanNames::fromMethod)
             .newInstrumenter();
   }
 
-  public static Instrumenter<Method, Void> instrumenter() {
+  public static Instrumenter<ClassAndMethod, Void> instrumenter() {
     return INSTRUMENTER;
   }
 }