Skip to content

Commit 62a9e91

Browse files
authored
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.
1 parent ccbbae9 commit 62a9e91

4 files changed

Lines changed: 47 additions & 30 deletions

File tree

src/datadog/datadog_agent.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -414,8 +414,9 @@ void DatadogAgent::send_heartbeat_and_telemetry() {
414414
}
415415

416416
void DatadogAgent::send_configuration_change() {
417-
send_telemetry("app-client-configuration-change",
418-
tracer_telemetry_->configuration_change());
417+
if (auto payload = tracer_telemetry_->configuration_change()) {
418+
send_telemetry("app-client-configuration-change", *payload);
419+
}
419420
}
420421

421422
void DatadogAgent::send_app_closing() {

src/datadog/tracer_telemetry.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -355,10 +355,14 @@ std::string TracerTelemetry::app_closing() {
355355
return message_batch_payload;
356356
}
357357

358-
std::string TracerTelemetry::configuration_change() {
358+
Optional<std::string> TracerTelemetry::configuration_change() {
359+
if (configuration_snapshot_.empty()) return nullopt;
360+
361+
std::vector<ConfigMetadata> current_configuration;
362+
std::swap(current_configuration, configuration_snapshot_);
363+
359364
auto configuration_json = nlohmann::json::array();
360-
for (const auto& config_metadata : configuration_snapshot_) {
361-
// if (config_metadata.value.empty()) continue;
365+
for (const auto& config_metadata : current_configuration) {
362366
configuration_json.emplace_back(
363367
generate_configuration_field(config_metadata));
364368
}

src/datadog/tracer_telemetry.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ class TracerTelemetry {
150150
// been modified, a `generate-metrics` message.
151151
std::string app_closing();
152152
// Construct an `app-client-configuration-change` message.
153-
std::string configuration_change();
153+
Optional<std::string> configuration_change();
154154
};
155155

156156
} // namespace tracing

test/test_tracer_telemetry.cpp

Lines changed: 36 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -110,16 +110,9 @@ TEST_CASE("Tracer telemetry", "[telemetry]") {
110110
CHECK(cfg_payload[0] == expected_conf);
111111

112112
SECTION("generates a configuration change event") {
113-
SECTION("empty configuration generate a valid payload") {
114-
auto config_change_message = nlohmann::json::parse(
115-
tracer_telemetry.configuration_change(), nullptr, false);
116-
REQUIRE(config_change_message.is_discarded() == false);
117-
REQUIRE(is_valid_telemetry_payload(app_started) == true);
118-
119-
CHECK(config_change_message["request_type"] ==
120-
"app-client-configuration-change");
121-
CHECK(config_change_message["payload"]["configuration"].is_array());
122-
CHECK(config_change_message["payload"]["configuration"].empty());
113+
SECTION("empty configuration do not generate a valid payload") {
114+
auto config_update = tracer_telemetry.configuration_change();
115+
CHECK(!config_update);
123116
}
124117

125118
SECTION("valid configurations update") {
@@ -130,8 +123,10 @@ TEST_CASE("Tracer telemetry", "[telemetry]") {
130123
Error{Error::Code::OTHER, "empty field"}}};
131124

132125
tracer_telemetry.capture_configuration_change(new_config);
133-
auto config_change_message = nlohmann::json::parse(
134-
tracer_telemetry.configuration_change(), nullptr, false);
126+
auto updates = tracer_telemetry.configuration_change();
127+
REQUIRE(updates);
128+
auto config_change_message =
129+
nlohmann::json::parse(*updates, nullptr, false);
135130
REQUIRE(config_change_message.is_discarded() == false);
136131
REQUIRE(is_valid_telemetry_payload(config_change_message) == true);
137132

@@ -141,25 +136,42 @@ TEST_CASE("Tracer telemetry", "[telemetry]") {
141136
CHECK(config_change_message["payload"]["configuration"].size() == 2);
142137

143138
const std::unordered_map<std::string, nlohmann::json> expected_json{
144-
{"service", nlohmann::json{{"name", "service"},
145-
{"value", "increase seq_id"},
146-
{"seq_id", 2},
147-
{"origin", "env_var"}}},
148-
{"trace_enabled",
149-
nlohmann::json{{"name", "trace_enabled"},
150-
{"value", ""},
151-
{"seq_id", 1},
152-
{"origin", "default"},
153-
{"error",
154-
{{"code", Error::Code::OTHER},
155-
{"message", "empty field"}}}}}};
139+
{
140+
"service",
141+
nlohmann::json{
142+
{"name", "service"},
143+
{"value", "increase seq_id"},
144+
{"seq_id", 2},
145+
{"origin", "env_var"},
146+
},
147+
},
148+
{
149+
"trace_enabled",
150+
nlohmann::json{
151+
{"name", "trace_enabled"},
152+
{"value", ""},
153+
{"seq_id", 1},
154+
{"origin", "default"},
155+
{
156+
"error",
157+
{
158+
{"code", Error::Code::OTHER},
159+
{"message", "empty field"},
160+
},
161+
},
162+
},
163+
},
164+
};
156165

157166
for (const auto& conf :
158167
config_change_message["payload"]["configuration"]) {
159168
auto expected_conf = expected_json.find(conf["name"]);
160169
REQUIRE(expected_conf != expected_json.cend());
161170
CHECK(expected_conf->second == conf);
162171
}
172+
173+
// No update -> no configuration update
174+
CHECK(tracer_telemetry.configuration_change() == nullopt);
163175
}
164176
}
165177
}

0 commit comments

Comments
 (0)