Skip to content

Commit 6bf7492

Browse files
evanchoolyevanchooly
authored
Implement safety guards for TriggerProbes (#7843)
* Implement sampling, conditions, and cooldown for TriggerProbes * reinstate LogProbe.Sampling update Sampling to new schema from idan * more tests for jacoco API clean up
1 parent 78e60c1 commit 6bf7492

File tree

14 files changed

+524
-249
lines changed

14 files changed

+524
-249
lines changed

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

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
import com.datadog.debugger.instrumentation.InstrumentationResult;
1010
import com.datadog.debugger.probe.LogProbe;
1111
import com.datadog.debugger.probe.ProbeDefinition;
12+
import com.datadog.debugger.probe.Sampled;
13+
import com.datadog.debugger.probe.Sampling;
1214
import com.datadog.debugger.sink.DebuggerSink;
1315
import com.datadog.debugger.util.ExceptionHelper;
1416
import datadog.trace.api.Config;
@@ -239,16 +241,15 @@ public ProbeImplementation resolve(String encodedProbeId) {
239241
private static void applyRateLimiter(
240242
ConfigurationComparer changes, LogProbe.Sampling globalSampling) {
241243
// ensure rate is up-to-date for all new probes
242-
for (ProbeDefinition addedDefinitions : changes.getAddedDefinitions()) {
243-
if (addedDefinitions instanceof LogProbe) {
244-
LogProbe probe = (LogProbe) addedDefinitions;
245-
LogProbe.Sampling sampling = probe.getSampling();
246-
ProbeRateLimiter.setRate(
247-
probe.getId(),
248-
sampling != null
249-
? sampling.getSnapshotsPerSecond()
250-
: getDefaultRateLimitPerProbe(probe),
251-
probe.isCaptureSnapshot());
244+
for (ProbeDefinition added : changes.getAddedDefinitions()) {
245+
if (added instanceof Sampled) {
246+
Sampled probe = (Sampled) added;
247+
Sampling sampling = probe.getSampling();
248+
double rate = getDefaultRateLimitPerProbe(probe);
249+
if (sampling != null && sampling.getEventsPerSecond() != 0) {
250+
rate = sampling.getEventsPerSecond();
251+
}
252+
ProbeRateLimiter.setRate(probe.getId(), rate, probe.isCaptureSnapshot());
252253
}
253254
}
254255
// remove rate for all removed probes
@@ -263,7 +264,7 @@ private static void applyRateLimiter(
263264
}
264265
}
265266

266-
private static double getDefaultRateLimitPerProbe(LogProbe probe) {
267+
private static double getDefaultRateLimitPerProbe(Sampled probe) {
267268
return probe.isCaptureSnapshot()
268269
? ProbeRateLimiter.DEFAULT_SNAPSHOT_RATE
269270
: ProbeRateLimiter.DEFAULT_LOG_RATE;

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
import org.slf4j.LoggerFactory;
4646

4747
/** Stores definition of a log probe */
48-
public class LogProbe extends ProbeDefinition {
48+
public class LogProbe extends ProbeDefinition implements Sampled {
4949
private static final Logger LOGGER = LoggerFactory.getLogger(LogProbe.class);
5050
private static final Limits LIMITS = new Limits(1, 3, 8192, 5);
5151
private static final int LOG_MSG_LIMIT = 8192;
@@ -220,8 +220,8 @@ public static Limits toLimits(Capture capture) {
220220
}
221221

222222
/** Stores sampling configuration */
223-
public static final class Sampling {
224-
private final double snapshotsPerSecond;
223+
public static final class Sampling extends com.datadog.debugger.probe.Sampling {
224+
private double snapshotsPerSecond;
225225

226226
public Sampling(double snapshotsPerSecond) {
227227
this.snapshotsPerSecond = snapshotsPerSecond;
@@ -231,6 +231,11 @@ public double getSnapshotsPerSecond() {
231231
return snapshotsPerSecond;
232232
}
233233

234+
@Override
235+
public double getEventsPerSecond() {
236+
return snapshotsPerSecond;
237+
}
238+
234239
@Generated
235240
@Override
236241
public boolean equals(Object o) {

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package com.datadog.debugger.probe;
2+
3+
public interface Sampled {
4+
Sampling getSampling();
5+
6+
String getId();
7+
8+
boolean isCaptureSnapshot();
9+
}

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

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
package com.datadog.debugger.probe;
2+
3+
import java.util.Objects;
4+
5+
/** Stores sampling configuration */
6+
public class Sampling {
7+
private int coolDownInSeconds;
8+
private double eventsPerSecond;
9+
private volatile long nextExecution;
10+
private int executionInterval = -1;
11+
12+
public Sampling() {}
13+
14+
public Sampling(int coolDownInSeconds) {
15+
this.coolDownInSeconds = coolDownInSeconds;
16+
this.executionInterval = coolDownInSeconds * 1000;
17+
}
18+
19+
public Sampling(double eventsPerSecond) {
20+
this.eventsPerSecond = eventsPerSecond;
21+
}
22+
23+
public Sampling(int coolDownInSeconds, double eventsPerSecond) {
24+
this.coolDownInSeconds = coolDownInSeconds;
25+
this.executionInterval = coolDownInSeconds * 1000;
26+
27+
this.eventsPerSecond = eventsPerSecond;
28+
}
29+
30+
public int getCoolDownInSeconds() {
31+
return coolDownInSeconds;
32+
}
33+
34+
public boolean inCoolDown() {
35+
if (System.currentTimeMillis() > nextExecution) {
36+
nextExecution = System.currentTimeMillis() + getExecutionInterval();
37+
return false;
38+
}
39+
return true;
40+
}
41+
42+
public double getEventsPerSecond() {
43+
return eventsPerSecond;
44+
}
45+
46+
private int getExecutionInterval() {
47+
if (executionInterval == -1) {
48+
executionInterval = coolDownInSeconds * 1000;
49+
}
50+
return executionInterval;
51+
}
52+
53+
@Override
54+
public boolean equals(Object o) {
55+
if (this == o) {
56+
return true;
57+
}
58+
if (!(o instanceof Sampling)) {
59+
return false;
60+
}
61+
Sampling sampling = (Sampling) o;
62+
return Objects.equals(coolDownInSeconds, sampling.coolDownInSeconds)
63+
&& Objects.equals(eventsPerSecond, sampling.eventsPerSecond);
64+
}
65+
66+
@Override
67+
public int hashCode() {
68+
return Objects.hash(coolDownInSeconds, eventsPerSecond);
69+
}
70+
71+
@Override
72+
public String toString() {
73+
return String.format(
74+
"Sampling{coolDownInSeconds=%d, eventsPerSecond=%s}", coolDownInSeconds, eventsPerSecond);
75+
}
76+
}

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

Lines changed: 79 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,54 @@
11
package com.datadog.debugger.probe;
22

3+
import static java.lang.String.format;
4+
35
import com.datadog.debugger.agent.DebuggerAgent;
46
import com.datadog.debugger.agent.Generated;
7+
import com.datadog.debugger.el.EvaluationException;
8+
import com.datadog.debugger.el.ProbeCondition;
59
import com.datadog.debugger.instrumentation.CapturedContextInstrumentor;
610
import com.datadog.debugger.instrumentation.DiagnosticMessage;
711
import com.datadog.debugger.instrumentation.InstrumentationResult;
812
import com.datadog.debugger.instrumentation.MethodInfo;
913
import datadog.trace.bootstrap.debugger.CapturedContext;
1014
import datadog.trace.bootstrap.debugger.MethodLocation;
1115
import datadog.trace.bootstrap.debugger.ProbeId;
16+
import datadog.trace.bootstrap.debugger.ProbeRateLimiter;
1217
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
1318
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
19+
import datadog.trace.bootstrap.instrumentation.api.Tags;
1420
import java.util.Arrays;
1521
import java.util.List;
1622
import java.util.Objects;
1723
import org.slf4j.Logger;
1824
import org.slf4j.LoggerFactory;
1925

20-
public class TriggerProbe extends ProbeDefinition {
26+
public class TriggerProbe extends ProbeDefinition implements Sampled {
2127
private static final Logger LOGGER = LoggerFactory.getLogger(TriggerProbe.class);
2228

29+
private ProbeCondition probeCondition;
30+
private Sampling sampling;
31+
private String sessionId;
32+
2333
// no-arg constructor is required by Moshi to avoid creating instance with unsafe and by-passing
2434
// constructors, including field initializers.
2535
public TriggerProbe() {
26-
this(LANGUAGE, null, null);
36+
this(null, null, null, null, null);
37+
}
38+
39+
public TriggerProbe(
40+
ProbeId probeId,
41+
String[] tagStrs,
42+
Where where,
43+
ProbeCondition probeCondition,
44+
Sampling sampling) {
45+
super("java", probeId, tagStrs, where, MethodLocation.ENTRY);
46+
this.probeCondition = probeCondition;
47+
this.sampling = sampling;
2748
}
2849

29-
public TriggerProbe(String language, ProbeId probeId, Where where) {
30-
super(language, probeId, (Tag[]) null, where, MethodLocation.ENTRY);
50+
public TriggerProbe(ProbeId probeId, Where where) {
51+
this(probeId, null, where, null, null);
3152
}
3253

3354
@Override
@@ -37,20 +58,59 @@ public InstrumentationResult.Status instrument(
3758
.instrument();
3859
}
3960

61+
public Sampling getSampling() {
62+
return sampling;
63+
}
64+
65+
public TriggerProbe setSampling(Sampling sampling) {
66+
this.sampling = sampling;
67+
return this;
68+
}
69+
70+
public TriggerProbe setProbeCondition(ProbeCondition probeCondition) {
71+
this.probeCondition = probeCondition;
72+
return this;
73+
}
74+
4075
@Override
4176
public void evaluate(
42-
CapturedContext context, CapturedContext.Status status, MethodLocation methodLocation) {
43-
decorateTags();
77+
CapturedContext context, CapturedContext.Status status, MethodLocation location) {
78+
79+
if (sampling == null || !sampling.inCoolDown()) {
80+
boolean sample = true;
81+
if (!hasCondition()) {
82+
sample = MethodLocation.isSame(location, evaluateAt) && ProbeRateLimiter.tryProbe(id);
83+
}
84+
boolean value = evaluateCondition(context);
85+
86+
if (hasCondition() && value || !hasCondition() && sample) {
87+
decorateTags();
88+
}
89+
}
90+
}
91+
92+
private boolean evaluateCondition(CapturedContext capture) {
93+
if (probeCondition == null) {
94+
return true;
95+
}
96+
long start = System.nanoTime();
97+
try {
98+
return !probeCondition.execute(capture);
99+
} catch (EvaluationException ex) {
100+
DebuggerAgent.getSink().getProbeStatusSink().addError(probeId, ex);
101+
return false;
102+
} finally {
103+
LOGGER.debug(
104+
"ProbeCondition for probe[{}] evaluated in {}ns", id, (System.nanoTime() - start));
105+
}
44106
}
45107

46108
private void decorateTags() {
47109
AgentTracer.TracerAPI tracerAPI = AgentTracer.get();
48110

49111
AgentSpan agentSpan = tracerAPI.activeSpan().getLocalRootSpan();
50-
agentSpan.setTag("_dd.p.debug", "1");
51-
agentSpan.setTag("_dd.ld.probe_id", probeId.getId());
52-
53-
DebuggerAgent.getSink().getProbeStatusSink().addEmitting(probeId);
112+
agentSpan.setTag(Tags.PROPAGATED_DEBUG, "1");
113+
agentSpan.setTag(format("_dd.ld.probe_id.%s", probeId.getId()), true);
54114
}
55115

56116
@Override
@@ -61,8 +121,12 @@ public CapturedContext.Status createStatus() {
61121
@Generated
62122
@Override
63123
public boolean equals(Object o) {
64-
if (this == o) return true;
65-
if (o == null || getClass() != o.getClass()) return false;
124+
if (this == o) {
125+
return true;
126+
}
127+
if (o == null || getClass() != o.getClass()) {
128+
return false;
129+
}
66130
TriggerProbe that = (TriggerProbe) o;
67131
return Objects.equals(language, that.language)
68132
&& Objects.equals(id, that.id)
@@ -81,33 +145,10 @@ public int hashCode() {
81145
return result;
82146
}
83147

84-
@Generated
85148
@Override
86149
public String toString() {
87-
return getClass().getSimpleName()
88-
+ "{"
89-
+ "language='"
90-
+ language
91-
+ '\''
92-
+ ", id='"
93-
+ id
94-
+ '\''
95-
+ ", version="
96-
+ version
97-
+ ", where="
98-
+ where
99-
+ ", evaluateAt="
100-
+ evaluateAt
101-
+ "} ";
102-
}
103-
104-
public static Builder builder() {
105-
return new Builder();
106-
}
107-
108-
public static class Builder extends ProbeDefinition.Builder<Builder> {
109-
public TriggerProbe build() {
110-
return new TriggerProbe(language, probeId, where);
111-
}
150+
return format(
151+
"TriggerProbe{id='%s', where=%s, sampling=%s, probeCondition=%s}",
152+
id, where, sampling, probeCondition);
112153
}
113154
}

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@
7373
import java.util.List;
7474
import java.util.Map;
7575
import java.util.Optional;
76+
import java.util.stream.Collectors;
7677
import org.joor.Reflect;
7778
import org.joor.ReflectException;
7879
import org.junit.jupiter.api.Assertions;
@@ -2375,7 +2376,7 @@ public void allProbesSameMethod() throws IOException, URISyntaxException {
23752376
.where(where)
23762377
.build())
23772378
.add(LogProbe.builder().probeId(PROBE_ID3).where(where).build())
2378-
.add(TriggerProbe.builder().probeId(PROBE_ID4).where(where).build())
2379+
.add(new TriggerProbe(PROBE_ID4, where))
23792380
.build();
23802381

23812382
CoreTracer tracer = CoreTracer.builder().build();
@@ -2392,8 +2393,15 @@ public void allProbesSameMethod() throws IOException, URISyntaxException {
23922393
int result = Reflect.onClass(testClass).call("main", "1").get();
23932394
// log probe
23942395
assertEquals(3, result);
2395-
assertEquals(1, snapshotListener.snapshots.size());
2396-
Snapshot snapshot = snapshotListener.snapshots.get(0);
2396+
List<Snapshot> snapshots = snapshotListener.snapshots;
2397+
assertEquals(
2398+
1,
2399+
snapshots.size(),
2400+
"More than one probe emitted a snapshot: "
2401+
+ snapshots.stream()
2402+
.map(snapshot -> snapshot.getProbe().getId())
2403+
.collect(Collectors.toList()));
2404+
Snapshot snapshot = snapshots.get(0);
23972405
assertEquals(PROBE_ID3.getId(), snapshot.getProbe().getId());
23982406
// span (deco) probe
23992407
assertEquals(1, traceInterceptor.getTrace().size());

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -391,14 +391,12 @@ protected TestSnapshotListener installProbes(
391391
for (LogProbe probe : logProbes) {
392392
if (probe.getSampling() != null) {
393393
ProbeRateLimiter.setRate(
394-
probe.getId(),
395-
probe.getSampling().getSnapshotsPerSecond(),
396-
probe.isCaptureSnapshot());
394+
probe.getId(), probe.getSampling().getEventsPerSecond(), probe.isCaptureSnapshot());
397395
}
398396
}
399397
}
400398
if (configuration.getSampling() != null) {
401-
ProbeRateLimiter.setGlobalSnapshotRate(configuration.getSampling().getSnapshotsPerSecond());
399+
ProbeRateLimiter.setGlobalSnapshotRate(configuration.getSampling().getEventsPerSecond());
402400
}
403401

404402
return listener;

0 commit comments

Comments
 (0)