From 0c1a8c53b05564459d95e76a48b7f1915a404203 Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Sat, 5 Apr 2025 13:21:43 +0200 Subject: [PATCH 1/4] [part 5] refactor!(telemetry): use configured intervals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, telemetry metrics were collected every 10 seconds and heartbeats were sent every 60 seconds, matching the default configuration. However, these intervals can be customized via environment variables or code. This commit updates the telemetry logic to use the configured intervals—whether set through environment variables or directly in the code—ensuring that metrics , heartbeat and logs messages are sent accordingly. --- src/datadog/telemetry/telemetry_impl.cpp | 24 +++-- test/telemetry/test_telemetry.cpp | 131 +++++++++++++++++++---- 2 files changed, 125 insertions(+), 30 deletions(-) diff --git a/src/datadog/telemetry/telemetry_impl.cpp b/src/datadog/telemetry/telemetry_impl.cpp index 7eee29e1..f6fc457d 100644 --- a/src/datadog/telemetry/telemetry_impl.cpp +++ b/src/datadog/telemetry/telemetry_impl.cpp @@ -94,18 +94,15 @@ Telemetry::Telemetry(FinalizedConfiguration config, } void Telemetry::schedule_tasks() { - // Only schedule this if telemetry is enabled. - // Every 10 seconds, have the tracer telemetry capture the metrics - // values. Every 60 seconds, also report those values to the datadog - // agent. tasks_.emplace_back(scheduler_->schedule_recurring_event( - std::chrono::seconds(10), [this, n = 0]() mutable { - n++; - tracer_telemetry_->capture_metrics(); - if (n % 6 == 0) { - send_heartbeat_and_telemetry(); - } - })); + config_.heartbeat_interval, + [this]() { send_heartbeat_and_telemetry(); })); + + if (config_.report_metrics) { + tasks_.emplace_back(scheduler_->schedule_recurring_event( + config_.metrics_interval, + [this]() mutable { tracer_telemetry_->capture_metrics(); })); + } } Telemetry::~Telemetry() { @@ -197,15 +194,20 @@ 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; tracer_telemetry_->log(std::move(message), LogLevel::ERROR); } void Telemetry::log_error(std::string message, std::string stacktrace) { + if (!config_.report_logs) return; tracer_telemetry_->log(std::move(message), LogLevel::ERROR, stacktrace); } void Telemetry::log_warning(std::string message) { + if (!config_.report_logs) return; tracer_telemetry_->log(std::move(message), LogLevel::WARNING); } diff --git a/test/telemetry/test_telemetry.cpp b/test/telemetry/test_telemetry.cpp index 8d81c13f..3d0d5369 100644 --- a/test/telemetry/test_telemetry.cpp +++ b/test/telemetry/test_telemetry.cpp @@ -10,13 +10,13 @@ #include "datadog/runtime_id.h" #include "datadog/telemetry/telemetry_impl.h" -#include "mocks/event_schedulers.h" #include "mocks/http_clients.h" #include "mocks/loggers.h" #include "test.h" using namespace datadog::tracing; using namespace datadog::telemetry; +using namespace std::chrono_literals; namespace { bool is_valid_telemetry_payload(const nlohmann::json& json) { @@ -31,6 +31,43 @@ bool is_valid_telemetry_payload(const nlohmann::json& json) { json.contains("/host"_json_pointer); } +struct FakeEventScehduler : public EventScheduler { + size_t count_tasks = 0; + std::function heartbeat_callback = nullptr; + std::function metrics_callback = nullptr; + Optional heartbeat_interval; + Optional metrics_interval; + bool cancelled = false; + + // NOTE: White box testing. This is a limitation of the event scheduler API. + Cancel schedule_recurring_event(std::chrono::steady_clock::duration interval, + std::function callback) override { + if (count_tasks == 0) { + heartbeat_callback = callback; + heartbeat_interval = interval; + } else if (count_tasks == 1) { + metrics_callback = callback; + metrics_interval = interval; + } + count_tasks++; + return [this]() { cancelled = true; }; + } + + void trigger_heartbeat() { + assert(heartbeat_callback != nullptr); + heartbeat_callback(); + } + + void trigger_metrics_capture() { + assert(metrics_callback != nullptr); + metrics_callback(); + } + + std::string config() const override { + return nlohmann::json::object({{"type", "FakeEventScheduler"}}).dump(); + } +}; + } // namespace TEST_CASE("Tracer telemetry", "[telemetry]") { @@ -43,27 +80,13 @@ TEST_CASE("Tracer telemetry", "[telemetry]") { auto logger = std::make_shared(); auto client = std::make_shared(); - auto scheduler = std::make_shared(); - - auto trigger_heartbeat = [&]() { - // White box testing. The current implementation send a heartbeat every 60s - // and the task is executed every 10s. - // TODO(@dmehala): should depends on the config - scheduler->event_callback(); - scheduler->event_callback(); - scheduler->event_callback(); - scheduler->event_callback(); - scheduler->event_callback(); - scheduler->event_callback(); - }; + auto scheduler = std::make_shared(); const TracerSignature tracer_signature{ /* runtime_id = */ RuntimeID::generate(), /* service = */ "testsvc", /* environment = */ "test"}; - const std::string ignore{""}; - auto url = HTTPClient::URL::parse("http://localhost:8000"); Telemetry telemetry{*finalize_config(), logger, @@ -227,7 +250,7 @@ TEST_CASE("Tracer telemetry", "[telemetry]") { SECTION("generates a heartbeat message") { client->clear(); - trigger_heartbeat(); + scheduler->trigger_heartbeat(); auto heartbeat_message = client->request_body; auto message_batch = nlohmann::json::parse(heartbeat_message); @@ -240,7 +263,8 @@ TEST_CASE("Tracer telemetry", "[telemetry]") { SECTION("captures metrics and sends generate-metrics payload") { telemetry.metrics().tracer.trace_segments_created_new.inc(); REQUIRE(telemetry.metrics().tracer.trace_segments_created_new.value() == 1); - trigger_heartbeat(); + scheduler->trigger_metrics_capture(); + scheduler->trigger_heartbeat(); REQUIRE(telemetry.metrics().tracer.trace_segments_created_new.value() == 0); @@ -322,7 +346,7 @@ TEST_CASE("Tracer telemetry", "[telemetry]") { client->clear(); test_case.apply(telemetry, test_case.input, test_case.stacktrace); - trigger_heartbeat(); + scheduler->trigger_heartbeat(); auto message_batch = nlohmann::json::parse(client->request_body); REQUIRE(is_valid_telemetry_payload(message_batch)); @@ -345,3 +369,72 @@ TEST_CASE("Tracer telemetry", "[telemetry]") { } } } + +TEST_CASE("Tracer telemetry configuration", "[telemetry]") { + // Cases: + // - when `report_metrics` is set to false. No metrics are reported. + // - when `report_logs` is set to false. No logs are reported. + // - respects interval defined. + // - telemetry disabled doesn't send anything. + + auto logger = std::make_shared(); + auto client = std::make_shared(); + auto scheduler = std::make_shared(); + std::vector> metrics; + + const TracerSignature tracer_signature{ + /* runtime_id = */ RuntimeID::generate(), + /* service = */ "testsvc", + /* environment = */ "test"}; + + auto url = HTTPClient::URL::parse("http://localhost:8000"); + + SECTION("disabling metrics reporting do not collect metrics") { + Configuration cfg; + cfg.report_metrics = false; + + auto final_cfg = finalize_config(cfg); + REQUIRE(final_cfg); + + Telemetry telemetry(*final_cfg, logger, client, metrics, scheduler, *url); + CHECK(scheduler->metrics_callback == nullptr); + CHECK(scheduler->metrics_interval == nullopt); + } + + SECTION("intervals are respected") { + Configuration cfg; + cfg.metrics_interval_seconds = .5; + cfg.heartbeat_interval_seconds = 30; + + auto final_cfg = finalize_config(cfg); + REQUIRE(final_cfg); + + Telemetry telemetry(*final_cfg, logger, client, metrics, scheduler, *url); + CHECK(scheduler->metrics_callback != nullptr); + CHECK(scheduler->metrics_interval == 500ms); + + CHECK(scheduler->heartbeat_callback != nullptr); + CHECK(scheduler->metrics_interval != 30s); + } + + SECTION("disabling logs reporting do not collect logs") { + client->clear(); + + Configuration cfg; + cfg.report_logs = false; + + auto final_cfg = finalize_config(cfg); + REQUIRE(final_cfg); + + Telemetry telemetry(*final_cfg, logger, client, metrics, scheduler, *url); + telemetry.log_error("error"); + + // NOTE(@dmehala): logs are sent with an heartbeat. + scheduler->trigger_heartbeat(); + + auto message_batch = nlohmann::json::parse(client->request_body); + REQUIRE(is_valid_telemetry_payload(message_batch)); + REQUIRE(message_batch["payload"].size() == 1); + CHECK(message_batch["payload"][0]["request_type"] == "app-heartbeat"); + } +} From 26762092a18eb4e11dd52e4a092f766709486b74 Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Mon, 7 Apr 2025 09:20:35 +0200 Subject: [PATCH 2/4] Apply suggestions from code review --- src/datadog/telemetry/telemetry_impl.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/datadog/telemetry/telemetry_impl.cpp b/src/datadog/telemetry/telemetry_impl.cpp index f6fc457d..db87e888 100644 --- a/src/datadog/telemetry/telemetry_impl.cpp +++ b/src/datadog/telemetry/telemetry_impl.cpp @@ -194,8 +194,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; tracer_telemetry_->log(std::move(message), LogLevel::ERROR); From f3dbc998b45b06eb4473e3acafec347af2ede348 Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Mon, 7 Apr 2025 17:13:53 +0200 Subject: [PATCH 3/4] Apply suggestions from code review --- test/telemetry/test_telemetry.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/telemetry/test_telemetry.cpp b/test/telemetry/test_telemetry.cpp index 3d0d5369..224c5034 100644 --- a/test/telemetry/test_telemetry.cpp +++ b/test/telemetry/test_telemetry.cpp @@ -31,7 +31,7 @@ bool is_valid_telemetry_payload(const nlohmann::json& json) { json.contains("/host"_json_pointer); } -struct FakeEventScehduler : public EventScheduler { +struct FakeEventScheduler : public EventScheduler { size_t count_tasks = 0; std::function heartbeat_callback = nullptr; std::function metrics_callback = nullptr; @@ -80,7 +80,7 @@ TEST_CASE("Tracer telemetry", "[telemetry]") { auto logger = std::make_shared(); auto client = std::make_shared(); - auto scheduler = std::make_shared(); + auto scheduler = std::make_shared(); const TracerSignature tracer_signature{ /* runtime_id = */ RuntimeID::generate(), From a0a29c30ec8efe20b51f83a6bfcb229e70a34437 Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Mon, 7 Apr 2025 17:35:02 +0200 Subject: [PATCH 4/4] Apply suggestions from code review --- test/telemetry/test_telemetry.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/telemetry/test_telemetry.cpp b/test/telemetry/test_telemetry.cpp index 224c5034..d76da491 100644 --- a/test/telemetry/test_telemetry.cpp +++ b/test/telemetry/test_telemetry.cpp @@ -379,7 +379,7 @@ TEST_CASE("Tracer telemetry configuration", "[telemetry]") { auto logger = std::make_shared(); auto client = std::make_shared(); - auto scheduler = std::make_shared(); + auto scheduler = std::make_shared(); std::vector> metrics; const TracerSignature tracer_signature{