Capture exception snapshots only once an hour (#6983)

* Capture exception snapshots only once an hour

Keep a timestamp associated to the fingerprint to know when last time
we capture snapshots for this exception. Hardcoded once an hour will
probably make it a config token for this later

* Instant.MIN

remove comment

* use seconds instead of millisecs with Instant.MIN

Author: jpbempel

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

@@ -28,13 +28,15 @@
 import java.lang.ref.WeakReference;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.time.Duration;
 import java.util.zip.ZipOutputStream;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /** Debugger agent implementation */
 public class DebuggerAgent {
   private static final Logger LOGGER = LoggerFactory.getLogger(DebuggerAgent.class);
+  public static final Duration EXCEPTION_CAPTURE_INTERVAL = Duration.ofHours(1);
   private static ConfigurationPoller configurationPoller;
   private static DebuggerSink sink;
   private static String agentVersion;
@@ -81,7 +83,8 @@ public static synchronized void run(
     DebuggerContext.initTracer(new DebuggerTracer(debuggerSink.getProbeStatusSink()));
     if (config.isDebuggerExceptionEnabled()) {
       DefaultExceptionDebugger defaultExceptionDebugger =
-          new DefaultExceptionDebugger(configurationUpdater, classNameFiltering);
+          new DefaultExceptionDebugger(
+              configurationUpdater, classNameFiltering, EXCEPTION_CAPTURE_INTERVAL);
       DebuggerContext.initExceptionDebugger(defaultExceptionDebugger);
     }
     if (config.isDebuggerInstrumentTheWorld()) {

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/exception/DefaultExceptionDebugger.java

@@ -12,6 +12,7 @@
 import datadog.trace.bootstrap.debugger.DebuggerContext;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import datadog.trace.util.AgentTaskScheduler;
+import java.time.Duration;
 import java.util.List;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -32,8 +33,13 @@ public class DefaultExceptionDebugger implements DebuggerContext.ExceptionDebugg
   private final ClassNameFiltering classNameFiltering;
 
   public DefaultExceptionDebugger(
-      ConfigurationUpdater configurationUpdater, ClassNameFiltering classNameFiltering) {
-    this(new ExceptionProbeManager(classNameFiltering), configurationUpdater, classNameFiltering);
+      ConfigurationUpdater configurationUpdater,
+      ClassNameFiltering classNameFiltering,
+      Duration captureInterval) {
+    this(
+        new ExceptionProbeManager(classNameFiltering, captureInterval),
+        configurationUpdater,
+        classNameFiltering);
   }
 
   DefaultExceptionDebugger(
@@ -64,6 +70,7 @@ public void handleException(Throwable t, AgentSpan span) {
         return;
       }
       processSnapshotsAndSetTags(t, span, state, innerMostException);
+      exceptionProbeManager.updateLastCapture(fingerprint);
     } else {
       if (exceptionProbeManager.createProbesForException(innerMostException.getStackTrace())) {
         AgentTaskScheduler.INSTANCE.execute(() -> applyExceptionConfiguration(fingerprint));

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/exception/ExceptionProbeManager.java

@@ -7,12 +7,15 @@
 import com.datadog.debugger.util.ExceptionHelper;
 import com.datadog.debugger.util.WeakIdentityHashMap;
 import datadog.trace.bootstrap.debugger.ProbeId;
+import java.time.Clock;
+import java.time.Duration;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.ConcurrentHashMap;
 import org.slf4j.Logger;
@@ -22,15 +25,28 @@
 public class ExceptionProbeManager {
   private static final Logger LOGGER = LoggerFactory.getLogger(ExceptionProbeManager.class);
 
-  private final Set<String> fingerprints = ConcurrentHashMap.newKeySet();
+  private final Map<String, Instant> fingerprints = new ConcurrentHashMap<>();
   private final Map<String, ExceptionProbe> probes = new ConcurrentHashMap<>();
   private final ClassNameFiltering classNameFiltering;
   // FIXME: if this becomes a bottleneck, find a way to make it concurrent weak identity hashmap
   private final Map<Throwable, ThrowableState> snapshotsByThrowable =
       Collections.synchronizedMap(new WeakIdentityHashMap<>());
+  private final long captureIntervalS;
+  private final Clock clock;
 
-  public ExceptionProbeManager(ClassNameFiltering classNameFiltering) {
+  public ExceptionProbeManager(ClassNameFiltering classNameFiltering, Duration captureInterval) {
+    this(classNameFiltering, captureInterval, Clock.systemUTC());
+  }
+
+  ExceptionProbeManager(ClassNameFiltering classNameFiltering) {
+    this(classNameFiltering, Duration.ofHours(1), Clock.systemUTC());
+  }
+
+  ExceptionProbeManager(
+      ClassNameFiltering classNameFiltering, Duration captureInterval, Clock clock) {
     this.classNameFiltering = classNameFiltering;
+    this.captureIntervalS = captureInterval.getSeconds();
+    this.clock = clock;
   }
 
   public ClassNameFiltering getClassNameFiltering() {
@@ -62,7 +78,7 @@ public boolean createProbesForException(StackTraceElement[] stackTraceElements)
   }
 
   void addFingerprint(String fingerprint) {
-    fingerprints.add(fingerprint);
+    fingerprints.put(fingerprint, Instant.MIN);
   }
 
   private static ExceptionProbe createMethodProbe(
@@ -73,15 +89,23 @@ private static ExceptionProbe createMethodProbe(
   }
 
   public boolean isAlreadyInstrumented(String fingerprint) {
-    return fingerprints.contains(fingerprint);
+    return fingerprints.containsKey(fingerprint);
   }
 
   public Collection<ExceptionProbe> getProbes() {
     return probes.values();
   }
 
   public boolean shouldCaptureException(String fingerprint) {
-    return fingerprints.contains(fingerprint);
+    return shouldCaptureException(fingerprint, clock);
+  }
+
+  boolean shouldCaptureException(String fingerprint, Clock clock) {
+    Instant lastCapture = fingerprints.get(fingerprint);
+    if (lastCapture == null) {
+      return false;
+    }
+    return ChronoUnit.SECONDS.between(lastCapture, Instant.now(clock)) >= captureIntervalS;
   }
 
   public void addSnapshot(Snapshot snapshot) {
@@ -104,6 +128,14 @@ public ThrowableState getSateByThrowable(Throwable throwable) {
     return snapshotsByThrowable.get(throwable);
   }
 
+  public void updateLastCapture(String fingerprint) {
+    updateLastCapture(fingerprint, clock);
+  }
+
+  void updateLastCapture(String fingerprint, Clock clock) {
+    fingerprints.put(fingerprint, Instant.now(clock));
+  }
+
   public static class ThrowableState {
     private final String exceptionId;
     private List<Snapshot> snapshots;

dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/exception/DefaultExceptionDebuggerTest.java

@@ -58,7 +58,8 @@ class DefaultExceptionDebuggerTest {
   public void setUp() {
     configurationUpdater = mock(ConfigurationUpdater.class);
     classNameFiltering = new ClassNameFiltering(emptySet());
-    exceptionDebugger = new DefaultExceptionDebugger(configurationUpdater, classNameFiltering);
+    exceptionDebugger =
+        new DefaultExceptionDebugger(configurationUpdater, classNameFiltering, Duration.ofHours(1));
     listener = new TestSnapshotListener(createConfig(), mock(ProbeStatusSink.class));
     DebuggerAgentHelper.injectSink(listener);
   }

dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/exception/ExceptionProbeInstrumentationTest.java

@@ -35,7 +35,9 @@
 import datadog.trace.core.CoreTracer;
 import java.lang.instrument.ClassFileTransformer;
 import java.lang.instrument.Instrumentation;
+import java.time.Clock;
 import java.time.Duration;
+import java.time.Instant;
 import java.util.Map;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -216,6 +218,35 @@ public void recursive() throws Exception {
     assertEquals(1, globalSampler.getCallCount());
   }
 
+  @Test
+  public void captureOncePerHour() throws Exception {
+    Config config = createConfig();
+    Clock clockMock = mock(Clock.class);
+    when(clockMock.instant()).thenReturn(Instant.now());
+    ExceptionProbeManager exceptionProbeManager =
+        new ExceptionProbeManager(classNameFiltering, Duration.ofHours(1), clockMock);
+    TestSnapshotListener listener =
+        setupExceptionDebugging(config, exceptionProbeManager, classNameFiltering);
+    final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot20";
+    Class<?> testClass = compileAndLoadClass(CLASS_NAME);
+    // instrument RuntimeException stacktrace
+    String fingerprint0 = callMethodThrowingRuntimeException(testClass);
+    assertWithTimeout(
+        () -> exceptionProbeManager.isAlreadyInstrumented(fingerprint0), Duration.ofSeconds(30));
+    // generate snapshots RuntimeException
+    callMethodThrowingRuntimeException(testClass);
+    assertEquals(1, listener.snapshots.size());
+    listener.snapshots.clear();
+    // second call, no snapshot should be generated
+    callMethodThrowingRuntimeException(testClass);
+    assertEquals(0, listener.snapshots.size());
+    // Fast-forward 1 hour
+    when(clockMock.instant()).thenReturn(Instant.now().plus(Duration.ofMinutes(61)));
+    // second call, snapshot should be generated
+    callMethodThrowingRuntimeException(testClass);
+    assertEquals(1, listener.snapshots.size());
+  }
+
   private static void assertExceptionMsg(String expectedMsg, Snapshot snapshot) {
     assertEquals(
         expectedMsg, snapshot.getCaptures().getReturn().getCapturedThrowable().getMessage());

dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/exception/ExceptionProbeManagerTest.java

@@ -7,6 +7,9 @@
 import com.datadog.debugger.probe.ExceptionProbe;
 import com.datadog.debugger.util.ClassNameFiltering;
 import datadog.trace.api.Config;
+import java.time.Clock;
+import java.time.Duration;
+import java.time.Instant;
 import java.util.Collections;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -41,7 +44,7 @@ void instrumentSingleFrame() {
     ExceptionProbeManager exceptionProbeManager = new ExceptionProbeManager(classNameFiltering);
 
     String fingerprint = Fingerprinter.fingerprint(exception, classNameFiltering);
-    assertEquals("d2e9d63e304d95f6435d77bf4d0d387521591e550be21d432339a14ee1cb40", fingerprint);
+    assertEquals("1c27b291764c9d387fb85247bb7c2711f885aadfbf2f64fed34b2e0c64c5a2", fingerprint);
     exceptionProbeManager.createProbesForException(exception.getStackTrace());
     assertEquals(1, exceptionProbeManager.getProbes().size());
     ExceptionProbe exceptionProbe = exceptionProbeManager.getProbes().iterator().next();
@@ -70,4 +73,19 @@ void filterAllFrames() {
     exceptionProbeManager.createProbesForException(exception.getStackTrace());
     assertEquals(0, exceptionProbeManager.getProbes().size());
   }
+
+  @Test
+  void lastCapture() {
+    ClassNameFiltering classNameFiltering = ClassNameFiltering.allowAll();
+    ExceptionProbeManager exceptionProbeManager = new ExceptionProbeManager(classNameFiltering);
+    RuntimeException exception = new RuntimeException("test");
+    String fingerprint = Fingerprinter.fingerprint(exception, classNameFiltering);
+    exceptionProbeManager.addFingerprint(fingerprint);
+    assertTrue(exceptionProbeManager.shouldCaptureException(fingerprint));
+    exceptionProbeManager.updateLastCapture(fingerprint);
+    assertFalse(exceptionProbeManager.shouldCaptureException(fingerprint));
+    Clock clock =
+        Clock.fixed(Instant.now().plus(Duration.ofMinutes(61)), Clock.systemUTC().getZone());
+    assertTrue(exceptionProbeManager.shouldCaptureException(fingerprint, clock));
+  }
 }