Support RESTeasy 4.0 version in unit tests

* Split jaxrs-2.0 into jaxrs-2.0-common and jaxrs-2.0-testing (similar to e.g. play) * Support newest RESTEasy version in unit tests
open-telemetry · Sep 9, 2020 · 0603649 · 0603649
1 parent c1556fa
commit 0603649
Show file tree

Hide file tree

Showing 32 changed files with 507 additions and 264 deletions.
diff --git a/instrumentation/dropwizard-testing/dropwizard-testing.gradle b/instrumentation/dropwizard-testing/dropwizard-testing.gradle
@@ -4,14 +4,11 @@ ext {
 apply from: "$rootDir/gradle/instrumentation.gradle"
 
 dependencies {
-  testImplementation project(':instrumentation:jaxrs:jaxrs-2.0')
+  testImplementation project(':instrumentation:jaxrs:jaxrs-2.0:jaxrs-2.0-jersey-2.0')
   testImplementation project(':instrumentation:servlet:servlet-3.0')
 
   // First version with DropwizardTestSupport:
   testImplementation group: 'io.dropwizard', name: 'dropwizard-testing', version: '0.8.0'
   testImplementation group: 'javax.xml.bind', name: 'jaxb-api', version: '2.2.3'
   testImplementation group: 'com.fasterxml.jackson.module', name: 'jackson-module-afterburner', version: '2.9.10'
-
-  // Anything 1.0+ fails with a java.lang.NoClassDefFoundError: org/eclipse/jetty/server/RequestLog
-//  latestDepTestLibrary group: 'io.dropwizard', name: 'dropwizard-testing', version: '1.+'
 }
diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/jaxrs-2.0-common.gradle b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-common/jaxrs-2.0-common.gradle
@@ -0,0 +1,18 @@
+apply from: "$rootDir/gradle/instrumentation.gradle"
+
+muzzle {
+  fail {
+    group = "javax.ws.rs"
+    module = "jsr311-api"
+    versions = "[,]"
+  }
+  pass {
+    group = "javax.ws.rs"
+    module = "javax.ws.rs-api"
+    versions = "[,]"
+  }
+}
+
+dependencies {
+  compileOnly group: 'javax.ws.rs', name: 'javax.ws.rs-api', version: '2.0'
+}
diff --git a/...bstractRequestContextInstrumentation.java → ...bstractRequestContextInstrumentation.java b/...bstractRequestContextInstrumentation.java → ...bstractRequestContextInstrumentation.java
@@ -19,15 +19,14 @@
 import static io.opentelemetry.instrumentation.auto.jaxrs.v2_0.JaxRsAnnotationsTracer.TRACER;
 import static io.opentelemetry.javaagent.tooling.ClassLoaderMatcher.hasClassesNamed;
 import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.implementsInterface;
-import static io.opentelemetry.trace.TracingContextUtils.currentContextWith;
 import static java.util.Collections.singletonMap;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.named;
 import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
 import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
 
+import io.opentelemetry.context.Scope;
 import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
-import io.opentelemetry.instrumentation.auto.api.SpanWithScope;
 import io.opentelemetry.javaagent.tooling.Instrumenter;
 import io.opentelemetry.trace.Span;
 import java.lang.reflect.Method;
@@ -74,39 +73,35 @@ public Map<? extends ElementMatcher<? super MethodDescription>, String> transfor
   }
 
   public static class RequestFilterHelper {
-    public static SpanWithScope createOrUpdateAbortSpan(
+    public static Span createOrUpdateAbortSpan(
         ContainerRequestContext context, Class<?> resourceClass, Method method) {
 
       if (method != null && resourceClass != null) {
         context.setProperty(JaxRsAnnotationsTracer.ABORT_HANDLED, true);
-        Span parent = BaseTracer.getCurrentServerSpan();
-        Span span = TRACER.getCurrentSpan();
+        Span serverSpan = BaseTracer.getCurrentServerSpan();
+        Span currentSpan = TRACER.getCurrentSpan();
 
-        if (span == null) {
-          span = TRACER.startSpan(resourceClass, method);
-          return new SpanWithScope(span, currentContextWith(span));
+        if (currentSpan == null || currentSpan == serverSpan) {
+          return TRACER.startSpan(resourceClass, method);
         } else {
-          TRACER.updateSpanNames(span, parent, resourceClass, method);
-          return null;
+          TRACER.updateSpanNames(currentSpan, serverSpan, resourceClass, method);
         }
-      } else {
-        return null;
       }
+      return null;
     }
 
-    public static void closeSpanAndScope(SpanWithScope spanWithScope, Throwable throwable) {
-      if (spanWithScope == null) {
+    public static void closeSpanAndScope(Span span, Scope scope, Throwable throwable) {
+      if (span == null || scope == null) {
         return;
       }
 
-      Span span = spanWithScope.getSpan();
       if (throwable != null) {
         TRACER.endExceptionally(span, throwable);
       } else {
         TRACER.end(span);
       }
 
-      spanWithScope.closeScope();
+      scope.close();
     }
   }
 }
diff --git a/...ontainerRequestFilterInstrumentation.java → ...ontainerRequestFilterInstrumentation.java b/...ontainerRequestFilterInstrumentation.java → ...ontainerRequestFilterInstrumentation.java
diff --git a/...DefaultRequestContextInstrumentation.java → ...DefaultRequestContextInstrumentation.java b/...DefaultRequestContextInstrumentation.java → ...DefaultRequestContextInstrumentation.java
@@ -17,15 +17,15 @@
 package io.opentelemetry.instrumentation.auto.jaxrs.v2_0;
 
 import static io.opentelemetry.instrumentation.auto.jaxrs.v2_0.JaxRsAnnotationsTracer.TRACER;
-import static io.opentelemetry.trace.TracingContextUtils.currentContextWith;
 
 import com.google.auto.service.AutoService;
-import io.opentelemetry.instrumentation.auto.api.SpanWithScope;
+import io.opentelemetry.context.Scope;
 import io.opentelemetry.javaagent.tooling.Instrumenter;
 import io.opentelemetry.trace.Span;
 import java.lang.reflect.Method;
 import javax.ws.rs.container.ContainerRequestContext;
 import net.bytebuddy.asm.Advice;
+import net.bytebuddy.asm.Advice.Local;
 
 /**
  * Default context instrumentation.
@@ -40,10 +40,13 @@
 public class DefaultRequestContextInstrumentation extends AbstractRequestContextInstrumentation {
   public static class ContainerRequestContextAdvice {
     @Advice.OnMethodEnter(suppress = Throwable.class)
-    public static SpanWithScope createGenericSpan(@Advice.This ContainerRequestContext context) {
-
+    public static void createGenericSpan(
+        @Advice.This ContainerRequestContext context,
+        @Local("otelSpan") Span span,
+        @Local("otelScope") Scope scope) {
       if (context.getProperty(JaxRsAnnotationsTracer.ABORT_HANDLED) == null) {
-        Class filterClass = (Class) context.getProperty(JaxRsAnnotationsTracer.ABORT_FILTER_CLASS);
+        Class<?> filterClass =
+            (Class<?>) context.getProperty(JaxRsAnnotationsTracer.ABORT_FILTER_CLASS);
         Method method = null;
         try {
           method = filterClass.getMethod("filter", ContainerRequestContext.class);
@@ -52,29 +55,19 @@ public static SpanWithScope createGenericSpan(@Advice.This ContainerRequestConte
           // can only be aborted inside the filter method
         }
 
-        Span span = TRACER.startSpan(filterClass, method);
-
-        return new SpanWithScope(span, currentContextWith(span));
+        span = RequestFilterHelper.createOrUpdateAbortSpan(context, filterClass, method);
+        if (span != null) {
+          scope = TRACER.startScope(span);
+        }
       }
-
-      return null;
     }
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void stopSpan(
-        @Advice.Enter SpanWithScope spanWithScope, @Advice.Thrown Throwable throwable) {
-      if (spanWithScope == null) {
-        return;
-      }
-
-      Span span = spanWithScope.getSpan();
-      if (throwable != null) {
-        TRACER.endExceptionally(span, throwable);
-      } else {
-        TRACER.end(span);
-      }
-
-      spanWithScope.closeScope();
+        @Local("otelSpan") Span span,
+        @Local("otelScope") Scope scope,
+        @Advice.Thrown Throwable throwable) {
+      RequestFilterHelper.closeSpanAndScope(span, scope, throwable);
     }
   }
 }
diff --git a/...v2_0/JaxRsAnnotationsInstrumentation.java → ...v2_0/JaxRsAnnotationsInstrumentation.java b/...v2_0/JaxRsAnnotationsInstrumentation.java → ...v2_0/JaxRsAnnotationsInstrumentation.java
@@ -21,18 +21,17 @@
 import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.hasSuperMethod;
 import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.safeHasSuperType;
 import static io.opentelemetry.javaagent.tooling.matcher.NameMatchers.namedOneOf;
-import static io.opentelemetry.trace.TracingContextUtils.currentContextWith;
 import static java.util.Collections.singletonMap;
 import static net.bytebuddy.matcher.ElementMatchers.declaresMethod;
 import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.named;
 
 import com.google.auto.service.AutoService;
+import io.opentelemetry.context.Scope;
 import io.opentelemetry.instrumentation.auto.api.CallDepthThreadLocalMap;
 import io.opentelemetry.instrumentation.auto.api.ContextStore;
 import io.opentelemetry.instrumentation.auto.api.InstrumentationContext;
-import io.opentelemetry.instrumentation.auto.api.SpanWithScope;
 import io.opentelemetry.javaagent.tooling.Instrumenter;
 import io.opentelemetry.trace.Span;
 import java.lang.reflect.Method;
@@ -101,11 +100,13 @@ public Map<? extends ElementMatcher<? super MethodDescription>, String> transfor
   public static class JaxRsAnnotationsAdvice {
 
     @Advice.OnMethodEnter(suppress = Throwable.class)
-    public static SpanWithScope nameSpan(
+    public static void nameSpan(
         @Advice.This Object target,
         @Advice.Origin Method method,
         @Advice.AllArguments Object[] args,
-        @Advice.Local("asyncResponse") AsyncResponse asyncResponse) {
+        @Advice.Local("otelSpan") Span span,
+        @Advice.Local("otelScope") Scope scope,
+        @Advice.Local("otelAsyncResponse") AsyncResponse asyncResponse) {
       ContextStore<AsyncResponse, Span> contextStore = null;
       for (Object arg : args) {
         if (arg instanceof AsyncResponse) {
@@ -118,39 +119,39 @@ public static SpanWithScope nameSpan(
              * could work around this by using a list instead, but we likely don't want the extra
              * span anyway.
              */
-            return null;
+            return;
           }
           break;
         }
       }
 
       if (CallDepthThreadLocalMap.incrementCallDepth(Path.class) > 0) {
-        return null;
+        return;
       }
 
