From c43f93cd60ff874eb5ac6e4d21a99e5599734aca Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Thu, 27 Feb 2025 16:43:16 -0500 Subject: [PATCH 1/3] if the service aren't found on the ImplBase, look for an AsyncService interface which *should* have the service methods. --- .../server/MethodHandlersInstrumentation.java | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/MethodHandlersInstrumentation.java b/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/MethodHandlersInstrumentation.java index 6d1058e01a2..322870cd67b 100644 --- a/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/MethodHandlersInstrumentation.java +++ b/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/MethodHandlersInstrumentation.java @@ -10,6 +10,7 @@ import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.api.Platform; import java.lang.reflect.Method; +import java.util.Arrays; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -52,9 +53,19 @@ public static class BuildAdvice { public static void onEnter(@Advice.Argument(0) Object serviceImpl) { try { Class serviceClass = serviceImpl.getClass(); - Class superclass = serviceClass.getSuperclass(); - if (superclass != null) { - for (Method method : superclass.getDeclaredMethods()) { + Class superClass = findImplBase(serviceClass); + if (superClass != null) { + Method[] declaredMethods = superClass.getDeclaredMethods(); + // bindService() would be the only method in this case and it's irrelevant + if (declaredMethods.length == 1) { + declaredMethods = + Arrays.stream(serviceClass.getInterfaces()) + .filter(i -> i.getSimpleName().equals("AsyncService")) + .findFirst() + .orElse(superClass) + .getDeclaredMethods(); + } + for (Method method : declaredMethods) { try { entry(serviceClass.getDeclaredMethod(method.getName(), method.getParameterTypes())); } catch (Throwable e) { @@ -66,5 +77,13 @@ public static void onEnter(@Advice.Argument(0) Object serviceImpl) { // this should be logged somehow } } + + public static Class findImplBase(Class serviceClass) { + Class superclass = serviceClass.getSuperclass(); + while (superclass != null && !superclass.getSimpleName().endsWith("ImplBase")) { + superclass = superclass.getSuperclass(); + } + return superclass; + } } } From 3911b6d86b5205ae650e8a43146494851f9bfbee Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Thu, 27 Feb 2025 17:41:34 -0500 Subject: [PATCH 2/3] inline the method. I should have known better. :) --- .../grpc/server/MethodHandlersInstrumentation.java | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/MethodHandlersInstrumentation.java b/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/MethodHandlersInstrumentation.java index 322870cd67b..de7709f9214 100644 --- a/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/MethodHandlersInstrumentation.java +++ b/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/MethodHandlersInstrumentation.java @@ -53,7 +53,10 @@ public static class BuildAdvice { public static void onEnter(@Advice.Argument(0) Object serviceImpl) { try { Class serviceClass = serviceImpl.getClass(); - Class superClass = findImplBase(serviceClass); + Class superClass = serviceClass.getSuperclass(); + while (superClass != null && !superClass.getSimpleName().endsWith("ImplBase")) { + superClass = superClass.getSuperclass(); + } if (superClass != null) { Method[] declaredMethods = superClass.getDeclaredMethods(); // bindService() would be the only method in this case and it's irrelevant @@ -77,13 +80,5 @@ public static void onEnter(@Advice.Argument(0) Object serviceImpl) { // this should be logged somehow } } - - public static Class findImplBase(Class serviceClass) { - Class superclass = serviceClass.getSuperclass(); - while (superclass != null && !superclass.getSimpleName().endsWith("ImplBase")) { - superclass = superclass.getSuperclass(); - } - return superclass; - } } } From c0f1a8fc497d7758089d5773c2c975258340a630 Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Thu, 27 Feb 2025 20:12:13 -0500 Subject: [PATCH 3/3] remove the lambda --- .../server/MethodHandlersInstrumentation.java | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/MethodHandlersInstrumentation.java b/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/MethodHandlersInstrumentation.java index de7709f9214..35cc6e901ba 100644 --- a/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/MethodHandlersInstrumentation.java +++ b/dd-java-agent/instrumentation/grpc-1.5/src/main/java/datadog/trace/instrumentation/grpc/server/MethodHandlersInstrumentation.java @@ -10,7 +10,6 @@ import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.api.Platform; import java.lang.reflect.Method; -import java.util.Arrays; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.matcher.ElementMatcher; @@ -44,7 +43,7 @@ public boolean isEnabled() { public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( isConstructor().and(takesArguments(2)), - MethodHandlersInstrumentation.class.getName() + "$BuildAdvice"); + "datadog.trace.instrumentation.grpc.server.MethodHandlersInstrumentation$BuildAdvice"); } public static class BuildAdvice { @@ -58,17 +57,16 @@ public static void onEnter(@Advice.Argument(0) Object serviceImpl) { superClass = superClass.getSuperclass(); } if (superClass != null) { - Method[] declaredMethods = superClass.getDeclaredMethods(); // bindService() would be the only method in this case and it's irrelevant - if (declaredMethods.length == 1) { - declaredMethods = - Arrays.stream(serviceClass.getInterfaces()) - .filter(i -> i.getSimpleName().equals("AsyncService")) - .findFirst() - .orElse(superClass) - .getDeclaredMethods(); + if (superClass.getDeclaredMethods().length == 1) { + for (Class i : serviceClass.getInterfaces()) { + if (i.getSimpleName().equals("AsyncService")) { + superClass = i; + break; + } + } } - for (Method method : declaredMethods) { + for (Method method : superClass.getDeclaredMethods()) { try { entry(serviceClass.getDeclaredMethod(method.getName(), method.getParameterTypes())); } catch (Throwable e) {