Skip to content

Activate _dd.p.ksr tests for all languages#6466

Open
bm1549 wants to merge 13 commits intomainfrom
brian.marks/activate-ksr-generation-test
Open

Activate _dd.p.ksr tests for all languages#6466
bm1549 wants to merge 13 commits intomainfrom
brian.marks/activate-ksr-generation-test

Conversation

@bm1549
Copy link
Copy Markdown
Contributor

@bm1549 bm1549 commented Mar 11, 2026

Motivation

Activate the Test_Knuth_Sample_Rate (_dd.p.ksr) parametric tests across all tracer languages. Previously, most languages had these tests partially activated via the "easy win" script with individual sub-test exclusions. This PR cleans up those manifest entries and updates the tests themselves to be more thorough and correct.

Changes

Manifest updates (all 9 languages)

  • C++: Removed per-sub-test missing_feature exclusion, bumped version gate from >=2.0.0 to >=2.1.0. Cleaned up "easy win activation script" comments.
  • .NET: Removed per-sub-test missing_feature exclusion, bumped version gate from >=3.36.0 to >=3.41.0. Cleaned up "easy win activation script" comments.
  • Go: Removed per-sub-test missing_feature exclusion, bumped version gate from >=2.5.0 to >=2.8.0. Cleaned up "easy win activation script" comments.
  • Java: Removed per-sub-test missing_feature exclusion, bumped version gate from >=1.58.2+... to >=1.61.0. Cleaned up "easy win activation script" comments.
  • Node.js: Changed from missing_feature to version-gated (>=5.93.0).
  • PHP: Changed from missing_feature to version-gated (>=1.18.0).
  • Python: Bumped version gate from v3.14.0.dev to v4.7.0.
  • Ruby: Removed per-sub-test missing_feature exclusion, bumped version gate from >=2.27.0 to >=2.31.0. Cleaned up "easy win activation script" comments.
  • Rust: Replaced per-sub-test missing_feature exclusion with a class-level version gate (>=0.4.0).

Test changes (test_sampling_span_tags.py)

  • test_sampling_knuth_sample_rate_trace_sampling_rule: Simplified from a 3-trace/3-flush pattern to a single span, using find_only_span. Added precision boundary test cases (6-digit precision cutoff, rounding to zero, rounding up with 0.00000051). Renamed parameter from sample_rate to expected_ksr for clarity.
  • test_sampling_extract_knuth_sample_rate_distributed_tracing_datadog: Added a second parametrized case that verifies upstream _dd.p.ksr is propagated unchanged even when local sampling rules are configured.
  • test_sampling_knuth_sample_rate_not_set_for_default (new): Verifies that when no sampling rules or agent rates are configured, _dd.p.ksr is either absent or "1".

Workflow

  1. ⚠️ Create your PR as draft ⚠️
  2. Work on you PR until the CI passes
  3. Mark it as ready for review
    • Test logic is modified? -> Get a review from RFC owner.
    • Framework is modified, or non obvious usage of it -> get a review from R&P team

🚀 Once your PR is reviewed and the CI green, you can merge it!

🛟 #apm-shared-testing 🛟

Reviewer checklist

  • Anything but tests/ or manifests/ is modified ? I have the approval from R&P team
  • A docker base image is modified?
    • the relevant build-XXX-image label is present
  • A scenario is added, removed or renamed?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 11, 2026

CODEOWNERS have been resolved as:

manifests/cpp.yml                                                       @DataDog/dd-trace-cpp
manifests/dotnet.yml                                                    @DataDog/apm-dotnet @DataDog/asm-dotnet
manifests/golang.yml                                                    @DataDog/dd-trace-go-guild
manifests/java.yml                                                      @DataDog/asm-java @DataDog/apm-java
manifests/nodejs.yml                                                    @DataDog/dd-trace-js
manifests/php.yml                                                       @DataDog/apm-php @DataDog/asm-php
manifests/python.yml                                                    @DataDog/apm-python @DataDog/asm-python
manifests/ruby.yml                                                      @DataDog/ruby-guild @DataDog/asm-ruby
manifests/rust.yml                                                      @DataDog/apm-rust
tests/parametric/test_sampling_span_tags.py                             @DataDog/system-tests-core @DataDog/apm-sdk-capabilities