-      Span span = TRACER.startSpan(target.getClass(), method);
+      span = TRACER.startSpan(target.getClass(), method);
 
       if (contextStore != null && asyncResponse != null) {
         contextStore.put(asyncResponse, span);
       }
 
-      return new SpanWithScope(span, currentContextWith(span));
+      scope = TRACER.startScope(span);
     }
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void stopSpan(
-        @Advice.Enter SpanWithScope spanWithScope,
         @Advice.Thrown Throwable throwable,
-        @Advice.Local("asyncResponse") AsyncResponse asyncResponse) {
-      if (spanWithScope == null) {
+        @Advice.Local("otelSpan") Span span,
+        @Advice.Local("otelScope") Scope scope,
+        @Advice.Local("otelAsyncResponse") AsyncResponse asyncResponse) {
+      if (span == null || scope == null) {
         return;
       }
       CallDepthThreadLocalMap.reset(Path.class);
 
-      Span span = spanWithScope.getSpan();
       if (throwable != null) {
         TRACER.endExceptionally(span, throwable);
-        spanWithScope.closeScope();
+        scope.close();
         return;
       }
 
@@ -161,7 +162,7 @@ public static void stopSpan(
       if (asyncResponse == null || !asyncResponse.isSuspended()) {
         TRACER.end(span);
       }
-      spanWithScope.closeScope();
+      scope.close();
       // else span finished by AsyncResponseAdvice
     }
   }

diff --git a/...to/jaxrs/v2_0/JaxRsAnnotationsTracer.java → ...to/jaxrs/v2_0/JaxRsAnnotationsTracer.java b/...to/jaxrs/v2_0/JaxRsAnnotationsTracer.java → ...to/jaxrs/v2_0/JaxRsAnnotationsTracer.java
@@ -18,6 +18,7 @@
 
 import static io.opentelemetry.instrumentation.auto.api.WeakMap.Provider.newWeakMap;
 
+import io.opentelemetry.context.Scope;
 import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
 import io.opentelemetry.instrumentation.auto.api.WeakMap;
 import io.opentelemetry.javaagent.tooling.ClassHierarchyIterable;
@@ -59,6 +60,10 @@ public void updateSpanNames(Span span, Span serverSpan, Class<?> target, Method
     }
   }
 
