diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java index cf3d3d8763..551df0e1c8 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java @@ -18,7 +18,7 @@ import com.amazonaws.Request; import com.amazonaws.Response; import com.amazonaws.handlers.HandlerContextKey; import com.amazonaws.handlers.RequestHandler2; -import io.opentracing.Span; +import io.opentracing.Scope; import io.opentracing.SpanContext; import io.opentracing.Tracer; import io.opentracing.propagation.Format; @@ -28,7 +28,11 @@ import io.opentracing.tag.Tags; /** Tracing Request Handler */ public class TracingRequestHandler extends RequestHandler2 { - private final HandlerContextKey contextKey = new HandlerContextKey<>("span"); + // Note: aws1.x sdk doesn't have any truly async clients so we can store scope in request context + // safely. + private static final HandlerContextKey SCOPE_CONTEXT_KEY = + new HandlerContextKey<>("DatadogScope"); + private final SpanContext parentContext; // for Async Client private final Tracer tracer; @@ -64,30 +68,32 @@ public class TracingRequestHandler extends RequestHandler2 { spanBuilder.asChildOf(parentContext); } - final Span span = spanBuilder.start(); - SpanDecorator.onRequest(request, span); + final Scope scope = spanBuilder.startActive(true); + SpanDecorator.onRequest(request, scope.span()); + // We inject headers at aws-client level because aws requests may be signed and adding headers + // on http-client level may break signature. tracer.inject( - span.context(), + scope.span().context(), Format.Builtin.HTTP_HEADERS, new TextMapInjectAdapter(request.getHeaders())); - request.addHandlerContext(contextKey, span); + request.addHandlerContext(SCOPE_CONTEXT_KEY, scope); } /** {@inheritDoc} */ @Override public void afterResponse(final Request request, final Response response) { - final Span span = request.getHandlerContext(contextKey); - SpanDecorator.onResponse(response, span); - span.finish(); + final Scope scope = request.getHandlerContext(SCOPE_CONTEXT_KEY); + SpanDecorator.onResponse(response, scope.span()); + scope.close(); } /** {@inheritDoc} */ @Override public void afterError(final Request request, final Response response, final Exception e) { - final Span span = request.getHandlerContext(contextKey); - SpanDecorator.onError(e, span); - span.finish(); + final Scope scope = request.getHandlerContext(SCOPE_CONTEXT_KEY); + SpanDecorator.onError(e, scope.span()); + scope.close(); } } diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test/groovy/AWSClientTest.groovy b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test/groovy/AWSClientTest.groovy index 691fc7ae3a..18b8845a1e 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test/groovy/AWSClientTest.groovy +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test/groovy/AWSClientTest.groovy @@ -73,27 +73,8 @@ class AWSClientTest extends AgentTestRunner { client.requestHandler2s.size() == handlerCount client.requestHandler2s.get(0).getClass().getSimpleName() == "TracingRequestHandler" - assertTraces(2) { - trace(0, 1) { - span(0) { - operationName "http.request" - resourceName "$method /$url" - errored false - parent() // FIXME: This should be a child of the aws.http call. - tags { - "$Tags.COMPONENT.key" "apache-httpclient" - "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "$server.address/$url" - "$Tags.PEER_HOSTNAME.key" "localhost" - "$Tags.PEER_PORT.key" server.address.port - "$Tags.HTTP_METHOD.key" "$method" - "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT - "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT - defaultTags() - } - } - } - trace(1, 1) { + assertTraces(1) { + trace(0, 2) { span(0) { serviceName "java-aws-sdk" operationName "aws.http" @@ -107,18 +88,34 @@ class AWSClientTest extends AgentTestRunner { "$Tags.HTTP_METHOD.key" "$method" "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT - "aws.service" String + "aws.service" { it.contains(service) } "aws.endpoint" "$server.address" "aws.operation" "${operation}Request" "aws.agent" "java-aws-sdk" defaultTags() } } + span(1) { + operationName "http.request" + resourceName "$method /$url" + errored false + childOf(span(0)) + tags { + "$Tags.COMPONENT.key" "apache-httpclient" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "$server.address/$url" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" server.address.port + "$Tags.HTTP_METHOD.key" "$method" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT + defaultTags() + } + } } } - // Not sure why these are children of the aws.http span: - server.lastRequest.headers.get("x-datadog-trace-id") == TEST_WRITER[1][0].traceId - server.lastRequest.headers.get("x-datadog-parent-id") == TEST_WRITER[1][0].spanId + server.lastRequest.headers.get("x-datadog-trace-id") == TEST_WRITER[0][0].traceId + server.lastRequest.headers.get("x-datadog-parent-id") == TEST_WRITER[0][0].spanId where: service | operation | method | url | handlerCount | call | body | client diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test_1_11_106/groovy/AWSClientTest.groovy b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test_1_11_106/groovy/AWSClientTest.groovy index 339356f88d..8e1a259dad 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test_1_11_106/groovy/AWSClientTest.groovy +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test_1_11_106/groovy/AWSClientTest.groovy @@ -102,27 +102,8 @@ class AWSClientTest extends AgentTestRunner { client.requestHandler2s.size() == handlerCount client.requestHandler2s.get(0).getClass().getSimpleName() == "TracingRequestHandler" - assertTraces(2) { - trace(0, 1) { - span(0) { - operationName "http.request" - resourceName "$method /$url" - errored false - parent() // FIXME: This should be a child of the aws.http call. - tags { - "$Tags.COMPONENT.key" "apache-httpclient" - "$Tags.HTTP_STATUS.key" 200 - "$Tags.HTTP_URL.key" "$server.address/$url" - "$Tags.PEER_HOSTNAME.key" "localhost" - "$Tags.PEER_PORT.key" server.address.port - "$Tags.HTTP_METHOD.key" "$method" - "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT - "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT - defaultTags() - } - } - } - trace(1, 1) { + assertTraces(1) { + trace(0, 2) { span(0) { serviceName "java-aws-sdk" operationName "aws.http" @@ -136,18 +117,34 @@ class AWSClientTest extends AgentTestRunner { "$Tags.HTTP_METHOD.key" "$method" "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT - "aws.service" String + "aws.service" { it.contains(service) } "aws.endpoint" "$server.address" "aws.operation" "${operation}Request" "aws.agent" "java-aws-sdk" defaultTags() } } + span(1) { + operationName "http.request" + resourceName "$method /$url" + errored false + childOf(span(0)) + tags { + "$Tags.COMPONENT.key" "apache-httpclient" + "$Tags.HTTP_STATUS.key" 200 + "$Tags.HTTP_URL.key" "$server.address/$url" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" server.address.port + "$Tags.HTTP_METHOD.key" "$method" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT + defaultTags() + } + } } } - // Not sure why these are children of the aws.http span: - server.lastRequest.headers.get("x-datadog-trace-id") == TEST_WRITER[1][0].traceId - server.lastRequest.headers.get("x-datadog-parent-id") == TEST_WRITER[1][0].spanId + server.lastRequest.headers.get("x-datadog-trace-id") == TEST_WRITER[0][0].traceId + server.lastRequest.headers.get("x-datadog-parent-id") == TEST_WRITER[0][0].spanId where: service | operation | method | url | handlerCount | call | body | client