diff --git a/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/spanorigin/CodeOriginInfo.java b/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/spanorigin/CodeOriginInfo.java index 5656cbd2d5d9..06d3bca07ebb 100644 --- a/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/spanorigin/CodeOriginInfo.java +++ b/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/spanorigin/CodeOriginInfo.java @@ -19,16 +19,6 @@ public static void entry(Method method) { } } - public static void entry(Class type, Method method) { - if (InstrumenterConfig.get().isCodeOriginEnabled()) { - String signature = - stream(method.getParameterTypes()) - .map(Class::getTypeName) - .collect(Collectors.joining(", ", "(", ")")); - captureCodeOrigin(signature); - } - } - public static void entry(String name, Class target, String method, Class[] types) { if (InstrumenterConfig.get().isCodeOriginEnabled()) { captureCodeOrigin(name, target, method, types, true); diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/codeorigin/DefaultCodeOriginRecorder.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/codeorigin/DefaultCodeOriginRecorder.java index 91c740c1fc22..2f6bd48d317b 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/codeorigin/DefaultCodeOriginRecorder.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/codeorigin/DefaultCodeOriginRecorder.java @@ -1,14 +1,12 @@ package com.datadog.debugger.codeorigin; import static com.datadog.debugger.agent.ConfigurationAcceptor.Source.CODE_ORIGIN; -import static java.util.Arrays.asList; import static java.util.Arrays.stream; import com.datadog.debugger.agent.ConfigurationUpdater; import com.datadog.debugger.exception.Fingerprinter; import com.datadog.debugger.probe.CodeOriginProbe; import com.datadog.debugger.probe.Where; -import com.datadog.debugger.util.ClassNameFiltering; import datadog.trace.api.Config; import datadog.trace.bootstrap.debugger.CapturedContext; import datadog.trace.bootstrap.debugger.DebuggerContext; @@ -18,19 +16,13 @@ import datadog.trace.bootstrap.instrumentation.api.AgentTracer; import datadog.trace.util.AgentTaskScheduler; import datadog.trace.util.stacktrace.StackWalkerFactory; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.InputStream; import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.Map; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; -import org.objectweb.asm.ClassReader; -import org.objectweb.asm.tree.ClassNode; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -43,92 +35,30 @@ public class DefaultCodeOriginRecorder implements CodeOriginRecorder { private final Map probes = new ConcurrentHashMap<>(); - private final AgentTaskScheduler taskScheduler; - private final int maxUserFrames; - // this really should only be used for testing - public DefaultCodeOriginRecorder() { - maxUserFrames = 8; - configurationUpdater = null; - DebuggerContext.initClassNameFilter( - new ClassNameFiltering( - new HashSet<>( - asList( - "sun", - "org.junit", - "java.", - "org.gradle", - "com.sun", - "worker.org.gradle", - "datadog", - "com.datadog.debugger.probe", - "com.datadog.debugger.codeorigin")))); - new ClassNameFiltering( - new HashSet<>( - asList( - "sun", - "org.junit", - "java.", - "org.gradle", - "com.sun", - "worker.org.gradle", - "datadog", - "com.datadog.debugger.probe", - "com.datadog.debugger.codeorigin"))); - taskScheduler = AgentTaskScheduler.INSTANCE; - } - public DefaultCodeOriginRecorder(Config config, ConfigurationUpdater configurationUpdater) { this.configurationUpdater = configurationUpdater; maxUserFrames = config.getDebuggerCodeOriginMaxUserFrames(); - taskScheduler = AgentTaskScheduler.INSTANCE; - } - - public DefaultCodeOriginRecorder( - Config config, ConfigurationUpdater configurationUpdater, AgentTaskScheduler taskScheduler) { - this.configurationUpdater = configurationUpdater; - maxUserFrames = config.getDebuggerCodeOriginMaxUserFrames(); - this.taskScheduler = taskScheduler; } @Override public String captureCodeOrigin(String signature) { StackTraceElement element = findPlaceInStack(); String fingerprint = Fingerprinter.fingerprint(element); - if (fingerprint == null) { - LOG.debug("Unable to fingerprint stack trace"); - return null; - } CodeOriginProbe probe; - AgentSpan span = AgentTracer.activeSpan(); - if (!isAlreadyInstrumented(fingerprint)) { - Where where = - Where.of( - element.getClassName(), - element.getMethodName(), - signature, - String.valueOf(element.getLineNumber())); - - probe = - new CodeOriginProbe( - new ProbeId(UUID.randomUUID().toString(), 0), - where.getSignature(), - where, - maxUserFrames); - addFingerprint(fingerprint, probe); - - installProbe(probe); - if (span != null) { - // committing here manually so that first run probe encounters decorate the span until the - // instrumentation gets installed - probe.commit( - CapturedContext.EMPTY_CONTEXT, CapturedContext.EMPTY_CONTEXT, Collections.emptyList()); - } - - } else { + if (isAlreadyInstrumented(fingerprint)) { probe = fingerprints.get(fingerprint); + } else { + probe = + createProbe( + signature, + Where.of( + element.getClassName(), + element.getMethodName(), + signature, + String.valueOf(element.getLineNumber()))); } return probe.getId(); @@ -142,30 +72,42 @@ public String captureCodeOrigin( if (isAlreadyInstrumented(name)) { probe = fingerprints.get(name); } else { - Where where = - Where.of( - target.getName(), - method, - stream(types).map(Class::getTypeName).collect(Collectors.joining(", ", "(", ")"))); - probe = - new CodeOriginProbe( - new ProbeId(UUID.randomUUID().toString(), 0), - where.getSignature(), - where, - maxUserFrames); - addFingerprint(name, probe); - - installProbe(probe); - // committing here manually so that first run probe encounters decorate the span until the - // instrumentation gets installed - probe.commit( - CapturedContext.EMPTY_CONTEXT, CapturedContext.EMPTY_CONTEXT, Collections.emptyList()); + createProbe( + name, + Where.of( + target.getName(), + method, + stream(types) + .map(Class::getTypeName) + .collect(Collectors.joining(", ", "(", ")")))); } return probe.getId(); } + private CodeOriginProbe createProbe(String fingerPrint, Where where) { + CodeOriginProbe probe; + AgentSpan span = AgentTracer.activeSpan(); + + probe = + new CodeOriginProbe( + new ProbeId(UUID.randomUUID().toString(), 0), + where.getSignature(), + where, + maxUserFrames); + addFingerprint(fingerPrint, probe); + + installProbe(probe); + // committing here manually so that first run probe encounters decorate the span until the + // instrumentation gets installed + if (span != null) { + probe.commit( + CapturedContext.EMPTY_CONTEXT, CapturedContext.EMPTY_CONTEXT, Collections.emptyList()); + } + return probe; + } + private StackTraceElement findPlaceInStack() { return StackWalkerFactory.INSTANCE.walk( stream -> @@ -187,7 +129,8 @@ public String installProbe(CodeOriginProbe probe) { CodeOriginProbe installed = probes.putIfAbsent(probe.getId(), probe); if (installed == null) { if (configurationUpdater != null) { - taskScheduler.execute(() -> configurationUpdater.accept(CODE_ORIGIN, getProbes())); + AgentTaskScheduler.INSTANCE.execute( + () -> configurationUpdater.accept(CODE_ORIGIN, getProbes())); } return probe.getId(); } @@ -201,24 +144,4 @@ public CodeOriginProbe getProbe(String probeId) { public Collection getProbes() { return probes.values(); } - - private ClassNode parseClassFile(String className) { - byte[] bytes = new byte[8192]; - try (InputStream inputStream = - getClass() - .getClassLoader() - .getResourceAsStream(String.format("%s.class", className.replace('.', '/')))) { - ByteArrayOutputStream bao = new ByteArrayOutputStream(); - int bytesRead; - while ((bytesRead = inputStream.read(bytes)) != -1) { - bao.write(bytes, 0, bytesRead); - } - ClassNode classNode = new ClassNode(); - new ClassReader(bao.toByteArray()).accept(classNode, ClassReader.SKIP_FRAMES); - return classNode; - } catch (IOException e) { - LOG.error("Can't read class file information for {}", className); - return null; - } - } } diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturingTestBase.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturingTestBase.java index 991bdee2b2ec..56d913780cd7 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturingTestBase.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturingTestBase.java @@ -3,6 +3,7 @@ import static com.datadog.debugger.util.MoshiSnapshotHelper.NOT_CAPTURED_REASON; import static com.datadog.debugger.util.MoshiSnapshotTestHelper.VALUE_ADAPTER; import static com.datadog.debugger.util.TestHelper.setFieldInConfig; +import static datadog.trace.util.AgentThreadFactory.AgentThread.TASK_SCHEDULER; import static java.util.Arrays.asList; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -30,6 +31,7 @@ import datadog.trace.bootstrap.debugger.ProbeImplementation; import datadog.trace.bootstrap.debugger.ProbeRateLimiter; import datadog.trace.bootstrap.debugger.util.Redaction; +import datadog.trace.util.AgentTaskScheduler; import java.io.File; import java.io.IOException; import java.lang.instrument.ClassFileTransformer; @@ -351,6 +353,15 @@ public static LogProbe.Builder createProbeBuilder( protected TestSnapshotListener installProbes( Configuration configuration, ProbeDefinition... probes) { + + AgentTaskScheduler.INSTANCE = + new AgentTaskScheduler(TASK_SCHEDULER) { + @Override + public void execute(final Runnable target) { + target.run(); + } + }; + config = mock(Config.class); when(config.isDebuggerEnabled()).thenReturn(true); when(config.isDebuggerClassFileDumpEnabled()).thenReturn(true); diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/origin/CodeOriginTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/origin/CodeOriginTest.java index 3f38c4f80a38..e854ef251ec2 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/origin/CodeOriginTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/origin/CodeOriginTest.java @@ -154,6 +154,33 @@ public void testCaptureCodeOriginWithNullSignature() { assertFalse(probe.entrySpanProbe()); } + @Test + public void testCaptureCodeOriginWithExplicitInfo() throws IOException, URISyntaxException { + final String CLASS_NAME = "com.datadog.debugger.CodeOrigin04"; + final Class testClass = compileAndLoadClass(CLASS_NAME); + installProbes(); + CodeOriginProbe probe = + codeOriginRecorder.getProbe( + codeOriginRecorder.captureCodeOrigin( + "explicit", testClass, "main", new Class[] {int.class}, true)); + assertNotNull(probe, "The probe should have been created."); + assertTrue(probe.entrySpanProbe(), "Should be an entry probe."); + } + + @Test + public void testDuplicateInstrumentations() throws IOException, URISyntaxException { + final String CLASS_NAME = "com.datadog.debugger.CodeOrigin04"; + final Class testClass = compileAndLoadClass(CLASS_NAME); + installProbes(); + String probe1 = + codeOriginRecorder.captureCodeOrigin( + "explicit", testClass, "main", new Class[] {int.class}, true); + String probe2 = + codeOriginRecorder.captureCodeOrigin( + "explicit", testClass, "main", new Class[] {int.class}, true); + assertEquals(probe1, probe2); + } + @NotNull private List codeOriginProbes(String type) { CodeOriginProbe entry = 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 8722deb71dd7..88e3c159503b 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 @@ -44,30 +44,25 @@ public void methodAdvice(MethodTransformer transformer) { public static class BuildAdvice { @Advice.OnMethodEnter(suppress = Throwable.class) - public static void onEnter(@Advice.Argument(0) Object serviceImpl) { + public static void onEnter(@Advice.Argument(0) Object serviceImpl) throws Exception { Class serviceClass = serviceImpl.getClass(); Class superclass = serviceClass.getSuperclass(); if (superclass != null) { - try { - Class enclosingClass = superclass.getEnclosingClass(); - Field serviceNameField = enclosingClass.getDeclaredField("SERVICE_NAME"); - String serviceName = (String) serviceNameField.get(enclosingClass); - for (Method method : superclass.getDeclaredMethods()) { - try { - Method declaredMethod = - serviceClass.getDeclaredMethod(method.getName(), method.getParameterTypes()); - CodeOriginInfo.entry( - String.format("%s/%s", serviceName, method.getName()), - serviceClass, - declaredMethod.getName(), - declaredMethod.getParameterTypes()); - } catch (NoSuchMethodException e) { - // service method not override on the impl. skipping instrumentation - } + Class enclosingClass = superclass.getEnclosingClass(); + Field serviceNameField = enclosingClass.getDeclaredField("SERVICE_NAME"); + String serviceName = (String) serviceNameField.get(enclosingClass); + for (Method method : superclass.getDeclaredMethods()) { + try { + Method declaredMethod = + serviceClass.getDeclaredMethod(method.getName(), method.getParameterTypes()); + CodeOriginInfo.entry( + String.format("%s/%s", serviceName, method.getName()), + serviceClass, + declaredMethod.getName(), + declaredMethod.getParameterTypes()); + } catch (NoSuchMethodException e) { + // service method not overridden on the impl. skipping instrumentation. } - } catch (ReflectiveOperationException e) { - // need to find a way to log this - // LOG.debug("Member not found. Not instrumenting {}", serviceClass.getName(), e); } } } diff --git a/dd-java-agent/instrumentation/grpc-1.5/src/test/groovy/GrpcCodeOriginTest.groovy b/dd-java-agent/instrumentation/grpc-1.5/src/test/groovy/GrpcCodeOriginTest.groovy index e50dabc6d4bd..ace368a92a4d 100644 --- a/dd-java-agent/instrumentation/grpc-1.5/src/test/groovy/GrpcCodeOriginTest.groovy +++ b/dd-java-agent/instrumentation/grpc-1.5/src/test/groovy/GrpcCodeOriginTest.groovy @@ -300,13 +300,13 @@ abstract class GrpcCodeOriginTest extends VersionedNamingTestBase { DebuggerContext.initClassFilter(new DenyListHelper(null)) DebuggerContext.initValueSerializer(new JsonSnapshotSerializer()) - def scheduler = new AgentTaskScheduler(null) { + AgentTaskScheduler.INSTANCE = new AgentTaskScheduler(null) { @Override void execute(Runnable target) { target.run() } } - DebuggerContext.initCodeOrigin(new DefaultCodeOriginRecorder(config, configurationUpdater, scheduler)) + DebuggerContext.initCodeOrigin(new DefaultCodeOriginRecorder(config, configurationUpdater)) } } diff --git a/internal-api/src/main/java/datadog/trace/util/AgentTaskScheduler.java b/internal-api/src/main/java/datadog/trace/util/AgentTaskScheduler.java index 97c98c97fa86..6a50c40ec9cf 100644 --- a/internal-api/src/main/java/datadog/trace/util/AgentTaskScheduler.java +++ b/internal-api/src/main/java/datadog/trace/util/AgentTaskScheduler.java @@ -21,7 +21,7 @@ public class AgentTaskScheduler implements Executor { private static final Logger log = LoggerFactory.getLogger(AgentTaskScheduler.class); - public static final AgentTaskScheduler INSTANCE = new AgentTaskScheduler(TASK_SCHEDULER); + public static AgentTaskScheduler INSTANCE = new AgentTaskScheduler(TASK_SCHEDULER); private static final long SHUTDOWN_TIMEOUT = 5; // seconds