+  public Scope startScope(Span span) {
+    return tracer.withSpan(span);
+  }
+
   private void updateSpanName(Span span, String spanName) {
     if (!spanName.isEmpty()) {
       span.updateName(spanName);

diff --git a/..._0/JaxRsAsyncResponseInstrumentation.java → ..._0/JaxRsAsyncResponseInstrumentation.java b/..._0/JaxRsAsyncResponseInstrumentation.java → ..._0/JaxRsAsyncResponseInstrumentation.java
diff --git a/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/jaxrs-2.0-jersey-2.0.gradle b/instrumentation/jaxrs/jaxrs-2.0/jaxrs-2.0-jersey-2.0/jaxrs-2.0-jersey-2.0.gradle
@@ -2,7 +2,7 @@ apply from: "$rootDir/gradle/instrumentation.gradle"
 
 muzzle {
   // Cant assert fails because muzzle assumes all instrumentations will fail
-  // Instrumentations in jaxrs-2.0 will pass
+  // Instrumentations in jaxrs-2.0-common will pass
   pass {
     group = "org.glassfish.jersey.core"
     module = "jersey-server"
@@ -14,5 +14,13 @@ dependencies {
   compileOnly group: 'javax.ws.rs', name: 'javax.ws.rs-api', version: '2.0'
   compileOnly group: 'org.glassfish.jersey.core', name: 'jersey-server', version: '2.0'
 
-  implementation project(':instrumentation:jaxrs:jaxrs-2.0')
+  implementation project(':instrumentation:jaxrs:jaxrs-2.0:jaxrs-2.0-common')
+
+  testImplementation project(':instrumentation:servlet:servlet-3.0')
+  testImplementation project(':instrumentation:jaxrs:jaxrs-2.0:jaxrs-2.0-testing')
+
+  // First version with DropwizardTestSupport:
+  testLibrary group: 'io.dropwizard', name: 'dropwizard-testing', version: '0.8.0'
+  testImplementation group: 'javax.xml.bind', name: 'jaxb-api', version: '2.2.3'
+  testImplementation group: 'com.fasterxml.jackson.module', name: 'jackson-module-afterburner', version: '2.9.10'
 }
diff --git a/...io/opentelemetry/instrumentation/auto/jaxrs/v2_0/JerseyRequestContextInstrumentation.java b/...io/opentelemetry/instrumentation/auto/jaxrs/v2_0/JerseyRequestContextInstrumentation.java
@@ -16,14 +16,18 @@
 
 package io.opentelemetry.instrumentation.auto.jaxrs.v2_0;
 
+import static io.opentelemetry.instrumentation.auto.jaxrs.v2_0.JaxRsAnnotationsTracer.TRACER;
+
 import com.google.auto.service.AutoService;
-import io.opentelemetry.instrumentation.auto.api.SpanWithScope;
+import io.opentelemetry.context.Scope;
 import io.opentelemetry.javaagent.tooling.Instrumenter;
+import io.opentelemetry.trace.Span;
 import java.lang.reflect.Method;
 import javax.ws.rs.container.ContainerRequestContext;
 import javax.ws.rs.container.ResourceInfo;
 import javax.ws.rs.core.UriInfo;
 import net.bytebuddy.asm.Advice;
+import net.bytebuddy.asm.Advice.Local;
 
 /**
  * Jersey specific context instrumentation.
@@ -38,26 +42,32 @@
 public class JerseyRequestContextInstrumentation extends AbstractRequestContextInstrumentation {
   public static class ContainerRequestContextAdvice {
     @Advice.OnMethodEnter(suppress = Throwable.class)
-    public static SpanWithScope decorateAbortSpan(@Advice.This ContainerRequestContext context) {
+    public static void decorateAbortSpan(
+        @Advice.This ContainerRequestContext context,
+        @Local("otelSpan") Span span,
+        @Local("otelScope") Scope scope) {
       UriInfo uriInfo = context.getUriInfo();
 
       if (context.getProperty(JaxRsAnnotationsTracer.ABORT_HANDLED) == null
           && uriInfo instanceof ResourceInfo) {
 
         ResourceInfo resourceInfo = (ResourceInfo) uriInfo;
         Method method = resourceInfo.getResourceMethod();
-        Class resourceClass = resourceInfo.getResourceClass();
+        Class<?> resourceClass = resourceInfo.getResourceClass();
 
-        return RequestFilterHelper.createOrUpdateAbortSpan(context, resourceClass, method);
+        span = RequestFilterHelper.createOrUpdateAbortSpan(context, resourceClass, method);
+        if (span != null) {
+          scope = TRACER.startScope(span);
+        }
       }
-
-      return null;
     }
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void stopSpan(
-        @Advice.Enter SpanWithScope scope, @Advice.Thrown Throwable throwable) {
-      RequestFilterHelper.closeSpanAndScope(scope, throwable);
+        @Local("otelSpan") Span span,
+        @Local("otelScope") Scope scope,
+        @Advice.Thrown Throwable throwable) {
+      RequestFilterHelper.closeSpanAndScope(span, scope, throwable);
     }
   }
 }
diff --git a/...jaxrs-2.0/jaxrs-2.0-jersey-2.0/src/test/groovy/JerseyAnnotationInstrumentationTest.groovy b/...jaxrs-2.0/jaxrs-2.0-jersey-2.0/src/test/groovy/JerseyAnnotationInstrumentationTest.groovy
@@ -0,0 +1,18 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+class JerseyAnnotationInstrumentationTest extends JaxRsAnnotationsInstrumentationTest {
+}