fix: _dd.p.ksr formatting to use 6 significant digits without trailing zeros#288
fix: _dd.p.ksr formatting to use 6 significant digits without trailing zeros#288
_dd.p.ksr formatting to use 6 significant digits without trailing zeros#288Conversation
… zeros
Replace std::to_string() (which uses sprintf "%f" producing 6 trailing
decimal places) with snprintf "%.6g" which produces up to 6 significant
digits with no trailing zeros. This matches the behavior of Python's
f"{rate:.6g}" and Go's strconv.FormatFloat(rate, 'g', 6, 64).
Examples: 1.0 -> "1", 0.5 -> "0.5", 0.0 -> "0"
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
BenchmarksBenchmark execution time: 2026-03-12 15:33:47 Comparing candidate commit d9275b8 in PR branch Found 1 performance improvements and 0 performance regressions! Performance is the same for 0 metrics, 0 unstable metrics. scenario:BM_TraceTinyCCSource
|
Reformat the emplace_back call to match clang-format's expected style (arguments on one line with alignment) to fix the verify CI job. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
🎯 Code Coverage (details) 🔗 Commit SHA: d9275b8 | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #288 +/- ##
==========================================
- Coverage 87.76% 87.70% -0.07%
==========================================
Files 84 84
Lines 5658 5668 +10
==========================================
+ Hits 4966 4971 +5
- Misses 692 697 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The ksr trace tag should only be set when the sampling decision comes from an explicit source (agent rate, rule, or remote rule). When the DEFAULT mechanism is used — meaning no agent configuration has been received yet — the rate is a hardcoded 100% and ksr is meaningless. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Well done.
EDIT: I'm trying to enforce semantic commits, please prefix your commit message with fix: during the merge. Thanks.
_dd.p.ksr formatting to use 6 significant digits without trailing zeros
Co-authored-by: Damien Mehala <damien.mehala@datadoghq.com>
Fix two issues introduced by GitHub suggestion commits: 1. Correct the error condition: `ec == std::errc()` means success, not failure — changed to `ec != std::errc()` to properly detect errors. 2. Fix indentation from 4-space to 2-space to match project style and pass clang-format checks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
# 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>
### 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>
Description
Fix
_dd.p.ksrformatting to use 6 significant digits without trailing zeros, usingstd::to_charswithstd::chars_format::general. Also skip setting_dd.p.ksrwhen the sampling mechanism is DEFAULT (no agent configuration received yet).Changes:
snprintfwithstd::to_charsfor_dd.p.ksrformattingstd::chars_format::generalwith 6 significant digits (matches Go tracer behavior)_dd.p.ksrwhenSamplingMechanism::DEFAULTis activestd::to_charsconversion failuresMotivation
The previous
snprintfapproach with%gformat produced inconsistent output compared to other tracers.std::to_charswithstd::chars_format::generalgives us precise control over significant digits while automatically removing trailing zeros.Additionally, setting
_dd.p.ksrwith the DEFAULT mechanism is meaningless since no sampling configuration has been received from the agent yet.Additional Notes
Jira ticket: [PROJ-IDENT]
🤖 Generated with Claude Code