Skip to content

Commit 1572290

Browse files
bm1549claudedmehala
authored
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 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> * Fix clang-format violation in trace_segment.cpp 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> * Do not set _dd.p.ksr when using the DEFAULT sampling mechanism 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> * Replace snprintf with std::to_chars for _dd.p.ksr formatting Address PR feedback: remove format_rate function and use std::to_chars with std::chars_format::general inline instead of snprintf. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Update src/datadog/trace_segment.cpp Co-authored-by: Damien Mehala <damien.mehala@datadoghq.com> * Apply suggestion from @bm1549 * Fix clang-format violation and logic bug in ksr error handling 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> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Damien Mehala <damien.mehala@datadoghq.com>
1 parent f8c3913 commit 1572290

File tree

3 files changed

+41
-16
lines changed

3 files changed

+41
-16
lines changed

src/datadog/trace_segment.cpp

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <datadog/telemetry/telemetry.h>
1212
#include <datadog/trace_segment.h>
1313

14+
#include <array>
1415
#include <cassert>
1516
#include <charconv>
1617
#include <string>
@@ -321,9 +322,23 @@ void TraceSegment::make_sampling_decision_if_null() {
321322

322323
update_decision_maker_trace_tag();
323324

324-
trace_tags_.emplace_back(
325-
tags::internal::ksr,
326-
std::to_string(*sampling_decision_->configured_rate));
325+
// Only set ksr when the sampling mechanism is explicit (agent rate, rule, or
326+
// remote rule). The DEFAULT mechanism means we haven't received any
327+
// configuration from the agent yet, so ksr would be meaningless.
328+
if (sampling_decision_->mechanism &&
329+
*sampling_decision_->mechanism != int(SamplingMechanism::DEFAULT)) {
330+
std::array<char, 8> buf;
331+
const auto [ptr, ec] = std::to_chars(buf.data(), buf.data() + buf.size(),
332+
*sampling_decision_->configured_rate,
333+
std::chars_format::general, 6);
334+
if (ec != std::errc()) {
335+
std::string error{"string conversion failed: "};
336+
error += std::make_error_code(ec).message();
337+
logger_->log_error(error);
338+
return;
339+
}
340+
trace_tags_.emplace_back(tags::internal::ksr, std::string(buf.data(), ptr));
341+
}
327342
}
328343

329344
void TraceSegment::update_decision_maker_trace_tag() {

test/test_span.cpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -784,8 +784,8 @@ TEST_SPAN("injecting W3C tracestate header") {
784784
{"x-datadog-parent-id", "1"},
785785
{"x-datadog-origin", "France"},
786786
},
787-
// The "s:-1" and "t.ksr:0.000000" comes from the 0% sample rate.
788-
"dd=s:-1;p:$parent_id;o:France;t.ksr:0.000000"},
787+
// The "s:-1" and "t.ksr:0" comes from the 0% sample rate.
788+
"dd=s:-1;p:$parent_id;o:France;t.ksr:0"},
789789

790790
{__LINE__,
791791
"trace tags",
@@ -794,8 +794,8 @@ TEST_SPAN("injecting W3C tracestate header") {
794794
{"x-datadog-parent-id", "1"},
795795
{"x-datadog-tags", "_dd.p.foo=x,_dd.p.bar=y,ignored=wrong_prefix"},
796796
},
797-
// The "s:-1" and "t.ksr:0.000000" comes from the 0% sample rate.
798-
"dd=s:-1;p:$parent_id;t.foo:x;t.bar:y;t.ksr:0.000000"},
797+
// The "s:-1" and "t.ksr:0" comes from the 0% sample rate.
798+
"dd=s:-1;p:$parent_id;t.foo:x;t.bar:y;t.ksr:0"},
799799

800800
{__LINE__,
801801
"extra fields",
@@ -825,7 +825,7 @@ TEST_SPAN("injecting W3C tracestate header") {
825825
},
826826
// The "s:-1" comes from the 0% sample rate.
827827
"dd=s:-1;p:$parent_id;o:France_ is a country~nation_ so is "
828-
"______.;t.ksr:0.000000",
828+
"______.;t.ksr:0",
829829
},
830830

831831
{__LINE__,
@@ -836,7 +836,7 @@ TEST_SPAN("injecting W3C tracestate header") {
836836
{"x-datadog-tags", "_dd.p.a;d台北x =foo,_dd.p.ok=bar"},
837837
},
838838
// The "s:-1" comes from the 0% sample rate.
839-
"dd=s:-1;p:$parent_id;t.a_d______x_:foo;t.ok:bar;t.ksr:0.000000"},
839+
"dd=s:-1;p:$parent_id;t.a_d______x_:foo;t.ok:bar;t.ksr:0"},
840840

