From a11b888d7a83fa5c08dba20d6c302c8325ca116c Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Mon, 8 Apr 2019 10:58:46 -0700 Subject: [PATCH 1/2] Update ratpack instrumentation and remove default disabled. --- .../tooling/muzzle/ReferenceCreator.java | 92 ++- .../netty41/AttributeKeys.java | 4 + .../ChannelFutureListenerInstrumentation.java | 2 +- .../ratpack-1.4/ratpack-1.4.gradle | 11 +- .../latestDepTest/groovy/RatpackTest.groovy | 775 ++++++++++++++++++ .../RatpackHttpClientInstrumentation.java | 79 -- .../ratpack/RatpackInstrumentation.java | 132 --- .../ServerErrorHandlerInstrumentation.java | 59 ++ .../ServerRegistryInstrumentation.java | 44 + .../ratpack/RatpackServerDecorator.java | 69 ++ .../ratpack/RatpackServerRegistryAdvice.java | 14 + .../ratpack/TracingHandler.java | 71 ++ .../ratpack/impl/RatpackHttpClientAdvice.java | 144 ---- .../impl/RatpackRequestExtractAdapter.java | 29 - .../ratpack/impl/RatpackServerAdvice.java | 59 -- .../impl/RequestSpecInjectAdapter.java | 29 - .../ratpack/impl/TracingHandler.java | 89 -- .../ratpack/impl/WrappedRequestSpec.java | 145 ---- .../src/test/groovy/RatpackTest.groovy | 468 +++++++++-- .../agent/test/asserts/TagsAssert.groovy | 10 +- .../java/datadog/opentracing/DDTracer.java | 1 + .../scopemanager/ContextualScopeManager.java | 3 +- .../scopemanager/ScopeContext.java | 1 + 23 files changed, 1513 insertions(+), 817 deletions(-) create mode 100644 dd-java-agent/instrumentation/ratpack-1.4/src/latestDepTest/groovy/RatpackTest.groovy delete mode 100644 dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackHttpClientInstrumentation.java delete mode 100644 dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackInstrumentation.java create mode 100644 dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ServerErrorHandlerInstrumentation.java create mode 100644 dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ServerRegistryInstrumentation.java create mode 100644 dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerDecorator.java create mode 100644 dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerRegistryAdvice.java create mode 100644 dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/TracingHandler.java delete mode 100644 dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/RatpackHttpClientAdvice.java delete mode 100644 dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/RatpackRequestExtractAdapter.java delete mode 100644 dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/RatpackServerAdvice.java delete mode 100644 dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/RequestSpecInjectAdapter.java delete mode 100644 dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/TracingHandler.java delete mode 100644 dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/WrappedRequestSpec.java diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/ReferenceCreator.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/ReferenceCreator.java index d1a3d76c54..2aafef2161 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/ReferenceCreator.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/ReferenceCreator.java @@ -32,10 +32,10 @@ public class ReferenceCreator extends ClassVisitor { *

For now we're hardcoding this to the instrumentation package so we only create references * from the method advice and helper classes. */ - private static String REFERENCE_CREATION_PACKAGE = "datadog.trace.instrumentation."; + private static final String REFERENCE_CREATION_PACKAGE = "datadog.trace.instrumentation."; public static Map createReferencesFrom( - String entryPointClassName, ClassLoader loader) { + final String entryPointClassName, final ClassLoader loader) { return ReferenceCreator.createReferencesFrom(entryPointClassName, loader, true); } @@ -48,7 +48,7 @@ public class ReferenceCreator extends ClassVisitor { * @return Map of [referenceClassName -> Reference] */ private static Map createReferencesFrom( - String entryPointClassName, ClassLoader loader, boolean startFromMethodBodies) { + final String entryPointClassName, final ClassLoader loader, boolean startFromMethodBodies) { final Set visitedSources = new HashSet<>(); final Map references = new HashMap<>(); @@ -58,37 +58,38 @@ public class ReferenceCreator extends ClassVisitor { while (!instrumentationQueue.isEmpty()) { final String className = instrumentationQueue.remove(); visitedSources.add(className); + final InputStream in = loader.getResourceAsStream(Utils.getResourceName(className)); try { - final InputStream in = loader.getResourceAsStream(Utils.getResourceName(className)); - try { - final ReferenceCreator cv = new ReferenceCreator(null, startFromMethodBodies); - // only start from method bodies on the first pass - startFromMethodBodies = false; - final ClassReader reader = new ClassReader(in); - reader.accept(cv, ClassReader.SKIP_FRAMES); + final ReferenceCreator cv = new ReferenceCreator(null, startFromMethodBodies); + // only start from method bodies on the first pass + startFromMethodBodies = false; + final ClassReader reader = new ClassReader(in); + reader.accept(cv, ClassReader.SKIP_FRAMES); - Map instrumentationReferences = cv.getReferences(); - for (Map.Entry entry : instrumentationReferences.entrySet()) { - // Don't generate references created outside of the datadog instrumentation package. - if (!visitedSources.contains(entry.getKey()) - && entry.getKey().startsWith(REFERENCE_CREATION_PACKAGE)) { - instrumentationQueue.add(entry.getKey()); - } - if (references.containsKey(entry.getKey())) { - references.put( - entry.getKey(), references.get(entry.getKey()).merge(entry.getValue())); - } else { - references.put(entry.getKey(), entry.getValue()); - } + final Map instrumentationReferences = cv.getReferences(); + for (final Map.Entry entry : instrumentationReferences.entrySet()) { + // Don't generate references created outside of the datadog instrumentation package. + if (!visitedSources.contains(entry.getKey()) + && entry.getKey().startsWith(REFERENCE_CREATION_PACKAGE)) { + instrumentationQueue.add(entry.getKey()); } - - } finally { - if (in != null) { - in.close(); + if (references.containsKey(entry.getKey())) { + references.put(entry.getKey(), references.get(entry.getKey()).merge(entry.getValue())); + } else { + references.put(entry.getKey(), entry.getValue()); + } + } + + } catch (final IOException e) { + throw new IllegalStateException("Error reading class " + className, e); + } finally { + if (in != null) { + try { + in.close(); + } catch (final IOException e) { + throw new IllegalStateException("Error closing class " + className, e); } } - } catch (IOException ioe) { - throw new IllegalStateException(ioe); } } return references; @@ -99,7 +100,7 @@ public class ReferenceCreator extends ClassVisitor { * *

foo/bar/Baz -> foo/bar/ */ - private static String internalPackageName(String internalName) { + private static String internalPackageName(final String internalName) { return internalName.replaceAll("/[^/]+$", ""); } @@ -108,7 +109,7 @@ public class ReferenceCreator extends ClassVisitor { * * @return A reference flag with the required level of access. */ - private static Reference.Flag computeMinimumClassAccess(Type from, Type to) { + private static Reference.Flag computeMinimumClassAccess(final Type from, final Type to) { if (from.getInternalName().equalsIgnoreCase(to.getInternalName())) { return Reference.Flag.PRIVATE_OR_HIGHER; } else if (internalPackageName(from.getInternalName()) @@ -124,7 +125,7 @@ public class ReferenceCreator extends ClassVisitor { * * @return A reference flag with the required level of access. */ - private static Reference.Flag computeMinimumFieldAccess(Type from, Type to) { + private static Reference.Flag computeMinimumFieldAccess(final Type from, final Type to) { if (from.getInternalName().equalsIgnoreCase(to.getInternalName())) { return Reference.Flag.PRIVATE_OR_HIGHER; } else if (internalPackageName(from.getInternalName()) @@ -142,7 +143,8 @@ public class ReferenceCreator extends ClassVisitor { * * @return A reference flag with the required level of access. */ - private static Reference.Flag computeMinimumMethodAccess(Type from, Type to, Type methodType) { + private static Reference.Flag computeMinimumMethodAccess( + final Type from, final Type to, final Type methodType) { if (from.getInternalName().equalsIgnoreCase(to.getInternalName())) { return Reference.Flag.PRIVATE_OR_HIGHER; } else { @@ -163,12 +165,13 @@ public class ReferenceCreator extends ClassVisitor { return type; } - private Map references = new HashMap<>(); + private final Map references = new HashMap<>(); private String refSourceClassName; private Type refSourceType; - private boolean createFromMethodBodiesOnly; + private final boolean createFromMethodBodiesOnly; - private ReferenceCreator(ClassVisitor classVisitor, boolean createFromMethodBodiesOnly) { + private ReferenceCreator( + final ClassVisitor classVisitor, final boolean createFromMethodBodiesOnly) { super(Opcodes.ASM7, classVisitor); this.createFromMethodBodiesOnly = createFromMethodBodiesOnly; } @@ -177,7 +180,7 @@ public class ReferenceCreator extends ClassVisitor { return references; } - private void addReference(Reference ref) { + private void addReference(final Reference ref) { if (references.containsKey(ref.getClassName())) { references.put(ref.getClassName(), references.get(ref.getClassName()).merge(ref)); } else { @@ -203,7 +206,11 @@ public class ReferenceCreator extends ClassVisitor { @Override public FieldVisitor visitField( - int access, String name, String descriptor, String signature, Object value) { + final int access, + final String name, + final String descriptor, + final String signature, + final Object value) { // Additional references we could check // - annotations on field @@ -228,7 +235,7 @@ public class ReferenceCreator extends ClassVisitor { private class AdviceReferenceMethodVisitor extends MethodVisitor { private int currentLineNumber = -1; - public AdviceReferenceMethodVisitor(MethodVisitor methodVisitor) { + public AdviceReferenceMethodVisitor(final MethodVisitor methodVisitor) { super(Opcodes.ASM7, methodVisitor); } @@ -239,7 +246,8 @@ public class ReferenceCreator extends ClassVisitor { } @Override - public void visitFieldInsn(int opcode, String owner, String name, String descriptor) { + public void visitFieldInsn( + final int opcode, final String owner, final String name, final String descriptor) { // Additional references we could check // * DONE owner class // * DONE owner class has a field (name) @@ -253,7 +261,7 @@ public class ReferenceCreator extends ClassVisitor { final Type ownerType = Type.getType("L" + owner + ";"); final Type fieldType = Type.getType(descriptor); - List fieldFlags = new ArrayList<>(); + final List fieldFlags = new ArrayList<>(); fieldFlags.add(computeMinimumFieldAccess(refSourceType, ownerType)); fieldFlags.add( opcode == Opcodes.GETSTATIC || opcode == Opcodes.PUTSTATIC @@ -324,7 +332,7 @@ public class ReferenceCreator extends ClassVisitor { } } - Type ownerType = Type.getType("L" + owner + ";"); + final Type ownerType = Type.getType("L" + owner + ";"); final List methodFlags = new ArrayList<>(); methodFlags.add( diff --git a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/AttributeKeys.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/AttributeKeys.java index c6fbc7d070..9b978483ba 100644 --- a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/AttributeKeys.java +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/AttributeKeys.java @@ -10,6 +10,10 @@ public class AttributeKeys { PARENT_CONNECT_CONTINUATION_ATTRIBUTE_KEY = AttributeKey.valueOf("datadog.trace.instrumentation.netty41.parent.connect.continuation"); + /** + * This constant is copied over to datadog.trace.instrumentation.ratpack.server.TracingHandler, so + * if this changes, that must also change. + */ public static final AttributeKey SERVER_ATTRIBUTE_KEY = AttributeKey.valueOf(HttpServerTracingHandler.class.getName() + ".span"); diff --git a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/ChannelFutureListenerInstrumentation.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/ChannelFutureListenerInstrumentation.java index 034d139e89..c32ed8da8b 100644 --- a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/ChannelFutureListenerInstrumentation.java +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/ChannelFutureListenerInstrumentation.java @@ -102,7 +102,7 @@ public class ChannelFutureListenerInstrumentation extends Instrumenter.Default { @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) public static void deactivateScope(@Advice.Enter final TraceScope scope) { if (scope != null) { - ((Scope) scope).close(); + scope.close(); } } } diff --git a/dd-java-agent/instrumentation/ratpack-1.4/ratpack-1.4.gradle b/dd-java-agent/instrumentation/ratpack-1.4/ratpack-1.4.gradle index c4868c4926..fcd4ffd6c3 100644 --- a/dd-java-agent/instrumentation/ratpack-1.4/ratpack-1.4.gradle +++ b/dd-java-agent/instrumentation/ratpack-1.4/ratpack-1.4.gradle @@ -10,6 +10,12 @@ muzzle { module = 'ratpack-core' versions = "[1.4.0,)" } + // Some maven dependencies are missing for pre 1.0 ratpack, so we can't assertInverse. + fail { + group = "io.ratpack" + module = 'ratpack-core' + versions = "[1.0,1.4.0)" + } } apply from: "${rootDir}/gradle/java.gradle" @@ -39,9 +45,7 @@ dependencies { apply plugin: 'org.unbroken-dome.test-sets' testSets { - latestDepTest { - dirName = 'test' - } + latestDepTest } dependencies { @@ -61,6 +65,7 @@ dependencies { testCompile project(':dd-java-agent:testing') testCompile project(':dd-java-agent:instrumentation:java-concurrent') + testCompile project(':dd-java-agent:instrumentation:netty-4.1') testCompile group: 'io.ratpack', name: 'ratpack-groovy-test', version: '1.4.0' latestDepTestCompile group: 'io.ratpack', name: 'ratpack-groovy-test', version: '+' } diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/latestDepTest/groovy/RatpackTest.groovy b/dd-java-agent/instrumentation/ratpack-1.4/src/latestDepTest/groovy/RatpackTest.groovy new file mode 100644 index 0000000000..fa88adc401 --- /dev/null +++ b/dd-java-agent/instrumentation/ratpack-1.4/src/latestDepTest/groovy/RatpackTest.groovy @@ -0,0 +1,775 @@ +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.agent.test.utils.OkHttpUtils +import datadog.trace.api.DDSpanTypes +import datadog.trace.api.DDTags +import datadog.trace.context.TraceScope +import io.netty.channel.AbstractChannel +import io.opentracing.Scope +import io.opentracing.Span +import io.opentracing.tag.Tags +import io.opentracing.util.GlobalTracer +import okhttp3.HttpUrl +import okhttp3.OkHttpClient +import okhttp3.Request +import ratpack.exec.Promise +import ratpack.exec.util.ParallelBatch +import ratpack.groovy.test.embed.GroovyEmbeddedApp +import ratpack.handling.internal.HandlerException +import ratpack.http.HttpUrlBuilder +import ratpack.http.client.HttpClient +import ratpack.path.PathBinding +import ratpack.test.exec.ExecHarness +import spock.lang.Retry + +import java.util.concurrent.CountDownLatch + +import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT + +@Retry +class RatpackTest extends AgentTestRunner { + + OkHttpClient client = OkHttpUtils.client() + + + def "test path call"() { + setup: + def app = GroovyEmbeddedApp.ratpack { + handlers { + get { + context.render("success") + } + } + } + def request = new Request.Builder() + .url(app.address.toURL()) + .get() + .build() + + when: + def resp = client.newCall(request).execute() + + then: + resp.code() == 200 + resp.body.string() == "success" + + assertTraces(1) { + trace(0, 2) { + span(0) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "netty.request" + spanType DDSpanTypes.HTTP_SERVER + parent() + errored false + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + defaultTags() + } + } + span(1) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "ratpack.handler" + spanType DDSpanTypes.HTTP_SERVER + childOf(span(0)) + errored false + tags { + "$Tags.COMPONENT.key" "ratpack" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "/" + defaultTags() + } + } + } + } + } + + def "test path with bindings call"() { + setup: + def app = GroovyEmbeddedApp.ratpack { + handlers { + prefix(":foo/:bar?") { + get("baz") { ctx -> + context.render(ctx.get(PathBinding).description) + } + } + } + } + def request = new Request.Builder() + .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" + + assertTraces(1) { + trace(0, 2) { + span(0) { + resourceName "GET /:foo/:bar?/baz" + serviceName "unnamed-java-app" + operationName "netty.request" + spanType DDSpanTypes.HTTP_SERVER + parent() + errored false + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "${app.address}a/b/baz" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + defaultTags() + } + } + span(1) { + resourceName "GET /:foo/:bar?/baz" + serviceName "unnamed-java-app" + operationName "ratpack.handler" + spanType DDSpanTypes.HTTP_SERVER + childOf(span(0)) + errored false + tags { + "$Tags.COMPONENT.key" "ratpack" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "/a/b/baz" + defaultTags() + } + } + } + } + } + + def "test handler error response"() { + setup: + def app = GroovyEmbeddedApp.ratpack { + handlers { + get { + 0 / 0 + } + } + } + def request = new Request.Builder() + .url(app.address.toURL()) + .get() + .build() + when: + def resp = client.newCall(request).execute() + then: + resp.code() == 500 + + assertTraces(1) { + trace(0, 2) { + span(0) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "netty.request" + spanType DDSpanTypes.HTTP_SERVER + parent() + errored true + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 500 + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + errorTags(ArithmeticException, "Division undefined") + defaultTags() + } + } + span(1) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "ratpack.handler" + spanType DDSpanTypes.HTTP_SERVER + childOf(span(0)) + errored true + tags { + "$Tags.COMPONENT.key" "ratpack" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 500 + "$Tags.HTTP_URL.key" "/" + errorTags(HandlerException, "java.lang.ArithmeticException: Division undefined") + defaultTags() + } + } + } + } + } + + def "test promise error response"() { + setup: + def app = GroovyEmbeddedApp.ratpack { + handlers { + get { + Promise.async { + 0 / 0 + }.then { + context.render(it) + } + } + } + } + def request = new Request.Builder() + .url(app.address.toURL()) + .get() + .build() + when: + def resp = client.newCall(request).execute() + then: + resp.code() == 500 + + assertTraces(1) { + trace(0, 2) { + span(0) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "netty.request" + spanType DDSpanTypes.HTTP_SERVER + parent() + errored true + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 500 + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + errorTags(ArithmeticException, "Division undefined") + defaultTags() + } + } + span(1) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "ratpack.handler" + spanType DDSpanTypes.HTTP_SERVER + childOf(span(0)) + errored true + tags { + "$Tags.COMPONENT.key" "ratpack" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 500 + "$Tags.HTTP_URL.key" "/" + "$Tags.ERROR.key" true + defaultTags() + } + } + } + } + } + + def "test render error response"() { + setup: + def app = GroovyEmbeddedApp.ratpack { + handlers { + get { + context.render(Promise.sync { + return "fail " + 0 / 0 + }) + } + } + } + def request = new Request.Builder() + .url(app.address.toURL()) + .get() + .build() + when: + def resp = client.newCall(request).execute() + then: + resp.code() == 500 + + assertTraces(1) { + trace(0, 2) { + span(0) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "netty.request" + spanType DDSpanTypes.HTTP_SERVER + parent() + errored true + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 500 + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + errorTags(ArithmeticException, "Division undefined") + defaultTags() + } + } + span(1) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "ratpack.handler" + spanType DDSpanTypes.HTTP_SERVER + childOf(span(0)) + errored true + tags { + "$Tags.COMPONENT.key" "ratpack" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 500 + "$Tags.HTTP_URL.key" "/" + "$Tags.ERROR.key" true + defaultTags() + } + } + } + } + } + + def "test path call using ratpack http client"() { + setup: + + def external = GroovyEmbeddedApp.ratpack { + handlers { + get("nested") { + context.render("succ") + } + get("nested2") { + context.render("ess") + } + } + } + + def app = GroovyEmbeddedApp.ratpack { + handlers { + get { HttpClient httpClient -> + // 1st internal http client call to nested + httpClient.get(HttpUrlBuilder.base(external.address).path("nested").build()) + .map { it.body.text } + .flatMap { t -> + // make a 2nd http request and concatenate the two bodies together + httpClient.get(HttpUrlBuilder.base(external.address).path("nested2").build()) map { t + it.body.text } + } + .then { + context.render(it) + } + } + } + } + def request = new Request.Builder() + .url(app.address.toURL()) + .get() + .build() + + when: + def resp = client.newCall(request).execute() + + then: + resp.code() == 200 + resp.body().string() == "success" + + // 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) + assertTraces(3) { + // simulated external system, first call + trace(0, 2) { + span(0) { + resourceName "GET /nested" + serviceName "unnamed-java-app" + operationName "netty.request" + spanType DDSpanTypes.HTTP_SERVER + childOf(trace(2).get(3)) + errored false + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "${external.address}nested" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + defaultTags(true) + } + } + span(1) { + resourceName "GET /nested" + serviceName "unnamed-java-app" + operationName "ratpack.handler" + spanType DDSpanTypes.HTTP_SERVER + childOf(span(0)) + errored false + tags { + "$Tags.COMPONENT.key" "ratpack" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "/nested" + defaultTags() + } + } + } +// // simulated external system, second call + trace(1, 2) { + span(0) { + resourceName "GET /nested2" + serviceName "unnamed-java-app" + operationName "netty.request" + spanType DDSpanTypes.HTTP_SERVER + childOf(trace(2).get(2)) + errored false + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "${external.address}nested2" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + defaultTags(true) + } + } + span(1) { + resourceName "GET /nested2" + serviceName "unnamed-java-app" + operationName "ratpack.handler" + spanType DDSpanTypes.HTTP_SERVER + childOf(span(0)) + errored false + tags { + "$Tags.COMPONENT.key" "ratpack" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "/nested2" + defaultTags() + } + } + } + trace(2, 4) { + // main app span that processed the request from OKHTTP request + span(0) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "netty.request" + spanType DDSpanTypes.HTTP_SERVER + parent() + errored false + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + defaultTags() + } + } + span(1) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "ratpack.handler" + spanType DDSpanTypes.HTTP_SERVER + childOf(span(0)) + errored false + tags { + "$Tags.COMPONENT.key" "ratpack" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_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(2) { + resourceName "GET /?" + serviceName "unnamed-java-app" + operationName "netty.client.request" + spanType DDSpanTypes.HTTP_CLIENT + childOf(span(3)) + errored false + tags { + "$Tags.COMPONENT.key" "netty-client" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "${external.address}nested2" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + defaultTags() + } + } + // First http client call that receives the 'Succ' of Success + span(3) { + resourceName "GET /nested" + serviceName "unnamed-java-app" + operationName "netty.client.request" + spanType DDSpanTypes.HTTP_CLIENT + childOf(span(0)) + errored false + tags { + "$Tags.COMPONENT.key" "netty-client" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "${external.address}nested" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + defaultTags() + } + } + } + } + } + + def "test ratpack http client error handling"() { + setup: + def badAddress = new URI("http://localhost:$UNUSABLE_PORT") + + def app = GroovyEmbeddedApp.ratpack { + handlers { + get { HttpClient httpClient -> + httpClient.get(badAddress) + .map { it.body.text } + .then { + context.render(it) + } + } + } + } + def request = new Request.Builder() + .url(app.address.toURL()) + .get() + .build() + + when: + def resp = client.newCall(request).execute() + + then: + resp.code() == 500 + + assertTraces(1) { + trace(0, 3) { + span(0) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "netty.request" + spanType DDSpanTypes.HTTP_SERVER + parent() + errored true + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 500 + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + "$Tags.ERROR.key" true + defaultTags() + } + } + span(1) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "ratpack.handler" + spanType DDSpanTypes.HTTP_SERVER + childOf(span(0)) + errored true + tags { + "$Tags.COMPONENT.key" "ratpack" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 500 + "$Tags.HTTP_URL.key" "/" + "$Tags.ERROR.key" true + defaultTags() + } + } + span(2) { + operationName "netty.connect" + resourceName "netty.connect" + // Notice the span is a child of the netty span. + // I don't think we can easily get it rooted under the ratpack span + // since the "parent" is stored on the channel. + childOf(span(0)) + errored true + tags { + "$Tags.COMPONENT.key" "netty" + errorTags(AbstractChannel.AnnotatedConnectException, String) + defaultTags() + } + } + } + } + } + + def "test forked path call and start span in handler (#startSpanInHandler)"() { + setup: + def app = GroovyEmbeddedApp.ratpack { + handlers { + get { + TraceScope scope + if (startSpanInHandler) { + Span childSpan = GlobalTracer.get() + .buildSpan("ratpack.exec-test") + .withTag(DDTags.RESOURCE_NAME, "INSIDE-TEST") + .start() + scope = GlobalTracer.get().scopeManager().activate(childSpan, true) + } + def latch = new CountDownLatch(1) + try { + scope?.setAsyncPropagation(true) + GlobalTracer.get().activeSpan().setBaggageItem("test-baggage", "foo") + + context.render(testPromise(latch).fork()) + } finally { + scope?.close() + latch.countDown() + } + } + } + } + def request = new Request.Builder() + .url(app.address.toURL()) + .get() + .build() + + when: + def resp = client.newCall(request).execute() + + then: + resp.code() == 200 + resp.body().string() == "foo" + + assertTraces(1) { + trace(0, (startSpanInHandler ? 3 : 2)) { + if (startSpanInHandler) { + span(0) { + resourceName "INSIDE-TEST" + serviceName "unnamed-java-app" + operationName "ratpack.exec-test" + childOf(span(2)) + errored false + tags { + defaultTags() + } + } + } + span(startSpanInHandler ? 1 : 0) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "netty.request" + spanType DDSpanTypes.HTTP_SERVER + parent() + errored false + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + defaultTags() + } + } + span(startSpanInHandler ? 2 : 1) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "ratpack.handler" + spanType DDSpanTypes.HTTP_SERVER + childOf(span(startSpanInHandler ? 1 : 0)) + errored false + tags { + "$Tags.COMPONENT.key" "ratpack" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "/" + defaultTags() + } + } + } + } + + where: + startSpanInHandler << [true, false] + } + + def "forked executions inherit parent scope"() { + when: + def result = ExecHarness.yieldSingle({}, { + 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() + .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 testPromise(CountDownLatch latch = null) { + Promise.sync { + latch?.await() + Scope tracerScope = GlobalTracer.get().scopeManager().active() + return tracerScope?.span()?.getBaggageItem("test-baggage") + } + } +} diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackHttpClientInstrumentation.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackHttpClientInstrumentation.java deleted file mode 100644 index 485d3525be..0000000000 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackHttpClientInstrumentation.java +++ /dev/null @@ -1,79 +0,0 @@ -package datadog.trace.instrumentation.ratpack; - -import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; -import static net.bytebuddy.matcher.ElementMatchers.isInterface; -import static net.bytebuddy.matcher.ElementMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.not; -import static net.bytebuddy.matcher.ElementMatchers.takesArguments; - -import com.google.auto.service.AutoService; -import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.instrumentation.ratpack.impl.RatpackHttpClientAdvice; -import java.net.URI; -import java.util.HashMap; -import java.util.Map; -import net.bytebuddy.description.method.MethodDescription; -import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.matcher.ElementMatcher; - -@AutoService(Instrumenter.class) -public final class RatpackHttpClientInstrumentation extends Instrumenter.Default { - - public static final TypeDescription.ForLoadedType URI_TYPE_DESCRIPTION = - new TypeDescription.ForLoadedType(URI.class); - - public RatpackHttpClientInstrumentation() { - super(RatpackInstrumentation.EXEC_NAME); - } - - @Override - protected boolean defaultEnabled() { - // FIXME: Injecting ContextualScopeManager is probably a bug. Verify and check all ratpack - // helpers before enabling. - return false; - } - - @Override - public ElementMatcher typeMatcher() { - return not(isInterface()).and(safeHasSuperType(named("ratpack.http.client.HttpClient"))); - } - - @Override - public String[] helperClassNames() { - return new String[] { - // http helpers - "datadog.trace.instrumentation.ratpack.impl.RatpackHttpClientAdvice$RatpackHttpClientRequestAdvice", - "datadog.trace.instrumentation.ratpack.impl.RatpackHttpClientAdvice$RatpackHttpClientRequestStreamAdvice", - "datadog.trace.instrumentation.ratpack.impl.RatpackHttpClientAdvice$RatpackHttpGetAdvice", - "datadog.trace.instrumentation.ratpack.impl.RatpackHttpClientAdvice$RequestAction", - "datadog.trace.instrumentation.ratpack.impl.RatpackHttpClientAdvice$ResponseAction", - "datadog.trace.instrumentation.ratpack.impl.RatpackHttpClientAdvice$StreamedResponseAction", - "datadog.trace.instrumentation.ratpack.impl.RequestSpecInjectAdapter", - "datadog.trace.instrumentation.ratpack.impl.WrappedRequestSpec", - }; - } - - @Override - public Map, String> transformers() { - final Map, String> transformers = new HashMap<>(); - transformers.put( - named("request") - .and( - takesArguments( - URI_TYPE_DESCRIPTION, RatpackInstrumentation.ACTION_TYPE_DESCRIPTION)), - RatpackHttpClientAdvice.RatpackHttpClientRequestAdvice.class.getName()); - transformers.put( - named("requestStream") - .and( - takesArguments( - URI_TYPE_DESCRIPTION, RatpackInstrumentation.ACTION_TYPE_DESCRIPTION)), - RatpackHttpClientAdvice.RatpackHttpClientRequestStreamAdvice.class.getName()); - transformers.put( - named("get") - .and( - takesArguments( - URI_TYPE_DESCRIPTION, RatpackInstrumentation.ACTION_TYPE_DESCRIPTION)), - RatpackHttpClientAdvice.RatpackHttpGetAdvice.class.getName()); - return transformers; - } -} diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackInstrumentation.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackInstrumentation.java deleted file mode 100644 index d9bab6d0c6..0000000000 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/RatpackInstrumentation.java +++ /dev/null @@ -1,132 +0,0 @@ -package datadog.trace.instrumentation.ratpack; - -import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; -import static java.util.Collections.singletonMap; -import static net.bytebuddy.matcher.ElementMatchers.isInterface; -import static net.bytebuddy.matcher.ElementMatchers.isMethod; -import static net.bytebuddy.matcher.ElementMatchers.isStatic; -import static net.bytebuddy.matcher.ElementMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.not; -import static net.bytebuddy.matcher.ElementMatchers.returns; -import static net.bytebuddy.matcher.ElementMatchers.takesArguments; - -import com.google.auto.service.AutoService; -import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.instrumentation.ratpack.impl.RatpackServerAdvice; -import java.lang.reflect.Modifier; -import java.util.Map; -import lombok.extern.slf4j.Slf4j; -import net.bytebuddy.description.method.MethodDescription; -import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.matcher.ElementMatcher; - -@AutoService(Instrumenter.class) -@Slf4j -public final class RatpackInstrumentation extends Instrumenter.Default { - - static final String EXEC_NAME = "ratpack"; - - static final TypeDescription.Latent ACTION_TYPE_DESCRIPTION = - new TypeDescription.Latent("ratpack.func.Action", Modifier.PUBLIC, null); - - public RatpackInstrumentation() { - super(EXEC_NAME); - } - - @Override - protected boolean defaultEnabled() { - return false; - } - - @Override - public ElementMatcher typeMatcher() { - return named("ratpack.server.internal.ServerRegistry"); - } - - @Override - public String[] helperClassNames() { - return new String[] { - // service registry helpers - "datadog.trace.instrumentation.ratpack.impl.RatpackRequestExtractAdapter", - "datadog.trace.instrumentation.ratpack.impl.RatpackServerAdvice", - "datadog.trace.instrumentation.ratpack.impl.RatpackServerAdvice$RatpackServerRegistryAdvice", - "datadog.trace.instrumentation.ratpack.impl.TracingHandler" - }; - } - - @Override - public Map, String> transformers() { - return singletonMap( - isMethod().and(isStatic()).and(named("buildBaseRegistry")), - RatpackServerAdvice.RatpackServerRegistryAdvice.class.getName()); - } - - @AutoService(Instrumenter.class) - public static class ExecStarterInstrumentation extends Instrumenter.Default { - - public ExecStarterInstrumentation() { - super(EXEC_NAME); - } - - @Override - protected boolean defaultEnabled() { - return false; - } - - @Override - public ElementMatcher typeMatcher() { - return not(isInterface()).and(safeHasSuperType(named("ratpack.exec.ExecStarter"))); - } - - @Override - public String[] helperClassNames() { - return new String[] { - // exec helpers - "datadog.trace.instrumentation.ratpack.impl.RatpackServerAdvice$ExecStarterAdvice", - "datadog.trace.instrumentation.ratpack.impl.RatpackServerAdvice$ExecStarterAction" - }; - } - - @Override - public Map, String> transformers() { - return singletonMap( - named("register").and(takesArguments(ACTION_TYPE_DESCRIPTION)), - RatpackServerAdvice.ExecStarterAdvice.class.getName()); - } - } - - @AutoService(Instrumenter.class) - public static class ExecutionInstrumentation extends Default { - - public ExecutionInstrumentation() { - super(EXEC_NAME); - } - - @Override - protected boolean defaultEnabled() { - return false; - } - - @Override - public ElementMatcher typeMatcher() { - return named("ratpack.exec.Execution") - .or(not(isInterface()).and(safeHasSuperType(named("ratpack.exec.Execution")))); - } - - @Override - public String[] helperClassNames() { - return new String[] { - // exec helpers - "datadog.trace.instrumentation.ratpack.impl.RatpackServerAdvice$ExecStarterAdvice", - "datadog.trace.instrumentation.ratpack.impl.RatpackServerAdvice$ExecStarterAction" - }; - } - - @Override - public Map, String> transformers() { - return singletonMap( - named("fork").and(returns(named("ratpack.exec.ExecStarter"))), - RatpackServerAdvice.ExecutionAdvice.class.getName()); - } - } -} diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ServerErrorHandlerInstrumentation.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ServerErrorHandlerInstrumentation.java new file mode 100644 index 0000000000..62e6f5dc74 --- /dev/null +++ b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ServerErrorHandlerInstrumentation.java @@ -0,0 +1,59 @@ +package datadog.trace.instrumentation.ratpack; + +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; +import static datadog.trace.instrumentation.ratpack.RatpackServerDecorator.DECORATE; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import io.opentracing.Span; +import io.opentracing.util.GlobalTracer; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public class ServerErrorHandlerInstrumentation extends Instrumenter.Default { + + public ServerErrorHandlerInstrumentation() { + super("ratpack"); + } + + @Override + public ElementMatcher typeMatcher() { + return named("ratpack.exec.Execution") + .or(not(isInterface()).and(safeHasSuperType(named("ratpack.error.ServerErrorHandler")))); + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ServerDecorator", + "datadog.trace.agent.decorator.HttpServerDecorator", + packageName + ".RatpackServerDecorator", + }; + } + + @Override + public Map, String> transformers() { + return singletonMap( + named("error").and(takesArgument(1, Throwable.class)), ErrorHandlerAdvice.class.getName()); + } + + public static class ErrorHandlerAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void captureThrowable(@Advice.Argument(1) final Throwable throwable) { + final Span span = GlobalTracer.get().activeSpan(); + if (span != null) { + DECORATE.onError(span, throwable); + } + } + } +} diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ServerRegistryInstrumentation.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ServerRegistryInstrumentation.java new file mode 100644 index 0000000000..01024f65c4 --- /dev/null +++ b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ServerRegistryInstrumentation.java @@ -0,0 +1,44 @@ +package datadog.trace.instrumentation.ratpack; + +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.isStatic; +import static net.bytebuddy.matcher.ElementMatchers.named; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import java.util.Map; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public class ServerRegistryInstrumentation extends Instrumenter.Default { + + public ServerRegistryInstrumentation() { + super("ratpack"); + } + + @Override + public ElementMatcher typeMatcher() { + return named("ratpack.server.internal.ServerRegistry"); + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ServerDecorator", + "datadog.trace.agent.decorator.HttpServerDecorator", + packageName + ".RatpackServerDecorator", + packageName + ".TracingHandler", + }; + } + + @Override + public Map, String> transformers() { + return singletonMap( + isMethod().and(isStatic()).and(named("buildBaseRegistry")), + packageName + ".RatpackServerRegistryAdvice"); + } +} diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerDecorator.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerDecorator.java new file mode 100644 index 0000000000..52ea594c83 --- /dev/null +++ b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerDecorator.java @@ -0,0 +1,69 @@ +package datadog.trace.instrumentation.ratpack; + +import datadog.trace.agent.decorator.HttpServerDecorator; +import datadog.trace.api.DDTags; +import io.opentracing.Span; +import ratpack.handling.Context; +import ratpack.http.Request; +import ratpack.http.Response; +import ratpack.http.Status; + +public class RatpackServerDecorator extends HttpServerDecorator { + public static final RatpackServerDecorator DECORATE = new RatpackServerDecorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"ratpack"}; + } + + @Override + protected String component() { + return "ratpack"; + } + + @Override + protected String method(final Request request) { + return request.getMethod().getName(); + } + + @Override + protected String url(final Request request) { + return request.getUri(); + } + + @Override + protected String hostname(final Request request) { + return null; + } + + @Override + protected Integer port(final Request request) { + return null; + } + + @Override + protected Integer status(final Response response) { + final Status status = response.getStatus(); + if (status != null) { + return status.getCode(); + } else { + return null; + } + } + + public Span onContext(final Span span, final Context ctx) { + + String description = ctx.getPathBinding().getDescription(); + if (description == null || description.isEmpty()) { + description = ctx.getRequest().getUri(); + } + if (!description.startsWith("/")) { + description = "/" + description; + } + + final String resourceName = ctx.getRequest().getMethod().getName() + " " + description; + span.setTag(DDTags.RESOURCE_NAME, resourceName); + + return span; + } +} diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerRegistryAdvice.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerRegistryAdvice.java new file mode 100644 index 0000000000..1806a12e8b --- /dev/null +++ b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerRegistryAdvice.java @@ -0,0 +1,14 @@ +package datadog.trace.instrumentation.ratpack; + +import net.bytebuddy.asm.Advice; +import ratpack.handling.HandlerDecorator; +import ratpack.registry.Registry; + +public class RatpackServerRegistryAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + public static void injectTracing(@Advice.Return(readOnly = false) Registry registry) { + registry = + registry.join( + Registry.builder().add(HandlerDecorator.prepend(new TracingHandler())).build()); + } +} diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/TracingHandler.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/TracingHandler.java new file mode 100644 index 0000000000..fe69a0d7f0 --- /dev/null +++ b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/TracingHandler.java @@ -0,0 +1,71 @@ +package datadog.trace.instrumentation.ratpack; + +import static datadog.trace.instrumentation.ratpack.RatpackServerDecorator.DECORATE; + +import datadog.trace.context.TraceScope; +import io.netty.util.Attribute; +import io.netty.util.AttributeKey; +import io.opentracing.Scope; +import io.opentracing.Span; +import io.opentracing.Tracer; +import io.opentracing.util.GlobalTracer; +import ratpack.handling.Context; +import ratpack.handling.Handler; +import ratpack.http.Request; + +/** + * This Ratpack handler reads tracing headers from the incoming request, starts a span and ensures + * that the span is closed when the response is sent + */ +public final class TracingHandler implements Handler { + /** + * This constant is copied over from datadog.trace.instrumentation.netty41.AttributeKeys. The key + * string must be kept consistent. + */ + public static final AttributeKey SERVER_ATTRIBUTE_KEY = + AttributeKey.valueOf( + "datadog.trace.instrumentation.netty41.server.HttpServerTracingHandler.span"); + + @Override + public void handle(final Context ctx) { + final Tracer tracer = GlobalTracer.get(); + final Request request = ctx.getRequest(); + + final Attribute spanAttribute = + ctx.getDirectChannelAccess().getChannel().attr(SERVER_ATTRIBUTE_KEY); + final Span nettySpan = spanAttribute.get(); + + // Relying on executor instrumentation to assume the netty span is in context as the parent. + final Span ratpackSpan = tracer.buildSpan("ratpack.handler").start(); + DECORATE.afterStart(ratpackSpan); + DECORATE.onRequest(ratpackSpan, request); + + try (final Scope scope = tracer.scopeManager().activate(ratpackSpan, false)) { + if (scope instanceof TraceScope) { + ((TraceScope) scope).setAsyncPropagation(true); + } + + ctx.getResponse() + .beforeSend( + response -> { + try (final Scope ignored = tracer.scopeManager().activate(ratpackSpan, false)) { + if (nettySpan != null) { + // Rename the netty span resource name with the ratpack route. + DECORATE.onContext(nettySpan, ctx); + } + DECORATE.onResponse(ratpackSpan, response); + DECORATE.onContext(ratpackSpan, ctx); + DECORATE.beforeFinish(ratpackSpan); + ratpackSpan.finish(); + } + }); + + ctx.next(); + } catch (final Throwable e) { + DECORATE.onError(ratpackSpan, e); + DECORATE.beforeFinish(ratpackSpan); + ratpackSpan.finish(); // Do we need to finish? + throw e; + } + } +} diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/RatpackHttpClientAdvice.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/RatpackHttpClientAdvice.java deleted file mode 100644 index a8a8570b88..0000000000 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/RatpackHttpClientAdvice.java +++ /dev/null @@ -1,144 +0,0 @@ -package datadog.trace.instrumentation.ratpack.impl; - -import static io.opentracing.log.Fields.ERROR_OBJECT; - -import io.opentracing.Span; -import io.opentracing.tag.Tags; -import io.opentracing.util.GlobalTracer; -import java.util.Collections; -import java.util.concurrent.atomic.AtomicReference; -import net.bytebuddy.asm.Advice; -import ratpack.exec.Promise; -import ratpack.exec.Result; -import ratpack.func.Action; -import ratpack.http.client.ReceivedResponse; -import ratpack.http.client.RequestSpec; -import ratpack.http.client.StreamedResponse; - -public class RatpackHttpClientAdvice { - public static class RequestAction implements Action { - - private final Action requestAction; - private final AtomicReference spanRef; - - public RequestAction(Action requestAction, AtomicReference spanRef) { - this.requestAction = requestAction; - this.spanRef = spanRef; - } - - @Override - public void execute(RequestSpec requestSpec) throws Exception { - WrappedRequestSpec wrappedRequestSpec; - if (requestSpec instanceof WrappedRequestSpec) { - wrappedRequestSpec = (WrappedRequestSpec) requestSpec; - } else { - wrappedRequestSpec = - new WrappedRequestSpec( - requestSpec, - GlobalTracer.get(), - GlobalTracer.get().scopeManager().active(), - spanRef); - } - requestAction.execute(wrappedRequestSpec); - } - } - - public static class ResponseAction implements Action> { - private final AtomicReference spanRef; - - public ResponseAction(AtomicReference spanRef) { - this.spanRef = spanRef; - } - - @Override - public void execute(Result result) { - Span span = spanRef.get(); - if (span == null) { - return; - } - span.finish(); - if (result.isError()) { - Tags.ERROR.set(span, true); - span.log(Collections.singletonMap(ERROR_OBJECT, result.getThrowable())); - } else { - Tags.HTTP_STATUS.set(span, result.getValue().getStatusCode()); - } - } - } - - public static class StreamedResponseAction implements Action> { - private final Span span; - - public StreamedResponseAction(Span span) { - this.span = span; - } - - @Override - public void execute(Result result) { - span.finish(); - if (result.isError()) { - Tags.ERROR.set(span, true); - span.log(Collections.singletonMap(ERROR_OBJECT, result.getThrowable())); - } else { - Tags.HTTP_STATUS.set(span, result.getValue().getStatusCode()); - } - } - } - - public static class RatpackHttpClientRequestAdvice { - @Advice.OnMethodEnter - public static AtomicReference injectTracing( - @Advice.Argument(value = 1, readOnly = false) Action requestAction) { - AtomicReference span = new AtomicReference<>(); - - //noinspection UnusedAssignment - requestAction = new RequestAction(requestAction, span); - - return span; - } - - @Advice.OnMethodExit - public static void finishTracing( - @Advice.Return(readOnly = false) Promise promise, - @Advice.Enter AtomicReference ref) { - - //noinspection UnusedAssignment - promise = promise.wiretap(new ResponseAction(ref)); - } - } - - public static class RatpackHttpClientRequestStreamAdvice { - @Advice.OnMethodEnter - public static AtomicReference injectTracing( - @Advice.Argument(value = 1, readOnly = false) Action requestAction) { - AtomicReference span = new AtomicReference<>(); - - //noinspection UnusedAssignment - requestAction = new RequestAction(requestAction, span); - - return span; - } - - @Advice.OnMethodExit - public static void finishTracing( - @Advice.Return(readOnly = false) Promise promise, - @Advice.Enter AtomicReference ref) { - Span span = ref.get(); - if (span == null) { - return; - } - - //noinspection UnusedAssignment - promise = promise.wiretap(new StreamedResponseAction(span)); - } - } - - public static class RatpackHttpGetAdvice { - @Advice.OnMethodEnter - public static void ensureGetMethodSet( - @Advice.Argument(value = 1, readOnly = false) Action requestAction) { - //noinspection UnusedAssignment - requestAction = requestAction.prepend(RequestSpec::get); - } - } -} diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/RatpackRequestExtractAdapter.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/RatpackRequestExtractAdapter.java deleted file mode 100644 index f99bc4b040..0000000000 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/RatpackRequestExtractAdapter.java +++ /dev/null @@ -1,29 +0,0 @@ -package datadog.trace.instrumentation.ratpack.impl; - -import io.opentracing.propagation.TextMap; -import java.util.Iterator; -import java.util.Map; -import ratpack.http.Request; -import ratpack.util.MultiValueMap; - -/** - * Simple request extractor in the same vein as @see - * io.opentracing.contrib.web.servlet.filter.HttpServletRequestExtractAdapter - */ -public class RatpackRequestExtractAdapter implements TextMap { - private final MultiValueMap headers; - - RatpackRequestExtractAdapter(final Request request) { - headers = request.getHeaders().asMultiValueMap(); - } - - @Override - public Iterator> iterator() { - return headers.entrySet().iterator(); - } - - @Override - public void put(final String key, final String value) { - throw new UnsupportedOperationException("This class should be used only with Tracer.inject()!"); - } -} diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/RatpackServerAdvice.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/RatpackServerAdvice.java deleted file mode 100644 index 6e30632d02..0000000000 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/RatpackServerAdvice.java +++ /dev/null @@ -1,59 +0,0 @@ -package datadog.trace.instrumentation.ratpack.impl; - -import io.opentracing.Scope; -import io.opentracing.util.GlobalTracer; -import lombok.extern.slf4j.Slf4j; -import net.bytebuddy.asm.Advice; -import ratpack.exec.ExecStarter; -import ratpack.func.Action; -import ratpack.handling.HandlerDecorator; -import ratpack.registry.Registry; -import ratpack.registry.RegistrySpec; - -@Slf4j -public class RatpackServerAdvice { - public static class RatpackServerRegistryAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - public static void injectTracing(@Advice.Return(readOnly = false) Registry registry) { - registry = - registry.join( - Registry.builder().add(HandlerDecorator.prepend(new TracingHandler())).build()); - } - } - - public static class ExecStarterAdvice { - @Advice.OnMethodEnter - public static void addScopeToRegistry( - @Advice.Argument(value = 0, readOnly = false) Action action) { - final Scope active = GlobalTracer.get().scopeManager().active(); - if (active != null) { - action = new ExecStarterAction(active).append(action); - } - } - } - - public static class ExecutionAdvice { - @Advice.OnMethodExit - public static void addScopeToRegistry(@Advice.Return final ExecStarter starter) { - final Scope active = GlobalTracer.get().scopeManager().active(); - if (active != null) { - starter.register(new ExecStarterAction(active)); - } - } - } - - public static class ExecStarterAction implements Action { - private final Scope active; - - public ExecStarterAction(final Scope active) { - this.active = active; - } - - @Override - public void execute(final RegistrySpec spec) { - if (active != null) { - spec.add(active); - } - } - } -} diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/RequestSpecInjectAdapter.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/RequestSpecInjectAdapter.java deleted file mode 100644 index 10383b4c9d..0000000000 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/RequestSpecInjectAdapter.java +++ /dev/null @@ -1,29 +0,0 @@ -package datadog.trace.instrumentation.ratpack.impl; - -import io.opentracing.propagation.TextMap; -import java.util.Iterator; -import java.util.Map; -import ratpack.http.client.RequestSpec; - -/** - * SimpleTextMap to add headers to an outgoing Ratpack HttpClient request - * - * @see datadog.trace.instrumentation.apachehttpclient.DDTracingClientExec.HttpHeadersInjectAdapter - */ -public class RequestSpecInjectAdapter implements TextMap { - private final RequestSpec requestSpec; - - public RequestSpecInjectAdapter(RequestSpec requestSpec) { - this.requestSpec = requestSpec; - } - - @Override - public Iterator> iterator() { - throw new UnsupportedOperationException("Should be used only with tracer#inject()"); - } - - @Override - public void put(String key, String value) { - requestSpec.getHeaders().add(key, value); - } -} diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/TracingHandler.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/TracingHandler.java deleted file mode 100644 index 0cabc4d3fc..0000000000 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/TracingHandler.java +++ /dev/null @@ -1,89 +0,0 @@ -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; -import io.opentracing.propagation.Format; -import io.opentracing.tag.Tags; -import io.opentracing.util.GlobalTracer; -import ratpack.handling.Context; -import ratpack.handling.Handler; -import ratpack.http.Request; -import ratpack.http.Status; - -/** - * This Ratpack handler reads tracing headers from the incoming request, starts a scope and ensures - * that the scope is closed when the response is sent - */ -public final class TracingHandler implements Handler { - @Override - public void handle(final Context ctx) { - final Request request = ctx.getRequest(); - - final SpanContext extractedContext = - GlobalTracer.get() - .extract(Format.Builtin.HTTP_HEADERS, new RatpackRequestExtractAdapter(request)); - - final Scope scope = - GlobalTracer.get() - .buildSpan("ratpack.handler") - .asChildOf(extractedContext) - .withTag(Tags.COMPONENT.getKey(), "ratpack") - .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER) - .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_SERVER) - .withTag(Tags.HTTP_METHOD.getKey(), request.getMethod().getName()) - .withTag(Tags.HTTP_URL.getKey(), request.getUri()) - .startActive(false); - - if (scope instanceof TraceScope) { - ((TraceScope) scope).setAsyncPropagation(true); - } - - final Span rootSpan = scope.span(); - - ctx.getResponse() - .beforeSend( - response -> { - final Scope responseScope = GlobalTracer.get().scopeManager().active(); - - 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(rootSpan, true); - } - Tags.HTTP_STATUS.set(rootSpan, status.getCode()); - } - - rootSpan.finish(); - }); - - ctx.onClose( - requestOutcome -> { - final Scope activeScope = GlobalTracer.get().scopeManager().active(); - if (activeScope != null) { - activeScope.close(); - } - }); - - ctx.next(); - } - - private static String getResourceName(final Context ctx) { - String description = ctx.getPathBinding().getDescription(); - if (description == null || description.isEmpty()) { - description = ctx.getRequest().getUri(); - } - if (!description.startsWith("/")) { - description = "/" + description; - } - return ctx.getRequest().getMethod().getName() + " " + description; - } -} diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/WrappedRequestSpec.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/WrappedRequestSpec.java deleted file mode 100644 index dc1e690876..0000000000 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/impl/WrappedRequestSpec.java +++ /dev/null @@ -1,145 +0,0 @@ -package datadog.trace.instrumentation.ratpack.impl; - -import datadog.trace.api.DDSpanTypes; -import datadog.trace.api.DDTags; -import io.opentracing.Scope; -import io.opentracing.Span; -import io.opentracing.Tracer; -import io.opentracing.propagation.Format; -import io.opentracing.tag.Tags; -import java.net.URI; -import java.time.Duration; -import java.util.concurrent.atomic.AtomicReference; -import javax.net.ssl.SSLContext; -import ratpack.func.Action; -import ratpack.func.Function; -import ratpack.http.HttpMethod; -import ratpack.http.MutableHeaders; -import ratpack.http.client.ReceivedResponse; -import ratpack.http.client.RequestSpec; - -/** - * RequestSpec wrapper that captures the method type, sets up redirect handling and starts new spans - * when a method type is set. - */ -public final class WrappedRequestSpec implements RequestSpec { - - private final RequestSpec delegate; - private final Tracer tracer; - private final Scope scope; - private final AtomicReference spanRef; - - WrappedRequestSpec( - final RequestSpec spec, - final Tracer tracer, - final Scope scope, - final AtomicReference spanRef) { - delegate = spec; - this.tracer = tracer; - this.scope = scope; - this.spanRef = spanRef; - delegate.onRedirect(this::redirectHandler); - } - - /* - * Default redirect handler that ensures the span is marked as received before - * a new span is created. - * - */ - private Action redirectHandler(final ReceivedResponse response) { - // handler.handleReceive(response.getStatusCode(), null, span.get()); - return (s) -> new WrappedRequestSpec(s, tracer, scope, spanRef); - } - - @Override - public RequestSpec redirects(final int maxRedirects) { - delegate.redirects(maxRedirects); - return this; - } - - @Override - public RequestSpec onRedirect( - final Function> function) { - - final Function> wrapped = - (ReceivedResponse response) -> redirectHandler(response).append(function.apply(response)); - - delegate.onRedirect(wrapped); - return this; - } - - @Override - public RequestSpec sslContext(final SSLContext sslContext) { - delegate.sslContext(sslContext); - return this; - } - - @Override - public MutableHeaders getHeaders() { - return delegate.getHeaders(); - } - - @Override - public RequestSpec maxContentLength(final int numBytes) { - delegate.maxContentLength(numBytes); - return this; - } - - @Override - public RequestSpec headers(final Action action) throws Exception { - delegate.headers(action); - return this; - } - - @Override - public RequestSpec method(final HttpMethod method) { - final Span span = - tracer - .buildSpan("ratpack.client-request") - .asChildOf(scope != null ? scope.span() : null) - .withTag(Tags.COMPONENT.getKey(), "ratpack-httpclient") - .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT) - .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT) - .withTag(Tags.HTTP_URL.getKey(), getUri().toString()) - .withTag(Tags.HTTP_METHOD.getKey(), method.getName()) - .start(); - spanRef.set(span); - delegate.method(method); - tracer.inject(span.context(), Format.Builtin.HTTP_HEADERS, new RequestSpecInjectAdapter(this)); - return this; - } - - @Override - public RequestSpec decompressResponse(final boolean shouldDecompress) { - delegate.decompressResponse(shouldDecompress); - return this; - } - - @Override - public URI getUri() { - return delegate.getUri(); - } - - @Override - public RequestSpec connectTimeout(final Duration duration) { - delegate.connectTimeout(duration); - return this; - } - - @Override - public RequestSpec readTimeout(final Duration duration) { - delegate.readTimeout(duration); - return this; - } - - @Override - public Body getBody() { - return delegate.getBody(); - } - - @Override - public RequestSpec body(final Action action) throws Exception { - delegate.body(action); - return this; - } -} diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/test/groovy/RatpackTest.groovy b/dd-java-agent/instrumentation/ratpack-1.4/src/test/groovy/RatpackTest.groovy index a9e8c1f45a..54f5d953ff 100644 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/test/groovy/RatpackTest.groovy +++ b/dd-java-agent/instrumentation/ratpack-1.4/src/test/groovy/RatpackTest.groovy @@ -13,17 +13,20 @@ import okhttp3.Request import ratpack.exec.Promise import ratpack.exec.util.ParallelBatch import ratpack.groovy.test.embed.GroovyEmbeddedApp +import ratpack.handling.internal.HandlerException import ratpack.http.HttpUrlBuilder import ratpack.http.client.HttpClient import ratpack.path.PathBinding import ratpack.test.exec.ExecHarness import spock.lang.Retry +import java.util.concurrent.CountDownLatch +import java.util.regex.Pattern + +import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT + @Retry class RatpackTest extends AgentTestRunner { - static { - System.setProperty("dd.integration.ratpack.enabled", "true") - } OkHttpClient client = OkHttpUtils.client() @@ -50,13 +53,32 @@ class RatpackTest extends AgentTestRunner { resp.body.string() == "success" assertTraces(1) { - trace(0, 1) { + trace(0, 2) { span(0) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "netty.request" + spanType DDSpanTypes.HTTP_SERVER + parent() + errored false + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + defaultTags() + } + } + span(1) { resourceName "GET /" serviceName "unnamed-java-app" operationName "ratpack.handler" spanType DDSpanTypes.HTTP_SERVER - parent() + childOf(span(0)) errored false tags { "$Tags.COMPONENT.key" "ratpack" @@ -95,13 +117,32 @@ class RatpackTest extends AgentTestRunner { resp.body.string() == ":foo/:bar?/baz" assertTraces(1) { - trace(0, 1) { + trace(0, 2) { span(0) { + resourceName "GET /:foo/:bar?/baz" + serviceName "unnamed-java-app" + operationName "netty.request" + spanType DDSpanTypes.HTTP_SERVER + parent() + errored false + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "${app.address}a/b/baz" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + defaultTags() + } + } + span(1) { resourceName "GET /:foo/:bar?/baz" serviceName "unnamed-java-app" operationName "ratpack.handler" spanType DDSpanTypes.HTTP_SERVER - parent() + childOf(span(0)) errored false tags { "$Tags.COMPONENT.key" "ratpack" @@ -116,7 +157,133 @@ class RatpackTest extends AgentTestRunner { } } - def "test error response"() { + def "test handler error response"() { + setup: + def app = GroovyEmbeddedApp.ratpack { + handlers { + get { + 0 / 0 + } + } + } + def request = new Request.Builder() + .url(app.address.toURL()) + .get() + .build() + when: + def resp = client.newCall(request).execute() + then: + resp.code() == 500 + + assertTraces(1) { + trace(0, 2) { + span(0) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "netty.request" + spanType DDSpanTypes.HTTP_SERVER + parent() + errored true + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 500 + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + errorTags(ArithmeticException, Pattern.compile("Division( is)? undefined")) + defaultTags() + } + } + span(1) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "ratpack.handler" + spanType DDSpanTypes.HTTP_SERVER + childOf(span(0)) + errored true + tags { + "$Tags.COMPONENT.key" "ratpack" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 500 + "$Tags.HTTP_URL.key" "/" + errorTags(HandlerException, Pattern.compile("java.lang.ArithmeticException: Division( is)? undefined")) + defaultTags() + } + } + } + } + } + + def "test promise error response"() { + setup: + def app = GroovyEmbeddedApp.ratpack { + handlers { + get { + Promise.async { + 0 / 0 + }.then { + context.render(it) + } + } + } + } + def request = new Request.Builder() + .url(app.address.toURL()) + .get() + .build() + when: + def resp = client.newCall(request).execute() + then: + resp.code() == 500 + + assertTraces(1) { + trace(0, 2) { + span(0) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "netty.request" + spanType DDSpanTypes.HTTP_SERVER + parent() + errored true + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 500 + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + errorTags(ArithmeticException, Pattern.compile("Division( is)? undefined")) + defaultTags() + } + } + span(1) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "ratpack.handler" + spanType DDSpanTypes.HTTP_SERVER + childOf(span(0)) + errored true + tags { + "$Tags.COMPONENT.key" "ratpack" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 500 + "$Tags.HTTP_URL.key" "/" + "$Tags.ERROR.key" true + defaultTags() + } + } + } + } + } + + def "test render error response"() { setup: def app = GroovyEmbeddedApp.ratpack { handlers { @@ -137,13 +304,33 @@ class RatpackTest extends AgentTestRunner { resp.code() == 500 assertTraces(1) { - trace(0, 1) { + trace(0, 2) { span(0) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "netty.request" + spanType DDSpanTypes.HTTP_SERVER + parent() + errored true + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 500 + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + errorTags(ArithmeticException, Pattern.compile("Division( is)? undefined")) + defaultTags() + } + } + span(1) { resourceName "GET /" serviceName "unnamed-java-app" operationName "ratpack.handler" spanType DDSpanTypes.HTTP_SERVER - parent() + childOf(span(0)) errored true tags { "$Tags.COMPONENT.key" "ratpack" @@ -151,8 +338,7 @@ class RatpackTest extends AgentTestRunner { "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 500 "$Tags.HTTP_URL.key" "/" - "error" true -// errorTags(Exception, String) // TODO: find out how to get throwable in instrumentation + "$Tags.ERROR.key" true defaultTags() } } @@ -207,13 +393,32 @@ class RatpackTest extends AgentTestRunner { // 1st is nested from the external server (the result of the 1st internal http client call) assertTraces(3) { // simulated external system, first call - trace(0, 1) { + trace(0, 2) { span(0) { + resourceName "GET /nested" + serviceName "unnamed-java-app" + operationName "netty.request" + spanType DDSpanTypes.HTTP_SERVER + childOf(trace(2).get(3)) + errored false + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "${external.address}nested" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + defaultTags(true) + } + } + span(1) { resourceName "GET /nested" serviceName "unnamed-java-app" operationName "ratpack.handler" spanType DDSpanTypes.HTTP_SERVER - childOf(trace(2).get(2)) + childOf(span(0)) errored false tags { "$Tags.COMPONENT.key" "ratpack" @@ -221,18 +426,37 @@ class RatpackTest extends AgentTestRunner { "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 200 "$Tags.HTTP_URL.key" "/nested" - defaultTags(true) + defaultTags() } } } - // simulated external system, second call - trace(1, 1) { +// // simulated external system, second call + trace(1, 2) { span(0) { + resourceName "GET /nested2" + serviceName "unnamed-java-app" + operationName "netty.request" + spanType DDSpanTypes.HTTP_SERVER + childOf(trace(2).get(2)) + errored false + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "${external.address}nested2" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + defaultTags(true) + } + } + span(1) { resourceName "GET /nested2" serviceName "unnamed-java-app" operationName "ratpack.handler" spanType DDSpanTypes.HTTP_SERVER - childOf(trace(2).get(1)) + childOf(span(0)) errored false tags { "$Tags.COMPONENT.key" "ratpack" @@ -240,19 +464,38 @@ class RatpackTest extends AgentTestRunner { "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 200 "$Tags.HTTP_URL.key" "/nested2" - defaultTags(true) + defaultTags() } } } - trace(2, 3) { + trace(2, 4) { // main app span that processed the request from OKHTTP request span(0) { resourceName "GET /" serviceName "unnamed-java-app" - operationName "ratpack.handler" + operationName "netty.request" spanType DDSpanTypes.HTTP_SERVER parent() errored false + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + defaultTags() + } + } + span(1) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "ratpack.handler" + spanType DDSpanTypes.HTTP_SERVER + childOf(span(0)) + errored false tags { "$Tags.COMPONENT.key" "ratpack" "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER @@ -263,36 +506,125 @@ class RatpackTest extends AgentTestRunner { } } // Second http client call that receives the 'ess' of Success - span(1) { + span(2) { resourceName "GET /?" serviceName "unnamed-java-app" - operationName "ratpack.client-request" + operationName "netty.client.request" spanType DDSpanTypes.HTTP_CLIENT - childOf(span(0)) + childOf(span(3)) errored false tags { - "$Tags.COMPONENT.key" "ratpack-httpclient" + "$Tags.COMPONENT.key" "netty-client" "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 200 "$Tags.HTTP_URL.key" "${external.address}nested2" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer defaultTags() } } // First http client call that receives the 'Succ' of Success - span(2) { + span(3) { resourceName "GET /nested" serviceName "unnamed-java-app" - operationName "ratpack.client-request" + operationName "netty.client.request" spanType DDSpanTypes.HTTP_CLIENT childOf(span(0)) errored false tags { - "$Tags.COMPONENT.key" "ratpack-httpclient" + "$Tags.COMPONENT.key" "netty-client" "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 200 "$Tags.HTTP_URL.key" "${external.address}nested" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + defaultTags() + } + } + } + } + } + + def "test ratpack http client error handling"() { + setup: + def badAddress = new URI("http://localhost:$UNUSABLE_PORT") + + def app = GroovyEmbeddedApp.ratpack { + handlers { + get { HttpClient httpClient -> + httpClient.get(badAddress) + .map { it.body.text } + .then { + context.render(it) + } + } + } + } + def request = new Request.Builder() + .url(app.address.toURL()) + .get() + .build() + + when: + def resp = client.newCall(request).execute() + + then: + resp.code() == 500 + + assertTraces(1) { + trace(0, 3) { + span(0) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "netty.request" + spanType DDSpanTypes.HTTP_SERVER + parent() + errored true + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 500 + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + errorTags(ConnectException, String) + defaultTags() + } + } + span(1) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "ratpack.handler" + spanType DDSpanTypes.HTTP_SERVER + childOf(span(0)) + errored true + tags { + "$Tags.COMPONENT.key" "ratpack" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 500 + "$Tags.HTTP_URL.key" "/" + "$Tags.ERROR.key" true + defaultTags() + } + } + span(2) { + operationName "netty.connect" + resourceName "netty.connect" + // Notice the span is a child of the netty span. + // I don't think we can easily get it rooted under the ratpack span + // since the "parent" is stored on the channel. + childOf(span(0)) + errored true + tags { + "$Tags.COMPONENT.key" "netty" + errorTags(ConnectException, String) defaultTags() } } @@ -305,26 +637,24 @@ class RatpackTest extends AgentTestRunner { def app = GroovyEmbeddedApp.ratpack { handlers { get { - final Scope scope = !startSpanInHandler ? GlobalTracer.get().scopeManager().active() : - GlobalTracer.get() + TraceScope scope + if (startSpanInHandler) { + Span childSpan = GlobalTracer.get() .buildSpan("ratpack.exec-test") .withTag(DDTags.RESOURCE_NAME, "INSIDE-TEST") - .startActive(false) - - if (startSpanInHandler) { - ((TraceScope) scope).setAsyncPropagation(true) + .start() + scope = GlobalTracer.get().scopeManager().activate(childSpan, true) } - scope.span().setBaggageItem("test-baggage", "foo") + def latch = new CountDownLatch(1) + try { + scope?.setAsyncPropagation(true) + GlobalTracer.get().activeSpan().setBaggageItem("test-baggage", "foo") - final Span startedSpan = startSpanInHandler ? scope.span() : null - if (startSpanInHandler) { - scope.close() - context.onClose { - startedSpan.finish() - } + context.render(testPromise(latch).fork()) + } finally { + scope?.close() + latch.countDown() } - - context.render(testPromise().fork()) } } } @@ -341,13 +671,44 @@ class RatpackTest extends AgentTestRunner { resp.body().string() == "foo" assertTraces(1) { - trace(0, (startSpanInHandler ? 2 : 1)) { + trace(0, (startSpanInHandler ? 3 : 2)) { + if (startSpanInHandler) { + span(0) { + resourceName "INSIDE-TEST" + serviceName "unnamed-java-app" + operationName "ratpack.exec-test" + childOf(span(2)) + errored false + tags { + defaultTags() + } + } + } span(startSpanInHandler ? 1 : 0) { + resourceName "GET /" + serviceName "unnamed-java-app" + operationName "netty.request" + spanType DDSpanTypes.HTTP_SERVER + parent() + errored false + tags { + "$Tags.COMPONENT.key" "netty" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "$app.address" + "$Tags.PEER_HOSTNAME.key" "$app.address.host" + "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" + "$Tags.PEER_PORT.key" Integer + defaultTags() + } + } + span(startSpanInHandler ? 2 : 1) { resourceName "GET /" serviceName "unnamed-java-app" operationName "ratpack.handler" spanType DDSpanTypes.HTTP_SERVER - parent() + childOf(span(startSpanInHandler ? 1 : 0)) errored false tags { "$Tags.COMPONENT.key" "ratpack" @@ -358,18 +719,6 @@ class RatpackTest extends AgentTestRunner { defaultTags() } } - if (startSpanInHandler) { - span(0) { - resourceName "INSIDE-TEST" - serviceName "unnamed-java-app" - operationName "ratpack.exec-test" - childOf(span(1)) - errored false - tags { - defaultTags() - } - } - } } } @@ -416,10 +765,11 @@ class RatpackTest extends AgentTestRunner { } // returns a promise that contains the active scope's "test-baggage" baggage - Promise testPromise() { + Promise testPromise(CountDownLatch latch = null) { Promise.sync { + latch?.await() Scope tracerScope = GlobalTracer.get().scopeManager().active() - return tracerScope.span().getBaggageItem("test-baggage") + return tracerScope?.span()?.getBaggageItem("test-baggage") } } } diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/TagsAssert.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/TagsAssert.groovy index 99ec088a70..2052bead03 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/TagsAssert.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/TagsAssert.groovy @@ -52,13 +52,13 @@ class TagsAssert { errorTags(errorType, null) } - def errorTags(Class errorType, Object message) { - methodMissing("error", [true].toArray()) - methodMissing("error.type", [errorType.name].toArray()) - methodMissing("error.stack", [String].toArray()) + def errorTags(Class errorType, message) { + tag("error", true) + tag("error.type", errorType.name) + tag("error.stack", String) if (message != null) { - methodMissing("error.msg", [message].toArray()) + tag("error.msg", message) } } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java index a564ed5875..5fc697c000 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java @@ -293,6 +293,7 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace spanContextDecorators.put(decorator.getMatchingTag(), list); } + @Deprecated public void addScopeContext(final ScopeContext context) { scopeManager.addScopeContext(context); } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContextualScopeManager.java b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContextualScopeManager.java index 49d4ba4a6d..c569bdffb5 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContextualScopeManager.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContextualScopeManager.java @@ -39,12 +39,13 @@ public class ContextualScopeManager implements ScopeManager { return tlsScope.get(); } + @Deprecated public void addScopeContext(final ScopeContext context) { scopeContexts.addFirst(context); } /** Attach a listener to scope activation events */ - public void addScopeListener(ScopeListener listener) { + public void addScopeListener(final ScopeListener listener) { scopeListeners.add(listener); } } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ScopeContext.java b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ScopeContext.java index 37786b0b8d..b256d9ac7b 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ScopeContext.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ScopeContext.java @@ -3,6 +3,7 @@ package datadog.opentracing.scopemanager; import io.opentracing.ScopeManager; /** Represents a ScopeManager that is only valid in certain cases such as on a specific thread. */ +@Deprecated public interface ScopeContext extends ScopeManager { /** From c4ac5b94ec17b01a1b4ae43aa3946a082aa9561b Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Wed, 10 Apr 2019 16:41:08 -0700 Subject: [PATCH 2/2] Fix span relationships and other review issues. --- .../tooling/muzzle/ReferenceCreator.java | 4 +- .../netty40/AttributeKeys.java | 3 +- .../ChannelFutureListenerInstrumentation.java | 8 ++ .../netty41/AttributeKeys.java | 3 +- .../ChannelFutureListenerInstrumentation.java | 8 ++ .../latestDepTest/groovy/RatpackTest.groovy | 113 ++++-------------- .../ratpack/ExecStreamInstrumentation.java | 108 +++++++++++++++++ .../ServerRegistryInstrumentation.java | 2 +- ...yAdvice.java => ServerRegistryAdvice.java} | 4 +- .../ratpack/TracingHandler.java | 5 +- .../src/test/groovy/RatpackTest.groovy | 108 +++-------------- .../test/server/http/TestHttpServer.groovy | 1 + .../datadog/trace/context/TraceScope.java | 5 +- 13 files changed, 180 insertions(+), 192 deletions(-) create mode 100644 dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ExecStreamInstrumentation.java rename dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/{RatpackServerRegistryAdvice.java => ServerRegistryAdvice.java} (71%) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/ReferenceCreator.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/ReferenceCreator.java index 2aafef2161..52289b56ff 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/ReferenceCreator.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/ReferenceCreator.java @@ -46,9 +46,11 @@ public class ReferenceCreator extends ClassVisitor { * @param loader Classloader used to read class bytes. * @param startFromMethodBodies if true only create refs from method bodies. * @return Map of [referenceClassName -> Reference] + * @throws IllegalStateException if class is not found or unable to be loaded. */ private static Map createReferencesFrom( - final String entryPointClassName, final ClassLoader loader, boolean startFromMethodBodies) { + final String entryPointClassName, final ClassLoader loader, boolean startFromMethodBodies) + throws IllegalStateException { final Set visitedSources = new HashSet<>(); final Map references = new HashMap<>(); diff --git a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/AttributeKeys.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/AttributeKeys.java index 52ca942763..b1b1ecaafa 100644 --- a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/AttributeKeys.java +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/AttributeKeys.java @@ -1,6 +1,7 @@ package datadog.trace.instrumentation.netty40; import datadog.trace.context.TraceScope; +import datadog.trace.instrumentation.netty40.client.HttpClientTracingHandler; import datadog.trace.instrumentation.netty40.server.HttpServerTracingHandler; import io.netty.util.AttributeKey; import io.opentracing.Span; @@ -14,5 +15,5 @@ public class AttributeKeys { new AttributeKey<>(HttpServerTracingHandler.class.getName() + ".span"); public static final AttributeKey CLIENT_ATTRIBUTE_KEY = - new AttributeKey<>(HttpServerTracingHandler.class.getName() + ".span"); + new AttributeKey<>(HttpClientTracingHandler.class.getName() + ".span"); } diff --git a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/ChannelFutureListenerInstrumentation.java b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/ChannelFutureListenerInstrumentation.java index 51ebfc8965..8dbdc73265 100644 --- a/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/ChannelFutureListenerInstrumentation.java +++ b/dd-java-agent/instrumentation/netty-4.0/src/main/java/datadog/trace/instrumentation/netty40/ChannelFutureListenerInstrumentation.java @@ -43,6 +43,14 @@ public class ChannelFutureListenerInstrumentation extends Instrumenter.Default { return new String[] { packageName + ".AttributeKeys", "datadog.trace.agent.decorator.BaseDecorator", + // client helpers + "datadog.trace.agent.decorator.ClientDecorator", + "datadog.trace.agent.decorator.HttpClientDecorator", + packageName + ".client.NettyHttpClientDecorator", + packageName + ".client.NettyResponseInjectAdapter", + packageName + ".client.HttpClientRequestTracingHandler", + packageName + ".client.HttpClientResponseTracingHandler", + packageName + ".client.HttpClientTracingHandler", // server helpers "datadog.trace.agent.decorator.ServerDecorator", "datadog.trace.agent.decorator.HttpServerDecorator", diff --git a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/AttributeKeys.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/AttributeKeys.java index 9b978483ba..9a8ea9a0a0 100644 --- a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/AttributeKeys.java +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/AttributeKeys.java @@ -1,6 +1,7 @@ package datadog.trace.instrumentation.netty41; import datadog.trace.context.TraceScope; +import datadog.trace.instrumentation.netty41.client.HttpClientTracingHandler; import datadog.trace.instrumentation.netty41.server.HttpServerTracingHandler; import io.netty.util.AttributeKey; import io.opentracing.Span; @@ -18,5 +19,5 @@ public class AttributeKeys { AttributeKey.valueOf(HttpServerTracingHandler.class.getName() + ".span"); public static final AttributeKey CLIENT_ATTRIBUTE_KEY = - AttributeKey.valueOf(HttpServerTracingHandler.class.getName() + ".span"); + AttributeKey.valueOf(HttpClientTracingHandler.class.getName() + ".span"); } diff --git a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/ChannelFutureListenerInstrumentation.java b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/ChannelFutureListenerInstrumentation.java index c32ed8da8b..069c31e312 100644 --- a/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/ChannelFutureListenerInstrumentation.java +++ b/dd-java-agent/instrumentation/netty-4.1/src/main/java/datadog/trace/instrumentation/netty41/ChannelFutureListenerInstrumentation.java @@ -43,6 +43,14 @@ public class ChannelFutureListenerInstrumentation extends Instrumenter.Default { return new String[] { packageName + ".AttributeKeys", "datadog.trace.agent.decorator.BaseDecorator", + // client helpers + "datadog.trace.agent.decorator.ClientDecorator", + "datadog.trace.agent.decorator.HttpClientDecorator", + packageName + ".client.NettyHttpClientDecorator", + packageName + ".client.NettyResponseInjectAdapter", + packageName + ".client.HttpClientRequestTracingHandler", + packageName + ".client.HttpClientResponseTracingHandler", + packageName + ".client.HttpClientTracingHandler", // server helpers "datadog.trace.agent.decorator.ServerDecorator", "datadog.trace.agent.decorator.HttpServerDecorator", diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/latestDepTest/groovy/RatpackTest.groovy b/dd-java-agent/instrumentation/ratpack-1.4/src/latestDepTest/groovy/RatpackTest.groovy index fa88adc401..7e6b1839e2 100644 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/latestDepTest/groovy/RatpackTest.groovy +++ b/dd-java-agent/instrumentation/ratpack-1.4/src/latestDepTest/groovy/RatpackTest.groovy @@ -19,18 +19,18 @@ import ratpack.http.HttpUrlBuilder import ratpack.http.client.HttpClient import ratpack.path.PathBinding import ratpack.test.exec.ExecHarness -import spock.lang.Retry import java.util.concurrent.CountDownLatch +import java.util.regex.Pattern +import static datadog.trace.agent.test.server.http.TestHttpServer.distributedRequestTrace +import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT -@Retry class RatpackTest extends AgentTestRunner { OkHttpClient client = OkHttpUtils.client() - def "test path call"() { setup: def app = GroovyEmbeddedApp.ratpack { @@ -193,7 +193,7 @@ class RatpackTest extends AgentTestRunner { "$Tags.PEER_HOSTNAME.key" "$app.address.host" "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" "$Tags.PEER_PORT.key" Integer - errorTags(ArithmeticException, "Division undefined") + errorTags(ArithmeticException, Pattern.compile("Division( is)? undefined")) defaultTags() } } @@ -210,7 +210,7 @@ class RatpackTest extends AgentTestRunner { "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 500 "$Tags.HTTP_URL.key" "/" - errorTags(HandlerException, "java.lang.ArithmeticException: Division undefined") + errorTags(HandlerException, Pattern.compile("java.lang.ArithmeticException: Division( is)? undefined")) defaultTags() } } @@ -258,7 +258,7 @@ class RatpackTest extends AgentTestRunner { "$Tags.PEER_HOSTNAME.key" "$app.address.host" "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" "$Tags.PEER_PORT.key" Integer - errorTags(ArithmeticException, "Division undefined") + errorTags(ArithmeticException, Pattern.compile("Division( is)? undefined")) defaultTags() } } @@ -321,7 +321,7 @@ class RatpackTest extends AgentTestRunner { "$Tags.PEER_HOSTNAME.key" "$app.address.host" "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" "$Tags.PEER_PORT.key" Integer - errorTags(ArithmeticException, "Division undefined") + errorTags(ArithmeticException, Pattern.compile("Division( is)? undefined")) defaultTags() } } @@ -349,13 +349,16 @@ class RatpackTest extends AgentTestRunner { def "test path call using ratpack http client"() { setup: - def external = GroovyEmbeddedApp.ratpack { + // Use jetty based server to avoid confusion. + def external = httpServer { handlers { get("nested") { - context.render("succ") + handleDistributedRequest() + response.send("succ") } get("nested2") { - context.render("ess") + handleDistributedRequest() + response.send("ess") } } } @@ -392,82 +395,9 @@ class RatpackTest extends AgentTestRunner { // 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) assertTraces(3) { - // simulated external system, first call - trace(0, 2) { - span(0) { - resourceName "GET /nested" - serviceName "unnamed-java-app" - operationName "netty.request" - spanType DDSpanTypes.HTTP_SERVER - childOf(trace(2).get(3)) - errored false - tags { - "$Tags.COMPONENT.key" "netty" - "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER - "$Tags.HTTP_METHOD.key" "GET" - "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "${external.address}nested" - "$Tags.PEER_HOSTNAME.key" "$app.address.host" - "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" - "$Tags.PEER_PORT.key" Integer - defaultTags(true) - } - } - span(1) { - resourceName "GET /nested" - serviceName "unnamed-java-app" - operationName "ratpack.handler" - spanType DDSpanTypes.HTTP_SERVER - childOf(span(0)) - errored false - tags { - "$Tags.COMPONENT.key" "ratpack" - "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER - "$Tags.HTTP_METHOD.key" "GET" - "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "/nested" - defaultTags() - } - } - } -// // simulated external system, second call - trace(1, 2) { - span(0) { - resourceName "GET /nested2" - serviceName "unnamed-java-app" - operationName "netty.request" - spanType DDSpanTypes.HTTP_SERVER - childOf(trace(2).get(2)) - errored false - tags { - "$Tags.COMPONENT.key" "netty" - "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER - "$Tags.HTTP_METHOD.key" "GET" - "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "${external.address}nested2" - "$Tags.PEER_HOSTNAME.key" "$app.address.host" - "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" - "$Tags.PEER_PORT.key" Integer - defaultTags(true) - } - } - span(1) { - resourceName "GET /nested2" - serviceName "unnamed-java-app" - operationName "ratpack.handler" - spanType DDSpanTypes.HTTP_SERVER - childOf(span(0)) - errored false - tags { - "$Tags.COMPONENT.key" "ratpack" - "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER - "$Tags.HTTP_METHOD.key" "GET" - "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "/nested2" - defaultTags() - } - } - } + distributedRequestTrace(it, 0, trace(2).get(3)) + distributedRequestTrace(it, 1, trace(2).get(2)) + trace(2, 4) { // main app span that processed the request from OKHTTP request span(0) { @@ -518,7 +448,7 @@ class RatpackTest extends AgentTestRunner { "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "${external.address}nested2" + "$Tags.HTTP_URL.key" "${external.address}/nested2" "$Tags.PEER_HOSTNAME.key" "$app.address.host" "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" "$Tags.PEER_PORT.key" Integer @@ -531,14 +461,14 @@ class RatpackTest extends AgentTestRunner { serviceName "unnamed-java-app" operationName "netty.client.request" spanType DDSpanTypes.HTTP_CLIENT - childOf(span(0)) + childOf(span(1)) errored false tags { "$Tags.COMPONENT.key" "netty-client" "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "${external.address}nested" + "$Tags.HTTP_URL.key" "${external.address}/nested" "$Tags.PEER_HOSTNAME.key" "$app.address.host" "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" "$Tags.PEER_PORT.key" Integer @@ -617,10 +547,7 @@ class RatpackTest extends AgentTestRunner { span(2) { operationName "netty.connect" resourceName "netty.connect" - // Notice the span is a child of the netty span. - // I don't think we can easily get it rooted under the ratpack span - // since the "parent" is stored on the channel. - childOf(span(0)) + childOf(span(1)) errored true tags { "$Tags.COMPONENT.key" "netty" diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ExecStreamInstrumentation.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ExecStreamInstrumentation.java new file mode 100644 index 0000000000..4ee441be6a --- /dev/null +++ b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ExecStreamInstrumentation.java @@ -0,0 +1,108 @@ +package datadog.trace.instrumentation.ratpack; + +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; +import static net.bytebuddy.matcher.ElementMatchers.isInterface; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.context.TraceScope; +import io.opentracing.Scope; +import io.opentracing.util.GlobalTracer; +import java.util.Collections; +import java.util.Map; +import lombok.extern.slf4j.Slf4j; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; +import ratpack.exec.internal.Continuation; +import ratpack.func.Action; +import ratpack.path.PathBinding; + +@AutoService(Instrumenter.class) +public final class ExecStreamInstrumentation extends Instrumenter.Default { + + public ExecStreamInstrumentation() { + super("ratpack"); + } + + @Override + public ElementMatcher typeMatcher() { + return not(isInterface()) + .and(safeHasSuperType(named("ratpack.exec.internal.DefaultExecution"))); + } + + // ratpack.exec.internal.DefaultExecution.delimit + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".ExecStreamInstrumentation$ActionWrapper", + }; + } + + @Override + public Map, String> transformers() { + return Collections.singletonMap( + named("delimit") + .or(named("delimitStream")) + .and(takesArgument(0, named("ratpack.func.Action"))) + .and(takesArgument(1, named("ratpack.func.Action"))), + WrapActionAdvice.class.getName()); + } + + public static class WrapActionAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void wrap( + @Advice.Argument(value = 0, readOnly = false) Action onError, + @Advice.Argument(value = 1, readOnly = false) Action segment) { + final Scope scope = GlobalTracer.get().scopeManager().active(); + if (scope instanceof TraceScope) { + final TraceScope.Continuation continuation = ((TraceScope) scope).capture(); + onError = ActionWrapper.wrapIfNeeded(onError, continuation); + segment = ActionWrapper.wrapIfNeeded(segment, continuation); + } + } + + public void muzzleCheck(final PathBinding binding) { + // This was added in 1.4. Added here to ensure consistency with other instrumentation. + binding.getDescription(); + } + } + + @Slf4j + public static class ActionWrapper implements Action { + private final Action delegate; + private final TraceScope.Continuation traceContinuation; + + private ActionWrapper( + final Action delegate, final TraceScope.Continuation traceContinuation) { + this.delegate = delegate; + this.traceContinuation = traceContinuation; + } + + @Override + public void execute(final T subject) throws Exception { + if (traceContinuation != null) { + try (final TraceScope scope = traceContinuation.activate()) { + scope.setAsyncPropagation(true); + delegate.execute(subject); + } + } else { + delegate.execute(subject); + } + } + + public static Action wrapIfNeeded( + final Action delegate, final TraceScope.Continuation traceContinuation) { + if (delegate instanceof ActionWrapper) { + return delegate; + } + log.debug("Wrapping action task {}", delegate); + return new ActionWrapper<>(delegate, traceContinuation); + } + } +} diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ServerRegistryInstrumentation.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ServerRegistryInstrumentation.java index 01024f65c4..45ec7e5f06 100644 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ServerRegistryInstrumentation.java +++ b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java/datadog/trace/instrumentation/ratpack/ServerRegistryInstrumentation.java @@ -39,6 +39,6 @@ public class ServerRegistryInstrumentation extends Instrumenter.Default { public Map, String> transformers() { return singletonMap( isMethod().and(isStatic()).and(named("buildBaseRegistry")), - packageName + ".RatpackServerRegistryAdvice"); + packageName + ".ServerRegistryAdvice"); } } diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerRegistryAdvice.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/ServerRegistryAdvice.java similarity index 71% rename from dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerRegistryAdvice.java rename to dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/ServerRegistryAdvice.java index 1806a12e8b..24425d80ee 100644 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/RatpackServerRegistryAdvice.java +++ b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/ServerRegistryAdvice.java @@ -4,11 +4,11 @@ import net.bytebuddy.asm.Advice; import ratpack.handling.HandlerDecorator; import ratpack.registry.Registry; -public class RatpackServerRegistryAdvice { +public class ServerRegistryAdvice { @Advice.OnMethodExit(suppress = Throwable.class) public static void injectTracing(@Advice.Return(readOnly = false) Registry registry) { registry = registry.join( - Registry.builder().add(HandlerDecorator.prepend(new TracingHandler())).build()); + Registry.builder().add(HandlerDecorator.prepend(TracingHandler.INSTANCE)).build()); } } diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/TracingHandler.java b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/TracingHandler.java index fe69a0d7f0..8790bb59fd 100644 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/TracingHandler.java +++ b/dd-java-agent/instrumentation/ratpack-1.4/src/main/java8/datadog/trace/instrumentation/ratpack/TracingHandler.java @@ -18,6 +18,8 @@ import ratpack.http.Request; * that the span is closed when the response is sent */ public final class TracingHandler implements Handler { + public static Handler INSTANCE = new TracingHandler(); + /** * This constant is copied over from datadog.trace.instrumentation.netty41.AttributeKeys. The key * string must be kept consistent. @@ -64,7 +66,8 @@ public final class TracingHandler implements Handler { } catch (final Throwable e) { DECORATE.onError(ratpackSpan, e); DECORATE.beforeFinish(ratpackSpan); - ratpackSpan.finish(); // Do we need to finish? + // finish since the callback probably didn't get added. + ratpackSpan.finish(); throw e; } } diff --git a/dd-java-agent/instrumentation/ratpack-1.4/src/test/groovy/RatpackTest.groovy b/dd-java-agent/instrumentation/ratpack-1.4/src/test/groovy/RatpackTest.groovy index 54f5d953ff..092bc48115 100644 --- a/dd-java-agent/instrumentation/ratpack-1.4/src/test/groovy/RatpackTest.groovy +++ b/dd-java-agent/instrumentation/ratpack-1.4/src/test/groovy/RatpackTest.groovy @@ -18,19 +18,18 @@ import ratpack.http.HttpUrlBuilder import ratpack.http.client.HttpClient import ratpack.path.PathBinding import ratpack.test.exec.ExecHarness -import spock.lang.Retry import java.util.concurrent.CountDownLatch import java.util.regex.Pattern +import static datadog.trace.agent.test.server.http.TestHttpServer.distributedRequestTrace +import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT -@Retry class RatpackTest extends AgentTestRunner { OkHttpClient client = OkHttpUtils.client() - def "test path call"() { setup: def app = GroovyEmbeddedApp.ratpack { @@ -349,13 +348,16 @@ class RatpackTest extends AgentTestRunner { def "test path call using ratpack http client"() { setup: - def external = GroovyEmbeddedApp.ratpack { + // Use jetty based server to avoid confusion. + def external = httpServer { handlers { get("nested") { - context.render("succ") + handleDistributedRequest() + response.send("succ") } get("nested2") { - context.render("ess") + handleDistributedRequest() + response.send("ess") } } } @@ -392,82 +394,9 @@ class RatpackTest extends AgentTestRunner { // 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) assertTraces(3) { - // simulated external system, first call - trace(0, 2) { - span(0) { - resourceName "GET /nested" - serviceName "unnamed-java-app" - operationName "netty.request" - spanType DDSpanTypes.HTTP_SERVER - childOf(trace(2).get(3)) - errored false - tags { - "$Tags.COMPONENT.key" "netty" - "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER - "$Tags.HTTP_METHOD.key" "GET" - "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "${external.address}nested" - "$Tags.PEER_HOSTNAME.key" "$app.address.host" - "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" - "$Tags.PEER_PORT.key" Integer - defaultTags(true) - } - } - span(1) { - resourceName "GET /nested" - serviceName "unnamed-java-app" - operationName "ratpack.handler" - spanType DDSpanTypes.HTTP_SERVER - childOf(span(0)) - errored false - tags { - "$Tags.COMPONENT.key" "ratpack" - "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER - "$Tags.HTTP_METHOD.key" "GET" - "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "/nested" - defaultTags() - } - } - } -// // simulated external system, second call - trace(1, 2) { - span(0) { - resourceName "GET /nested2" - serviceName "unnamed-java-app" - operationName "netty.request" - spanType DDSpanTypes.HTTP_SERVER - childOf(trace(2).get(2)) - errored false - tags { - "$Tags.COMPONENT.key" "netty" - "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER - "$Tags.HTTP_METHOD.key" "GET" - "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "${external.address}nested2" - "$Tags.PEER_HOSTNAME.key" "$app.address.host" - "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" - "$Tags.PEER_PORT.key" Integer - defaultTags(true) - } - } - span(1) { - resourceName "GET /nested2" - serviceName "unnamed-java-app" - operationName "ratpack.handler" - spanType DDSpanTypes.HTTP_SERVER - childOf(span(0)) - errored false - tags { - "$Tags.COMPONENT.key" "ratpack" - "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER - "$Tags.HTTP_METHOD.key" "GET" - "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "/nested2" - defaultTags() - } - } - } + distributedRequestTrace(it, 0, trace(2).get(3)) + distributedRequestTrace(it, 1, trace(2).get(2)) + trace(2, 4) { // main app span that processed the request from OKHTTP request span(0) { @@ -518,7 +447,7 @@ class RatpackTest extends AgentTestRunner { "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "${external.address}nested2" + "$Tags.HTTP_URL.key" "${external.address}/nested2" "$Tags.PEER_HOSTNAME.key" "$app.address.host" "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" "$Tags.PEER_PORT.key" Integer @@ -531,14 +460,14 @@ class RatpackTest extends AgentTestRunner { serviceName "unnamed-java-app" operationName "netty.client.request" spanType DDSpanTypes.HTTP_CLIENT - childOf(span(0)) + childOf(span(1)) errored false tags { "$Tags.COMPONENT.key" "netty-client" "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "${external.address}nested" + "$Tags.HTTP_URL.key" "${external.address}/nested" "$Tags.PEER_HOSTNAME.key" "$app.address.host" "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" "$Tags.PEER_PORT.key" Integer @@ -593,7 +522,7 @@ class RatpackTest extends AgentTestRunner { "$Tags.PEER_HOSTNAME.key" "$app.address.host" "$Tags.PEER_HOST_IPV4.key" "127.0.0.1" "$Tags.PEER_PORT.key" Integer - errorTags(ConnectException, String) + "$Tags.ERROR.key" true defaultTags() } } @@ -610,17 +539,14 @@ class RatpackTest extends AgentTestRunner { "$Tags.HTTP_METHOD.key" "GET" "$Tags.HTTP_STATUS.key" 500 "$Tags.HTTP_URL.key" "/" - "$Tags.ERROR.key" true + errorTags(ConnectException, String) defaultTags() } } span(2) { operationName "netty.connect" resourceName "netty.connect" - // Notice the span is a child of the netty span. - // I don't think we can easily get it rooted under the ratpack span - // since the "parent" is stored on the channel. - childOf(span(0)) + childOf(span(1)) errored true tags { "$Tags.COMPONENT.key" "netty" diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/server/http/TestHttpServer.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/server/http/TestHttpServer.groovy index 1a7ae75f99..0abfb6ecfc 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/server/http/TestHttpServer.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/server/http/TestHttpServer.groovy @@ -298,6 +298,7 @@ class TestHttpServer implements AutoCloseable { assert body != null send() + resp.setContentLength(body.bytes.length) resp.writer.print(body) } } diff --git a/dd-trace-api/src/main/java/datadog/trace/context/TraceScope.java b/dd-trace-api/src/main/java/datadog/trace/context/TraceScope.java index 866c81eeb6..709301d8fb 100644 --- a/dd-trace-api/src/main/java/datadog/trace/context/TraceScope.java +++ b/dd-trace-api/src/main/java/datadog/trace/context/TraceScope.java @@ -1,7 +1,9 @@ package datadog.trace.context; +import java.io.Closeable; + /** An object when can propagate a datadog trace across multiple threads. */ -public interface TraceScope { +public interface TraceScope extends Closeable { /** * Prevent the trace attached to this TraceScope from reporting until the returned Continuation * finishes. @@ -11,6 +13,7 @@ public interface TraceScope { Continuation capture(); /** Close the activated context and allow any underlying spans to finish. */ + @Override void close(); /** If true, this context will propagate across async boundaries. */