Make twilio instrumentation consistent with others
MODIFY
instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioAsyncInstrumentation.java
instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioAsyncInstrumentation.java
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<T> implements FutureCallback<T> {
/** 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);
}
}
}

MODIFY
instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioSyncInstrumentation.java
instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioSyncInstrumentation.java
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);
}
}
}

MODIFY
instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioTracer.java
instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioTracer.java
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.

MODIFY
instrumentation/twilio-6.6/javaagent/src/test/groovy/test/TwilioClientTest.groovy
instrumentation/twilio-6.6/javaagent/src/test/groovy/test/TwilioClientTest.groovy
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")
}

Make twilio instrumentation consistent with others (#1835)