From 8c59b37941fa9fe6d9b728287873785cbdc21c3a Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 15 Nov 2024 17:03:45 -0500 Subject: [PATCH 01/17] testing(parametric): update start span parametric endpoint --- test/system-tests/main.cpp | 4 ++ test/system-tests/request_handler.cpp | 69 +++++++++++++++++---------- test/system-tests/request_handler.h | 1 + 3 files changed, 48 insertions(+), 26 deletions(-) diff --git a/test/system-tests/main.cpp b/test/system-tests/main.cpp index 395affa2..59bb2e08 100644 --- a/test/system-tests/main.cpp +++ b/test/system-tests/main.cpp @@ -108,6 +108,10 @@ int main(int argc, char* argv[]) { [&handler](const httplib::Request& req, httplib::Response& res) { handler.on_inject_headers(req, res); }); + svr.Post("/trace/span/extract_headers", + [&handler](const httplib::Request& req, httplib::Response& res) { + handler.on_extract_headers(req, res); + }); svr.Post("/trace/span/flush", [&handler](const httplib::Request& req, httplib::Response& res) { handler.on_span_flush(req, res); diff --git a/test/system-tests/request_handler.cpp b/test/system-tests/request_handler.cpp index 08801b77..d1555570 100644 --- a/test/system-tests/request_handler.cpp +++ b/test/system-tests/request_handler.cpp @@ -116,8 +116,8 @@ void RequestHandler::on_span_start(const httplib::Request& req, if (auto parent_id = utils::get_if_exists(request_json, "parent_id")) { if (*parent_id != 0) { - auto parent_span_it = active_spans_.find(*parent_id); - if (parent_span_it == active_spans_.cend()) { + auto parent_span_it = spans_.find(*parent_id); + if (parent_span_it == spans_.cend()) { const auto msg = "on_span_start: span not found for id " + std::to_string(*parent_id); VALIDATION_ERROR(res, msg); @@ -125,25 +125,14 @@ void RequestHandler::on_span_start(const httplib::Request& req, auto span = parent_span_it->second.create_child(span_cfg); success(span, res); - active_spans_.emplace(span.id(), std::move(span)); - return; - } - } - - if (auto http_headers = utils::get_if_exists( - request_json, "http_headers")) { - if (!http_headers->empty()) { - auto span = tracer_.extract_or_create_span( - utils::HeaderReader(*http_headers), span_cfg); - success(span, res); - active_spans_.emplace(span.id(), std::move(span)); + spans_.emplace(span.id(), std::move(span)); return; } } auto span = tracer_.create_span(span_cfg); success(span, res); - active_spans_.emplace(span.id(), std::move(span)); + spans_.emplace(span.id(), std::move(span)); } void RequestHandler::on_span_end(const httplib::Request& req, @@ -155,14 +144,13 @@ void RequestHandler::on_span_end(const httplib::Request& req, VALIDATION_ERROR(res, "on_span_end: missing `span_id` field."); } - auto span_it = active_spans_.find(*span_id); - if (span_it == active_spans_.cend()) { + auto span_it = spans_.find(*span_id); + if (span_it == spans_.cend()) { const auto msg = "on_span_end: span not found for id " + std::to_string(*span_id); VALIDATION_ERROR(res, msg); } - active_spans_.erase(span_it); res.status = 200; } @@ -175,8 +163,8 @@ void RequestHandler::on_set_meta(const httplib::Request& req, VALIDATION_ERROR(res, "on_set_meta: missing `span_id` field."); } - auto span_it = active_spans_.find(*span_id); - if (span_it == active_spans_.cend()) { + auto span_it = spans_.find(*span_id); + if (span_it == spans_.cend()) { const auto msg = "on_set_meta: span not found for id " + std::to_string(*span_id); VALIDATION_ERROR(res, msg); @@ -198,8 +186,8 @@ void RequestHandler::on_set_metric(const httplib::Request& /* req */, VALIDATION_ERROR(res, "on_set_meta: missing `span_id` field."); } - auto span_it = active_spans_.find(*span_id); - if (span_it == active_spans_.cend()) { + auto span_it = spans_.find(*span_id); + if (span_it == spans_.cend()) { const auto msg = "on_set_meta: span not found for id " + std::to_string(*span_id); VALIDATION_ERROR(res, msg); @@ -221,8 +209,8 @@ void RequestHandler::on_inject_headers(const httplib::Request& req, VALIDATION_ERROR(res, "on_inject_headers: missing `span_id` field."); } - auto span_it = active_spans_.find(*span_id); - if (span_it == active_spans_.cend()) { + auto span_it = spans_.find(*span_id); + if (span_it == spans_.cend()) { const auto msg = "on_inject_headers: span not found for id " + std::to_string(*span_id); VALIDATION_ERROR(res, msg); @@ -240,9 +228,38 @@ void RequestHandler::on_inject_headers(const httplib::Request& req, res.set_content(response_json.dump(), "application/json"); } + +void RequestHandler::on_extract_headers(const httplib::Request& req, + httplib::Response& res) { + const auto request_json = nlohmann::json::parse(req.body); + auto http_headers = utils::get_if_exists(request_json, "http_headers"); + if (!http_headers) { + VALIDATION_ERROR(res, "on_extract_headers: missing `http_headers` field."); + } + + auto span = tracer_.extract_span( + utils::HeaderReader(*http_headers), span_cfg); + + auto success = [](const datadog::tracing::Span& span, + httplib::Response& res) { + // clang-format off + const auto response_body = nlohmann::json{ + { "span_id", span.id() } + }; + // clang-format on + + res.set_content(response_body.dump(), "application/json"); + }; + + success(span, res); + spans_.emplace(span.id(), std::move(span)); +} + + void RequestHandler::on_span_flush(const httplib::Request& /* req */, httplib::Response& res) { scheduler_->flush_telemetry(); + spans_.clear(); res.status = 200; } @@ -261,8 +278,8 @@ void RequestHandler::on_span_error(const httplib::Request& req, VALIDATION_ERROR(res, "on_span_error: missing `span_id` field."); } - auto span_it = active_spans_.find(*span_id); - if (span_it == active_spans_.cend()) { + auto span_it = spans_.find(*span_id); + if (span_it == spans_.cend()) { const auto msg = "on_span_error: span not found for id " + std::to_string(*span_id); VALIDATION_ERROR(res, msg); diff --git a/test/system-tests/request_handler.h b/test/system-tests/request_handler.h index a1f93b0a..1b96c4d3 100644 --- a/test/system-tests/request_handler.h +++ b/test/system-tests/request_handler.h @@ -25,6 +25,7 @@ class RequestHandler final { void on_set_meta(const httplib::Request& req, httplib::Response& res); void on_set_metric(const httplib::Request& /* req */, httplib::Response& res); void on_inject_headers(const httplib::Request& req, httplib::Response& res); + void on_extract_headers(const httplib::Request& req, httplib::Response& res); void on_span_flush(const httplib::Request& /* req */, httplib::Response& res); void on_stats_flush(const httplib::Request& /* req */, httplib::Response& res); From 4d7f38c7edb9627513a734a24c158a41d2d81532 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 15 Nov 2024 17:21:32 -0500 Subject: [PATCH 02/17] remove setting ddorgin, this is not up to spec --- test/system-tests/request_handler.cpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/test/system-tests/request_handler.cpp b/test/system-tests/request_handler.cpp index d1555570..b81dad53 100644 --- a/test/system-tests/request_handler.cpp +++ b/test/system-tests/request_handler.cpp @@ -94,13 +94,6 @@ void RequestHandler::on_span_start(const httplib::Request& req, span_cfg.resource = *resource; } - if (auto origin = - utils::get_if_exists(request_json, "origin")) { - logger_->log_info( - "[start_span] origin, but this can only be set via the " - "'x-datadog-origin' header"); - } - auto success = [](const datadog::tracing::Span& span, httplib::Response& res) { // clang-format off From b35ee1cc9a4ed2e9ec8d1ea20c5eba610c5212c5 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 22 Nov 2024 15:01:01 -0500 Subject: [PATCH 03/17] add spans_ --- test/system-tests/request_handler.cpp | 1 + test/system-tests/request_handler.h | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/test/system-tests/request_handler.cpp b/test/system-tests/request_handler.cpp index b81dad53..b0fd6d10 100644 --- a/test/system-tests/request_handler.cpp +++ b/test/system-tests/request_handler.cpp @@ -230,6 +230,7 @@ void RequestHandler::on_extract_headers(const httplib::Request& req, VALIDATION_ERROR(res, "on_extract_headers: missing `http_headers` field."); } + datadog::tracing::SpanConfig span_cfg; auto span = tracer_.extract_span( utils::HeaderReader(*http_headers), span_cfg); diff --git a/test/system-tests/request_handler.h b/test/system-tests/request_handler.h index 1b96c4d3..ee73c6f3 100644 --- a/test/system-tests/request_handler.h +++ b/test/system-tests/request_handler.h @@ -35,7 +35,7 @@ class RequestHandler final { datadog::tracing::Tracer tracer_; std::shared_ptr scheduler_; std::shared_ptr logger_; - std::unordered_map active_spans_; + std::unordered_map spans_; #undef VALIDATION_ERROR }; From 55ec27d5f959e912bf603845f7275f7419bb4c11 Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Mon, 2 Dec 2024 15:10:14 +0100 Subject: [PATCH 04/17] fix compilation errors --- test/system-tests/request_handler.cpp | 29 +++++++++++---------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/test/system-tests/request_handler.cpp b/test/system-tests/request_handler.cpp index b0fd6d10..67e662ef 100644 --- a/test/system-tests/request_handler.cpp +++ b/test/system-tests/request_handler.cpp @@ -221,35 +221,30 @@ void RequestHandler::on_inject_headers(const httplib::Request& req, res.set_content(response_json.dump(), "application/json"); } - void RequestHandler::on_extract_headers(const httplib::Request& req, - httplib::Response& res) { + httplib::Response& res) { const auto request_json = nlohmann::json::parse(req.body); - auto http_headers = utils::get_if_exists(request_json, "http_headers"); + auto http_headers = utils::get_if_exists( + request_json, "http_headers"); if (!http_headers) { VALIDATION_ERROR(res, "on_extract_headers: missing `http_headers` field."); } datadog::tracing::SpanConfig span_cfg; - auto span = tracer_.extract_span( - utils::HeaderReader(*http_headers), span_cfg); - - auto success = [](const datadog::tracing::Span& span, - httplib::Response& res) { - // clang-format off - const auto response_body = nlohmann::json{ - { "span_id", span.id() } - }; - // clang-format on + auto span = + tracer_.extract_span(utils::HeaderReader(*http_headers), span_cfg); + if (auto error = span.if_error()) { + VALIDATION_ERROR(res, error->with_prefix("on_extract_headers: ").message); + } - res.set_content(response_body.dump(), "application/json"); + const auto response_body = nlohmann::json{ + {"span_id", span->id()}, }; - success(span, res); - spans_.emplace(span.id(), std::move(span)); + res.set_content(response_body.dump(), "application/json"); + spans_.emplace(span->id(), std::move(*span)); } - void RequestHandler::on_span_flush(const httplib::Request& /* req */, httplib::Response& res) { scheduler_->flush_telemetry(); From 1484a485dcec85a6b9b041eb04f110d004d51a27 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Mon, 23 Dec 2024 15:49:45 -0500 Subject: [PATCH 05/17] move extract headers to its own method, this is broken --- include/datadog/tracer.h | 7 +++++++ src/datadog/tracer.cpp | 31 +++++++++++++++++++++---------- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/include/datadog/tracer.h b/include/datadog/tracer.h index 1e3e6650..659f1d3f 100644 --- a/include/datadog/tracer.h +++ b/include/datadog/tracer.h @@ -18,6 +18,7 @@ #include "id_generator.h" #include "optional.h" #include "span.h" +#include "extraction_util.h" #include "tracer_config.h" #include "tracer_signature.h" @@ -61,6 +62,9 @@ class Tracer { Span create_span(); Span create_span(const SpanConfig& config); + // ... + Expected extract_headers(const DictReader& reader, std::unique_ptr span_data); + // Return a span whose parent and other context is parsed from the specified // `reader`, and whose attributes are determined by the optionally specified // `config`. If there is no tracing information in `reader`, then return an @@ -69,6 +73,9 @@ class Tracer { Expected extract_span(const DictReader& reader); Expected extract_span(const DictReader& reader, const SpanConfig& config); + Expected extract_span(ExtractedData& merged_context, + const SpanConfig& config, + std::unique_ptr span_data); // Return a span extracted from the specified `reader` (see `extract_span`). // If there is no span to extract, then return a span that is the root of a diff --git a/src/datadog/tracer.cpp b/src/datadog/tracer.cpp index 321ce308..4a8029df 100644 --- a/src/datadog/tracer.cpp +++ b/src/datadog/tracer.cpp @@ -16,7 +16,6 @@ #include "config_manager.h" #include "datadog_agent.h" #include "extracted_data.h" -#include "extraction_util.h" #include "hex.h" #include "json.hpp" #include "platform_util.h" @@ -143,17 +142,9 @@ Span Tracer::create_span(const SpanConfig& config) { return span; } -Expected Tracer::extract_span(const DictReader& reader) { - return extract_span(reader, SpanConfig{}); -} - -Expected Tracer::extract_span(const DictReader& reader, - const SpanConfig& config) { +Expected Tracer::extract_headers(const DictReader& reader, std::unique_ptr span_data) { assert(!extraction_styles_.empty()); - AuditedReader audited_reader{reader}; - - auto span_data = std::make_unique(); Optional first_style_with_trace_id; Optional first_style_with_parent_id; std::unordered_map extracted_contexts; @@ -272,6 +263,26 @@ Expected Tracer::extract_span(const DictReader& reader, merged_context.headers_examined)); } + return std::move(merged_context); +} + +Expected Tracer::extract_span(const DictReader& reader) { + return extract_span(reader, SpanConfig{}); +} + +Expected Tracer::extract_span(const DictReader& reader, + const SpanConfig& config) { + auto span_data = std::make_unique(); + auto merged_context = extract_headers(reader, std::move(span_data)); + if (auto* error = merged_context.if_error()) { + return *error; + } + return extract_span(*merged_context, config, std::move(span_data)); +} + +Expected Tracer::extract_span(ExtractedData& merged_context, + const SpanConfig& config, + std::unique_ptr span_data) { // We're done extracting fields. Now create the span. // This is similar to what we do in `create_span`. span_data->apply_config(*config_manager_->span_defaults(), config, clock_); From c13f7283f328126b28f518df4d25ee0d8e83dee1 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 3 Jan 2025 14:31:58 -0500 Subject: [PATCH 06/17] Revert "move extract headers to its own method, this is broken" This reverts commit 1484a485dcec85a6b9b041eb04f110d004d51a27. --- include/datadog/tracer.h | 7 ------- src/datadog/tracer.cpp | 31 ++++++++++--------------------- 2 files changed, 10 insertions(+), 28 deletions(-) diff --git a/include/datadog/tracer.h b/include/datadog/tracer.h index 659f1d3f..1e3e6650 100644 --- a/include/datadog/tracer.h +++ b/include/datadog/tracer.h @@ -18,7 +18,6 @@ #include "id_generator.h" #include "optional.h" #include "span.h" -#include "extraction_util.h" #include "tracer_config.h" #include "tracer_signature.h" @@ -62,9 +61,6 @@ class Tracer { Span create_span(); Span create_span(const SpanConfig& config); - // ... - Expected extract_headers(const DictReader& reader, std::unique_ptr span_data); - // Return a span whose parent and other context is parsed from the specified // `reader`, and whose attributes are determined by the optionally specified // `config`. If there is no tracing information in `reader`, then return an @@ -73,9 +69,6 @@ class Tracer { Expected extract_span(const DictReader& reader); Expected extract_span(const DictReader& reader, const SpanConfig& config); - Expected extract_span(ExtractedData& merged_context, - const SpanConfig& config, - std::unique_ptr span_data); // Return a span extracted from the specified `reader` (see `extract_span`). // If there is no span to extract, then return a span that is the root of a diff --git a/src/datadog/tracer.cpp b/src/datadog/tracer.cpp index 4a8029df..321ce308 100644 --- a/src/datadog/tracer.cpp +++ b/src/datadog/tracer.cpp @@ -16,6 +16,7 @@ #include "config_manager.h" #include "datadog_agent.h" #include "extracted_data.h" +#include "extraction_util.h" #include "hex.h" #include "json.hpp" #include "platform_util.h" @@ -142,9 +143,17 @@ Span Tracer::create_span(const SpanConfig& config) { return span; } -Expected Tracer::extract_headers(const DictReader& reader, std::unique_ptr span_data) { +Expected Tracer::extract_span(const DictReader& reader) { + return extract_span(reader, SpanConfig{}); +} + +Expected Tracer::extract_span(const DictReader& reader, + const SpanConfig& config) { assert(!extraction_styles_.empty()); + AuditedReader audited_reader{reader}; + + auto span_data = std::make_unique(); Optional first_style_with_trace_id; Optional first_style_with_parent_id; std::unordered_map extracted_contexts; @@ -263,26 +272,6 @@ Expected Tracer::extract_headers(const DictReader& reader, std::u merged_context.headers_examined)); } - return std::move(merged_context); -} - -Expected Tracer::extract_span(const DictReader& reader) { - return extract_span(reader, SpanConfig{}); -} - -Expected Tracer::extract_span(const DictReader& reader, - const SpanConfig& config) { - auto span_data = std::make_unique(); - auto merged_context = extract_headers(reader, std::move(span_data)); - if (auto* error = merged_context.if_error()) { - return *error; - } - return extract_span(*merged_context, config, std::move(span_data)); -} - -Expected Tracer::extract_span(ExtractedData& merged_context, - const SpanConfig& config, - std::unique_ptr span_data) { // We're done extracting fields. Now create the span. // This is similar to what we do in `create_span`. span_data->apply_config(*config_manager_->span_defaults(), config, clock_); From e2e3e76ef347648f8a51b1039fe0bb465d0c84d3 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 3 Jan 2025 22:05:51 -0500 Subject: [PATCH 07/17] best effort to extract headers in one endpoint and create a local root span in another [probably won't work tho] --- test/system-tests/request_handler.cpp | 35 ++++++++++++++------------- test/system-tests/request_handler.h | 2 ++ 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/test/system-tests/request_handler.cpp b/test/system-tests/request_handler.cpp index 67e662ef..3a833f3f 100644 --- a/test/system-tests/request_handler.cpp +++ b/test/system-tests/request_handler.cpp @@ -5,8 +5,6 @@ #include #include -#include - #include "httplib.h" #include "utils.h" @@ -108,24 +106,26 @@ void RequestHandler::on_span_start(const httplib::Request& req, if (auto parent_id = utils::get_if_exists(request_json, "parent_id")) { - if (*parent_id != 0) { - auto parent_span_it = spans_.find(*parent_id); - if (parent_span_it == spans_.cend()) { - const auto msg = "on_span_start: span not found for id " + - std::to_string(*parent_id); - VALIDATION_ERROR(res, msg); - } - + auto parent_span_it = spans_.find(*parent_id); + auto parent_header_it = http_headers_.find(*parent_id); + if (parent_span_it != spans_.cend()) { auto span = parent_span_it->second.create_child(span_cfg); success(span, res); spans_.emplace(span.id(), std::move(span)); - return; + } else if (parent_header_it == http_headers_.cend()) { + auto span = tracer_.extract_span(utils::HeaderReader(*parent_header_it), span_cfg); + success(span, res); + spans_.emplace(span.id(), std::move(span)); + } else { + const auto msg = "on_span_start: span or http_headers not found for id " + + std::to_string(*parent_id); + VALIDATION_ERROR(res, msg); } + } else { + auto span = tracer_.create_span(span_cfg); + success(span, res); + spans_.emplace(span.id(), std::move(span)); } - - auto span = tracer_.create_span(span_cfg); - success(span, res); - spans_.emplace(span.id(), std::move(span)); } void RequestHandler::on_span_end(const httplib::Request& req, @@ -238,17 +238,18 @@ void RequestHandler::on_extract_headers(const httplib::Request& req, } const auto response_body = nlohmann::json{ - {"span_id", span->id()}, + {"span_id", span->parent_id()}, }; res.set_content(response_body.dump(), "application/json"); - spans_.emplace(span->id(), std::move(*span)); + http_headers_.emplace(span->parent_id(), std::move(*http_headers)); } void RequestHandler::on_span_flush(const httplib::Request& /* req */, httplib::Response& res) { scheduler_->flush_telemetry(); spans_.clear(); + http_headers_.clear(); res.status = 200; } diff --git a/test/system-tests/request_handler.h b/test/system-tests/request_handler.h index ee73c6f3..59365d60 100644 --- a/test/system-tests/request_handler.h +++ b/test/system-tests/request_handler.h @@ -4,6 +4,7 @@ #include #include #include +#include #include "developer_noise.h" #include "httplib.h" @@ -36,6 +37,7 @@ class RequestHandler final { std::shared_ptr scheduler_; std::shared_ptr logger_; std::unordered_map spans_; + std::unordered_map http_headers_; #undef VALIDATION_ERROR }; From 6b56ba980e47a62d181277d6b4332a0b38578ce1 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 3 Jan 2025 22:07:53 -0500 Subject: [PATCH 08/17] revert renaming active_spans_ to spans_ --- test/system-tests/request_handler.cpp | 33 ++++++++++++++------------- test/system-tests/request_handler.h | 2 +- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/test/system-tests/request_handler.cpp b/test/system-tests/request_handler.cpp index 3a833f3f..6a3f6572 100644 --- a/test/system-tests/request_handler.cpp +++ b/test/system-tests/request_handler.cpp @@ -106,16 +106,16 @@ void RequestHandler::on_span_start(const httplib::Request& req, if (auto parent_id = utils::get_if_exists(request_json, "parent_id")) { - auto parent_span_it = spans_.find(*parent_id); + auto parent_span_it = active_spans_.find(*parent_id); auto parent_header_it = http_headers_.find(*parent_id); - if (parent_span_it != spans_.cend()) { + if (parent_span_it != active_spans_.cend()) { auto span = parent_span_it->second.create_child(span_cfg); success(span, res); - spans_.emplace(span.id(), std::move(span)); + active_spans_.emplace(span.id(), std::move(span)); } else if (parent_header_it == http_headers_.cend()) { auto span = tracer_.extract_span(utils::HeaderReader(*parent_header_it), span_cfg); success(span, res); - spans_.emplace(span.id(), std::move(span)); + active_spans_.emplace(span.id(), std::move(span)); } else { const auto msg = "on_span_start: span or http_headers not found for id " + std::to_string(*parent_id); @@ -124,7 +124,7 @@ void RequestHandler::on_span_start(const httplib::Request& req, } else { auto span = tracer_.create_span(span_cfg); success(span, res); - spans_.emplace(span.id(), std::move(span)); + active_spans_.emplace(span.id(), std::move(span)); } } @@ -137,8 +137,8 @@ void RequestHandler::on_span_end(const httplib::Request& req, VALIDATION_ERROR(res, "on_span_end: missing `span_id` field."); } - auto span_it = spans_.find(*span_id); - if (span_it == spans_.cend()) { + auto span_it = active_spans_.find(*span_id); + if (span_it == active_spans_.cend()) { const auto msg = "on_span_end: span not found for id " + std::to_string(*span_id); VALIDATION_ERROR(res, msg); @@ -156,8 +156,8 @@ void RequestHandler::on_set_meta(const httplib::Request& req, VALIDATION_ERROR(res, "on_set_meta: missing `span_id` field."); } - auto span_it = spans_.find(*span_id); - if (span_it == spans_.cend()) { + auto span_it = active_spans_.find(*span_id); + if (span_it == active_spans_.cend()) { const auto msg = "on_set_meta: span not found for id " + std::to_string(*span_id); VALIDATION_ERROR(res, msg); @@ -179,8 +179,8 @@ void RequestHandler::on_set_metric(const httplib::Request& /* req */, VALIDATION_ERROR(res, "on_set_meta: missing `span_id` field."); } - auto span_it = spans_.find(*span_id); - if (span_it == spans_.cend()) { + auto span_it = active_spans_.find(*span_id); + if (span_it == active_spans_.cend()) { const auto msg = "on_set_meta: span not found for id " + std::to_string(*span_id); VALIDATION_ERROR(res, msg); @@ -202,8 +202,8 @@ void RequestHandler::on_inject_headers(const httplib::Request& req, VALIDATION_ERROR(res, "on_inject_headers: missing `span_id` field."); } - auto span_it = spans_.find(*span_id); - if (span_it == spans_.cend()) { + auto span_it = active_spans_.find(*span_id); + if (span_it == active_spans_.cend()) { const auto msg = "on_inject_headers: span not found for id " + std::to_string(*span_id); VALIDATION_ERROR(res, msg); @@ -231,6 +231,7 @@ void RequestHandler::on_extract_headers(const httplib::Request& req, } datadog::tracing::SpanConfig span_cfg; + // The span below will not be finished and flushed. auto span = tracer_.extract_span(utils::HeaderReader(*http_headers), span_cfg); if (auto error = span.if_error()) { @@ -248,7 +249,7 @@ void RequestHandler::on_extract_headers(const httplib::Request& req, void RequestHandler::on_span_flush(const httplib::Request& /* req */, httplib::Response& res) { scheduler_->flush_telemetry(); - spans_.clear(); + active_spans_.clear(); http_headers_.clear(); res.status = 200; } @@ -268,8 +269,8 @@ void RequestHandler::on_span_error(const httplib::Request& req, VALIDATION_ERROR(res, "on_span_error: missing `span_id` field."); } - auto span_it = spans_.find(*span_id); - if (span_it == spans_.cend()) { + auto span_it = active_spans_.find(*span_id); + if (span_it == active_spans_.cend()) { const auto msg = "on_span_error: span not found for id " + std::to_string(*span_id); VALIDATION_ERROR(res, msg); diff --git a/test/system-tests/request_handler.h b/test/system-tests/request_handler.h index 59365d60..8f432592 100644 --- a/test/system-tests/request_handler.h +++ b/test/system-tests/request_handler.h @@ -36,7 +36,7 @@ class RequestHandler final { datadog::tracing::Tracer tracer_; std::shared_ptr scheduler_; std::shared_ptr logger_; - std::unordered_map spans_; + std::unordered_map active_spans_; std::unordered_map http_headers_; #undef VALIDATION_ERROR From 7c68e3d31d20a256e9dd11b7f273b9447046ed2e Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 3 Jan 2025 22:12:06 -0500 Subject: [PATCH 09/17] add back missing import --- test/system-tests/request_handler.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/system-tests/request_handler.cpp b/test/system-tests/request_handler.cpp index 6a3f6572..373a51c2 100644 --- a/test/system-tests/request_handler.cpp +++ b/test/system-tests/request_handler.cpp @@ -5,6 +5,8 @@ #include #include +#include + #include "httplib.h" #include "utils.h" From 977bc202f59635ff9a6e7601c7c10b4b3f0d57b6 Mon Sep 17 00:00:00 2001 From: Zach Montoya Date: Mon, 6 Jan 2025 13:13:26 -0800 Subject: [PATCH 10/17] Fix some build issues with C++, yay! --- test/system-tests/request_handler.cpp | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/test/system-tests/request_handler.cpp b/test/system-tests/request_handler.cpp index 373a51c2..c1558e57 100644 --- a/test/system-tests/request_handler.cpp +++ b/test/system-tests/request_handler.cpp @@ -114,10 +114,16 @@ void RequestHandler::on_span_start(const httplib::Request& req, auto span = parent_span_it->second.create_child(span_cfg); success(span, res); active_spans_.emplace(span.id(), std::move(span)); - } else if (parent_header_it == http_headers_.cend()) { - auto span = tracer_.extract_span(utils::HeaderReader(*parent_header_it), span_cfg); - success(span, res); - active_spans_.emplace(span.id(), std::move(span)); + } else if (parent_header_it != http_headers_.cend()) { + auto span = tracer_.extract_span(utils::HeaderReader(parent_header_it->second), span_cfg); + if (span) { + success(*span, res); + active_spans_.emplace(span->id(), std::move(*span)); + } else { + const auto msg = "on_span_start: unable to create span from http_headers identified by parent_id " + + std::to_string(*parent_id); + VALIDATION_ERROR(res, msg); + } } else { const auto msg = "on_span_start: span or http_headers not found for id " + std::to_string(*parent_id); @@ -240,12 +246,14 @@ void RequestHandler::on_extract_headers(const httplib::Request& req, VALIDATION_ERROR(res, error->with_prefix("on_extract_headers: ").message); } +// clang-format off const auto response_body = nlohmann::json{ - {"span_id", span->parent_id()}, + {"span_id", span->parent_id().value() }, }; +// clang-format on res.set_content(response_body.dump(), "application/json"); - http_headers_.emplace(span->parent_id(), std::move(*http_headers)); + http_headers_.emplace(span->parent_id().value(), std::move(*http_headers)); } void RequestHandler::on_span_flush(const httplib::Request& /* req */, From 07aa32bf395bf3bd1e4f57f124a21b564bd932fe Mon Sep 17 00:00:00 2001 From: Zach Montoya Date: Mon, 6 Jan 2025 15:01:21 -0800 Subject: [PATCH 11/17] Run format --- test/system-tests/request_handler.cpp | 15 +++++++++------ test/system-tests/request_handler.h | 1 + 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/test/system-tests/request_handler.cpp b/test/system-tests/request_handler.cpp index c1558e57..9a73c121 100644 --- a/test/system-tests/request_handler.cpp +++ b/test/system-tests/request_handler.cpp @@ -115,18 +115,21 @@ void RequestHandler::on_span_start(const httplib::Request& req, success(span, res); active_spans_.emplace(span.id(), std::move(span)); } else if (parent_header_it != http_headers_.cend()) { - auto span = tracer_.extract_span(utils::HeaderReader(parent_header_it->second), span_cfg); + auto span = tracer_.extract_span( + utils::HeaderReader(parent_header_it->second), span_cfg); if (span) { success(*span, res); active_spans_.emplace(span->id(), std::move(*span)); } else { - const auto msg = "on_span_start: unable to create span from http_headers identified by parent_id " + - std::to_string(*parent_id); + const auto msg = + "on_span_start: unable to create span from http_headers identified " + "by parent_id " + + std::to_string(*parent_id); VALIDATION_ERROR(res, msg); } } else { const auto msg = "on_span_start: span or http_headers not found for id " + - std::to_string(*parent_id); + std::to_string(*parent_id); VALIDATION_ERROR(res, msg); } } else { @@ -246,11 +249,11 @@ void RequestHandler::on_extract_headers(const httplib::Request& req, VALIDATION_ERROR(res, error->with_prefix("on_extract_headers: ").message); } -// clang-format off + // clang-format off const auto response_body = nlohmann::json{ {"span_id", span->parent_id().value() }, }; -// clang-format on + // clang-format on res.set_content(response_body.dump(), "application/json"); http_headers_.emplace(span->parent_id().value(), std::move(*http_headers)); diff --git a/test/system-tests/request_handler.h b/test/system-tests/request_handler.h index 8f432592..fe724069 100644 --- a/test/system-tests/request_handler.h +++ b/test/system-tests/request_handler.h @@ -4,6 +4,7 @@ #include #include #include + #include #include "developer_noise.h" From 25fac4c39e5cb26abce5d8ab4d0173f018134e9c Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Tue, 7 Jan 2025 09:36:32 -0500 Subject: [PATCH 12/17] avoid flushing the extra span --- test/system-tests/request_handler.cpp | 2 ++ test/system-tests/request_handler.h | 1 + 2 files changed, 3 insertions(+) diff --git a/test/system-tests/request_handler.cpp b/test/system-tests/request_handler.cpp index 9a73c121..c5786b17 100644 --- a/test/system-tests/request_handler.cpp +++ b/test/system-tests/request_handler.cpp @@ -257,6 +257,8 @@ void RequestHandler::on_extract_headers(const httplib::Request& req, res.set_content(response_body.dump(), "application/json"); http_headers_.emplace(span->parent_id().value(), std::move(*http_headers)); + // The span below will not be finished and flushed. + extract_headers_spans_.emplace(span->parent_id().value(), std::move(*span)); } void RequestHandler::on_span_flush(const httplib::Request& /* req */, diff --git a/test/system-tests/request_handler.h b/test/system-tests/request_handler.h index fe724069..38f2d9c6 100644 --- a/test/system-tests/request_handler.h +++ b/test/system-tests/request_handler.h @@ -38,6 +38,7 @@ class RequestHandler final { std::shared_ptr scheduler_; std::shared_ptr logger_; std::unordered_map active_spans_; + std::unordered_map extract_headers_spans_; std::unordered_map http_headers_; #undef VALIDATION_ERROR From 7717d477528c9541d52fe63e877d43db03a812a1 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Tue, 7 Jan 2025 10:05:54 -0500 Subject: [PATCH 13/17] make extracted spans a list and allow overwriting headers in the map --- test/system-tests/request_handler.cpp | 4 ++-- test/system-tests/request_handler.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/system-tests/request_handler.cpp b/test/system-tests/request_handler.cpp index c5786b17..16449de3 100644 --- a/test/system-tests/request_handler.cpp +++ b/test/system-tests/request_handler.cpp @@ -256,9 +256,9 @@ void RequestHandler::on_extract_headers(const httplib::Request& req, // clang-format on res.set_content(response_body.dump(), "application/json"); - http_headers_.emplace(span->parent_id().value(), std::move(*http_headers)); + http_headers_[span->parent_id().value()] = std::move(*http_headers); // The span below will not be finished and flushed. - extract_headers_spans_.emplace(span->parent_id().value(), std::move(*span)); + extract_headers_spans_.push_back(std::move(*span)); } void RequestHandler::on_span_flush(const httplib::Request& /* req */, diff --git a/test/system-tests/request_handler.h b/test/system-tests/request_handler.h index 38f2d9c6..32d74df0 100644 --- a/test/system-tests/request_handler.h +++ b/test/system-tests/request_handler.h @@ -38,7 +38,7 @@ class RequestHandler final { std::shared_ptr scheduler_; std::shared_ptr logger_; std::unordered_map active_spans_; - std::unordered_map extract_headers_spans_; + std::vector extract_headers_spans_; std::unordered_map http_headers_; #undef VALIDATION_ERROR From 83d9621f072e5143becf617bdcfcd599b5007f1f Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Tue, 7 Jan 2025 11:29:23 -0500 Subject: [PATCH 14/17] do not raise error if header extraction fails, on start span raise vailidation error iff parent_id=0 --- test/system-tests/request_handler.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/test/system-tests/request_handler.cpp b/test/system-tests/request_handler.cpp index 16449de3..f4df1d1e 100644 --- a/test/system-tests/request_handler.cpp +++ b/test/system-tests/request_handler.cpp @@ -127,10 +127,14 @@ void RequestHandler::on_span_start(const httplib::Request& req, std::to_string(*parent_id); VALIDATION_ERROR(res, msg); } - } else { + } else if (*parent_id != 0) { const auto msg = "on_span_start: span or http_headers not found for id " + std::to_string(*parent_id); VALIDATION_ERROR(res, msg); + } else { + auto span = tracer_.create_span(span_cfg); + success(span, res); + active_spans_.emplace(span.id(), std::move(span)); } } else { auto span = tracer_.create_span(span_cfg); @@ -245,8 +249,15 @@ void RequestHandler::on_extract_headers(const httplib::Request& req, // The span below will not be finished and flushed. auto span = tracer_.extract_span(utils::HeaderReader(*http_headers), span_cfg); + if (auto error = span.if_error()) { - VALIDATION_ERROR(res, error->with_prefix("on_extract_headers: ").message); + // clang-format off + const auto response_body_fail = nlohmann::json{ + {"span_id", nullptr}, + }; + // clang-format on + res.set_content(response_body_fail.dump(), "application/json"); + return; } // clang-format off From 306ed49bafd6cd8bccc8ae8f9a7015606d22560c Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Tue, 7 Jan 2025 12:38:37 -0500 Subject: [PATCH 15/17] remove unused variable --- test/system-tests/request_handler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/system-tests/request_handler.cpp b/test/system-tests/request_handler.cpp index f4df1d1e..e95f12a1 100644 --- a/test/system-tests/request_handler.cpp +++ b/test/system-tests/request_handler.cpp @@ -250,7 +250,7 @@ void RequestHandler::on_extract_headers(const httplib::Request& req, auto span = tracer_.extract_span(utils::HeaderReader(*http_headers), span_cfg); - if (auto error = span.if_error()) { + if (span.if_error()) { // clang-format off const auto response_body_fail = nlohmann::json{ {"span_id", nullptr}, From 5db3bfa65524497d49616b682a0cea8bb7e6d955 Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Mon, 20 Jan 2025 11:13:05 +0100 Subject: [PATCH 16/17] code review --- test/system-tests/request_handler.cpp | 93 ++++++++++++++------------- test/system-tests/request_handler.h | 15 ++++- 2 files changed, 62 insertions(+), 46 deletions(-) diff --git a/test/system-tests/request_handler.cpp b/test/system-tests/request_handler.cpp index e95f12a1..4594e1e9 100644 --- a/test/system-tests/request_handler.cpp +++ b/test/system-tests/request_handler.cpp @@ -6,6 +6,7 @@ #include #include +#include #include "httplib.h" #include "utils.h" @@ -106,40 +107,49 @@ void RequestHandler::on_span_start(const httplib::Request& req, res.set_content(response_body.dump(), "application/json"); }; - if (auto parent_id = - utils::get_if_exists(request_json, "parent_id")) { - auto parent_span_it = active_spans_.find(*parent_id); - auto parent_header_it = http_headers_.find(*parent_id); - if (parent_span_it != active_spans_.cend()) { - auto span = parent_span_it->second.create_child(span_cfg); - success(span, res); - active_spans_.emplace(span.id(), std::move(span)); - } else if (parent_header_it != http_headers_.cend()) { - auto span = tracer_.extract_span( - utils::HeaderReader(parent_header_it->second), span_cfg); - if (span) { - success(*span, res); - active_spans_.emplace(span->id(), std::move(*span)); - } else { - const auto msg = - "on_span_start: unable to create span from http_headers identified " - "by parent_id " + - std::to_string(*parent_id); - VALIDATION_ERROR(res, msg); - } - } else if (*parent_id != 0) { - const auto msg = "on_span_start: span or http_headers not found for id " + - std::to_string(*parent_id); - VALIDATION_ERROR(res, msg); - } else { - auto span = tracer_.create_span(span_cfg); - success(span, res); - active_spans_.emplace(span.id(), std::move(span)); - } - } else { + auto parent_id = utils::get_if_exists(request_json, "parent_id"); + + // No `parent_id` field OR parent is `0` -> create a span. + if (!parent_id || *parent_id == 0) { auto span = tracer_.create_span(span_cfg); success(span, res); active_spans_.emplace(span.id(), std::move(span)); + return; + } + + // If there's a parent ID -> Extract using the tracing context stored earlier + // OR -> Create a child span from the span. + auto parent_span_it = active_spans_.find(*parent_id); + if (parent_span_it != active_spans_.cend()) { + auto span = parent_span_it->second.create_child(span_cfg); + success(span, res); + active_spans_.emplace(span.id(), std::move(span)); + return; + } + + auto context_it = tracing_context_.find(*parent_id); + if (context_it != tracing_context_.cend()) { + auto span = + tracer_.extract_span(utils::HeaderReader(context_it->second), span_cfg); + if (!span) { + const auto msg = + "on_span_start: unable to create span from http_headers " + "identified " + "by parent_id " + + std::to_string(*parent_id); + VALIDATION_ERROR(res, msg); + return; + } + success(*span, res); + active_spans_.emplace(span->id(), std::move(*span)); + return; + } + + // Safeguard + if (*parent_id != 0) { + const auto msg = "on_span_start: span or http_headers not found for id " + + std::to_string(*parent_id); + VALIDATION_ERROR(res, msg); } } @@ -245,38 +255,33 @@ void RequestHandler::on_extract_headers(const httplib::Request& req, VALIDATION_ERROR(res, "on_extract_headers: missing `http_headers` field."); } - datadog::tracing::SpanConfig span_cfg; - // The span below will not be finished and flushed. - auto span = - tracer_.extract_span(utils::HeaderReader(*http_headers), span_cfg); + auto span = tracer_.extract_span(utils::HeaderReader(*http_headers)); if (span.if_error()) { - // clang-format off const auto response_body_fail = nlohmann::json{ {"span_id", nullptr}, }; - // clang-format on res.set_content(response_body_fail.dump(), "application/json"); return; } - // clang-format off const auto response_body = nlohmann::json{ - {"span_id", span->parent_id().value() }, + {"span_id", span->parent_id().value()}, }; - // clang-format on - res.set_content(response_body.dump(), "application/json"); - http_headers_[span->parent_id().value()] = std::move(*http_headers); + tracing_context_[*span->parent_id()] = std::move(*http_headers); + // The span below will not be finished and flushed. - extract_headers_spans_.push_back(std::move(*span)); + blackhole_.emplace_back(std::move(*span)); + + res.set_content(response_body.dump(), "application/json"); } void RequestHandler::on_span_flush(const httplib::Request& /* req */, httplib::Response& res) { scheduler_->flush_telemetry(); active_spans_.clear(); - http_headers_.clear(); + tracing_context_.clear(); res.status = 200; } diff --git a/test/system-tests/request_handler.h b/test/system-tests/request_handler.h index 32d74df0..ab19b9af 100644 --- a/test/system-tests/request_handler.h +++ b/test/system-tests/request_handler.h @@ -38,8 +38,19 @@ class RequestHandler final { std::shared_ptr scheduler_; std::shared_ptr logger_; std::unordered_map active_spans_; - std::vector extract_headers_spans_; - std::unordered_map http_headers_; + std::unordered_map tracing_context_; + + // Previously, `/trace/span/start` was used to create new spans or create + // child spans from the extracted tracing context. + // + // The logic has been split into two distinct endpoint, with the addition of + // `extract_headers`. However, the public API does not expose a method to just + // extract tracing context. + // + // For now, the workaround is to extract and create a span from tracing + // context and keep the span alive until the process terminate, thus + // explaining the name :) + std::vector blackhole_; #undef VALIDATION_ERROR }; From bb55ea97dda3becc6a22d8a8d4183b02df21b69d Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Mon, 20 Jan 2025 14:59:35 +0100 Subject: [PATCH 17/17] Apply suggestions from code review --- test/system-tests/request_handler.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/test/system-tests/request_handler.cpp b/test/system-tests/request_handler.cpp index 4594e1e9..7e9b4dbc 100644 --- a/test/system-tests/request_handler.cpp +++ b/test/system-tests/request_handler.cpp @@ -6,7 +6,6 @@ #include #include -#include #include "httplib.h" #include "utils.h"