diff --git a/src/datadog/w3c_propagation.cpp b/src/datadog/w3c_propagation.cpp index 54a78faf..37ad755c 100644 --- a/src/datadog/w3c_propagation.cpp +++ b/src/datadog/w3c_propagation.cpp @@ -222,6 +222,25 @@ Optional extract_traceparent(ExtractedData& result, return nullopt; } +bool is_valid_datadog_trace_state(StringView datadog_trace_state) { + const std::size_t end = datadog_trace_state.size(); + std::size_t item_begin = 0; + while (item_begin < end) { + const std::size_t item_end = datadog_trace_state.find(';', item_begin); + const StringView item = + datadog_trace_state.substr(item_begin, item_end - item_begin); + item_begin = item_end == StringView::npos ? end : item_end + 1; + + const StringView trimmed_item = trim(item); + if (trimmed_item.data() != item.data() || + trimmed_item.size() != item.size()) { + return false; + } + } + + return true; +} + // Fill the specified `result` with information parsed from the specified // `datadog_trace_state`. `datadog_trace_state` is the value of the "dd" entry // in the W3C "tracestate" header. @@ -326,16 +345,18 @@ void parse_w3c_tracestate_member( ExtractedData& result, StringView member, std::unordered_map& span_tags, std::string& other_w3c_tracestate, - std::size_t& other_w3c_tracestate_member_count) { + std::size_t& other_w3c_tracestate_member_count, + bool& has_datadog_trace_state) { const std::size_t separator = member.find('='); const StringView key = member.substr(0, separator); const StringView member_value = separator == StringView::npos ? StringView{} : member.substr(separator + 1); if (key == "dd") { + has_datadog_trace_state = true; if (member_value.size() > max_datadog_tracestate_value_size) { span_tags[tags::internal::propagation_error] = "extract_max_size"; - } else { + } else if (is_valid_datadog_trace_state(member_value)) { parse_datadog_trace_state(result, member_value); } } else if (key == "ot") { @@ -367,12 +388,29 @@ void parse_w3c_tracestate( std::unordered_map& span_tags) { std::string other_w3c_tracestate; std::size_t other_w3c_tracestate_member_count = 0; + bool has_datadog_trace_state = false; for_each_w3c_tracestate_member(w3c_tracestate, [&](StringView member) { parse_w3c_tracestate_member(result, member, span_tags, other_w3c_tracestate, - other_w3c_tracestate_member_count); + other_w3c_tracestate_member_count, + has_datadog_trace_state); return true; }); + if (has_datadog_trace_state) { + std::string canonical_other_w3c_tracestate; + for_each_w3c_tracestate_member( + other_w3c_tracestate, [&](StringView member) { + if (!member.empty()) { + if (!canonical_other_w3c_tracestate.empty()) { + canonical_other_w3c_tracestate += ','; + } + append(canonical_other_w3c_tracestate, member); + } + return true; + }); + other_w3c_tracestate = std::move(canonical_other_w3c_tracestate); + } + if (!other_w3c_tracestate.empty()) { result.additional_w3c_tracestate = std::move(other_w3c_tracestate); } diff --git a/test/test_span.cpp b/test/test_span.cpp index 6b6f0d40..8f8ae3cc 100644 --- a/test/test_span.cpp +++ b/test/test_span.cpp @@ -815,6 +815,33 @@ 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__, + "skip dd entry with whitespace-only subentry", + { + {"traceparent", + "00-00000000000000000000000000000001-0000000000000001-01"}, + {"tracestate", "foo=1,dd=s:0; ;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 4ab69071..bda5404e 100644 --- a/test/test_tracer.cpp +++ b/test/test_tracer.cpp @@ -997,6 +997,149 @@ 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__, + "dd entry with whitespace-only subentry", + traceparent_keep, // traceparent + "foo=1,dd=s:0; ;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 whitespace-only first subentry", + traceparent_keep, // traceparent + "foo=1,dd= ;s:2;o:some", // 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 empty list member after dd", + traceparent_keep, // traceparent + "dd=s:2;o:some,,foo=1", // 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",