Add telemetry metrics for dropped/skipped events

dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/DebuggerContext.java

@@ -21,35 +21,6 @@ public class DebuggerContext {
   private static final Logger LOGGER = LoggerFactory.getLogger(DebuggerContext.class);
   private static final ThreadLocal<Boolean> IN_PROBE = ThreadLocal.withInitial(() -> Boolean.FALSE);
 
-  public enum SkipCause {
-    RATE {
-      @Override
-      public String tag() {
-        return "cause:rate";
-      }
-    },
-    CONDITION {
-      @Override
-      public String tag() {
-        return "cause:condition";
-      }
-    },
-    DEBUG_SESSION_DISABLED {
-      @Override
-      public String tag() {
-        return "cause:debug session disabled";
-      }
-    },
-    BUDGET {
-      @Override
-      public String tag() {
-        return "cause:budget_exceeded";
-      }
-    };
-
-    public abstract String tag();
-  }
-
   public interface ProbeResolver {
     ProbeImplementation resolve(int probeIndex);
   }

dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/EvaluationTimeOutException.java

@@ -0,0 +1,7 @@
+package com.datadog.debugger.el;
+
+public class EvaluationTimeOutException extends EvaluationException {
+  public EvaluationTimeOutException(String message, String expr) {
+    super(message, expr);
+  }
+}

dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/ComparisonExpression.java

@@ -4,6 +4,7 @@
 
 import com.datadog.debugger.el.EvalContext;
 import com.datadog.debugger.el.EvaluationException;
+import com.datadog.debugger.el.EvaluationTimeOutException;
 import com.datadog.debugger.el.PrettyPrintVisitor;
 import com.datadog.debugger.el.Value;
 import com.datadog.debugger.el.Visitor;
@@ -38,6 +39,8 @@ public Boolean evaluate(EvalContext evalContext) {
       boolean result = operator.apply(leftValue, rightValue);
       checkTimeout(evalContext.getTimeoutChecker(), this);
       return result;
+    } catch (EvaluationTimeOutException e) {
+      throw new EvaluationTimeOutException(e.getMessage(), PrettyPrintVisitor.print(this));
     } catch (EvaluationException e) {
       throw new EvaluationException(e.getMessage(), PrettyPrintVisitor.print(this));
     }

dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/ExpressionHelper.java

@@ -1,6 +1,7 @@
 package com.datadog.debugger.el.expressions;
 
 import com.datadog.debugger.el.EvaluationException;
+import com.datadog.debugger.el.EvaluationTimeOutException;
 import com.datadog.debugger.el.Expression;
 import com.datadog.debugger.el.PrettyPrintVisitor;
 import com.datadog.debugger.el.RedactedException;
@@ -20,7 +21,7 @@ public static void throwRedactedException(Expression<?> expr) {
 
   public static void checkTimeout(TimeoutChecker checker, Expression<?> expr) {
     if (checker.isTimedOut()) {
-      throw new EvaluationException(
+      throw new EvaluationTimeOutException(
           "timeout (" + checker.getTimeOut().toMillis() + "ms)", PrettyPrintVisitor.print(expr));
     }
   }

dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/IsDefinedExpression.java

@@ -4,6 +4,7 @@
 
 import com.datadog.debugger.el.EvalContext;
 import com.datadog.debugger.el.EvaluationException;
+import com.datadog.debugger.el.EvaluationTimeOutException;
 import com.datadog.debugger.el.Value;
 import com.datadog.debugger.el.Visitor;
 
@@ -23,6 +24,8 @@ public Boolean evaluate(EvalContext evalContext) {
     try {
       Value<?> value = valueExpression.evaluate(evalContext);
       return value.isUndefined() ? Boolean.FALSE : Boolean.TRUE;
+    } catch (EvaluationTimeOutException ex) {
+      throw ex;
     } catch (EvaluationException ex) {
       return Boolean.FALSE;
     } finally {

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

@@ -21,7 +21,6 @@
 import com.datadog.debugger.symbol.WireFilter;
 import com.datadog.debugger.uploader.BatchUploader;
 import com.datadog.debugger.util.ClassNameFiltering;
-import com.datadog.debugger.util.DebuggerMetrics;
 import datadog.communication.ddagent.DDAgentFeaturesDiscovery;
 import datadog.communication.ddagent.SharedCommunicationObjects;
 import datadog.remoteconfig.ConfigurationPoller;
@@ -30,6 +29,7 @@
 import datadog.trace.api.config.DebuggerConfig;
 import datadog.trace.api.config.TraceInstrumentationConfig;
 import datadog.trace.api.debugger.DebuggerConfigBridge;
+import datadog.trace.api.debugger.DebuggerMetricCollector;
 import datadog.trace.api.flare.TracerFlare;
 import datadog.trace.api.git.GitInfo;
 import datadog.trace.api.git.GitInfoProvider;
@@ -368,12 +368,7 @@ private static DebuggerSink createDebuggerSink(
     SnapshotSink snapshotSink = new SnapshotSink(config, tags, lowRateUploader, highRateUploader);
     SymbolSink symbolSink = new SymbolSink(config);
     return new DebuggerSink(
-        config,
-        tags,
-        DebuggerMetrics.getInstance(config),
-        probeStatusSink,
-        snapshotSink,
-        symbolSink);
+        config, tags, DebuggerMetricCollector.get(), probeStatusSink, snapshotSink, symbolSink);
   }
 
   public static String getDefaultTagsMergedWithGlobalTags(Config config) {

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

@@ -24,12 +24,12 @@
 import com.datadog.debugger.sink.SymbolSink;
 import com.datadog.debugger.uploader.BatchUploader;
 import com.datadog.debugger.util.ClassFileLines;
-import com.datadog.debugger.util.DebuggerMetrics;
 import com.datadog.debugger.util.SpringHelper;
 import datadog.environment.JavaVirtualMachine;
 import datadog.environment.SystemProperties;
 import datadog.trace.agent.tooling.AgentStrategies;
 import datadog.trace.api.Config;
+import datadog.trace.api.debugger.DebuggerMetricCollector;
 import datadog.trace.bootstrap.debugger.MethodLocation;
 import datadog.trace.bootstrap.debugger.ProbeId;
 import datadog.trace.bootstrap.debugger.ProbeImplementation;
@@ -197,7 +197,7 @@ public DebuggerTransformer(
         new DebuggerSink(
             config,
             "",
-            DebuggerMetrics.getInstance(config),
+            DebuggerMetricCollector.get(),
             new ProbeStatusSink(config, config.getFinalDebuggerSnapshotUrl(), false),
             new SnapshotSink(
                 config,

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/probe/LogProbe.java

@@ -1,12 +1,14 @@
 package com.datadog.debugger.probe;
 
 import static com.datadog.debugger.probe.LogProbe.Capture.toLimits;
+import static datadog.trace.api.debugger.DebuggerMetricCollector.SkippedReason.RATE_LIMIT;
 import static java.lang.String.format;
 
 import com.datadog.debugger.agent.DebuggerAgent;
 import com.datadog.debugger.agent.Generated;
 import com.datadog.debugger.agent.StringTemplateBuilder;
 import com.datadog.debugger.el.EvaluationException;
+import com.datadog.debugger.el.EvaluationTimeOutException;
 import com.datadog.debugger.el.ProbeCondition;
 import com.datadog.debugger.el.Value;
 import com.datadog.debugger.el.ValueScript;
@@ -25,10 +27,10 @@
 import datadog.trace.api.Config;
 import datadog.trace.api.CorrelationIdentifier;
 import datadog.trace.api.DDTraceId;
+import datadog.trace.api.debugger.DebuggerMetricCollector;
 import datadog.trace.api.sampling.Sampler;
 import datadog.trace.bootstrap.debugger.CapturedContext;
 import datadog.trace.bootstrap.debugger.CapturedContextProbe;
-import datadog.trace.bootstrap.debugger.DebuggerContext;
 import datadog.trace.bootstrap.debugger.EvaluationError;
 import datadog.trace.bootstrap.debugger.Limits;
 import datadog.trace.bootstrap.debugger.MethodLocation;
@@ -509,7 +511,11 @@ public InstrumentationResult.Status instrument(
   public boolean isReadyToCapture() {
     if (!hasCondition()) {
       // we are sampling here to avoid creating CapturedContext when the sampling result is negative
-      return ProbeRateLimiter.tryProbe(sampler, isFullSnapshot());
+      boolean sampled = ProbeRateLimiter.tryProbe(sampler, isFullSnapshot());
+      if (!sampled) {
+        DebuggerAgent.getSink().skipSnapshot(id, RATE_LIMIT);
+      }
+      return sampled;
     }
     return true;
   }
@@ -583,13 +589,8 @@ private void sample(LogStatus logStatus, MethodLocation methodLocation) {
         !logStatus.getDebugSessionStatus().isDisabled()
             && ProbeRateLimiter.tryProbe(localSampler, isFullSnapshot());
     logStatus.setSampled(sampled);
-    if (!sampled) {
-      DebuggerAgent.getSink()
-          .skipSnapshot(
-              id,
-              logStatus.getDebugSessionStatus().isDisabled()
-                  ? DebuggerContext.SkipCause.DEBUG_SESSION_DISABLED
-                  : DebuggerContext.SkipCause.RATE);
+    if (!sampled && !logStatus.getDebugSessionStatus().isDisabled()) {
+      DebuggerAgent.getSink().skipSnapshot(id, RATE_LIMIT);
     }
   }
 
@@ -603,6 +604,12 @@ private boolean evaluateCondition(CapturedContext capture, LogStatus status) {
       if (!probeCondition.execute(capture, timeoutChecker)) {
         return false;
       }
+    } catch (EvaluationTimeOutException ex) {
+      DebuggerAgent.getSink()
+          .skipSnapshot(id, DebuggerMetricCollector.SkippedReason.EVALUATION_TIME_OUT);
+      status.addError(new EvaluationError(ex.getExpr(), ex.getMessage()));
+      status.setConditionErrors(true);
+      return false;
     } catch (EvaluationException ex) {
       status.addError(new EvaluationError(ex.getExpr(), ex.getMessage()));
       status.setConditionErrors(true);
@@ -631,11 +638,7 @@ public void commit(
         if (snapshotProcessor != null) {
           snapshotProcessor.accept(snapshot);
         }
-      } else {
-        sink.skipSnapshot(id, DebuggerContext.SkipCause.BUDGET);
       }
-    } else {
-      sink.skipSnapshot(id, DebuggerContext.SkipCause.CONDITION);
     }
   }
 
@@ -857,7 +860,6 @@ public void commit(CapturedContext lineContext, int line) {
         return;
       }
     }
-    sink.skipSnapshot(id, DebuggerContext.SkipCause.CONDITION);
   }
 
   @Override

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

@@ -1,17 +1,17 @@
 package com.datadog.debugger.sink;
 
+import static datadog.trace.api.debugger.DebuggerMetricCollector.DroppedReason.QUEUE_FULL;
+
 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;
+import datadog.trace.api.debugger.DebuggerMetricCollector;
 import datadog.trace.api.internal.VisibleForTesting;
-import datadog.trace.bootstrap.debugger.DebuggerContext.SkipCause;
 import datadog.trace.bootstrap.debugger.ProbeId;
 import datadog.trace.util.AgentTaskScheduler;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicLong;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -25,18 +25,12 @@ public class DebuggerSink {
   private static final long LOW_RATE_INITIAL_FLUSH_INTERVAL = 1000;
   static final long LOW_RATE_STEP_SIZE = 200;
   private static final String PREFIX = "debugger.sink.";
-  private static final String DROPPED_REQ_METRIC = PREFIX + "dropped.requests";
-  private static final String UPLOAD_REMAINING_CAP_METRIC =
-      PREFIX + "upload.queue.remaining.capacity";
-  private static final String CURRENT_FLUSH_INTERVAL_METRIC = PREFIX + "current.flush.interval";
-  private static final String SKIP_METRIC = PREFIX + "skip";
 
   private final ProbeStatusSink probeStatusSink;
   private final SnapshotSink snapshotSink;
   private final SymbolSink symbolSink;
-  private final DebuggerMetrics debuggerMetrics;
+  private final DebuggerMetricCollector metricCollector;
   private final String tags;
-  private final AtomicLong highRateDropped = new AtomicLong();
   private final int uploadFlushInterval;
   private final AgentTaskScheduler lowRateScheduler = AgentTaskScheduler.get();
   private volatile AgentTaskScheduler.Scheduled<DebuggerSink> lowRateScheduled;
@@ -47,7 +41,7 @@ public DebuggerSink(Config config, ProbeStatusSink probeStatusSink) {
     this(
         config,
         null,
-        DebuggerMetrics.getInstance(config),
+        DebuggerMetricCollector.get(),
         probeStatusSink,
         new SnapshotSink(
             config,
@@ -65,12 +59,12 @@ public DebuggerSink(Config config, ProbeStatusSink probeStatusSink) {
   public DebuggerSink(
       Config config,
       String tags,
-      DebuggerMetrics debuggerMetrics,
+      DebuggerMetricCollector metricCollector,
       ProbeStatusSink probeStatusSink,
       SnapshotSink snapshotSink,
       SymbolSink symbolSink) {
     this.tags = tags;
-    this.debuggerMetrics = debuggerMetrics;
+    this.metricCollector = metricCollector;
     this.probeStatusSink = probeStatusSink;
     this.snapshotSink = snapshotSink;
     this.symbolSink = symbolSink;
@@ -123,7 +117,7 @@ public SymbolSink getSymbolSink() {
   public void addSnapshot(Snapshot snapshot) {
     boolean added = snapshotSink.addLowRate(snapshot);
     if (!added) {
-      debuggerMetrics.count(DROPPED_REQ_METRIC, 1);
+      metricCollector.recordEventDropped(QUEUE_FULL);
     } else {
       if (!(snapshot.getProbe() instanceof ExceptionProbe)) {
         // do not report emitting for exception probes
@@ -135,10 +129,7 @@ public void addSnapshot(Snapshot snapshot) {
   public void addHighRateSnapshot(Snapshot snapshot) {
     boolean added = snapshotSink.addHighRate(snapshot);
     if (!added) {
-      long dropped = highRateDropped.incrementAndGet();
-      if (dropped % 100 == 0) {
-        debuggerMetrics.count(DROPPED_REQ_METRIC, 100);
-      }
+      metricCollector.recordEventDropped(QUEUE_FULL);
     } else {
       probeStatusSink.addEmitting(snapshot.getProbe().getProbeId());
     }
@@ -168,8 +159,6 @@ void lowRateFlush(DebuggerSink ignored) {
   }
 
   private void reconsiderLowRateFlushInterval(DebuggerSink debuggerSink) {
-    debuggerMetrics.histogram(UPLOAD_REMAINING_CAP_METRIC, snapshotSink.remainingCapacity());
-    debuggerMetrics.histogram(CURRENT_FLUSH_INTERVAL_METRIC, currentLowRateFlushInterval);
     doReconsiderLowRateFlushInterval();
   }
 
@@ -242,8 +231,8 @@ private void reportError(ProbeId probeId, DiagnosticMessage msg) {
   }
 
   /** Notifies the snapshot was skipped for one of the SkipCause reason */
-  public void skipSnapshot(String probeId, SkipCause cause) {
-    debuggerMetrics.incrementCounter(SKIP_METRIC, cause.tag(), "probe_id:" + probeId);
+  public void skipSnapshot(String probeId, DebuggerMetricCollector.SkippedReason reason) {
+    metricCollector.recordEventSkipped(reason);
   }
 
   long getCurrentLowRateFlushInterval() {

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

@@ -29,7 +29,7 @@ public class SnapshotSink {
   public static final int LOW_RATE_CAPACITY = 1024;
   static final int HIGH_RATE_MIN_FLUSH_INTERVAL_MS = 1;
   static final int HIGH_RATE_MAX_FLUSH_INTERVAL_MS = 100;
-  private static final int HIGH_RATE_CAPACITY = 1024;
+  public static final int HIGH_RATE_CAPACITY = 1024;
   private static final int HIGH_RATE_10_PERCENT_CAPACITY = HIGH_RATE_CAPACITY / 10;
   private static final int HIGH_RATE_25_PERCENT_CAPACITY = HIGH_RATE_CAPACITY / 4;
   private static final int HIGH_RATE_75_PERCENT_CAPACITY = HIGH_RATE_CAPACITY * 3 / 4;

dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java

@@ -2041,8 +2041,6 @@ public void evaluateAtExitFalse() throws IOException, URISyntaxException {
     int result = Reflect.onClass(testClass).call("main", "1").get();
     assertEquals(3, result);
     assertEquals(0, listener.snapshots.size());
-    assertTrue(listener.skipped);
-    assertEquals(DebuggerContext.SkipCause.CONDITION, listener.cause);
   }
 
   @Test

dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/LogProbesInstrumentationTest.java

@@ -601,7 +601,7 @@ private Snapshot assertOneSnapshot(TestSnapshotListener listener) {
   }
 
   private Snapshot assertOneSnapshot(ProbeId probeId, TestSnapshotListener listener) {
-    Assertions.assertFalse(listener.skipped, "Snapshot skipped because " + listener.cause);
+    Assertions.assertFalse(listener.skipped, "Snapshot skipped because " + listener.reason);
     Assertions.assertEquals(1, listener.snapshots.size());
     Snapshot snapshot = listener.snapshots.get(0);
     Assertions.assertEquals(probeId.getId(), snapshot.getProbe().getId());

dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/ProbeInstrumentationTest.java

@@ -4,7 +4,7 @@
 import com.datadog.debugger.sink.ProbeStatusSink;
 import com.datadog.debugger.sink.Snapshot;
 import datadog.trace.api.Config;
-import datadog.trace.bootstrap.debugger.DebuggerContext;
+import datadog.trace.api.debugger.DebuggerMetricCollector;
 import java.lang.instrument.ClassFileTransformer;
 import java.lang.instrument.Instrumentation;
 import java.util.ArrayList;
@@ -46,7 +46,7 @@ public void addHighRateSnapshot(Snapshot snapshot) {
     }
 
     @Override
-    public void skipSnapshot(String probeId, DebuggerContext.SkipCause cause) {}
+    public void skipSnapshot(String probeId, DebuggerMetricCollector.SkippedReason reason) {}
 
     public List<Snapshot> getSnapshots() {
       return snapshots;