W3C propagation internal shared tests (#43)

* extracting a zero trace ID is an error

* change default propagation style to just Datadog

* disallow extra traceparent fields in version zero

* escape equal signs in "origin" field of W3C tracestate

* comment on = -> ~ in origin

Author: dgoffredo

src/datadog/error.h

@@ -69,6 +69,7 @@ struct Error {
     ENVOY_HTTP_CLIENT_FAILURE = 44,
     MULTIPLE_PROPAGATION_STYLE_ENVIRONMENT_VARIABLES = 45,
     DUPLICATE_PROPAGATION_STYLE = 46,
+    ZERO_TRACE_ID = 47,
   };
 
   Code code;

src/datadog/tracer.cpp

@@ -363,6 +363,7 @@ Expected<Span> Tracer::extract_span(const DictReader& reader,
   //           producing a root span
   //     - if origin is _not_ set, then it's an error
   // - trace ID and parent ID means we're extracting a child span
+  // - if trace ID is zero, then that's an error.
 
   if (!trace_id && !parent_id) {
     return Error{Error::NO_SPAN_TO_EXTRACT,
@@ -396,11 +397,16 @@ Expected<Span> Tracer::extract_span(const DictReader& reader,
     parent_id = 0;
   }
 
-  // We're done extracting fields.  Now create the span.
-  // This is similar to what we do in `create_span`.
   assert(parent_id);
   assert(trace_id);
 
+  if (*trace_id == 0) {
+    return Error{Error::ZERO_TRACE_ID,
+                 "extracted zero value for trace ID, which is invalid"};
+  }
+
+  // We're done extracting fields.  Now create the span.
+  // This is similar to what we do in `create_span`.
   span_data->apply_config(*defaults_, config, clock_);
   span_data->span_id = generator_->span_id();
   span_data->trace_id = *trace_id;

src/datadog/tracer_config.h

@@ -63,8 +63,7 @@ struct TracerConfig {
   // All styles indicated by `injection_styles` are used for injection.
   // `injection_styles` is overridden by the `DD_TRACE_PROPAGATION_STYLE_INJECT`
   // and `DD_TRACE_PROPAGATION_STYLE` environment variables.
-  std::vector<PropagationStyle> injection_styles = {PropagationStyle::W3C,
-                                                    PropagationStyle::DATADOG};
+  std::vector<PropagationStyle> injection_styles = {PropagationStyle::DATADOG};
 
   // `extraction_styles` indicates with which tracing systems trace propagation
   // will be compatible when extracting (receiving) trace context.
@@ -74,8 +73,7 @@ struct TracerConfig {
   // `extraction_styles` is overridden by the
   // `DD_TRACE_PROPAGATION_STYLE_EXTRACT` and `DD_TRACE_PROPAGATION_STYLE`
   // environment variables.
-  std::vector<PropagationStyle> extraction_styles = {PropagationStyle::W3C,
-                                                     PropagationStyle::DATADOG};
+  std::vector<PropagationStyle> extraction_styles = {PropagationStyle::DATADOG};
 
   // `report_hostname` indicates whether the tracer will include the result of
   // `gethostname` with traces sent to the collector.

src/datadog/w3c_propagation.cpp

@@ -53,7 +53,8 @@ Optional<std::string> extract_traceparent(ExtractedData& result,
       "([0-9a-f]{16})"  // hex parent span ID (match group 3)
       "-"
       "([0-9a-f]{2})"  // hex "trace-flags" (match group 4)
-      "(?:$|-.*)";     // either the end, or a hyphen preceding further fields
+      "($|-.*)";  // either the end, or a hyphen preceding further fields (match
+                  // group 5)
   static const std::regex regex{pattern};
 
   std::match_results<StringView::iterator> match;
@@ -62,18 +63,23 @@ Optional<std::string> extract_traceparent(ExtractedData& result,
   }
 
   assert(match.ready());
-  assert(match.size() == 4 + 1);
+  assert(match.size() == 5 + 1);
 
   const auto to_string_view = [](const auto& submatch) {
     assert(submatch.first <= submatch.second);
     return StringView{submatch.first,
                       std::size_t(submatch.second - submatch.first)};
   };
 
-  if (to_string_view(match[1]) == "ff") {
+  const auto version = to_string_view(match[1]);
+  if (version == "ff") {
     return "invalid_version";
   }
 
+  if (version == "00" && !to_string_view(match[5]).empty()) {
+    return "malformed_traceparent";
+  }
+
   result.trace_id = *TraceID::parse_hex(to_string_view(match[2]));
   if (result.trace_id == 0) {
     return "trace_id_zero";
@@ -190,6 +196,11 @@ void parse_datadog_tracestate(ExtractedData& result, StringView datadog_value) {
     const auto value = range(kv_separator + 1, pair_end);
     if (key == "o") {
       result.origin = std::string{value};
+      // Equal signs are allowed in the value of "origin," but equal signs are
+      // also special characters in the `tracestate` encoding. So, equal signs
+      // that would appear in the "origin" value are converted to tildes during
+      // encoding. Here, in decoding, we undo the conversion.
+      std::replace(result.origin->begin(), result.origin->end(), '~', '=');
     } else if (key == "s") {
       const auto maybe_priority = parse_int(value, 10);
       if (!maybe_priority) {
@@ -322,7 +333,8 @@ std::string encode_datadog_tracestate(
     result += ";o:";
     result += *origin;
     std::replace_if(result.end() - origin->size(), result.end(),
-                    verboten(0x20, 0x7e, ",;="), '_');
+                    verboten(0x20, 0x7e, ",;~"), '_');
+    std::replace(result.end() - origin->size(), result.end(), '=', '~');
   }
 
   for (const auto& [key, value] : trace_tags) {

test/test_span.cpp

@@ -628,7 +628,7 @@ TEST_CASE("injecting W3C tracestate header") {
      {{"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;o:France_ is a country_nation_ so is ______."},
+     "dd=s:-1;o:France_ is a country~nation_ so is ______."},
 
     {__LINE__, "replace invalid characters in trace tag key",
      {{"x-datadog-trace-id", "1"}, {"x-datadog-parent-id", "1"},

test/test_tracer.cpp

@@ -369,6 +369,18 @@ TEST_CASE("span extraction") {
           {"x-b3-spanid", "def"},
           {"x-b3-sampled", "99999999999999999999999999"}},
          Error::OUT_OF_RANGE_INTEGER},
+        {__LINE__,
+         "zero x-datadog-trace-id",
+         {PropagationStyle::DATADOG},
+         {{"x-datadog-trace-id", "0"},
+          {"x-datadog-parent-id", "1234"},
+          {"x-datadog-sampling-priority", "0"}},
+         Error::ZERO_TRACE_ID},
+        {__LINE__,
+         "zero x-b3-traceid",
+         {PropagationStyle::B3},
+         {{"x-b3-traceid", "0"}, {"x-b3-spanid", "123"}, {"x-b3-sampled", "0"}},
+         Error::ZERO_TRACE_ID},
     }));
 
     CAPTURE(test_case.line);
@@ -624,6 +636,10 @@ TEST_CASE("span extraction") {
         {__LINE__, "invalid: parent ID zero",
          "00-4bf92f3577b34da6a3ce929d0e0e4736-0000000000000000-00", // traceparent
          "parent_id_zero"}, // expected_error_tag_value
+
+        {__LINE__, "invalid: trailing characters when version is zero",
+         "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00-foo", // traceparent
+         "malformed_traceparent"}, // expected_error_tag_value
     }));
     // clang-format on
 
@@ -807,6 +823,12 @@ TEST_CASE("span extraction") {
           nullopt, // expected_additional_w3c_tracestate
           "x:wow;y:wow"}, // expected_additional_datadog_w3c_tracestate
 
+        {__LINE__, "origin with escaped equal sign",
+         traceparent_drop, // traceparent
+         "dd=o:France~country", // tracestate
+         0, // expected_sampling_priority
+         "France=country"}, // expected_origin
+
          {__LINE__, "traceparent and tracestate sampling agree (1/4)",
           traceparent_drop, // traceparent
           "dd=s:0", // tracestate

test/test_tracer_config.cpp

@@ -1003,12 +1003,12 @@ TEST_CASE("TracerConfig propagation styles") {
   TracerConfig config;
   config.defaults.service = "testsvc";
 
-  SECTION("default styles are [W3C, Datadog]") {
+  SECTION("default style is Datadog") {
     auto finalized = finalize_config(config);
     REQUIRE(finalized);
 
     const std::vector<PropagationStyle> expected_styles = {
-        PropagationStyle::W3C, PropagationStyle::DATADOG};
+        PropagationStyle::DATADOG};
 
     REQUIRE(finalized->injection_styles == expected_styles);
     REQUIRE(finalized->extraction_styles == expected_styles);