Switch gRPC instrumentation to instrument public ServerBuilder class. (…

…#1839) * Switch gRPC instrumentation to instrument public ServerBuilder class. * A bit more readability (hopefully) * hack * Remove library dedupe for now since we need a better story.
open-telemetry · Dec 7, 2020 · 7199595 · 7199595
1 parent 4cbfb36
commit 7199595
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 26 deletions.
diff --git a/instrumentation/grpc-1.5/javaagent/grpc-1.5-javaagent.gradle b/instrumentation/grpc-1.5/javaagent/grpc-1.5-javaagent.gradle
@@ -20,7 +20,7 @@ muzzle {
   pass {
     group = "io.grpc"
     module = "grpc-core"
-    versions = "[1.5.0, 1.33.0)" // see https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/1453
+    versions = "[1.5.0,)"
   }
 }
 
@@ -35,11 +35,5 @@ dependencies {
   testLibrary group: 'io.grpc', name: 'grpc-protobuf', version: grpcVersion
   testLibrary group: 'io.grpc', name: 'grpc-stub', version: grpcVersion
 
-  // see https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/1453
-  latestDepTestLibrary group: 'io.grpc', name: 'grpc-core', version: '1.32.+'
-  latestDepTestLibrary group: 'io.grpc', name: 'grpc-netty', version: '1.32.+'
-  latestDepTestLibrary group: 'io.grpc', name: 'grpc-protobuf', version: '1.32.+'
-  latestDepTestLibrary group: 'io.grpc', name: 'grpc-stub', version: '1.32.+'
-
   testImplementation project(':instrumentation:grpc-1.5:testing')
 }
diff --git a/...o/opentelemetry/javaagent/instrumentation/grpc/v1_5/GrpcServerBuilderInstrumentation.java b/...o/opentelemetry/javaagent/instrumentation/grpc/v1_5/GrpcServerBuilderInstrumentation.java
@@ -5,14 +5,18 @@
 
 package io.opentelemetry.javaagent.instrumentation.grpc.v1_5;
 
-import static java.util.Collections.singletonMap;
+import static io.opentelemetry.javaagent.tooling.ClassLoaderMatcher.hasClassesNamed;
+import static io.opentelemetry.javaagent.tooling.bytebuddy.matcher.AgentElementMatchers.safeHasSuperType;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
+import static net.bytebuddy.matcher.ElementMatchers.isPublic;
 import static net.bytebuddy.matcher.ElementMatchers.named;
+import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
 
-import io.grpc.ServerInterceptor;
+import io.grpc.ServerBuilder;
 import io.opentelemetry.instrumentation.grpc.v1_5.server.TracingServerInterceptor;
+import io.opentelemetry.javaagent.instrumentation.api.CallDepthThreadLocalMap;
 import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
-import java.util.List;
+import java.util.Collections;
 import java.util.Map;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.method.MethodDescription;
@@ -21,33 +25,36 @@
 
 public class GrpcServerBuilderInstrumentation implements TypeInstrumentation {
 
+  @Override
+  public ElementMatcher<ClassLoader> classLoaderOptimization() {
+    return hasClassesNamed("io.grpc.ServerBuilder");
+  }
+
   @Override
   public ElementMatcher<TypeDescription> typeMatcher() {
-    return named("io.grpc.internal.AbstractServerImplBuilder");
+    return safeHasSuperType(named("io.grpc.ServerBuilder"));
   }
 
   @Override
   public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
-    return singletonMap(
-        isMethod().and(named("build")),
-        GrpcServerBuilderInstrumentation.class.getName() + "$AddInterceptorAdvice");
+    return Collections.singletonMap(
+        isMethod().and(isPublic()).and(named("build")).and(takesArguments(0)),
+        GrpcServerBuilderInstrumentation.class.getName() + "$BuildAdvice");
   }
 
-  public static class AddInterceptorAdvice {
+  public static class BuildAdvice {
 
     @Advice.OnMethodEnter(suppress = Throwable.class)
-    public static void addInterceptor(
-        @Advice.FieldValue("interceptors") List<ServerInterceptor> interceptors) {
-      boolean shouldRegister = true;
-      for (ServerInterceptor interceptor : interceptors) {
-        if (interceptor instanceof TracingServerInterceptor) {
-          shouldRegister = false;
-          break;
-        }
-      }
-      if (shouldRegister) {
-        interceptors.add(TracingServerInterceptor.newInterceptor());
+    public static void onEnter(@Advice.This ServerBuilder<?> serverBuilder) {
+      int callDepth = CallDepthThreadLocalMap.incrementCallDepth(ServerBuilder.class);
+      if (callDepth == 0) {
+        serverBuilder.intercept(TracingServerInterceptor.newInterceptor());
       }
     }
+
+    @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
+    public static void onExit(@Advice.This ServerBuilder<?> serverBuilder) {
+      CallDepthThreadLocalMap.decrementCallDepth(ServerBuilder.class);
+    }
   }
 }