Skip to content

Commit 4d64c46

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

File tree

2 files changed

+129
-18
lines changed

2 files changed

+129
-18
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 & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -109,10 +109,31 @@ CONFIG_MANAGER_TEST("remote configuration handling") {
109109
}
110110

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

127154
const auto old_trace_sampler_config =
128155
config_manager.trace_sampler()->config_json();
@@ -232,10 +259,35 @@ CONFIG_MANAGER_TEST("remote configuration handling") {
232259
}
233260

234261
SECTION(
235-
"payload without `tracing_tags` does not raise an error nor update the "
262+
"payload without `tracing_tags` or with a null value does not raise an "
263+
"error nor update the "
236264
"list of tags") {
237-
config_update.content = R"({
265+
struct TestCase {
266+
size_t line;
267+
std::string_view name;
268+
std::string_view input;
269+
};
270+
271+
const auto test_case = GENERATE(values<TestCase>({
272+
{
273+
__LINE__,
274+
"tracing_tags is missing",
275+
"",
276+
},
277+
{
278+
__LINE__,
279+
"tracing_tags is null",
280+
R"("tracing_tags": null,)",
281+
},
282+
}));
283+
284+
CAPTURE(test_case.line);
285+
CAPTURE(test_case.name);
286+
287+
char payload[1024];
288+
std::snprintf(payload, 1024, R"({
238289
"lib_config": {
290+
%s
239291
"library_language": "all",
240292
"library_version": "latest",
241293
"service_name": "testsvc",
@@ -245,7 +297,10 @@ CONFIG_MANAGER_TEST("remote configuration handling") {
245297
"service": "testsvc",
246298
"env": "test"
247299
}
248-
})";
300+
})",
301+
test_case.input.data());
302+
303+
config_update.content = payload;
249304

250305
const auto old_tags = config_manager.span_defaults()->tags;
251306

@@ -357,10 +412,34 @@ CONFIG_MANAGER_TEST("remote configuration handling") {
357412
}
358413

359414
SECTION(
360-
"An RC payload without `tracing_enabled` does not raise an error nor "
361-
"update the value") {
362-
config_update.content = R"({
415+
"An RC payload without `tracing_enabled` or with a null value does not "
416+
"raise an error nor update the value") {
417+
struct TestCase {
418+
size_t line;
419+
std::string_view name;
420+
std::string_view input;
421+
};
422+
423+
const auto test_case = GENERATE(values<TestCase>({
424+
{
425+
__LINE__,
426+
"tracing_enabled is absent from the RC payload",
427+
"",
428+
},
429+
{
430+
__LINE__,
431+
"tracing_enabled is null",
432+
R"("tracing_enabled": null,)",
433+
},
434+
}));
435+
436+
CAPTURE(test_case.line);
437+
CAPTURE(test_case.name);
438+
439+
char payload[1024];
440+
std::snprintf(payload, 1024, R"({
363441
"lib_config": {
442+
%s
364443
"library_language": "all",
365444
"library_version": "latest",
366445
"service_name": "testsvc",
@@ -370,7 +449,10 @@ CONFIG_MANAGER_TEST("remote configuration handling") {
370449
"service": "testsvc",
371450
"env": "test"
372451
}
373-
})";
452+
})",
453+
test_case.input.data());
454+
455+
config_update.content = payload;
374456

375457
const auto old_tracing_status = config_manager.report_traces();
376458

@@ -526,9 +608,33 @@ CONFIG_MANAGER_TEST("remote configuration handling") {
526608
CHECK(old_sampler_cfg == new_sampler_cfg);
527609
}
528610

529-
SECTION("empty") {
530-
config_update.content = R"({
611+
SECTION("null value or the absence of the field is ignored") {
612+
struct TestCase {
613+
size_t line;
614+
std::string_view name;
615+
std::string_view input;
616+
};
617+
618+
const auto test_case = GENERATE(values<TestCase>({
619+
{
620+
__LINE__,
621+
"tracing_sampling_rules is absent from the RC payload",
622+
"",
623+
},
624+
{
625+
__LINE__,
626+
"tracing_sampling_rules is null",
627+
R"("tracing_sampling_rules": null,)",
628+
},
629+
}));
630+
631+
CAPTURE(test_case.line);
632+
CAPTURE(test_case.name);
633+
634+
char payload[1024];
635+
std::snprintf(payload, 1024, R"({
531636
"lib_config": {
637+
%s
532638
"library_language": "all",
533639
"library_version": "latest",
534640
"service_name": "testsvc",
@@ -538,7 +644,10 @@ CONFIG_MANAGER_TEST("remote configuration handling") {
538644
"service": "testsvc",
539645
"env": "test"
540646
}
541-
})";
647+
})",
648+
test_case.input.data());
649+
650+
config_update.content = payload;
542651

543652
const auto old_sampler_cfg =
544653
config_manager.trace_sampler()->config_json();

0 commit comments

Comments
 (0)