Skip to content

Commit f4b1632

Browse files
jpbempeldevflow.devflow-routing-intake
andauthored
Add coordinated sampling for snapshot probes (#12452)
Add coordinated sampling for snapshot probes Adds coordinated sampling for full debugger snapshots using the active Datadog context, using Context API. State of Coordinated Sampling is stored into a Context attached to the root local span. The first probe’s sampling decision controls related probes, avoiding fragmented snapshot sets. Ensures each probe emits at most once per coordinated context. Log-only probes remain independent. Encapsulates ProbeDefinition.probeId, updating callers to use getProbeId(). address codex comments fix exception probe sampling Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent e304fe0 commit f4b1632

14 files changed

Lines changed: 712 additions & 52 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,6 @@ public int hashCode() {
126126
public String toString() {
127127
return String.format(
128128
"CodeOriginProbe{probeId=%s, entrySpanProbe=%s, signature=%s, where=%s, location=%s}",
129-
probeId, entrySpanProbe, signature, where, location);
129+
getProbeId(), entrySpanProbe, signature, where, location);
130130
}
131131
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,13 @@ public boolean isLineProbe() {
6868
return false;
6969
}
7070

71+
@Override
72+
protected boolean useCoordinatedSampling() {
73+
// exception probes have their own independent exception sampling flow and must not share a
74+
// trace-wide sampling decision with ordinary snapshot probes on the same local root span.
75+
return false;
76+
}
77+
7178
@Override
7279
public CapturedContext.Status createStatus() {
7380
return new ExceptionProbeStatus(this);

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

Lines changed: 133 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package com.datadog.debugger.probe;
22

33
import static com.datadog.debugger.probe.LogProbe.Capture.toLimits;
4+
import static com.datadog.debugger.probe.LogProbe.CoordinatedSamplingState.Status.DROP;
5+
import static com.datadog.debugger.probe.LogProbe.CoordinatedSamplingState.Status.EMIT;
46
import static datadog.trace.api.debugger.DebuggerMetricCollector.SkippedReason.RATE_LIMIT;
57
import static java.lang.String.format;
68

@@ -24,6 +26,8 @@
2426
import com.squareup.moshi.JsonAdapter;
2527
import com.squareup.moshi.JsonReader;
2628
import com.squareup.moshi.JsonWriter;
29+
import datadog.context.Context;
30+
import datadog.context.ContextKey;
2731
import datadog.trace.api.Config;
2832
import datadog.trace.api.CorrelationIdentifier;
2933
import datadog.trace.api.DDTraceId;
@@ -52,6 +56,8 @@
5256
import java.util.List;
5357
import java.util.Map;
5458
import java.util.Objects;
59+
import java.util.Set;
60+
import java.util.concurrent.ConcurrentHashMap;
5561
import java.util.concurrent.atomic.AtomicInteger;
5662
import java.util.function.Consumer;
5763
import org.slf4j.Logger;
@@ -62,7 +68,8 @@ public class LogProbe extends ProbeDefinition implements Sampled, CapturedContex
6268
private static final Logger LOGGER = LoggerFactory.getLogger(LogProbe.class);
6369
private static final Limits LIMITS = new Limits(1, 3, 8192, 5);
6470
private static final int LOG_MSG_LIMIT = 8192;
65-
71+
private static final ContextKey<CoordinatedSamplingState> SAMPLING_KEY =
72+
ContextKey.named("debugger_sampling");
6673
public static final int CAPTURING_PROBE_BUDGET = 10;
6774
public static final int NON_CAPTURING_PROBE_BUDGET = 1000;
6875

@@ -511,7 +518,7 @@ public InstrumentationResult.Status instrument(
511518
public boolean isReadyToCapture() {
512519
if (!hasCondition()) {
513520
// we are sampling here to avoid creating CapturedContext when the sampling result is negative
514-
boolean sampled = ProbeRateLimiter.tryProbe(sampler, isFullSnapshot());
521+
boolean sampled = trySample(sampler);
515522
if (!sampled) {
516523
DebuggerAgent.getSink().skipSnapshot(id, RATE_LIMIT);
517524
}
@@ -581,15 +588,29 @@ private void sample(LogStatus logStatus, MethodLocation methodLocation) {
581588
if (!MethodLocation.isSame(methodLocation, evaluateAt)) {
582589
return;
583590
}
584-
// if condition has error and no capture Snapshot, the error is reported using errorSampler
585-
// at 1/s rate instead of the log template one
586-
Sampler localSampler =
587-
logStatus.hasConditionErrors && !isFullSnapshot() ? errorSampler : sampler;
588-
boolean sampled =
589-
!logStatus.getDebugSessionStatus().isDisabled()
590-
&& ProbeRateLimiter.tryProbe(localSampler, isFullSnapshot());
591+
DebugSessionStatus debugSessionStatus = logStatus.getDebugSessionStatus();
592+
if (debugSessionStatus.isDisabled()) {
593+
return;
594+
}
595+
boolean sampled;
596+
if (debugSessionStatus.isActive()) {
597+
// LogStatus.shouldSend() emits unconditionally when the debug session is ACTIVE, so this
598+
// probe must not record a coordinated decision that shouldSend() would ignore anyway.
599+
// Instead, make sure the shared trace-level state lets the other full-snapshot probes on
600+
// this trace emit too, rather than leaving/recording a DROP that would suppress them.
601+
sampled = true;
602+
if (isFullSnapshot() && useCoordinatedSampling()) {
603+
forceCoordinatedEmit();
604+
}
605+
} else {
606+
// if condition has error and no capture Snapshot, the error is reported using errorSampler
607+
// at 1/s rate instead of the log template one
608+
Sampler localSampler =
609+
logStatus.hasConditionErrors && !isFullSnapshot() ? errorSampler : sampler;
610+
sampled = trySample(localSampler);
611+
}
591612
logStatus.setSampled(sampled);
592-
if (!sampled && !logStatus.getDebugSessionStatus().isDisabled()) {
613+
if (!sampled) {
593614
DebuggerAgent.getSink().skipSnapshot(id, RATE_LIMIT);
594615
}
595616
}
@@ -642,6 +663,104 @@ public void commit(
642663
}
643664
}
644665

666+
/**
667+
* Holds the once-per-trace coordinated sampling decision shared by all full-snapshot probes on a
668+
* given local root span, so that either all of them emit or none of them do.
669+
*/
670+
static class CoordinatedSamplingState {
671+
/** Outcome of the first probe's sampling decision for the trace, cached on the root span. */
672+
enum Status {
673+
/** The trace was not sampled; every probe sharing this state must not emit. */
674+
DROP,
675+
/** The trace was sampled; probes sharing this state may emit, once each. */
676+
EMIT
677+
}
678+
679+
private final Set<String> emittedProbeIds;
680+
private final Status status;
681+
682+
CoordinatedSamplingState(Status status) {
683+
this.status = status;
684+
if (status == EMIT) {
685+
emittedProbeIds = ConcurrentHashMap.newKeySet();
686+
} else {
687+
emittedProbeIds = Collections.emptySet();
688+
}
689+
}
690+
691+
boolean tryEmit(String probeEncodedId) {
692+
return status == EMIT && emittedProbeIds.add(probeEncodedId);
693+
}
694+
}
695+
696+
/**
697+
* Whether this probe shares the trace-wide {@link CoordinatedSamplingState} with other
698+
* full-snapshot probes on the same local root span, so they all emit together or not at all.
699+
* Probes with their own independent sampling flow (e.g. {@link ExceptionProbe}) must not
700+
* participate, or their unrelated decision would leak into (and be leaked into by) ordinary
701+
* snapshot probes sharing the same trace.
702+
*/
703+
protected boolean useCoordinatedSampling() {
704+
return true;
705+
}
706+
707+
private boolean trySample(Sampler sampler) {
708+
if (!isFullSnapshot()) {
709+
return ProbeRateLimiter.tryProbe(sampler, false);
710+
}
711+
if (!useCoordinatedSampling()) {
712+
return ProbeRateLimiter.tryProbe(sampler, true);
713+
}
714+
715+
AgentSpan localRootSpan = getActiveLocalRootSpan();
716+
if (localRootSpan == null) {
717+
return ProbeRateLimiter.tryProbe(sampler, true);
718+
}
719+
720+
CoordinatedSamplingState state = Context.from(localRootSpan).get(SAMPLING_KEY);
721+
if (state == null) {
722+
synchronized (localRootSpan) {
723+
Context traceContext = Context.from(localRootSpan);
724+
state = traceContext.get(SAMPLING_KEY);
725+
if (state == null) {
726+
boolean sampled = ProbeRateLimiter.tryProbe(sampler, true);
727+
state = new CoordinatedSamplingState(sampled ? EMIT : DROP);
728+
traceContext.with(SAMPLING_KEY, state).attachTo(localRootSpan);
729+
}
730+
}
731+
}
732+
return state.tryEmit(getProbeId().getEncodedId());
733+
}
734+
735+
/**
736+
* Ensures the trace-level coordinated sampling state allows emission, upgrading a previously
737+
* cached DROP (recorded by an ordinary probe before this debug session became active) so that
738+
* every full-snapshot probe on this trace gets a chance to emit.
739+
*/
740+
private void forceCoordinatedEmit() {
741+
AgentSpan localRootSpan = getActiveLocalRootSpan();
742+
if (localRootSpan == null) {
743+
return;
744+
}
745+
synchronized (localRootSpan) {
746+
Context traceContext = Context.from(localRootSpan);
747+
CoordinatedSamplingState state = traceContext.get(SAMPLING_KEY);
748+
if (state == null || state.status != EMIT) {
749+
traceContext.with(SAMPLING_KEY, new CoordinatedSamplingState(EMIT)).attachTo(localRootSpan);
750+
}
751+
}
752+
}
753+
754+
private AgentSpan getActiveLocalRootSpan() {
755+
TracerAPI tracer = AgentTracer.get();
756+
AgentSpan activeSpan = tracer != null ? tracer.activeSpan() : null;
757+
if (activeSpan == null || activeSpan == AgentTracer.noopSpan()) {
758+
return null;
759+
}
760+
AgentSpan localRootSpan = activeSpan.getLocalRootSpan();
761+
return localRootSpan != AgentTracer.noopSpan() ? localRootSpan : null;
762+
}
763+
645764
protected Snapshot createSnapshot() {
646765
int maxDepth = capture != null ? capture.maxReferenceDepth : -1;
647766
return new Snapshot(Thread.currentThread(), this, maxDepth);
@@ -652,8 +771,9 @@ protected boolean fillSnapshot(
652771
CapturedContext exitContext,
653772
List<CapturedContext.CapturedThrowable> caughtExceptions,
654773
Snapshot snapshot) {
655-
LogStatus entryStatus = convertStatus(entryContext.getStatus(probeId.getEncodedId()));
656-
LogStatus exitStatus = convertStatus(exitContext.getStatus(probeId.getEncodedId()));
774+
String probeEncodedId = getProbeId().getEncodedId();
775+
LogStatus entryStatus = convertStatus(entryContext.getStatus(probeEncodedId));
776+
LogStatus exitStatus = convertStatus(exitContext.getStatus(probeEncodedId));
657777
String message = null;
658778
switch (evaluateAt) {
659779
case ENTRY:
@@ -829,7 +949,7 @@ protected void commitSnapshot(Snapshot snapshot, DebuggerSink sink) {
829949

830950
@Override
831951
public void commit(CapturedContext lineContext, int line) {
832-
LogStatus status = (LogStatus) lineContext.getStatus(probeId.getEncodedId());
952+
LogStatus status = (LogStatus) lineContext.getStatus(getProbeId().getEncodedId());
833953
if (status == null) {
834954
return;
835955
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public abstract class ProbeDefinition implements ProbeImplementation {
2828
protected final String language;
2929
protected final String id;
3030
protected final int version;
31-
protected transient ProbeId probeId;
31+
private transient ProbeId probeId;
3232
protected final Tag[] tags;
3333
protected final Map<String, String> tagMap = new HashMap<>();
3434
protected final Where where;

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -273,10 +273,11 @@ public void commit(
273273
CapturedContext entryContext,
274274
CapturedContext exitContext,
275275
List<CapturedContext.CapturedThrowable> caughtExceptions) {
276+
String probeEncodedId = getProbeId().getEncodedId();
276277
CapturedContext.Status status =
277278
evaluateAt == MethodLocation.EXIT
278-
? exitContext.getStatus(probeId.getEncodedId())
279-
: entryContext.getStatus(probeId.getEncodedId());
279+
? exitContext.getStatus(probeEncodedId)
280+
: entryContext.getStatus(probeEncodedId);
280281
if (status == null) {
281282
return;
282283
}
@@ -287,7 +288,7 @@ public void commit(
287288

288289
@Override
289290
public void commit(CapturedContext lineContext, int line) {
290-
CapturedContext.Status status = lineContext.getStatus(probeId.getEncodedId());
291+
CapturedContext.Status status = lineContext.getStatus(getProbeId().getEncodedId());
291292
if (status == null) {
292293
return;
293294
}
@@ -319,7 +320,7 @@ private void decorateTags(SpanDecorationStatus status) {
319320
}
320321
if (!tagsToDecorate.isEmpty()) {
321322
// only send EMITTING status if we set at least one tag
322-
DebuggerAgent.getSink().getProbeStatusSink().addEmitting(probeId);
323+
DebuggerAgent.getSink().getProbeStatusSink().addEmitting(getProbeId());
323324
}
324325
}
325326

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ private boolean evaluateCondition(CapturedContext capture) {
140140
Duration timeout = Duration.ofMillis(Config.get().getDynamicInstrumentationEvalTimeout());
141141
return probeCondition.execute(capture, TimeoutChecker.create(Config.get(), timeout));
142142
} catch (Exception ex) {
143-
DebuggerAgent.getSink().getProbeStatusSink().addError(probeId, ex);
143+
DebuggerAgent.getSink().getProbeStatusSink().addError(getProbeId(), ex);
144144
return false;
145145
} finally {
146146
LOGGER.debug(
@@ -199,7 +199,7 @@ public String toString() {
199199
language,
200200
location,
201201
probeCondition,
202-
probeId,
202+
getProbeId(),
203203
sampling,
204204
tagMap,
205205
Arrays.toString(tags),

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

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import static org.junit.jupiter.api.Assertions.assertNotNull;
1919
import static org.junit.jupiter.api.Assertions.assertNull;
2020
import static org.junit.jupiter.api.Assertions.assertTrue;
21+
import static org.junit.jupiter.params.provider.Arguments.arguments;
2122
import static org.mockito.ArgumentMatchers.eq;
2223
import static org.mockito.Mockito.mock;
2324
import static org.mockito.Mockito.times;
@@ -444,15 +445,6 @@ public void multiProbeSameMethod() throws IOException, URISyntaxException {
444445
assertCaptureReturnValue(snapshot1.getCaptures().getReturn(), "int", "31");
445446
}
446447

447-
private List<Snapshot> assertSnapshots(
448-
TestSnapshotListener listener, int expectedCount, ProbeId... probeIds) {
449-
assertEquals(expectedCount, listener.snapshots.size());
450-
for (int i = 0; i < probeIds.length; i++) {
451-
assertEquals(probeIds[i].getId(), listener.snapshots.get(i).getProbe().getId());
452-
}
453-
return listener.snapshots;
454-
}
455-
456448
@Test
457449
public void catchBlock() throws IOException, URISyntaxException {
458450
final String CLASS_NAME = "CapturedSnapshot02";
@@ -908,9 +900,14 @@ public void fieldExtractorDepth1() throws IOException, URISyntaxException {
908900
@Test
909901
public void fieldExtractorDuplicateUnionDepth() throws IOException, URISyntaxException {
910902
final String CLASS_NAME = "CapturedSnapshot04";
911-
LogProbe.Builder builder = createProbeBuilder(PROBE_ID, CLASS_NAME, "createSimpleData", "()");
912-
LogProbe probe1 = builder.capture(0, 100, 50, Limits.DEFAULT_FIELD_COUNT).build();
913-
LogProbe probe2 = builder.capture(3, 100, 50, Limits.DEFAULT_FIELD_COUNT).build();
903+
LogProbe probe1 =
904+
createProbeBuilder(PROBE_ID1, CLASS_NAME, "createSimpleData", "()")
905+
.capture(0, 100, 50, Limits.DEFAULT_FIELD_COUNT)
906+
.build();
907+
LogProbe probe2 =
908+
createProbeBuilder(PROBE_ID2, CLASS_NAME, "createSimpleData", "()")
909+
.capture(3, 100, 50, Limits.DEFAULT_FIELD_COUNT)
910+
.build();
914911
TestSnapshotListener listener = installProbes(probe1, probe2);
915912
Class<?> testClass = compileAndLoadClass(CLASS_NAME);
916913
int result = Reflect.onClass(testClass).call("main", "").get();
@@ -2330,7 +2327,8 @@ public void enumConstructorArgs() throws IOException, URISyntaxException {
23302327
final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot23";
23312328
final String ENUM_CLASS = CLASS_NAME + "$MyEnum";
23322329
TestSnapshotListener listener =
2333-
installProbes(createMethodProbe(PROBE_ID, ENUM_CLASS, "<init>", null));
2330+
installProbes(
2331+
createProbeBuilder(PROBE_ID, ENUM_CLASS, "<init>", null).sampling(10).build());
23342332
Class<?> testClass = compileAndLoadClass(CLASS_NAME);
23352333
int result = Reflect.onClass(testClass).call("main", "").get();
23362334
assertEquals(2, result);

‎dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturingTestBase.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import com.datadog.debugger.probe.Sampled;
1515
import com.datadog.debugger.sink.DebuggerSink;
1616
import com.datadog.debugger.sink.ProbeStatusSink;
17+
import com.datadog.debugger.sink.Snapshot;
1718
import com.datadog.debugger.util.MoshiHelper;
1819
import com.datadog.debugger.util.MoshiSnapshotTestHelper;
1920
import com.datadog.debugger.util.SerializerWithLimits;
@@ -160,6 +161,15 @@ private static Collection<?> getCollection(CapturedContext.CapturedValue capture
160161
}
161162
}
162163

164+
protected List<Snapshot> assertSnapshots(
165+
TestSnapshotListener listener, int expectedCount, ProbeId... probeIds) {
166+
assertEquals(expectedCount, listener.snapshots.size());
167+
for (int i = 0; i < probeIds.length; i++) {
168+
assertEquals(probeIds[i].getId(), listener.snapshots.get(i).getProbe().getId());
169+
}
170+
return listener.snapshots;
171+
}
172+
163173
protected void assertCaptureFields(
164174
CapturedContext context, String name, String typeName, Map<Object, Object> expectedMap) {
165175
CapturedContext.CapturedValue field = getFields(context.getArguments().get("this")).get(name);

0 commit comments

Comments
 (0)