Migrate GWT to Instrumenter API (#3146)

* Migrate GWT to Instrumenter API * TODO * instrumenter()
open-telemetry · Jun 1, 2021 · 5cbade4 · 5cbade4
1 parent c3dedbb
commit 5cbade4
Show file tree

Hide file tree

Showing 5 changed files with 106 additions and 68 deletions.
diff --git a/...c/main/java/io/opentelemetry/javaagent/instrumentation/gwt/GwtRpcAttributesExtractor.java b/...c/main/java/io/opentelemetry/javaagent/instrumentation/gwt/GwtRpcAttributesExtractor.java
@@ -0,0 +1,26 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.gwt;
+
+import io.opentelemetry.instrumentation.api.instrumenter.rpc.RpcAttributesExtractor;
+import java.lang.reflect.Method;
+
+final class GwtRpcAttributesExtractor extends RpcAttributesExtractor<Method, Void> {
+  @Override
+  protected String system(Method method) {
+    return "gwt";
+  }
+
+  @Override
+  protected String service(Method method) {
+    return method.getDeclaringClass().getName();
+  }
+
+  @Override
+  protected String method(Method method) {
+    return method.getName();
+  }
+}
diff --git a/...t/src/main/java/io/opentelemetry/javaagent/instrumentation/gwt/GwtRpcInstrumentation.java b/...t/src/main/java/io/opentelemetry/javaagent/instrumentation/gwt/GwtRpcInstrumentation.java
@@ -5,7 +5,7 @@
 
 package io.opentelemetry.javaagent.instrumentation.gwt;
 
-import static io.opentelemetry.javaagent.instrumentation.gwt.GwtTracer.tracer;
+import static io.opentelemetry.javaagent.instrumentation.gwt.GwtSingletons.instrumenter;
 import static net.bytebuddy.matcher.ElementMatchers.named;
 import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
 import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
@@ -14,6 +14,7 @@
 import io.opentelemetry.context.Scope;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
+import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
 import java.lang.reflect.Method;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
@@ -52,23 +53,25 @@ public void transform(TypeTransformer transformer) {
   public static class InvokeAndEncodeResponseAdvice {
     @Advice.OnMethodEnter(suppress = Throwable.class)
     public static void onEnter(
-        @Advice.Argument(0) Object target,
         @Advice.Argument(1) Method method,
         @Advice.Local("otelContext") Context context,
         @Advice.Local("otelScope") Scope scope) {
-
-      context = tracer().startRpcSpan(target, method);
+      context =
+          instrumenter()
+              .start(Java8BytecodeBridge.currentContext(), method)
+              .with(GwtSingletons.RPC_CONTEXT_KEY, true);
       scope = context.makeCurrent();
     }
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void onExit(
-        @Advice.Thrown Throwable throwable,
+        @Advice.Argument(1) Method method,
         @Advice.Local("otelContext") Context context,
-        @Advice.Local("otelScope") Scope scope) {
+        @Advice.Local("otelScope") Scope scope,
+        @Advice.Thrown Throwable throwable) {
       scope.close();
 
-      tracer().endSpan(context, throwable);
+      instrumenter().end(context, method, null, throwable);
     }
   }
 
@@ -78,7 +81,12 @@ public static void onEnter(@Advice.Argument(1) Throwable throwable) {
       if (throwable == null) {
         return;
       }
-      tracer().rpcFailure(throwable);
+      Context context = Java8BytecodeBridge.currentContext();
+      if (context.get(GwtSingletons.RPC_CONTEXT_KEY) == null) {
+        // not inside rpc invocation
+        return;
+      }
+      Java8BytecodeBridge.spanFromContext(context).recordException(throwable);
     }
   }
 }
