diff --git a/src/datadog/span.cpp b/src/datadog/span.cpp index 81235e1f..5a6a12cb 100644 --- a/src/datadog/span.cpp +++ b/src/datadog/span.cpp @@ -101,27 +101,33 @@ const std::string& Span::name() const { return data_->name; } const std::string& Span::resource_name() const { return data_->resource; } Optional Span::lookup_tag(StringView name) const { - if (tags::is_internal(name)) { + const auto found = data_->tags.find(std::string(name)); + if (found == data_->tags.end()) { return nullopt; } + return found->second; +} - const auto found = data_->tags.find(std::string(name)); - if (found == data_->tags.end()) { +Optional Span::lookup_metric(StringView name) const { + const auto found = data_->numeric_tags.find(std::string(name)); + if (found == data_->numeric_tags.end()) { return nullopt; } return found->second; } void Span::set_tag(StringView name, StringView value) { - if (!tags::is_internal(name)) { - data_->tags.insert_or_assign(std::string(name), std::string(value)); - } + data_->tags.insert_or_assign(std::string(name), std::string(value)); } -void Span::remove_tag(StringView name) { - if (!tags::is_internal(name)) { - data_->tags.erase(std::string(name)); - } +void Span::set_metric(StringView name, double value) { + data_->numeric_tags.insert_or_assign(std::string(name), value); +} + +void Span::remove_tag(StringView name) { data_->tags.erase(std::string(name)); } + +void Span::remove_metric(StringView name) { + data_->numeric_tags.erase(std::string(name)); } void Span::set_service_name(StringView service) { diff --git a/src/datadog/span.h b/src/datadog/span.h index 089e3064..803658d2 100644 --- a/src/datadog/span.h +++ b/src/datadog/span.h @@ -125,12 +125,19 @@ class Span { // Return the value of the tag having the specified `name`, or return null if // there is no such tag. Optional lookup_tag(StringView name) const; + // Return the value of the metric having the specified `name`, or return null + // if there is no such metric. + Optional lookup_metric(StringView name) const; // Overwrite the tag having the specified `name` so that it has the specified // `value`, or create a new tag. void set_tag(StringView name, StringView value); + // Overwrite the metric having the specified `name` so that it has the + // specified `value`, or create a new metric. + void set_metric(StringView name, double value); // Delete the tag having the specified `name` if it exists. void remove_tag(StringView name); - + // Delete the metric having the specified `name` if it exists. + void remove_metric(StringView name); // Set the name of the service associated with this span, e.g. // "ingress-nginx-useast1". void set_service_name(StringView); diff --git a/src/datadog/span_data.cpp b/src/datadog/span_data.cpp index b0a67fc8..e575285d 100644 --- a/src/datadog/span_data.cpp +++ b/src/datadog/span_data.cpp @@ -51,9 +51,7 @@ void SpanData::apply_config(const SpanDefaults& defaults, tags.insert_or_assign(tags::version, version); } for (const auto& [key, value] : config.tags) { - if (!tags::is_internal(key)) { - tags.insert_or_assign(key, value); - } + tags.insert_or_assign(key, value); } resource = config.resource.value_or(name); @@ -139,4 +137,4 @@ Expected msgpack_encode( } } // namespace tracing -} // namespace datadog \ No newline at end of file +} // namespace datadog diff --git a/test/system-tests/request_handler.cpp b/test/system-tests/request_handler.cpp index ca4bb7cd..1846ddc2 100644 --- a/test/system-tests/request_handler.cpp +++ b/test/system-tests/request_handler.cpp @@ -165,8 +165,24 @@ void RequestHandler::on_set_meta(const httplib::Request& req, void RequestHandler::on_set_metric(const httplib::Request& /* req */, httplib::Response& res) { - // No method available for directly setting a span metric. - // Returning OK instead of UNIMPLEMENTED to satisfy the test framework. + const auto request_json = nlohmann::json::parse(res.body); + + auto span_id = utils::get_if_exists(request_json, "span_id"); + if (!span_id) { + VALIDATION_ERROR(res, "on_set_meta: missing `span_id` field."); + } + + 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); + } + + auto& span = span_it->second; + span.set_metric(request_json.at("key").get(), + request_json.at("value").get()); + res.status = 200; } diff --git a/test/test_span.cpp b/test/test_span.cpp index 3cdf599c..f6aee85a 100644 --- a/test/test_span.cpp +++ b/test/test_span.cpp @@ -46,6 +46,9 @@ TEST_CASE("set_tag") { span.set_tag("foo", "lemon"); span.set_tag("foo.bar", "mint"); span.set_tag("foo.baz", "blueberry"); + span.set_tag("_dd.secret.sauce", "thousand islands"); + span.set_tag("_dd_not_internal", ""); + span.set_tag("_dd.chipmunk", ""); } REQUIRE(collector->chunks.size() == 1); @@ -57,18 +60,21 @@ TEST_CASE("set_tag") { REQUIRE(span.tags.at("foo") == "lemon"); REQUIRE(span.tags.at("foo.bar") == "mint"); REQUIRE(span.tags.at("foo.baz") == "blueberry"); + REQUIRE(span.tags.at("_dd.secret.sauce") == "thousand islands"); + REQUIRE(span.tags.at("_dd_not_internal") == ""); + REQUIRE(span.tags.at("_dd.chipmunk") == ""); } SECTION("tags can be overwritten") { { SpanConfig config; - config.tags = { - {"color", "purple"}, - {"turtle.depth", "all the way down"}, - }; + config.tags = {{"color", "purple"}, + {"turtle.depth", "all the way down"}, + {"_dd.tag", "written"}}; auto span = tracer.create_span(config); span.set_tag("color", "green"); span.set_tag("bonus", "applied"); + span.set_tag("_dd.tag", "overwritten"); } REQUIRE(collector->chunks.size() == 1); @@ -80,29 +86,7 @@ TEST_CASE("set_tag") { REQUIRE(span.tags.at("color") == "green"); REQUIRE(span.tags.at("turtle.depth") == "all the way down"); REQUIRE(span.tags.at("bonus") == "applied"); - } - - SECTION("can't set internal tags directly") { - { - auto span = tracer.create_span(); - span.set_tag("foo", "lemon"); - span.set_tag("_dd.secret.sauce", "thousand islands"); - span.set_tag("_dd_not_internal", ""); - // _dd.p.dm will end up in the tags due to how sampling works - // span.set_tag("_dd.p.dm", "-4"); - span.set_tag("_dd.chipmunk", ""); - } - - REQUIRE(collector->chunks.size() == 1); - const auto& chunk = collector->chunks.front(); - REQUIRE(chunk.size() == 1); - const auto& span_ptr = chunk.front(); - REQUIRE(span_ptr); - const auto& span = *span_ptr; - REQUIRE(span.tags.at("foo") == "lemon"); - REQUIRE(span.tags.count("_dd.secret.sauce") == 0); - REQUIRE(span.tags.at("_dd_not_internal") == ""); - REQUIRE(span.tags.count("_dd.chipmunk") == 0); + REQUIRE(span.tags.at("_dd.tag") == "overwritten"); } } @@ -120,15 +104,18 @@ TEST_CASE("lookup_tag") { auto span = tracer.create_span(); REQUIRE(!span.lookup_tag("nope")); REQUIRE(!span.lookup_tag("also nope")); + REQUIRE(!span.lookup_tag("_dd.nope")); } SECTION("lookup after set") { auto span = tracer.create_span(); span.set_tag("color", "purple"); span.set_tag("turtle.depth", "all the way down"); + span.set_tag("_dd.tag", "found"); REQUIRE(span.lookup_tag("color") == "purple"); REQUIRE(span.lookup_tag("turtle.depth") == "all the way down"); + REQUIRE(span.lookup_tag("_dd.tag") == "found"); } SECTION("lookup after config") { @@ -136,18 +123,13 @@ TEST_CASE("lookup_tag") { config.tags = { {"color", "purple"}, {"turtle.depth", "all the way down"}, + {"_dd.tag", "found"}, }; auto span = tracer.create_span(config); REQUIRE(span.lookup_tag("color") == "purple"); REQUIRE(span.lookup_tag("turtle.depth") == "all the way down"); - } - - SECTION("internal tags redacted") { - auto span = tracer.create_span(); - REQUIRE(!span.lookup_tag("_dd.this")); - REQUIRE(!span.lookup_tag("_dd.that")); - REQUIRE(!span.lookup_tag("_dd.the.other.thing")); + REQUIRE(span.lookup_tag("_dd.tag") == "found"); } } @@ -164,22 +146,141 @@ TEST_CASE("remove_tag") { SECTION("doesn't have to be there already") { auto span = tracer.create_span(); span.remove_tag("not even there"); + span.remove_tag("_dd.tag"); } SECTION("after removal, lookup yields null") { SpanConfig config; - config.tags = {{"mayfly", "carpe diem"}}; + config.tags = {{"mayfly", "carpe diem"}, {"_dd.mayfly", "carpe diem"}}; auto span = tracer.create_span(config); span.set_tag("foo", "bar"); span.remove_tag("mayfly"); + span.remove_tag("_dd.mayfly"); span.remove_tag("foo"); REQUIRE(!span.lookup_tag("mayfly")); + REQUIRE(!span.lookup_tag("_dd.mayfly")); REQUIRE(!span.lookup_tag("foo")); } } +TEST_CASE("set_metric") { + TracerConfig config; + config.service = "testsvc"; + const auto collector = std::make_shared(); + config.collector = collector; + config.logger = std::make_shared(); + + auto finalized_config = finalize_config(config); + REQUIRE(finalized_config); + Tracer tracer{*finalized_config}; + + SECTION("metrics end up in the collector") { + { + auto span = tracer.create_span(); + span.set_metric("foo", 5.0); + span.set_metric("foo.bar", 3.0); + span.set_metric("foo.baz", 1.0); + span.set_metric("_dd.secret.sauce", 2.0); + span.set_metric("_dd_not_internal", 3.0); + span.set_metric("_dd.chipmunk", 4.0); + } + + REQUIRE(collector->chunks.size() == 1); + const auto& chunk = collector->chunks.front(); + REQUIRE(chunk.size() == 1); + const auto& span_ptr = chunk.front(); + REQUIRE(span_ptr); + const auto& span = *span_ptr; + REQUIRE(span.numeric_tags.at("foo") == 5.0); + REQUIRE(span.numeric_tags.at("foo.bar") == 3.0); + REQUIRE(span.numeric_tags.at("foo.baz") == 1.0); + REQUIRE(span.numeric_tags.at("_dd.secret.sauce") == 2.0); + REQUIRE(span.numeric_tags.at("_dd_not_internal") == 3.0); + REQUIRE(span.numeric_tags.at("_dd.chipmunk") == 4.0); + } + + SECTION("metrics can be overwritten") { + { + auto span = tracer.create_span(); + span.set_metric("color", 2.0); + span.set_metric("color", 1.0); + span.set_metric("bonus", 6.0); + span.set_metric("bonus", 5.0); + } + + REQUIRE(collector->chunks.size() == 1); + const auto& chunk = collector->chunks.front(); + REQUIRE(chunk.size() == 1); + const auto& span_ptr = chunk.front(); + REQUIRE(span_ptr); + const auto& span = *span_ptr; + REQUIRE(span.numeric_tags.at("color") == 1.0); + REQUIRE(span.numeric_tags.at("bonus") == 5.0); + } +} + +TEST_CASE("lookup_metric") { + TracerConfig config; + config.service = "testsvc"; + config.collector = std::make_shared(); + config.logger = std::make_shared(); + + auto finalized_config = finalize_config(config); + REQUIRE(finalized_config); + Tracer tracer{*finalized_config}; + + SECTION("not found is null") { + auto span = tracer.create_span(); + REQUIRE(!span.lookup_metric("nope")); + REQUIRE(!span.lookup_metric("also nope")); + REQUIRE(!span.lookup_metric("_dd.nope")); + } + + SECTION("lookup after set") { + auto span = tracer.create_span(); + span.set_metric("color", 11.0); + span.set_metric("turtle.depth", 6.0); + span.set_metric("_dd.this", 33.0); + + REQUIRE(span.lookup_metric("color") == 11.0); + REQUIRE(span.lookup_metric("turtle.depth") == 6.0); + REQUIRE(span.lookup_metric("_dd.this") == 33.0); + } +} + +TEST_CASE("remove_metric") { + TracerConfig config; + config.service = "testsvc"; + config.collector = std::make_shared(); + config.logger = std::make_shared(); + + auto finalized_config = finalize_config(config); + REQUIRE(finalized_config); + Tracer tracer{*finalized_config}; + + SECTION("doesn't have to be there already") { + auto span = tracer.create_span(); + span.remove_metric("not even there"); + } + + SECTION("after removal, lookup yields null") { + auto span = tracer.create_span(); + span.set_metric("mayfly", 10.0); + span.set_metric("foo", 11.0); + span.set_metric("_dd.metric", 1.0); + + span.remove_metric("mayfly"); + span.remove_metric("foo"); + span.remove_metric("_dd.metric"); + + REQUIRE(!span.lookup_metric("mayfly")); + REQUIRE(!span.lookup_metric("foo")); + REQUIRE(!span.lookup_metric("_dd.metric")); + } +} + TEST_CASE("span duration") { TracerConfig config; config.service = "testsvc";