Skip to content

Add _dd.p.ksr propagated tag for Knuth sampling rate#3701

Draft
bm1549 wants to merge 5 commits intomasterfrom
brian.marks/add-ksr-tag
Draft

Add _dd.p.ksr propagated tag for Knuth sampling rate#3701
bm1549 wants to merge 5 commits intomasterfrom
brian.marks/add-ksr-tag

Conversation

@bm1549
Copy link

@bm1549 bm1549 commented Mar 11, 2026

Description

Adds _dd.p.ksr (Knuth Sampling Rate) as a propagated tag set when agent-based or rule-based sampling decisions are made. The tag is stored in span meta (string type) with up to 6 significant digits and no trailing zeros.

To enable consistent sampling across tracers and backend retention filters, the backend needs to know the sampling rate applied by the tracer. Without transmitting the tracer's rate via _dd.p.ksr, backend resampling cannot correctly compute effective rates in multi-stage sampling scenarios.

See RFC: "Transmit Knuth sampling rate to backend"

Key files changed:

  • ext/priority_sampling/priority_sampling.c — Added dd_update_knuth_sampling_rate_tag() function
  • 4 phpt test files for rule sampling, default sampling, manual exclusion, and formatting

Related PRs across tracers:

Reviewer checklist

  • Test coverage seems ok.
  • Appropriate labels assigned.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@datadog-official
Copy link

datadog-official bot commented Mar 11, 2026

⚠️ Tests

Fix all issues with BitsAI or with Cursor

⚠️ Warnings

❄️ 8 New flaky tests detected

tmp/build_extension/tests/ext/priority_sampling/025-ksr-tag-rule-sampling.phpt (_dd.p.ksr propagated tag is set for rule-based sampling) from PHP.tmp.build_extension.tests.ext.priority_sampling (Datadog) (Fix with Cursor)
002+ 
004+ 
006+ NULL
007+ _dd.p.ksr = -
001- Rule OK
002- _dd.p.ksr = 0.3
     _dd.p.dm = (-3|-)
tmp/build_extension/tests/ext/priority_sampling/026-ksr-tag-default-sampling.phpt (_dd.p.ksr propagated tag is NOT set for default sampling (only for explicit agent rates)) from PHP.tmp.build_extension.tests.ext.priority_sampling (Datadog) (Fix with Cursor)
002+ 
004+ Agent PSR missing
001- Agent PSR OK
     _dd.p.ksr = not set
006+ 
003- _dd.p.dm = -0
008+ 
010+ _dd.p.dm =
tmp/build_extension/tests/ext/priority_sampling/027-ksr-tag-not-set-manual.phpt (_dd.p.ksr propagated tag is NOT set for manual sampling) from PHP.tmp.build_extension.tests.ext.priority_sampling (Datadog) (Fix with Cursor)
001+ Fatal error: Uncaught Error: Attempt to modify property "meta" on null in tmp/build_extension/tests/ext/priority_sampling/027-ksr-tag-not-set-manual.php:3
002+ Stack trace:
003+ #0 {main}
001- No rule_psr OK
002- _dd.p.ksr = -
003- _dd.p.dm = -4
004+   thrown in tmp/build_extension/tests/ext/priority_sampling/027-ksr-tag-not-set-manual.php on line 3
View all

🧪 1036 Tests failed

testSearchPhpBinaries from integration.DDTrace\Tests\Integration\PHPInstallerTest (Datadog) (Fix with Cursor)
Risky Test
phpvfscomposer://tests/vendor/phpunit/phpunit/phpunit:52
testSimplePushAndProcess from laravel-58-test.DDTrace\Tests\Integrations\Laravel\V5_8\QueueTest (Datadog) (Fix with Cursor)
Risky Test
phpvfscomposer://tests/vendor/phpunit/phpunit/phpunit:97
testSimplePushAndProcess from laravel-8x-test.DDTrace\Tests\Integrations\Laravel\V8_x\QueueTest (Datadog) (Fix with Cursor)
DDTrace\Tests\Integrations\Laravel\V8_x\QueueTest::testSimplePushAndProcess
Test code or tested code printed unexpected output: spanLinksTraceId: 69b0ee9c0000000055b245017a561d01
tid: 69b0ee9c00000000
hexProcessTraceId: 55b245017a561d01
hexProcessSpanId: d7540cd5a9af9810
processTraceId: 6175073911728119041
processSpanId: 15516040728012625936

phpvfscomposer://tests/vendor/phpunit/phpunit/phpunit:106
View all
This comment will be updated automatically if new data arrives.
🔗 Commit SHA: db13664 | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback!

The default sampling mechanism (DD_MECHANISM_DEFAULT) is not an agent
rate. Per the RFC, _dd.p.ksr should only be set for "Agent Sampling
rate or Trace Sampling rules". This prevents _dd.p.ksr from appearing
on every span when no agent rates are configured.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov-commenter
Copy link

codecov-commenter commented Mar 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.30%. Comparing base (42d0acc) to head (db13664).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3701      +/-   ##
==========================================
- Coverage   62.40%   62.30%   -0.11%     
==========================================
  Files         142      142              
  Lines       13586    13586              
  Branches     1775     1775              
==========================================
- Hits         8479     8465      -14     
- Misses       4301     4314      +13     
- Partials      806      807       +1     

see 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 42d0acc...db13664. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

bm1549 and others added 2 commits March 10, 2026 22:35
The inferred proxy sampling rules test uses explicit DD_TRACE_SAMPLING_RULES
which triggers rule-based sampling. The ksr tag now appears in span meta.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Two fixes for remaining CI failures after the initial KSR tag implementation:

