From 28e5cb5dd61b32d706f8be80a58c571b43608e7b Mon Sep 17 00:00:00 2001 From: jack-berg <34418638+jack-berg@users.noreply.github.com> Date: Wed, 8 Sep 2021 15:10:03 -0500 Subject: [PATCH] Transition twilio-6.6 module to instrumenter API (#4045) * Convert twilio-6.6 module to instrumenter API * Rename TwilioTracer to TwilioSingletons --- .../twilio/TwilioAsyncInstrumentation.java | 27 ++-- ...TwilioExperimentalAttributesExtractor.java | 94 +++++++++++++ .../twilio/TwilioSingletons.java | 43 ++++++ .../twilio/TwilioSyncInstrumentation.java | 19 ++- .../instrumentation/twilio/TwilioTracer.java | 132 ------------------ 5 files changed, 163 insertions(+), 152 deletions(-) create mode 100644 instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioExperimentalAttributesExtractor.java create mode 100644 instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioSingletons.java delete mode 100644 instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioTracer.java 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 87f38e4e28..d37edf572b 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 @@ -8,7 +8,8 @@ package io.opentelemetry.javaagent.instrumentation.twilio; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.extendsClass; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; import static io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge.currentContext; -import static io.opentelemetry.javaagent.instrumentation.twilio.TwilioTracer.tracer; +import static io.opentelemetry.javaagent.instrumentation.twilio.TwilioSingletons.instrumenter; +import static io.opentelemetry.javaagent.instrumentation.twilio.TwilioSingletons.spanName; import static net.bytebuddy.matcher.ElementMatchers.isAbstract; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -76,13 +77,15 @@ public class TwilioAsyncInstrumentation implements TypeInstrumentation { @Advice.This Object that, @Advice.Origin("#m") String methodName, @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { + @Advice.Local("otelScope") Scope scope, + @Advice.Local("otelSpanName") String spanName) { Context parentContext = currentContext(); - if (!tracer().shouldStartSpan(parentContext)) { + spanName = spanName(that, methodName); + if (!instrumenter().shouldStart(parentContext, spanName)) { return; } - context = tracer().startSpan(parentContext, that, methodName); + context = instrumenter().start(parentContext, spanName); scope = context.makeCurrent(); } @@ -92,7 +95,8 @@ public class TwilioAsyncInstrumentation implements TypeInstrumentation { @Advice.Thrown Throwable throwable, @Advice.Return ListenableFuture response, @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { + @Advice.Local("otelScope") Scope scope, + @Advice.Local("otelSpanName") String spanName) { if (scope == null) { return; } @@ -101,12 +105,12 @@ public class TwilioAsyncInstrumentation implements TypeInstrumentation { if (throwable != null) { // There was an synchronous error, // which means we shouldn't wait for a callback to close the span. - tracer().endExceptionally(context, throwable); + instrumenter().end(context, spanName, null, 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<>(context), Twilio.getExecutorService()); + response, new SpanFinishingCallback<>(context, spanName), Twilio.getExecutorService()); } } } @@ -120,18 +124,21 @@ public class TwilioAsyncInstrumentation implements TypeInstrumentation { /** Span that we should finish and annotate when the future is complete. */ private final Context context; - public SpanFinishingCallback(Context context) { + private final String spanName; + + public SpanFinishingCallback(Context context, String spanName) { this.context = context; + this.spanName = spanName; } @Override public void onSuccess(Object result) { - tracer().end(context, result); + instrumenter().end(context, spanName, result, null); } @Override public void onFailure(Throwable t) { - tracer().endExceptionally(context, t); + instrumenter().end(context, spanName, null, t); } } } diff --git a/instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioExperimentalAttributesExtractor.java b/instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioExperimentalAttributesExtractor.java new file mode 100644 index 0000000000..f828edd25a --- /dev/null +++ b/instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioExperimentalAttributesExtractor.java @@ -0,0 +1,94 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.twilio; + +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.Uninterruptibles; +import com.twilio.rest.api.v2010.account.Call; +import com.twilio.rest.api.v2010.account.Message; +import io.opentelemetry.api.common.AttributesBuilder; +import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor; +import java.lang.reflect.Method; +import java.util.concurrent.TimeUnit; +import org.checkerframework.checker.nullness.qual.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +class TwilioExperimentalAttributesExtractor extends AttributesExtractor { + + private static final Logger logger = + LoggerFactory.getLogger(TwilioExperimentalAttributesExtractor.class); + + @Override + protected void onStart(AttributesBuilder attributes, String s) {} + + @Override + protected void onEnd(AttributesBuilder attributes, String s, @Nullable Object result) { + if (result == null) { + return; + } + + // Unwrap ListenableFuture (if present) + if (result instanceof ListenableFuture) { + try { + result = + Uninterruptibles.getUninterruptibly( + (ListenableFuture) result, 0, TimeUnit.MICROSECONDS); + } catch (Exception e) { + logger.debug("Error unwrapping result", e); + } + } + + // Provide helpful metadata for some of the more common response types + attributes.put("twilio.type", result.getClass().getCanonicalName()); + + // Instrument the most popular resource types directly + if (result instanceof Message) { + Message message = (Message) result; + attributes.put("twilio.account", message.getAccountSid()); + attributes.put("twilio.sid", message.getSid()); + Message.Status status = message.getStatus(); + if (status != null) { + attributes.put("twilio.status", status.toString()); + } + } else if (result instanceof Call) { + Call call = (Call) result; + attributes.put("twilio.account", call.getAccountSid()); + attributes.put("twilio.sid", call.getSid()); + attributes.put("twilio.parentSid", call.getParentCallSid()); + Call.Status status = call.getStatus(); + if (status != null) { + attributes.put("twilio.status", status.toString()); + } + } else { + // Use reflection to gather insight from other types; note that Twilio requests take close to + // 1 second, so the added hit from reflection here is relatively minimal in the grand scheme + // of things + setTagIfPresent(attributes, result, "twilio.sid", "getSid"); + setTagIfPresent(attributes, result, "twilio.account", "getAccountSid"); + setTagIfPresent(attributes, result, "twilio.status", "getStatus"); + } + } + + /** + * Helper method for calling a getter using reflection. This will be slow, so only use when + * required. + */ + private static void setTagIfPresent( + AttributesBuilder attributes, Object result, String tag, String getter) { + try { + Method method = result.getClass().getMethod(getter); + Object value = method.invoke(result); + + if (value != null) { + attributes.put(tag, value.toString()); + } + + } catch (Exception e) { + // Expected that this won't work for all result types + } + } +} diff --git a/instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioSingletons.java b/instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioSingletons.java new file mode 100644 index 0000000000..030f3bf78b --- /dev/null +++ b/instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioSingletons.java @@ -0,0 +1,43 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.twilio; + +import static io.opentelemetry.instrumentation.api.instrumenter.SpanKindExtractor.alwaysClient; + +import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.instrumentation.api.config.Config; +import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; +import io.opentelemetry.instrumentation.api.instrumenter.InstrumenterBuilder; +import io.opentelemetry.instrumentation.api.tracer.SpanNames; + +public class TwilioSingletons { + + private static final boolean CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES = + Config.get().getBoolean("otel.instrumentation.twilio.experimental-span-attributes", false); + + private static final Instrumenter INSTRUMENTER; + + static { + InstrumenterBuilder instrumenterBuilder = + Instrumenter.newBuilder( + GlobalOpenTelemetry.get(), "io.opentelemetry.twilio-6.6", str -> str); + + if (CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES) { + instrumenterBuilder.addAttributesExtractor(new TwilioExperimentalAttributesExtractor()); + } + + INSTRUMENTER = instrumenterBuilder.newInstrumenter(alwaysClient()); + } + + public static Instrumenter instrumenter() { + return INSTRUMENTER; + } + + /** Derive span name from service execution metadata. */ + public static String spanName(Object serviceExecutor, String methodName) { + return SpanNames.fromMethod(serviceExecutor.getClass(), methodName); + } +} 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 4dc24a051f..d807277c15 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 @@ -8,7 +8,7 @@ package io.opentelemetry.javaagent.instrumentation.twilio; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.extendsClass; import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed; import static io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge.currentContext; -import static io.opentelemetry.javaagent.instrumentation.twilio.TwilioTracer.tracer; +import static io.opentelemetry.javaagent.instrumentation.twilio.TwilioSingletons.instrumenter; import static net.bytebuddy.matcher.ElementMatchers.isAbstract; import static net.bytebuddy.matcher.ElementMatchers.isMethod; import static net.bytebuddy.matcher.ElementMatchers.isPublic; @@ -69,13 +69,15 @@ public class TwilioSyncInstrumentation implements TypeInstrumentation { @Advice.This Object that, @Advice.Origin("#m") String methodName, @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { + @Advice.Local("otelScope") Scope scope, + @Advice.Local("otelSpanName") String spanName) { Context parentContext = currentContext(); - if (!tracer().shouldStartSpan(parentContext)) { + spanName = TwilioSingletons.spanName(that, methodName); + if (!instrumenter().shouldStart(parentContext, spanName)) { return; } - context = tracer().startSpan(parentContext, that, methodName); + context = instrumenter().start(parentContext, spanName); scope = context.makeCurrent(); } @@ -85,17 +87,14 @@ public class TwilioSyncInstrumentation implements TypeInstrumentation { @Advice.Thrown Throwable throwable, @Advice.Return Object response, @Advice.Local("otelContext") Context context, - @Advice.Local("otelScope") Scope scope) { + @Advice.Local("otelScope") Scope scope, + @Advice.Local("otelSpanName") String spanName) { if (scope == null) { return; } scope.close(); - if (throwable != null) { - tracer().endExceptionally(context, throwable); - } else { - tracer().end(context, response); - } + instrumenter().end(context, spanName, response, throwable); } } } 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 deleted file mode 100644 index 49842f6b2b..0000000000 --- a/instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioTracer.java +++ /dev/null @@ -1,132 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.javaagent.instrumentation.twilio; - -import static io.opentelemetry.api.trace.SpanKind.CLIENT; - -import com.google.common.util.concurrent.ListenableFuture; -import com.google.common.util.concurrent.Uninterruptibles; -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.instrumentation.api.config.Config; -import io.opentelemetry.instrumentation.api.tracer.BaseTracer; -import io.opentelemetry.instrumentation.api.tracer.SpanNames; -import java.lang.reflect.Method; -import java.util.concurrent.TimeUnit; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -public class TwilioTracer extends BaseTracer { - - private static final boolean CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES = - Config.get().getBoolean("otel.instrumentation.twilio.experimental-span-attributes", false); - - private static final Logger logger = LoggerFactory.getLogger(TwilioTracer.class); - - public static final TwilioTracer TRACER = new TwilioTracer(); - - public static TwilioTracer tracer() { - return TRACER; - } - - public boolean shouldStartSpan(Context parentContext) { - return shouldStartSpan(parentContext, CLIENT); - } - - public Context startSpan(Context parentContext, Object serviceExecutor, String methodName) { - String spanName = spanNameOnServiceExecution(serviceExecutor, methodName); - Span span = spanBuilder(parentContext, spanName, CLIENT).startSpan(); - return withClientSpan(parentContext, span); - } - - /** Decorate trace based on service execution metadata. */ - private static String spanNameOnServiceExecution(Object serviceExecutor, String methodName) { - return SpanNames.fromMethod(serviceExecutor.getClass(), methodName); - } - - /** Annotate the span with the results of the operation. */ - public void end(Context context, Object result) { - - // Unwrap ListenableFuture (if present) - if (result instanceof ListenableFuture) { - try { - result = - Uninterruptibles.getUninterruptibly( - (ListenableFuture) result, 0, TimeUnit.MICROSECONDS); - } catch (Exception e) { - logger.debug("Error unwrapping result", e); - } - } - - Span span = Span.fromContext(context); - - // Nothing to do here, so return - if (result == null) { - span.end(); - return; - } - - if (CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES) { - // Provide helpful metadata for some of the more common response types - span.setAttribute("twilio.type", result.getClass().getCanonicalName()); - - // Instrument the most popular resource types directly - if (result instanceof Message) { - Message message = (Message) result; - span.setAttribute("twilio.account", message.getAccountSid()); - span.setAttribute("twilio.sid", message.getSid()); - Message.Status status = message.getStatus(); - if (status != null) { - span.setAttribute("twilio.status", status.toString()); - } - } else if (result instanceof Call) { - Call call = (Call) result; - span.setAttribute("twilio.account", call.getAccountSid()); - span.setAttribute("twilio.sid", call.getSid()); - span.setAttribute("twilio.parentSid", call.getParentCallSid()); - Call.Status status = call.getStatus(); - if (status != null) { - span.setAttribute("twilio.status", status.toString()); - } - } else { - // Use reflection to gather insight from other types; note that Twilio requests take close - // to - // 1 second, so the added hit from reflection here is relatively minimal in the grand scheme - // of things - setTagIfPresent(span, result, "twilio.sid", "getSid"); - setTagIfPresent(span, result, "twilio.account", "getAccountSid"); - setTagIfPresent(span, result, "twilio.status", "getStatus"); - } - } - - super.end(context); - } - - /** - * Helper method for calling a getter using reflection. This will be slow, so only use when - * required. - */ - private static void setTagIfPresent(Span span, Object result, String tag, String getter) { - try { - Method method = result.getClass().getMethod(getter); - Object value = method.invoke(result); - - if (value != null) { - span.setAttribute(tag, value.toString()); - } - - } catch (Exception e) { - // Expected that this won't work for all result types - } - } - - @Override - protected String getInstrumentationName() { - return "io.opentelemetry.twilio-6.6"; - } -}