From 9db9927bd2764d127c8a2ae3a8b42ba56d216aeb Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Sun, 31 May 2020 14:55:17 +0900 Subject: [PATCH 1/7] Move client span creation to decorator and suppress creation of nested client spans. --- .../decorator/ClientDecorator.java | 24 ++++++++++++ .../decorator/HttpClientDecorator.java | 5 +++ .../v2_2/TracingExecutionInterceptor.java | 5 +-- .../v4_0/ApacheHttpClientInstrumentation.java | 10 +---- .../v2_2/TracingExecutionInterceptor.java | 4 +- .../src/test/groovy/Aws2ClientTest.groovy | 39 +++---------------- 6 files changed, 38 insertions(+), 49 deletions(-) diff --git a/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecorator.java b/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecorator.java index c5489782e118..038a2ec9bf43 100644 --- a/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecorator.java +++ b/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecorator.java @@ -15,10 +15,34 @@ */ package io.opentelemetry.auto.bootstrap.instrumentation.decorator; +import static io.opentelemetry.auto.bootstrap.WeakMap.Provider.newWeakMap; + +import io.opentelemetry.auto.bootstrap.WeakMap; +import io.opentelemetry.trace.DefaultSpan; import io.opentelemetry.trace.Span; +import io.opentelemetry.trace.Span.Kind; +import io.opentelemetry.trace.Tracer; public abstract class ClientDecorator extends BaseDecorator { + // Work around the fact that we cannot read the kind of currentSpan by keeping track of the spans + // we created. + private static final WeakMap CLIENT_SPANS = newWeakMap(); + + public Span getOrCreateSpan(String name, Tracer tracer) { + final Span current = tracer.getCurrentSpan(); + + if (Boolean.TRUE.equals(CLIENT_SPANS.get(current))) { + // We don't want to create two client spans for a given client call, suppress inner spans. + return DefaultSpan.getInvalid(); + } + + final Span clientSpan = + tracer.spanBuilder(name).setSpanKind(Kind.CLIENT).setParent(current).startSpan(); + CLIENT_SPANS.put(clientSpan, true); + return clientSpan; + } + @Override public Span afterStart(final Span span) { assert span != null; diff --git a/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/HttpClientDecorator.java b/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/HttpClientDecorator.java index de72de7daf07..348fc8fbf6fb 100644 --- a/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/HttpClientDecorator.java +++ b/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/HttpClientDecorator.java @@ -20,6 +20,7 @@ import io.opentelemetry.auto.instrumentation.api.Tags; import io.opentelemetry.trace.Span; import io.opentelemetry.trace.Status; +import io.opentelemetry.trace.Tracer; import java.net.URI; import java.net.URISyntaxException; import lombok.extern.slf4j.Slf4j; @@ -35,6 +36,10 @@ public abstract class HttpClientDecorator extends ClientDecor protected abstract Integer status(RESPONSE response); + public Span getOrCreateSpan(REQUEST request, Tracer tracer) { + return getOrCreateSpan(spanNameForRequest(request), tracer); + } + public String spanNameForRequest(final REQUEST request) { if (request == null) { return DEFAULT_SPAN_NAME; diff --git a/instrumentation-core/aws-sdk/aws-sdk-2.2-core/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java b/instrumentation-core/aws-sdk/aws-sdk-2.2-core/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java index b322ab2a8d1a..2b7ec1fcd913 100644 --- a/instrumentation-core/aws-sdk/aws-sdk-2.2-core/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java +++ b/instrumentation-core/aws-sdk/aws-sdk-2.2-core/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java @@ -40,10 +40,7 @@ final class TracingExecutionInterceptor implements ExecutionInterceptor { public void beforeExecution( final Context.BeforeExecution context, final ExecutionAttributes executionAttributes) { final Span span = - AwsSdk.tracer() - .spanBuilder(DECORATE.spanName(executionAttributes)) - .setSpanKind(kind) - .startSpan(); + DECORATE.getOrCreateSpan(DECORATE.spanName(executionAttributes), AwsSdk.tracer()); DECORATE.afterStart(span); executionAttributes.putAttribute(SPAN_ATTRIBUTE, span); } diff --git a/instrumentation/apache-httpclient/apache-httpclient-4.0/src/main/java/io/opentelemetry/auto/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-4.0/src/main/java/io/opentelemetry/auto/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java index ef185422bad4..fee8d968269d 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-4.0/src/main/java/io/opentelemetry/auto/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java +++ b/instrumentation/apache-httpclient/apache-httpclient-4.0/src/main/java/io/opentelemetry/auto/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java @@ -21,7 +21,6 @@ import static io.opentelemetry.auto.tooling.ClassLoaderMatcher.hasClassesNamed; import static io.opentelemetry.auto.tooling.bytebuddy.matcher.AgentElementMatchers.implementsInterface; import static io.opentelemetry.context.ContextUtils.withScopedContext; -import static io.opentelemetry.trace.Span.Kind.CLIENT; import static io.opentelemetry.trace.TracingContextUtils.withSpan; import static net.bytebuddy.matcher.ElementMatchers.isAbstract; import static net.bytebuddy.matcher.ElementMatchers.isMethod; @@ -172,18 +171,13 @@ public Map, String> transfor public static class HelperMethods { public static SpanWithScope doMethodEnter(final HttpUriRequest request) { - final Span span = - TRACER.spanBuilder(DECORATE.spanNameForRequest(request)).setSpanKind(CLIENT).startSpan(); + final Span span = DECORATE.getOrCreateSpan(request, TRACER); DECORATE.afterStart(span); DECORATE.onRequest(span, request); final Context context = withSpan(span, Context.current()); - final boolean awsClientCall = request.getHeaders("amz-sdk-invocation-id").length > 0; - // AWS calls are often signed, so we can't add headers without breaking the signature. - if (!awsClientCall) { - OpenTelemetry.getPropagators().getHttpTextFormat().inject(context, request, SETTER); - } + OpenTelemetry.getPropagators().getHttpTextFormat().inject(context, request, SETTER); final Scope scope = withScopedContext(context); return new SpanWithScope(span, scope); diff --git a/instrumentation/aws-sdk/aws-sdk-2.2/src/main/java8/io/opentelemetry/auto/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java b/instrumentation/aws-sdk/aws-sdk-2.2/src/main/java8/io/opentelemetry/auto/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java index 71c639bcce31..ca3b0a301602 100644 --- a/instrumentation/aws-sdk/aws-sdk-2.2/src/main/java8/io/opentelemetry/auto/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java +++ b/instrumentation/aws-sdk/aws-sdk-2.2/src/main/java8/io/opentelemetry/auto/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java @@ -20,7 +20,6 @@ import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.awssdk.v2_2.AwsSdk; import io.opentelemetry.trace.Span; -import io.opentelemetry.trace.Span.Kind; import java.io.InputStream; import java.nio.ByteBuffer; import java.util.Optional; @@ -67,8 +66,7 @@ public static class ScopeHolder { OVERRIDE_CONFIGURATION_CONSUMER = builder -> builder.addExecutionInterceptor( - // Agent will trace HTTP calls too so use INTERNAL kind. - new TracingExecutionInterceptor(AwsSdk.newInterceptor(Kind.INTERNAL))); + new TracingExecutionInterceptor(AwsSdk.newInterceptor())); private final ExecutionInterceptor delegate; diff --git a/instrumentation/aws-sdk/aws-sdk-2.2/src/test/groovy/Aws2ClientTest.groovy b/instrumentation/aws-sdk/aws-sdk-2.2/src/test/groovy/Aws2ClientTest.groovy index b02b21348a55..0275215aaca8 100644 --- a/instrumentation/aws-sdk/aws-sdk-2.2/src/test/groovy/Aws2ClientTest.groovy +++ b/instrumentation/aws-sdk/aws-sdk-2.2/src/test/groovy/Aws2ClientTest.groovy @@ -51,7 +51,6 @@ import java.util.concurrent.atomic.AtomicReference import static io.opentelemetry.auto.test.server.http.TestHttpServer.httpServer import static io.opentelemetry.trace.Span.Kind.CLIENT -import static io.opentelemetry.trace.Span.Kind.INTERNAL class Aws2ClientTest extends AgentTestRunner { @@ -90,10 +89,10 @@ class Aws2ClientTest extends AgentTestRunner { response.class.simpleName.startsWith(operation) || response instanceof ResponseInputStream assertTraces(1) { - trace(0, 2) { + trace(0, 1) { span(0) { operationName "$service.$operation" - spanKind INTERNAL + spanKind CLIENT errored false parent() tags { @@ -119,19 +118,6 @@ class Aws2ClientTest extends AgentTestRunner { } } } - span(1) { - operationName expectedOperationName(method) - spanKind CLIENT - errored false - childOf span(0) - tags { - "$MoreTags.NET_PEER_NAME" "localhost" - "$MoreTags.NET_PEER_PORT" server.address.port - "$Tags.HTTP_URL" { it.startsWith("${server.address}${path}") } - "$Tags.HTTP_METHOD" "$method" - "$Tags.HTTP_STATUS" 200 - } - } } } server.lastRequest.headers.get("traceparent") == null @@ -193,7 +179,7 @@ class Aws2ClientTest extends AgentTestRunner { trace(0, 1) { span(0) { operationName "$service.$operation" - spanKind INTERNAL + spanKind CLIENT errored false parent() tags { @@ -301,10 +287,10 @@ class Aws2ClientTest extends AgentTestRunner { thrown SdkClientException assertTraces(1) { - trace(0, 5) { + trace(0, 1) { span(0) { operationName "S3.GetObject" - spanKind INTERNAL + spanKind CLIENT errored true parent() tags { @@ -319,21 +305,6 @@ class Aws2ClientTest extends AgentTestRunner { errorTags SdkClientException, "Unable to execute HTTP request: Read timed out" } } - (1..4).each { - span(it) { - operationName expectedOperationName("GET") - spanKind CLIENT - errored true - childOf span(0) - tags { - "$MoreTags.NET_PEER_NAME" "localhost" - "$MoreTags.NET_PEER_PORT" server.address.port - "$Tags.HTTP_URL" "$server.address/somebucket/somekey" - "$Tags.HTTP_METHOD" "GET" - errorTags SocketTimeoutException, "Read timed out" - } - } - } } } From c2ee0436f8185534bdb6b17b7b611f540c99d19c Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Sun, 31 May 2020 15:09:08 +0900 Subject: [PATCH 2/7] Retry TODO --- .../aws-sdk/aws-sdk-2.2/src/test/groovy/Aws2ClientTest.groovy | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/instrumentation/aws-sdk/aws-sdk-2.2/src/test/groovy/Aws2ClientTest.groovy b/instrumentation/aws-sdk/aws-sdk-2.2/src/test/groovy/Aws2ClientTest.groovy index 0275215aaca8..81e34fbd6c37 100644 --- a/instrumentation/aws-sdk/aws-sdk-2.2/src/test/groovy/Aws2ClientTest.groovy +++ b/instrumentation/aws-sdk/aws-sdk-2.2/src/test/groovy/Aws2ClientTest.groovy @@ -263,7 +263,8 @@ class Aws2ClientTest extends AgentTestRunner { """ } - def "timeout and retry errors captured"() { + // TODO(anuraaga): Add events for retries. + def "timeout and retry errors not captured"() { setup: def server = httpServer { handlers { From d57444707cad6ea1aa1d0370d03e5654784b7ced Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Tue, 2 Jun 2020 17:10:20 +0900 Subject: [PATCH 3/7] Store subtree client span in context. --- .../decorator/ClientDecorator.java | 41 +++++++++++++------ .../v4_0/ApacheHttpClientInstrumentation.java | 9 ++-- .../v2_2/TracingExecutionInterceptor.java | 5 +-- 3 files changed, 37 insertions(+), 18 deletions(-) diff --git a/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecorator.java b/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecorator.java index 038a2ec9bf43..a1b357d00fc6 100644 --- a/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecorator.java +++ b/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecorator.java @@ -15,32 +15,49 @@ */ package io.opentelemetry.auto.bootstrap.instrumentation.decorator; -import static io.opentelemetry.auto.bootstrap.WeakMap.Provider.newWeakMap; - -import io.opentelemetry.auto.bootstrap.WeakMap; +import io.grpc.Context; +import io.opentelemetry.context.ContextUtils; +import io.opentelemetry.context.Scope; import io.opentelemetry.trace.DefaultSpan; import io.opentelemetry.trace.Span; import io.opentelemetry.trace.Span.Kind; import io.opentelemetry.trace.Tracer; +import io.opentelemetry.trace.TracingContextUtils; public abstract class ClientDecorator extends BaseDecorator { - // Work around the fact that we cannot read the kind of currentSpan by keeping track of the spans - // we created. - private static final WeakMap CLIENT_SPANS = newWeakMap(); + // Keeps track of the client span in a subtree corresponding to a client request. + private static final Context.Key CONTEXT_CLIENT_SPAN_KEY = + Context.key("opentelemetry-trace-auto-client-span-key"); + + public static Scope currentContextWith(final Span clientSpan) { + if (!clientSpan.getContext().isValid()) { + return TracingContextUtils.currentContextWith(clientSpan); + } + return ContextUtils.withScopedContext( + TracingContextUtils.withSpan( + clientSpan, Context.current().withValue(CONTEXT_CLIENT_SPAN_KEY, clientSpan))); + } + + public static Context withSpan(final Span clientSpan, final Context context) { + if (!clientSpan.getContext().isValid()) { + return TracingContextUtils.withSpan(clientSpan, context); + } + return TracingContextUtils.withSpan( + clientSpan, Context.current().withValue(CONTEXT_CLIENT_SPAN_KEY, clientSpan)); + } public Span getOrCreateSpan(String name, Tracer tracer) { - final Span current = tracer.getCurrentSpan(); + final Context context = Context.current(); + final Span clientSpan = CONTEXT_CLIENT_SPAN_KEY.get(context); - if (Boolean.TRUE.equals(CLIENT_SPANS.get(current))) { + if (clientSpan != null) { // We don't want to create two client spans for a given client call, suppress inner spans. return DefaultSpan.getInvalid(); } - final Span clientSpan = - tracer.spanBuilder(name).setSpanKind(Kind.CLIENT).setParent(current).startSpan(); - CLIENT_SPANS.put(clientSpan, true); - return clientSpan; + final Span current = TracingContextUtils.getSpan(context); + return tracer.spanBuilder(name).setSpanKind(Kind.CLIENT).setParent(current).startSpan(); } @Override diff --git a/instrumentation/apache-httpclient/apache-httpclient-4.0/src/main/java/io/opentelemetry/auto/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-4.0/src/main/java/io/opentelemetry/auto/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java index fee8d968269d..d289174180b3 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-4.0/src/main/java/io/opentelemetry/auto/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java +++ b/instrumentation/apache-httpclient/apache-httpclient-4.0/src/main/java/io/opentelemetry/auto/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java @@ -21,7 +21,6 @@ import static io.opentelemetry.auto.tooling.ClassLoaderMatcher.hasClassesNamed; import static io.opentelemetry.auto.tooling.bytebuddy.matcher.AgentElementMatchers.implementsInterface; import static io.opentelemetry.context.ContextUtils.withScopedContext; -import static io.opentelemetry.trace.TracingContextUtils.withSpan; import static net.bytebuddy.matcher.ElementMatchers.isAbstract; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.named; @@ -33,6 +32,7 @@ import io.grpc.Context; import io.opentelemetry.OpenTelemetry; import io.opentelemetry.auto.bootstrap.CallDepthThreadLocalMap; +import io.opentelemetry.auto.bootstrap.instrumentation.decorator.ClientDecorator; import io.opentelemetry.auto.instrumentation.api.SpanWithScope; import io.opentelemetry.auto.tooling.Instrumenter; import io.opentelemetry.context.Scope; @@ -176,8 +176,11 @@ public static SpanWithScope doMethodEnter(final HttpUriRequest request) { DECORATE.afterStart(span); DECORATE.onRequest(span, request); - final Context context = withSpan(span, Context.current()); - OpenTelemetry.getPropagators().getHttpTextFormat().inject(context, request, SETTER); + final Context context = ClientDecorator.withSpan(span, Context.current()); + // TODO(anuraaga): Seems like a bug that invalid context still gets injected by the injector. + if (span.getContext().isValid()) { + OpenTelemetry.getPropagators().getHttpTextFormat().inject(context, request, SETTER); + } final Scope scope = withScopedContext(context); return new SpanWithScope(span, scope); diff --git a/instrumentation/aws-sdk/aws-sdk-2.2/src/main/java8/io/opentelemetry/auto/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java b/instrumentation/aws-sdk/aws-sdk-2.2/src/main/java8/io/opentelemetry/auto/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java index ca3b0a301602..32bcc8e207a7 100644 --- a/instrumentation/aws-sdk/aws-sdk-2.2/src/main/java8/io/opentelemetry/auto/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java +++ b/instrumentation/aws-sdk/aws-sdk-2.2/src/main/java8/io/opentelemetry/auto/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java @@ -15,8 +15,7 @@ */ package io.opentelemetry.auto.instrumentation.awssdk.v2_2; -import static io.opentelemetry.trace.TracingContextUtils.currentContextWith; - +import io.opentelemetry.auto.bootstrap.instrumentation.decorator.ClientDecorator; import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.awssdk.v2_2.AwsSdk; import io.opentelemetry.trace.Span; @@ -133,7 +132,7 @@ public void beforeTransmission( if (span != null) { // This scope will be closed by AwsHttpClientInstrumentation since ExecutionInterceptor API // doesn't provide a way to run code in the same thread after transmission has been scheduled. - ScopeHolder.CURRENT.set(currentContextWith(span)); + ScopeHolder.CURRENT.set(ClientDecorator.currentContextWith(span)); } } From 76b32219cdbca9385344d4d07a2d1e7566b423af Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Fri, 5 Jun 2020 17:41:23 +0900 Subject: [PATCH 4/7] Apply new pattern to AWS V1 SDK instrumentation too, cleanup, and javadoc --- .../decorator/ClientDecorator.java | 19 +++-- .../awssdk/v1_11/TracingRequestHandler.java | 7 +- .../src/test/groovy/AWS1ClientTest.groovy | 69 +++---------------- .../groovy/AWS0ClientTest.groovy | 69 +++---------------- 4 files changed, 35 insertions(+), 129 deletions(-) diff --git a/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecorator.java b/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecorator.java index a1b357d00fc6..4ad22913a5cd 100644 --- a/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecorator.java +++ b/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecorator.java @@ -30,15 +30,18 @@ public abstract class ClientDecorator extends BaseDecorator { private static final Context.Key CONTEXT_CLIENT_SPAN_KEY = Context.key("opentelemetry-trace-auto-client-span-key"); + /** + * Creates a {@link Context} with the provided {@link Span} and sets it as the current {@link + * Context} + * + * @return a {@link Scope} that will restore the previous context. All callers of this method must + * also call {@link Scope#close()} when this next context is no longer needed. + */ public static Scope currentContextWith(final Span clientSpan) { - if (!clientSpan.getContext().isValid()) { - return TracingContextUtils.currentContextWith(clientSpan); - } - return ContextUtils.withScopedContext( - TracingContextUtils.withSpan( - clientSpan, Context.current().withValue(CONTEXT_CLIENT_SPAN_KEY, clientSpan))); + return ContextUtils.withScopedContext(withSpan(clientSpan, Context.current())); } + /** Returns a new {@link Context} forked from {@code context} with the {@link Span} set. */ public static Context withSpan(final Span clientSpan, final Context context) { if (!clientSpan.getContext().isValid()) { return TracingContextUtils.withSpan(clientSpan, context); @@ -47,6 +50,10 @@ public static Context withSpan(final Span clientSpan, final Context context) { clientSpan, Context.current().withValue(CONTEXT_CLIENT_SPAN_KEY, clientSpan)); } + /** + * Returns a new client {@link Span} if there is no client {@link Span} in the current {@link + * Context}, or an invalid {@link Span} otherwise. + */ public Span getOrCreateSpan(String name, Tracer tracer) { final Context context = Context.current(); final Span clientSpan = CONTEXT_CLIENT_SPAN_KEY.get(context); diff --git a/instrumentation/aws-sdk/aws-sdk-1.11/src/main/java/io/opentelemetry/auto/instrumentation/awssdk/v1_11/TracingRequestHandler.java b/instrumentation/aws-sdk/aws-sdk-1.11/src/main/java/io/opentelemetry/auto/instrumentation/awssdk/v1_11/TracingRequestHandler.java index c3f392a17b0b..00281ff9bad5 100644 --- a/instrumentation/aws-sdk/aws-sdk-1.11/src/main/java/io/opentelemetry/auto/instrumentation/awssdk/v1_11/TracingRequestHandler.java +++ b/instrumentation/aws-sdk/aws-sdk-1.11/src/main/java/io/opentelemetry/auto/instrumentation/awssdk/v1_11/TracingRequestHandler.java @@ -16,7 +16,6 @@ package io.opentelemetry.auto.instrumentation.awssdk.v1_11; import static io.opentelemetry.auto.instrumentation.awssdk.v1_11.RequestMeta.SPAN_SCOPE_PAIR_CONTEXT_KEY; -import static io.opentelemetry.trace.TracingContextUtils.currentContextWith; import com.amazonaws.AmazonWebServiceRequest; import com.amazonaws.Request; @@ -24,6 +23,7 @@ import com.amazonaws.handlers.RequestHandler2; import io.opentelemetry.OpenTelemetry; import io.opentelemetry.auto.bootstrap.ContextStore; +import io.opentelemetry.auto.bootstrap.instrumentation.decorator.ClientDecorator; import io.opentelemetry.auto.instrumentation.api.SpanWithScope; import io.opentelemetry.trace.Span; import io.opentelemetry.trace.Tracer; @@ -47,11 +47,12 @@ public AmazonWebServiceRequest beforeMarshalling(final AmazonWebServiceRequest r @Override public void beforeRequest(final Request request) { - final Span span = TRACER.spanBuilder(decorate.spanNameForRequest(request)).startSpan(); + final Span span = decorate.getOrCreateSpan(request, TRACER); decorate.afterStart(span); decorate.onRequest(span, request); request.addHandlerContext( - SPAN_SCOPE_PAIR_CONTEXT_KEY, new SpanWithScope(span, currentContextWith(span))); + SPAN_SCOPE_PAIR_CONTEXT_KEY, + new SpanWithScope(span, ClientDecorator.currentContextWith(span))); } @Override diff --git a/instrumentation/aws-sdk/aws-sdk-1.11/src/test/groovy/AWS1ClientTest.groovy b/instrumentation/aws-sdk/aws-sdk-1.11/src/test/groovy/AWS1ClientTest.groovy index ce2cb6082469..bcd2968ada9e 100644 --- a/instrumentation/aws-sdk/aws-sdk-1.11/src/test/groovy/AWS1ClientTest.groovy +++ b/instrumentation/aws-sdk/aws-sdk-1.11/src/test/groovy/AWS1ClientTest.groovy @@ -47,8 +47,6 @@ import io.opentelemetry.auto.bootstrap.instrumentation.decorator.HttpClientDecor import io.opentelemetry.auto.instrumentation.api.MoreTags import io.opentelemetry.auto.instrumentation.api.Tags import io.opentelemetry.auto.test.AgentTestRunner -import org.apache.http.conn.HttpHostConnectException -import org.apache.http.impl.execchain.RequestAbortedException import spock.lang.AutoCleanup import spock.lang.Shared @@ -57,7 +55,6 @@ import java.util.concurrent.atomic.AtomicReference import static io.opentelemetry.auto.test.server.http.TestHttpServer.httpServer import static io.opentelemetry.auto.test.utils.PortUtils.UNUSABLE_PORT import static io.opentelemetry.trace.Span.Kind.CLIENT -import static io.opentelemetry.trace.Span.Kind.INTERNAL class AWS1ClientTest extends AgentTestRunner { @@ -149,10 +146,10 @@ class AWS1ClientTest extends AgentTestRunner { client.requestHandler2s.get(0).getClass().getSimpleName() == "TracingRequestHandler" assertTraces(1) { - trace(0, 2) { + trace(0, 1) { span(0) { operationName "$service.$operation" - spanKind INTERNAL + spanKind CLIENT errored false parent() tags { @@ -170,19 +167,6 @@ class AWS1ClientTest extends AgentTestRunner { } } } - span(1) { - operationName expectedOperationName(method) - spanKind CLIENT - errored false - childOf span(0) - tags { - "$MoreTags.NET_PEER_NAME" "localhost" - "$MoreTags.NET_PEER_PORT" server.address.port - "$Tags.HTTP_URL" "${server.address}${path}" - "$Tags.HTTP_METHOD" "$method" - "$Tags.HTTP_STATUS" 200 - } - } } } server.lastRequest.headers.get("traceparent") == null @@ -236,10 +220,10 @@ class AWS1ClientTest extends AgentTestRunner { thrown SdkClientException assertTraces(1) { - trace(0, 2) { + trace(0, 1) { span(0) { operationName "$service.$operation" - spanKind INTERNAL + spanKind CLIENT errored true parent() tags { @@ -257,19 +241,6 @@ class AWS1ClientTest extends AgentTestRunner { errorTags SdkClientException, ~/Unable to execute HTTP request/ } } - span(1) { - operationName expectedOperationName(method) - spanKind CLIENT - errored true - childOf span(0) - tags { - "$MoreTags.NET_PEER_NAME" "localhost" - "$MoreTags.NET_PEER_PORT" UNUSABLE_PORT - "$Tags.HTTP_URL" "http://localhost:${UNUSABLE_PORT}/$url" - "$Tags.HTTP_METHOD" "$method" - errorTags HttpHostConnectException, ~/Connection refused/ - } - } } } @@ -298,7 +269,7 @@ class AWS1ClientTest extends AgentTestRunner { trace(0, 1) { span(0) { operationName "S3.HeadBucket" - spanKind INTERNAL + spanKind CLIENT errored true parent() tags { @@ -316,7 +287,8 @@ class AWS1ClientTest extends AgentTestRunner { } } - def "timeout and retry errors captured"() { + // TODO(anuraaga): Add events for retries. + def "timeout and retry errors not captured"() { setup: def server = httpServer { handlers { @@ -337,10 +309,10 @@ class AWS1ClientTest extends AgentTestRunner { thrown AmazonClientException assertTraces(1) { - trace(0, 5) { + trace(0, 1) { span(0) { operationName "S3.GetObject" - spanKind INTERNAL + spanKind CLIENT errored true parent() tags { @@ -360,29 +332,6 @@ class AWS1ClientTest extends AgentTestRunner { } } } - (1..4).each { - span(it) { - operationName expectedOperationName("GET") - spanKind CLIENT - errored true - childOf span(0) - tags { - "$MoreTags.NET_PEER_NAME" "localhost" - "$MoreTags.NET_PEER_PORT" server.address.port - "$Tags.HTTP_URL" "$server.address/someBucket/someKey" - "$Tags.HTTP_METHOD" "GET" - try { - errorTags SocketException, "Socket closed" - } catch (AssertionError e) { - try { - errorTags SocketException, "Socket Closed" // windows - } catch (AssertionError f) { - errorTags RequestAbortedException, "Request aborted" - } - } - } - } - } } } diff --git a/instrumentation/aws-sdk/aws-sdk-1.11/src/test_before_1_11_106/groovy/AWS0ClientTest.groovy b/instrumentation/aws-sdk/aws-sdk-1.11/src/test_before_1_11_106/groovy/AWS0ClientTest.groovy index 248155bdbc85..13d100856e19 100644 --- a/instrumentation/aws-sdk/aws-sdk-1.11/src/test_before_1_11_106/groovy/AWS0ClientTest.groovy +++ b/instrumentation/aws-sdk/aws-sdk-1.11/src/test_before_1_11_106/groovy/AWS0ClientTest.groovy @@ -34,8 +34,6 @@ import io.opentelemetry.auto.bootstrap.instrumentation.decorator.HttpClientDecor import io.opentelemetry.auto.instrumentation.api.MoreTags import io.opentelemetry.auto.instrumentation.api.Tags import io.opentelemetry.auto.test.AgentTestRunner -import org.apache.http.conn.HttpHostConnectException -import org.apache.http.impl.execchain.RequestAbortedException import spock.lang.AutoCleanup import spock.lang.Shared @@ -44,7 +42,6 @@ import java.util.concurrent.atomic.AtomicReference import static io.opentelemetry.auto.test.server.http.TestHttpServer.httpServer import static io.opentelemetry.auto.test.utils.PortUtils.UNUSABLE_PORT import static io.opentelemetry.trace.Span.Kind.CLIENT -import static io.opentelemetry.trace.Span.Kind.INTERNAL class AWS0ClientTest extends AgentTestRunner { @@ -112,10 +109,10 @@ class AWS0ClientTest extends AgentTestRunner { client.requestHandler2s.get(0).getClass().getSimpleName() == "TracingRequestHandler" assertTraces(1) { - trace(0, 2) { + trace(0, 1) { span(0) { operationName "$service.$operation" - spanKind INTERNAL + spanKind CLIENT errored false parent() tags { @@ -133,19 +130,6 @@ class AWS0ClientTest extends AgentTestRunner { } } } - span(1) { - operationName expectedOperationName(method) - spanKind CLIENT - errored false - childOf span(0) - tags { - "$MoreTags.NET_PEER_NAME" "localhost" - "$MoreTags.NET_PEER_PORT" server.address.port - "$Tags.HTTP_URL" "${server.address}${path}" - "$Tags.HTTP_METHOD" "$method" - "$Tags.HTTP_STATUS" 200 - } - } } } server.lastRequest.headers.get("traceparent") == null @@ -181,10 +165,10 @@ class AWS0ClientTest extends AgentTestRunner { thrown AmazonClientException assertTraces(1) { - trace(0, 2) { + trace(0, 1) { span(0) { operationName "$service.$operation" - spanKind INTERNAL + spanKind CLIENT errored true parent() tags { @@ -202,19 +186,6 @@ class AWS0ClientTest extends AgentTestRunner { errorTags AmazonClientException, ~/Unable to execute HTTP request/ } } - span(1) { - operationName expectedOperationName(method) - spanKind CLIENT - errored true - childOf span(0) - tags { - "$MoreTags.NET_PEER_NAME" "localhost" - "$MoreTags.NET_PEER_PORT" UNUSABLE_PORT - "$Tags.HTTP_URL" "http://localhost:${UNUSABLE_PORT}/$url" - "$Tags.HTTP_METHOD" "$method" - errorTags HttpHostConnectException, ~/Connection refused/ - } - } } } @@ -243,7 +214,7 @@ class AWS0ClientTest extends AgentTestRunner { trace(0, 1) { span(0) { operationName "S3.GetObject" - spanKind INTERNAL + spanKind CLIENT errored true parent() tags { @@ -262,7 +233,8 @@ class AWS0ClientTest extends AgentTestRunner { } } - def "timeout and retry errors captured"() { + // TODO(anuraaga): Add events for retries. + def "timeout and retry errors not captured"() { setup: def server = httpServer { handlers { @@ -283,10 +255,10 @@ class AWS0ClientTest extends AgentTestRunner { thrown AmazonClientException assertTraces(1) { - trace(0, 5) { + trace(0, 1) { span(0) { operationName "S3.GetObject" - spanKind INTERNAL + spanKind CLIENT errored true parent() tags { @@ -302,29 +274,6 @@ class AWS0ClientTest extends AgentTestRunner { errorTags AmazonClientException, ~/Unable to execute HTTP request/ } } - (1..4).each { - span(it) { - operationName expectedOperationName("GET") - spanKind CLIENT - errored true - childOf span(0) - tags { - "$MoreTags.NET_PEER_NAME" "localhost" - "$MoreTags.NET_PEER_PORT" server.address.port - "$Tags.HTTP_URL" "$server.address/someBucket/someKey" - "$Tags.HTTP_METHOD" "GET" - try { - errorTags SocketException, "Socket closed" - } catch (AssertionError e) { - try { - errorTags SocketException, "Socket Closed" // windows - } catch (AssertionError f) { - errorTags RequestAbortedException, "Request aborted" - } - } - } - } - } } } From 95d8115c353b7ffdd8fb4d4746de96d48e29fc37 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Mon, 8 Jun 2020 13:19:08 +0900 Subject: [PATCH 5/7] Update test --- .../aws-sdk/aws-sdk-2.2/src/test/groovy/Aws2ClientTest.groovy | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/instrumentation/aws-sdk/aws-sdk-2.2/src/test/groovy/Aws2ClientTest.groovy b/instrumentation/aws-sdk/aws-sdk-2.2/src/test/groovy/Aws2ClientTest.groovy index 0ab216e10d3b..b6edf375a919 100644 --- a/instrumentation/aws-sdk/aws-sdk-2.2/src/test/groovy/Aws2ClientTest.groovy +++ b/instrumentation/aws-sdk/aws-sdk-2.2/src/test/groovy/Aws2ClientTest.groovy @@ -280,9 +280,8 @@ class Aws2ClientTest extends AgentTestRunner { then: assertTraces(1) { - trace(0, 2) { + trace(0, 1) { span(0) {} - span(1) {} } } server.lastRequest.headers.get("x-name") == "value" From 9051e500537a0ff117ae71141c2e62057c8ff6c0 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Mon, 8 Jun 2020 13:37:48 +0900 Subject: [PATCH 6/7] Unit tests --- .../decorator/ClientDecorator.java | 7 +- .../decorator/ClientDecoratorTest.groovy | 75 +++++++++++++++++++ .../v2_2/TracingExecutionInterceptor.java | 3 +- 3 files changed, 81 insertions(+), 4 deletions(-) diff --git a/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecorator.java b/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecorator.java index 4ad22913a5cd..bdca8deea26f 100644 --- a/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecorator.java +++ b/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecorator.java @@ -27,7 +27,8 @@ public abstract class ClientDecorator extends BaseDecorator { // Keeps track of the client span in a subtree corresponding to a client request. - private static final Context.Key CONTEXT_CLIENT_SPAN_KEY = + // Visible for testing + static final Context.Key CONTEXT_CLIENT_SPAN_KEY = Context.key("opentelemetry-trace-auto-client-span-key"); /** @@ -47,14 +48,14 @@ public static Context withSpan(final Span clientSpan, final Context context) { return TracingContextUtils.withSpan(clientSpan, context); } return TracingContextUtils.withSpan( - clientSpan, Context.current().withValue(CONTEXT_CLIENT_SPAN_KEY, clientSpan)); + clientSpan, context.withValue(CONTEXT_CLIENT_SPAN_KEY, clientSpan)); } /** * Returns a new client {@link Span} if there is no client {@link Span} in the current {@link * Context}, or an invalid {@link Span} otherwise. */ - public Span getOrCreateSpan(String name, Tracer tracer) { + public static Span getOrCreateSpan(String name, Tracer tracer) { final Context context = Context.current(); final Span clientSpan = CONTEXT_CLIENT_SPAN_KEY.get(context); diff --git a/agent-bootstrap/src/test/groovy/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecoratorTest.groovy b/agent-bootstrap/src/test/groovy/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecoratorTest.groovy index b319276845d4..3758e5b9c925 100644 --- a/agent-bootstrap/src/test/groovy/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecoratorTest.groovy +++ b/agent-bootstrap/src/test/groovy/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecoratorTest.groovy @@ -15,10 +15,16 @@ */ package io.opentelemetry.auto.bootstrap.instrumentation.decorator +import io.grpc.Context +import io.opentelemetry.OpenTelemetry import io.opentelemetry.trace.Span +import io.opentelemetry.trace.Tracer +import io.opentelemetry.trace.TracingContextUtils class ClientDecoratorTest extends BaseDecoratorTest { + private static final Tracer TRACER = OpenTelemetry.getTracerProvider().get("io.opentelemetry.auto") + def span = Mock(Span) def "test afterStart"() { @@ -44,6 +50,75 @@ class ClientDecoratorTest extends BaseDecoratorTest { 0 * _ } + def "test getOrCreateSpan when no existing client span"() { + when: + def span = ClientDecorator.getOrCreateSpan("test", TRACER) + + then: + assert span.getContext().isValid() + } + + def "test getOrCreateSpan when existing client span"() { + setup: + def existing = ClientDecorator.getOrCreateSpan("existing", TRACER) + def scope = ClientDecorator.currentContextWith(existing) + + when: + def span = ClientDecorator.getOrCreateSpan("test", TRACER) + + then: + assert !span.getContext().isValid() + + cleanup: + scope.close() + } + + def "test getOrCreateSpan internal after client span"() { + setup: + def client = ClientDecorator.getOrCreateSpan("existing", TRACER) + def scope = ClientDecorator.currentContextWith(client) + + when: + def internal = TRACER.spanBuilder("internal").setSpanKind(Span.Kind.INTERNAL).startSpan() + def scope2 = TracingContextUtils.currentContextWith(internal) + + then: + assert internal.getContext().isValid() + assert ClientDecorator.CONTEXT_CLIENT_SPAN_KEY.get(Context.current()) == client + assert TracingContextUtils.getSpan(Context.current()) == internal + + cleanup: + scope2.close() + scope.close() + } + + def "test withSpan"() { + setup: + def span = ClientDecorator.getOrCreateSpan("test", TRACER) + + when: + def context = ClientDecorator.withSpan(span, Context.ROOT) + + then: + assert ClientDecorator.CONTEXT_CLIENT_SPAN_KEY.get(context) == span + assert TracingContextUtils.getSpan(context) == span + } + + def "test currentContextWith"() { + setup: + def span = ClientDecorator.getOrCreateSpan("test", TRACER) + + when: + def scope = ClientDecorator.currentContextWith(span) + + then: + assert ClientDecorator.CONTEXT_CLIENT_SPAN_KEY.get(Context.current()) == span + assert TracingContextUtils.getSpan(Context.current()) == span + + cleanup: + scope.close() + } + @Override def newDecorator() { return newDecorator("test-service") diff --git a/instrumentation-core/aws-sdk/aws-sdk-2.2-core/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java b/instrumentation-core/aws-sdk/aws-sdk-2.2-core/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java index 2b7ec1fcd913..25c90504ad52 100644 --- a/instrumentation-core/aws-sdk/aws-sdk-2.2-core/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java +++ b/instrumentation-core/aws-sdk/aws-sdk-2.2-core/src/main/java/io/opentelemetry/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java @@ -17,6 +17,7 @@ import static io.opentelemetry.instrumentation.awssdk.v2_2.AwsSdkClientDecorator.DECORATE; +import io.opentelemetry.auto.bootstrap.instrumentation.decorator.ClientDecorator; import io.opentelemetry.trace.Span; import io.opentelemetry.trace.Span.Kind; import software.amazon.awssdk.core.interceptor.Context; @@ -40,7 +41,7 @@ final class TracingExecutionInterceptor implements ExecutionInterceptor { public void beforeExecution( final Context.BeforeExecution context, final ExecutionAttributes executionAttributes) { final Span span = - DECORATE.getOrCreateSpan(DECORATE.spanName(executionAttributes), AwsSdk.tracer()); + ClientDecorator.getOrCreateSpan(DECORATE.spanName(executionAttributes), AwsSdk.tracer()); DECORATE.afterStart(span); executionAttributes.putAttribute(SPAN_ATTRIBUTE, span); } From 6f07eb92a7e53492f697b3be574c6b4d6f7267cc Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Mon, 8 Jun 2020 17:07:51 +0900 Subject: [PATCH 7/7] Cleanups --- .../decorator/ClientDecorator.java | 24 ++++++------------- .../decorator/ClientDecoratorTest.groovy | 24 ++++--------------- .../v4_0/ApacheHttpClientInstrumentation.java | 3 +-- .../awssdk/v1_11/TracingRequestHandler.java | 4 +++- .../v2_2/TracingExecutionInterceptor.java | 4 +++- 5 files changed, 19 insertions(+), 40 deletions(-) diff --git a/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecorator.java b/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecorator.java index bdca8deea26f..098be6010462 100644 --- a/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecorator.java +++ b/agent-bootstrap/src/main/java/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecorator.java @@ -16,8 +16,6 @@ package io.opentelemetry.auto.bootstrap.instrumentation.decorator; import io.grpc.Context; -import io.opentelemetry.context.ContextUtils; -import io.opentelemetry.context.Scope; import io.opentelemetry.trace.DefaultSpan; import io.opentelemetry.trace.Span; import io.opentelemetry.trace.Span.Kind; @@ -32,23 +30,15 @@ public abstract class ClientDecorator extends BaseDecorator { Context.key("opentelemetry-trace-auto-client-span-key"); /** - * Creates a {@link Context} with the provided {@link Span} and sets it as the current {@link - * Context} - * - * @return a {@link Scope} that will restore the previous context. All callers of this method must - * also call {@link Scope#close()} when this next context is no longer needed. + * Returns a new {@link Context} forked from the {@linkplain Context#current()} current context} + * with the {@link Span} set. */ - public static Scope currentContextWith(final Span clientSpan) { - return ContextUtils.withScopedContext(withSpan(clientSpan, Context.current())); - } - - /** Returns a new {@link Context} forked from {@code context} with the {@link Span} set. */ - public static Context withSpan(final Span clientSpan, final Context context) { - if (!clientSpan.getContext().isValid()) { - return TracingContextUtils.withSpan(clientSpan, context); + public static Context currentContextWith(final Span clientSpan) { + Context context = Context.current(); + if (clientSpan.getContext().isValid()) { + context = context.withValue(CONTEXT_CLIENT_SPAN_KEY, clientSpan); } - return TracingContextUtils.withSpan( - clientSpan, context.withValue(CONTEXT_CLIENT_SPAN_KEY, clientSpan)); + return TracingContextUtils.withSpan(clientSpan, context); } /** diff --git a/agent-bootstrap/src/test/groovy/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecoratorTest.groovy b/agent-bootstrap/src/test/groovy/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecoratorTest.groovy index 3758e5b9c925..504d448c73d0 100644 --- a/agent-bootstrap/src/test/groovy/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecoratorTest.groovy +++ b/agent-bootstrap/src/test/groovy/io/opentelemetry/auto/bootstrap/instrumentation/decorator/ClientDecoratorTest.groovy @@ -17,6 +17,7 @@ package io.opentelemetry.auto.bootstrap.instrumentation.decorator import io.grpc.Context import io.opentelemetry.OpenTelemetry +import io.opentelemetry.context.ContextUtils import io.opentelemetry.trace.Span import io.opentelemetry.trace.Tracer import io.opentelemetry.trace.TracingContextUtils @@ -61,7 +62,7 @@ class ClientDecoratorTest extends BaseDecoratorTest { def "test getOrCreateSpan when existing client span"() { setup: def existing = ClientDecorator.getOrCreateSpan("existing", TRACER) - def scope = ClientDecorator.currentContextWith(existing) + def scope = ContextUtils.withScopedContext(ClientDecorator.currentContextWith(existing)) when: def span = ClientDecorator.getOrCreateSpan("test", TRACER) @@ -76,7 +77,7 @@ class ClientDecoratorTest extends BaseDecoratorTest { def "test getOrCreateSpan internal after client span"() { setup: def client = ClientDecorator.getOrCreateSpan("existing", TRACER) - def scope = ClientDecorator.currentContextWith(client) + def scope = ContextUtils.withScopedContext(ClientDecorator.currentContextWith(client)) when: def internal = TRACER.spanBuilder("internal").setSpanKind(Span.Kind.INTERNAL).startSpan() @@ -92,33 +93,18 @@ class ClientDecoratorTest extends BaseDecoratorTest { scope.close() } - def "test withSpan"() { + def "test currentContextWith"() { setup: def span = ClientDecorator.getOrCreateSpan("test", TRACER) when: - def context = ClientDecorator.withSpan(span, Context.ROOT) + def context = ClientDecorator.currentContextWith(span) then: assert ClientDecorator.CONTEXT_CLIENT_SPAN_KEY.get(context) == span assert TracingContextUtils.getSpan(context) == span } - def "test currentContextWith"() { - setup: - def span = ClientDecorator.getOrCreateSpan("test", TRACER) - - when: - def scope = ClientDecorator.currentContextWith(span) - - then: - assert ClientDecorator.CONTEXT_CLIENT_SPAN_KEY.get(Context.current()) == span - assert TracingContextUtils.getSpan(Context.current()) == span - - cleanup: - scope.close() - } - @Override def newDecorator() { return newDecorator("test-service") diff --git a/instrumentation/apache-httpclient/apache-httpclient-4.0/src/main/java/io/opentelemetry/auto/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-4.0/src/main/java/io/opentelemetry/auto/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java index d289174180b3..1041348e15f7 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-4.0/src/main/java/io/opentelemetry/auto/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java +++ b/instrumentation/apache-httpclient/apache-httpclient-4.0/src/main/java/io/opentelemetry/auto/instrumentation/apachehttpclient/v4_0/ApacheHttpClientInstrumentation.java @@ -176,8 +176,7 @@ public static SpanWithScope doMethodEnter(final HttpUriRequest request) { DECORATE.afterStart(span); DECORATE.onRequest(span, request); - final Context context = ClientDecorator.withSpan(span, Context.current()); - // TODO(anuraaga): Seems like a bug that invalid context still gets injected by the injector. + final Context context = ClientDecorator.currentContextWith(span); if (span.getContext().isValid()) { OpenTelemetry.getPropagators().getHttpTextFormat().inject(context, request, SETTER); } diff --git a/instrumentation/aws-sdk/aws-sdk-1.11/src/main/java/io/opentelemetry/auto/instrumentation/awssdk/v1_11/TracingRequestHandler.java b/instrumentation/aws-sdk/aws-sdk-1.11/src/main/java/io/opentelemetry/auto/instrumentation/awssdk/v1_11/TracingRequestHandler.java index 00281ff9bad5..ff56a9b9b8a4 100644 --- a/instrumentation/aws-sdk/aws-sdk-1.11/src/main/java/io/opentelemetry/auto/instrumentation/awssdk/v1_11/TracingRequestHandler.java +++ b/instrumentation/aws-sdk/aws-sdk-1.11/src/main/java/io/opentelemetry/auto/instrumentation/awssdk/v1_11/TracingRequestHandler.java @@ -25,6 +25,7 @@ import io.opentelemetry.auto.bootstrap.ContextStore; import io.opentelemetry.auto.bootstrap.instrumentation.decorator.ClientDecorator; import io.opentelemetry.auto.instrumentation.api.SpanWithScope; +import io.opentelemetry.context.ContextUtils; import io.opentelemetry.trace.Span; import io.opentelemetry.trace.Tracer; @@ -52,7 +53,8 @@ public void beforeRequest(final Request request) { decorate.onRequest(span, request); request.addHandlerContext( SPAN_SCOPE_PAIR_CONTEXT_KEY, - new SpanWithScope(span, ClientDecorator.currentContextWith(span))); + new SpanWithScope( + span, ContextUtils.withScopedContext(ClientDecorator.currentContextWith(span)))); } @Override diff --git a/instrumentation/aws-sdk/aws-sdk-2.2/src/main/java8/io/opentelemetry/auto/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java b/instrumentation/aws-sdk/aws-sdk-2.2/src/main/java8/io/opentelemetry/auto/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java index 0ddb733d4e7d..9f08e5e07cfc 100644 --- a/instrumentation/aws-sdk/aws-sdk-2.2/src/main/java8/io/opentelemetry/auto/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java +++ b/instrumentation/aws-sdk/aws-sdk-2.2/src/main/java8/io/opentelemetry/auto/instrumentation/awssdk/v2_2/TracingExecutionInterceptor.java @@ -19,6 +19,7 @@ import io.opentelemetry.auto.bootstrap.WeakMap; import io.opentelemetry.auto.bootstrap.instrumentation.decorator.ClientDecorator; +import io.opentelemetry.context.ContextUtils; import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.awssdk.v2_2.AwsSdk; import io.opentelemetry.trace.Span; @@ -150,7 +151,8 @@ public void beforeTransmission( if (span != null) { // This scope will be closed by AwsHttpClientInstrumentation since ExecutionInterceptor API // doesn't provide a way to run code in the same thread after transmission has been scheduled. - ScopeHolder.CURRENT.set(ClientDecorator.currentContextWith(span)); + ScopeHolder.CURRENT.set( + ContextUtils.withScopedContext(ClientDecorator.currentContextWith(span))); } }