Support new @WithSpan annotation in spring-boot-autoconfigure (open-t…

…elemetry#6378)
helios · Oct 31, 2022 · 4d01d81 · 4d01d81
1 parent 2468dd9
commit 4d01d81
Show file tree

Hide file tree

Showing 8 changed files with 280 additions and 117 deletions.
diff --git a/instrumentation/spring/spring-boot-autoconfigure/build.gradle.kts b/instrumentation/spring/spring-boot-autoconfigure/build.gradle.kts
@@ -9,12 +9,11 @@ val versions: Map<String, String> by project
 val springBootVersion = versions["org.springframework.boot"]
 
 dependencies {
-  implementation(project(":instrumentation-annotations-support"))
-
   implementation("org.springframework.boot:spring-boot-autoconfigure:$springBootVersion")
   annotationProcessor("org.springframework.boot:spring-boot-autoconfigure-processor:$springBootVersion")
   implementation("javax.validation:validation-api:2.0.1.Final")
 
+  implementation(project(":instrumentation-annotations-support"))
   implementation(project(":instrumentation:spring:spring-web-3.1:library"))
   implementation(project(":instrumentation:spring:spring-webmvc-3.1:library"))
   implementation(project(":instrumentation:spring:spring-webflux-5.0:library"))
@@ -37,6 +36,7 @@ dependencies {
   compileOnly("io.opentelemetry:opentelemetry-exporter-jaeger")
   compileOnly("io.opentelemetry:opentelemetry-exporter-otlp")
   compileOnly("io.opentelemetry:opentelemetry-exporter-zipkin")
+  compileOnly(project(":instrumentation-annotations"))
 
   testImplementation("org.springframework.boot:spring-boot-starter-actuator:$springBootVersion")
   testImplementation("org.springframework.boot:spring-boot-starter-aop:$springBootVersion")
@@ -58,7 +58,7 @@ dependencies {
   testImplementation("io.opentelemetry:opentelemetry-exporter-jaeger")
   testImplementation("io.opentelemetry:opentelemetry-exporter-otlp")
   testImplementation("io.opentelemetry:opentelemetry-exporter-zipkin")
-  testImplementation(project(":instrumentation-annotations-support"))
+  testImplementation(project(":instrumentation-annotations"))
 }
 
 tasks.compileTestJava {

diff --git a/.../java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/JoinPointRequest.java b/.../java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/JoinPointRequest.java
@@ -5,29 +5,63 @@
 
 package io.opentelemetry.instrumentation.spring.autoconfigure.aspects;
 
-import io.opentelemetry.extension.annotations.WithSpan;
+import io.opentelemetry.api.trace.SpanKind;
+import io.opentelemetry.instrumentation.annotations.WithSpan;
+import io.opentelemetry.instrumentation.api.util.SpanNames;
 import java.lang.reflect.Method;
 import org.aspectj.lang.JoinPoint;
 import org.aspectj.lang.reflect.MethodSignature;
 
 final class JoinPointRequest {
+
   private final JoinPoint joinPoint;
   private final Method method;
-  private final WithSpan annotation;
+  private final String spanName;
+  private final SpanKind spanKind;
 
   JoinPointRequest(JoinPoint joinPoint) {
     this.joinPoint = joinPoint;
     MethodSignature methodSignature = (MethodSignature) joinPoint.getSignature();
     this.method = methodSignature.getMethod();
-    this.annotation = this.method.getDeclaredAnnotation(WithSpan.class);
+
+    // in rare cases, when interface method does not have annotations but the implementation does,
+    // and the AspectJ factory is configured to proxy interfaces, this class will receive the
+    // abstract interface method (without annotations) instead of the implementation method (with
+    // annotations); these defaults prevent NPEs in this scenario
+    String spanName = "";
+    SpanKind spanKind = SpanKind.INTERNAL;
+
+    io.opentelemetry.extension.annotations.WithSpan oldAnnotation =
+        this.method.getDeclaredAnnotation(io.opentelemetry.extension.annotations.WithSpan.class);
+    if (oldAnnotation != null) {
+      spanName = oldAnnotation.value();
+      spanKind = oldAnnotation.kind();
+    }
+
+    WithSpan annotation = this.method.getDeclaredAnnotation(WithSpan.class);
+    if (annotation != null) {
+      spanName = annotation.value();
+      spanKind = annotation.kind();
+    }
+
+    if (spanName.isEmpty()) {
+      spanName = SpanNames.fromMethod(method);
+    }
+
+    this.spanName = spanName;
+    this.spanKind = spanKind;
   }
 
-  Method method() {
-    return method;
+  String spanName() {
+    return spanName;
+  }
+
+  SpanKind spanKind() {
+    return spanKind;
   }
 
-  WithSpan annotation() {
-    return annotation;
+  Method method() {
+    return method;
   }
 
   Object[] args() {

diff --git a/...in/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspect.java b/...in/java/io/opentelemetry/instrumentation/spring/autoconfigure/aspects/WithSpanAspect.java
@@ -6,29 +6,28 @@
 package io.opentelemetry.instrumentation.spring.autoconfigure.aspects;
 
 import io.opentelemetry.api.OpenTelemetry;
-import io.opentelemetry.api.trace.SpanKind;
+import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.Scope;
-import io.opentelemetry.extension.annotations.WithSpan;
+import io.opentelemetry.instrumentation.annotations.WithSpan;
 import io.opentelemetry.instrumentation.api.annotation.support.MethodSpanAttributesExtractor;
 import io.opentelemetry.instrumentation.api.annotation.support.ParameterAttributeNamesExtractor;
 import io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationEndSupport;
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
 import io.opentelemetry.instrumentation.api.instrumenter.code.CodeAttributesExtractor;
-import io.opentelemetry.instrumentation.api.util.SpanNames;
 import org.aspectj.lang.ProceedingJoinPoint;
 import org.aspectj.lang.annotation.Around;
 import org.aspectj.lang.annotation.Aspect;
 import org.springframework.core.ParameterNameDiscoverer;
 
 /**
- * Uses Spring-AOP to wrap methods marked by {@link WithSpan} in a {@link
- * io.opentelemetry.api.trace.Span}.
+ * Uses Spring-AOP to wrap methods marked by {@link WithSpan} (or the deprecated {@link
+ * io.opentelemetry.extension.annotations.WithSpan}) in a {@link Span}.
  *
  * <p>Ensure methods annotated with {@link WithSpan} are implemented on beans managed by the Spring
  * container.
  *
- * <p>Note: This Aspect uses spring-aop to proxy beans. Therefore the {@link WithSpan} annotation
+ * <p>Note: This Aspect uses spring-aop to proxy beans. Therefore, the {@link WithSpan} annotation
  * can not be applied to constructors.
  */
 @Aspect
@@ -44,31 +43,20 @@ public WithSpanAspect(
         new WithSpanAspectParameterAttributeNamesExtractor(parameterNameDiscoverer);
 
     instrumenter =
-        Instrumenter.builder(openTelemetry, INSTRUMENTATION_NAME, WithSpanAspect::spanName)
+        Instrumenter.builder(openTelemetry, INSTRUMENTATION_NAME, JoinPointRequest::spanName)
             .addAttributesExtractor(
                 CodeAttributesExtractor.create(JointPointCodeAttributesExtractor.INSTANCE))
             .addAttributesExtractor(
                 MethodSpanAttributesExtractor.newInstance(
                     JoinPointRequest::method,
                     parameterAttributeNamesExtractor,
                     JoinPointRequest::args))
-            .buildInstrumenter(WithSpanAspect::spanKind);
+            .buildInstrumenter(JoinPointRequest::spanKind);
   }
 
-  private static String spanName(JoinPointRequest request) {
-    WithSpan annotation = request.annotation();
-    String spanName = annotation.value();
-    if (spanName.isEmpty()) {
-      return SpanNames.fromMethod(request.method());
-    }
-    return spanName;
-  }
-
-  private static SpanKind spanKind(JoinPointRequest request) {
-    return request.annotation().kind();
-  }
-
-  @Around("@annotation(io.opentelemetry.extension.annotations.WithSpan)")
+  @Around(
+      "@annotation(io.opentelemetry.extension.annotations.WithSpan) || "
+          + "@annotation(io.opentelemetry.instrumentation.annotations.WithSpan)")
   public Object traceMethod(ProceedingJoinPoint pjp) throws Throwable {
 
     JoinPointRequest request = new JoinPointRequest(pjp);

diff --git a/...entation/spring/autoconfigure/aspects/WithSpanAspectParameterAttributeNamesExtractor.java b/...entation/spring/autoconfigure/aspects/WithSpanAspectParameterAttributeNamesExtractor.java
@@ -5,7 +5,7 @@
 
 package io.opentelemetry.instrumentation.spring.autoconfigure.aspects;
 
-import io.opentelemetry.extension.annotations.SpanAttribute;
+import io.opentelemetry.instrumentation.annotations.SpanAttribute;
 import io.opentelemetry.instrumentation.api.annotation.support.ParameterAttributeNamesExtractor;
 import java.lang.reflect.Method;
 import java.lang.reflect.Parameter;
@@ -34,13 +34,23 @@ public String[] extract(Method method, Parameter[] parameters) {
 
   @Nullable
   private static String attributeName(Parameter parameter, String[] parameterNames, int index) {
+    io.opentelemetry.extension.annotations.SpanAttribute oldAnnotation =
+        parameter.getDeclaredAnnotation(io.opentelemetry.extension.annotations.SpanAttribute.class);
     SpanAttribute annotation = parameter.getDeclaredAnnotation(SpanAttribute.class);
-    if (annotation == null) {
+    if (oldAnnotation == null && annotation == null) {
       return null;
     }
-    String value = annotation.value();
-    if (!value.isEmpty()) {
-      return value;
+    if (annotation != null) {
+      String value = annotation.value();
+      if (!value.isEmpty()) {
+        return value;
+      }
+    }
+    if (oldAnnotation != null) {
+      String value = oldAnnotation.value();
+      if (!value.isEmpty()) {
+        return value;
+      }
     }
     if (parameterNames != null && index < parameterNames.length) {
       String parameterName = parameterNames[index];