From 375aa167afa40e3dc81afe5eb354e871a56c3446 Mon Sep 17 00:00:00 2001 From: "datadog-prod-us1-3[bot]" <266080212+datadog-prod-us1-3[bot]@users.noreply.github.com> Date: Tue, 25 Aug 2026 16:08:29 +0000 Subject: [PATCH] APMS-20291 Fix W3C tracestate parsing Co-authored-by: bm1549 <12128670+bm1549@users.noreply.github.com> --- src/datadog/w3c_propagation.cpp | 68 +++++++++++++++++---- test/test_span.cpp | 18 ++++++ test/test_tracer.cpp | 104 ++++++++++++++++++++++++++++++++ 3 files changed, 177 insertions(+), 13 deletions(-) diff --git a/src/datadog/w3c_propagation.cpp b/src/datadog/w3c_propagation.cpp index acba657f..15073c18 100644 --- a/src/datadog/w3c_propagation.cpp +++ b/src/datadog/w3c_propagation.cpp @@ -131,6 +131,18 @@ struct PartiallyParsedTracestate { std::string other_entries; }; +void append_tracestate_member(std::string& destination, StringView member) { + member = trim(member); + if (member.empty()) { + return; + } + + if (!destination.empty()) { + destination += ','; + } + append(destination, member); +} + // Return the separate Datadog-specific and non-Datadog-specific portions of the // specified `tracestate`. If `tracestate` does not have a Datadog-specific // portion, return `nullopt`. @@ -138,6 +150,7 @@ Optional parse_tracestate(StringView tracestate) { const std::size_t begin = 0; const std::size_t end = tracestate.size(); std::size_t pair_begin = begin; + std::string other_entries; while (pair_begin < end) { const std::size_t pair_end = tracestate.find(',', pair_begin); // Note that since this `pair` is `strip`ped, `pair_begin` is not @@ -154,6 +167,7 @@ Optional parse_tracestate(StringView tracestate) { // This is an invalid entry because it contains a non-whitespace character // but not a "=". // Let's move on to the next entry. + append_tracestate_member(other_entries, pair); pair_begin = (pair_end == StringView::npos) ? end : pair_end + 1; continue; } @@ -161,25 +175,27 @@ Optional parse_tracestate(StringView tracestate) { const auto key = pair.substr(0, kv_separator); if (key != "dd") { // On to the next. + append_tracestate_member(other_entries, pair); pair_begin = (pair_end == StringView::npos) ? end : pair_end + 1; continue; } PartiallyParsedTracestate result; result.datadog_value = pair.substr(kv_separator + 1); - // `result->other_entries` is whatever was before the "dd" entry and - // whatever is after the "dd" entry, but without an extra comma in the - // middle. - if (pair_begin != 0) { - // There's a prefix - append(result.other_entries, tracestate.substr(0, pair_begin - 1)); - if (pair_end != StringView::npos && pair_end + 1 < end) { - // and a suffix - append(result.other_entries, tracestate.substr(pair_end)); - } - } else if (pair_end != StringView::npos && pair_end + 1 < end) { - // There's just a suffix - append(result.other_entries, tracestate.substr(pair_end + 1)); + result.other_entries = std::move(other_entries); + + std::size_t remaining_pair_begin = + (pair_end == StringView::npos) ? end : pair_end + 1; + while (remaining_pair_begin < end) { + const std::size_t remaining_pair_end = + tracestate.find(',', remaining_pair_begin); + append_tracestate_member( + result.other_entries, + tracestate.substr(remaining_pair_begin, + remaining_pair_end - remaining_pair_begin)); + remaining_pair_begin = (remaining_pair_end == StringView::npos) + ? end + : remaining_pair_end + 1; } return result; @@ -187,6 +203,28 @@ Optional parse_tracestate(StringView tracestate) { return nullopt; } + +bool is_valid_datadog_tracestate(StringView datadog_value) { + const std::size_t end = datadog_value.size(); + std::size_t pair_begin = 0; + while (pair_begin < end) { + const std::size_t pair_end = datadog_value.find(';', pair_begin); + const auto pair = datadog_value.substr(pair_begin, pair_end - pair_begin); + pair_begin = (pair_end == StringView::npos) ? end : pair_end + 1; + + const auto trimmed = trim(pair); + if (trimmed.empty()) { + continue; + } + + if (trimmed.data() != pair.data() || trimmed.size() != pair.size()) { + return false; + } + } + + return true; +} + // Fill the specified `result` with information parsed from the specified // `datadog_value`. `datadog_value` is the value of the "dd" entry in the // "tracestate" header. @@ -310,6 +348,10 @@ void extract_tracestate( return; } + if (!is_valid_datadog_tracestate(datadog_value)) { + return; + } + parse_datadog_tracestate(result, datadog_value); } diff --git a/test/test_span.cpp b/test/test_span.cpp index 6608e60b..b0be3188 100644 --- a/test/test_span.cpp +++ b/test/test_span.cpp @@ -813,6 +813,24 @@ TEST_SPAN("injecting W3C tracestate header") { // The "s:0" comes from the sampling decision in `traceparent_drop`. "dd=s:0;p:$parent_id;foo:bar;boing:boing"}, + {__LINE__, + "trim outer tracestate OWS", + { + {"traceparent", + "00-00000000000000000000000000000001-0000000000000001-01"}, + {"tracestate", "foo=1 , dd=s:2;o:some , bar=2"}, + }, + "dd=s:2;p:$parent_id;o:some,foo=1,bar=2"}, + + {__LINE__, + "skip dd entry with interior OWS", + { + {"traceparent", + "00-00000000000000000000000000000001-0000000000000001-01"}, + {"tracestate", "foo=1,dd=s:0;t.dm:934086a686-4; t.x:y"}, + }, + "dd=s:1;p:$parent_id,foo=1"}, + {__LINE__, "all of the above", { diff --git a/test/test_tracer.cpp b/test/test_tracer.cpp index 55f8d15c..331b2575 100644 --- a/test/test_tracer.cpp +++ b/test/test_tracer.cpp @@ -997,6 +997,110 @@ TEST_TRACER("span extraction") { "0000000000000000", // expected_datadog_w3c_parent_id, }, + { + __LINE__, + "dd entry with trailing semicolon", + traceparent_keep, // traceparent + "foo=1,dd=s:2;o:some;", // tracestate + 2, // expected_sampling_priority + "some", // expected_origin + {}, // expected_trace_tags + "foo=1", // expected_additional_w3c_tracestate + nullopt, // expected_additional_datadog_w3c_tracestate + "0000000000000000", // expected_datadog_w3c_parent_id, + }, + + { + __LINE__, + "dd entry with trailing semicolon and OWS", + traceparent_keep, // traceparent + "foo=1,dd=s:2;o:some; \t", // tracestate + 2, // expected_sampling_priority + "some", // expected_origin + {}, // expected_trace_tags + "foo=1", // expected_additional_w3c_tracestate + nullopt, // expected_additional_datadog_w3c_tracestate + "0000000000000000", // expected_datadog_w3c_parent_id, + }, + + { + __LINE__, + "dd entry with double semicolon", + traceparent_keep, // traceparent + "foo=1,dd=s:2;;o:some", // tracestate + 2, // expected_sampling_priority + "some", // expected_origin + {}, // expected_trace_tags + "foo=1", // expected_additional_w3c_tracestate + nullopt, // expected_additional_datadog_w3c_tracestate + "0000000000000000", // expected_datadog_w3c_parent_id, + }, + + { + __LINE__, + "dd entry with leading semicolon", + traceparent_keep, // traceparent + "foo=1,dd=;s:2;o:some", // tracestate + 2, // expected_sampling_priority + "some", // expected_origin + {}, // expected_trace_tags + "foo=1", // expected_additional_w3c_tracestate + nullopt, // expected_additional_datadog_w3c_tracestate + "0000000000000000", // expected_datadog_w3c_parent_id, + }, + + { + __LINE__, + "dd entry with interior OWS", + traceparent_keep, // traceparent + "foo=1,dd=s:0;t.dm:934086a686-4; t.x:y", // tracestate + 1, // expected_sampling_priority + nullopt, // expected_origin + {}, // expected_trace_tags + "foo=1", // expected_additional_w3c_tracestate + nullopt, // expected_additional_datadog_w3c_tracestate + "0000000000000000", // expected_datadog_w3c_parent_id, + }, + + { + __LINE__, + "dd entry with OWS after first subentry", + traceparent_keep, // traceparent + "foo=1,dd=s:0; t.dm:934086a686-4", // tracestate + 1, // expected_sampling_priority + nullopt, // expected_origin + {}, // expected_trace_tags + "foo=1", // expected_additional_w3c_tracestate + nullopt, // expected_additional_datadog_w3c_tracestate + "0000000000000000", // expected_datadog_w3c_parent_id, + }, + + { + __LINE__, + "outer empty list member before dd", + traceparent_keep, // traceparent + "foo=1,,dd=s:2;o:some", // tracestate + 2, // expected_sampling_priority + "some", // expected_origin + {}, // expected_trace_tags + "foo=1", // expected_additional_w3c_tracestate + nullopt, // expected_additional_datadog_w3c_tracestate + "0000000000000000", // expected_datadog_w3c_parent_id, + }, + + { + __LINE__, + "outer OWS around list members", + traceparent_keep, // traceparent + "foo=1 , dd=s:2;o:some , bar=2", // tracestate + 2, // expected_sampling_priority + "some", // expected_origin + {}, // expected_trace_tags + "foo=1,bar=2", // expected_additional_w3c_tracestate + nullopt, // expected_additional_datadog_w3c_tracestate + "0000000000000000", // expected_datadog_w3c_parent_id, + }, + { __LINE__, "origin, trace tags, parent, and extra fields",