feat: send knuth sampling rates tags

Changes:
  - Set `_dd.p.ksr` tag with the knuth sampling rates computed.
  - Remove remaining devs from sampling delegation.
  - Add unit tests.

Author: dmehala

src/datadog/tags.cpp

@@ -29,10 +29,10 @@ const std::string trace_id_high = "_dd.p.tid";
 const std::string process_id = "process_id";
 const std::string language = "language";
 const std::string runtime_id = "runtime-id";
-const std::string sampling_decider = "_dd.is_sampling_decider";
 const std::string w3c_parent_id = "_dd.parent_id";
 const std::string trace_source = "_dd.p.ts";
 const std::string apm_enabled = "_dd.apm.enabled";
+const std::string ksr = "_dd.p.ksr";
 
 }  // namespace internal
 

src/datadog/tags.h

@@ -37,10 +37,10 @@ extern const std::string trace_id_high;
 extern const std::string process_id;
 extern const std::string language;
 extern const std::string runtime_id;
-extern const std::string sampling_decider;
 extern const std::string w3c_parent_id;
 extern const std::string trace_source;  // _dd.p.ts
 extern const std::string apm_enabled;   // _dd.apm.enabled
+extern const std::string ksr;           // _dd.p.ksr
 
 }  // namespace internal
 

src/datadog/trace_segment.cpp

@@ -11,6 +11,7 @@
 #include <datadog/trace_segment.h>
 
 #include <cassert>
+#include <charconv>
 #include <string>
 #include <unordered_map>
 #include <utility>
@@ -226,13 +227,6 @@ void TraceSegment::span_finished() {
     }
   }
 
-  if (decision.origin == SamplingDecision::Origin::DELEGATED &&
-      local_root.parent_id == 0) {
-    // Convey the fact that, even though we are the root service, we delegated
-    // the sampling decision and so are not the "sampling decider."
-    local_root.tags[tags::internal::sampling_decider] = "0";
-  }
-
   // RFC seems to only mandate that this be set if the trace is kept.
   // However, system-tests expect this to be always be set.
   // Add it all the time; can't hurt
