Skip to content

feat: ensure tracecontext headers take precedence over datadog (AIT-10281) #142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zacharycmontoya merged 7 commits into from
Aug 2, 2024
Merged

feat: ensure tracecontext headers take precedence over datadog (AIT-10281) #142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
921835e
Add a new test for the W3C Phase 3 Support: TEST_CASE("span extractio…
zacharycmontoya Jul 19, 2024
49e973f
Implement updates to the tracer to pass the new W3C Phase 3 unit test
zacharycmontoya Jul 19, 2024
9800c72
Fix test build issues for WSL 2 Ubuntu 24.04
zacharycmontoya Jul 24, 2024
1742ee9
Update datadog::tracing::merge to merge trace contexts using a map an…
zacharycmontoya Jul 24, 2024
3279017
Small refactor + format
zacharycmontoya Jul 24, 2024
55967d9
Apply suggestions from code review
zacharycmontoya Aug 2, 2024
5a17ca1
Remove TODO that got clarified
zacharycmontoya Aug 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 30 additions & 33 deletions src/datadog/extraction_util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <unordered_map>

#include "extracted_data.h"
#include "hex.h"
#include "logger.h"
#include "parse_util.h"
#include "string_util.h"
Expand Down Expand Up @@ -250,51 +251,47 @@ void AuditedReader::visit(
});
}

