feat: send knuth sampling rates tags (#250)

* 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

@@ -784,8 +784,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 +794,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 +815,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 +836,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 +846,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 +857,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

@@ -310,6 +310,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 +325,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 +345,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 +392,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
@@ -2034,3 +2036,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);
+}