From 352efb3863d554519004d9e8048130ddc65bcf88 Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Fri, 24 Jan 2025 13:32:51 -0500 Subject: [PATCH 1/4] add a tag tracking the overall invocation account --- .../bootstrap/debugger/DebuggerContext.java | 6 ++++++ .../com/datadog/debugger/probe/LogProbe.java | 21 +++++++++++++------ .../datadog/debugger/probe/LogProbeTest.java | 5 ++++- 3 files changed, 25 insertions(+), 7 deletions(-) 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 9d1c793ea8e..ba15385eebd 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 @@ -39,6 +39,12 @@ public String tag() { public String tag() { return "cause:debug session disabled"; } + }, + BUDGET { + @Override + public String tag() { + return "cause:budget exceeded"; + } }; public abstract String tag(); 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 f07f17cc558..e2f08fc47c9 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,6 +1,7 @@ package com.datadog.debugger.probe; import static com.datadog.debugger.probe.LogProbe.Capture.toLimits; +import static java.lang.String.format; import com.datadog.debugger.agent.DebuggerAgent; import com.datadog.debugger.agent.Generated; @@ -575,14 +576,17 @@ public void commit( CapturedContext exitContext, List caughtExceptions) { Snapshot snapshot = createSnapshot(); - boolean shouldCommit = - inBudget() && fillSnapshot(entryContext, exitContext, caughtExceptions, snapshot); + boolean shouldCommit = fillSnapshot(entryContext, exitContext, caughtExceptions, snapshot); DebuggerSink sink = DebuggerAgent.getSink(); if (shouldCommit) { - commitSnapshot(snapshot, sink); incrementBudget(); - if (snapshotProcessor != null) { - snapshotProcessor.accept(snapshot); + if (inBudget()) { + commitSnapshot(snapshot, sink); + if (snapshotProcessor != null) { + snapshotProcessor.accept(snapshot); + } + } else { + sink.skipSnapshot(id, DebuggerContext.SkipCause.BUDGET); } } else { sink.skipSnapshot(id, DebuggerContext.SkipCause.CONDITION); @@ -866,7 +870,7 @@ public String toString() { private boolean inBudget() { AtomicInteger budgetLevel = getBudgetLevel(); - return budgetLevel == null || budgetLevel.get() < PROBE_BUDGET; + return budgetLevel == null || budgetLevel.get() <= PROBE_BUDGET; } private AtomicInteger getBudgetLevel() { @@ -881,6 +885,11 @@ private void incrementBudget() { AtomicInteger budgetLevel = getBudgetLevel(); if (budgetLevel != null) { budgetLevel.incrementAndGet(); + TracerAPI tracer = AgentTracer.get(); + AgentSpan span = tracer != null ? tracer.activeSpan() : null; + if (span != null) { + span.getLocalRootSpan().setTag(format("_dd.ld.probe_id.%s", id), budgetLevel.get()); + } } } 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 19019ffa3e6..92b4be9e4cd 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 @@ -110,9 +110,12 @@ private void runTrace(TracerAPI tracer) { logProbe.evaluate(entryContext, new LogStatus(logProbe), MethodLocation.ENTRY); logProbe.evaluate(exitContext, new LogStatus(logProbe), MethodLocation.EXIT); - for (int i = 0; i < 20; i++) { + for (int i = 0; i < LogProbe.PROBE_BUDGET * 2; i++) { logProbe.commit(entryContext, exitContext, emptyList()); } + assertEquals( + LogProbe.PROBE_BUDGET * 2, + span.getLocalRootSpan().getTag(format("_dd.ld.probe_id.%s", logProbe.id))); } } From 195a0fbda0f04063a5741b6198d28db7ef1f0c7d Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Fri, 24 Jan 2025 17:51:16 -0500 Subject: [PATCH 2/4] two budgets for capture vs non-capture snapshots --- .../com/datadog/debugger/probe/LogProbe.java | 7 +- .../datadog/debugger/sink/SnapshotSink.java | 4 -- .../datadog/debugger/probe/LogProbeTest.java | 65 +++++++++++++++---- 3 files changed, 59 insertions(+), 17 deletions(-) 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 e2f08fc47c9..95ba3426135 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 @@ -63,7 +63,8 @@ public class LogProbe extends ProbeDefinition implements Sampled { private static final Limits LIMITS = new Limits(1, 3, 8192, 5); private static final int LOG_MSG_LIMIT = 8192; - public static final int PROBE_BUDGET = 10; + public static final int CAPTURING_PROBE_BUDGET = 10; + public static final int NON_CAPTURING_PROBE_BUDGET = 1000; /** Stores part of a templated message either a str or an expression */ public static class Segment { @@ -870,7 +871,9 @@ public String toString() { private boolean inBudget() { AtomicInteger budgetLevel = getBudgetLevel(); - return budgetLevel == null || budgetLevel.get() <= PROBE_BUDGET; + return budgetLevel == null + || budgetLevel.get() + <= (captureSnapshot ? CAPTURING_PROBE_BUDGET : NON_CAPTURING_PROBE_BUDGET); } private AtomicInteger getBudgetLevel() { 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 168ef43a19e..8b47882c8fa 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 @@ -53,10 +53,6 @@ public SnapshotSink(Config config, String tags, BatchUploader snapshotUploader) this.snapshotUploader = snapshotUploader; } - public BlockingQueue getLowRateSnapshots() { - return lowRateSnapshots; - } - public void start() { if (started.compareAndSet(false, true)) { highRateScheduled = 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 92b4be9e4cd..5ea385c1c3e 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 @@ -16,6 +16,7 @@ import com.datadog.debugger.sink.DebuggerSink; import com.datadog.debugger.sink.ProbeStatusSink; import com.datadog.debugger.sink.Snapshot; +import datadog.trace.api.Config; import datadog.trace.api.IdGenerationStrategy; import datadog.trace.bootstrap.debugger.CapturedContext; import datadog.trace.bootstrap.debugger.EvaluationError; @@ -28,6 +29,8 @@ import datadog.trace.bootstrap.instrumentation.api.ScopeSource; import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.core.CoreTracer; +import java.util.ArrayList; +import java.util.List; import java.util.stream.Stream; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -80,20 +83,27 @@ public void noDebugSession() { @Test public void budgets() { - DebuggerSink sink = new DebuggerSink(getConfig(), mock(ProbeStatusSink.class)); + BudgetSink sink = new BudgetSink(getConfig(), mock(ProbeStatusSink.class)); + DebuggerAgentHelper.injectSink(sink); - assertEquals(0, sink.getSnapshotSink().getLowRateSnapshots().size()); + assertEquals(0, sink.captures.size()); TracerAPI tracer = CoreTracer.builder().idGenerationStrategy(IdGenerationStrategy.fromName("random")).build(); AgentTracer.registerIfAbsent(tracer); int runs = 100; for (int i = 0; i < runs; i++) { - runTrace(tracer); + runTrace(tracer, true); } - assertEquals(runs * LogProbe.PROBE_BUDGET, sink.getSnapshotSink().getLowRateSnapshots().size()); + assertEquals(runs * LogProbe.CAPTURING_PROBE_BUDGET, sink.captures.size()); + + runs = 2000; + for (int i = 0; i < runs; i++) { + runTrace(tracer, false); + } + assertEquals(runs * LogProbe.NON_CAPTURING_PROBE_BUDGET, sink.highRate.size()); } - private void runTrace(TracerAPI tracer) { + private void runTrace(TracerAPI tracer, boolean captureSnapshot) { AgentSpan span = tracer.startSpan("budget testing", "test span"); span.setTag(Tags.PROPAGATED_DEBUG, "12345:1"); try (AgentScope scope = tracer.activateSpan(span, ScopeSource.MANUAL)) { @@ -102,20 +112,23 @@ private void runTrace(TracerAPI tracer) { createLog("I'm in a debug session") .probeId(ProbeId.newId()) .tags("session_id:12345") - .captureSnapshot(true) + .captureSnapshot(captureSnapshot) .build(); - 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); - for (int i = 0; i < LogProbe.PROBE_BUDGET * 2; i++) { + int budget = + logProbe.isCaptureSnapshot() + ? LogProbe.CAPTURING_PROBE_BUDGET + : LogProbe.NON_CAPTURING_PROBE_BUDGET; + int runs = budget + 20; + + for (int i = 0; i < runs; i++) { logProbe.commit(entryContext, exitContext, emptyList()); } - assertEquals( - LogProbe.PROBE_BUDGET * 2, - span.getLocalRootSpan().getTag(format("_dd.ld.probe_id.%s", logProbe.id))); + assertEquals(runs, span.getLocalRootSpan().getTag(format("_dd.ld.probe_id.%s", logProbe.id))); } } @@ -296,4 +309,34 @@ private Builder createLog(String template) { .probeId(PROBE_ID) .template(template, parseTemplate(template)); } + + private static class BudgetSink extends DebuggerSink { + + public List captures = new ArrayList<>(); + public List highRate = new ArrayList<>(); + + public BudgetSink(Config config, ProbeStatusSink probeStatusSink) { + super(config, probeStatusSink); + } + + @Override + public void addHighRateSnapshot(Snapshot snapshot) { + highRate.add(snapshot); + } + + @Override + public void addSnapshot(Snapshot snapshot) { + captures.add(snapshot); + } + + @Override + public void start() { + super.start(); + } + + @Override + public void stop() { + super.stop(); + } + } } From 75c7dae7362a8618872c1a51bf822b62723f6aeb Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Fri, 24 Jan 2025 18:15:26 -0500 Subject: [PATCH 3/4] use _ instead of space in case metrics tags don't allow spaces --- .../java/datadog/trace/bootstrap/debugger/DebuggerContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 ba15385eebd..5acd251b78a 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 @@ -43,7 +43,7 @@ public String tag() { BUDGET { @Override public String tag() { - return "cause:budget exceeded"; + return "cause:budget_exceeded"; } }; From 0df836a771710ca13e387ed914c65dfef1c91a0e Mon Sep 17 00:00:00 2001 From: Justin Lee Date: Fri, 24 Jan 2025 18:51:32 -0500 Subject: [PATCH 4/4] we just need to store the counts --- .../datadog/debugger/probe/LogProbeTest.java | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) 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 5ea385c1c3e..5c048a47fd8 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 @@ -29,8 +29,6 @@ import datadog.trace.bootstrap.instrumentation.api.ScopeSource; import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.core.CoreTracer; -import java.util.ArrayList; -import java.util.List; import java.util.stream.Stream; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -84,9 +82,8 @@ public void noDebugSession() { @Test public void budgets() { BudgetSink sink = new BudgetSink(getConfig(), mock(ProbeStatusSink.class)); - DebuggerAgentHelper.injectSink(sink); - assertEquals(0, sink.captures.size()); + TracerAPI tracer = CoreTracer.builder().idGenerationStrategy(IdGenerationStrategy.fromName("random")).build(); AgentTracer.registerIfAbsent(tracer); @@ -94,13 +91,15 @@ public void budgets() { for (int i = 0; i < runs; i++) { runTrace(tracer, true); } - assertEquals(runs * LogProbe.CAPTURING_PROBE_BUDGET, sink.captures.size()); + assertEquals(runs * LogProbe.CAPTURING_PROBE_BUDGET, sink.captures); - runs = 2000; + sink = new BudgetSink(getConfig(), mock(ProbeStatusSink.class)); + DebuggerAgentHelper.injectSink(sink); + runs = 1010; for (int i = 0; i < runs; i++) { runTrace(tracer, false); } - assertEquals(runs * LogProbe.NON_CAPTURING_PROBE_BUDGET, sink.highRate.size()); + assertEquals(runs * LogProbe.NON_CAPTURING_PROBE_BUDGET, sink.highRate); } private void runTrace(TracerAPI tracer, boolean captureSnapshot) { @@ -312,8 +311,8 @@ private Builder createLog(String template) { private static class BudgetSink extends DebuggerSink { - public List captures = new ArrayList<>(); - public List highRate = new ArrayList<>(); + public int captures; + public int highRate; public BudgetSink(Config config, ProbeStatusSink probeStatusSink) { super(config, probeStatusSink); @@ -321,12 +320,12 @@ public BudgetSink(Config config, ProbeStatusSink probeStatusSink) { @Override public void addHighRateSnapshot(Snapshot snapshot) { - highRate.add(snapshot); + highRate++; } @Override public void addSnapshot(Snapshot snapshot) { - captures.add(snapshot); + captures++; } @Override