ExtractedData merge(const std::vector<ExtractedData>& contexts) {
ExtractedData merge(
const PropagationStyle first_style,
const std::unordered_map<PropagationStyle, ExtractedData>& contexts) {
ExtractedData result;

const auto found = std::find_if(
contexts.begin(), contexts.end(),
[](const ExtractedData& data) { return data.trace_id.has_value(); });

const auto found = contexts.find(first_style);
if (found == contexts.end()) {
// Nothing extracted a trace ID. Return the first context that includes a
// parent ID, if any, or otherwise just return an empty `ExtractedData`.
// The purpose of looking for a parent ID is to allow for the error
// "extracted a parent ID without a trace ID," if that's what happened.
const auto other = std::find_if(
contexts.begin(), contexts.end(),
[](const ExtractedData& data) { return data.parent_id.has_value(); });
if (other != contexts.end()) {
result = *other;
}
return result;
}

// `found` refers to the first extracted context that yielded a trace ID.
// This will be our main context.
//
// If the style of `found` is not W3C, then examine the remaining contexts
// for W3C-style tracestate that we might want to include in `result`.
result = *found;
if (result.style == PropagationStyle::W3C) {
return result;
}
// If the W3C style is present and its trace-id matches, we'll update the main
// context with tracestate information that we want to include in `result`. We
// may also need to use Datadog header information (only when the trace-id
// matches).
result = found->second;

const auto other =
std::find_if(found + 1, contexts.end(), [&](const ExtractedData& data) {
return data.style == PropagationStyle::W3C &&
data.trace_id == found->trace_id;
});
const auto w3c = contexts.find(PropagationStyle::W3C);
const auto dd = contexts.find(PropagationStyle::DATADOG);

if (other != contexts.end()) {
result.datadog_w3c_parent_id = other->datadog_w3c_parent_id;
result.additional_w3c_tracestate = other->additional_w3c_tracestate;
if (w3c != contexts.end() && w3c->second.trace_id == result.trace_id) {
result.additional_w3c_tracestate = w3c->second.additional_w3c_tracestate;
result.additional_datadog_w3c_tracestate =
other->additional_datadog_w3c_tracestate;
w3c->second.additional_datadog_w3c_tracestate;
result.headers_examined.insert(result.headers_examined.end(),
other->headers_examined.begin(),
other->headers_examined.end());
w3c->second.headers_examined.begin(),
w3c->second.headers_examined.end());

if (result.parent_id != w3c->second.parent_id) {
if (w3c->second.datadog_w3c_parent_id &&
w3c->second.datadog_w3c_parent_id != "0000000000000000") {
result.datadog_w3c_parent_id = w3c->second.datadog_w3c_parent_id;
} else if (dd != contexts.end() &&
dd->second.trace_id == result.trace_id &&
dd->second.parent_id.has_value()) {
result.datadog_w3c_parent_id = hex_padded(dd->second.parent_id.value());
}

result.parent_id = w3c->second.parent_id;
}
}

return result;
Expand Down
9 changes: 6 additions & 3 deletions src/datadog/extraction_util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,12 @@ struct AuditedReader : public DictReader {
// Combine the specified trace `contexts`, each of which was extracted in a
// particular propagation style, into one `ExtractedData` that includes fields
// from compatible elements of `contexts`, and return the resulting
// `ExtractedData`. The order of the elements of `contexts` must correspond to
// the order of the configured extraction propagation styles.
ExtractedData merge(const std::vector<ExtractedData>& contexts);
// `ExtractedData`. The `first_style` specifies the first configured extraction
// propagation style that has been extracted and the other contexts will be
// merged with it, so long as the trace-ids match.
ExtractedData merge(
const PropagationStyle first_style,
const std::unordered_map<PropagationStyle, ExtractedData>& contexts);

} // namespace tracing
} // namespace datadog
32 changes: 28 additions & 4 deletions src/datadog/tracer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,9 @@ Expected<Span> Tracer::extract_span(const DictReader& reader,
AuditedReader audited_reader{reader};

auto span_data = std::make_unique<SpanData>();
std::vector<ExtractedData> extracted_contexts;
Optional<PropagationStyle> first_style_with_trace_id;
Optional<PropagationStyle> first_style_with_parent_id;
std::unordered_map<PropagationStyle, ExtractedData> extracted_contexts;

for (const auto style : extraction_styles_) {
using Extractor = decltype(&extract_datadog); // function pointer
Expand All @@ -175,11 +177,33 @@ Expected<Span> Tracer::extract_span(const DictReader& reader,
return error->with_prefix(
extraction_error_prefix(style, audited_reader.entries_found));
}
extracted_contexts.push_back(std::move(*data));
extracted_contexts.back().headers_examined = audited_reader.entries_found;

if (!first_style_with_trace_id && data->trace_id.has_value()) {
first_style_with_trace_id = style;
}

if (!first_style_with_parent_id && data->parent_id.has_value()) {
first_style_with_parent_id = style;
}

data->headers_examined = audited_reader.entries_found;
extracted_contexts.emplace(style, std::move(*data));
}

auto merged_context = merge(extracted_contexts);
ExtractedData merged_context;
if (!first_style_with_trace_id) {
// Nothing extracted a trace ID. Return the first context that includes a
// parent ID, if any, or otherwise just return an empty `ExtractedData`.
// The purpose of looking for a parent ID is to allow for the error
// "extracted a parent ID without a trace ID," if that's what happened.
if (first_style_with_parent_id) {
auto other = extracted_contexts.find(*first_style_with_parent_id);
assert(other != extracted_contexts.end());
merged_context = other->second;
}
} else {
merged_context = merge(*first_style_with_trace_id, extracted_contexts);
}

// Some information might be missing.
// Here are the combinations considered:
Expand Down
8 changes: 4 additions & 4 deletions test/test_curl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ TEST_CASE("setopt failures", "[curl]") {
#define CASE(OPTION) \
{ OPTION, #OPTION }

const auto &[which_fails, name] = GENERATE(
auto test_case = GENERATE(
values<TestCase>({CASE(CURLOPT_ERRORBUFFER), CASE(CURLOPT_HEADERDATA),
CASE(CURLOPT_HEADERFUNCTION), CASE(CURLOPT_HTTPHEADER),
CASE(CURLOPT_POST), CASE(CURLOPT_POSTFIELDS),
Expand All @@ -369,17 +369,17 @@ TEST_CASE("setopt failures", "[curl]") {

#undef CASE

CAPTURE(name);
CAPTURE(test_case.name);
MockCurlLibrary library;
library.fail = which_fails;
library.fail = test_case.which_fails;

const auto clock = default_clock;
const auto logger = std::make_shared<NullLogger>();
const auto client = std::make_shared<Curl>(logger, clock, library);

const auto ignore = [](auto &&...) {};
HTTPClient::URL url;
if (which_fails == CURLOPT_UNIX_SOCKET_PATH) {
if (test_case.which_fails == CURLOPT_UNIX_SOCKET_PATH) {
url.scheme = "unix";
url.path = "/foo/bar.sock";
} else {
Expand Down
11 changes: 6 additions & 5 deletions test/test_span.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,13 +633,14 @@ TEST_CASE("injecting W3C traceparent header") {
int sampling_priority;
std::string expected_flags;
};
const auto& [sampling_priority, expected_flags] = GENERATE(
auto test_case = GENERATE(
values<TestCase>({{-1, "00"}, {0, "00"}, {1, "01"}, {2, "01"}}));

CAPTURE(sampling_priority);
CAPTURE(expected_flags);
CAPTURE(test_case.sampling_priority);
CAPTURE(test_case.expected_flags);

span.trace_segment().override_sampling_priority(sampling_priority);
span.trace_segment().override_sampling_priority(
test_case.sampling_priority);

MockDictWriter writer;
span.inject(writer);
Expand All @@ -649,7 +650,7 @@ TEST_CASE("injecting W3C traceparent header") {
// The "cafebabe"s come from `expected_id`.
const std::string expected =
"00-000000000000000000000000cafebabe-00000000cafebabe-" +
expected_flags;
test_case.expected_flags;
REQUIRE(found->second == expected);
}
}
Expand Down
125 changes: 125 additions & 0 deletions test/test_tracer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1109,6 +1109,131 @@ TEST_CASE("span extraction") {
REQUIRE(span_tags.empty());
}

SECTION("W3C Phase 3 support - Preferring tracecontext") {
// Tests behavior from system-test
// test_headers_tracecontext.py::test_tracestate_w3c_p_extract_datadog_w3c
struct TestCase {
int line;
std::string name;
std::string traceparent;
Optional<std::string> tracestate;
Optional<std::string> dd_trace_id;
Optional<std::string> dd_parent_id;
Optional<std::string> dd_tags;
Optional<std::uint64_t> expected_parent_id;
Optional<std::string> expected_datadog_w3c_parent_id = {};
};

auto test_case = GENERATE(values<TestCase>({
{
__LINE__, "identical trace info",
"00-11111111111111110000000000000001-000000003ade68b1-"
"01", // traceparent
"dd=s:2;p:000000003ade68b1,foo=1", // tracestate
"1", // x-datadog-trace-id
"987654321", // x-datadog-parent-id
"_dd.p.tid=1111111111111111", // x-datadog-tags
987654321, // expected_parent_id
nullopt, // expected_datadog_w3c_parent_id,
},

{
__LINE__, "trace ids do not match",
"00-11111111111111110000000000000002-000000003ade68b1-"
"01", // traceparent
"dd=s:2;p:000000000000000a,foo=1", // tracestate
"2", // x-datadog-trace-id
"10", // x-datadog-parent-id
"_dd.p.tid=2222222222222222", // x-datadog-tags
10, // expected_parent_id
nullopt, // expected_datadog_w3c_parent_id,
},

{
__LINE__, "same trace, non-matching parent ids",
"00-11111111111111110000000000000003-000000003ade68b1-"
"01", // traceparent
"dd=s:2;p:000000000000000a,foo=1", // tracestate
"3", // x-datadog-trace-id
"10", // x-datadog-parent-id
"_dd.p.tid=1111111111111111", // x-datadog-tags
987654321, // expected_parent_id
"000000000000000a", // expected_datadog_w3c_parent_id,
},

{
__LINE__, "non-matching span, missing p value",
"00-00000000000000000000000000000004-000000003ade68b1-"
"01", // traceparent
"dd=s:2,foo=1", // tracestate
"4", // x-datadog-trace-id
"10", // x-datadog-parent-id
nullopt, // x-datadog-tags
987654321, // expected_parent_id
"000000000000000a", // expected_datadog_w3c_parent_id,
},

{
__LINE__, "non-matching span, non-matching p value",
"00-00000000000000000000000000000005-000000003ade68b1-"
"01", // traceparent
"dd=s:2;p:8fffffffffffffff,foo=1", // tracestate
"5", // x-datadog-trace-id
"10", // x-datadog-parent-id
nullopt, // x-datadog-tags
987654321, // expected_parent_id
"8fffffffffffffff", // expected_datadog_w3c_parent_id,
},
}));

CAPTURE(test_case.name);
CAPTURE(test_case.line);
CAPTURE(test_case.traceparent);
CAPTURE(test_case.tracestate);
CAPTURE(test_case.dd_trace_id);
CAPTURE(test_case.dd_parent_id);
CAPTURE(test_case.dd_tags);

const auto collector = std::make_shared<MockCollector>();
const auto logger = std::make_shared<MockLogger>();

TracerConfig config;
config.collector = collector;
config.logger = logger;
config.service = "service1";
config.delegate_trace_sampling = false;
std::vector<PropagationStyle> extraction_styles{
PropagationStyle::DATADOG, PropagationStyle::B3, PropagationStyle::W3C};
config.extraction_styles = extraction_styles;

auto valid_config = finalize_config(config);
REQUIRE(valid_config);
Tracer tracer{*valid_config};

std::unordered_map<std::string, std::string> headers;
headers["traceparent"] = test_case.traceparent;
if (test_case.tracestate) {
headers["tracestate"] = *test_case.tracestate;
}
if (test_case.dd_trace_id) {
headers["x-datadog-trace-id"] = *test_case.dd_trace_id;
}
if (test_case.dd_parent_id) {
headers["x-datadog-parent-id"] = *test_case.dd_parent_id;
}
if (test_case.dd_tags) {
headers["x-datadog-tags"] = *test_case.dd_tags;
}
MockDictReader reader{headers};

const auto span = tracer.extract_span(reader);
REQUIRE(span);

REQUIRE(span->parent_id() == test_case.expected_parent_id);
REQUIRE(span->lookup_tag(tags::internal::w3c_parent_id) ==
test_case.expected_datadog_w3c_parent_id);
}

SECTION("_dd.parent_id") {
auto finalized_config = finalize_config(config);
REQUIRE(finalized_config);
Expand Down