1. serializer.c: Copy _dd.p.ksr from the root span to the inferred proxy
   span during serialization (using false to keep it on the root span too),
   matching the behavior expected by sampling_rules.phpt.

2. SpanChecker.php: Auto-ignore _dd.p.ksr in withExactTags() assertions
   unless the test explicitly tests for it, consistent with how _dd.p.dm
   and _dd.p.tid are handled. This fixes ResponseStatusCodeTest and other
   integration tests that don't test KSR behavior directly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@pr-commenter
Copy link

pr-commenter bot commented Mar 11, 2026

Benchmarks [ tracer ]

Benchmark execution time: 2026-03-11 05:33:12

Comparing candidate commit db13664 in PR branch brian.marks/add-ksr-tag with baseline commit 42d0acc in branch master.

Found 0 performance improvements and 23 performance regressions! Performance is the same for 170 metrics, 1 unstable metrics.

scenario:ContextPropagationBench/benchInject64Bit

  • 🟥 execution_time [+281.115ns; +370.885ns] or [+14.672%; +19.357%]

scenario:ContextPropagationBench/benchInject64Bit-opcache

  • 🟥 execution_time [+225.883ns; +316.117ns] or [+11.674%; +16.337%]

scenario:MessagePackSerializationBench/benchMessagePackSerialization-opcache

  • 🟥 execution_time [+2.540µs; +3.840µs] or [+2.429%; +3.672%]

scenario:SamplingRuleMatchingBench/benchGlobMatching1

  • 🟥 execution_time [+58.684ns; +186.516ns] or [+2.543%; +8.081%]

scenario:SamplingRuleMatchingBench/benchGlobMatching1-opcache

  • 🟥 execution_time [+140.506ns; +291.094ns] or [+6.015%; +12.461%]

scenario:SamplingRuleMatchingBench/benchGlobMatching2

  • 🟥 execution_time [+161.701ns; +314.699ns] or [+7.082%; +13.783%]

scenario:SamplingRuleMatchingBench/benchGlobMatching2-opcache

  • 🟥 execution_time [+90.793ns; +222.407ns] or [+3.745%; +9.174%]

scenario:SamplingRuleMatchingBench/benchGlobMatching3

  • 🟥 execution_time [+159.655ns; +297.745ns] or [+6.732%; +12.555%]

scenario:SamplingRuleMatchingBench/benchGlobMatching3-opcache

  • 🟥 execution_time [+189.472ns; +276.528ns] or [+7.769%; +11.339%]

scenario:SamplingRuleMatchingBench/benchGlobMatching4

  • 🟥 execution_time [+131.640ns; +256.160ns] or [+5.604%; +10.906%]

scenario:SamplingRuleMatchingBench/benchGlobMatching4-opcache

  • 🟥 execution_time [+98.318ns; +227.682ns] or [+3.959%; +9.169%]

scenario:SamplingRuleMatchingBench/benchRegexMatching1

  • 🟥 execution_time [+112.841ns; +193.759ns] or [+8.762%; +15.045%]

scenario:SamplingRuleMatchingBench/benchRegexMatching1-opcache

  • 🟥 execution_time [+154.722ns; +209.478ns] or [+11.162%; +15.113%]

scenario:SamplingRuleMatchingBench/benchRegexMatching2

  • 🟥 execution_time [+151.875ns; +208.725ns] or [+12.054%; +16.567%]

scenario:SamplingRuleMatchingBench/benchRegexMatching2-opcache

  • 🟥 execution_time [+148.227ns; +202.373ns] or [+10.790%; +14.731%]

scenario:SamplingRuleMatchingBench/benchRegexMatching3

  • 🟥 execution_time [+168.841ns; +209.959ns] or [+13.368%; +16.624%]

scenario:SamplingRuleMatchingBench/benchRegexMatching3-opcache

  • 🟥 execution_time [+150.548ns; +202.052ns] or [+10.778%; +14.465%]

scenario:SamplingRuleMatchingBench/benchRegexMatching4

  • 🟥 execution_time [+121.674ns; +172.326ns] or [+9.492%; +13.443%]

scenario:SamplingRuleMatchingBench/benchRegexMatching4-opcache

  • 🟥 execution_time [+10.110µs; +10.233µs] or [+735.567%; +744.462%]

scenario:SpanBench/benchOpenTelemetryAPI

  • 🟥 mem_peak [+4.495MB; +4.495MB] or [+10.399%; +10.399%]

scenario:SpanBench/benchOpenTelemetryAPI-opcache

  • 🟥 mem_peak [+4.493MB; +4.493MB] or [+11.174%; +11.174%]

scenario:SpanBench/benchOpenTelemetryInteroperability

  • 🟥 execution_time [+4.734µs; +6.676µs] or [+2.618%; +3.693%]

scenario:SpanBench/benchOpenTelemetryInteroperability-opcache

  • 🟥 execution_time [+4.856µs; +9.651µs] or [+2.852%; +5.668%]

In the hot sampling path, avoid re-allocating strings and updating
hash tables when _dd.p.ksr already contains the same value. This
optimization recovers most of the performance regression seen in
SamplingRuleMatchingBench when sampling is re-evaluated with the
same rate (the common case for long-running spans).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
zend_hash_str_del(metrics, ZEND_STRL("_dd.rule_psr"));
} else {
zend_hash_str_update(metrics, ZEND_STRL("_dd.rule_psr"), &sample_rate_zv);
dd_update_knuth_sampling_rate_tag(span, sample_rate);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it intentional that the _dd.p.ksr is not deleted where the psr is deleted?

#include <json/json.h>

#include "../configuration.h"
#include "../tracer_tag_propagation/tracer_tag_propagation.h"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant header inclusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI Generated Largely based on code generated by an AI or LLM. This label is the same across all dd-trace-* repos

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants