Fix Exception Probes to not emit any probe status (#9669)

Fix Exception Probes to not emit any probe status

filter on probe definition type

Author: jpbempel

dd-java-agent/agent-ci-visibility/civisibility-test-fixtures/src/main/groovy/datadog/trace/civisibility/CiVisibilitySmokeTest.groovy

@@ -149,7 +149,7 @@ abstract class CiVisibilitySmokeTest extends Specification {
     assert logs.findAll { log -> ((String) log.message).endsWith("is emitting.") }.size() == expectedCount
   }
 
-  private static verifySnapshots(List<Map<String, Object>> logs, expectedCount) {
+  protected static verifySnapshots(List<Map<String, Object>> logs, expectedCount) {
     assert logs.size() == expectedCount
 
     def requiredLogFields = ["logger.name", "logger.method", "dd.spanid", "dd.traceid"]

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/ConfigurationUpdater.java

@@ -7,6 +7,7 @@
 import static datadog.trace.api.telemetry.LogCollector.SEND_TELEMETRY;
 
 import com.datadog.debugger.instrumentation.InstrumentationResult;
+import com.datadog.debugger.probe.ExceptionProbe;
 import com.datadog.debugger.probe.LogProbe;
 import com.datadog.debugger.probe.ProbeDefinition;
 import com.datadog.debugger.probe.Sampled;
@@ -176,6 +177,10 @@ private void handleProbesChanges(ConfigurationComparer changes, Configuration ne
 
   private void reportReceived(ConfigurationComparer changes) {
     for (ProbeDefinition def : changes.getAddedDefinitions()) {
+      if (def instanceof ExceptionProbe) {
+        // do not report received for exception probes
+        continue;
+      }
       sink.addReceived(def.getProbeId());
     }
     for (ProbeDefinition def : changes.getRemovedDefinitions()) {

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerTransformer.java

@@ -590,6 +590,10 @@ private void handleInstrumentationResult(
       if (listener != null) {
         listener.instrumentationResult(definition, result);
       }
+      if (definition instanceof ExceptionProbe) {
+        // do not report diagnostics for exception probes
+        continue;
+      }
       List<DiagnosticMessage> diagnosticMessages =
           result.getDiagnostics().get(definition.getProbeId());
       if (!result.getDiagnostics().isEmpty()) {

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/sink/DebuggerSink.java

@@ -1,6 +1,7 @@
 package com.datadog.debugger.sink;
 
 import com.datadog.debugger.instrumentation.DiagnosticMessage;
+import com.datadog.debugger.probe.ExceptionProbe;
 import com.datadog.debugger.uploader.BatchUploader;
 import com.datadog.debugger.util.DebuggerMetrics;
 import datadog.trace.api.Config;
@@ -123,7 +124,10 @@ public void addSnapshot(Snapshot snapshot) {
     if (!added) {
       debuggerMetrics.count(DROPPED_REQ_METRIC, 1);
     } else {
-      probeStatusSink.addEmitting(snapshot.getProbe().getProbeId());
+      if (!(snapshot.getProbe() instanceof ExceptionProbe)) {
+        // do not report emitting for exception probes
+        probeStatusSink.addEmitting(snapshot.getProbe().getProbeId());
+      }
     }
   }
 

dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/ExceptionDebuggerIntegrationTest.java

@@ -49,7 +49,7 @@ protected ProcessBuilder createProcessBuilder(Path logFilePath, String... params
   void testSimpleSingleFrameException() throws Exception {
     appUrl = startAppAndAndGetUrl();
     execute(appUrl, TRACED_METHOD_NAME, "oops"); // instrumenting first exception
-    waitForInstrumentation(appUrl);
+    waitForInstrumentation(appUrl, SERVER_DEBUGGER_TEST_APP_CLASS, false);
     execute(appUrl, TRACED_METHOD_NAME, "oops"); // collecting snapshots and sending them
     registerTraceListener(this::receiveExceptionReplayTrace);
     registerSnapshotListener(this::receiveSnapshot);
@@ -116,7 +116,7 @@ void testNoSubsequentCaptureAfterFirst() throws Exception {
   void test3CapturedFrames() throws Exception {
     appUrl = startAppAndAndGetUrl();
     execute(appUrl, TRACED_METHOD_NAME, "deepOops"); // instrumenting first exception
-    waitForInstrumentation(appUrl);
+    waitForInstrumentation(appUrl, SERVER_DEBUGGER_TEST_APP_CLASS, false);
     execute(appUrl, TRACED_METHOD_NAME, "deepOops"); // collecting snapshots and sending them
     registerTraceListener(this::receiveExceptionReplayTrace);
     registerSnapshotListener(this::receiveSnapshot);
@@ -173,7 +173,7 @@ void test5CapturedFrames() throws Exception {
     additionalJvmArgs.add("-Ddd.exception.replay.capture.max.frames=5");
     appUrl = startAppAndAndGetUrl();
     execute(appUrl, TRACED_METHOD_NAME, "deepOops"); // instrumenting first exception
-    waitForInstrumentation(appUrl);
+    waitForInstrumentation(appUrl, SERVER_DEBUGGER_TEST_APP_CLASS, false);
     execute(appUrl, TRACED_METHOD_NAME, "deepOops"); // collecting snapshots and sending them
     registerTraceListener(this::receiveExceptionReplayTrace);
     registerSnapshotListener(this::receiveSnapshot);
@@ -249,7 +249,7 @@ void test5CapturedFrames() throws Exception {
   void test3CapturedRecursiveFrames() throws Exception {
     appUrl = startAppAndAndGetUrl();
     execute(appUrl, TRACED_METHOD_NAME, "recursiveOops"); // instrumenting first exception
-    waitForInstrumentation(appUrl);
+    waitForInstrumentation(appUrl, SERVER_DEBUGGER_TEST_APP_CLASS, false);
     execute(appUrl, TRACED_METHOD_NAME, "recursiveOops"); // collecting snapshots and sending them
     registerTraceListener(this::receiveExceptionReplayTrace);
     registerSnapshotListener(this::receiveSnapshot);
@@ -296,7 +296,7 @@ void testLambdaHiddenFrames() throws Exception {
     additionalJvmArgs.add("-XX:+ShowHiddenFrames");
     appUrl = startAppAndAndGetUrl();
     execute(appUrl, TRACED_METHOD_NAME, "lambdaOops"); // instrumenting first exception
-    waitForInstrumentation(appUrl);
+    waitForInstrumentation(appUrl, SERVER_DEBUGGER_TEST_APP_CLASS, false);
     execute(appUrl, TRACED_METHOD_NAME, "lambdaOops"); // collecting snapshots and sending them
     registerTraceListener(this::receiveExceptionReplayTrace);
     registerSnapshotListener(this::receiveSnapshot);

dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/InProductEnablementIntegrationTest.java

@@ -51,7 +51,7 @@ void testDynamicInstrumentationEnablementWithLineProbe() throws Exception {
     setCurrentConfiguration(createConfig(probe));
     waitForFeatureStarted(appUrl, "Dynamic Instrumentation");
     execute(appUrl, "topLevelMethod", "");
-    waitForInstrumentation(appUrl, "datadog.smoketest.debugger.TopLevel");
+    waitForInstrumentation(appUrl, "datadog.smoketest.debugger.TopLevel", true);
     // disable DI
     setConfigOverrides(createConfigOverrides(false, false));
     waitForFeatureStopped(appUrl, "Dynamic Instrumentation");
@@ -81,7 +81,7 @@ void testExceptionReplayEnablement() throws Exception {
     setConfigOverrides(createConfigOverrides(false, true));
     waitForFeatureStarted(appUrl, "Exception Replay");
     execute(appUrl, TRACED_METHOD_NAME, "oops"); // instrumenting first exception
-    waitForInstrumentation(appUrl);
+    waitForInstrumentation(appUrl, SERVER_DEBUGGER_TEST_APP_CLASS, false);
     // disable ER
     setConfigOverrides(createConfigOverrides(false, false));
     waitForFeatureStopped(appUrl, "Exception Replay");

dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/ServerAppDebuggerIntegrationTest.java

@@ -99,25 +99,28 @@ protected void execute(String appUrl, String methodName, String arg) throws IOEx
   }
 
   protected void waitForInstrumentation(String appUrl) throws Exception {
-    waitForInstrumentation(appUrl, SERVER_DEBUGGER_TEST_APP_CLASS);
+    waitForInstrumentation(appUrl, SERVER_DEBUGGER_TEST_APP_CLASS, true);
   }
 
-  protected void waitForInstrumentation(String appUrl, String className) throws Exception {
+  protected void waitForInstrumentation(
+      String appUrl, String className, boolean waitOnProbeStatuses) throws Exception {
     String url = String.format(appUrl + "/waitForInstrumentation?classname=%s", className);
     LOG.info("waitForInstrumentation with url={}", url);
     sendRequest(url);
-    AtomicBoolean received = new AtomicBoolean();
-    AtomicBoolean installed = new AtomicBoolean();
-    registerProbeStatusListener(
-        probeStatus -> {
-          if (probeStatus.getDiagnostics().getStatus() == ProbeStatus.Status.RECEIVED) {
-            received.set(true);
-          }
-          if (probeStatus.getDiagnostics().getStatus() == ProbeStatus.Status.INSTALLED) {
-            installed.set(true);
-          }
-        });
-    processRequests(() -> received.get() && installed.get());
+    if (waitOnProbeStatuses) {
+      AtomicBoolean received = new AtomicBoolean();
+      AtomicBoolean installed = new AtomicBoolean();
+      registerProbeStatusListener(
+          probeStatus -> {
+            if (probeStatus.getDiagnostics().getStatus() == ProbeStatus.Status.RECEIVED) {
+              received.set(true);
+            }
+            if (probeStatus.getDiagnostics().getStatus() == ProbeStatus.Status.INSTALLED) {
+              installed.set(true);
+            }
+          });
+      processRequests(() -> received.get() && installed.get());
+    }
     LOG.info("instrumentation done");
   }
 

dd-smoke-tests/junit-console/src/test/groovy/datadog/smoketest/JUnitConsoleSmokeTest.groovy

@@ -59,7 +59,7 @@ class JUnitConsoleSmokeTest extends CiVisibilitySmokeTest {
 
     def additionalDynamicTags = ["content.meta.['_dd.debug.error.6.snapshot_id']", "content.meta.['_dd.debug.error.exception_id']"]
     verifyEventsAndCoverages(projectName, "junit-console", "headless", mockBackend.waitForEvents(7), mockBackend.waitForCoverages(0), additionalDynamicTags)
-    verifySnapshotLogs(mockBackend.waitForLogs(5), 1, 2)
+    verifySnapshots(mockBackend.waitForLogs(2), 2)
 
     where:
     projectName = "test_junit_console_failed_test_replay"

dd-smoke-tests/maven/src/test/groovy/datadog/smoketest/MavenSmokeTest.groovy

@@ -247,7 +247,7 @@ class MavenSmokeTest extends CiVisibilitySmokeTest {
 
     def additionalDynamicTags = ["content.meta.['_dd.debug.error.3.snapshot_id']", "content.meta.['_dd.debug.error.exception_id']"]
     verifyEventsAndCoverages(projectName, "maven", mavenVersion, mockBackend.waitForEvents(7), mockBackend.waitForCoverages(0), additionalDynamicTags)
-    verifySnapshotLogs(mockBackend.waitForLogs(5), 1, 2)
+    verifySnapshots(mockBackend.waitForLogs(2), 2)
 
     where:
     projectName                            | mavenVersion