Skip to content

Commit 840f009

Browse files
jpbempeldevflow.devflow-routing-intake
jpbempel
and
devflow.devflow-routing-intake
authored
Fix NullPointerException on samplers (#11230)
Fix NullPointerException on samplers In some rare cases samplers are null for LogProbe because they are initialize separately in applyRateLimiter for new definitions. To fix this we refactor the way probe are deserialized when received from RC in a dedicated class where samplers are also initialized earlier and not based on new definitions compared to existing Configuration. SpanDecorationProbe created by builders are initialized in dedicated constructor. Introduce builder to TriggerProbe with same mechanism than other probes to harmonize. fix smoke test Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent ac5f8b1 commit 840f009

15 files changed

Lines changed: 350 additions & 125 deletions

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,7 @@ public boolean hasProbeRelatedChanges() {
9292

9393
public boolean hasRateLimitRelatedChanged() {
9494
return originalConfiguration != null
95-
&& originalConfiguration.getSampling() != incomingConfiguration.getSampling()
96-
|| hasProbeRelatedChanges();
95+
&& originalConfiguration.getSampling() != incomingConfiguration.getSampling();
9796
}
9897

9998
List<String> findChangesInBlockedTypes() {

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

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import com.datadog.debugger.probe.ExceptionProbe;
1010
import com.datadog.debugger.probe.LogProbe;
1111
import com.datadog.debugger.probe.ProbeDefinition;
12-
import com.datadog.debugger.probe.Sampled;
1312
import com.datadog.debugger.sink.DebuggerSink;
1413
import com.datadog.debugger.util.ExceptionHelper;
1514
import com.datadog.debugger.util.SpringHelper;
@@ -170,7 +169,7 @@ private void applyNewConfiguration(Configuration newConfiguration) {
170169
if (changes.hasRateLimitRelatedChanged()) {
171170
// apply rate limit config first to avoid racing with execution/instrumentation
172171
// of probes requiring samplers
173-
applyRateLimiter(changes, newConfiguration.getSampling());
172+
applyRateLimiter(newConfiguration.getSampling());
174173
}
175174
currentConfiguration = newConfiguration;
176175
if (changes.hasProbeRelatedChanges()) {
@@ -436,15 +435,7 @@ public ProbeImplementation resolve(int probeIndex) {
436435
return probeMetadata.getProbe(probeIndex);
437436
}
438437

439-
private static void applyRateLimiter(
440-
ConfigurationComparer changes, LogProbe.Sampling globalSampling) {
441-
// ensure rate is up-to-date for all new probes
442-
for (ProbeDefinition added : changes.getAddedDefinitions()) {
443-
if (added instanceof Sampled) {
444-
Sampled probe = (Sampled) added;
445-
probe.initSamplers();
446-
}
447-
}
438+
private static void applyRateLimiter(LogProbe.Sampling globalSampling) {
448439
// set global sampling
449440
if (globalSampling != null) {
450441
ProbeRateLimiter.setGlobalSnapshotRate(globalSampling.getSnapshotsPerSecond());

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

Lines changed: 12 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,24 @@
11
package com.datadog.debugger.agent;
22

33
import static com.datadog.debugger.agent.ConfigurationAcceptor.Source.REMOTE_CONFIG;
4+
import static com.datadog.debugger.probe.ProbeDefinitionDeserializer.deserializeConfiguration;
5+
import static com.datadog.debugger.probe.ProbeDefinitionDeserializer.deserializeLogProbe;
6+
import static com.datadog.debugger.probe.ProbeDefinitionDeserializer.deserializeMetricProbe;
7+
import static com.datadog.debugger.probe.ProbeDefinitionDeserializer.deserializeSpanDecorationProbe;
8+
import static com.datadog.debugger.probe.ProbeDefinitionDeserializer.deserializeSpanProbe;
9+
import static com.datadog.debugger.probe.ProbeDefinitionDeserializer.deserializeTriggerProbe;
410

511
import com.datadog.debugger.probe.LogProbe;
612
import com.datadog.debugger.probe.MetricProbe;
713
import com.datadog.debugger.probe.ProbeDefinition;
814
import com.datadog.debugger.probe.SpanDecorationProbe;
915
import com.datadog.debugger.probe.SpanProbe;
1016
import com.datadog.debugger.probe.TriggerProbe;
11-
import com.datadog.debugger.util.MoshiHelper;
12-
import com.squareup.moshi.JsonAdapter;
1317
import datadog.remoteconfig.PollingRateHinter;
1418
import datadog.remoteconfig.state.ConfigKey;
1519
import datadog.remoteconfig.state.ProductListener;
1620
import datadog.trace.api.Config;
1721
import datadog.trace.util.TagsHelper;
18-
import java.io.ByteArrayInputStream;
1922
import java.io.IOException;
2023
import java.util.ArrayList;
2124
import java.util.Collection;
@@ -24,7 +27,6 @@
2427
import java.util.function.Consumer;
2528
import java.util.function.Predicate;
2629
import java.util.regex.Pattern;
27-
import okio.Okio;
2830
import org.slf4j.Logger;
2931
import org.slf4j.LoggerFactory;
3032

@@ -37,54 +39,6 @@ public class DebuggerProductChangesListener implements ProductListener {
3739
private static final Logger LOGGER =
3840
LoggerFactory.getLogger(DebuggerProductChangesListener.class);
3941

40-
static class Adapter {
41-
static final JsonAdapter<Configuration> CONFIGURATION_JSON_ADAPTER =
42-
MoshiHelper.createMoshiConfig().adapter(Configuration.class);
43-
44-
static final JsonAdapter<MetricProbe> METRIC_PROBE_JSON_ADAPTER =
45-
MoshiHelper.createMoshiConfig().adapter(MetricProbe.class);
46-
47-
static final JsonAdapter<LogProbe> LOG_PROBE_JSON_ADAPTER =
48-
MoshiHelper.createMoshiConfig().adapter(LogProbe.class);
49-
50-
static final JsonAdapter<SpanProbe> SPAN_PROBE_JSON_ADAPTER =
51-
MoshiHelper.createMoshiConfig().adapter(SpanProbe.class);
52-
53-
static final JsonAdapter<TriggerProbe> TRIGGER_PROBE_JSON_ADAPTER =
54-
MoshiHelper.createMoshiConfig().adapter(TriggerProbe.class);
55-
56-
static final JsonAdapter<SpanDecorationProbe> SPAN_DECORATION_PROBE_JSON_ADAPTER =
57-
MoshiHelper.createMoshiConfig().adapter(SpanDecorationProbe.class);
58-
59-
static Configuration deserializeConfiguration(byte[] content) throws IOException {
60-
return deserialize(CONFIGURATION_JSON_ADAPTER, content);
61-
}
62-
63-
static MetricProbe deserializeMetricProbe(byte[] content) throws IOException {
64-
return deserialize(METRIC_PROBE_JSON_ADAPTER, content);
65-
}
66-
67-
static LogProbe deserializeLogProbe(byte[] content) throws IOException {
68-
return deserialize(LOG_PROBE_JSON_ADAPTER, content);
69-
}
70-
71-
static SpanProbe deserializeSpanProbe(byte[] content) throws IOException {
72-
return deserialize(SPAN_PROBE_JSON_ADAPTER, content);
73-
}
74-
75-
static TriggerProbe deserializeTriggerProbe(byte[] content) throws IOException {
76-
return deserialize(TRIGGER_PROBE_JSON_ADAPTER, content);
77-
}
78-
79-
static SpanDecorationProbe deserializeSpanDecorationProbe(byte[] content) throws IOException {
80-
return deserialize(SPAN_DECORATION_PROBE_JSON_ADAPTER, content);
81-
}
82-
83-
private static <T> T deserialize(JsonAdapter<T> adapter, byte[] content) throws IOException {
84-
return adapter.fromJson(Okio.buffer(Okio.source(new ByteArrayInputStream(content))));
85-
}
86-
}
87-
8842
private static final Predicate<String> IS_UUID =
8943
Pattern.compile(
9044
"^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$")
@@ -105,22 +59,22 @@ public void accept(ConfigKey configKey, byte[] content, PollingRateHinter pollin
10559
String configId = configKey.getConfigId();
10660
try {
10761
if (configId.startsWith(METRIC_PROBE_PREFIX)) {
108-
MetricProbe metricProbe = Adapter.deserializeMetricProbe(content);
62+
MetricProbe metricProbe = deserializeMetricProbe(content);
10963
configChunks.put(configId, definitions -> definitions.add(metricProbe));
11064
} else if (configId.startsWith(LOG_PROBE_PREFIX)) {
111-
LogProbe logProbe = Adapter.deserializeLogProbe(content);
65+
LogProbe logProbe = deserializeLogProbe(content);
11266
configChunks.put(configId, definitions -> definitions.add(logProbe));
11367
} else if (configId.startsWith(SPAN_PROBE_PREFIX)) {
114-
SpanProbe spanProbe = Adapter.deserializeSpanProbe(content);
68+
SpanProbe spanProbe = deserializeSpanProbe(content);
11569
configChunks.put(configId, definitions -> definitions.add(spanProbe));
11670
} else if (configId.startsWith(TRIGGER_PROBE_PREFIX)) {
117-
TriggerProbe triggerProbe = Adapter.deserializeTriggerProbe(content);
71+
TriggerProbe triggerProbe = deserializeTriggerProbe(content);
11872
configChunks.put(configId, definitions -> definitions.add(triggerProbe));
11973
} else if (configId.startsWith(SPAN_DECORATION_PROBE_PREFIX)) {
120-
SpanDecorationProbe spanDecorationProbe = Adapter.deserializeSpanDecorationProbe(content);
74+
SpanDecorationProbe spanDecorationProbe = deserializeSpanDecorationProbe(content);
12175
configChunks.put(configId, definitions -> definitions.add(spanDecorationProbe));
12276
} else if (IS_UUID.test(configId)) {
123-
Configuration newConfig = Adapter.deserializeConfiguration(content);
77+
Configuration newConfig = deserializeConfiguration(content);
12478
if (newConfig.getService().equals(serviceName)) {
12579
configChunks.put(
12680
configId,

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

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
package com.datadog.debugger.probe;
2+
3+
import com.datadog.debugger.agent.Configuration;
4+
import com.datadog.debugger.util.MoshiHelper;
5+
import com.squareup.moshi.JsonAdapter;
6+
import java.io.ByteArrayInputStream;
7+
import java.io.IOException;
8+
import okio.Okio;
9+
10+
public class ProbeDefinitionDeserializer {
11+
static final JsonAdapter<Configuration> CONFIGURATION_JSON_ADAPTER =
12+
MoshiHelper.createMoshiConfig().adapter(Configuration.class);
13+
static final JsonAdapter<MetricProbe> METRIC_PROBE_JSON_ADAPTER =
14+
MoshiHelper.createMoshiConfig().adapter(MetricProbe.class);
15+
static final JsonAdapter<LogProbe> LOG_PROBE_JSON_ADAPTER =
16+
MoshiHelper.createMoshiConfig().adapter(LogProbe.class);
17+
static final JsonAdapter<SpanProbe> SPAN_PROBE_JSON_ADAPTER =
18+
MoshiHelper.createMoshiConfig().adapter(SpanProbe.class);
19+
static final JsonAdapter<TriggerProbe> TRIGGER_PROBE_JSON_ADAPTER =
20+
MoshiHelper.createMoshiConfig().adapter(TriggerProbe.class);
21+
static final JsonAdapter<SpanDecorationProbe> SPAN_DECORATION_PROBE_JSON_ADAPTER =
22+
MoshiHelper.createMoshiConfig().adapter(SpanDecorationProbe.class);
23+
24+
public static Configuration deserializeConfiguration(byte[] content) throws IOException {
25+
return deserialize(CONFIGURATION_JSON_ADAPTER, content);
26+
}
27+
28+
public static MetricProbe deserializeMetricProbe(byte[] content) throws IOException {
29+
return deserialize(METRIC_PROBE_JSON_ADAPTER, content);
30+
}
31+
32+
public static LogProbe deserializeLogProbe(byte[] content) throws IOException {
33+
LogProbe logProbe = deserialize(LOG_PROBE_JSON_ADAPTER, content);
34+
logProbe.initSamplers();
35+
return logProbe;
36+
}
37+
38+
public static SpanProbe deserializeSpanProbe(byte[] content) throws IOException {
39+
return deserialize(SPAN_PROBE_JSON_ADAPTER, content);
40+
}
41+
42+
public static TriggerProbe deserializeTriggerProbe(byte[] content) throws IOException {
43+
TriggerProbe triggerProbe = deserialize(TRIGGER_PROBE_JSON_ADAPTER, content);
44+
triggerProbe.initSamplers();
45+
return triggerProbe;
46+
}
47+
48+
public static SpanDecorationProbe deserializeSpanDecorationProbe(byte[] content)
49+
throws IOException {
50+
SpanDecorationProbe spanDecorationProbe =
51+
deserialize(SPAN_DECORATION_PROBE_JSON_ADAPTER, content);
52+
spanDecorationProbe.initSamplers();
53+
return spanDecorationProbe;
54+
}
55+
56+
private static <T> T deserialize(JsonAdapter<T> adapter, byte[] content) throws IOException {
57+
return adapter.fromJson(Okio.buffer(Okio.source(new ByteArrayInputStream(content))));
58+
}
59+
}

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

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,18 @@ public SpanDecorationProbe(
179179
this.decorations = decorations;
180180
}
181181

182+
public SpanDecorationProbe(SpanDecorationProbe.Builder builder) {
183+
this(
184+
builder.language,
185+
builder.probeId,
186+
builder.tagStrs,
187+
builder.where,
188+
builder.evaluateAt,
189+
builder.targetSpan,
190+
builder.decorations);
191+
initSamplers();
192+
}
193+
182194
@Override
183195
public InstrumentationResult.Status instrument(
184196
MethodInfo methodInfo, List<DiagnosticMessage> diagnostics, List<Integer> probeIndices) {
@@ -399,26 +411,25 @@ public static SpanDecorationProbe.Builder builder() {
399411

400412
public static class Builder extends ProbeDefinition.Builder<SpanDecorationProbe.Builder> {
401413
private TargetSpan targetSpan;
402-
private List<Decoration> decorate;
414+
private List<Decoration> decorations;
403415

404416
public Builder targetSpan(TargetSpan targetSpan) {
405417
this.targetSpan = targetSpan;
406418
return this;
407419
}
408420

409-
public Builder decorate(List<Decoration> decorate) {
410-
this.decorate = decorate;
421+
public Builder decorations(List<Decoration> decorations) {
422+
this.decorations = decorations;
411423
return this;
412424
}
413425

414-
public Builder decorate(Decoration decoration) {
415-
this.decorate = Collections.singletonList(decoration);
426+
public Builder decorations(Decoration decoration) {
427+
this.decorations = Collections.singletonList(decoration);
416428
return this;
417429
}
418430

419431
public SpanDecorationProbe build() {
420-
return new SpanDecorationProbe(
421-
LANGUAGE, probeId, tagStrs, where, evaluateAt, targetSpan, decorate);
432+
return new SpanDecorationProbe(this);
422433
}
423434
}
424435
}

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

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ public TriggerProbe(ProbeId probeId, Where where) {
5353
this(probeId, null, where, null, null);
5454
}
5555

56+
public TriggerProbe(TriggerProbe.Builder builder) {
57+
this(builder.probeId, builder.tagStrs, builder.where, builder.probeCondition, builder.sampling);
58+
initSamplers();
59+
}
60+
5661
@Override
5762
public InstrumentationResult.Status instrument(
5863
MethodInfo methodInfo, List<DiagnosticMessage> diagnostics, List<Integer> probeIndices) {
@@ -144,7 +149,7 @@ private void decorateTags() {
144149

145150
AgentSpan agentSpan = tracerAPI.activeSpan().getLocalRootSpan();
146151
agentSpan.setTag(Tags.PROPAGATED_DEBUG, sessionId + ":1");
147-
agentSpan.setTag(format("_dd.ld.probe_id.%s", probeId.getId()), true);
152+
agentSpan.setTag(format("_dd.ld.probe_id.%s", getProbeId().getId()), true);
148153
}
149154

150155
@Override
@@ -197,4 +202,27 @@ public String toString() {
197202
version,
198203
where);
199204
}
205+
206+
public static TriggerProbe.Builder builder() {
207+
return new TriggerProbe.Builder();
208+
}
209+
210+
public static class Builder extends ProbeDefinition.Builder<TriggerProbe.Builder> {
211+
private ProbeCondition probeCondition;
212+
private Sampling sampling;
213+
214+
public TriggerProbe.Builder when(ProbeCondition probeCondition) {
215+
this.probeCondition = probeCondition;
216+
return this;
217+
}
218+
219+
public TriggerProbe.Builder sampling(Sampling sampling) {
220+
this.sampling = sampling;
221+
return this;
222+
}
223+
224+
public TriggerProbe build() {
225+
return new TriggerProbe(this);
226+
}
227+
}
200228
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2824,7 +2824,7 @@ public void allProbesSameMethod() throws IOException, URISyntaxException {
28242824
.probeId(PROBE_ID)
28252825
.where(where)
28262826
.targetSpan(SpanDecorationProbe.TargetSpan.ACTIVE)
2827-
.decorate(
2827+
.decorations(
28282828
new SpanDecorationProbe.Decoration(
28292829
null,
28302830
Arrays.asList(
@@ -2842,7 +2842,7 @@ public void allProbesSameMethod() throws IOException, URISyntaxException {
28422842
.where(where)
28432843
.build())
28442844
.add(LogProbe.builder().probeId(PROBE_ID3).where(where).build())
2845-
.add(new TriggerProbe(PROBE_ID4, where))
2845+
.add(TriggerProbe.builder().probeId(PROBE_ID4).where(where).build())
28462846
.build();
28472847

28482848
CoreTracer tracer = CoreTracer.builder().build();

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import com.datadog.debugger.probe.SpanDecorationProbe;
1919
import com.datadog.debugger.probe.SpanProbe;
2020
import com.datadog.debugger.probe.TriggerProbe;
21-
import com.datadog.debugger.probe.Where;
2221
import com.datadog.debugger.util.MoshiHelper;
2322
import com.squareup.moshi.JsonAdapter;
2423
import com.squareup.moshi.Types;
@@ -410,7 +409,10 @@ private static LogProbe createLog(
410409

411410
private static TriggerProbe createTriggerProbe(
412411
String id, String typeName, String methodName, String signature) {
413-
return new TriggerProbe(new ProbeId(id, 0), Where.of(typeName, methodName, signature));
412+
return TriggerProbe.builder()
413+
.probeId(new ProbeId(id, 0))
414+
.where(typeName, methodName, signature)
415+
.build();
414416
}
415417

416418
private static SpanProbe createSpan(
@@ -438,7 +440,7 @@ private static SpanDecorationProbe createDecorationSpan(
438440
.evaluateAt(MethodLocation.ENTRY)
439441
.tags("tag1:value1", "tag2:value2")
440442
.targetSpan(targetSpan)
441-
.decorate(decoration)
443+
.decorations(decoration)
442444
.build();
443445
}
444446

0 commit comments

Comments
 (0)