Skip to content

Commit 793a2ad

Browse files
bm1549devflow.devflow-routing-intake
bm1549
and
devflow.devflow-routing-intake
authored
Add _dd.p.ksr propagated tag for Knuth sampling rate (#10802)
Add _dd.p.ksr propagated tag for Knuth sampling rate Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> fix: remove unused imports and update OT tests for _dd.p.ksr propagation - Remove unused imports in KnuthSamplingRateTest (CodeNarc violations) - Update OT31ApiTest and OT33ApiTest to expect _dd.p.ksr in x-datadog-tags when agent-rate sampler runs (UNSET priority) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> fix: add t.ksr to expected tracestate in OT compatibility tests The _dd.p.ksr propagated tag also appears in W3C tracestate as t.ksr. Update OT31 and OT33 test expectations for the UNSET context priority case where the agent-rate sampler runs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Fix forbidden API usage in formatKnuthSamplingRate Replace String#replaceAll (a forbidden API in this codebase) with manual character-based trailing-zero stripping logic that has the same semantics but avoids the regex-based method. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> fix: correct _dd.p.ksr ordering in OT x-datadog-tags expectations The PTagsCodec headerValue method outputs tags in order: dm, tid, ksr. The test datadogTags list had ksr before tid, causing a comparison failure. Reorder to match the actual output: dm, tid, ksr. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> fix: add _dd.p.ksr to expected meta maps in DDAgentApiTest The ksr implementation now adds _dd.p.ksr tag to spans with agent sampling rate, so the msgpack serialization test expectations need to include it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> fix: update propagation tests to include _dd.p.ksr in expected headers - OpenTelemetryTest: fix x-datadog-tags ordering (tid before ksr) - DatadogPropagatorTest: add ksr to expected tags when UNSET priority - OpenTracing32Test: add ksr and tid handling for UNSET priority case All follow PTagsCodec ordering: dm → tid → ksr Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> fix: add ksr and tid handling to OpenTracing31Test inject extract Update the test inject extract expectations to include _dd.p.ksr=1 in x-datadog-tags and t.ksr:1 in tracestate for the UNSET sampling case, following PTagsCodec ordering: dm -> tid -> ksr. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Address PR feedback: fix locale bug, move ksr formatting to PropagationTags - Use Locale.ROOT in String.format to prevent locale-sensitive decimal separators (e.g. comma in fr_FR) from corrupting x-datadog-tags - Move formatKnuthSamplingRate() from DDSpan to PTagsFactory so propagation encoding logic stays in the propagation layer - Change updateKnuthSamplingRate signature to accept double instead of pre-formatted String - Use indexOf('.') instead of contains(".") for minor perf improvement - Update test to exercise formatting through PropagationTags API Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> ci: retry - transient JBoss 503 failure in smoke tests Merge branch 'master' into brian.marks/add-ksr-tag Merge branch 'master' into brian.marks/add-ksr-tag Address PR feedback: rename datadogTags to expectedDataTags, store ksr as primitive double Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Optimize formatKnuthSamplingRate: replace String.format with char-array arithmetic Replace String.format(Locale.ROOT, "%.6g", rate) with manual char-array formatting that avoids Formatter/stream/boxing allocations. Benchmarks show ~30x improvement (320ns -> 11ns). Additionally, cache the TagValue in updateKnuthSamplingRate() so that getKnuthSamplingRateTagValue() is a simple volatile read (~1.2ns) instead of re-formatting and re-looking up the TagValue on every header injection. Add JMH benchmark (KnuthSamplingRateFormatBenchmark) comparing all three approaches and expand test coverage to all magnitude buckets. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Eliminate String.format entirely from formatKnuthSamplingRate Implement scientific notation (rates < 1e-4) with manual char-array formatting, removing the last String.format dependency. The method now uses zero Formatter/Locale allocations for all rate values. Add test coverage for scientific notation: 1e-05, 5e-05, 1.23457e-05, 1e-07, 5.5e-10. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Optimize KSR update path with static rate→TagValue cache Two changes: 1. Benchmark (address dougqh feedback): - Switch to Throughput mode + @threads(8) to surface GC pressure - @State(Scope.Thread): each thread gets its own PTags, models real traces - Add updateRateFreshTrace: resets instance cache each iteration to model per-trace cost (the actual hot path Doug was concerned about) - Update run instructions to include -t 8 -prof gc 2. Static cache for KSR TagValue (Option A): - Add static volatile cachedKsrRate + cachedKsrTagValue to PTags - On updateKnuthSamplingRate, check static cache before formatting - Eliminates char[]+String allocation on the per-trace path in steady state (every new PTags starts with NaN; without the cache, each trace root re-formats even when the rate is constant) - Race is benign: two threads computing the same rate produce equal TagValues - gc.alloc.rate.norm: updateRateFreshTrace goes from 80 B/op → ≈ 0 B/op structurally (not JIT-dependent) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: devflow.devflow-routing-intake <devflow.devflow-routing-intake@kubernetes.us1.ddbuild.io>
1 parent 828c9cc commit 793a2ad

File tree

13 files changed

+605
-24
lines changed

13 files changed

+605
-24
lines changed

‎dd-java-agent/instrumentation/opentelemetry/opentelemetry-0.3/src/test/groovy/OpenTelemetryTest.groovy‎

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -271,24 +271,38 @@ class OpenTelemetryTest extends InstrumentationSpecification {
271271
httpPropagator.inject(context, textMap, new TextMapSetter())
272272

273273
then:
274-
def expectedTraceparent = "00-${span.delegate.traceId.toHexStringPadded(32)}" +
275-
"-${DDSpanId.toHexStringPadded(span.delegate.spanId)}" +
274+
def traceId = span.delegate.traceId as DDTraceId
275+
def spanId = span.delegate.spanId
276+
def expectedTraceparent = "00-${traceId.toHexStringPadded(32)}" +
277+
"-${DDSpanId.toHexStringPadded(spanId)}" +
276278
"-" + (propagatedPriority > 0 ? "01" : "00")
277-
def expectedTracestate = "dd=s:${propagatedPriority};p:${DDSpanId.toHexStringPadded(span.delegate.spanId)}"
278-
def expectedDatadogTags = null
279+
def expectedTracestate = "dd=s:${propagatedPriority};p:${DDSpanId.toHexStringPadded(spanId)}"
280+
def expectedDataTags = []
279281
if (propagatedMechanism != UNKNOWN) {
280-
expectedDatadogTags = "_dd.p.dm=-" + propagatedMechanism
281-
expectedTracestate+= ";t.dm:-" + propagatedMechanism
282+
expectedDataTags << "_dd.p.dm=-" + propagatedMechanism
283+
expectedTracestate += ";t.dm:-" + propagatedMechanism
284+
}
285+
if (traceId.toHighOrderLong() != 0) {
286+
expectedTracestate += ";t.tid:${traceId.toHexStringPadded(32).substring(0, 16)}"
287+
}
288+
if (contextPriority == UNSET) {
289+
expectedTracestate += ";t.ksr:1"
290+
}
291+
if (traceId.toHighOrderLong() != 0) {
292+
expectedDataTags << "_dd.p.tid=" + traceId.toHexStringPadded(32).substring(0, 16)
293+
}
294+
if (contextPriority == UNSET) {
295+
expectedDataTags << "_dd.p.ksr=1"
282296
}
283297
def expectedTextMap = [
284-
"x-datadog-trace-id" : "$span.delegate.traceId",
285-
"x-datadog-parent-id" : "$span.delegate.spanId",
298+
"x-datadog-trace-id" : "$traceId",
299+
"x-datadog-parent-id" : "$spanId",
286300
"x-datadog-sampling-priority": propagatedPriority.toString(),
287301
"traceparent" : expectedTraceparent,
288302
"tracestate" : expectedTracestate,
289303
]
290-
if (expectedDatadogTags != null) {
291-
expectedTextMap.put("x-datadog-tags", expectedDatadogTags)
304+
if (!expectedDataTags.empty) {
305+
expectedTextMap.put("x-datadog-tags", expectedDataTags.join(','))
292306
}
293307
textMap == expectedTextMap
294308

‎dd-java-agent/instrumentation/opentelemetry/opentelemetry-1.4/src/test/groovy/opentelemetry14/context/propagation/DatadogPropagatorTest.groovy‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ class DatadogPropagatorTest extends AgentPropagatorTest {
3737
if (traceId.length() == 32) {
3838
tags+= '_dd.p.tid='+ traceId.substring(0, 16)
3939
}
40+
if (sampling == UNSET) {
41+
tags+= '_dd.p.ksr=1'
42+
}
4043
assert headers['x-datadog-tags'] == tags.join(',')
4144
assert headers['x-datadog-sampling-priority'] == samplingPriority
4245
}

‎dd-java-agent/instrumentation/opentracing/opentracing-0.31/src/test/groovy/OpenTracing31Test.groovy‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -284,22 +284,30 @@ class OpenTracing31Test extends InstrumentationSpecification {
284284
"-${DDSpanId.toHexStringPadded(context.delegate.spanId)}" +
285285
"-" + (propagatedPriority > 0 ? "01" : "00")
286286
def expectedTracestate = "dd=s:${propagatedPriority};p:${DDSpanId.toHexStringPadded(context.delegate.spanId)}"
287-
def expectedDatadogTags = null
287+
def datadogTags = []
288288
if (propagatedPriority > 0) {
289289
def effectiveSamplingMechanism = contextPriority == UNSET ? AGENT_RATE : samplingMechanism
290-
expectedDatadogTags = "_dd.p.dm=-" + effectiveSamplingMechanism
291-
expectedTracestate+= ";t.dm:-" + effectiveSamplingMechanism
290+
datadogTags << "_dd.p.dm=-" + effectiveSamplingMechanism
291+
expectedTracestate += ";t.dm:-" + effectiveSamplingMechanism
292+
}
293+
def traceId = context.delegate.traceId as DDTraceId
294+
if (traceId.toHighOrderLong() != 0) {
295+
expectedTracestate += ";t.tid:${traceId.toHexStringPadded(32).substring(0, 16)}"
296+
datadogTags << "_dd.p.tid=" + traceId.toHexStringPadded(32).substring(0, 16)
297+
}
298+
if (contextPriority == UNSET) {
299+
expectedTracestate += ";t.ksr:1"
300+
datadogTags << "_dd.p.ksr=1"
292301
}
293-
294302
def expectedTextMap = [
295303
"x-datadog-trace-id" : "$context.delegate.traceId",
296304
"x-datadog-parent-id" : "$context.delegate.spanId",
297305
"x-datadog-sampling-priority": propagatedPriority.toString(),
298306
"traceparent" : expectedTraceparent,
299307
"tracestate" : expectedTracestate,
300308
]
301-
if (expectedDatadogTags != null) {
302-
expectedTextMap.put("x-datadog-tags", expectedDatadogTags)
309+
if (!datadogTags.empty) {
310+
expectedTextMap.put("x-datadog-tags", datadogTags.join(','))
303311
}
304312
textMap == expectedTextMap
305313

‎dd-java-agent/instrumentation/opentracing/opentracing-0.32/src/test/groovy/OpenTracing32Test.groovy‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import datadog.trace.agent.test.InstrumentationSpecification
22
import datadog.trace.api.DDSpanId
33
import datadog.trace.api.DDTags
44
import datadog.trace.api.DDTraceId
5+
import datadog.trace.api.internal.util.LongStringUtils
56
import datadog.trace.api.interceptor.MutableSpan
67
import datadog.trace.bootstrap.instrumentation.api.AgentTracer
78
import datadog.trace.bootstrap.instrumentation.api.ResourceNamePriorities
@@ -299,21 +300,30 @@ class OpenTracing32Test extends InstrumentationSpecification {
299300
"-${DDSpanId.toHexStringPadded(context.delegate.spanId)}" +
300301
"-" + (propagatedPriority > 0 ? "01" : "00")
301302
def expectedTracestate = "dd=s:${propagatedPriority};p:${DDSpanId.toHexStringPadded(context.delegate.spanId)}"
302-
def expectedDatadogTags = null
303+
def datadogTags = []
303304
if (propagatedPriority > 0) {
304305
def effectiveSamplingMechanism = contextPriority == UNSET ? AGENT_RATE : samplingMechanism
305-
expectedDatadogTags = "_dd.p.dm=-" + effectiveSamplingMechanism
306+
datadogTags << "_dd.p.dm=-" + effectiveSamplingMechanism
306307
expectedTracestate+= ";t.dm:-" + effectiveSamplingMechanism
307308
}
309+
def traceId = context.delegate.traceId as DDTraceId
310+
if (traceId.toHighOrderLong() != 0) {
311+
expectedTracestate+= ";t.tid:${traceId.toHexStringPadded(32).substring(0, 16)}"
312+
datadogTags << "_dd.p.tid=" + LongStringUtils.toHexStringPadded(traceId.toHighOrderLong(), 16)
313+
}
314+
if (contextPriority == UNSET) {
315+
expectedTracestate+= ";t.ksr:1"
316+
datadogTags << "_dd.p.ksr=1"
317+
}
308318
def expectedTextMap = [
309319
"x-datadog-trace-id" : "$context.delegate.traceId",
310320
"x-datadog-parent-id" : "$context.delegate.spanId",
311321
"x-datadog-sampling-priority": propagatedPriority.toString(),
312322
"traceparent" : expectedTraceparent,
313323
"tracestate" : expectedTracestate
314324
]
315-
if (expectedDatadogTags != null) {
316-
expectedTextMap.put("x-datadog-tags", expectedDatadogTags)
325+
if (!datadogTags.empty) {
326+
expectedTextMap.put("x-datadog-tags", datadogTags.join(','))
317327
}
318328
textMap == expectedTextMap
319329

‎dd-trace-core/src/jmh/java/datadog/trace/core/propagation/ptags/KnuthSamplingRateFormatBenchmark.java‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
package datadog.trace.core.propagation.ptags;
2+
3+
import static java.util.concurrent.TimeUnit.SECONDS;
4+
5+
import datadog.trace.core.propagation.PropagationTags;
6+
import java.util.Locale;
7+
import org.openjdk.jmh.annotations.Benchmark;
8+
import org.openjdk.jmh.annotations.BenchmarkMode;
9+
import org.openjdk.jmh.annotations.Fork;
10+
import org.openjdk.jmh.annotations.Level;
11+
import org.openjdk.jmh.annotations.Measurement;
12+
import org.openjdk.jmh.annotations.Mode;
13+
import org.openjdk.jmh.annotations.OutputTimeUnit;
14+
import org.openjdk.jmh.annotations.Param;
15+
import org.openjdk.jmh.annotations.Scope;
16+
import org.openjdk.jmh.annotations.Setup;
17+
import org.openjdk.jmh.annotations.State;
18+
import org.openjdk.jmh.annotations.Threads;
19+
import org.openjdk.jmh.annotations.Warmup;
20+
import org.openjdk.jmh.infra.Blackhole;
21+
22+
/**
23+
* Benchmarks for formatting the Knuth sampling rate (_dd.p.ksr tag value).
24+
*
25+
* <p>The format requirement is %.6g semantics: 6 significant figures, no trailing zeros, using
26+
* fixed notation for values in [1e-4, 1] and scientific notation for smaller values.
27+
*
28+
* <p>Run with:
29+
*
30+
* <pre>
31+
* ./gradlew :dd-trace-core:jmhJar
32+
* java -jar dd-trace-core/build/libs/dd-trace-core-*-jmh.jar KnuthSamplingRateFormatBenchmark \
33+
* -t 8 -prof gc
34+
* </pre>
35+
*
36+
* <p>Use {@code -t 8} (or higher) to surface GC pressure from multi-threaded allocation. Use {@code
37+
* -prof gc} to see alloc rate (bytes/op) per benchmark — that's the primary signal for whether the
38+
* hot path is allocation-free.
39+
*/
40+
@State(Scope.Thread)
41+
@Warmup(iterations = 3, time = 10, timeUnit = SECONDS)
42+
@Measurement(iterations = 5, time = 10, timeUnit = SECONDS)
43+
@BenchmarkMode(Mode.Throughput)
44+
@OutputTimeUnit(SECONDS)
45+
@Threads(8)
46+
@Fork(value = 1)
47+
public class KnuthSamplingRateFormatBenchmark {
48+
49+
/**
50+
* Representative sampling rates. Most real-world rates are in [0.001, 1.0]. The 0.0001 value
51+
* exercises the edge of the fixed-notation range.
52+
*/
53+
@Param({"0.5", "0.1", "0.01", "0.001", "0.0001", "0.123456789", "0.999999"})
54+
double rate;
55+
56+
PTagsFactory.PTags ptags;
57+
58+
@Setup(Level.Trial)
59+
public void setUp() {
60+
ptags = (PTagsFactory.PTags) PropagationTags.factory().empty();
61+
ptags.updateKnuthSamplingRate(rate);
62+
}
63+
64+
/** Baseline: old implementation using String.format + substring trimming. */
65+
@Benchmark
66+
public void stringFormat(Blackhole bh) {
67+
bh.consume(stringFormatImpl(rate));
68+
}
69+
70+
/** Custom formatter: char-array arithmetic, no Formatter allocation. */
71+
@Benchmark
72+
public void customFormat(Blackhole bh) {
73+
bh.consume(PTagsFactory.PTags.formatKnuthSamplingRate(rate));
74+
}
75+
76+
/**
77+
* Cached TagValue: the full getKnuthSamplingRateTagValue() hot-path after caching. Should be
78+
* near-zero allocation (volatile read only).
79+
*/
80+
@Benchmark
81+
public void cachedTagValue(Blackhole bh) {
82+
bh.consume(ptags.getKnuthSamplingRateTagValue());
83+
}
84+
85+
/**
86+
* Models the per-trace allocation cost: resets the instance cache (simulating a new PTags), then
87+
* calls updateKnuthSamplingRate. This is what every trace root pays. With the static cache
88+
* applied, this should also be near-zero allocation after warmup.
89+
*/
90+
@Benchmark
91+
public void updateRateFreshTrace(Blackhole bh) {
92+
ptags.updateKnuthSamplingRate(Double.NaN); // reset instance cache, like a new PTags
93+
ptags.updateKnuthSamplingRate(rate);
94+
bh.consume(ptags.getKnuthSamplingRateTagValue());
95+
}
96+
97+
// ---- old implementation for comparison ----
98+
99+
static String stringFormatImpl(double rate) {
100+
String formatted = String.format(Locale.ROOT, "%.6g", rate);
101+
int dotIndex = formatted.indexOf('.');
102+
if (dotIndex >= 0) {
103+
int end = formatted.length();
104+
while (end > dotIndex + 1 && formatted.charAt(end - 1) == '0') {
105+
end--;
106+
}
107+
if (formatted.charAt(end - 1) == '.') {
108+
end--;
109+
}
110+
formatted = formatted.substring(0, end);
111+
}
112+
return formatted;
113+
}
114+
}

‎dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,12 @@ public DDSpan setSamplingPriority(
646646
int samplingPriority, CharSequence rate, double sampleRate, int samplingMechanism) {
647647
if (context.setSamplingPriority(samplingPriority, samplingMechanism)) {
648648
setMetric(rate, sampleRate);
649+
if (samplingMechanism == SamplingMechanism.AGENT_RATE
650+
|| samplingMechanism == SamplingMechanism.LOCAL_USER_RULE
651+
|| samplingMechanism == SamplingMechanism.REMOTE_USER_RULE
652+
|| samplingMechanism == SamplingMechanism.REMOTE_ADAPTIVE_RULE) {
653+
context.getPropagationTags().updateKnuthSamplingRate(sampleRate);
654+
}
649655
}
650656
return this;
651657
}

‎dd-trace-core/src/main/java/datadog/trace/core/propagation/PropagationTags.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,16 @@ public interface Factory {
130130

131131
public abstract String getDebugPropagation();
132132

133+
/**
134+
* Updates the Knuth sampling rate (_dd.p.ksr) propagated tag. This records the sampling rate that
135+
* was applied when making an agent-based or rule-based sampling decision. The rate is formatted
136+
* with up to 6 significant digits and no trailing zeros, matching the Go/Python reference
137+
* implementations (%.6g format).
138+
*
139+
* @param rate the sampling rate value
140+
*/
141+
public abstract void updateKnuthSamplingRate(double rate);
142+
133143
public HashMap<String, String> createTagMap() {
134144
HashMap<String, String> result = new HashMap<>();
135145
fillTagMap(result);

‎dd-trace-core/src/main/java/datadog/trace/core/propagation/ptags/PTagsCodec.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ abstract class PTagsCodec {
1818
protected static final TagKey TRACE_ID_TAG = TagKey.from("tid");
1919
protected static final TagKey TRACE_SOURCE_TAG = TagKey.from("ts");
2020
protected static final TagKey DEBUG_TAG = TagKey.from("debug");
21+
protected static final TagKey KNUTH_SAMPLING_RATE_TAG = TagKey.from("ksr");
2122
protected static final String PROPAGATION_ERROR_MALFORMED_TID = "malformed_tid ";
2223
protected static final String PROPAGATION_ERROR_INCONSISTENT_TID = "inconsistent_tid ";
2324
protected static final TagKey UPSTREAM_SERVICES_DEPRECATED_TAG = TagKey.from("upstream_services");
@@ -49,6 +50,11 @@ static String headerValue(PTagsCodec codec, PTags ptags) {
4950
if (ptags.getDebugPropagation() != null) {
5051
size = codec.appendTag(sb, DEBUG_TAG, TagValue.from(ptags.getDebugPropagation()), size);
5152
}
53+
if (ptags.getKnuthSamplingRateTagValue() != null) {
54+
size =
55+
codec.appendTag(
56+
sb, KNUTH_SAMPLING_RATE_TAG, ptags.getKnuthSamplingRateTagValue(), size);
57+
}
5258
Iterator<TagElement> it = ptags.getTagPairs().iterator();
5359
while (it.hasNext() && !codec.isTooLarge(sb, size)) {
5460
TagElement tagKey = it.next();
@@ -103,6 +109,11 @@ static void fillTagMap(PTags propagationTags, Map<String, String> tagMap) {
103109
tagMap.put(
104110
DEBUG_TAG.forType(Encoding.DATADOG).toString(), propagationTags.getDebugPropagation());
105111
}
112+
if (propagationTags.getKnuthSamplingRateTagValue() != null) {
113+
tagMap.put(
114+
KNUTH_SAMPLING_RATE_TAG.forType(Encoding.DATADOG).toString(),
115+
propagationTags.getKnuthSamplingRateTagValue().forType(Encoding.DATADOG).toString());
116+
}
106117
if (propagationTags.getTraceIdHighOrderBitsHexTagValue() != null) {
107118
tagMap.put(
108119
TRACE_ID_TAG.forType(Encoding.DATADOG).toString(),

0 commit comments

Comments
 (0)