optimize method probe without condition (#9834)

jpbempel · web-flow · commit 9329e985dbd0 · 2025-10-23T16:30:19.000+02:00
In case of a method probe without condition we can move the sampling
is currently done in LogProbe::evaluate to LogProbe::isReadToCapture.
This way if the sample fails we return false on isReadyTocapture and
no CapturedContext is created and nothing captured. If we sample the
execution the CapturedContext will be created we capture the context
and serialize the result, but we are sure that we are not doing for
nothing.
We introduce also a differentiation for single probe/multiple probe
in calling evaluate that is propagated to probe implementation. it
helps to differentiate the behavior based on the instrumentation
diff --git a/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/CapturedContext.java b/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/CapturedContext.java
@@ -297,7 +297,8 @@ public Status evaluate(
       ProbeImplementation probeImplementation,
       String thisClassName,
       long startTimestamp,
-      MethodLocation methodLocation) {
+      MethodLocation methodLocation,
+      boolean singleProbe) {
     Status status =
         statusByProbeId.computeIfAbsent(
             probeImplementation.getProbeId().getEncodedId(),
@@ -311,7 +312,7 @@ public Status evaluate(
     boolean shouldEvaluate =
         MethodLocation.isSame(methodLocation, probeImplementation.getEvaluateAt());
     if (shouldEvaluate) {
-      probeImplementation.evaluate(this, status, methodLocation);
+      probeImplementation.evaluate(this, status, methodLocation, singleProbe);
     }
     return status;
   }
diff --git a/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/DebuggerContext.java b/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/DebuggerContext.java
@@ -311,7 +311,11 @@ public static void evalContext(
         }
         CapturedContext.Status status =
             context.evaluate(
-                probeImplementation, callingClass.getTypeName(), startTimestamp, methodLocation);
+                probeImplementation,
+                callingClass.getTypeName(),
+                startTimestamp,
+                methodLocation,
+                false);
         needFreeze |= status.shouldFreezeContext();
       }
       // only freeze the context when we have at lest one snapshot probe, and we should send
@@ -343,7 +347,11 @@ public static void evalContext(
       }
       CapturedContext.Status status =
           context.evaluate(
-              probeImplementation, callingClass.getTypeName(), startTimestamp, methodLocation);
+              probeImplementation,
+              callingClass.getTypeName(),
+              startTimestamp,
+              methodLocation,
+              true);
       boolean needFreeze = status.shouldFreezeContext();
       // only freeze the context when we have at lest one snapshot probe, and we should send
       // snapshot