@@ -292,6 +286,10 @@ void TraceSegment::make_sampling_decision_if_null() {
   sampling_decision_ = trace_sampler_->decide(local_root);
 
   update_decision_maker_trace_tag();
+
+  trace_tags_.emplace_back(
+      tags::internal::ksr,
+      std::to_string(*sampling_decision_->configured_rate));
 }
 
 void TraceSegment::update_decision_maker_trace_tag() {

test/test_span.cpp

@@ -499,7 +499,9 @@ TEST_SPAN("injection") {
     REQUIRE(writer.items.empty() == false);
 
     // Consume the span that MUST be kept for service liveness.
-    { apm_disabled_tracer.create_span(); }
+    {
+      apm_disabled_tracer.create_span();
+    }
 
     auto span = apm_disabled_tracer.create_span();
 
@@ -784,8 +786,8 @@ TEST_SPAN("injecting W3C tracestate header") {
            {"x-datadog-parent-id", "1"},
            {"x-datadog-origin", "France"},
        },
-       // The "s:-1" comes from the 0% sample rate.
-       "dd=s:-1;p:$parent_id;o:France"},
+       // The "s:-1" and "t.ksr:0.000000" comes from the 0% sample rate.
+       "dd=s:-1;p:$parent_id;o:France;t.ksr:0.000000"},
 
       {__LINE__,
        "trace tags",
@@ -794,8 +796,8 @@ TEST_SPAN("injecting W3C tracestate header") {
            {"x-datadog-parent-id", "1"},
            {"x-datadog-tags", "_dd.p.foo=x,_dd.p.bar=y,ignored=wrong_prefix"},
        },
-       // The "s:-1" comes from the 0% sample rate.
-       "dd=s:-1;p:$parent_id;t.foo:x;t.bar:y"},
+       // The "s:-1" and "t.ksr:0.000000"  comes from the 0% sample rate.
+       "dd=s:-1;p:$parent_id;t.foo:x;t.bar:y;t.ksr:0.000000"},
 
       {__LINE__,
        "extra fields",
@@ -815,15 +817,18 @@ TEST_SPAN("injecting W3C tracestate header") {
        // The "s:0" comes from the sampling decision in `traceparent_drop`.
        "dd=s:0;p:$parent_id;o:France;t.foo:x;t.bar:y;foo:bar;boing:boing"},
 
-      {__LINE__,
-       "replace invalid characters in origin",
-       {
-           {"x-datadog-trace-id", "1"},
-           {"x-datadog-parent-id", "1"},
-           {"x-datadog-origin", "France, is a country=nation; so is 台北."},
-       },
-       // The "s:-1" comes from the 0% sample rate.
-       "dd=s:-1;p:$parent_id;o:France_ is a country~nation_ so is ______."},
+      {
+          __LINE__,
+          "replace invalid characters in origin",
+          {
+              {"x-datadog-trace-id", "1"},
+              {"x-datadog-parent-id", "1"},
+              {"x-datadog-origin", "France, is a country=nation; so is 台北."},
+          },
+          // The "s:-1" comes from the 0% sample rate.
+          "dd=s:-1;p:$parent_id;o:France_ is a country~nation_ so is "
+          "______.;t.ksr:0.000000",
+      },
 
       {__LINE__,
        "replace invalid characters in trace tag key",
@@ -833,7 +838,7 @@ TEST_SPAN("injecting W3C tracestate header") {
            {"x-datadog-tags", "_dd.p.a;d台北x =foo,_dd.p.ok=bar"},
        },
        // The "s:-1" comes from the 0% sample rate.
-       "dd=s:-1;p:$parent_id;t.a_d______x_:foo;t.ok:bar"},
+       "dd=s:-1;p:$parent_id;t.a_d______x_:foo;t.ok:bar;t.ksr:0.000000"},
 
       {__LINE__,
        "replace invalid characters in trace tag value",
@@ -843,7 +848,8 @@ TEST_SPAN("injecting W3C tracestate header") {
            {"x-datadog-tags", "_dd.p.wacky=hello fr~d; how are คุณ?"},
        },
        // The "s:-1" comes from the 0% sample rate.
-       "dd=s:-1;p:$parent_id;t.wacky:hello fr_d_ how are _________?"},
+       "dd=s:-1;p:$parent_id;t.wacky:hello fr_d_ how are "
+       "_________?;t.ksr:0.000000"},
 
       {__LINE__,
        "replace equal signs with tildes in trace tag value",
@@ -853,7 +859,7 @@ TEST_SPAN("injecting W3C tracestate header") {
            {"x-datadog-tags", "_dd.p.base64_thingy=d2Fra2EhIHdhaw=="},
        },
        // The "s:-1" comes from the 0% sample rate.
-       "dd=s:-1;p:$parent_id;t.base64_thingy:d2Fra2EhIHdhaw~~"},
+       "dd=s:-1;p:$parent_id;t.base64_thingy:d2Fra2EhIHdhaw~~;t.ksr:0.000000"},
 
       {__LINE__,
        "oversized origin truncates it and subsequent fields",

test/test_trace_segment.cpp

@@ -236,7 +236,9 @@ TEST_CASE("TraceSegment finalization of spans") {
             {"x-datadog-sampling-priority", std::to_string(sampling_priority)},
         };
         MockDictReader reader{headers};
-        { auto span = tracer.extract_span(reader); }
+        {
+          auto span = tracer.extract_span(reader);
+        }
         REQUIRE(collector->span_count() == 1);
         REQUIRE(collector->first_span().numeric_tags.at(
                     tags::internal::sampling_priority) == sampling_priority);
@@ -310,6 +312,7 @@ TEST_CASE("TraceSegment finalization of spans") {
       REQUIRE_THAT(span.tags, ContainsSubset(filtered));
       // "_dd.p.dm" will be added, because we made a sampling decision.
       REQUIRE(span.tags.count("_dd.p.dm") == 1);
+      REQUIRE(span.tags.count("_dd.p.ksr") == 1);
     }
 
     SECTION("rate tags") {
@@ -324,6 +327,7 @@ TEST_CASE("TraceSegment finalization of spans") {
         REQUIRE(collector->span_count() == 1);
         const auto& span = collector->first_span();
         REQUIRE(span.numeric_tags.at(tags::internal::agent_sample_rate) == 1.0);
+        REQUIRE(span.tags.at(tags::internal::ksr) == "1.000000");
       }
 
       SECTION(
@@ -343,17 +347,24 @@ TEST_CASE("TraceSegment finalization of spans") {
           auto span = tracer.create_span();
           (void)span;
         }
-        REQUIRE(collector_response->span_count() == 1);
+        {
+          REQUIRE(collector_response->span_count() == 1);
+          const auto& span = collector_response->first_span();
+          CHECK(span.tags.at(tags::internal::ksr) == "1.000000");
+        }
 
         collector_response->chunks.clear();
         // Second trace will use the rate from `collector->response`.
         {
           auto span = tracer.create_span();
           (void)span;
         }
-        REQUIRE(collector_response->span_count() == 1);
-        const auto& span = collector_response->first_span();
-        REQUIRE(span.numeric_tags.at(tags::internal::agent_sample_rate) == 1.0);
+        {
+          REQUIRE(collector_response->span_count() == 1);
+          const auto& span = collector_response->first_span();
+          CHECK(span.numeric_tags.at(tags::internal::agent_sample_rate) == 1.0);
+          CHECK(span.tags.at(tags::internal::ksr) == "1.000000");
+        }
       }
 
       SECTION("rules (implicit and explicit)") {
@@ -383,6 +394,7 @@ TEST_CASE("TraceSegment finalization of spans") {
         const auto& span = collector->first_span();
         REQUIRE(span.numeric_tags.at(tags::internal::rule_sample_rate) ==
                 sample_rate);
+        CHECK(span.tags.at(tags::internal::ksr) == std::to_string(sample_rate));
         if (sample_rate == 1.0) {
           REQUIRE(span.numeric_tags.at(
                       tags::internal::rule_limiter_sample_rate) == 1.0);

test/test_tracer.cpp

@@ -995,11 +995,13 @@ TEST_TRACER("span extraction") {
             __LINE__,
             "origin, trace tags, parent, and extra fields",
             traceparent_drop,  // traceparent
-            "dd=o:France;p:00000000000d69ac;t.foo:thing1;t.bar:thing2;x:wow;y:"
+            "dd=o:France;p:00000000000d69ac;t.ksr:0.728;t.foo:thing1;t.bar:"
+            "thing2;x:wow;y:"
             "wow",     // tracestate
             0,         // expected_sampling_priority
             "France",  // expected_origin
             {
+                {"_dd.p.ksr", "0.728"},
                 {"_dd.p.foo", "thing1"},
                 {"_dd.p.bar", "thing2"},
             },                   // expected_trace_tags
@@ -1844,7 +1846,9 @@ TEST_TRACER("APM tracing disabled") {
       auto finalized_config = finalize_config(config, clock);
       REQUIRE(finalized_config);
       Tracer tracer{*finalized_config};
-      { auto root1 = tracer.create_span(); }
+      {
+        auto root1 = tracer.create_span();
+      }
       REQUIRE(collector->chunks.size() == 1);
       REQUIRE(collector->chunks.front().size() == 1);
 
@@ -1923,7 +1927,9 @@ TEST_TRACER("APM tracing disabled") {
 
     // When APM Tracing is disabled, we allow one trace per second for service
     // liveness. To ensure consistency, consume the limiter slot.
-    { tracer.create_span(); }
+    {
+      tracer.create_span();
+    }
     collector->chunks.clear();
 
     // Case 1: extracted context with priority, but no `_dd.p.ts` → depends if
@@ -2034,3 +2040,22 @@ TEST_TRACER("APM tracing disabled") {
     }
   }
 }
+
+TEST_TRACER("_dd.p.ksr is NOT set when overriding the sampling decision") {
+  const auto collector = std::make_shared<MockCollector>();
+
+  TracerConfig config;
+  config.collector = collector;
+  auto finalized_config = finalize_config(config);
+  REQUIRE(finalized_config);
+
+  Tracer tracer{*finalized_config};
+
+  {
+    auto span = tracer.create_span();
+    span.trace_segment().override_sampling_priority(10);
+  }
+
+  auto c = collector->first_span();
+  CHECK(c.tags.count(tags::internal::ksr) == 0);
+}