diff --git a/...javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/gwt/GwtSingletons.java b/...javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/gwt/GwtSingletons.java
@@ -0,0 +1,42 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.gwt;
+
+import io.opentelemetry.api.GlobalOpenTelemetry;
+import io.opentelemetry.context.ContextKey;
+import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
+import io.opentelemetry.instrumentation.api.instrumenter.rpc.RpcAttributesExtractor;
+import io.opentelemetry.instrumentation.api.instrumenter.rpc.RpcSpanNameExtractor;
+import java.lang.reflect.Method;
+
+public final class GwtSingletons {
+
+  private static final String INSTRUMENTATION_NAME = "io.opentelemetry.javaagent.gwt-2.0";
+
+  public static final ContextKey<Boolean> RPC_CONTEXT_KEY =
+      ContextKey.named("opentelemetry-gwt-rpc-context-key");
+
+  private static final Instrumenter<Method, Void> INSTRUMENTER;
+
+  static {
+    RpcAttributesExtractor<Method, Void> rpcAttributes = new GwtRpcAttributesExtractor();
+    INSTRUMENTER =
+        Instrumenter.<Method, Void>newBuilder(
+                GlobalOpenTelemetry.get(),
+                INSTRUMENTATION_NAME,
+                RpcSpanNameExtractor.create(rpcAttributes))
+            .addAttributesExtractor(rpcAttributes)
+            // TODO(anuraaga): This should be a server span, but we currently have no way to merge
+            // with the HTTP instrumentation's server span.
+            .newInstrumenter();
+  }
+
+  public static Instrumenter<Method, Void> instrumenter() {
+    return INSTRUMENTER;
+  }
+
+  private GwtSingletons() {}
+}
diff --git a/...2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/gwt/GwtTracer.java b/...2.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/gwt/GwtTracer.java
diff --git a/instrumentation/gwt-2.0/javaagent/src/test/groovy/GwtTest.groovy b/instrumentation/gwt-2.0/javaagent/src/test/groovy/GwtTest.groovy
@@ -3,12 +3,11 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import static io.opentelemetry.instrumentation.test.utils.TraceUtils.basicSpan
-
 import io.opentelemetry.api.trace.SpanKind
 import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
 import io.opentelemetry.instrumentation.test.asserts.TraceAssert
 import io.opentelemetry.instrumentation.test.base.HttpServerTestTrait
+import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
 import java.util.concurrent.TimeUnit
 import okhttp3.HttpUrl
 import okhttp3.Request
@@ -119,7 +118,16 @@ class GwtTest extends AgentInstrumentationSpecification implements HttpServerTes
     assertTraces(1) {
       trace(0, 2) {
         serverSpan(it, 0, getContextPath() + "/greeting/greet")
-        basicSpan(it, 1, "MessageServiceImpl.sendMessage", span(0))
+        span(1) {
+          name "test.gwt.shared.MessageService/sendMessage"
+          kind SpanKind.INTERNAL
+          childOf(span(0))
+          attributes {
+            "${SemanticAttributes.RPC_SYSTEM.key}" "gwt"
+            "${SemanticAttributes.RPC_SERVICE.key}" "test.gwt.shared.MessageService"
+            "${SemanticAttributes.RPC_METHOD.key}" "sendMessage"
+          }
+        }
       }
     }
     clearExportedData()
@@ -134,7 +142,17 @@ class GwtTest extends AgentInstrumentationSpecification implements HttpServerTes
     assertTraces(1) {
       trace(0, 2) {
         serverSpan(it, 0, getContextPath() + "/greeting/greet")
-        basicSpan(it, 1, "MessageServiceImpl.sendMessage", span(0), new IOException())
+        span(1) {
+          name "test.gwt.shared.MessageService/sendMessage"
+          kind SpanKind.INTERNAL
+          childOf(span(0))
+          errorEvent(IOException)
+          attributes {
+            "${SemanticAttributes.RPC_SYSTEM.key}" "gwt"
+            "${SemanticAttributes.RPC_SERVICE.key}" "test.gwt.shared.MessageService"
+            "${SemanticAttributes.RPC_METHOD.key}" "sendMessage"
+          }
+        }
       }
     }