@datadog-prod-us1-6
Copy link
Copy Markdown

datadog-prod-us1-6 bot commented Mar 11, 2026

⚠️ Tests

Fix all issues with BitsAI or with Cursor

⚠️ Warnings

🧪 3 Tests failed

tests.parametric.test_sampling_span_tags.Test_Knuth_Sample_Rate.test_sampling_knuth_sample_rate_trace_sampling_rule[below_precision_rounds_to_zero, parametric-golang] from system_tests_suite   View in Datadog   (Fix with Cursor)
ValueError: Number (1) of traces not available from test agent, got 0:
[]

self = <tests.parametric.test_sampling_span_tags.Test_Knuth_Sample_Rate object at 0x7f6c68d16420>
test_agent = <utils.docker_fixtures._test_agent.TestAgentAPI object at 0x7f6c36a942f0>
test_library = <utils.docker_fixtures._test_clients._test_client_parametric.ParametricTestClientApi object at 0x7f6c37267c20>
expected_ksr = '0'

    @pytest.mark.parametrize(
        ("library_env", "expected_ksr"),
...
tests.parametric.test_sampling_span_tags.Test_Knuth_Sample_Rate.test_sampling_knuth_sample_rate_trace_sampling_rule[rounds_up_to_one_millionth, parametric-golang] from system_tests_suite   View in Datadog   (Fix with Cursor)
ValueError: Number (1) of traces not available from test agent, got 0:
[]

self = <tests.parametric.test_sampling_span_tags.Test_Knuth_Sample_Rate object at 0x7f6c68d16540>
test_agent = <utils.docker_fixtures._test_agent.TestAgentAPI object at 0x7f6c372bc650>
test_library = <utils.docker_fixtures._test_clients._test_client_parametric.ParametricTestClientApi object at 0x7f6c372bdb50>
expected_ksr = '0.000001'

    @pytest.mark.parametrize(
        ("library_env", "expected_ksr"),
...
tests.parametric.test_sampling_span_tags.Test_Knuth_Sample_Rate.test_sampling_knuth_sample_rate_trace_sampling_rule[six_decimal_precision_boundary, parametric-golang] from system_tests_suite   View in Datadog   (Fix with Cursor)
ValueError: Number (1) of traces not available from test agent, got 0:
[]

self = <tests.parametric.test_sampling_span_tags.Test_Knuth_Sample_Rate object at 0x7f6c68d16360>
test_agent = <utils.docker_fixtures._test_agent.TestAgentAPI object at 0x7f6c372bf290>
test_library = <utils.docker_fixtures._test_clients._test_client_parametric.ParametricTestClientApi object at 0x7f6c3725d160>
expected_ksr = '0.000001'

    @pytest.mark.parametrize(
        ("library_env", "expected_ksr"),
...

ℹ️ Info

No other issues found (see more)

❄️ No new flaky tests detected

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 57d4551 | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback!

@bm1549 bm1549 changed the title Activate _dd.p.ksr generation test for all languages Activate _dd.p.ksr generation test for all languages [[email protected]/add-ksr-tag][[email protected]/add-ksr-tag][[email protected]/add-ksr-tag][[email protected]/add-ksr-tag][[email protected]/fix-ksr-formatting][[email protected]/add-ksr-tag] Mar 11, 2026
@bm1549 bm1549 changed the title Activate _dd.p.ksr generation test for all languages [[email protected]/add-ksr-tag][[email protected]/add-ksr-tag][[email protected]/add-ksr-tag][[email protected]/add-ksr-tag][[email protected]/fix-ksr-formatting][[email protected]/add-ksr-tag] Activate _dd.p.ksr tests for all languages [[email protected]/add-ksr-tag][[email protected]/add-ksr-tag][[email protected]/add-ksr-tag][[email protected]/add-ksr-tag][[email protected]/fix-ksr-formatting][[email protected]/add-ksr-tag][[email protected]/fix-ksr-default] Mar 11, 2026
@bm1549 bm1549 force-pushed the brian.marks/activate-ksr-generation-test branch from edd74ed to e0c7ff5 Compare March 11, 2026 03:04
@bm1549 bm1549 changed the base branch from main to brian.marks/multi-branch-override March 11, 2026 03:04
@bm1549 bm1549 force-pushed the brian.marks/activate-ksr-generation-test branch from e0c7ff5 to 87b1d29 Compare March 11, 2026 03:25
Copy link
Copy Markdown
Contributor

@mabdinur mabdinur left a comment

Choose a reason for hiding this comment

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

We need to address the dotnet and cpp failures but aside from that the changes look good to me

bm1549 added a commit to DataDog/dd-trace-rs that referenced this pull request Mar 19, 2026
# What does this PR do?

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.

`format_sampling_rate` now returns `Option<String>` and guards against
invalid inputs (negative, >1.0, NaN, infinity), returning `None` instead
of producing garbage output.

# Motivation

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"

# Additional Notes

Key files changed:
- `datadog-opentelemetry/src/core/constants.rs` — Added
`SAMPLING_KNUTH_RATE_TAG_KEY` constant
- `datadog-opentelemetry/src/sampling/datadog_sampler.rs` — Added
`format_sampling_rate()` helper (returns `Option<String>`, defensive
against invalid rates) and set ksr in agent/rule sampling paths
- Updated 2 snapshot JSON files

Related PRs across tracers:
- Java: DataDog/dd-trace-java#10802
- .NET: DataDog/dd-trace-dotnet#8287
- Ruby: DataDog/dd-trace-rb#5436
- Node.js: DataDog/dd-trace-js#7741
- PHP: DataDog/dd-trace-php#3701
- C++: DataDog/dd-trace-cpp#288
- System tests: DataDog/system-tests#6466

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
bm1549 added a commit to DataDog/dd-trace-go that referenced this pull request Mar 19, 2026
### What does this PR do?

Fixes `_dd.p.ksr` (Knuth Sampling Rate) to only be set on spans when the
agent has provided sampling rates via `readRatesJSON()`. Previously, ksr
was unconditionally set in `prioritySampler.apply()`, including when the
rate was the initial client-side default (1.0) before any agent response
arrived.

Also refactors `prioritySampler` to consolidate lock acquisitions:
extracts `getRateLocked()` so `apply()` acquires `ps.mu.RLock` only once
to read both the rate and `agentRatesLoaded`.

### Motivation

Cross-language consistency: Python, Java, PHP, and other tracers only
set ksr when actual agent rates or sampling rules are applied, not for
the default fallback. This aligns Go with that behavior.

See RFC: "Transmit Knuth sampling rate to backend"

### Additional Notes

- Added `agentRatesLoaded` bool field to `prioritySampler`, set to
`true` in `readRatesJSON()`
- `apply()` now gates ksr behind `agentRatesLoaded` check
- Extracted `getRateLocked()` to avoid double lock acquisition in
`apply()`
- Rule-based sampling path (`applyTraceRuleSampling` in span.go)
unchanged — correctly always sets ksr
- Tests added: `ksr-not-set-without-agent-rates` and
`ksr-set-after-agent-rates-received`

Related PRs across tracers:
- Java: DataDog/dd-trace-java#10802
- .NET: DataDog/dd-trace-dotnet#8287
- Ruby: DataDog/dd-trace-rb#5436
- Node.js: DataDog/dd-trace-js#7741
- PHP: DataDog/dd-trace-php#3701
- Rust: DataDog/dd-trace-rs#180
- C++: DataDog/dd-trace-cpp#288
- System tests: DataDog/system-tests#6466

### Reviewer's Checklist

- [x] Changed code has unit tests for its functionality at or near 100%
coverage.
- [x] [System-Tests](https://github.com/DataDog/system-tests/) covering
this feature have been added and enabled with the va.b.c-dev version
tag.
- [ ] There is a benchmark for any new code, or changes to existing
code.
- [x] If this interacts with the agent in a new way, a system test has
been added.
- [x] New code is free of linting errors. You can check this by running
`make lint` locally.
- [x] New code doesn't break existing tests. You can check this by
running `make test` locally.
- [ ] Add an appropriate team label so this PR gets put in the right
place for the release notes.
- [ ] All generated files are up to date. You can check this by running
`make generate` locally.
- [ ] Non-trivial go.mod changes, e.g. adding new modules, are reviewed
by @DataDog/dd-trace-go-guild. Make sure all nested modules are up to
date by running `make fix-modules` locally.

Unsure? Have a question? Request a review!

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
Co-authored-by: Dario Castañé <[email protected]>
Co-authored-by: Mikayla Toffler <[email protected]>
@bm1549 bm1549 force-pushed the brian.marks/activate-ksr-generation-test branch from b2e25c1 to 8822171 Compare March 19, 2026 21:28
@bm1549 bm1549 changed the title Activate _dd.p.ksr tests for all languages [[email protected]/add-ksr-tag][[email protected]/add-ksr-tag][[email protected]/add-ksr-tag][[email protected]/add-ksr-tag][[email protected]/fix-ksr-formatting][[email protected]/add-ksr-tag][[email protected]/fix-ksr-default] Activate _dd.p.ksr tests for all languages Mar 20, 2026
@bm1549 bm1549 changed the base branch from brian.marks/multi-branch-override to main March 20, 2026 18:05
@bm1549 bm1549 force-pushed the brian.marks/activate-ksr-generation-test branch from 8822171 to 612252e Compare March 20, 2026 18:21
bm1549 and others added 2 commits March 23, 2026 19:42
Add tests for the _dd.p.ksr (Knuth sample rate) tag:
- test_sampling_knuth_sample_rate_trace_sampling_rule: verifies ksr is
  set to "1" when a sampling rule with rate 1.0 is configured
- test_sampling_extract_knuth_sample_rate_distributed_tracing_datadog:
  verifies upstream ksr is extracted from Datadog headers and propagated
  unchanged, including when local sampling rules are also configured
- test_sampling_extract_knuth_sample_rate_distributed_tracing_tracecontext:
  same for tracecontext (W3C) propagation style
- test_sampling_knuth_sample_rate_not_set_for_default: verifies ksr is
  absent or "1" under default (no explicit rules) sampling

Manifests updated with version gates for all languages (feature not yet
released): Java >=1.61.0, Python v3.14.0.dev, Node.js >=5.90.0,
Go >=2.9.0, Ruby >=2.30.0, C++ >=2.1.0, .NET >=3.40.0, Rust >=0.1.0.
PHP manifest uses per-test entries since branch overrides are unsupported.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <[email protected]>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <[email protected]>
@bm1549 bm1549 force-pushed the brian.marks/activate-ksr-generation-test branch from 64b5952 to e75c182 Compare March 23, 2026 23:43
bm1549 and others added 3 commits March 23, 2026 19:49
Test KSR tag formatting at precision boundaries:
- 0.000001 -> "0.000001" (exactly at 6-digit precision)
- 0.0000001 -> "0" (below precision, rounds to zero)
- 0.0000005 -> "0.000001" (rounds up to nearest representable value)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…odejs, python, ruby

Bug: APMAPI-1869

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Remove per-sub-test missing_feature exclusions for PHP and add a
class-level version gate since branch override support ships in 1.17.0.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@bm1549 bm1549 marked this pull request as ready for review March 27, 2026 19:40
@bm1549 bm1549 requested review from a team as code owners March 27, 2026 19:40
@bm1549 bm1549 requested review from P403n1x87, cataphract, claponcet, daniel-romano-DD, dougqh and rachelyangdog and removed request for a team March 27, 2026 19:40
@bm1549
Copy link
Copy Markdown
Contributor Author

bm1549 commented Mar 27, 2026

The golang dev test is currently failing, but the changes have been merged to main in DataDog/dd-trace-go#4603

I expect that test will start passing when the dev artifact updates

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants