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 index 77ad82b864a..cb2c204a7e8 100644 --- 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 @@ -21,35 +21,6 @@ public class DebuggerContext { private static final Logger LOGGER = LoggerFactory.getLogger(DebuggerContext.class); private static final ThreadLocal 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); } diff --git a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/EvaluationTimeOutException.java b/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/EvaluationTimeOutException.java new file mode 100644 index 00000000000..5de93d44552 --- /dev/null +++ b/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); + } +} diff --git a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/ComparisonExpression.java b/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/ComparisonExpression.java index 8a13febaad2..e14c47a67be 100644 --- a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/ComparisonExpression.java +++ b/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)); } diff --git a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/ExpressionHelper.java b/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/ExpressionHelper.java index 10b7a73911c..f7df3cf61e2 100644 --- a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/ExpressionHelper.java +++ b/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)); } } diff --git a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/IsDefinedExpression.java b/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/IsDefinedExpression.java index 27da03e06ae..24ac98be2d8 100644 --- a/dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/IsDefinedExpression.java +++ b/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 { diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java index d8dd9f5b8f5..2eaaf14cbab 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java +++ b/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) { diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerTransformer.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerTransformer.java index 6551b9cf9f6..ba70b3dd75b 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerTransformer.java +++ b/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, 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 index 0d4ac850c14..b85f38401f2 100644 --- 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 @@ -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 diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/sink/DebuggerSink.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/sink/DebuggerSink.java index becf47b0337..4ad1390b00e 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/sink/DebuggerSink.java +++ b/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 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() { diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/sink/SnapshotSink.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/sink/SnapshotSink.java index e5f44254aca..694d92112c1 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/sink/SnapshotSink.java +++ b/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; diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java index e48b7514685..5144cc9f7c3 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java +++ b/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 diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/LogProbesInstrumentationTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/LogProbesInstrumentationTest.java index 1066f3d269d..7777a1d133a 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/LogProbesInstrumentationTest.java +++ b/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()); diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/ProbeInstrumentationTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/ProbeInstrumentationTest.java index 56a327b209c..d7908a16004 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/ProbeInstrumentationTest.java +++ b/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 getSnapshots() { return snapshots; 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 index c74396b10e5..1928cc52829 100644 --- 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 @@ -2,6 +2,8 @@ import static com.datadog.debugger.agent.CapturingTestBase.getConfig; import static com.datadog.debugger.util.LogProbeTestHelper.parseTemplate; +import static datadog.trace.api.debugger.DebuggerMetricCollector.SkippedReason.EVALUATION_TIME_OUT; +import static datadog.trace.api.debugger.DebuggerMetricCollector.SkippedReason.RATE_LIMIT; import static java.lang.String.format; import static java.lang.Thread.currentThread; import static java.util.Collections.emptyList; @@ -9,10 +11,18 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import com.datadog.debugger.agent.DebuggerAgentHelper; import com.datadog.debugger.el.DSL; +import com.datadog.debugger.el.EvaluationTimeOutException; import com.datadog.debugger.el.ProbeCondition; import com.datadog.debugger.el.ValueScript; import com.datadog.debugger.probe.LogProbe.Builder; @@ -390,6 +400,65 @@ public void captureExpressionsInActiveDebugSession() { } } + @Test + public void isReadyToCaptureRateLimitedRecordsSkip() { + DebuggerSink sink = spy(new DebuggerSink(getConfig(), mock(ProbeStatusSink.class))); + DebuggerAgentHelper.injectSink(sink); + try { + ProbeRateLimiter.setSamplerSupplier(rate -> new ConstantSampler(false)); + LogProbe logProbe = createLog(null).build(); + logProbe.initSamplers(); + Assertions.assertFalse(logProbe.isReadyToCapture()); + verify(sink).skipSnapshot(PROBE_ID.getId(), RATE_LIMIT); + } finally { + ProbeRateLimiter.setSamplerSupplier(null); + } + } + + @Test + public void evaluateConditionTimeoutRecordsSkipAndConditionErrors() { + DebuggerSink sink = spy(new DebuggerSink(getConfig(), mock(ProbeStatusSink.class))); + DebuggerAgentHelper.injectSink(sink); + ProbeCondition timingOutCondition = mock(ProbeCondition.class); + when(timingOutCondition.execute(any(), any())) + .thenThrow(new EvaluationTimeOutException("timeout after 100ms", "slow.expr")); + LogProbe logProbe = + createLog(null).evaluateAt(MethodLocation.EXIT).when(timingOutCondition).build(); + CapturedContext context = new CapturedContext(); + LogStatus status = new LogStatus(logProbe); + + // methodLocation (ENTRY) intentionally differs from evaluateAt (EXIT) so that sample() is a + // no-op here, isolating the skipSnapshot call to evaluateCondition()'s timeout handling. + logProbe.evaluate(context, status, MethodLocation.ENTRY, false); + + Assertions.assertFalse(status.getCondition()); + assertTrue(status.hasConditionErrors()); + assertEquals(1, status.getErrors().size()); + assertEquals("slow.expr", status.getErrors().get(0).getExpr()); + assertEquals("timeout after 100ms", status.getErrors().get(0).getMessage()); + verify(sink).skipSnapshot(PROBE_ID.getId(), EVALUATION_TIME_OUT); + verify(sink, times(1)).skipSnapshot(anyString(), any()); + } + + @Test + public void evaluateConditionFalseDoesNotSkipSnapshot() { + DebuggerSink sink = spy(new DebuggerSink(getConfig(), mock(ProbeStatusSink.class))); + DebuggerAgentHelper.injectSink(sink); + LogProbe logProbe = + createLog(null) + .evaluateAt(MethodLocation.EXIT) + .when(new ProbeCondition(DSL.when(DSL.eq(DSL.value(1), DSL.value(2))), "1 == 2")) + .build(); + CapturedContext context = new CapturedContext(); + LogStatus status = new LogStatus(logProbe); + + logProbe.evaluate(context, status, MethodLocation.EXIT, false); + + Assertions.assertFalse(status.getCondition()); + Assertions.assertFalse(status.hasConditionErrors()); + verify(sink, never()).skipSnapshot(anyString(), any()); + } + private Builder createLog(String template) { return LogProbe.builder() .language(LANGUAGE) diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/sink/DebuggerSinkTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/sink/DebuggerSinkTest.java index a435392816a..56d251330bd 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/sink/DebuggerSinkTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/sink/DebuggerSinkTest.java @@ -1,5 +1,8 @@ package com.datadog.debugger.sink; +import static datadog.trace.api.debugger.DebuggerMetricCollector.DroppedReason.QUEUE_FULL; +import static datadog.trace.api.debugger.DebuggerMetricCollector.SkippedReason.EVALUATION_TIME_OUT; +import static datadog.trace.api.debugger.DebuggerMetricCollector.SkippedReason.RATE_LIMIT; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; @@ -20,13 +23,13 @@ import com.datadog.debugger.agent.ProbeStatus; import com.datadog.debugger.instrumentation.DiagnosticMessage; import com.datadog.debugger.uploader.BatchUploader; -import com.datadog.debugger.util.DebuggerMetrics; import com.datadog.debugger.util.MoshiHelper; import com.datadog.debugger.util.MoshiSnapshotTestHelper; import com.squareup.moshi.JsonAdapter; import com.squareup.moshi.Types; import datadog.trace.api.Config; import datadog.trace.api.ProcessTags; +import datadog.trace.api.debugger.DebuggerMetricCollector; import datadog.trace.bootstrap.debugger.CapturedContext; import datadog.trace.bootstrap.debugger.CapturedContext.CapturedValue; import datadog.trace.bootstrap.debugger.CapturedStackFrame; @@ -496,7 +499,7 @@ public void addDiagnostic() { @Test public void skipSnapshot() { - DebuggerMetrics debuggerMetrics = spy(DebuggerMetrics.getInstance(config)); + DebuggerMetricCollector metricCollector = spy(DebuggerMetricCollector.get()); SnapshotSink snapshotSink = new SnapshotSink( config, @@ -510,14 +513,61 @@ public void skipSnapshot() { "Logs", config, config.getFinalDebuggerSnapshotUrl(), SnapshotSink.RETRY_POLICY)); SymbolSink symbolSink = new SymbolSink(config); DebuggerSink sink = - new DebuggerSink(config, "", debuggerMetrics, probeStatusSink, snapshotSink, symbolSink); + new DebuggerSink(config, "", metricCollector, probeStatusSink, snapshotSink, symbolSink); Snapshot snapshot = createSnapshot(); - sink.skipSnapshot(snapshot.getProbe().getId(), DebuggerContext.SkipCause.CONDITION); - verify(debuggerMetrics) - .incrementCounter(anyString(), eq("cause:condition"), eq("probe_id:" + PROBE_ID.getId())); - sink.skipSnapshot(snapshot.getProbe().getId(), DebuggerContext.SkipCause.RATE); - verify(debuggerMetrics) - .incrementCounter(anyString(), eq("cause:rate"), eq("probe_id:" + PROBE_ID.getId())); + sink.skipSnapshot(snapshot.getProbe().getId(), RATE_LIMIT); + verify(metricCollector).recordEventSkipped(eq(RATE_LIMIT)); + } + + @Test + public void skipSnapshotEvaluationTimeOut() { + DebuggerMetricCollector metricCollector = spy(DebuggerMetricCollector.get()); + DebuggerSink sink = + new DebuggerSink( + config, + "", + metricCollector, + probeStatusSink, + new SnapshotSink(config, "", snapshotUploader, logUploader), + new SymbolSink(config)); + Snapshot snapshot = createSnapshot(); + sink.skipSnapshot(snapshot.getProbe().getId(), EVALUATION_TIME_OUT); + verify(metricCollector).recordEventSkipped(eq(EVALUATION_TIME_OUT)); + } + + @Test + public void addSnapshotQueueFullRecordsDropped() { + DebuggerMetricCollector metricCollector = spy(DebuggerMetricCollector.get()); + SnapshotSink snapshotSink = new SnapshotSink(config, "", snapshotUploader, logUploader); + DebuggerSink sink = + new DebuggerSink( + config, "", metricCollector, probeStatusSink, snapshotSink, new SymbolSink(config)); + Snapshot snapshot = createSnapshot(); + for (int i = 0; i < SnapshotSink.LOW_RATE_CAPACITY; i++) { + sink.addSnapshot(snapshot); + } + verify(metricCollector, times(0)).recordEventDropped(QUEUE_FULL); + sink.addSnapshot(snapshot); + verify(metricCollector, times(1)).recordEventDropped(QUEUE_FULL); + } + + @Test + public void addHighRateSnapshotRecordsEveryDrop() { + DebuggerMetricCollector metricCollector = spy(DebuggerMetricCollector.get()); + SnapshotSink snapshotSink = new SnapshotSink(config, "", snapshotUploader, logUploader); + DebuggerSink sink = + new DebuggerSink( + config, "", metricCollector, probeStatusSink, snapshotSink, new SymbolSink(config)); + Snapshot snapshot = createSnapshot(); + // fill the high rate queue to capacity + for (int i = 0; i < SnapshotSink.HIGH_RATE_CAPACITY; i++) { + sink.addHighRateSnapshot(snapshot); + } + verify(metricCollector, times(0)).recordEventDropped(QUEUE_FULL); + for (int i = 0; i < 3; i++) { + sink.addHighRateSnapshot(snapshot); + } + verify(metricCollector, times(3)).recordEventDropped(QUEUE_FULL); } private JsonSnapshotSerializer.IntakeRequest assertOneIntakeRequest(String strPayload) @@ -543,7 +593,7 @@ private DebuggerSink createDefaultDebuggerSink() { return new DebuggerSink( config, tags, - DebuggerMetrics.getInstance(config), + DebuggerMetricCollector.get(), probeStatusSink, new SnapshotSink(config, tags, snapshotUploader, logUploader), new SymbolSink(config)); @@ -555,7 +605,7 @@ private DebuggerSink createDebuggerSink(BatchUploader diagnosticUploader, boolea return new DebuggerSink( config, tags, - DebuggerMetrics.getInstance(config), + DebuggerMetricCollector.get(), probeSink, new SnapshotSink(config, tags, snapshotUploader, logUploader), new SymbolSink(config)); diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/util/TestSnapshotListener.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/util/TestSnapshotListener.java index 67a196361a2..44c626d2fb8 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/util/TestSnapshotListener.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/util/TestSnapshotListener.java @@ -4,13 +4,13 @@ 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.util.ArrayList; import java.util.List; public class TestSnapshotListener extends DebuggerSink { public boolean skipped; - public DebuggerContext.SkipCause cause; + public DebuggerMetricCollector.SkippedReason reason; public List snapshots = new ArrayList<>(); public TestSnapshotListener(Config config, ProbeStatusSink probeStatusSink) { @@ -18,9 +18,9 @@ public TestSnapshotListener(Config config, ProbeStatusSink probeStatusSink) { } @Override - public void skipSnapshot(String probeId, DebuggerContext.SkipCause cause) { + public void skipSnapshot(String probeId, DebuggerMetricCollector.SkippedReason reason) { skipped = true; - this.cause = cause; + this.reason = reason; } @Override diff --git a/internal-api/src/main/java/datadog/trace/api/debugger/DebuggerMetricCollector.java b/internal-api/src/main/java/datadog/trace/api/debugger/DebuggerMetricCollector.java new file mode 100644 index 00000000000..2f44eb5ea38 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/api/debugger/DebuggerMetricCollector.java @@ -0,0 +1,110 @@ +package datadog.trace.api.debugger; + +import datadog.trace.api.telemetry.MetricCollector; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.atomic.AtomicLongArray; + +public class DebuggerMetricCollector + implements MetricCollector { + private static final DebuggerMetricCollector INSTANCE = new DebuggerMetricCollector(); + + interface Reason { + String getTag(); + } + + public enum DroppedReason implements Reason { + QUEUE_FULL("reason:queueFull"), + PAYLOAD_TOO_LARGE("reason:payloadTooLarge"), + ; + + private final String tag; + + DroppedReason(String tag) { + this.tag = tag; + } + + @Override + public String getTag() { + return tag; + } + } + + public enum SkippedReason implements Reason { + RATE_LIMIT("reason:rateLimitProbe"), + EVALUATION_TIME_OUT("reason:evaluationTimeOut"), + ; + + private final String tag; + + SkippedReason(String tag) { + this.tag = tag; + } + + @Override + public String getTag() { + return tag; + } + } + + private final BlockingQueue metricsQueue = + new ArrayBlockingQueue<>(RAW_QUEUE_SIZE); + private final AtomicLongArray eventDroppedCounters = + new AtomicLongArray(DroppedReason.values().length); + private final AtomicLongArray eventSkippedCounters = + new AtomicLongArray(SkippedReason.values().length); + + public static DebuggerMetricCollector get() { + return INSTANCE; + } + + private DebuggerMetricCollector() {} + + public void recordEventDropped(DroppedReason reason) { + eventDroppedCounters.incrementAndGet(reason.ordinal()); + } + + public void recordEventSkipped(SkippedReason reason) { + eventSkippedCounters.incrementAndGet(reason.ordinal()); + } + + @Override + public void prepareMetrics() { + addCounterMetric(eventDroppedCounters, "events.dropped", DroppedReason.values()); + addCounterMetric(eventSkippedCounters, "events.skipped", SkippedReason.values()); + } + + private & Reason> void addCounterMetric( + AtomicLongArray counters, String name, E[] enumValues) { + for (E enumValue : enumValues) { + // get and reset + long value = counters.getAndSet(enumValue.ordinal(), 0); + if (value > 0) { + metricsQueue.offer(new DebuggerMetric(name, value, enumValue.getTag())); + } + } + } + + @Override + public Collection drain() { + if (metricsQueue.isEmpty()) { + return Collections.emptyList(); + } + List metrics = new ArrayList<>(); + metricsQueue.drainTo(metrics); + return metrics; + } + + public static class DebuggerMetric extends MetricCollector.Metric { + + private static final String NAMESPACE = "live_debugger"; + + public DebuggerMetric(String metricName, long value, String... tags) { + super(NAMESPACE, true, metricName, "count", value, tags); + } + } +} diff --git a/internal-api/src/test/java/datadog/trace/api/debugger/DebuggerMetricCollectorTest.java b/internal-api/src/test/java/datadog/trace/api/debugger/DebuggerMetricCollectorTest.java new file mode 100644 index 00000000000..671a7b97d63 --- /dev/null +++ b/internal-api/src/test/java/datadog/trace/api/debugger/DebuggerMetricCollectorTest.java @@ -0,0 +1,119 @@ +package datadog.trace.api.debugger; + +import static datadog.trace.api.debugger.DebuggerMetricCollector.DroppedReason.PAYLOAD_TOO_LARGE; +import static datadog.trace.api.debugger.DebuggerMetricCollector.DroppedReason.QUEUE_FULL; +import static datadog.trace.api.debugger.DebuggerMetricCollector.SkippedReason.EVALUATION_TIME_OUT; +import static datadog.trace.api.debugger.DebuggerMetricCollector.SkippedReason.RATE_LIMIT; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import datadog.trace.api.debugger.DebuggerMetricCollector.DebuggerMetric; +import java.util.Collection; +import java.util.Collections; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class DebuggerMetricCollectorTest { + + private final DebuggerMetricCollector collector = DebuggerMetricCollector.get(); + + @BeforeEach + public void clearQueue() { + collector.drain(); + } + + @Test + public void drainWithoutRecordsReturnsEmpty() { + assertEquals(0, collector.drain().size()); + } + + @Test + public void prepareMetricsWithoutRecordsProducesNoMetric() { + collector.prepareMetrics(); + assertEquals(0, collector.drain().size()); + } + + @Test + public void recordEventDroppedSurfacesOnPrepareAndDrain() { + collector.recordEventDropped(QUEUE_FULL); + collector.prepareMetrics(); + + Collection metrics = collector.drain(); + assertEquals(1, metrics.size()); + DebuggerMetric metric = metrics.iterator().next(); + assertEquals("live_debugger", metric.namespace); + assertEquals("events.dropped", metric.metricName); + assertEquals("count", metric.type); + assertTrue(metric.common); + assertEquals(1L, metric.value); + assertEquals(Collections.singletonList("reason:queueFull"), metric.tags); + } + + @Test + public void recordEventSkippedSurfacesOnPrepareAndDrain() { + collector.recordEventSkipped(EVALUATION_TIME_OUT); + collector.prepareMetrics(); + + Collection metrics = collector.drain(); + assertEquals(1, metrics.size()); + DebuggerMetric metric = metrics.iterator().next(); + assertEquals("events.skipped", metric.metricName); + assertEquals(1L, metric.value); + assertEquals(Collections.singletonList("reason:evaluationTimeOut"), metric.tags); + } + + @Test + public void countersAccumulateBeforePrepare() { + collector.recordEventDropped(QUEUE_FULL); + collector.recordEventDropped(QUEUE_FULL); + collector.recordEventDropped(QUEUE_FULL); + collector.prepareMetrics(); + + Collection metrics = collector.drain(); + assertEquals(1, metrics.size()); + assertEquals(3L, metrics.iterator().next().value); + } + + @Test + public void distinctReasonsProduceDistinctMetrics() { + collector.recordEventDropped(QUEUE_FULL); + collector.recordEventDropped(PAYLOAD_TOO_LARGE); + collector.prepareMetrics(); + + Collection metrics = collector.drain(); + assertEquals(2, metrics.size()); + assertTrue(metrics.stream().anyMatch(m -> m.tags.contains("reason:queueFull"))); + assertTrue(metrics.stream().anyMatch(m -> m.tags.contains("reason:payloadTooLarge"))); + } + + @Test + public void droppedAndSkippedReasonsAreIndependent() { + collector.recordEventDropped(QUEUE_FULL); + collector.recordEventSkipped(RATE_LIMIT); + collector.prepareMetrics(); + + Collection metrics = collector.drain(); + assertEquals(2, metrics.size()); + assertTrue(metrics.stream().anyMatch(m -> m.metricName.equals("events.dropped"))); + assertTrue(metrics.stream().anyMatch(m -> m.metricName.equals("events.skipped"))); + } + + @Test + public void prepareMetricsResetsCountersAfterDrain() { + collector.recordEventDropped(QUEUE_FULL); + collector.prepareMetrics(); + assertEquals(1, collector.drain().size()); + + // no new events recorded, so a second prepare/drain cycle should be empty + collector.prepareMetrics(); + assertEquals(0, collector.drain().size()); + } + + @Test + public void drainClearsQueue() { + collector.recordEventDropped(QUEUE_FULL); + collector.prepareMetrics(); + assertEquals(1, collector.drain().size()); + assertEquals(0, collector.drain().size()); + } +} diff --git a/telemetry/build.gradle.kts b/telemetry/build.gradle.kts index 1e11e30fac7..e6bdf1ebc03 100644 --- a/telemetry/build.gradle.kts +++ b/telemetry/build.gradle.kts @@ -20,6 +20,7 @@ val excludedClassesCoverage by extra( "datadog.telemetry.metric.CiVisibilityMetricPeriodicAction", "datadog.telemetry.metric.OtelSpiMetricPeriodicAction", "datadog.telemetry.metric.OtlpTelemetryPeriodicAction", + "datadog.telemetry.metric.DebuggerMetricPeriodicAction", ) ) extra["excludedClassesBranchCoverage"] = listOf( diff --git a/telemetry/src/main/java/datadog/telemetry/TelemetrySystem.java b/telemetry/src/main/java/datadog/telemetry/TelemetrySystem.java index 31b833aa42a..667fe414cb6 100644 --- a/telemetry/src/main/java/datadog/telemetry/TelemetrySystem.java +++ b/telemetry/src/main/java/datadog/telemetry/TelemetrySystem.java @@ -12,6 +12,7 @@ import datadog.telemetry.metric.CiVisibilityMetricPeriodicAction; import datadog.telemetry.metric.ConfigInversionMetricPeriodicAction; import datadog.telemetry.metric.CoreMetricsPeriodicAction; +import datadog.telemetry.metric.DebuggerMetricPeriodicAction; import datadog.telemetry.metric.IastMetricPeriodicAction; import datadog.telemetry.metric.LLMObsMetricPeriodicAction; import datadog.telemetry.metric.OtelEnvMetricPeriodicAction; @@ -67,6 +68,7 @@ static Thread createTelemetryRunnable( actions.add(new IntegrationPeriodicAction()); actions.add(new WafMetricPeriodicAction()); actions.add(new OtlpTelemetryPeriodicAction()); + actions.add(new DebuggerMetricPeriodicAction()); if (Verbosity.OFF != Config.get().getIastTelemetryVerbosity()) { actions.add(new IastMetricPeriodicAction()); } diff --git a/telemetry/src/main/java/datadog/telemetry/metric/DebuggerMetricPeriodicAction.java b/telemetry/src/main/java/datadog/telemetry/metric/DebuggerMetricPeriodicAction.java new file mode 100644 index 00000000000..00ddf9a8554 --- /dev/null +++ b/telemetry/src/main/java/datadog/telemetry/metric/DebuggerMetricPeriodicAction.java @@ -0,0 +1,12 @@ +package datadog.telemetry.metric; + +import datadog.trace.api.debugger.DebuggerMetricCollector; +import datadog.trace.api.telemetry.MetricCollector; + +public class DebuggerMetricPeriodicAction extends MetricPeriodicAction { + + @Override + public MetricCollector collector() { + return DebuggerMetricCollector.get(); + } +}