Skip to content

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

Draft
bm1549 wants to merge 3 commits intomainfrom
brian.marks/activate-ksr-generation-test
Draft

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

Conversation

@bm1549
Copy link
Contributor

@bm1549 bm1549 commented Mar 11, 2026

Motivation

Activate _dd.p.ksr (Knuth sample rate) system tests for all languages. These tests verify that tracers correctly set and propagate _dd.p.ksr in span meta when trace sampling rules are applied.

See RFC: "Transmit Knuth sampling rate to backend"

Changes

Commit 1: Support multiple [lang@branch] overrides in PR title

Allow multiple tracer branch overrides like [java@branch1][dotnet@branch2] in PR titles, so different libraries can each use their own dev branch when running system tests.

  • get_target_branch action now outputs a JSON map of library → branch (target-branch-map)
  • compute-workflow-parameters workflow extracts per-library branches using Python
  • All workflow references updated from target-branch to target-branch-map
  • Logic implemented in Python (shell: python) for clarity and robustness
  • Docs updated accordingly

Commit 2: Activate _dd.p.ksr generation tests for all languages

Added tests for the _dd.p.ksr 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 (merged from the removed test_sampling_knuth_sample_rate_propagated_from_upstream per reviewer feedback)
  • 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.

Workflow

  1. ⚠️ Create your PR as draft ⚠️
  2. Work on you PR until the CI passes
  3. Mark it as ready for review

Related PRs across tracers:

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?

🤖 Generated with Claude Code

@github-actions
Copy link
Contributor

github-actions bot commented Mar 11, 2026

CODEOWNERS have been resolved as:

.github/actions/get_target_branch/action.yaml                           @DataDog/system-tests-core
.github/workflows/ci.yml                                                @DataDog/system-tests-core
.github/workflows/compute-workflow-parameters.yml                       @DataDog/system-tests-core
.github/workflows/compute_libraries_and_scenarios.yml                   @DataDog/system-tests-core
.github/workflows/system-tests.yml                                      @DataDog/system-tests-core
docs/CI/github-actions.md                                               @DataDog/system-tests-core
docs/CI/system-tests-ci.md                                              @DataDog/system-tests-core
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/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

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

⚠️ Tests

Fix all issues with BitsAI or with Cursor

⚠️ Warnings

🧪 1 Test failed

tests.parametric.test_sampling_span_tags.Test_Knuth_Sample_Rate.test_sampling_knuth_sample_rate_trace_sampling_rule[library_env0, parametric-dotnet] from system_tests_suite (Datadog) (Fix with Cursor)
AssertionError: Expected _dd.p.ksr='1' for sampling rule rate 1.0, got: None
assert None == '1'
 +  where None = <built-in method get of dict object at 0x7f5e8ff57200>('_dd.p.ksr')
 +    where <built-in method get of dict object at 0x7f5e8ff57200> = {'_dd.p.dm': '-3', '_dd.p.tid': '69bd936900000000', 'language': 'dotnet', 'runtime-id': 'f096f455-ed1c-41e5-a692-da350c136042'}.get

self = <tests.parametric.test_sampling_span_tags.Test_Knuth_Sample_Rate object at 0x7f5ec4cca930>
test_agent = <utils.docker_fixtures._test_agent.TestAgentAPI object at 0x7f5e9048da90>
test_library = <utils.docker_fixtures._test_clients._test_client_parametric.ParametricTestClientApi object at 0x7f5e901d3e00>

    @pytest.mark.parametrize(
...

ℹ️ Info

No other issues found (see more)

❄️ No new flaky tests detected

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 64b5952 | 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 [java@brian.marks/add-ksr-tag][dotnet@brian.marks/add-ksr-tag][ruby@brian.marks/add-ksr-tag][nodejs@brian.marks/add-ksr-tag][cpp@brian.marks/fix-ksr-formatting][rust@brian.marks/add-ksr-tag] Mar 11, 2026
@bm1549 bm1549 changed the title Activate _dd.p.ksr generation test for all languages [java@brian.marks/add-ksr-tag][dotnet@brian.marks/add-ksr-tag][ruby@brian.marks/add-ksr-tag][nodejs@brian.marks/add-ksr-tag][cpp@brian.marks/fix-ksr-formatting][rust@brian.marks/add-ksr-tag] Activate _dd.p.ksr tests for all languages [java@brian.marks/add-ksr-tag][dotnet@brian.marks/add-ksr-tag][ruby@brian.marks/add-ksr-tag][nodejs@brian.marks/add-ksr-tag][cpp@brian.marks/fix-ksr-formatting][rust@brian.marks/add-ksr-tag][golang@brian.marks/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
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

}
],
)
def test_sampling_knuth_sample_rate_propagated_from_upstream(
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: This case is roughly equivalent to test_sampling_extract_knuth_sample_rate_distributed_tracing_datadog. We can update the existing test to set DD_TRACE_SAMPLING_RULES and use a ksr value of _dd.p.ksr and we should get the same test coverage.

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 <noreply@anthropic.com>
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) <noreply@anthropic.com>
Co-authored-by: Dario Castañé <dario.castane@datadoghq.com>
Co-authored-by: Mikayla Toffler <46911781+mtoffl01@users.noreply.github.com>
@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 [java@brian.marks/add-ksr-tag][dotnet@brian.marks/add-ksr-tag][ruby@brian.marks/add-ksr-tag][nodejs@brian.marks/add-ksr-tag][cpp@brian.marks/fix-ksr-formatting][rust@brian.marks/add-ksr-tag][golang@brian.marks/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 and others added 2 commits March 20, 2026 14:20
Previously, specifying a tracer branch override in the PR title (e.g.
[java@my-branch]) only supported a single override at a time. This
change allows multiple overrides like [java@branch1][dotnet@branch2]
so that different libraries can each use their own dev branch.

The get_target_branch action now outputs a JSON map of library to
branch (target-branch-map), and the compute-workflow-parameters
workflow extracts the correct branch for each library using Python
before passing it to load-binary.sh. Logic is implemented in Python
(shell: python) for clarity and robustness. All workflow references
updated from target-branch to target-branch-map. Docs updated accordingly.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
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) <noreply@anthropic.com>
@bm1549 bm1549 force-pushed the brian.marks/activate-ksr-generation-test branch from 8822171 to 612252e Compare March 20, 2026 18:21
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
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.

2 participants