Skip to content

Commit b02dfdc

Browse files
dmehaladmehala
committed
fix(rc): handle null values in RC APM Tracing payloads
Ensures system-tests pass by properly ignoring null values APM Tracing RC payload.
1 parent c6185cf commit b02dfdc

File tree

2 files changed

+129
-16
lines changed

2 files changed

+129
-16
lines changed

‎src/datadog/config_manager.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ Expected<ConfigManager::Update> parse_dynamic_config(const nlohmann::json& j) {
221221
ConfigManager::Update config_update;
222222

223223
if (auto sampling_rate_it = j.find("tracing_sampling_rate");
224-
sampling_rate_it != j.cend()) {
224+
sampling_rate_it != j.cend() && !sampling_rate_it->is_null()) {
225225
if (!sampling_rate_it->is_number_float()) {
226226
return make_err_property_msg("tracing_sampling_rate",
227227
sampling_rate_it->type_name());
@@ -236,7 +236,8 @@ Expected<ConfigManager::Update> parse_dynamic_config(const nlohmann::json& j) {
236236
config_update.trace_sampling_rate = *maybe_rate;
237237
}
238238

239-
if (auto tags_it = j.find("tracing_tags"); tags_it != j.cend()) {
239+
if (auto tags_it = j.find("tracing_tags");
240+
tags_it != j.cend() && !tags_it->is_null()) {
240241
if (!tags_it->is_array()) {
241242
return make_err_property_msg("tracing_tags", tags_it->type_name());
242243
}
@@ -250,7 +251,7 @@ Expected<ConfigManager::Update> parse_dynamic_config(const nlohmann::json& j) {
250251
}
251252

252253
if (auto tracing_enabled_it = j.find("tracing_enabled");
253-
tracing_enabled_it != j.cend()) {
254+
tracing_enabled_it != j.cend() && !tracing_enabled_it->is_null()) {
254255
if (!tracing_enabled_it->is_boolean()) {
255256
return make_err_property_msg("tracing_enabled",
256257
tracing_enabled_it->type_name());
@@ -260,7 +261,8 @@ Expected<ConfigManager::Update> parse_dynamic_config(const nlohmann::json& j) {
260261
}
261262

262263
if (auto tracing_sampling_rules_it = j.find("tracing_sampling_rules");
263-
tracing_sampling_rules_it != j.cend()) {
264+
tracing_sampling_rules_it != j.cend() &&
265+
!tracing_sampling_rules_it->is_null()) {
264266
if (!tracing_sampling_rules_it->is_array()) {
265267
return make_err_property_msg("tracing_sampling_rules",
266268
tracing_sampling_rules_it->type_name());

‎test/test_config_manager.cpp‎

Lines changed: 123 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -109,10 +109,32 @@ CONFIG_MANAGER_TEST("remote configuration handling") {
109109
}
110110

111111
SECTION(
112-
"an RC payload without the `tracing_sampling_rate` does not raise an "
112+
"an RC payload without the `tracing_sampling_rate` or with a null "
113+
"value does not raise an "
113114
"error nor update the sampling rate") {
114-
config_update.content = R"({
115+
struct TestCase {
116+
size_t line;
117+
std::string_view name;
118+
std::string_view input;
119+
};
120+
121+
const auto test_case = GENERATE(values<TestCase>({
122+
{
123+
__LINE__,
124+
"empty",
125+
"",
126+
},
127+
{
128+
__LINE__,
129+
"null",
130+
R"("tracing_sampling_rate": null,)",
131+
},
132+
}));
133+
134+
char payload[1024];
135+
std::snprintf(payload, 1024, R"({
115136
"lib_config": {
137+
%s
116138
"library_language": "all",
117139
"library_version": "latest",
118140
"service_name": "testsvc",
@@ -122,7 +144,13 @@ CONFIG_MANAGER_TEST("remote configuration handling") {
122144
"service": "testsvc",
123145
"env": "test"
124146
}
125-
})";
147+
})",
148+
test_case.input.data());
149+
150+
config_update.content = payload;
151+
152+
CAPTURE(test_case.line);
153+
CAPTURE(test_case.name);
126154

127155
const auto old_trace_sampler_config =
128156
config_manager.trace_sampler()->config_json();
@@ -232,10 +260,35 @@ CONFIG_MANAGER_TEST("remote configuration handling") {
232260
}
233261

234262
SECTION(
235-
"payload without `tracing_tags` does not raise an error nor update the "
263+
"payload without `tracing_tags` or with a null value does not raise an "
264+
"error nor update the "
236265
"list of tags") {
237-
config_update.content = R"({
266+
struct TestCase {
267+
size_t line;
268+
std::string_view name;
269+
std::string_view input;
270+
};
271+
272+
const auto test_case = GENERATE(values<TestCase>({
273+
{
274+
__LINE__,
275+
"empty",
276+
"",
277+
},
278+
{
279+
__LINE__,
280+
"null",
281+
R"("tracing_tags": null,)",
282+
},
283+
}));
284+
285+
CAPTURE(test_case.line);
286+
CAPTURE(test_case.name);
287+
288+
char payload[1024];
289+
std::snprintf(payload, 1024, R"({
238290
"lib_config": {
291+
%s
239292
"library_language": "all",
240293
"library_version": "latest",
241294
"service_name": "testsvc",
@@ -245,7 +298,10 @@ CONFIG_MANAGER_TEST("remote configuration handling") {
245298
"service": "testsvc",
246299
"env": "test"
247300
}
248-
})";
301+
})",
302+
test_case.input.data());
303+
304+
config_update.content = payload;
249305

250306
const auto old_tags = config_manager.span_defaults()->tags;
251307

@@ -357,10 +413,35 @@ CONFIG_MANAGER_TEST("remote configuration handling") {
357413
}
358414

359415
SECTION(
360-
"An RC payload without `tracing_enabled` does not raise an error nor "
416+
"An RC payload without `tracing_enabled` or with a null value does not "
417+
"raise an error nor "
361418
"update the value") {
362-
config_update.content = R"({
419+
struct TestCase {
420+
size_t line;
421+
std::string_view name;
422+
std::string_view input;
423+
};
424+
425+
const auto test_case = GENERATE(values<TestCase>({
426+
{
427+
__LINE__,
428+
"empty",
429+
"",
430+
},
431+
{
432+
__LINE__,
433+
"null",
434+
R"("tracing_enabled": null,)",
435+
},
436+
}));
437+
438+
CAPTURE(test_case.line);
439+
CAPTURE(test_case.name);
440+
441+
char payload[1024];
442+
std::snprintf(payload, 1024, R"({
363443
"lib_config": {
444+
%s
364445
"library_language": "all",
365446
"library_version": "latest",
366447
"service_name": "testsvc",
@@ -370,7 +451,10 @@ CONFIG_MANAGER_TEST("remote configuration handling") {
370451
"service": "testsvc",
371452
"env": "test"
372453
}
373-
})";
454+
})",
455+
test_case.input.data());
456+
457+
config_update.content = payload;
374458

375459
const auto old_tracing_status = config_manager.report_traces();
376460

@@ -526,9 +610,33 @@ CONFIG_MANAGER_TEST("remote configuration handling") {
526610
CHECK(old_sampler_cfg == new_sampler_cfg);
527611
}
528612

529-
SECTION("empty") {
530-
config_update.content = R"({
613+
SECTION("null value or the absence of the field is ignored") {
614+
struct TestCase {
615+
size_t line;
616+
std::string_view name;
617+
std::string_view input;
618+
};
619+
620+
const auto test_case = GENERATE(values<TestCase>({
621+
{
622+
__LINE__,
623+
"tracing_sampling_rules is absent from the RC payload",
624+
"",
625+
},
626+
{
627+
__LINE__,
628+
"tracing_sampling_rules is null",
629+
R"("tracing_sampling_rules": null,)",
630+
},
631+
}));
632+
633+
CAPTURE(test_case.line);
634+
CAPTURE(test_case.name);
635+
636+
char payload[1024];
637+
std::snprintf(payload, 1024, R"({
531638
"lib_config": {
639+
%s
532640
"library_language": "all",
533641
"library_version": "latest",
534642
"service_name": "testsvc",
@@ -538,7 +646,10 @@ CONFIG_MANAGER_TEST("remote configuration handling") {
538646
"service": "testsvc",
539647
"env": "test"
540648
}
541-
})";
649+
})",
650+
test_case.input.data());
651+
652+
config_update.content = payload;
542653

543654
const auto old_sampler_cfg =
544655
config_manager.trace_sampler()->config_json();

0 commit comments

Comments
 (0)