@@ -371,7 +379,7 @@ public static void evalContextAndCommit(
           continue;
         }
         context.evaluate(
-            probeImplementation, callingClass.getTypeName(), -1, MethodLocation.DEFAULT);
+            probeImplementation, callingClass.getTypeName(), -1, MethodLocation.DEFAULT, false);
         probeImplementations.add(probeImplementation);
       }
       for (ProbeImplementation probeImplementation : probeImplementations) {
@@ -395,7 +403,8 @@ public static void evalContextAndCommit(
       if (probeImplementation == null) {
         return;
       }
-      context.evaluate(probeImplementation, callingClass.getTypeName(), -1, MethodLocation.DEFAULT);
+      context.evaluate(
+          probeImplementation, callingClass.getTypeName(), -1, MethodLocation.DEFAULT, true);
       probeImplementation.commit(context, line);
     } catch (Exception ex) {
       LOGGER.debug("Error in evalContextAndCommit: ", ex);
diff --git a/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/ProbeImplementation.java b/dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/ProbeImplementation.java
@@ -17,7 +17,10 @@ public interface ProbeImplementation {
   String getStrTags();
 
   void evaluate(
-      CapturedContext context, CapturedContext.Status status, MethodLocation methodLocation);
+      CapturedContext context,
+      CapturedContext.Status status,
+      MethodLocation methodLocation,
+      boolean singleProbe);
 
   void commit(
       CapturedContext entryContext,
@@ -82,7 +85,10 @@ public String getStrTags() {
 
     @Override
     public void evaluate(
-        CapturedContext context, CapturedContext.Status status, MethodLocation methodLocation) {}
+        CapturedContext context,
+        CapturedContext.Status status,
+        MethodLocation methodLocation,
+        boolean singleProbe) {}
 
     @Override
     public void commit(
diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ExceptionProbe.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ExceptionProbe.java
@@ -71,7 +71,10 @@ public CapturedContext.Status createStatus() {
 
   @Override
   public void evaluate(
-      CapturedContext context, CapturedContext.Status status, MethodLocation methodLocation) {
+      CapturedContext context,
+      CapturedContext.Status status,
+      MethodLocation methodLocation,
+      boolean singleProbe) {
     ExceptionProbeStatus exceptionStatus;
     if (status instanceof ExceptionProbeStatus) {
       exceptionStatus = (ExceptionProbeStatus) status;
@@ -108,7 +111,7 @@ public void evaluate(
         exceptionStatus.setForceSampling(true);
       }
       exceptionStatus.setCapture(true);
-      super.evaluate(context, status, methodLocation);
+      super.evaluate(context, status, methodLocation, singleProbe);
     }
   }
 
diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/LogProbe.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/LogProbe.java
@@ -485,7 +485,7 @@ public InstrumentationResult.Status instrument(
 
   @Override
   public boolean isReadyToCapture() {
-    if (isLineProbe() && !hasCondition()) {
+    if (!hasCondition()) {
       // we are sampling here to avoid creating CapturedContext when the sampling result is negative
       return ProbeRateLimiter.tryProbe(id);
     }
@@ -494,20 +494,25 @@ public boolean isReadyToCapture() {
 
   @Override
   public void evaluate(
-      CapturedContext context, CapturedContext.Status status, MethodLocation methodLocation) {
+      CapturedContext context,
+      CapturedContext.Status status,
+      MethodLocation methodLocation,
+      boolean singleProbe) {
     if (!(status instanceof LogStatus)) {
       throw new IllegalStateException("Invalid status: " + status.getClass());
     }
     LogStatus logStatus = (LogStatus) status;
-    if (isLineProbe() && !hasCondition()) {
-      // sampling was already done in isReadToCapture so we assume that if we are executing the
-      // current method it means the status should be sampled
-      if (!logStatus.getDebugSessionStatus().isDisabled()) {
-        logStatus.setSampled(true);
+    if (!hasCondition()) {
+      if (singleProbe) {
+        // sampling was already done in isReadToCapture so we assume that if we are executing the
+        // current method it means the status should be sampled
+        if (!logStatus.getDebugSessionStatus().isDisabled()) {
+          logStatus.setSampled(true);
+        }
+      } else {
+        // sample when no condition associated
+        sample(logStatus, methodLocation);
       }
-    } else if (!hasCondition()) {
-      // sample when no condition associated
-      sample(logStatus, methodLocation);
     }
     logStatus.setCondition(evaluateCondition(context, logStatus));
     CapturedContext.CapturedThrowable throwable = context.getCapturedThrowable();
diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ProbeDefinition.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/ProbeDefinition.java
@@ -137,7 +137,10 @@ public ProbeLocation getLocation() {
 
   @Override
   public void evaluate(
-      CapturedContext context, CapturedContext.Status status, MethodLocation methodLocation) {}
+      CapturedContext context,
+      CapturedContext.Status status,
+      MethodLocation methodLocation,
+      boolean singleProbe) {}
 
   @Override
   public void commit(
diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/SpanDecorationProbe.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/SpanDecorationProbe.java
@@ -143,7 +143,10 @@ public InstrumentationResult.Status instrument(
 
   @Override
   public void evaluate(
-      CapturedContext context, CapturedContext.Status status, MethodLocation methodLocation) {
+      CapturedContext context,
+      CapturedContext.Status status,
+      MethodLocation methodLocation,
+      boolean singleProbe) {
     for (Decoration decoration : decorations) {
       if (decoration.when != null) {
         try {
diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/TriggerProbe.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/TriggerProbe.java
@@ -96,7 +96,10 @@ public TriggerProbe setProbeCondition(ProbeCondition probeCondition) {
 
   @Override
   public void evaluate(
-      CapturedContext context, CapturedContext.Status status, MethodLocation location) {
+      CapturedContext context,
+      CapturedContext.Status status,
+      MethodLocation location,
+      boolean singleProbe) {
 
     Sampling sampling = getSampling();
     if (sampling == null || !sampling.inCoolDown()) {
diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/SnapshotSerializationTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/SnapshotSerializationTest.java
@@ -127,7 +127,8 @@ public void roundTripCapturedValue() throws IOException, URISyntaxException {
         new ProbeImplementation.NoopProbeImplementation(PROBE_ID, PROBE_LOCATION),
         String.class.getTypeName(),
         -1,
-        MethodLocation.EXIT);
+        MethodLocation.EXIT,
+        false);
     snapshot.setExit(context);
     String buffer = adapter.toJson(snapshot);
     Snapshot deserializedSnapshot = adapter.fromJson(buffer);
diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/exception/DefaultExceptionDebuggerTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/exception/DefaultExceptionDebuggerTest.java
@@ -367,7 +367,8 @@ private void evalAndCommitProbe(RuntimeException exception, ExceptionProbe excep
     exceptionProbe.buildLocation(null);
     CapturedContext capturedContext = new CapturedContext();
     capturedContext.addThrowable(exception);
-    capturedContext.evaluate(exceptionProbe, "", System.currentTimeMillis(), MethodLocation.EXIT);
+    capturedContext.evaluate(
+        exceptionProbe, "", System.currentTimeMillis(), MethodLocation.EXIT, false);
     exceptionProbe.commit(CapturedContext.EMPTY_CAPTURING_CONTEXT, capturedContext, emptyList());
   }
 
diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/probe/LogProbeTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/probe/LogProbeTest.java
@@ -147,8 +147,8 @@ private int runTrace(TracerAPI tracer, boolean captureSnapshot, Integer line, St
 
       CapturedContext entryContext = capturedContext(span, logProbe);
       CapturedContext exitContext = capturedContext(span, logProbe);
-      logProbe.evaluate(entryContext, new LogStatus(logProbe), MethodLocation.ENTRY);
-      logProbe.evaluate(exitContext, new LogStatus(logProbe), MethodLocation.EXIT);
+      logProbe.evaluate(entryContext, new LogStatus(logProbe), MethodLocation.ENTRY, false);
+      logProbe.evaluate(exitContext, new LogStatus(logProbe), MethodLocation.EXIT, false);
 
       int budget =
           logProbe.isCaptureSnapshot()
@@ -193,8 +193,8 @@ private boolean fillSnapshot(DebugSessionStatus status) {
 
       CapturedContext entryContext = capturedContext(span, logProbe);
       CapturedContext exitContext = capturedContext(span, logProbe);
-      logProbe.evaluate(entryContext, new LogStatus(logProbe), MethodLocation.ENTRY);
-      logProbe.evaluate(exitContext, new LogStatus(logProbe), MethodLocation.EXIT);
+      logProbe.evaluate(entryContext, new LogStatus(logProbe), MethodLocation.ENTRY, false);
+      logProbe.evaluate(exitContext, new LogStatus(logProbe), MethodLocation.EXIT, false);
 
       return logProbe.fillSnapshot(
           entryContext, exitContext, emptyList(), new Snapshot(currentThread(), logProbe, 3));
@@ -204,7 +204,11 @@ private boolean fillSnapshot(DebugSessionStatus status) {
   private static CapturedContext capturedContext(AgentSpan span, ProbeDefinition probeDefinition) {
     CapturedContext context = new CapturedContext();
     context.evaluate(
-        probeDefinition, "Log Probe test", System.currentTimeMillis(), MethodLocation.DEFAULT);
+        probeDefinition,
+        "Log Probe test",
+        System.currentTimeMillis(),
+        MethodLocation.DEFAULT,
+        false);
     return context;
   }
 
@@ -283,7 +287,7 @@ private void fillStatus(
 
   private LogStatus prepareContext(
       CapturedContext context, LogProbe logProbe, MethodLocation methodLocation) {
-    context.evaluate(logProbe, "", 0, methodLocation);
+    context.evaluate(logProbe, "", 0, methodLocation, false);
     return (LogStatus) context.getStatus(PROBE_ID.getEncodedId());
   }
 

Original file line number	Diff line number	Diff line change
`@@ -71,7 +71,10 @@ public CapturedContext.Status createStatus() {`
`71`	`71`
`72`	`72`	`@Override`
`73`	`73`	`public void evaluate(`
`74`		`- CapturedContext context, CapturedContext.Status status, MethodLocation methodLocation) {`
	`74`	`+ CapturedContext context,`
	`75`	`+ CapturedContext.Status status,`
	`76`	`+ MethodLocation methodLocation,`
	`77`	`+ boolean singleProbe) {`
`75`	`78`	`ExceptionProbeStatus exceptionStatus;`
`76`	`79`	`if (status instanceof ExceptionProbeStatus) {`
`77`	`80`	`exceptionStatus = (ExceptionProbeStatus) status;`
`@@ -108,7 +111,7 @@ public void evaluate(`
`108`	`111`	`exceptionStatus.setForceSampling(true);`
`109`	`112`	`}`
`110`	`113`	`exceptionStatus.setCapture(true);`
`111`		`- super.evaluate(context, status, methodLocation);`
	`114`	`+ super.evaluate(context, status, methodLocation, singleProbe);`
`112`	`115`	`}`
`113`	`116`	`}`
`114`	`117`
Original file line number	Diff line number	Diff line change
`@@ -367,7 +367,8 @@ private void evalAndCommitProbe(RuntimeException exception, ExceptionProbe excep`
`367`	`367`	`exceptionProbe.buildLocation(null);`
`368`	`368`	`CapturedContext capturedContext = new CapturedContext();`
`369`	`369`	`capturedContext.addThrowable(exception);`
`370`		`- capturedContext.evaluate(exceptionProbe, "", System.currentTimeMillis(), MethodLocation.EXIT);`
	`370`	`+ capturedContext.evaluate(`
	`371`	`+ exceptionProbe, "", System.currentTimeMillis(), MethodLocation.EXIT, false);`
`371`	`372`	`exceptionProbe.commit(CapturedContext.EMPTY_CAPTURING_CONTEXT, capturedContext, emptyList());`
`372`	`373`	`}`
`373`	`374`