diff --git a/instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioAsyncInstrumentation.java b/instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioAsyncInstrumentation.java index 5415a5be4a..20828fe2e7 100644 --- a/instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioAsyncInstrumentation.java +++ b/instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioAsyncInstrumentation.java @@ -5,6 +5,7 @@ package io.opentelemetry.javaagent.instrumentation.twilio; +import static io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge.currentContext; import static io.opentelemetry.javaagent.instrumentation.twilio.TwilioTracer.tracer; import static io.opentelemetry.javaagent.tooling.ClassLoaderMatcher.hasClassesNamed; import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.extendsClass; @@ -21,9 +22,8 @@ import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.twilio.Twilio; -import io.opentelemetry.api.trace.Span; +import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; -import io.opentelemetry.javaagent.instrumentation.api.CallDepthThreadLocalMap; import io.opentelemetry.javaagent.tooling.TypeInstrumentation; import java.util.Map; import net.bytebuddy.asm.Advice; @@ -78,20 +78,15 @@ public class TwilioAsyncInstrumentation implements TypeInstrumentation { public static void methodEnter( @Advice.This Object that, @Advice.Origin("#m") String methodName, - @Advice.Local("otelSpan") Span span, + @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { - - // Ensure that we only create a span for the top-level Twilio client method; except in the - // case of async operations where we want visibility into how long the task was delayed from - // starting. Our call depth checker does not span threads, so the async case is handled - // automatically for us. - if (CallDepthThreadLocalMap.incrementCallDepth(Twilio.class) > 0) { + Context parentContext = currentContext(); + if (!tracer().shouldStartSpan(parentContext)) { return; } - // Don't automatically close the span with the scope if we're executing an async method - span = tracer().startSpan(that, methodName); - scope = tracer().startScope(span); + context = tracer().startSpan(parentContext, that, methodName); + scope = context.makeCurrent(); } /** Method exit instrumentation. */ @@ -99,24 +94,22 @@ public class TwilioAsyncInstrumentation implements TypeInstrumentation { public static void methodExit( @Advice.Thrown Throwable throwable, @Advice.Return ListenableFuture response, - @Advice.Local("otelSpan") Span span, + @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { if (scope == null) { return; } - CallDepthThreadLocalMap.reset(Twilio.class); - // span finished in SpanFinishingCallback scope.close(); if (throwable != null) { // There was an synchronous error, // which means we shouldn't wait for a callback to close the span. - tracer().endExceptionally(span, throwable); + tracer().endExceptionally(context, throwable); } else { // We're calling an async operation, we still need to finish the span when it's // complete and report the results; set an appropriate callback Futures.addCallback( - response, new SpanFinishingCallback<>(span), Twilio.getExecutorService()); + response, new SpanFinishingCallback<>(context), Twilio.getExecutorService()); } } } @@ -128,22 +121,20 @@ public class TwilioAsyncInstrumentation implements TypeInstrumentation { public static class SpanFinishingCallback implements FutureCallback { /** Span that we should finish and annotate when the future is complete. */ - private final Span span; + private final Context context; - public SpanFinishingCallback(Span span) { - this.span = span; + public SpanFinishingCallback(Context context) { + this.context = context; } @Override public void onSuccess(Object result) { - tracer().end(span, result); - span.end(); + tracer().end(context, result); } @Override public void onFailure(Throwable t) { - tracer().endExceptionally(span, t); - span.end(); + tracer().endExceptionally(context, t); } } } diff --git a/instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioSyncInstrumentation.java b/instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioSyncInstrumentation.java index d76295c406..88c7a3f204 100644 --- a/instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioSyncInstrumentation.java +++ b/instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioSyncInstrumentation.java @@ -5,6 +5,7 @@ package io.opentelemetry.javaagent.instrumentation.twilio; +import static io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge.currentContext; import static io.opentelemetry.javaagent.instrumentation.twilio.TwilioTracer.tracer; import static io.opentelemetry.javaagent.tooling.ClassLoaderMatcher.hasClassesNamed; import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.extendsClass; @@ -15,10 +16,8 @@ import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isPublic; import static net.bytebuddy.matcher.ElementMatchers.not; -import com.twilio.Twilio; -import io.opentelemetry.api.trace.Span; +import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; -import io.opentelemetry.javaagent.instrumentation.api.CallDepthThreadLocalMap; import io.opentelemetry.javaagent.tooling.TypeInstrumentation; import java.util.Map; import net.bytebuddy.asm.Advice; @@ -74,19 +73,15 @@ public class TwilioSyncInstrumentation implements TypeInstrumentation { public static void methodEnter( @Advice.This Object that, @Advice.Origin("#m") String methodName, - @Advice.Local("otelSpan") Span span, + @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { - - // Ensure that we only create a span for the top-level Twilio client method; except in the - // case of async operations where we want visibility into how long the task was delayed from - // starting. Our call depth checker does not span threads, so the async case is handled - // automatically for us. - if (CallDepthThreadLocalMap.incrementCallDepth(Twilio.class) > 0) { + Context parentContext = currentContext(); + if (!tracer().shouldStartSpan(parentContext)) { return; } - span = tracer().startSpan(that, methodName); - scope = tracer().startScope(span); + context = tracer().startSpan(parentContext, that, methodName); + scope = context.makeCurrent(); } /** Method exit instrumentation. */ @@ -94,18 +89,17 @@ public class TwilioSyncInstrumentation implements TypeInstrumentation { public static void methodExit( @Advice.Thrown Throwable throwable, @Advice.Return Object response, - @Advice.Local("otelSpan") Span span, + @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { if (scope == null) { return; } - CallDepthThreadLocalMap.reset(Twilio.class); scope.close(); if (throwable != null) { - tracer().endExceptionally(span, throwable); + tracer().endExceptionally(context, throwable); } else { - tracer().end(span, response); + tracer().end(context, response); } } } diff --git a/instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioTracer.java b/instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioTracer.java index 2f12464a36..abd8b339b2 100644 --- a/instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioTracer.java +++ b/instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioTracer.java @@ -5,12 +5,13 @@ package io.opentelemetry.javaagent.instrumentation.twilio; +import static io.opentelemetry.api.trace.Span.Kind.CLIENT; + import com.google.common.util.concurrent.ListenableFuture; import com.twilio.rest.api.v2010.account.Call; import com.twilio.rest.api.v2010.account.Message; import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; -import io.opentelemetry.context.Scope; import io.opentelemetry.instrumentation.api.tracer.BaseTracer; import java.lang.reflect.Method; import java.util.concurrent.TimeUnit; @@ -27,15 +28,18 @@ public class TwilioTracer extends BaseTracer { return TRACER; } - public Span startSpan(Object serviceExecutor, String methodName) { - return tracer.spanBuilder(spanNameOnServiceExecution(serviceExecutor, methodName)).startSpan(); + public boolean shouldStartSpan(Context parentContext) { + return parentContext.get(CONTEXT_CLIENT_SPAN_KEY) == null; } - @Override - public Scope startScope(Span span) { - Context context = Context.current().with(span); - context = context.with(CONTEXT_CLIENT_SPAN_KEY, span); - return context.makeCurrent(); + public Context startSpan(Context parentContext, Object serviceExecutor, String methodName) { + Span span = + tracer + .spanBuilder(spanNameOnServiceExecution(serviceExecutor, methodName)) + .setSpanKind(CLIENT) + .setParent(parentContext) + .startSpan(); + return parentContext.with(span).with(CONTEXT_CLIENT_SPAN_KEY, span); } /** Decorate trace based on service execution metadata. */ @@ -44,7 +48,7 @@ public class TwilioTracer extends BaseTracer { } /** Annotate the span with the results of the operation. */ - public void end(Span span, Object result) { + public void end(Context context, Object result) { // Unwrap ListenableFuture (if present) if (result instanceof ListenableFuture) { @@ -55,6 +59,8 @@ public class TwilioTracer extends BaseTracer { } } + Span span = Span.fromContext(context); + // Nothing to do here, so return if (result == null) { span.end(); @@ -94,6 +100,10 @@ public class TwilioTracer extends BaseTracer { super.end(span); } + public void endExceptionally(Context context, Throwable throwable) { + super.endExceptionally(Span.fromContext(context), throwable); + } + /** * Helper method for calling a getter using reflection. This will be slow, so only use when * required. diff --git a/instrumentation/twilio-6.6/javaagent/src/test/groovy/test/TwilioClientTest.groovy b/instrumentation/twilio-6.6/javaagent/src/test/groovy/test/TwilioClientTest.groovy index 10199d8811..a99e544688 100644 --- a/instrumentation/twilio-6.6/javaagent/src/test/groovy/test/TwilioClientTest.groovy +++ b/instrumentation/twilio-6.6/javaagent/src/test/groovy/test/TwilioClientTest.groovy @@ -5,7 +5,7 @@ package test -import static io.opentelemetry.api.trace.Span.Kind.INTERNAL +import static io.opentelemetry.api.trace.Span.Kind.CLIENT import static io.opentelemetry.instrumentation.test.utils.TraceUtils.runUnderTrace import com.fasterxml.jackson.databind.ObjectMapper @@ -142,7 +142,7 @@ class TwilioClientTest extends AgentTestRunner { } span(1) { name "MessageCreator.create" - kind INTERNAL + kind CLIENT errored false attributes { "twilio.type" "com.twilio.rest.api.v2010.account.Message" @@ -186,7 +186,7 @@ class TwilioClientTest extends AgentTestRunner { } span(1) { name "CallCreator.create" - kind INTERNAL + kind CLIENT errored false attributes { "twilio.type" "com.twilio.rest.api.v2010.account.Call" @@ -252,7 +252,7 @@ class TwilioClientTest extends AgentTestRunner { } span(1) { name "MessageCreator.create" - kind INTERNAL + kind CLIENT childOf(span(0)) errored false attributes { @@ -334,7 +334,7 @@ class TwilioClientTest extends AgentTestRunner { } span(1) { name "MessageCreator.create" - kind INTERNAL + kind CLIENT childOf(span(0)) errored false attributes { @@ -413,7 +413,7 @@ class TwilioClientTest extends AgentTestRunner { message.body == "Hello, World!" assertTraces(1) { - trace(0, 3) { + trace(0, 2) { span(0) { name "test" errored false @@ -423,7 +423,7 @@ class TwilioClientTest extends AgentTestRunner { } span(1) { name "MessageCreator.createAsync" - kind INTERNAL + kind CLIENT childOf(span(0)) errored false attributes { @@ -433,18 +433,6 @@ class TwilioClientTest extends AgentTestRunner { "twilio.status" "sent" } } - span(2) { - name "MessageCreator.create" - kind INTERNAL - childOf(span(1)) - errored false - attributes { - "twilio.type" "com.twilio.rest.api.v2010.account.Message" - "twilio.account" "AC14984e09e497506cf0d5eb59b1f6ace7" - "twilio.sid" "MMXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" - "twilio.status" "sent" - } - } } } @@ -484,7 +472,7 @@ class TwilioClientTest extends AgentTestRunner { } span(1) { name "MessageCreator.create" - kind INTERNAL + kind CLIENT errored true errorEvent(ApiException, "Testing Failure") } @@ -512,7 +500,7 @@ class TwilioClientTest extends AgentTestRunner { trace(0, 1) { span(0) { name "MessageCreator.create" - kind INTERNAL + kind CLIENT hasNoParent() errored false attributes { @@ -556,7 +544,7 @@ class TwilioClientTest extends AgentTestRunner { message.body == "Hello, World!" assertTraces(1) { - trace(0, 3) { + trace(0, 2) { span(0) { name "test" errored false @@ -566,18 +554,7 @@ class TwilioClientTest extends AgentTestRunner { } span(1) { name "MessageCreator.createAsync" - kind INTERNAL - errored false - attributes { - "twilio.type" "com.twilio.rest.api.v2010.account.Message" - "twilio.account" "AC14984e09e497506cf0d5eb59b1f6ace7" - "twilio.sid" "MMXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX" - "twilio.status" "sent" - } - } - span(2) { - name "MessageCreator.create" - kind INTERNAL + kind CLIENT errored false attributes { "twilio.type" "com.twilio.rest.api.v2010.account.Message" @@ -627,7 +604,7 @@ class TwilioClientTest extends AgentTestRunner { expect: assertTraces(1) { - trace(0, 3) { + trace(0, 2) { span(0) { name "test" errored true @@ -636,13 +613,7 @@ class TwilioClientTest extends AgentTestRunner { } span(1) { name "MessageCreator.createAsync" - kind INTERNAL - errored true - errorEvent(ApiException, "Testing Failure") - } - span(2) { - name "MessageCreator.create" - kind INTERNAL + kind CLIENT errored true errorEvent(ApiException, "Testing Failure") }