Merge pull request #572 from DataDog/gary/refactor-ratpack

Refactor Ratpack

.circleci/config.yml

@@ -172,24 +172,6 @@ jobs:
     - store_artifacts:
         path: ./reports
 
-  scan_versions:
-    <<: *defaults
-    steps:
-    - checkout
-
-    - restore_cache:
-        # Reset the cache approx every release
-        keys:
-        - dd-trace-java-version-scan-{{ checksum "dd-trace-java.gradle" }}
-
-    - run:
-        name: Verify Version Scan
-        command: GRADLE_OPTS="-Ddatadog.forkedMaxHeapSize=4G -Ddatadog.forkedMinHeapSize=64M" ./gradlew verifyVersionScan --parallel --stacktrace --no-daemon --max-workers=8
-
-    - save_cache:
-        key: dd-trace-java-version-scan-{{ checksum "dd-trace-java.gradle" }}
-        paths: ~/.gradle
-
   muzzle:
     <<: *defaults
     steps:
@@ -290,13 +272,6 @@ workflows:
           tags:
             only: /.*/
 
-    - scan_versions:
-        requires:
-        - build
-        filters:
-          branches:
-            ignore: master
-
     - muzzle:
         requires:
         - build

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleVersionScanPlugin.java

@@ -89,14 +89,10 @@ public class MuzzleVersionScanPlugin {
                       .newInstance();
         }
         try {
-          // Ratpack injects the scope manager as a helper.
+          // verify helper injector works
-          // This is likely a bug, but we'll grandfather it out of the helper checks for now.
+          final String[] helperClassNames = instrumenter.helperClassNames();
-          if (!instrumenter.getClass().getName().contains("Ratpack")) {
+          if (helperClassNames.length > 0) {
-            // verify helper injector works
+            new HelperInjector(helperClassNames).transform(null, null, cl, null);
-            final String[] helperClassNames = instrumenter.helperClassNames();
-            if (helperClassNames.length > 0) {
-              new HelperInjector(helperClassNames).transform(null, null, cl, null);
-            }
           }
         } catch (final Exception e) {
           System.err.println(

dd-java-agent/instrumentation/ratpack-1.4/ratpack-1.4.gradle

@@ -4,16 +4,12 @@ ext {
   maxJavaVersionForTests = JavaVersion.VERSION_1_8
 }
 
-apply plugin: 'version-scan'
+muzzle {
-
+  pass {
-versionScan {
+    group = "io.ratpack"
-  group = "io.ratpack"
+    module = 'ratpack-core'
-  module = 'ratpack-core'
+    versions = "[1.4.0,)"
-  versions = "[1.4.0,)"
+  }
-  scanMethods = true
-  verifyPresent = [
-    "ratpack.path.PathBinding": "getDescription",
-  ]
 }
 
 apply from: "${rootDir}/gradle/java.gradle"
@@ -51,7 +47,6 @@ testSets {
 dependencies {
   main_java8CompileOnly group: 'io.ratpack', name: 'ratpack-core', version: '1.4.0'
 
-  main_java8Compile project(':dd-trace-ot')
   main_java8Compile project(':dd-java-agent:agent-tooling')
 
   main_java8Compile deps.bytebuddy
@@ -65,6 +60,7 @@ dependencies {
   compile sourceSets.main_java8.output
 
   testCompile project(':dd-java-agent:testing')
+  testCompile project(':dd-java-agent:instrumentation:java-concurrent')
   testCompile group: 'io.ratpack', name: 'ratpack-groovy-test', version: '1.4.0'
   latestDepTestCompile group: 'io.ratpack', name: 'ratpack-groovy-test', version: '+'
 }

dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackHttpClientInstrumentation.java

@@ -1,7 +1,6 @@
 package datadog.trace.instrumentation.ratpack;
 
 import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
-import static datadog.trace.instrumentation.ratpack.RatpackInstrumentation.CLASSLOADER_CONTAINS_RATPACK_1_4_OR_ABOVE;
 import static net.bytebuddy.matcher.ElementMatchers.isInterface;
 import static net.bytebuddy.matcher.ElementMatchers.named;
 import static net.bytebuddy.matcher.ElementMatchers.not;
@@ -38,11 +37,6 @@ public final class RatpackHttpClientInstrumentation extends Instrumenter.Default
     return not(isInterface()).and(safeHasSuperType(named("ratpack.http.client.HttpClient")));
   }
 
-  @Override
-  public ElementMatcher<ClassLoader> classLoaderMatcher() {
-    return CLASSLOADER_CONTAINS_RATPACK_1_4_OR_ABOVE;
-  }
-
   @Override
   public String[] helperClassNames() {
     return new String[] {
@@ -55,9 +49,6 @@ public final class RatpackHttpClientInstrumentation extends Instrumenter.Default
       "datadog.trace.instrumentation.ratpack.impl.RatpackHttpClientAdvice$StreamedResponseAction",
       "datadog.trace.instrumentation.ratpack.impl.RequestSpecInjectAdapter",
       "datadog.trace.instrumentation.ratpack.impl.WrappedRequestSpec",
-      // core helpers
-      "datadog.opentracing.scopemanager.ContextualScopeManager",
-      "datadog.opentracing.scopemanager.ScopeContext"
     };
   }
 

dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackInstrumentation.java

@@ -1,7 +1,6 @@
 package datadog.trace.instrumentation.ratpack;
 
 import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType;
-import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClassWithMethod;
 import static net.bytebuddy.matcher.ElementMatchers.isInterface;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.isStatic;
@@ -29,10 +28,6 @@ public final class RatpackInstrumentation extends Instrumenter.Default {
   static final TypeDescription.Latent ACTION_TYPE_DESCRIPTION =
       new TypeDescription.Latent("ratpack.func.Action", Modifier.PUBLIC, null);
 
-  static final ElementMatcher.Junction.AbstractBase<ClassLoader>
-      CLASSLOADER_CONTAINS_RATPACK_1_4_OR_ABOVE =
-          classLoaderHasClassWithMethod("ratpack.path.PathBinding", "getDescription");
-
   public RatpackInstrumentation() {
     super(EXEC_NAME);
   }
@@ -47,21 +42,11 @@ public final class RatpackInstrumentation extends Instrumenter.Default {
     return named("ratpack.server.internal.ServerRegistry");
   }
 
-  @Override
-  public ElementMatcher<ClassLoader> classLoaderMatcher() {
-    return CLASSLOADER_CONTAINS_RATPACK_1_4_OR_ABOVE;
-  }
-
   @Override
   public String[] helperClassNames() {
     return new String[] {
-      // core helpers
-      "datadog.opentracing.scopemanager.ContextualScopeManager",
-      "datadog.opentracing.scopemanager.ScopeContext",
       // service registry helpers
       "datadog.trace.instrumentation.ratpack.impl.RatpackRequestExtractAdapter",
-      "datadog.trace.instrumentation.ratpack.impl.RatpackScopeManager",
-      "datadog.trace.instrumentation.ratpack.impl.RatpackScopeManager$RatpackScope",
       "datadog.trace.instrumentation.ratpack.impl.RatpackServerAdvice",
       "datadog.trace.instrumentation.ratpack.impl.RatpackServerAdvice$RatpackServerRegistryAdvice",
       "datadog.trace.instrumentation.ratpack.impl.TracingHandler"
@@ -94,17 +79,9 @@ public final class RatpackInstrumentation extends Instrumenter.Default {
       return not(isInterface()).and(safeHasSuperType(named("ratpack.exec.ExecStarter")));
     }
 
-    @Override
-    public ElementMatcher<ClassLoader> classLoaderMatcher() {
-      return CLASSLOADER_CONTAINS_RATPACK_1_4_OR_ABOVE;
-    }
-
     @Override
     public String[] helperClassNames() {
       return new String[] {
-        // core helpers
-        "datadog.opentracing.scopemanager.ContextualScopeManager",
-        "datadog.opentracing.scopemanager.ScopeContext",
         // exec helpers
         "datadog.trace.instrumentation.ratpack.impl.RatpackServerAdvice$ExecStarterAdvice",
         "datadog.trace.instrumentation.ratpack.impl.RatpackServerAdvice$ExecStarterAction"
@@ -139,11 +116,6 @@ public final class RatpackInstrumentation extends Instrumenter.Default {
           .or(not(isInterface()).and(safeHasSuperType(named("ratpack.exec.Execution"))));
     }
 
-    @Override
-    public ElementMatcher<ClassLoader> classLoaderMatcher() {
-      return CLASSLOADER_CONTAINS_RATPACK_1_4_OR_ABOVE;
-    }
-
     @Override
     public String[] helperClassNames() {
       return new String[] {

dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/RatpackScopeManager.java

@@ -1,82 +0,0 @@
-package datadog.trace.instrumentation.ratpack.impl;
-
-import datadog.opentracing.scopemanager.ScopeContext;
-import io.opentracing.Scope;
-import io.opentracing.Span;
-import ratpack.exec.Execution;
-import ratpack.exec.UnmanagedThreadException;
-
-/**
- * This scope manager uses the Ratpack Execution to store the current Scope. This is a ratpack
- * registry analogous to a ThreadLocal but for an execution that may transfer between several
- * threads
- */
-public final class RatpackScopeManager implements ScopeContext {
-  @Override
-  public boolean inContext() {
-    return Execution.isManagedThread();
-  }
-
-  @Override
-  public Scope activate(Span span, boolean finishSpanOnClose) {
-    Execution execution = Execution.current();
-    RatpackScope ratpackScope =
-        new RatpackScope(
-            span, finishSpanOnClose, execution.maybeGet(RatpackScope.class).orElse(null));
-    // remove any existing RatpackScopes before adding it to the registry
-    execution
-        .maybeGet(RatpackScope.class)
-        .ifPresent(ignored -> execution.remove(RatpackScope.class));
-    execution.add(RatpackScope.class, ratpackScope);
-    execution.onComplete(
-        ratpackScope); // ensure that the scope is closed when the execution finishes
-    return ratpackScope;
-  }
-
-  @Override
-  public Scope active() {
-    try {
-      return Execution.current().maybeGet(RatpackScope.class).orElse(null);
-    } catch (UnmanagedThreadException ume) {
-      return null; // should never happen due to inContextCheck
-    }
-  }
-
-  static class RatpackScope implements Scope {
-    private final Span wrapped;
-    private final boolean finishOnClose;
-    private final RatpackScope toRestore;
-
-    RatpackScope(Span wrapped, boolean finishOnClose, RatpackScope toRestore) {
-      this.wrapped = wrapped;
-      this.finishOnClose = finishOnClose;
-      this.toRestore = toRestore;
-    }
-
-    @Override
-    public Span span() {
-      return wrapped;
-    }
-
-    @Override
-    public void close() {
-      Execution execution = Execution.current();
-      // only close if this scope is the current scope for this Execution
-      // As with ThreadLocalScope this shouldn't happen if users call methods in the expected order
-      execution
-          .maybeGet(RatpackScope.class)
-          .filter(s -> this == s)
-          .ifPresent(
-              ignore -> {
-                if (finishOnClose) {
-                  wrapped.finish();
-                }
-                // pop the execution "stack"
-                execution.remove(RatpackScope.class);
-                if (toRestore != null) {
-                  execution.add(toRestore);
-                }
-              });
-    }
-  }
-}

dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/RatpackServerAdvice.java

@@ -1,8 +1,6 @@
 package datadog.trace.instrumentation.ratpack.impl;
 
-import datadog.opentracing.scopemanager.ContextualScopeManager;
 import io.opentracing.Scope;
-import io.opentracing.ScopeManager;
 import io.opentracing.util.GlobalTracer;
 import lombok.extern.slf4j.Slf4j;
 import net.bytebuddy.asm.Advice;
@@ -17,25 +15,9 @@ public class RatpackServerAdvice {
   public static class RatpackServerRegistryAdvice {
     @Advice.OnMethodExit(suppress = Throwable.class)
     public static void injectTracing(@Advice.Return(readOnly = false) Registry registry) {
-      RatpackScopeManager ratpackScopeManager = new RatpackScopeManager();
-      // the value returned from ServerRegistry.buildBaseRegistry needs to be modified to add our
-      // scope manager and handler decorator to the registry
-      //noinspection UnusedAssignment
       registry =
           registry.join(
-              Registry.builder()
+              Registry.builder().add(HandlerDecorator.prepend(new TracingHandler())).build());
-                  .add(ScopeManager.class, ratpackScopeManager)
-                  .add(HandlerDecorator.prepend(new TracingHandler()))
-                  .build());
-
-      if (GlobalTracer.isRegistered()) {
-        if (GlobalTracer.get().scopeManager() instanceof ContextualScopeManager) {
-          ((ContextualScopeManager) GlobalTracer.get().scopeManager())
-              .addScopeContext(ratpackScopeManager);
-        }
-      } else {
-        log.warn("No GlobalTracer registered");
-      }
     }
   }
 
@@ -43,9 +25,8 @@ public class RatpackServerAdvice {
     @Advice.OnMethodEnter
     public static void addScopeToRegistry(
         @Advice.Argument(value = 0, readOnly = false) Action<? super RegistrySpec> action) {
-      Scope active = GlobalTracer.get().scopeManager().active();
+      final Scope active = GlobalTracer.get().scopeManager().active();
       if (active != null) {
-        //noinspection UnusedAssignment
         action = new ExecStarterAction(active).append(action);
       }
     }
@@ -53,8 +34,8 @@ public class RatpackServerAdvice {
 
   public static class ExecutionAdvice {
     @Advice.OnMethodExit
-    public static void addScopeToRegistry(@Advice.Return ExecStarter starter) {
+    public static void addScopeToRegistry(@Advice.Return final ExecStarter starter) {
-      Scope active = GlobalTracer.get().scopeManager().active();
+      final Scope active = GlobalTracer.get().scopeManager().active();
       if (active != null) {
         starter.register(new ExecStarterAction(active));
       }
@@ -64,13 +45,12 @@ public class RatpackServerAdvice {
   public static class ExecStarterAction implements Action<RegistrySpec> {
     private final Scope active;
 
-    @SuppressWarnings("WeakerAccess")
+    public ExecStarterAction(final Scope active) {
-    public ExecStarterAction(Scope active) {
       this.active = active;
     }
 
     @Override
-    public void execute(RegistrySpec spec) {
+    public void execute(final RegistrySpec spec) {
       if (active != null) {
         spec.add(active);
       }

dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/TracingHandler.java

@@ -2,6 +2,7 @@ package datadog.trace.instrumentation.ratpack.impl;
 
 import datadog.trace.api.DDSpanTypes;
 import datadog.trace.api.DDTags;
+import datadog.trace.context.TraceScope;
 import io.opentracing.Scope;
 import io.opentracing.Span;
 import io.opentracing.SpanContext;
@@ -35,23 +36,43 @@ public final class TracingHandler implements Handler {
             .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_SERVER)
             .withTag(Tags.HTTP_METHOD.getKey(), request.getMethod().getName())
             .withTag(Tags.HTTP_URL.getKey(), request.getUri())
-            .startActive(true);
+            .startActive(false);
+
+    if (scope instanceof TraceScope) {
+      ((TraceScope) scope).setAsyncPropagation(true);
+    }
+
+    final Span rootSpan = scope.span();
 
     ctx.getResponse()
         .beforeSend(
             response -> {
-              final Span span = scope.span();
+              final Scope responseScope = GlobalTracer.get().scopeManager().active();
-              span.setTag(DDTags.RESOURCE_NAME, getResourceName(ctx));
+
+              if (responseScope instanceof TraceScope) {
+                ((TraceScope) responseScope).setAsyncPropagation(false);
+              }
+
+              rootSpan.setTag(DDTags.RESOURCE_NAME, getResourceName(ctx));
               final Status status = response.getStatus();
               if (status != null) {
                 if (status.is5xx()) {
-                  Tags.ERROR.set(span, true);
+                  Tags.ERROR.set(rootSpan, true);
                 }
-                Tags.HTTP_STATUS.set(span, status.getCode());
+                Tags.HTTP_STATUS.set(rootSpan, status.getCode());
               }
-              scope.close();
+
+              rootSpan.finish();
             });
 
+    ctx.onClose(
+        requestOutcome -> {
+          final Scope activeScope = GlobalTracer.get().scopeManager().active();
+          if (activeScope != null) {
+            activeScope.close();
+          }
+        });
+
     ctx.next();
   }
 

dd-java-agent/instrumentation/ratpack-1.4/src/test/groovy/RatpackTest.groovy

@@ -1,9 +1,10 @@
-import datadog.opentracing.scopemanager.ContextualScopeManager
 import datadog.trace.agent.test.AgentTestRunner
 import datadog.trace.agent.test.utils.OkHttpUtils
 import datadog.trace.api.DDSpanTypes
-import datadog.trace.instrumentation.ratpack.impl.RatpackScopeManager
+import datadog.trace.api.DDTags
+import datadog.trace.context.TraceScope
 import io.opentracing.Scope
+import io.opentracing.tag.Tags
 import io.opentracing.util.GlobalTracer
 import okhttp3.HttpUrl
 import okhttp3.OkHttpClient
@@ -37,29 +38,35 @@ class RatpackTest extends AgentTestRunner {
       .url(app.address.toURL())
       .get()
       .build()
+
     when:
     def resp = client.newCall(request).execute()
+
     then:
     resp.code() == 200
     resp.body.string() == "success"
 
-    TEST_WRITER.size() == 1
+    assertTraces(1) {
-    def trace = TEST_WRITER.firstTrace()
+      trace(0, 1) {
-    trace.size() == 1
+        span(0) {
-    def span = trace[0]
+          resourceName "GET /"
-
+          serviceName "unnamed-java-app"
-    span.context().serviceName == "unnamed-java-app"
+          operationName "ratpack.handler"
-    span.context().operationName == "ratpack.handler"
+          spanType DDSpanTypes.HTTP_SERVER
-    span.context().resourceName == "GET /"
+          parent()
-    span.context().tags["component"] == "ratpack"
+          errored false
-    span.context().spanType == DDSpanTypes.HTTP_SERVER
+          tags {
-    !span.context().getErrorFlag()
+            "$Tags.COMPONENT.key" "ratpack"
-    span.context().tags["http.url"] == "/"
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER
-    span.context().tags["http.method"] == "GET"
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_SERVER
-    span.context().tags["span.kind"] == "server"
+            "$Tags.HTTP_METHOD.key" "GET"
-    span.context().tags["http.status_code"] == 200
+            "$Tags.HTTP_STATUS.key" 200
-    span.context().tags["thread.name"] != null
+            "$Tags.HTTP_URL.key" "/"
-    span.context().tags["thread.id"] != null
+            defaultTags()
+          }
+        }
+      }
+    }
   }
 
   def "test path with bindings call"() {
@@ -77,29 +84,35 @@ class RatpackTest extends AgentTestRunner {
       .url(HttpUrl.get(app.address).newBuilder().addPathSegments("a/b/baz").build())
       .get()
       .build()
+
     when:
     def resp = client.newCall(request).execute()
+
     then:
     resp.code() == 200
     resp.body.string() == ":foo/:bar?/baz"
 
-    TEST_WRITER.size() == 1
+    assertTraces(1) {
-    def trace = TEST_WRITER.firstTrace()
+      trace(0, 1) {
-    trace.size() == 1
+        span(0) {
-    def span = trace[0]
+          resourceName "GET /:foo/:bar?/baz"
-
+          serviceName "unnamed-java-app"
-    span.context().serviceName == "unnamed-java-app"
+          operationName "ratpack.handler"
-    span.context().operationName == "ratpack.handler"
+          spanType DDSpanTypes.HTTP_SERVER
-    span.context().resourceName == "GET /:foo/:bar?/baz"
+          parent()
-    span.context().tags["component"] == "ratpack"
+          errored false
-    span.context().spanType == DDSpanTypes.HTTP_SERVER
+          tags {
-    !span.context().getErrorFlag()
+            "$Tags.COMPONENT.key" "ratpack"
-    span.context().tags["http.url"] == "/a/b/baz"
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER
-    span.context().tags["http.method"] == "GET"
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_SERVER
-    span.context().tags["span.kind"] == "server"
+            "$Tags.HTTP_METHOD.key" "GET"
-    span.context().tags["http.status_code"] == 200
+            "$Tags.HTTP_STATUS.key" 200
-    span.context().tags["thread.name"] != null
+            "$Tags.HTTP_URL.key" "/a/b/baz"
-    span.context().tags["thread.id"] != null
+            defaultTags()
+          }
+        }
+      }
+    }
   }
 
   def "test error response"() {
@@ -107,7 +120,9 @@ class RatpackTest extends AgentTestRunner {
     def app = GroovyEmbeddedApp.ratpack {
       handlers {
         get {
-          context.clientError(500)
+          context.render(Promise.sync {
+            return "fail " + 0 / 0
+          })
         }
       }
     }
@@ -120,23 +135,29 @@ class RatpackTest extends AgentTestRunner {
     then:
     resp.code() == 500
 
-    TEST_WRITER.size() == 1
+    assertTraces(1) {
-    def trace = TEST_WRITER.firstTrace()
+      trace(0, 1) {
-    trace.size() == 1
+        span(0) {
-    def span = trace[0]
+          resourceName "GET /"
-
+          serviceName "unnamed-java-app"
-    span.context().getErrorFlag()
+          operationName "ratpack.handler"
-    span.context().serviceName == "unnamed-java-app"
+          spanType DDSpanTypes.HTTP_SERVER
-    span.context().operationName == "ratpack.handler"
+          parent()
-    span.context().resourceName == "GET /"
+          errored true
-    span.context().tags["component"] == "ratpack"
+          tags {
-    span.context().spanType == DDSpanTypes.HTTP_SERVER
+            "$Tags.COMPONENT.key" "ratpack"
-    span.context().tags["http.url"] == "/"
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER
-    span.context().tags["http.method"] == "GET"
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_SERVER
-    span.context().tags["span.kind"] == "server"
+            "$Tags.HTTP_METHOD.key" "GET"
-    span.context().tags["http.status_code"] == 500
+            "$Tags.HTTP_STATUS.key" 500
-    span.context().tags["thread.name"] != null
+            "$Tags.HTTP_URL.key" "/"
-    span.context().tags["thread.id"] != null
+            "error" true
+//            errorTags(Exception, String) // TODO: find out how to get throwable in instrumentation
+            defaultTags()
+          }
+        }
+      }
+    }
   }
 
   def "test path call using ratpack http client"() {
@@ -173,8 +194,10 @@ class RatpackTest extends AgentTestRunner {
       .url(app.address.toURL())
       .get()
       .build()
+
     when:
     def resp = client.newCall(request).execute()
+
     then:
     resp.code() == 200
     resp.body().string() == "success"
@@ -182,110 +205,225 @@ class RatpackTest extends AgentTestRunner {
     // 3rd is the three traces, ratpack, http client 2 and http client 1
     // 2nd is nested2 from the external server (the result of the 2nd internal http client call)
     // 1st is nested from the external server (the result of the 1st internal http client call)
-    TEST_WRITER.size() == 3
+    assertTraces(3) {
-    def trace = TEST_WRITER.get(2)
+      // simulated external system, first call
-    trace.size() == 3
+      trace(0, 1) {
-    def span = trace[0]
+        span(0) {
+          resourceName "GET /nested"
+          serviceName "unnamed-java-app"
+          operationName "ratpack.handler"
+          spanType DDSpanTypes.HTTP_SERVER
+          childOf(trace(2).get(2))
+          errored false
+          tags {
+            "$Tags.COMPONENT.key" "ratpack"
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_SERVER
+            "$Tags.HTTP_METHOD.key" "GET"
+            "$Tags.HTTP_STATUS.key" 200
+            "$Tags.HTTP_URL.key" "/nested"
+            defaultTags(true)
+          }
+        }
+      }
+      // simulated external system, second call
+      trace(1, 1) {
+        span(0) {
+          resourceName "GET /nested2"
+          serviceName "unnamed-java-app"
+          operationName "ratpack.handler"
+          spanType DDSpanTypes.HTTP_SERVER
+          childOf(trace(2).get(1))
+          errored false
+          tags {
+            "$Tags.COMPONENT.key" "ratpack"
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_SERVER
+            "$Tags.HTTP_METHOD.key" "GET"
+            "$Tags.HTTP_STATUS.key" 200
+            "$Tags.HTTP_URL.key" "/nested2"
+            defaultTags(true)
+          }
+        }
+      }
+      trace(2, 3) {
+        // main app span that processed the request from OKHTTP request
+        span(0) {
+          resourceName "GET /"
+          serviceName "unnamed-java-app"
+          operationName "ratpack.handler"
+          spanType DDSpanTypes.HTTP_SERVER
+          parent()
+          errored false
+          tags {
+            "$Tags.COMPONENT.key" "ratpack"
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_SERVER
+            "$Tags.HTTP_METHOD.key" "GET"
+            "$Tags.HTTP_STATUS.key" 200
+            "$Tags.HTTP_URL.key" "/"
+            defaultTags()
+          }
+        }
+        // Second http client call that receives the 'ess' of Success
+        span(1) {
+          resourceName "GET /?"
+          serviceName "unnamed-java-app"
+          operationName "ratpack.client-request"
+          spanType DDSpanTypes.HTTP_CLIENT
+          childOf(span(0))
+          errored false
+          tags {
+            "$Tags.COMPONENT.key" "ratpack-httpclient"
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
+            "$Tags.HTTP_METHOD.key" "GET"
+            "$Tags.HTTP_STATUS.key" 200
+            "$Tags.HTTP_URL.key" "${external.address}nested2"
+            defaultTags()
+          }
+        }
+        // First http client call that receives the 'Succ' of Success
+        span(2) {
+          resourceName "GET /nested"
+          serviceName "unnamed-java-app"
+          operationName "ratpack.client-request"
+          spanType DDSpanTypes.HTTP_CLIENT
+          childOf(span(0))
+          errored false
+          tags {
+            "$Tags.COMPONENT.key" "ratpack-httpclient"
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
+            "$Tags.HTTP_METHOD.key" "GET"
+            "$Tags.HTTP_STATUS.key" 200
+            "$Tags.HTTP_URL.key" "${external.address}nested"
+            defaultTags()
+          }
+        }
+      }
+    }
+  }
 
-    span.context().serviceName == "unnamed-java-app"
+  def "test forked path call and start span in handler (#startSpanInHandler)"() {
-    span.context().operationName == "ratpack.handler"
+    setup:
-    span.context().resourceName == "GET /"
+    def app = GroovyEmbeddedApp.ratpack {
-    span.context().tags["component"] == "ratpack"
+      handlers {
-    span.context().spanType == DDSpanTypes.HTTP_SERVER
+        get {
-    !span.context().getErrorFlag()
+          final Scope scope = !startSpanInHandler ? GlobalTracer.get().scopeManager().active() :
-    span.context().tags["http.url"] == "/"
+            GlobalTracer.get()
-    span.context().tags["http.method"] == "GET"
+              .buildSpan("ratpack.exec-test")
-    span.context().tags["span.kind"] == "server"
+              .withTag(DDTags.RESOURCE_NAME, "INSIDE-TEST")
-    span.context().tags["http.status_code"] == 200
+              .startActive(true)
-    span.context().tags["thread.name"] != null
-    span.context().tags["thread.id"] != null
 
-    def clientTrace1 = trace[1] // Second http client call that receives the 'ess' of Success
+          if (startSpanInHandler) {
+            ((TraceScope) scope).setAsyncPropagation(true)
+          }
+          scope.span().setBaggageItem("test-baggage", "foo")
+          context.render(testPromise().fork())
 
-    clientTrace1.context().serviceName == "unnamed-java-app"
+          if (startSpanInHandler) {
-    clientTrace1.context().operationName == "ratpack.client-request"
+            ((TraceScope) scope).setAsyncPropagation(false)
-    clientTrace1.context().tags["component"] == "ratpack-httpclient"
+          }
-    !clientTrace1.context().getErrorFlag()
+          scope.close()
-    clientTrace1.context().tags["http.url"] == "${external.address}nested2"
+        }
-    clientTrace1.context().tags["http.method"] == "GET"
+      }
-    clientTrace1.context().tags["span.kind"] == "client"
+    }
-    clientTrace1.context().tags["http.status_code"] == 200
+    def request = new Request.Builder()
-    clientTrace1.context().tags["thread.name"] != null
+      .url(app.address.toURL())
-    clientTrace1.context().tags["thread.id"] != null
+      .get()
+      .build()
 
-    def clientTrace2 = trace[2] // First http client call that receives the 'Succ' of Success
+    when:
+    def resp = client.newCall(request).execute()
 
-    clientTrace2.context().serviceName == "unnamed-java-app"
+    then:
-    clientTrace2.context().operationName == "ratpack.client-request"
+    resp.code() == 200
-    clientTrace1.context().tags["component"] == "ratpack-httpclient"
+    resp.body().string() == "foo"
-    !clientTrace2.context().getErrorFlag()
-    clientTrace2.context().tags["http.url"] == "${external.address}nested"
-    clientTrace2.context().tags["http.method"] == "GET"
-    clientTrace2.context().tags["span.kind"] == "client"
-    clientTrace2.context().tags["http.status_code"] == 200
-    clientTrace2.context().tags["thread.name"] != null
-    clientTrace2.context().tags["thread.id"] != null
 
-    def nestedTrace = TEST_WRITER.get(1)
+    assertTraces(1) {
-    nestedTrace.size() == 1
+      trace(0, (startSpanInHandler ? 2 : 1)) {
-    def nestedSpan = nestedTrace[0] // simulated external system, second call
+        span(startSpanInHandler ? 1 : 0) {
+          resourceName "GET /"
+          serviceName "unnamed-java-app"
+          operationName "ratpack.handler"
+          spanType DDSpanTypes.HTTP_SERVER
+          parent()
+          errored false
+          tags {
+            "$Tags.COMPONENT.key" "ratpack"
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_SERVER
+            "$Tags.HTTP_METHOD.key" "GET"
+            "$Tags.HTTP_STATUS.key" 200
+            "$Tags.HTTP_URL.key" "/"
+            defaultTags()
+          }
+        }
+        if (startSpanInHandler) {
+          span(0) {
+            resourceName "INSIDE-TEST"
+            serviceName "unnamed-java-app"
+            operationName "ratpack.exec-test"
+            spanType DDSpanTypes.HTTP_SERVER
+            childOf(span(1))
+            errored false
+            tags {
+              "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_SERVER
+              defaultTags()
+            }
+          }
+        }
+      }
+    }
 
-    nestedSpan.context().serviceName == "unnamed-java-app"
+    where:
-    nestedSpan.context().operationName == "ratpack.handler"
+    startSpanInHandler << [true, false]
-    nestedSpan.context().resourceName == "GET /nested2"
-    nestedSpan.context().tags["component"] == "ratpack"
-    nestedSpan.context().spanType == DDSpanTypes.HTTP_SERVER
-    !nestedSpan.context().getErrorFlag()
-    nestedSpan.context().tags["http.url"] == "/nested2"
-    nestedSpan.context().tags["http.method"] == "GET"
-    nestedSpan.context().tags["span.kind"] == "server"
-    nestedSpan.context().tags["http.status_code"] == 200
-    nestedSpan.context().tags["thread.name"] != null
-    nestedSpan.context().tags["thread.id"] != null
-
-    def nestedTrace2 = TEST_WRITER.get(0)
-    nestedTrace2.size() == 1
-    def nestedSpan2 = nestedTrace2[0] // simulated external system, first call
-
-    nestedSpan2.context().serviceName == "unnamed-java-app"
-    nestedSpan2.context().operationName == "ratpack.handler"
-    nestedSpan2.context().resourceName == "GET /nested"
-    nestedSpan2.context().tags["component"] == "ratpack"
-    nestedSpan2.context().spanType == DDSpanTypes.HTTP_SERVER
-    !nestedSpan2.context().getErrorFlag()
-    nestedSpan2.context().tags["http.url"] == "/nested"
-    nestedSpan2.context().tags["http.method"] == "GET"
-    nestedSpan2.context().tags["span.kind"] == "server"
-    nestedSpan2.context().tags["http.status_code"] == 200
-    nestedSpan2.context().tags["thread.name"] != null
-    nestedSpan2.context().tags["thread.id"] != null
   }
 
   def "forked executions inherit parent scope"() {
     when:
-    def result = ExecHarness.yieldSingle({ spec ->
+    def result = ExecHarness.yieldSingle({}, {
-      // This does the work of the initial instrumentation that occurs on the server registry. Because we are using
-      // ExecHarness for testing this does not get executed by the instrumentation
-      def ratpackScopeManager = new RatpackScopeManager()
-      spec.add(ratpackScopeManager)
-      ((ContextualScopeManager) GlobalTracer.get().scopeManager())
-        .addScopeContext(ratpackScopeManager)
-    }, {
       final Scope scope =
         GlobalTracer.get()
           .buildSpan("ratpack.exec-test")
+          .withTag(DDTags.RESOURCE_NAME, "INSIDE-TEST")
           .startActive(true)
+
+      ((TraceScope) scope).setAsyncPropagation(true)
       scope.span().setBaggageItem("test-baggage", "foo")
-      ParallelBatch.of(testPromise(), testPromise()).yield()
+      ParallelBatch.of(testPromise(), testPromise())
+        .yield()
+        .map({ now ->
+        // close the scope now that we got the baggage inside the promises
+        scope.close()
+        return now
+      })
     })
 
     then:
     result.valueOrThrow == ["foo", "foo"]
+    assertTraces(1) {
+      trace(0, 1) {
+        span(0) {
+          resourceName "INSIDE-TEST"
+          serviceName "unnamed-java-app"
+          operationName "ratpack.exec-test"
+          parent()
+          errored false
+          tags {
+            defaultTags()
+          }
+        }
+      }
+    }
   }
 
+  // returns a promise that contains the active scope's "test-baggage" baggage
   Promise<String> testPromise() {
     Promise.sync {
-      GlobalTracer.get().activeSpan().getBaggageItem("test-baggage")
+      Scope tracerScope = GlobalTracer.get().scopeManager().active()
+      return tracerScope.span().getBaggageItem("test-baggage")
     }
   }
 }