fix(telemetry): Handle empty configuration payload properly (#186)

This fix addresses two issues: 1. Prevents invalid telemetry payloads when there are no configuration entries. 2. Ensures configuration updates are correctly processed and not carried over to the next update.
DataDog · Feb 14, 2025 · 62a9e91 · 62a9e91
1 parent ccbbae9
commit 62a9e91
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 30 deletions.
diff --git a/src/datadog/datadog_agent.cpp b/src/datadog/datadog_agent.cpp
@@ -414,8 +414,9 @@ void DatadogAgent::send_heartbeat_and_telemetry() {
 }
 
 void DatadogAgent::send_configuration_change() {
-  send_telemetry("app-client-configuration-change",
-                 tracer_telemetry_->configuration_change());
+  if (auto payload = tracer_telemetry_->configuration_change()) {
+    send_telemetry("app-client-configuration-change", *payload);
+  }
 }
 
 void DatadogAgent::send_app_closing() {

diff --git a/src/datadog/tracer_telemetry.cpp b/src/datadog/tracer_telemetry.cpp
@@ -355,10 +355,14 @@ std::string TracerTelemetry::app_closing() {
   return message_batch_payload;
 }
 
-std::string TracerTelemetry::configuration_change() {
+Optional<std::string> TracerTelemetry::configuration_change() {
+  if (configuration_snapshot_.empty()) return nullopt;
+
+  std::vector<ConfigMetadata> current_configuration;
+  std::swap(current_configuration, configuration_snapshot_);
+
   auto configuration_json = nlohmann::json::array();
-  for (const auto& config_metadata : configuration_snapshot_) {
-    // if (config_metadata.value.empty()) continue;
+  for (const auto& config_metadata : current_configuration) {
     configuration_json.emplace_back(
         generate_configuration_field(config_metadata));
   }

diff --git a/src/datadog/tracer_telemetry.h b/src/datadog/tracer_telemetry.h
@@ -150,7 +150,7 @@ class TracerTelemetry {
   // been modified, a `generate-metrics` message.
   std::string app_closing();
   // Construct an `app-client-configuration-change` message.
-  std::string configuration_change();
+  Optional<std::string> configuration_change();
 };
 
 }  // namespace tracing

diff --git a/test/test_tracer_telemetry.cpp b/test/test_tracer_telemetry.cpp
@@ -110,16 +110,9 @@ TEST_CASE("Tracer telemetry", "[telemetry]") {
       CHECK(cfg_payload[0] == expected_conf);
 
       SECTION("generates a configuration change event") {
-        SECTION("empty configuration generate a valid payload") {
-          auto config_change_message = nlohmann::json::parse(
-              tracer_telemetry.configuration_change(), nullptr, false);
-          REQUIRE(config_change_message.is_discarded() == false);
-          REQUIRE(is_valid_telemetry_payload(app_started) == true);
-
-          CHECK(config_change_message["request_type"] ==
-                "app-client-configuration-change");
-          CHECK(config_change_message["payload"]["configuration"].is_array());
-          CHECK(config_change_message["payload"]["configuration"].empty());
+        SECTION("empty configuration do not generate a valid payload") {
+          auto config_update = tracer_telemetry.configuration_change();
+          CHECK(!config_update);
         }
 
         SECTION("valid configurations update") {
@@ -130,8 +123,10 @@ TEST_CASE("Tracer telemetry", "[telemetry]") {
                Error{Error::Code::OTHER, "empty field"}}};
 
           tracer_telemetry.capture_configuration_change(new_config);
-          auto config_change_message = nlohmann::json::parse(
-              tracer_telemetry.configuration_change(), nullptr, false);
+          auto updates = tracer_telemetry.configuration_change();
+          REQUIRE(updates);
+          auto config_change_message =
+              nlohmann::json::parse(*updates, nullptr, false);
           REQUIRE(config_change_message.is_discarded() == false);
           REQUIRE(is_valid_telemetry_payload(config_change_message) == true);
 
@@ -141,25 +136,42 @@ TEST_CASE("Tracer telemetry", "[telemetry]") {
           CHECK(config_change_message["payload"]["configuration"].size() == 2);
 
           const std::unordered_map<std::string, nlohmann::json> expected_json{
-              {"service", nlohmann::json{{"name", "service"},
-                                         {"value", "increase seq_id"},
-                                         {"seq_id", 2},
-                                         {"origin", "env_var"}}},
-              {"trace_enabled",
-               nlohmann::json{{"name", "trace_enabled"},
-                              {"value", ""},
-                              {"seq_id", 1},
-                              {"origin", "default"},
-                              {"error",
-                               {{"code", Error::Code::OTHER},
-                                {"message", "empty field"}}}}}};
+              {
+                  "service",
+                  nlohmann::json{
+                      {"name", "service"},
+                      {"value", "increase seq_id"},
+                      {"seq_id", 2},
+                      {"origin", "env_var"},
+                  },
+              },
+              {
+                  "trace_enabled",
+                  nlohmann::json{
+                      {"name", "trace_enabled"},
+                      {"value", ""},
+                      {"seq_id", 1},
+                      {"origin", "default"},
+                      {
+                          "error",
+                          {
+                              {"code", Error::Code::OTHER},
+                              {"message", "empty field"},
+                          },
+                      },
+                  },
+              },
+          };
 
           for (const auto& conf :
                config_change_message["payload"]["configuration"]) {
             auto expected_conf = expected_json.find(conf["name"]);
             REQUIRE(expected_conf != expected_json.cend());
             CHECK(expected_conf->second == conf);
           }
+
+          // No update -> no configuration update
+          CHECK(tracer_telemetry.configuration_change() == nullopt);
         }
       }
     }