841841
{__LINE__,
842842
"replace invalid characters in trace tag value",
@@ -847,7 +847,7 @@ TEST_SPAN("injecting W3C tracestate header") {
847847
},
848848
// The "s:-1" comes from the 0% sample rate.
849849
"dd=s:-1;p:$parent_id;t.wacky:hello fr_d_ how are "
850-
"_________?;t.ksr:0.000000"},
850+
"_________?;t.ksr:0"},
851851

852852
{__LINE__,
853853
"replace equal signs with tildes in trace tag value",
@@ -857,7 +857,7 @@ TEST_SPAN("injecting W3C tracestate header") {
857857
{"x-datadog-tags", "_dd.p.base64_thingy=d2Fra2EhIHdhaw=="},
858858
},
859859
// The "s:-1" comes from the 0% sample rate.
860-
"dd=s:-1;p:$parent_id;t.base64_thingy:d2Fra2EhIHdhaw~~;t.ksr:0.000000"},
860+
"dd=s:-1;p:$parent_id;t.base64_thingy:d2Fra2EhIHdhaw~~;t.ksr:0"},
861861

862862
{__LINE__,
863863
"oversized origin truncates it and subsequent fields",

test/test_trace_segment.cpp

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@
77
#include <datadog/tracer.h>
88
#include <datadog/tracer_config.h>
99

10+
#include <cstdio>
1011
#include <regex>
12+
#include <string>
1113
#include <vector>
1214

1315
#include "matchers.h"
@@ -310,7 +312,9 @@ TEST_CASE("TraceSegment finalization of spans") {
310312
REQUIRE_THAT(span.tags, ContainsSubset(filtered));
311313
// "_dd.p.dm" will be added, because we made a sampling decision.
312314
REQUIRE(span.tags.count("_dd.p.dm") == 1);
313-
REQUIRE(span.tags.count("_dd.p.ksr") == 1);
315+
// "_dd.p.ksr" is NOT set because this uses the DEFAULT mechanism (no
316+
// agent configuration received yet).
317+
REQUIRE(span.tags.count("_dd.p.ksr") == 0);
314318
}
315319

316320
SECTION("rate tags") {
@@ -325,7 +329,8 @@ TEST_CASE("TraceSegment finalization of spans") {
325329
REQUIRE(collector->span_count() == 1);
326330
const auto& span = collector->first_span();
327331
REQUIRE(span.numeric_tags.at(tags::internal::agent_sample_rate) == 1.0);
328-
REQUIRE(span.tags.at(tags::internal::ksr) == "1.000000");
332+
// ksr is NOT set for the DEFAULT mechanism.
333+
REQUIRE(span.tags.count(tags::internal::ksr) == 0);
329334
}
330335

331336
SECTION(
@@ -348,7 +353,8 @@ TEST_CASE("TraceSegment finalization of spans") {
348353
{
349354
REQUIRE(collector_response->span_count() == 1);
350355
const auto& span = collector_response->first_span();
351-
CHECK(span.tags.at(tags::internal::ksr) == "1.000000");
356+
// ksr is NOT set for the DEFAULT mechanism.
357+
CHECK(span.tags.count(tags::internal::ksr) == 0);
352358
}
353359

354360
collector_response->chunks.clear();
@@ -361,7 +367,7 @@ TEST_CASE("TraceSegment finalization of spans") {
361367
REQUIRE(collector_response->span_count() == 1);
362368
const auto& span = collector_response->first_span();
363369
CHECK(span.numeric_tags.at(tags::internal::agent_sample_rate) == 1.0);
364-
CHECK(span.tags.at(tags::internal::ksr) == "1.000000");
370+
CHECK(span.tags.at(tags::internal::ksr) == "1");
365371
}
366372
}
367373

@@ -392,7 +398,11 @@ TEST_CASE("TraceSegment finalization of spans") {
392398
const auto& span = collector->first_span();
393399
REQUIRE(span.numeric_tags.at(tags::internal::rule_sample_rate) ==
394400
sample_rate);
395-
CHECK(span.tags.at(tags::internal::ksr) == std::to_string(sample_rate));
401+
{
402+
char buf[32];
403+
std::snprintf(buf, sizeof(buf), "%.6g", sample_rate);
404+
CHECK(span.tags.at(tags::internal::ksr) == std::string(buf));
405+
}
396406
if (sample_rate == 1.0) {
397407
REQUIRE(span.numeric_tags.at(
398408
tags::internal::rule_limiter_sample_rate) == 1.0);

0 commit comments

Comments
 (0)