From 7f2a7f2508c4de31426a195d34a426bf6034b151 Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Sat, 5 Apr 2025 16:23:09 +0200 Subject: [PATCH 1/3] [part 7] refactor!(telemetry): improve telemetry lifecycle Changes: - add support for install_signature. - add support for product in configuration that will be reported in app-started message. - unit tests --- BUILD.bazel | 1 + include/datadog/environment.h | 5 +- include/datadog/telemetry/configuration.h | 12 ++ include/datadog/telemetry/product.h | 57 ++++++++ include/datadog/telemetry/telemetry.h | 20 +-- src/datadog/telemetry/configuration.cpp | 16 +++ src/datadog/telemetry/telemetry.cpp | 10 -- src/datadog/telemetry/telemetry_impl.cpp | 167 +++++++++++++--------- src/datadog/telemetry/telemetry_impl.h | 13 +- src/datadog/tracer.cpp | 2 - src/datadog/tracer_config.cpp | 18 ++- test/telemetry/test_configuration.cpp | 27 +++- test/telemetry/test_metrics.cpp | 7 +- test/telemetry/test_telemetry.cpp | 164 +++++++++++++++------ 14 files changed, 356 insertions(+), 163 deletions(-) create mode 100644 include/datadog/telemetry/product.h diff --git a/BUILD.bazel b/BUILD.bazel index e3ea62d7..eb59ad4f 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -119,6 +119,7 @@ cc_library( "include/datadog/telemetry/configuration.h", "include/datadog/telemetry/metrics.h", "include/datadog/telemetry/telemetry.h", + "include/datadog/telemetry/product.h", "include/datadog/remote_config/capability.h", "include/datadog/remote_config/listener.h", "include/datadog/remote_config/product.h", diff --git a/include/datadog/environment.h b/include/datadog/environment.h index 7a1e1fa5..266737dc 100644 --- a/include/datadog/environment.h +++ b/include/datadog/environment.h @@ -56,7 +56,10 @@ namespace environment { MACRO(DD_TELEMETRY_DEBUG) \ MACRO(DD_TRACE_BAGGAGE_MAX_ITEMS) \ MACRO(DD_TRACE_BAGGAGE_MAX_BYTES) \ - MACRO(DD_TELEMETRY_LOG_COLLECTION_ENABLED) + MACRO(DD_TELEMETRY_LOG_COLLECTION_ENABLED) \ + MACRO(DD_INSTRUMENTATION_INSTALL_ID) \ + MACRO(DD_INSTRUMENTATION_INSTALL_TYPE) \ + MACRO(DD_INSTRUMENTATION_INSTALL_TIME) #define WITH_COMMA(ARG) ARG, diff --git a/include/datadog/telemetry/configuration.h b/include/datadog/telemetry/configuration.h index 2c5e19cf..51693f26 100644 --- a/include/datadog/telemetry/configuration.h +++ b/include/datadog/telemetry/configuration.h @@ -1,10 +1,13 @@ #pragma once +#include #include #include +#include #include #include +#include namespace datadog::telemetry { @@ -38,6 +41,8 @@ struct Configuration { // Can be overriden by the `DD_TELEMETRY_LOG_COLLECTION_ENABLED` environment // variable. tracing::Optional report_logs; + // List of products reported in the `app-started` message. + std::vector products; }; struct FinalizedConfiguration { @@ -49,6 +54,13 @@ struct FinalizedConfiguration { std::chrono::steady_clock::duration heartbeat_interval; std::string integration_name; std::string integration_version; + std::vector products; + + // Onboarding metadata coming from `DD_INSTRUMENTATION_INSTALL_*` environment + // variables. + tracing::Optional install_id; + tracing::Optional install_type; + tracing::Optional install_time; friend tracing::Expected finalize_config( const Configuration&); diff --git a/include/datadog/telemetry/product.h b/include/datadog/telemetry/product.h new file mode 100644 index 00000000..e7dd7102 --- /dev/null +++ b/include/datadog/telemetry/product.h @@ -0,0 +1,57 @@ +#pragma once + +#include +#include + +#include +#include + +namespace datadog::telemetry { + +/// Represents a single product and its associated metadata. +struct Product final { + enum class Name : char { + tracing, + appsec, + profiler, + mlobs, + live_debugger, + rum, + }; + + /// The product name identifier from one of the possible values above. + Name name; + /// Flag indicating if the product is currently enabled. + bool enabled; + /// The version string of the product. + std::string version; + /// Optional error code related to the product status. + tracing::Optional error_code; + /// Optional error message related to the product status. + tracing::Optional error_message; + /// Map of configuration settings for the product. + std::unordered_map + configurations; +}; + +inline std::string_view to_string(Product::Name product) { + switch (product) { + case Product::Name::tracing: + return "tracing"; + case Product::Name::appsec: + return "appsec"; + case Product::Name::profiler: + return "profiler"; + case Product::Name::mlobs: + return "mlobs"; + case Product::Name::live_debugger: + return "live_debugger"; + case Product::Name::rum: + return "rum"; + } + + // unreachable. + return ""; +} + +} // namespace datadog::telemetry diff --git a/include/datadog/telemetry/telemetry.h b/include/datadog/telemetry/telemetry.h index 9f5fbc07..c791af79 100644 --- a/include/datadog/telemetry/telemetry.h +++ b/include/datadog/telemetry/telemetry.h @@ -19,8 +19,9 @@ namespace datadog::telemetry { /// Initialize the telemetry module -/// Once initialized, the telemetry module is running for the entier lifecycle -/// of the application. +/// Once initialized, sends a notification indicating the the application has +/// started. The telemetry module is running for the entire lifecycle of the +/// application. /// /// @param configuration The finalized configuration settings. /// @param logger User logger instance. @@ -36,21 +37,6 @@ void init(FinalizedConfiguration configuration, tracing::HTTPClient::URL agent_url, tracing::Clock clock = tracing::default_clock); -/// Sends a notification indicating that the application has started. -/// -/// This function is responsible for reporting the application has successfully -/// started. It takes a configuration map as a parameter, which contains various -/// configuration settings helping to understand how our product are used. -/// -/// @param conf A map containing configuration names and their corresponding -/// metadata. -/// -/// @note This function should be called after the application has completed its -/// initialization process to ensure that all components are aware of the -/// application's startup status. -void send_app_started(const std::unordered_map& conf); - /// Sends configuration changes. /// /// This function is responsible for sending reported configuration changes diff --git a/src/datadog/telemetry/configuration.cpp b/src/datadog/telemetry/configuration.cpp index 91c324db..cc8d2e85 100644 --- a/src/datadog/telemetry/configuration.cpp +++ b/src/datadog/telemetry/configuration.cpp @@ -122,6 +122,22 @@ tracing::Expected finalize_config( pick(env_config->integration_version, user_config.integration_version, tracing::tracer_version); + // products + result.products = user_config.products; + + // onboarding data + if (auto install_id = lookup(environment::DD_INSTRUMENTATION_INSTALL_ID)) { + result.install_id = std::string(*install_id); + } + if (auto install_type = + lookup(environment::DD_INSTRUMENTATION_INSTALL_TYPE)) { + result.install_type = std::string(*install_type); + } + if (auto install_time = + lookup(environment::DD_INSTRUMENTATION_INSTALL_TIME)) { + result.install_time = std::string(*install_time); + } + return result; } diff --git a/src/datadog/telemetry/telemetry.cpp b/src/datadog/telemetry/telemetry.cpp index 472c79ae..2c03f6be 100644 --- a/src/datadog/telemetry/telemetry.cpp +++ b/src/datadog/telemetry/telemetry.cpp @@ -61,16 +61,6 @@ void init(FinalizedConfiguration configuration, agent_url, clock}); } -void send_app_started(const std::unordered_map& conf) { - std::visit( - details::Overload{ - [&](Telemetry& telemetry) { telemetry.send_app_started(conf); }, - [](NoopTelemetry) {}, - }, - instance()); -} - void send_configuration_change() { std::visit( details::Overload{ diff --git a/src/datadog/telemetry/telemetry_impl.cpp b/src/datadog/telemetry/telemetry_impl.cpp index 184efa9d..87d01e35 100644 --- a/src/datadog/telemetry/telemetry_impl.cpp +++ b/src/datadog/telemetry/telemetry_impl.cpp @@ -163,13 +163,15 @@ Telemetry::Telemetry(FinalizedConfiguration config, "Error occurred during HTTP request for telemetry: ")); }; + send_telemetry("app-started", app_started()); schedule_tasks(); } void Telemetry::schedule_tasks() { tasks_.emplace_back(scheduler_->schedule_recurring_event( - config_.heartbeat_interval, - [this]() { send_heartbeat_and_telemetry(); })); + config_.heartbeat_interval, [this]() { + send_telemetry("app-heartbeat", heartbeat_and_telemetry()); + })); if (config_.report_metrics) { tasks_.emplace_back(scheduler_->schedule_recurring_event( @@ -183,7 +185,7 @@ Telemetry::~Telemetry() { capture_metrics(); // The app-closing message is bundled with a message containing the // final metric values. - send_app_closing(); + send_telemetry("app-closing", app_closing()); http_client_->drain(clock_().tick + 1s); } } @@ -239,7 +241,7 @@ Telemetry::Telemetry(Telemetry&& rhs) metrics_snapshots_.emplace_back(*m, MetricSnapshot{}); } - cancel_tasks(rhs.tasks_); + cancel_tasks(tasks_); schedule_tasks(); } @@ -302,8 +304,6 @@ Telemetry& Telemetry::operator=(Telemetry&& rhs) { return *this; } -// TODO(@dmehala): Move `report_logs` check in the serialization once -// `TracerTelemetry` will be removed. void Telemetry::log_error(std::string message) { if (!config_.report_logs) return; log(std::move(message), LogLevel::ERROR); @@ -340,7 +340,6 @@ void Telemetry::send_telemetry(StringView request_type, std::string payload) { } }; - // TODO(@dmehala): make `clock::instance()` a singleton auto post_result = http_client_->post( telemetry_endpoint_, set_telemetry_headers, std::move(payload), telemetry_on_response_, telemetry_on_error_, @@ -351,24 +350,24 @@ void Telemetry::send_telemetry(StringView request_type, std::string payload) { } } -void Telemetry::send_app_started( - const std::unordered_map& - config_metadata) { - send_telemetry("app-started", app_started(config_metadata)); -} - -void Telemetry::send_app_closing() { - send_telemetry("app-closing", app_closing()); -} +void Telemetry::send_configuration_change() { + if (configuration_snapshot_.empty()) return; -void Telemetry::send_heartbeat_and_telemetry() { - send_telemetry("app-heartbeat", heartbeat_and_telemetry()); -} + std::vector current_configuration; + std::swap(current_configuration, configuration_snapshot_); -void Telemetry::send_configuration_change() { - if (auto payload = configuration_change()) { - send_telemetry("app-client-configuration-change", *payload); + auto configuration_json = nlohmann::json::array(); + for (const auto& config_metadata : current_configuration) { + configuration_json.emplace_back( + generate_configuration_field(config_metadata)); } + + auto telemetry_body = + generate_telemetry_body("app-client-configuration-change"); + telemetry_body["payload"] = + nlohmann::json{{"configuration", configuration_json}}; + + send_telemetry("app-client-configuration-change", telemetry_body.dump()); } std::string Telemetry::heartbeat_and_telemetry() { @@ -523,45 +522,95 @@ std::string Telemetry::app_closing() { return message_batch_payload; } -std::string Telemetry::app_started( - const std::unordered_map& configurations) { +std::string Telemetry::app_started() { auto configuration_json = nlohmann::json::array(); - for (const auto& [_, config_metadata] : configurations) { - // if (config_metadata.value.empty()) continue; + auto product_json = nlohmann::json::object(); - configuration_json.emplace_back( - generate_configuration_field(config_metadata)); + for (const auto& product : config_.products) { + auto& configurations = product.configurations; + for (const auto& [_, config_metadata] : configurations) { + // if (config_metadata.value.empty()) continue; + + configuration_json.emplace_back( + generate_configuration_field(config_metadata)); + } + + /// NOTE(@dmehala): Telemetry API is tightly related to APM tracing and + /// assumes telemetry event can only be generated from a tracer. The + /// assumption is that the tracing product is always enabled and there + /// is no need to declare it. + if (product.name == Product::Name::tracing) continue; + + auto p = nlohmann::json{ + {to_string(product.name), + nlohmann::json{ + {"version", product.version}, + {"enabled", product.enabled}, + }}, + }; + + if (product.error_code || product.error_message) { + auto p_error = nlohmann::json{}; + if (product.error_code) { + p_error.emplace("code", *product.error_code); + } + if (product.error_message) { + p_error.emplace("message", *product.error_message); + } + + p.emplace("error", std::move(p_error)); + } + + product_json.emplace(std::move(p)); } - // clang-format off auto app_started_msg = nlohmann::json{ - {"request_type", "app-started"}, - {"payload", nlohmann::json{ - {"configuration", configuration_json} - }} + {"request_type", "app-started"}, + { + "payload", + nlohmann::json{ + {"configuration", configuration_json}, + {"products", product_json}, + }, + }, }; + if (config_.install_id || config_.install_time || config_.install_type) { + auto install_signature = nlohmann::json{}; + if (config_.install_id) { + install_signature.emplace("install_id", *config_.install_id); + } + if (config_.install_type) { + install_signature.emplace("install_type", *config_.install_type); + } + if (config_.install_time) { + install_signature.emplace("install_time", *config_.install_time); + } + + app_started_msg["payload"].emplace("install_signature", + std::move(install_signature)); + } + auto batch = generate_telemetry_body("message-batch"); - batch["payload"] = nlohmann::json::array({ - std::move(app_started_msg) - }); - // clang-format on + batch["payload"] = nlohmann::json::array({std::move(app_started_msg)}); if (!config_.integration_name.empty()) { - // clang-format off auto integration_msg = nlohmann::json{ - {"request_type", "app-integrations-change"}, - {"payload", nlohmann::json{ - {"integrations", nlohmann::json::array({ - nlohmann::json{ - {"name", config_.integration_name}, - {"version", config_.integration_version}, - {"enabled", true} - } - })} - }} + {"request_type", "app-integrations-change"}, + { + "payload", + nlohmann::json{ + { + "integrations", + nlohmann::json::array({ + nlohmann::json{{"name", config_.integration_name}, + {"version", config_.integration_version}, + {"enabled", true}}, + }), + }, + }, + }, }; - // clang-format on batch["payload"].emplace_back(std::move(integration_msg)); } @@ -569,26 +618,6 @@ std::string Telemetry::app_started( return batch.dump(); } -Optional Telemetry::configuration_change() { - if (configuration_snapshot_.empty()) return nullopt; - - std::vector current_configuration; - std::swap(current_configuration, configuration_snapshot_); - - auto configuration_json = nlohmann::json::array(); - for (const auto& config_metadata : current_configuration) { - configuration_json.emplace_back( - generate_configuration_field(config_metadata)); - } - - auto telemetry_body = - generate_telemetry_body("app-client-configuration-change"); - telemetry_body["payload"] = - nlohmann::json{{"configuration", configuration_json}}; - - return telemetry_body.dump(); -} - nlohmann::json Telemetry::generate_telemetry_body(std::string request_type) { std::time_t tracer_time = std::chrono::duration_cast( clock_().wall.time_since_epoch()) diff --git a/src/datadog/telemetry/telemetry_impl.h b/src/datadog/telemetry/telemetry_impl.h index 4d98305a..33a4d586 100644 --- a/src/datadog/telemetry/telemetry_impl.h +++ b/src/datadog/telemetry/telemetry_impl.h @@ -90,21 +90,14 @@ class Telemetry final { /// @param message The warning message to log. void log_warning(std::string message); - void send_app_started( - const std::unordered_map& - config_metadata); - void send_configuration_change(); void capture_configuration_change( const std::vector& new_configuration); - void send_app_closing(); - private: void send_telemetry(tracing::StringView request_type, std::string payload); - void send_heartbeat_and_telemetry(); void schedule_tasks(); void capture_metrics(); @@ -112,17 +105,13 @@ class Telemetry final { void log(std::string message, telemetry::LogLevel level, tracing::Optional stacktrace = tracing::nullopt); - tracing::Optional configuration_change(); - nlohmann::json generate_telemetry_body(std::string request_type); nlohmann::json generate_configuration_field( const tracing::ConfigMetadata& config_metadata); // Constructs an `app-started` message using information provided when // constructed and the tracer_config value passed in. - std::string app_started( - const std::unordered_map& - configurations); + std::string app_started(); // Constructs a messsage-batch containing `app-heartbeat`, and if metrics // have been modified, a `generate-metrics` message. std::string heartbeat_and_telemetry(); diff --git a/src/datadog/tracer.cpp b/src/datadog/tracer.cpp index 6023924b..dac30f13 100644 --- a/src/datadog/tracer.cpp +++ b/src/datadog/tracer.cpp @@ -76,8 +76,6 @@ Tracer::Tracer(const FinalizedTracerConfig& config, auto agent = std::make_shared(agent_config, config.logger, signature_, rc_listeners); collector_ = agent; - - telemetry::send_app_started(config.metadata); } for (const auto style : extraction_styles_) { diff --git a/src/datadog/tracer_config.cpp b/src/datadog/tracer_config.cpp index 9b145004..629cd130 100644 --- a/src/datadog/tracer_config.cpp +++ b/src/datadog/tracer_config.cpp @@ -425,13 +425,6 @@ Expected finalize_config(const TracerConfig &user_config, return std::move(span_sampler_config.error()); } - if (auto telemetry_final_config = - telemetry::finalize_config(user_config.telemetry)) { - final_config.telemetry = std::move(*telemetry_final_config); - } else { - return std::move(telemetry_final_config.error()); - } - // agent url final_config.agent_url = agent_finalized->url; @@ -443,6 +436,17 @@ Expected finalize_config(const TracerConfig &user_config, final_config.http_client = agent_finalized->http_client; + // telemetry + if (auto telemetry_final_config = + telemetry::finalize_config(user_config.telemetry)) { + final_config.telemetry = std::move(*telemetry_final_config); + final_config.telemetry.products.emplace_back(telemetry::Product{ + telemetry::Product::Name::tracing, true, tracer_version, nullopt, + nullopt, final_config.metadata}); + } else { + return std::move(telemetry_final_config.error()); + } + return final_config; } diff --git a/test/telemetry/test_configuration.cpp b/test/telemetry/test_configuration.cpp index 58380db8..24373e38 100644 --- a/test/telemetry/test_configuration.cpp +++ b/test/telemetry/test_configuration.cpp @@ -4,14 +4,14 @@ #include "../common/environment.h" #include "../test.h" -#define TELEMETRY_CONFIGURATION_TEST(x) \ - TEST_CASE(x, "[telemetry.configuration]") - namespace ddtest = datadog::test; using namespace datadog; using namespace std::literals; +#define TELEMETRY_CONFIGURATION_TEST(x) \ + TEST_CASE(x, "[telemetry],[telemetry.configuration]") + TELEMETRY_CONFIGURATION_TEST("defaults") { const auto cfg = telemetry::finalize_config(); REQUIRE(cfg); @@ -21,6 +21,9 @@ TELEMETRY_CONFIGURATION_TEST("defaults") { CHECK(cfg->report_metrics == true); CHECK(cfg->metrics_interval == 60s); CHECK(cfg->heartbeat_interval == 10s); + CHECK(cfg->install_id.has_value() == false); + CHECK(cfg->install_type.has_value() == false); + CHECK(cfg->install_time.has_value() == false); } TELEMETRY_CONFIGURATION_TEST("code override") { @@ -145,3 +148,21 @@ TELEMETRY_CONFIGURATION_TEST("validation") { } } } + +TELEMETRY_CONFIGURATION_TEST("installation infos are used when available") { + ddtest::EnvGuard install_id_env("DD_INSTRUMENTATION_INSTALL_ID", "1-2-3-4"); + ddtest::EnvGuard install_type_env("DD_INSTRUMENTATION_INSTALL_TYPE", "ssi"); + ddtest::EnvGuard install_time_env("DD_INSTRUMENTATION_INSTALL_TIME", "now"); + + const auto final_cfg = telemetry::finalize_config(); + REQUIRE(final_cfg); + + REQUIRE(final_cfg->install_id.has_value()); + CHECK(final_cfg->install_id.value() == "1-2-3-4"); + + REQUIRE(final_cfg->install_type.has_value()); + CHECK(final_cfg->install_type.value() == "ssi"); + + REQUIRE(final_cfg->install_time.has_value()); + CHECK(final_cfg->install_time.value() == "now"); +} diff --git a/test/telemetry/test_metrics.cpp b/test/telemetry/test_metrics.cpp index 9c0c8ca0..3078c084 100644 --- a/test/telemetry/test_metrics.cpp +++ b/test/telemetry/test_metrics.cpp @@ -6,7 +6,10 @@ using namespace datadog::telemetry; -TEST_CASE("Counter metrics", "[telemetry.metrics]") { +#define TELEMETRY_METRICS_TEST(x) \ + TEST_CASE(x, "[telemetry],[telemetry.metrics]") + +TELEMETRY_METRICS_TEST("Counter metrics") { CounterMetric metric = { "test.counter.metric", "test_scope", {"testing-testing:123"}, true}; @@ -18,7 +21,7 @@ TEST_CASE("Counter metrics", "[telemetry.metrics]") { REQUIRE(metric.value() == 0); } -TEST_CASE("Gauge metrics", "[telemetry.metrics]") { +TELEMETRY_METRICS_TEST("Gauge metrics") { GaugeMetric metric = { "test.gauge.metric", "test_scope", {"testing-testing:123"}, true}; metric.set(40); diff --git a/test/telemetry/test_telemetry.cpp b/test/telemetry/test_telemetry.cpp index d76da491..f3795475 100644 --- a/test/telemetry/test_telemetry.cpp +++ b/test/telemetry/test_telemetry.cpp @@ -8,12 +8,14 @@ #include #include +#include "../common/environment.h" #include "datadog/runtime_id.h" #include "datadog/telemetry/telemetry_impl.h" #include "mocks/http_clients.h" #include "mocks/loggers.h" #include "test.h" +namespace ddtest = datadog::test; using namespace datadog::tracing; using namespace datadog::telemetry; using namespace std::chrono_literals; @@ -70,14 +72,10 @@ struct FakeEventScheduler : public EventScheduler { } // namespace -TEST_CASE("Tracer telemetry", "[telemetry]") { - const std::time_t mock_time = 1672484400; - const Clock clock = [&mock_time]() { - TimePoint result; - result.wall = std::chrono::system_clock::from_time_t(mock_time); - return result; - }; +#define TELEMETRY_IMPLEMENTATION_TEST(x) \ + TEST_CASE(x, "[telemetry],[telemetry.impl]") +TELEMETRY_IMPLEMENTATION_TEST("Tracer telemetry lifecycle") { auto logger = std::make_shared(); auto client = std::make_shared(); auto scheduler = std::make_shared(); @@ -88,21 +86,18 @@ TEST_CASE("Tracer telemetry", "[telemetry]") { /* environment = */ "test"}; auto url = HTTPClient::URL::parse("http://localhost:8000"); - Telemetry telemetry{*finalize_config(), - logger, - client, - std::vector>{}, - scheduler, - *url, - clock}; - SECTION("generates app-started message") { + SECTION("ctor send app-started message") { SECTION("Without a defined integration") { + Telemetry telemetry{*finalize_config(), + logger, + client, + std::vector>{}, + scheduler, + *url}; /// By default the integration is `datadog` with the tracer version. /// TODO: remove the default because these datadog are already part of the /// request header. - telemetry.send_app_started({}); - auto app_started = nlohmann::json::parse(client->request_body); REQUIRE(is_valid_telemetry_payload(app_started) == true); REQUIRE(app_started["request_type"] == "message-batch"); @@ -114,6 +109,8 @@ TEST_CASE("Tracer telemetry", "[telemetry]") { } SECTION("With an integration") { + client->clear(); + Configuration cfg; cfg.integration_name = "nginx"; cfg.integration_version = "1.25.2"; @@ -124,9 +121,6 @@ TEST_CASE("Tracer telemetry", "[telemetry]") { scheduler, *url}; - client->clear(); - telemetry2.send_app_started({}); - auto app_started = nlohmann::json::parse(client->request_body); REQUIRE(is_valid_telemetry_payload(app_started) == true); REQUIRE(app_started["request_type"] == "message-batch"); @@ -140,14 +134,69 @@ TEST_CASE("Tracer telemetry", "[telemetry]") { } } + SECTION("With installation signature") { + client->clear(); + + ddtest::EnvGuard install_id_env("DD_INSTRUMENTATION_INSTALL_ID", + "68e75c48-57ca-4a12-adfc-575c4b05fcbe"); + ddtest::EnvGuard install_type_env("DD_INSTRUMENTATION_INSTALL_TYPE", + "k8s_single_step"); + ddtest::EnvGuard install_time_env("DD_INSTRUMENTATION_INSTALL_TIME", + "1703188212"); + + Telemetry telemetry4{*finalize_config(), + logger, + client, + std::vector>{}, + scheduler, + *url}; + + auto app_started = nlohmann::json::parse(client->request_body); + REQUIRE(is_valid_telemetry_payload(app_started) == true); + REQUIRE(app_started["request_type"] == "message-batch"); + REQUIRE(app_started["payload"].is_array()); + REQUIRE(app_started["payload"].size() == 2); + + auto& app_started_payload = app_started["payload"][0]; + CHECK(app_started_payload["request_type"] == "app-started"); + + auto install_payload = + app_started_payload["payload"]["install_signature"]; + REQUIRE(install_payload.is_object()); + + REQUIRE(install_payload.contains("install_id") == true); + CHECK(install_payload["install_id"] == + "68e75c48-57ca-4a12-adfc-575c4b05fcbe"); + + REQUIRE(install_payload.contains("install_id") == true); + CHECK(install_payload["install_type"] == "k8s_single_step"); + + REQUIRE(install_payload.contains("install_id") == true); + CHECK(install_payload["install_time"] == "1703188212"); + } + SECTION("With configuration") { - std::unordered_map configuration{ + client->clear(); + + Product product; + product.name = Product::Name::tracing; + product.enabled = true; + product.version = tracer_version; + product.configurations = std::unordered_map{ {ConfigName::SERVICE_NAME, ConfigMetadata(ConfigName::SERVICE_NAME, "foo", - ConfigMetadata::Origin::CODE)}}; + ConfigMetadata::Origin::CODE)}, + }; - client->clear(); - telemetry.send_app_started(configuration); + Configuration cfg; + cfg.products.emplace_back(std::move(product)); + + Telemetry telemetry3{*finalize_config(cfg), + logger, + client, + std::vector>{}, + scheduler, + *url}; auto app_started = nlohmann::json::parse(client->request_body); REQUIRE(is_valid_telemetry_payload(app_started) == true); @@ -176,7 +225,7 @@ TEST_CASE("Tracer telemetry", "[telemetry]") { SECTION("generates a configuration change event") { SECTION("empty configuration do not generate a valid payload") { client->clear(); - telemetry.send_configuration_change(); + telemetry3.send_configuration_change(); CHECK(client->request_body.empty()); } @@ -189,8 +238,8 @@ TEST_CASE("Tracer telemetry", "[telemetry]") { Error{Error::Code::OTHER, "empty field"}}}; client->clear(); - telemetry.capture_configuration_change(new_config); - telemetry.send_configuration_change(); + telemetry3.capture_configuration_change(new_config); + telemetry3.send_configuration_change(); auto updates = client->request_body; REQUIRE(!updates.empty()); @@ -241,13 +290,59 @@ TEST_CASE("Tracer telemetry", "[telemetry]") { // No update -> no configuration update client->clear(); - telemetry.send_configuration_change(); + telemetry3.send_configuration_change(); CHECK(client->request_body.empty()); } } } } + SECTION("dtor send app-closing message") { + { + Telemetry telemetry{*finalize_config(), + logger, + client, + std::vector>{}, + scheduler, + *url}; + client->clear(); + } + + auto message_batch = nlohmann::json::parse(client->request_body); + REQUIRE(is_valid_telemetry_payload(message_batch) == true); + REQUIRE(message_batch["payload"].size() == 1); + auto heartbeat = message_batch["payload"][0]; + REQUIRE(heartbeat["request_type"] == "app-closing"); + } +} + +TELEMETRY_IMPLEMENTATION_TEST("Tracer telemetry API") { + const std::time_t mock_time = 1672484400; + const Clock clock = [&mock_time]() { + TimePoint result; + result.wall = std::chrono::system_clock::from_time_t(mock_time); + return result; + }; + + auto logger = std::make_shared(); + auto client = std::make_shared(); + auto scheduler = std::make_shared(); + + const TracerSignature tracer_signature{ + /* runtime_id = */ RuntimeID::generate(), + /* service = */ "testsvc", + /* environment = */ "test"}; + + auto url = HTTPClient::URL::parse("http://localhost:8000"); + + Telemetry telemetry{*finalize_config(), + logger, + client, + std::vector>{}, + scheduler, + *url, + clock}; + SECTION("generates a heartbeat message") { client->clear(); scheduler->trigger_heartbeat(); @@ -288,17 +383,6 @@ TEST_CASE("Tracer telemetry", "[telemetry]") { REQUIRE(points[0][1] == 1); } - SECTION("generates an app-closing event") { - client->clear(); - telemetry.send_app_closing(); - - auto message_batch = nlohmann::json::parse(client->request_body); - REQUIRE(is_valid_telemetry_payload(message_batch) == true); - REQUIRE(message_batch["payload"].size() == 1); - auto heartbeat = message_batch["payload"][0]; - REQUIRE(heartbeat["request_type"] == "app-closing"); - } - SECTION("logs serialization") { SECTION("log level is correct") { struct TestCase { @@ -370,7 +454,7 @@ TEST_CASE("Tracer telemetry", "[telemetry]") { } } -TEST_CASE("Tracer telemetry configuration", "[telemetry]") { +TELEMETRY_IMPLEMENTATION_TEST("Tracer telemetry configuration") { // Cases: // - when `report_metrics` is set to false. No metrics are reported. // - when `report_logs` is set to false. No logs are reported. From 1f813e19ba444fa1dece0754508acaae653e68a0 Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Tue, 8 Apr 2025 17:11:43 +0200 Subject: [PATCH 2/3] fix bug --- include/datadog/telemetry/telemetry.h | 1 - src/datadog/telemetry/telemetry_impl.cpp | 11 ++++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/datadog/telemetry/telemetry.h b/include/datadog/telemetry/telemetry.h index c791af79..a2a887ba 100644 --- a/include/datadog/telemetry/telemetry.h +++ b/include/datadog/telemetry/telemetry.h @@ -9,7 +9,6 @@ #include #include -#include #include /// Telemetry functions are responsibles for handling internal telemetry data to diff --git a/src/datadog/telemetry/telemetry_impl.cpp b/src/datadog/telemetry/telemetry_impl.cpp index 87d01e35..ad6ec9c5 100644 --- a/src/datadog/telemetry/telemetry_impl.cpp +++ b/src/datadog/telemetry/telemetry_impl.cpp @@ -205,6 +205,8 @@ Telemetry::Telemetry(Telemetry&& rhs) seq_id_(rhs.seq_id_), config_seq_ids_(rhs.config_seq_ids_), host_info_(rhs.host_info_) { + cancel_tasks(rhs.tasks_); + // Register all the metrics that we're tracking by adding them to the // metrics_snapshots_ container. This allows for simpler iteration logic // when using the values in `generate-metrics` messages. @@ -241,12 +243,13 @@ Telemetry::Telemetry(Telemetry&& rhs) metrics_snapshots_.emplace_back(*m, MetricSnapshot{}); } - cancel_tasks(tasks_); schedule_tasks(); } Telemetry& Telemetry::operator=(Telemetry&& rhs) { if (&rhs != this) { + cancel_tasks(rhs.tasks_); + std::swap(metrics_, rhs.metrics_); std::swap(config_, rhs.config_); std::swap(logger_, rhs.logger_); @@ -298,7 +301,6 @@ Telemetry& Telemetry::operator=(Telemetry&& rhs) { metrics_snapshots_.emplace_back(*m, MetricSnapshot{}); } - cancel_tasks(rhs.tasks_); schedule_tasks(); } return *this; @@ -342,8 +344,7 @@ void Telemetry::send_telemetry(StringView request_type, std::string payload) { auto post_result = http_client_->post( telemetry_endpoint_, set_telemetry_headers, std::move(payload), - telemetry_on_response_, telemetry_on_error_, - tracing::default_clock().tick + 5s); + telemetry_on_response_, telemetry_on_error_, clock_().tick + 5s); if (auto* error = post_result.if_error()) { logger_->log_error( error->with_prefix("Unexpected error submitting telemetry event: ")); @@ -517,8 +518,8 @@ std::string Telemetry::app_closing() { auto telemetry_body = generate_telemetry_body("message-batch"); telemetry_body["payload"] = batch_payloads; - auto message_batch_payload = telemetry_body.dump(); + auto message_batch_payload = telemetry_body.dump(); return message_batch_payload; } From 2a257ea0bd737f1ca1bc98319e4a8a2f71e1f4ac Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Thu, 10 Apr 2025 14:10:17 +0200 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: pablomartinezbernardo <134320516+pablomartinezbernardo@users.noreply.github.com> --- include/datadog/telemetry/telemetry.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/datadog/telemetry/telemetry.h b/include/datadog/telemetry/telemetry.h index a2a887ba..b30f696f 100644 --- a/include/datadog/telemetry/telemetry.h +++ b/include/datadog/telemetry/telemetry.h @@ -18,7 +18,7 @@ namespace datadog::telemetry { /// Initialize the telemetry module -/// Once initialized, sends a notification indicating the the application has +/// Once initialized, sends a notification indicating that the application has /// started. The telemetry module is running for the entire lifecycle of the /// application. ///