Merge pull request #737 from DataDog/tyler/trace-jax-decorator

Tracing instrumentation migrate to Decorator

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/WeakMap.java

@@ -16,6 +16,8 @@ public interface WeakMap<K, V> {
 
   void put(K key, V value);
 
+  void putIfAbsent(K key, V value);
+
   @Slf4j
   class Provider {
     private static final AtomicReference<Supplier> provider =
@@ -81,6 +83,19 @@ public interface WeakMap<K, V> {
       map.put(key, value);
     }
 
+    @Override
+    public void putIfAbsent(final K key, final V value) {
+      // We can't use putIfAbsent since it was added in 1.8.
+      // As a result, we must use double check locking.
+      if (!map.containsKey(key)) {
+        synchronized (this) {
+          if (!map.containsKey(key)) {
+            map.put(key, value);
+          }
+        }
+      }
+    }
+
     @Override
     public String toString() {
       return map.toString();

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/decorator/BaseDecorator.java

@@ -4,8 +4,10 @@ import static io.opentracing.log.Fields.ERROR_OBJECT;
 
 import datadog.trace.api.Config;
 import datadog.trace.api.DDTags;
+import io.opentracing.Scope;
 import io.opentracing.Span;
 import io.opentracing.tag.Tags;
+import java.lang.reflect.Method;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.TreeSet;
@@ -16,11 +18,13 @@ public abstract class BaseDecorator {
   protected final float traceAnalyticsSampleRate;
 
   protected BaseDecorator() {
+    final String[] instrumentationNames = instrumentationNames();
     traceAnalyticsEnabled =
-        Config.traceAnalyticsIntegrationEnabled(
+        instrumentationNames.length > 0
-            new TreeSet<>(Arrays.asList(instrumentationNames())), traceAnalyticsDefault());
+            && Config.traceAnalyticsIntegrationEnabled(
+                new TreeSet<>(Arrays.asList(instrumentationNames)), traceAnalyticsDefault());
     float rate = 1.0f;
-    for (final String name : instrumentationNames()) {
+    for (final String name : instrumentationNames) {
       rate =
           Config.getFloatSettingFromEnvironment(
               "integration." + name + ".analytics.sample-rate", rate);
@@ -38,9 +42,17 @@ public abstract class BaseDecorator {
     return false;
   }
 
+  public Scope afterStart(final Scope scope) {
+    assert scope != null;
+    afterStart(scope.span());
+    return scope;
+  }
+
   public Span afterStart(final Span span) {
     assert span != null;
-    span.setTag(DDTags.SPAN_TYPE, spanType());
+    if (spanType() != null) {
+      span.setTag(DDTags.SPAN_TYPE, spanType());
+    }
     Tags.COMPONENT.set(span, component());
     if (traceAnalyticsEnabled) {
       span.setTag(DDTags.ANALYTICS_SAMPLE_RATE, traceAnalyticsSampleRate);
@@ -48,11 +60,23 @@ public abstract class BaseDecorator {
     return span;
   }
 
+  public Scope beforeFinish(final Scope scope) {
+    assert scope != null;
+    beforeFinish(scope.span());
+    return scope;
+  }
+
   public Span beforeFinish(final Span span) {
     assert span != null;
     return span;
   }
 
+  public Scope onError(final Scope scope, final Throwable throwable) {
+    assert scope != null;
+    onError(scope.span(), throwable);
+    return scope;
+  }
+
   public Span onError(final Span span, final Throwable throwable) {
     assert span != null;
     if (throwable != null) {
@@ -61,4 +85,28 @@ public abstract class BaseDecorator {
     }
     return span;
   }
+
+  /**
+   * This method is used to generate an acceptable span (operation) name based on a given method
+   * reference. Anonymous classes are named based on their parent.
+   *
+   * @param method
+   * @return
+   */
+  public String spanNameForMethod(final Method method) {
+    final Class<?> declaringClass = method.getDeclaringClass();
+    String className;
+    if (declaringClass.isAnonymousClass()) {
+      className = declaringClass.getName();
+      if (declaringClass.getPackage() != null) {
+        final String pkgName = declaringClass.getPackage().getName();
+        if (!pkgName.isEmpty()) {
+          className = declaringClass.getName().replace(pkgName, "").substring(1);
+        }
+      }
+    } else {
+      className = declaringClass.getSimpleName();
+    }
+    return className + "." + method.getName();
+  }
 }

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/HelperInjector.java

@@ -1,14 +1,16 @@
 package datadog.trace.agent.tooling;
 
 import static datadog.trace.agent.tooling.ClassLoaderMatcher.BOOTSTRAP_CLASSLOADER;
+import static datadog.trace.bootstrap.WeakMap.Provider.newWeakMap;
 
+import datadog.trace.bootstrap.WeakMap;
 import java.io.File;
 import java.io.IOException;
+import java.security.SecureClassLoader;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.Map;
@@ -24,9 +26,13 @@ import net.bytebuddy.utility.JavaModule;
 /** Injects instrumentation helper classes into the user's classloader. */
 @Slf4j
 public class HelperInjector implements Transformer {
+  // Need this because we can't put null into the injectedClassLoaders map.
+  private static final ClassLoader BOOTSTRAP_CLASSLOADER_PLACEHOLDER =
+      new SecureClassLoader(null) {};
+
   private final Set<String> helperClassNames;
   private Map<TypeDescription, byte[]> helperMap = null;
-  private final Set<ClassLoader> injectedClassLoaders = new HashSet<>();
+  private final WeakMap<ClassLoader, Boolean> injectedClassLoaders = newWeakMap();
 
   /**
    * Construct HelperInjector.
@@ -80,15 +86,18 @@ public class HelperInjector implements Transformer {
   public DynamicType.Builder<?> transform(
       final DynamicType.Builder<?> builder,
       final TypeDescription typeDescription,
-      final ClassLoader classLoader,
+      ClassLoader classLoader,
       final JavaModule module) {
     if (helperClassNames.size() > 0) {
       synchronized (this) {
-        if (!injectedClassLoaders.contains(classLoader)) {
+        if (classLoader == BOOTSTRAP_CLASSLOADER) {
+          classLoader = BOOTSTRAP_CLASSLOADER_PLACEHOLDER;
+        }
+        if (!injectedClassLoaders.containsKey(classLoader)) {
           try {
             final Map<TypeDescription, byte[]> helperMap = getHelperMap();
             log.debug("Injecting classes onto classloader {} -> {}", classLoader, helperClassNames);
-            if (classLoader == BOOTSTRAP_CLASSLOADER) {
+            if (classLoader == BOOTSTRAP_CLASSLOADER_PLACEHOLDER) {
               final Map<TypeDescription, Class<?>> injected =
                   ClassInjector.UsingInstrumentation.of(
                           new File(System.getProperty("java.io.tmpdir")),
@@ -108,13 +117,13 @@ public class HelperInjector implements Transformer {
                     + ". Failed to inject helper classes into instance "
                     + classLoader
                     + " of type "
-                    + (classLoader == BOOTSTRAP_CLASSLOADER
+                    + (classLoader == BOOTSTRAP_CLASSLOADER_PLACEHOLDER
                         ? "<bootstrap>"
                         : classLoader.getClass().getName()),
                 e);
             throw new RuntimeException(e);
           }
-          injectedClassLoaders.add(classLoader);
+          injectedClassLoaders.put(classLoader, true);
         }
       }
     }

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/WeakMapSuppliers.java

@@ -170,6 +170,11 @@ class WeakMapSuppliers {
       public void put(final K key, final V value) {
         map.put(key, value);
       }
+
+      @Override
+      public void putIfAbsent(final K key, final V value) {
+        map.putIfAbsent(key, value);
+      }
     }
 
     static class Inline implements WeakMap.Supplier {

dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/BaseDecoratorTest.groovy

@@ -1,6 +1,7 @@
 package datadog.trace.agent.decorator
 
 import datadog.trace.api.DDTags
+import io.opentracing.Scope
 import io.opentracing.Span
 import io.opentracing.tag.Tags
 import spock.lang.Shared
@@ -52,36 +53,81 @@ class BaseDecoratorTest extends Specification {
 
   def "test assert null span"() {
     when:
-    decorator.afterStart(null)
+    decorator.afterStart((Span) null)
 
     then:
     thrown(AssertionError)
 
     when:
-    decorator.onError(null, null)
+    decorator.onError((Span) null, null)
 
     then:
     thrown(AssertionError)
 
     when:
-    decorator.beforeFinish(null)
+    decorator.beforeFinish((Span) null)
 
     then:
     thrown(AssertionError)
   }
 
+  def "test assert null scope"() {
+    when:
+    decorator.afterStart((Scope) null)
+
+    then:
+    thrown(AssertionError)
+
+    when:
+    decorator.onError((Scope) null, null)
+
+    then:
+    thrown(AssertionError)
+
+    when:
+    decorator.beforeFinish((Scope) null)
+
+    then:
+    thrown(AssertionError)
+  }
+
+  def "test assert non-null scope"() {
+    setup:
+    def span = Mock(Span)
+    def scope = Mock(Scope)
+
+    when:
+    decorator.afterStart(scope)
+
+    then:
+    1 * scope.span() >> span
+
+    when:
+    decorator.onError(scope, null)
+
+    then:
+    1 * scope.span() >> span
+
+    when:
+    decorator.beforeFinish(scope)
+
+    then:
+    1 * scope.span() >> span
+  }
+
   def "test analytics rate default disabled"() {
     when:
-    BaseDecorator dec = newDecorator(defaultEnabled)
+    BaseDecorator dec = newDecorator(defaultEnabled, hasConfigNames)
 
     then:
     dec.traceAnalyticsEnabled == defaultEnabled
-    dec.traceAnalyticsSampleRate == sampleRate
+    dec.traceAnalyticsSampleRate == sampleRate.floatValue()
 
     where:
-    defaultEnabled | sampleRate
+    defaultEnabled | hasConfigNames | sampleRate
-    true           | 1.0
+    true           | false          | 1.0
-    false          | 1.0
+    false          | false          | 1.0
+    false          | true           | 1.0
   }
 
   def "test analytics rate enabled"() {
@@ -108,16 +154,32 @@ class BaseDecoratorTest extends Specification {
     true    | "test2"   | ""         | true            | 1.0
   }
 
+  def "test spanNameForMethod"() {
+    when:
+    def result = decorator.spanNameForMethod(method)
+
+    then:
+    result == "${name}.run"
+
+    where:
+    target                         | name
+    SomeInnerClass                 | "SomeInnerClass"
+    SomeNestedClass                | "SomeNestedClass"
+    SampleJavaClass.anonymousClass | "SampleJavaClass\$1"
+
+    method = target.getDeclaredMethod("run")
+  }
+
   def newDecorator() {
     return newDecorator(false)
   }
 
-  def newDecorator(final Boolean analyticsEnabledDefault) {
+  def newDecorator(boolean analyticsEnabledDefault, boolean emptyInstrumentationNames = false) {
-    return analyticsEnabledDefault ?
+    return emptyInstrumentationNames ?
       new BaseDecorator() {
         @Override
         protected String[] instrumentationNames() {
-          return ["test1", "test2"]
+          return []
         }
 
         @Override
@@ -134,21 +196,50 @@ class BaseDecoratorTest extends Specification {
           return true
         }
       } :
-      new BaseDecorator() {
+      analyticsEnabledDefault ?
-        @Override
+        new BaseDecorator() {
-        protected String[] instrumentationNames() {
+          @Override
-          return ["test1", "test2"]
+          protected String[] instrumentationNames() {
-        }
+            return ["test1", "test2"]
+          }
 
-        @Override
+          @Override
-        protected String spanType() {
+          protected String spanType() {
-          return "test-type"
+            return "test-type"
-        }
+          }
 
-        @Override
+          @Override
-        protected String component() {
+          protected String component() {
-          return "test-component"
+            return "test-component"
+          }
+
+          protected boolean traceAnalyticsDefault() {
+            return true
+          }
+        } :
+        new BaseDecorator() {
+          @Override
+          protected String[] instrumentationNames() {
+            return ["test1", "test2"]
+          }
+
+          @Override
+          protected String spanType() {
+            return "test-type"
+          }
+
+          @Override
+          protected String component() {
+            return "test-component"
+          }
         }
-      }
+  }
+
+  class SomeInnerClass implements Runnable {
+    void run() {}
+  }
+
+  static class SomeNestedClass implements Runnable {
+    void run() {}
   }
 }

dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/DatabaseClientDecoratorTest.groovy

@@ -75,7 +75,7 @@ class DatabaseClientDecoratorTest extends ClientDecoratorTest {
     def decorator = newDecorator()
 
     when:
-    decorator.afterStart(null)
+    decorator.afterStart((Span) null)
 
     then:
     thrown(AssertionError)

dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/decorator/SampleJavaClass.java

@@ -0,0 +1,14 @@
+package datadog.trace.agent.decorator;
+
+/**
+ * Used by {@link BaseDecoratorTest}. Groovy with Java 10+ doesn't seem to treat it properly as an
+ * anonymous class, so use a Java class instead.
+ */
+public class SampleJavaClass {
+  public static Class anonymousClass =
+      new Runnable() {
+
+        @Override
+        public void run() {}
+      }.getClass();
+}

dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/test/HelperInjectionTest.groovy

@@ -1,38 +1,54 @@
 package datadog.trace.agent.test
 
+
 import datadog.trace.agent.tooling.AgentInstaller
 import datadog.trace.agent.tooling.HelperInjector
 import datadog.trace.agent.tooling.Utils
 import net.bytebuddy.agent.ByteBuddyAgent
+import net.bytebuddy.description.type.TypeDescription
+import net.bytebuddy.dynamic.ClassFileLocator
+import net.bytebuddy.dynamic.loading.ClassInjector
 import spock.lang.Specification
+import spock.lang.Timeout
 
-import java.lang.reflect.Method
+import java.lang.ref.WeakReference
+import java.util.concurrent.atomic.AtomicReference
 
+import static datadog.trace.agent.test.utils.ClasspathUtils.isClassLoaded
 import static datadog.trace.agent.tooling.ClassLoaderMatcher.BOOTSTRAP_CLASSLOADER
+import static datadog.trace.util.gc.GCUtils.awaitGC
 
 class HelperInjectionTest extends Specification {
 
+  @Timeout(10)
   def "helpers injected to non-delegating classloader"() {
     setup:
     String helperClassName = HelperInjectionTest.getPackage().getName() + '.HelperClass'
     HelperInjector injector = new HelperInjector(helperClassName)
-    URLClassLoader emptyLoader = new URLClassLoader(new URL[0], (ClassLoader) null)
+    AtomicReference<URLClassLoader> emptyLoader = new AtomicReference<>(new URLClassLoader(new URL[0], (ClassLoader) null))
 
     when:
-    emptyLoader.loadClass(helperClassName)
+    emptyLoader.get().loadClass(helperClassName)
     then:
     thrown ClassNotFoundException
 
     when:
-    injector.transform(null, null, emptyLoader, null)
+    injector.transform(null, null, emptyLoader.get(), null)
-    emptyLoader.loadClass(helperClassName)
+    emptyLoader.get().loadClass(helperClassName)
     then:
-    isClassLoaded(helperClassName, emptyLoader)
+    isClassLoaded(helperClassName, emptyLoader.get())
     // injecting into emptyLoader should not load on agent's classloader
     !isClassLoaded(helperClassName, Utils.getAgentClassLoader())
 
-    cleanup:
+    when: "references to emptyLoader are gone"
-    emptyLoader?.close()
+    emptyLoader.get().close() // cleanup
+    def ref = new WeakReference(emptyLoader.get())
+    emptyLoader.set(null)
+
+    awaitGC(ref)
+
+    then: "HelperInjector doesn't prevent it from being collected"
+    null == ref.get()
   }
 
   def "helpers injected on bootstrap classloader"() {
@@ -55,16 +71,38 @@ class HelperInjectionTest extends Specification {
     helperClass.getClassLoader() == BOOTSTRAP_CLASSLOADER
   }
 
-  private static boolean isClassLoaded(String className, ClassLoader classLoader) {
+  def "check hard references on class injection"() {
-    final Method findLoadedClassMethod = ClassLoader.getDeclaredMethod("findLoadedClass", String)
+    setup:
-    try {
+    String helperClassName = HelperInjectionTest.getPackage().getName() + '.HelperClass'
-      findLoadedClassMethod.setAccessible(true)
+
-      Class<?> loadedClass = (Class<?>) findLoadedClassMethod.invoke(classLoader, className)
+    // Copied from HelperInjector:
-      return null != loadedClass && loadedClass.getClassLoader() == classLoader
+    final ClassFileLocator locator =
-    } catch (Exception e) {
+      ClassFileLocator.ForClassLoader.of(Utils.getAgentClassLoader())
-      throw new IllegalStateException(e)
+    final byte[] classBytes = locator.locate(helperClassName).resolve()
-    } finally {
+    final TypeDescription typeDesc =
-      findLoadedClassMethod.setAccessible(false)
+      new TypeDescription.Latent(
-    }
+        helperClassName, 0, null, Collections.<TypeDescription.Generic> emptyList())
+
+    AtomicReference<URLClassLoader> emptyLoader = new AtomicReference<>(new URLClassLoader(new URL[0], (ClassLoader) null))
+    AtomicReference<ClassInjector> injector = new AtomicReference<>(new ClassInjector.UsingReflection(emptyLoader.get()))
+    injector.get().inject([(typeDesc): classBytes])
+
+    when:
+    def injectorRef = new WeakReference(injector.get())
+    injector.set(null)
+
+    awaitGC(injectorRef)
+
+    then:
+    null == injectorRef.get()
+
+    when:
+    def loaderRef = new WeakReference(emptyLoader.get())
+    emptyLoader.set(null)
+
+    awaitGC(loaderRef)
+
+    then:
+    null == loaderRef.get()
   }
 }

dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/WeakConcurrentSupplierTest.groovy

@@ -1,14 +1,16 @@
 package datadog.trace.agent.tooling
 
-
 import datadog.trace.bootstrap.WeakMap
 import datadog.trace.util.gc.GCUtils
+import spock.lang.Retry
 import spock.lang.Shared
 import spock.lang.Specification
 
 import java.lang.ref.WeakReference
 import java.util.concurrent.TimeUnit
 
+@Retry
+// These tests fail sometimes in CI.
 class WeakConcurrentSupplierTest extends Specification {
   @Shared
   def weakConcurrentSupplier = new WeakMapSuppliers.WeakConcurrent()

dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientInstrumentationTest.groovy

@@ -20,13 +20,13 @@ import java.util.concurrent.CompletionStage
 import java.util.concurrent.ExecutionException
 
 import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
+import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT
 import static datadog.trace.agent.test.utils.TraceUtils.withConfigOverride
 
 class AkkaHttpClientInstrumentationTest extends AgentTestRunner {
 
   private static final String MESSAGE = "an\nmultiline\nhttp\nresponse"
   private static final long TIMEOUT = 10000L
-  private static final int UNUSED_PORT = 61 // this port should always be closed
 
   @AutoCleanup
   @Shared
@@ -109,7 +109,7 @@ class AkkaHttpClientInstrumentationTest extends AgentTestRunner {
 
   def "error request trace"() {
     setup:
-    def url = new URL("http://localhost:$UNUSED_PORT/test")
+    def url = new URL("http://localhost:$UNUSABLE_PORT/test")
 
     HttpRequest request = HttpRequest.create(url.toString())
     CompletionStage<HttpResponse> responseFuture =
@@ -139,7 +139,7 @@ class AkkaHttpClientInstrumentationTest extends AgentTestRunner {
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
             "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
             "$Tags.PEER_HOSTNAME.key" server.address.host
-            "$Tags.PEER_PORT.key" UNUSED_PORT
+            "$Tags.PEER_PORT.key" UNUSABLE_PORT
             "$Tags.COMPONENT.key" "akka-http-client"
             "$Tags.ERROR.key" true
             errorTags(StreamTcpException, { it.contains("Tcp command") })
@@ -241,7 +241,7 @@ class AkkaHttpClientInstrumentationTest extends AgentTestRunner {
   def "error request pool trace"() {
     setup:
     // Use port number that really should be closed
-    def url = new URL("http://localhost:$UNUSED_PORT/test")
+    def url = new URL("http://localhost:$UNUSABLE_PORT/test")
 
     def response = withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") {
       Source
@@ -272,7 +272,7 @@ class AkkaHttpClientInstrumentationTest extends AgentTestRunner {
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
             "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
             "$Tags.PEER_HOSTNAME.key" server.address.host
-            "$Tags.PEER_PORT.key" UNUSED_PORT
+            "$Tags.PEER_PORT.key" UNUSABLE_PORT
             "$Tags.COMPONENT.key" "akka-http-client"
             "$Tags.ERROR.key" true
             errorTags(StreamTcpException, { it.contains("Tcp command") })

dd-java-agent/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy

@@ -6,14 +6,13 @@ import datadog.trace.instrumentation.http_url_connection.UrlInstrumentation
 import io.opentracing.tag.Tags
 import io.opentracing.util.GlobalTracer
 
+import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT
 import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
 import static datadog.trace.agent.test.utils.TraceUtils.withConfigOverride
 import static datadog.trace.instrumentation.http_url_connection.HttpUrlConnectionInstrumentation.HttpUrlState.OPERATION_NAME
 
 class UrlConnectionTest extends AgentTestRunner {
 
-  private static final int UNUSED_PORT = 61 // this port should always be closed
-
   def "trace request with connection failure #scheme"() {
     when:
     withConfigOverride(Config.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN, "$renameService") {
@@ -53,7 +52,7 @@ class UrlConnectionTest extends AgentTestRunner {
             "$Tags.HTTP_URL.key" "$url"
             "$Tags.HTTP_METHOD.key" "GET"
             "$Tags.PEER_HOSTNAME.key" "localhost"
-            "$Tags.PEER_PORT.key" UNUSED_PORT
+            "$Tags.PEER_PORT.key" UNUSABLE_PORT
             errorTags ConnectException, String
             defaultTags()
           }
@@ -66,7 +65,7 @@ class UrlConnectionTest extends AgentTestRunner {
     "http"  | true
     "https" | false
 
-    url = new URI("$scheme://localhost:$UNUSED_PORT").toURL()
+    url = new URI("$scheme://localhost:$UNUSABLE_PORT").toURL()
   }
 
   def "trace request with connection failure to a local file with broken url path"() {

dd-java-agent/instrumentation/hystrix-1.4/src/main/java/datadog/trace/instrumentation/hystrix/HystrixCommandInstrumentation.java

@@ -1,7 +1,7 @@
 package datadog.trace.instrumentation.hystrix;
 
 import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
-import static io.opentracing.log.Fields.ERROR_OBJECT;
+import static datadog.trace.instrumentation.hystrix.HystrixDecorator.DECORATE;
 import static java.util.Collections.singletonMap;
 import static net.bytebuddy.matcher.ElementMatchers.isInterface;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
@@ -11,12 +11,8 @@ import static net.bytebuddy.matcher.ElementMatchers.not;
 import com.google.auto.service.AutoService;
 import com.netflix.hystrix.HystrixCommand;
 import datadog.trace.agent.tooling.Instrumenter;
-import datadog.trace.api.DDTags;
 import io.opentracing.Scope;
-import io.opentracing.Span;
-import io.opentracing.tag.Tags;
 import io.opentracing.util.GlobalTracer;
-import java.util.Collections;
 import java.util.Map;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.method.MethodDescription;
@@ -34,11 +30,16 @@ public class HystrixCommandInstrumentation extends Instrumenter.Default {
 
   @Override
   public ElementMatcher<TypeDescription> typeMatcher() {
-    // Not adding a version restriction because this should work with any version and add some
-    // benefit.
     return not(isInterface()).and(safeHasSuperType(named("com.netflix.hystrix.HystrixCommand")));
   }
 
+  @Override
+  public String[] helperClassNames() {
+    return new String[] {
+      "datadog.trace.agent.decorator.BaseDecorator", packageName + ".HystrixDecorator",
+    };
+  }
+
   @Override
   public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
     return singletonMap(
@@ -52,30 +53,17 @@ public class HystrixCommandInstrumentation extends Instrumenter.Default {
         @Advice.This final HystrixCommand<?> command,
         @Advice.Origin("#m") final String methodName) {
 
-      final String commandName = command.getCommandKey().name();
+      final Scope scope = GlobalTracer.get().buildSpan(OPERATION_NAME).startActive(true);
-      final String groupName = command.getCommandGroup().name();
+      DECORATE.afterStart(scope);
-      final boolean circuitOpen = command.isCircuitBreakerOpen();
+      DECORATE.onCommand(scope, command, methodName);
-
+      return scope;
-      final String resourceName = groupName + "." + commandName + "." + methodName;
-
-      return GlobalTracer.get()
-          .buildSpan(OPERATION_NAME)
-          .withTag(DDTags.RESOURCE_NAME, resourceName)
-          .withTag(Tags.COMPONENT.getKey(), "hystrix")
-          .withTag("hystrix.command", commandName)
-          .withTag("hystrix.group", groupName)
-          .withTag("hystrix.circuit-open", circuitOpen)
-          .startActive(true);
     }
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void stopSpan(
         @Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable) {
-      if (throwable != null) {
+      DECORATE.onError(scope, throwable);
-        final Span span = scope.span();
+      DECORATE.beforeFinish(scope);
-        Tags.ERROR.set(span, true);
-        span.log(Collections.singletonMap(ERROR_OBJECT, throwable));
-      }
       scope.close();
     }
   }

dd-java-agent/instrumentation/hystrix-1.4/src/main/java/datadog/trace/instrumentation/hystrix/HystrixDecorator.java

@@ -0,0 +1,43 @@
+package datadog.trace.instrumentation.hystrix;
+
+import com.netflix.hystrix.HystrixCommand;
+import datadog.trace.agent.decorator.BaseDecorator;
+import datadog.trace.api.DDTags;
+import io.opentracing.Scope;
+import io.opentracing.Span;
+
+public class HystrixDecorator extends BaseDecorator {
+  public static HystrixDecorator DECORATE = new HystrixDecorator();
+
+  @Override
+  protected String[] instrumentationNames() {
+    return new String[0];
+  }
+
+  @Override
+  protected String spanType() {
+    return null;
+  }
+
+  @Override
+  protected String component() {
+    return "hystrix";
+  }
+
+  public void onCommand(
+      final Scope scope, final HystrixCommand<?> command, final String methodName) {
+    if (command != null) {
+      final String commandName = command.getCommandKey().name();
+      final String groupName = command.getCommandGroup().name();
+      final boolean circuitOpen = command.isCircuitBreakerOpen();
+
+      final String resourceName = groupName + "." + commandName + "." + methodName;
+
+      final Span span = scope.span();
+      span.setTag(DDTags.RESOURCE_NAME, resourceName);
+      span.setTag("hystrix.command", commandName);
+      span.setTag("hystrix.group", groupName);
+      span.setTag("hystrix.circuit-open", circuitOpen);
+    }
+  }
+}

dd-java-agent/instrumentation/java-concurrent/akka-2.5-testing/src/test/groovy/AkkaExecutorInstrumentationTest.groovy

@@ -5,6 +5,7 @@ import datadog.opentracing.scopemanager.ContinuableScope
 import datadog.trace.agent.test.AgentTestRunner
 import datadog.trace.api.Trace
 import io.opentracing.util.GlobalTracer
+import spock.lang.Retry
 import spock.lang.Shared
 
 import java.lang.reflect.InvocationTargetException
@@ -19,6 +20,7 @@ import java.util.concurrent.TimeUnit
  * Test executor instrumentation for Akka specific classes.
  * This is to large extent a copy of ExecutorInstrumentationTest.
  */
+@Retry
 class AkkaExecutorInstrumentationTest extends AgentTestRunner {
 
   @Shared

dd-java-agent/instrumentation/java-concurrent/scala-testing/src/test/groovy/ScalaExecutorInstrumentationTest.groovy

@@ -5,6 +5,7 @@ import datadog.trace.api.Trace
 import io.opentracing.util.GlobalTracer
 import scala.concurrent.forkjoin.ForkJoinPool
 import scala.concurrent.forkjoin.ForkJoinTask
+import spock.lang.Retry
 import spock.lang.Shared
 
 import java.lang.reflect.InvocationTargetException
@@ -19,6 +20,7 @@ import java.util.concurrent.TimeUnit
  * Test executor instrumentation for Scala specific classes.
  * This is to large extent a copy of ExecutorInstrumentationTest.
  */
+@Retry
 class ScalaExecutorInstrumentationTest extends AgentTestRunner {
 
   @Shared

dd-java-agent/instrumentation/java-concurrent/src/test/groovy/ExecutorInstrumentationTest.groovy

@@ -5,6 +5,7 @@ import datadog.trace.api.Trace
 import datadog.trace.bootstrap.instrumentation.java.concurrent.CallableWrapper
 import datadog.trace.bootstrap.instrumentation.java.concurrent.RunnableWrapper
 import io.opentracing.util.GlobalTracer
+import spock.lang.Retry
 import spock.lang.Shared
 
 import java.lang.reflect.InvocationTargetException
@@ -18,6 +19,7 @@ import java.util.concurrent.ScheduledThreadPoolExecutor
 import java.util.concurrent.ThreadPoolExecutor
 import java.util.concurrent.TimeUnit
 
+@Retry
 class ExecutorInstrumentationTest extends AgentTestRunner {
 
   @Shared

dd-java-agent/instrumentation/jax-rs-annotations/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsAnnotationsDecorator.java

@@ -0,0 +1,116 @@
+package datadog.trace.instrumentation.jaxrs;
+
+import static datadog.trace.bootstrap.WeakMap.Provider.newWeakMap;
+
+import datadog.trace.agent.decorator.BaseDecorator;
+import datadog.trace.api.DDTags;
+import datadog.trace.bootstrap.WeakMap;
+import io.opentracing.Scope;
+import io.opentracing.Span;
+import io.opentracing.tag.Tags;
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Method;
+import java.util.LinkedList;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import javax.ws.rs.HttpMethod;
+import javax.ws.rs.Path;
+
+public class JaxRsAnnotationsDecorator extends BaseDecorator {
+  public static JaxRsAnnotationsDecorator DECORATE = new JaxRsAnnotationsDecorator();
+
+  private final WeakMap<Class, Map<Method, String>> resourceNames = newWeakMap();
+
+  @Override
+  protected String[] instrumentationNames() {
+    return new String[0];
+  }
+
+  @Override
+  protected String spanType() {
+    return null;
+  }
+
+  @Override
+  protected String component() {
+    return "jax-rs-controller";
+  }
+
+  public void updateParent(final Scope scope, final Method method) {
+    if (scope == null) {
+      return;
+    }
+    final Span span = scope.span();
+    Tags.COMPONENT.set(span, "jax-rs");
+
+    final Class<?> target = method.getDeclaringClass();
+    Map<Method, String> classMap = resourceNames.get(target);
+
+    if (classMap == null) {
+      resourceNames.putIfAbsent(target, new ConcurrentHashMap<Method, String>());
+      classMap = resourceNames.get(target);
+      // classMap should not be null at this point because we have a
+      // strong reference to target and don't manually clear the map.
+    }
+
+    String resourceName = classMap.get(method);
+    if (resourceName == null) {
+      final String httpMethod = locateHttpMethod(method);
+      final LinkedList<Path> paths = gatherPaths(method);
+      resourceName = buildResourceName(httpMethod, paths);
+      classMap.put(method, resourceName);
+    }
+
+    if (!resourceName.isEmpty()) {
+      span.setTag(DDTags.RESOURCE_NAME, resourceName);
+    }
+  }
+
+  private String locateHttpMethod(final Method method) {
+    String httpMethod = null;
+    for (final Annotation ann : method.getDeclaredAnnotations()) {
+      if (ann.annotationType().getAnnotation(HttpMethod.class) != null) {
+        httpMethod = ann.annotationType().getSimpleName();
+      }
+    }
+    return httpMethod;
+  }
+
+  private LinkedList<Path> gatherPaths(final Method method) {
+    Class<?> target = method.getDeclaringClass();
+    final LinkedList<Path> paths = new LinkedList<>();
+    while (target != Object.class) {
+      final Path annotation = target.getAnnotation(Path.class);
+      if (annotation != null) {
+        paths.push(annotation);
+      }
+      target = target.getSuperclass();
+    }
+    final Path methodPath = method.getAnnotation(Path.class);
+    if (methodPath != null) {
+      paths.add(methodPath);
+    }
+    return paths;
+  }
+
+  private String buildResourceName(final String httpMethod, final LinkedList<Path> paths) {
+    final String resourceName;
+    final StringBuilder resourceNameBuilder = new StringBuilder();
+    if (httpMethod != null) {
+      resourceNameBuilder.append(httpMethod);
+      resourceNameBuilder.append(" ");
+    }
+    Path last = null;
+    for (final Path path : paths) {
+      if (path.value().startsWith("/") || (last != null && last.value().endsWith("/"))) {
+        resourceNameBuilder.append(path.value());
+      } else {
+        resourceNameBuilder.append("/");
+        resourceNameBuilder.append(path.value());
+      }
+      last = path;
+    }
+    resourceName = resourceNameBuilder.toString().trim();
+    return resourceName;
+  }
+}

dd-java-agent/instrumentation/jax-rs-annotations/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsAnnotationsInstrumentation.java

@@ -1,7 +1,7 @@
 package datadog.trace.instrumentation.jaxrs;
 
 import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
-import static io.opentracing.log.Fields.ERROR_OBJECT;
+import static datadog.trace.instrumentation.jaxrs.JaxRsAnnotationsDecorator.DECORATE;
 import static java.util.Collections.singletonMap;
 import static net.bytebuddy.matcher.ElementMatchers.declaresMethod;
 import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith;
@@ -9,18 +9,10 @@ import static net.bytebuddy.matcher.ElementMatchers.named;
 
 import com.google.auto.service.AutoService;
 import datadog.trace.agent.tooling.Instrumenter;
-import datadog.trace.api.DDTags;
 import io.opentracing.Scope;
-import io.opentracing.Span;
-import io.opentracing.tag.Tags;
 import io.opentracing.util.GlobalTracer;
-import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
-import java.util.Collections;
-import java.util.LinkedList;
 import java.util.Map;
-import javax.ws.rs.HttpMethod;
-import javax.ws.rs.Path;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.method.MethodDescription;
 import net.bytebuddy.description.type.TypeDescription;
@@ -40,6 +32,13 @@ public final class JaxRsAnnotationsInstrumentation extends Instrumenter.Default
             .or(safeHasSuperType(declaresMethod(isAnnotatedWith(named("javax.ws.rs.Path"))))));
   }
 
+  @Override
+  public String[] helperClassNames() {
+    return new String[] {
+      "datadog.trace.agent.decorator.BaseDecorator", packageName + ".JaxRsAnnotationsDecorator",
+    };
+  }
+
   @Override
   public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
     return singletonMap(
@@ -58,83 +57,20 @@ public final class JaxRsAnnotationsInstrumentation extends Instrumenter.Default
 
     @Advice.OnMethodEnter(suppress = Throwable.class)
     public static Scope nameSpan(@Advice.Origin final Method method) {
-
+      // Rename the parent span according to the path represented by these annotations.
-      // TODO: do we need caching for this?
-      final LinkedList<Path> paths = new LinkedList<>();
-      Class<?> target = method.getDeclaringClass();
-      while (target != Object.class) {
-        final Path annotation = target.getAnnotation(Path.class);
-        if (annotation != null) {
-          paths.push(annotation);
-        }
-        target = target.getSuperclass();
-      }
-      final Path methodPath = method.getAnnotation(Path.class);
-      if (methodPath != null) {
-        paths.add(methodPath);
-      }
-      String httpMethod = null;
-      for (final Annotation ann : method.getDeclaredAnnotations()) {
-        if (ann.annotationType().getAnnotation(HttpMethod.class) != null) {
-          httpMethod = ann.annotationType().getSimpleName();
-        }
-      }
-
-      final StringBuilder resourceNameBuilder = new StringBuilder();
-      if (httpMethod != null) {
-        resourceNameBuilder.append(httpMethod);
-        resourceNameBuilder.append(" ");
-      }
-      Path last = null;
-      for (final Path path : paths) {
-        if (path.value().startsWith("/") || (last != null && last.value().endsWith("/"))) {
-          resourceNameBuilder.append(path.value());
-        } else {
-          resourceNameBuilder.append("/");
-          resourceNameBuilder.append(path.value());
-        }
-        last = path;
-      }
-      final String resourceName = resourceNameBuilder.toString().trim();
-
       final Scope scope = GlobalTracer.get().scopeManager().active();
-      if (scope != null && !resourceName.isEmpty()) {
+      DECORATE.updateParent(scope, method);
-        scope.span().setTag(DDTags.RESOURCE_NAME, resourceName);
-        Tags.COMPONENT.set(scope.span(), "jax-rs");
-      }
 
       // Now create a span representing the method execution.
-
+      final String operationName = DECORATE.spanNameForMethod(method);
-      final Class<?> clazz = method.getDeclaringClass();
+      return DECORATE.afterStart(GlobalTracer.get().buildSpan(operationName).startActive(true));
-      final String methodName = method.getName();
-
-      String className = clazz.getSimpleName();
-      if (className.isEmpty()) {
-        className = clazz.getName();
-        if (clazz.getPackage() != null) {
-          final String pkgName = clazz.getPackage().getName();
-          if (!pkgName.isEmpty()) {
-            className = clazz.getName().replace(pkgName, "").substring(1);
-          }
-        }
-      }
-
-      final String operationName = className + "." + methodName;
-
-      return GlobalTracer.get()
-          .buildSpan(operationName)
-          .withTag(Tags.COMPONENT.getKey(), "jax-rs-controller")
-          .startActive(true);
     }
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void stopSpan(
         @Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable) {
-      if (throwable != null) {
+      DECORATE.onError(scope, throwable);
-        final Span span = scope.span();
+      DECORATE.beforeFinish(scope);
-        Tags.ERROR.set(span, true);
-        span.log(Collections.singletonMap(ERROR_OBJECT, throwable));
-      }
       scope.close();
     }
   }

dd-java-agent/instrumentation/jax-rs-annotations/src/test/groovy/JaxRsAnnotationsInstrumentationTest.groovy

@@ -10,11 +10,13 @@ import javax.ws.rs.PUT
 import javax.ws.rs.Path
 
 import static datadog.trace.agent.test.utils.TraceUtils.runUnderTrace
+import static datadog.trace.instrumentation.jaxrs.JaxRsAnnotationsDecorator.DECORATE
 
 class JaxRsAnnotationsInstrumentationTest extends AgentTestRunner {
 
   def "span named '#name' from annotations on class"() {
     setup:
+    def startingCacheSize = DECORATE.resourceNames.size()
     runUnderTrace("test") {
       obj.call()
     }
@@ -42,6 +44,18 @@ class JaxRsAnnotationsInstrumentationTest extends AgentTestRunner {
         }
       }
     }
+    DECORATE.resourceNames.size() == startingCacheSize + 1
+    DECORATE.resourceNames.get(obj.class).size() == 1
+
+    when: "multiple calls to the same method"
+    runUnderTrace("test") {
+      (1..10).each {
+        obj.call()
+      }
+    }
+    then: "doesn't increase the cache size"
+    DECORATE.resourceNames.size() == startingCacheSize + 1
+    DECORATE.resourceNames.get(obj.class).size() == 1
 
     where:
     name                        | obj

dd-java-agent/instrumentation/jax-rs-client/connection-error-handling-jersey/src/main/java/datadog/trace/instrumentation/connection_error/jersey/JerseyClientConnectionErrorInstrumentation.java

@@ -1,5 +1,6 @@
 package datadog.trace.instrumentation.connection_error.jersey;
 
+import static io.opentracing.log.Fields.ERROR_OBJECT;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.isPublic;
 import static net.bytebuddy.matcher.ElementMatchers.named;
@@ -42,7 +43,9 @@ public final class JerseyClientConnectionErrorInstrumentation extends Instrument
 
   @Override
   public String[] helperClassNames() {
-    return new String[] {getClass().getName() + "$WrappedFuture"};
+    return new String[] {
+      getClass().getName() + "$WrappedFuture",
+    };
   }
 
   @Override
@@ -62,12 +65,11 @@ public final class JerseyClientConnectionErrorInstrumentation extends Instrument
         @Advice.FieldValue("requestContext") final ClientRequest context,
         @Advice.Thrown final Throwable throwable) {
       if (throwable != null) {
-
         final Object prop = context.getProperty(ClientTracingFilter.SPAN_PROPERTY_NAME);
         if (prop instanceof Span) {
           final Span span = (Span) prop;
           Tags.ERROR.set(span, true);
-          span.log(Collections.singletonMap(Fields.ERROR_OBJECT, throwable));
+          span.log(Collections.singletonMap(ERROR_OBJECT, throwable));
           span.finish();
         }
       }

dd-java-agent/instrumentation/jax-rs-client/connection-error-handling-resteasy/src/main/java/datadog/trace/instrumentation/connection_error/resteasy/ResteasyClientConnectionErrorInstrumentation.java

@@ -43,7 +43,9 @@ public final class ResteasyClientConnectionErrorInstrumentation extends Instrume
 
   @Override
   public String[] helperClassNames() {
-    return new String[] {getClass().getName() + "$WrappedFuture"};
+    return new String[] {
+      getClass().getName() + "$WrappedFuture",
+    };
   }
 
   @Override

dd-java-agent/instrumentation/jax-rs-client/src/main/java/datadog/trace/instrumentation/jaxrs/ClientTracingFilter.java

@@ -1,10 +1,10 @@
 package datadog.trace.instrumentation.jaxrs;
 
-import datadog.trace.api.DDSpanTypes;
+import static datadog.trace.instrumentation.jaxrs.JaxRsClientDecorator.DECORATE;
+
 import datadog.trace.api.DDTags;
 import io.opentracing.Span;
 import io.opentracing.propagation.Format;
-import io.opentracing.tag.Tags;
 import io.opentracing.util.GlobalTracer;
 import javax.annotation.Priority;
 import javax.ws.rs.Priorities;
@@ -25,13 +25,10 @@ public class ClientTracingFilter implements ClientRequestFilter, ClientResponseF
     final Span span =
         GlobalTracer.get()
             .buildSpan("jax-rs.client.call")
-            .withTag(Tags.COMPONENT.getKey(), "jax-rs.client")
-            .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT)
-            .withTag(Tags.HTTP_METHOD.getKey(), requestContext.getMethod())
-            .withTag(Tags.HTTP_URL.getKey(), requestContext.getUri().toString())
-            .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT)
             .withTag(DDTags.RESOURCE_NAME, requestContext.getMethod() + " jax-rs.client.call")
             .start();
+    DECORATE.afterStart(span);
+    DECORATE.onRequest(span, requestContext);
 
     log.debug("{} - client span started", span);
 
@@ -50,8 +47,8 @@ public class ClientTracingFilter implements ClientRequestFilter, ClientResponseF
     final Object spanObj = requestContext.getProperty(SPAN_PROPERTY_NAME);
     if (spanObj instanceof Span) {
       final Span span = (Span) spanObj;
-      Tags.HTTP_STATUS.set(span, responseContext.getStatus());
+      DECORATE.onResponse(span, responseContext);
-
+      DECORATE.beforeFinish(span);
       span.finish();
       log.debug("{} - client spanObj finished", spanObj);
     }

dd-java-agent/instrumentation/jax-rs-client/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsClientDecorator.java

@@ -0,0 +1,45 @@
+package datadog.trace.instrumentation.jaxrs;
+
+import datadog.trace.agent.decorator.HttpClientDecorator;
+import javax.ws.rs.client.ClientRequestContext;
+import javax.ws.rs.client.ClientResponseContext;
+
+public class JaxRsClientDecorator
+    extends HttpClientDecorator<ClientRequestContext, ClientResponseContext> {
+  public static final JaxRsClientDecorator DECORATE = new JaxRsClientDecorator();
+
+  @Override
+  protected String[] instrumentationNames() {
+    return new String[] {"jax-rs", "jaxrs", "jax-rs-client"};
+  }
+
+  @Override
+  protected String component() {
+    return "jax-rs.client";
+  }
+
+  @Override
+  protected String method(final ClientRequestContext httpRequest) {
+    return httpRequest.getMethod();
+  }
+
+  @Override
+  protected String url(final ClientRequestContext httpRequest) {
+    return httpRequest.getUri().toString();
+  }
+
+  @Override
+  protected String hostname(final ClientRequestContext httpRequest) {
+    return httpRequest.getUri().getHost();
+  }
+
+  @Override
+  protected Integer port(final ClientRequestContext httpRequest) {
+    return httpRequest.getUri().getPort();
+  }
+
+  @Override
+  protected Integer status(final ClientResponseContext httpResponse) {
+    return httpResponse.getStatus();
+  }
+}

dd-java-agent/instrumentation/jax-rs-client/src/main/java/datadog/trace/instrumentation/jaxrs/JaxRsClientInstrumentation.java

@@ -29,9 +29,13 @@ public final class JaxRsClientInstrumentation extends Instrumenter.Default {
   @Override
   public String[] helperClassNames() {
     return new String[] {
-      "datadog.trace.instrumentation.jaxrs.ClientTracingFeature",
+      "datadog.trace.agent.decorator.BaseDecorator",
-      "datadog.trace.instrumentation.jaxrs.ClientTracingFilter",
+      "datadog.trace.agent.decorator.ClientDecorator",
-      "datadog.trace.instrumentation.jaxrs.InjectAdapter"
+      "datadog.trace.agent.decorator.HttpClientDecorator",
+      packageName + ".JaxRsClientDecorator",
+      packageName + ".ClientTracingFeature",
+      packageName + ".ClientTracingFilter",
+      packageName + ".InjectAdapter",
     };
   }
 

dd-java-agent/instrumentation/jax-rs-client/src/test/groovy/JaxRsClientTest.groovy

@@ -1,5 +1,4 @@
 import datadog.trace.agent.test.AgentTestRunner
-import datadog.trace.agent.test.utils.PortUtils
 import datadog.trace.api.DDSpanTypes
 import datadog.trace.api.DDTags
 import io.opentracing.tag.Tags
@@ -19,12 +18,10 @@ import javax.ws.rs.core.Response
 import java.util.concurrent.ExecutionException
 
 import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
+import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT
 
 class JaxRsClientTest extends AgentTestRunner {
 
-  @Shared
-  def emptyPort = PortUtils.randomOpenPort()
-
   @AutoCleanup
   @Shared
   def server = httpServer {
@@ -61,12 +58,13 @@ class JaxRsClientTest extends AgentTestRunner {
           parent()
           errored false
           tags {
-
             "$Tags.COMPONENT.key" "jax-rs.client"
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
             "$Tags.HTTP_METHOD.key" "GET"
             "$Tags.HTTP_STATUS.key" 200
             "$Tags.HTTP_URL.key" "$server.address/ping"
+            "$Tags.PEER_HOSTNAME.key" "localhost"
+            "$Tags.PEER_PORT.key" server.address.port
             "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
             defaultTags()
           }
@@ -90,7 +88,7 @@ class JaxRsClientTest extends AgentTestRunner {
   def "#lib connection failure creates errored span"() {
     when:
     Client client = builder.build()
-    WebTarget service = client.target("http://localhost:$emptyPort/ping")
+    WebTarget service = client.target("http://localhost:$UNUSABLE_PORT/ping")
     if (async) {
       AsyncInvoker request = service.request(MediaType.TEXT_PLAIN).async()
       request.get().get()
@@ -115,7 +113,9 @@ class JaxRsClientTest extends AgentTestRunner {
             "$Tags.COMPONENT.key" "jax-rs.client"
             "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
             "$Tags.HTTP_METHOD.key" "GET"
-            "$Tags.HTTP_URL.key" "http://localhost:$emptyPort/ping"
+            "$Tags.HTTP_URL.key" "http://localhost:$UNUSABLE_PORT/ping"
+            "$Tags.PEER_HOSTNAME.key" "localhost"
+            "$Tags.PEER_PORT.key" UNUSABLE_PORT
             "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
             errorTags ProcessingException, String
             defaultTags()

dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceAdvice.java

@@ -1,14 +1,11 @@
 package datadog.trace.instrumentation.trace_annotation;
 
-import static io.opentracing.log.Fields.ERROR_OBJECT;
+import static datadog.trace.instrumentation.trace_annotation.TraceDecorator.DECORATE;
 
 import datadog.trace.api.Trace;
 import io.opentracing.Scope;
-import io.opentracing.Span;
-import io.opentracing.tag.Tags;
 import io.opentracing.util.GlobalTracer;
 import java.lang.reflect.Method;
-import java.util.Collections;
 import net.bytebuddy.asm.Advice;
 
 public class TraceAdvice {
@@ -18,34 +15,16 @@ public class TraceAdvice {
     final Trace trace = method.getAnnotation(Trace.class);
     String operationName = trace == null ? null : trace.operationName();
     if (operationName == null || operationName.isEmpty()) {
-      final Class<?> declaringClass = method.getDeclaringClass();
+      operationName = DECORATE.spanNameForMethod(method);
-      String className = declaringClass.getSimpleName();
-      if (className.isEmpty()) {
-        className = declaringClass.getName();
-        if (declaringClass.getPackage() != null) {
-          final String pkgName = declaringClass.getPackage().getName();
-          if (!pkgName.isEmpty()) {
-            className = declaringClass.getName().replace(pkgName, "").substring(1);
-          }
-        }
-      }
-      operationName = className + "." + method.getName();
     }
-
+    return DECORATE.afterStart(GlobalTracer.get().buildSpan(operationName).startActive(true));
-    return GlobalTracer.get()
-        .buildSpan(operationName)
-        .withTag(Tags.COMPONENT.getKey(), "trace")
-        .startActive(true);
   }
 
   @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
   public static void stopSpan(
       @Advice.Enter final Scope scope, @Advice.Thrown final Throwable throwable) {
-    if (throwable != null) {
+    DECORATE.onError(scope, throwable);
-      final Span span = scope.span();
+    DECORATE.beforeFinish(scope);
-      Tags.ERROR.set(span, true);
-      span.log(Collections.singletonMap(ERROR_OBJECT, throwable));
-    }
     scope.close();
   }
 }

dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceAnnotationsInstrumentation.java

@@ -82,6 +82,13 @@ public final class TraceAnnotationsInstrumentation extends Instrumenter.Default
     return safeHasSuperType(declaresMethod(isAnnotatedWith(methodTraceMatcher)));
   }
 
+  @Override
+  public String[] helperClassNames() {
+    return new String[] {
+      "datadog.trace.agent.decorator.BaseDecorator", packageName + ".TraceDecorator",
+    };
+  }
+
   @Override
   public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
     return Collections.singletonMap(

dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceConfigInstrumentation.java

@@ -120,6 +120,13 @@ public class TraceConfigInstrumentation implements Instrumenter {
       return safeHasSuperType(named(className));
     }
 
+    @Override
+    public String[] helperClassNames() {
+      return new String[] {
+        "datadog.trace.agent.decorator.BaseDecorator", packageName + ".TraceDecorator",
+      };
+    }
+
     @Override
     public Map<ElementMatcher<? super MethodDescription>, String> transformers() {
       ElementMatcher.Junction<MethodDescription> methodMatchers = null;

dd-java-agent/instrumentation/trace-annotation/src/main/java/datadog/trace/instrumentation/trace_annotation/TraceDecorator.java

@@ -0,0 +1,22 @@
+package datadog.trace.instrumentation.trace_annotation;
+
+import datadog.trace.agent.decorator.BaseDecorator;
+
+public class TraceDecorator extends BaseDecorator {
+  public static TraceDecorator DECORATE = new TraceDecorator();
+
+  @Override
+  protected String[] instrumentationNames() {
+    return new String[0];
+  }
+
+  @Override
+  protected String spanType() {
+    return null;
+  }
+
+  @Override
+  protected String component() {
+    return "trace";
+  }
+}

dd-java-agent/src/test/groovy/datadog/trace/agent/integration/classloading/ClassLoadingTest.groovy

@@ -6,11 +6,13 @@ import datadog.trace.agent.test.IntegrationTestUtils
 import datadog.trace.api.Trace
 import datadog.trace.util.gc.GCUtils
 import spock.lang.Specification
+import spock.lang.Timeout
 
 import java.lang.ref.WeakReference
 
 import static datadog.trace.agent.test.IntegrationTestUtils.createJarWithClasses
 
+@Timeout(10)
 class ClassLoadingTest extends Specification {
 
   final URL[] classpath = [createJarWithClasses(ClassToInstrument, ClassToInstrumentChild, Trace)]

dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/AgentTestRunner.java

@@ -178,13 +178,13 @@ public abstract class AgentTestRunner extends Specification {
 
   @AfterClass
   public static synchronized void agentCleanup() {
-    assert INSTRUMENTATION_ERROR_COUNT.get() == 0
-        : INSTRUMENTATION_ERROR_COUNT.get() + " Instrumentation errors during test";
-
     if (null != activeTransformer) {
       INSTRUMENTATION.removeTransformer(activeTransformer);
       activeTransformer = null;
     }
+    // Cleanup before assertion.
+    assert INSTRUMENTATION_ERROR_COUNT.get() == 0
+        : INSTRUMENTATION_ERROR_COUNT.get() + " Instrumentation errors during test";
   }
 
   public static void assertTraces(

dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/ClasspathUtils.java

@@ -13,6 +13,7 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.lang.reflect.Method;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLClassLoader;
@@ -156,4 +157,24 @@ public class ClasspathUtils {
     }
     return new URLClassLoader(urls.build().toArray(new URL[0]), null);
   }
+
+  // Moved this to a java class because groovy was adding a hard ref to classLoader
+  public static boolean isClassLoaded(final String className, final ClassLoader classLoader) {
+    try {
+      final Method findLoadedClassMethod =
+          ClassLoader.class.getDeclaredMethod("findLoadedClass", String.class);
+      try {
+        findLoadedClassMethod.setAccessible(true);
+        final Class<?> loadedClass =
+            (Class<?>) findLoadedClassMethod.invoke(classLoader, className);
+        return null != loadedClass && loadedClass.getClassLoader() == classLoader;
+      } catch (final Exception e) {
+        throw new IllegalStateException(e);
+      } finally {
+        findLoadedClassMethod.setAccessible(false);
+      }
+    } catch (final NoSuchMethodException e) {
+      throw new IllegalStateException(e);
+    }
+  }
 }

dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/utils/PortUtils.java

@@ -5,6 +5,8 @@ import java.net.ServerSocket;
 
 public class PortUtils {
 
+  public static int UNUSABLE_PORT = 61;
+
   /** Open up a random, reusable port. */
   public static int randomOpenPort() {
     final ServerSocket socket;

dd-java-agent/testing/src/test/java/muzzle/MuzzleWeakReferenceTest.java

@@ -14,7 +14,7 @@ public class MuzzleWeakReferenceTest {
    *
    * Even returning a WeakReference<ClassLoader> is enough for spock to create a strong ref.
    */
-  public static boolean classLoaderRefIsGarbageCollected() {
+  public static boolean classLoaderRefIsGarbageCollected() throws InterruptedException {
     ClassLoader loader = new URLClassLoader(new URL[0], null);
     final WeakReference<ClassLoader> clRef = new WeakReference<>(loader);
     final Reference[] refs =

utils/gc-utils/src/main/java/datadog/trace/util/gc/GCUtils.java

@@ -4,15 +4,18 @@ import java.lang.ref.WeakReference;
 
 public abstract class GCUtils {
 
-  public static void awaitGC() {
+  public static void awaitGC() throws InterruptedException {
     Object obj = new Object();
     final WeakReference<Object> ref = new WeakReference<>(obj);
     obj = null;
     awaitGC(ref);
   }
 
-  public static void awaitGC(final WeakReference<?> ref) {
+  public static void awaitGC(final WeakReference<?> ref) throws InterruptedException {
     while (ref.get() != null) {
+      if (Thread.interrupted()) {
+        throw new InterruptedException();
+      }
       System.gc();
       System.runFinalization();
     }