Skip to content

Commit 8d49177

Browse files
authored
Merge pull request #3701 from DataDog/brian.marks/add-ksr-tag
Add _dd.p.ksr propagated tag for Knuth sampling rate
1 parent 93187d9 commit 8d49177

8 files changed

Lines changed: 125 additions & 2 deletions

File tree

ext/priority_sampling/priority_sampling.c

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616
#include "agent_info.h"
1717

1818
/* Sampling constants */
19-
const uint64_t KNUTH_FACTOR = 1111111111111111111ULL;
20-
const uint64_t MAX_TRACE_ID = ~0ULL; // 2^64-1 - This represents the maximum value of a Trace ID
19+
static const uint64_t KNUTH_FACTOR = 1111111111111111111ULL;
20+
static const uint64_t MAX_TRACE_ID = ~0ULL; // 2^64-1 - This represents the maximum value of a Trace ID
2121

2222
ZEND_EXTERN_MODULE_GLOBALS(ddtrace);
2323

@@ -62,6 +62,24 @@ static void dd_update_decision_maker_tag(ddtrace_root_span_data *root_span,
6262
}
6363
}
6464

65+
static void dd_update_knuth_sampling_rate_tag(ddtrace_root_span_data *root_span, double sample_rate) {
66+
zend_array *meta = ddtrace_property_array(&root_span->property_meta);
67+
68+
char buf[32];
69+
snprintf(buf, sizeof(buf), "%.6g", sample_rate);
70+
71+
// Skip update if already set to the same value
72+
zval *existing = zend_hash_str_find(meta, ZEND_STRL("_dd.p.ksr"));
73+
if (existing && Z_TYPE_P(existing) == IS_STRING && strcmp(Z_STRVAL_P(existing), buf) == 0) {
74+
return;
75+
}
76+
77+
zval ksr;
78+
ZVAL_STRING(&ksr, buf);
79+
zend_hash_str_update(meta, ZEND_STRL("_dd.p.ksr"), &ksr);
80+
zend_hash_str_add_empty_element(ddtrace_property_array(&root_span->property_propagated_tags), ZEND_STRL("_dd.p.ksr"));
81+
}
82+
6583
static bool dd_check_sampling_rule(zend_array *rule, ddtrace_span_data *span) {
6684
zval *service = &span->property_service;
6785
zval *resource = &span->property_resource;
@@ -293,6 +311,7 @@ static void dd_decide_on_sampling(ddtrace_root_span_data *span) {
293311
ZVAL_LONG(&priority_zv, PRIORITY_SAMPLING_AUTO_REJECT);
294312
ddtrace_assign_variable(&span->property_sampling_priority, &priority_zv);
295313
}
314+
zend_hash_str_del(ddtrace_property_array(&span->property_meta), ZEND_STRL("_dd.p.ksr"));
296315
return;
297316
} else {
298317
sample_rate = result.sampling_rate;
@@ -318,8 +337,10 @@ static void dd_decide_on_sampling(ddtrace_root_span_data *span) {
318337

319338
if (mechanism == DD_MECHANISM_MANUAL) {
320339
zend_hash_str_del(metrics, ZEND_STRL("_dd.rule_psr"));
340+
zend_hash_str_del(ddtrace_property_array(&span->property_meta), ZEND_STRL("_dd.p.ksr"));
321341
} else {
322342
zend_hash_str_update(metrics, ZEND_STRL("_dd.rule_psr"), &sample_rate_zv);
343+
dd_update_knuth_sampling_rate_tag(span, sample_rate);
323344
}
324345

325346
zend_hash_str_del(metrics, ZEND_STRL("_dd.agent_psr"));
@@ -329,6 +350,11 @@ static void dd_decide_on_sampling(ddtrace_root_span_data *span) {
329350
priority = sampling && !limited ? PRIORITY_SAMPLING_AUTO_KEEP : PRIORITY_SAMPLING_AUTO_REJECT;
330351

331352
zend_hash_str_update(metrics, ZEND_STRL("_dd.agent_psr"), &sample_rate_zv);
353+
if (mechanism == DD_MECHANISM_AGENT_RATE) {
354+
dd_update_knuth_sampling_rate_tag(span, sample_rate);
355+
} else {
356+
zend_hash_str_del(ddtrace_property_array(&span->property_meta), ZEND_STRL("_dd.p.ksr"));
357+
}
332358
}
333359

334360
if (limited) {

ext/serializer.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1906,6 +1906,7 @@ ddog_SpanBytes *ddtrace_serialize_span_to_rust_span(ddtrace_span_data *span, ddo
19061906
transfer_meta_data(rust_span, serialized_inferred_span, "error.stack", false);
19071907
transfer_meta_data(rust_span, serialized_inferred_span, "track_error", false);
19081908
transfer_meta_data(rust_span, serialized_inferred_span, "_dd.p.dm", true);
1909+
transfer_meta_data(rust_span, serialized_inferred_span, "_dd.p.ksr", false);
19091910
transfer_meta_data(rust_span, serialized_inferred_span, "_dd.p.tid", true);
19101911

19111912
ddog_set_span_error(serialized_inferred_span, ddog_get_span_error(rust_span));

tests/Common/SpanChecker.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,10 @@ function ($key) use ($pattern) {
478478
if (!isset($expectedTags['_dd.p.dm'])) {
479479
unset($filtered['_dd.p.dm']);
480480
}
481+
// Ignore _dd.p.ksr unless explicitly tested
482+
if (!isset($expectedTags['_dd.p.ksr'])) {
483+
unset($filtered['_dd.p.ksr']);
484+
}
481485
// Ignore _dd.p.tid unless explicitly tested
482486
if (!isset($expectedTags['_dd.p.tid'])) {
483487
unset($filtered['_dd.p.tid']);

tests/ext/inferred_proxy/sampling_rules.phpt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ echo json_encode(dd_trace_serialize_closed_spans(), JSON_PRETTY_PRINT);
4848
"service": "foo",
4949
"type": "web",
5050
"meta": {
51+
"_dd.p.ksr": "0.3",
5152
"http.method": "GET",
5253
"http.status_code": "200",
5354
"http.url": "http:\/\/localhost:8888\/foo",
@@ -71,6 +72,7 @@ echo json_encode(dd_trace_serialize_closed_spans(), JSON_PRETTY_PRINT);
7172
"type": "web",
7273
"meta": {
7374
"_dd.p.dm": "-3",
75+
"_dd.p.ksr": "0.3",
7476
"_dd.p.tid": "%s",
7577
"component": "aws-apigateway",
7678
"http.method": "GET",
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
--TEST--
2+
_dd.p.ksr propagated tag is set for rule-based sampling
3+
--ENV--
4+
DD_TRACE_SAMPLING_RULES=[{"sample_rate": 0.3}]
5+
DD_TRACE_GENERATE_ROOT_SPAN=1
6+
--FILE--
7+
<?php
8+
$root = \DDTrace\root_span();
9+
10+
\DDTrace\get_priority_sampling();
11+
12+
if ($root->metrics["_dd.rule_psr"] == 0.3) {
13+
echo "Rule OK\n";
14+
} else {
15+
var_dump($root->metrics);
16+
}
17+
18+
echo "_dd.p.ksr = ", isset($root->meta["_dd.p.ksr"]) ? $root->meta["_dd.p.ksr"] : "-", "\n";
19+
echo "_dd.p.dm = ", isset($root->meta["_dd.p.dm"]) ? $root->meta["_dd.p.dm"] : "-", "\n";
20+
?>
21+
--EXPECTREGEX--
22+
Rule OK
23+
_dd.p.ksr = 0.3
24+
_dd.p.dm = (-3|-)
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
--TEST--
2+
_dd.p.ksr propagated tag is NOT set for default sampling (only for explicit agent rates)
3+
--ENV--
4+
DD_TRACE_GENERATE_ROOT_SPAN=1
5+
--FILE--
6+
<?php
7+
$root = \DDTrace\root_span();
8+
9+
\DDTrace\get_priority_sampling();
10+
11+
if ($root->metrics["_dd.agent_psr"] === 1.0) {
12+
echo "Agent PSR OK\n";
13+
} else {
14+
echo "Agent PSR missing\n";
15+
}
16+
17+
echo "_dd.p.ksr = ", isset($root->meta["_dd.p.ksr"]) ? $root->meta["_dd.p.ksr"] : "not set", "\n";
18+
echo "_dd.p.dm = {$root->meta["_dd.p.dm"]}\n";
19+
?>
20+
--EXPECT--
21+
Agent PSR OK
22+
_dd.p.ksr = not set
23+
_dd.p.dm = -0
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
--TEST--
2+
_dd.p.ksr propagated tag is NOT set for manual sampling
3+
--ENV--
4+
DD_TRACE_SAMPLE_RATE=1
5+
DD_TRACE_GENERATE_ROOT_SPAN=1
6+
--FILE--
7+
<?php
8+
$root = \DDTrace\root_span();
9+
$root->meta["manual.keep"] = true;
10+
11+
\DDTrace\get_priority_sampling();
12+
13+
if (!isset($root->metrics["_dd.rule_psr"])) {
14+
echo "No rule_psr OK\n";
15+
} else {
16+
echo "rule_psr unexpectedly set\n";
17+
}
18+
19+
echo "_dd.p.ksr = ", isset($root->meta["_dd.p.ksr"]) ? $root->meta["_dd.p.ksr"] : "-", "\n";
20+
echo "_dd.p.dm = {$root->meta["_dd.p.dm"]}\n";
21+
?>
22+
--EXPECT--
23+
No rule_psr OK
24+
_dd.p.ksr = -
25+
_dd.p.dm = -4
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
--TEST--
2+
_dd.p.ksr propagated tag formats rate with up to 6 significant digits and no trailing zeros
3+
--ENV--
4+
DD_TRACE_SAMPLING_RULES=[{"sample_rate": 0.7654321}]
5+
DD_TRACE_GENERATE_ROOT_SPAN=1
6+
--FILE--
7+
<?php
8+
$root = \DDTrace\root_span();
9+
10+
\DDTrace\get_priority_sampling();
11+
12+
echo "_dd.p.ksr = ", isset($root->meta["_dd.p.ksr"]) ? $root->meta["_dd.p.ksr"] : "-", "\n";
13+
// Verify it's a string in meta, not metrics
14+
echo "is_string = ", is_string($root->meta["_dd.p.ksr"] ?? null) ? "true" : "false", "\n";
15+
?>
16+
--EXPECT--
17+
_dd.p.ksr = 0.765432
18+
is_string = true

0 commit comments

Comments
 (0)