From ef36cfcd481bf274ed53aeefe626f891490c2535 Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Sat, 16 Aug 2025 19:55:45 +0200 Subject: [PATCH 1/2] fix(rc): rework remote configuration handling This commit refactors the way remote configurations are applied to the tracer. Previously, if a remote configuration contained both valid and invalid fields, the valid parts could be partially applied, potentially leading to inconsistent behavior. Now, the configuration is only applied if it is fully valid. Any invalid configuration is entirely rejected. This constitutes a breaking change. This also report errors when an invalid configuration is rejected. Changes: - Add fuzz test and input corpus for Remote Configuration. - Fix a crash when telemetry methods are invoked before intializing the module. - Improve unit test coverage of remote configuration. --- .circleci/config.yml | 4 + fuzz/remote-configuration/CMakeLists.txt | 13 + fuzz/remote-configuration/corpus/input1.json | 1 + fuzz/remote-configuration/corpus/input10.json | 5 + fuzz/remote-configuration/corpus/input11.json | 5 + fuzz/remote-configuration/corpus/input12.json | 5 + fuzz/remote-configuration/corpus/input13.json | 5 + fuzz/remote-configuration/corpus/input14.json | 5 + fuzz/remote-configuration/corpus/input15.json | 22 + fuzz/remote-configuration/corpus/input16.json | 22 + fuzz/remote-configuration/corpus/input17.json | 17 + fuzz/remote-configuration/corpus/input18.json | 4 + fuzz/remote-configuration/corpus/input2.json | 1 + fuzz/remote-configuration/corpus/input3.json | 1 + fuzz/remote-configuration/corpus/input4.json | 1 + fuzz/remote-configuration/corpus/input5.json | 1 + fuzz/remote-configuration/corpus/input6.json | 4 + fuzz/remote-configuration/corpus/input7.json | 4 + fuzz/remote-configuration/corpus/input8.json | 4 + fuzz/remote-configuration/corpus/input9.json | 5 + fuzz/remote-configuration/main.cpp | 36 ++ include/datadog/error.h | 1 + src/datadog/config_manager.cpp | 291 +++++----- src/datadog/config_manager.h | 10 +- src/datadog/telemetry/telemetry.cpp | 12 +- src/datadog/trace_sampler.cpp | 3 - src/datadog/trace_sampler.h | 2 + test/test_config_manager.cpp | 496 +++++++++++++++--- 28 files changed, 784 insertions(+), 196 deletions(-) create mode 100644 fuzz/remote-configuration/CMakeLists.txt create mode 100644 fuzz/remote-configuration/corpus/input1.json create mode 100644 fuzz/remote-configuration/corpus/input10.json create mode 100644 fuzz/remote-configuration/corpus/input11.json create mode 100644 fuzz/remote-configuration/corpus/input12.json create mode 100644 fuzz/remote-configuration/corpus/input13.json create mode 100644 fuzz/remote-configuration/corpus/input14.json create mode 100644 fuzz/remote-configuration/corpus/input15.json create mode 100644 fuzz/remote-configuration/corpus/input16.json create mode 100644 fuzz/remote-configuration/corpus/input17.json create mode 100644 fuzz/remote-configuration/corpus/input18.json create mode 100644 fuzz/remote-configuration/corpus/input2.json create mode 100644 fuzz/remote-configuration/corpus/input3.json create mode 100644 fuzz/remote-configuration/corpus/input4.json create mode 100644 fuzz/remote-configuration/corpus/input5.json create mode 100644 fuzz/remote-configuration/corpus/input6.json create mode 100644 fuzz/remote-configuration/corpus/input7.json create mode 100644 fuzz/remote-configuration/corpus/input8.json create mode 100644 fuzz/remote-configuration/corpus/input9.json create mode 100644 fuzz/remote-configuration/main.cpp diff --git a/.circleci/config.yml b/.circleci/config.yml index d42cbd99..ab2395c1 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -240,6 +240,10 @@ jobs: description: Run Base64 fuzzer command: timeout 5m ./.build/fuzz/base64/base64-fuzz duration: 5m + - run_timeout: + description: Run Remote Configuration fuzzer + command: timeout 5m ./.build/fuzz/remote-configuration/remote-config-fuzz + duration: 5m workflows: pull-request: diff --git a/fuzz/remote-configuration/CMakeLists.txt b/fuzz/remote-configuration/CMakeLists.txt new file mode 100644 index 00000000..5e302226 --- /dev/null +++ b/fuzz/remote-configuration/CMakeLists.txt @@ -0,0 +1,13 @@ +add_executable(remote-config-fuzz main.cpp) + +add_dependencies(remote-config-fuzz dd_trace_cpp-static) + +target_include_directories(remote-config-fuzz + PRIVATE + ${CMAKE_SOURCE_DIR}/src + ${CMAKE_SOURCE_DIR}/src/datadog +) + +target_link_libraries(remote-config-fuzz dd_trace_cpp-static) + +add_target_to_group(remote-config-fuzz dd_trace_cpp-fuzzers) diff --git a/fuzz/remote-configuration/corpus/input1.json b/fuzz/remote-configuration/corpus/input1.json new file mode 100644 index 00000000..0967ef42 --- /dev/null +++ b/fuzz/remote-configuration/corpus/input1.json @@ -0,0 +1 @@ +{} diff --git a/fuzz/remote-configuration/corpus/input10.json b/fuzz/remote-configuration/corpus/input10.json new file mode 100644 index 00000000..a1c52173 --- /dev/null +++ b/fuzz/remote-configuration/corpus/input10.json @@ -0,0 +1,5 @@ +{ + "targets": "eyJzaWduZWQiOiB7InZlcnNpb24iOiAyLCAidGFyZ2V0cyI6IHsiZW1wbG95ZWUvQVBNX1RSQUNJTkcvdmFsaWRfY29uZl9wYXRoL2NvbmZpZyI6IHt9LCAiYmFyIjoge319LCJjdXN0b20iOiB7Im9wYXF1ZV9iYWNrZW5kX3N0YXRlIjogIjE1In19fQ==", + "client_configs": ["employee/APM_TRACING/valid_conf_path/config"], + "target_files": 15 +} diff --git a/fuzz/remote-configuration/corpus/input11.json b/fuzz/remote-configuration/corpus/input11.json new file mode 100644 index 00000000..d10bd7da --- /dev/null +++ b/fuzz/remote-configuration/corpus/input11.json @@ -0,0 +1,5 @@ +{ + "targets": "eyJzaWduZWQiOiB7InZlcnNpb24iOiAyLCAidGFyZ2V0cyI6IHsiZW1wbG95ZWUvQVBNX1RSQUNJTkcvdmFsaWRfY29uZl9wYXRoL2NvbmZpZyI6IHt9LCAiYmFyIjoge319LCJjdXN0b20iOiB7Im9wYXF1ZV9iYWNrZW5kX3N0YXRlIjogIjE1In19fQ==", + "client_configs": ["employee/APM_TRACING/valid_conf_path/config"], + "target_files": [{"path": "employee/APM_TRACING/valid_conf_path/config", "raw": "Hello, Uranus!"}] +} diff --git a/fuzz/remote-configuration/corpus/input12.json b/fuzz/remote-configuration/corpus/input12.json new file mode 100644 index 00000000..b1dc13ea --- /dev/null +++ b/fuzz/remote-configuration/corpus/input12.json @@ -0,0 +1,5 @@ +{ + "targets": "eyJzaWduZWQiOiB7InZlcnNpb24iOiAyLCAidGFyZ2V0cyI6IHsiZW1wbG95ZWUvQVBNX1RSQUNJTkcvdmFsaWRfY29uZl9wYXRoL2NvbmZpZyI6IHt9LCAiYmFyIjoge319LCJjdXN0b20iOiB7Im9wYXF1ZV9iYWNrZW5kX3N0YXRlIjogIjE1In19fQ==", + "client_configs": ["employee/APM_TRACING/valid_conf_path/config"], + "target_files": [{"path": "employee/APM_TRACING/valid_conf_path/config", "raw": "bm90IGpzb24="}] +} diff --git a/fuzz/remote-configuration/corpus/input13.json b/fuzz/remote-configuration/corpus/input13.json new file mode 100644 index 00000000..8e5c325f --- /dev/null +++ b/fuzz/remote-configuration/corpus/input13.json @@ -0,0 +1,5 @@ +{ + "targets": "eyJzaWduZWQiOiB7InZlcnNpb24iOiAyLCAidGFyZ2V0cyI6IHsiZW1wbG95ZWUvQVBNX1RSQUNJTkcvdmFsaWRfY29uZl9wYXRoL2NvbmZpZyI6IHt9LCAiYmFyIjoge319LCJjdXN0b20iOiB7Im9wYXF1ZV9iYWNrZW5kX3N0YXRlIjogIjE1In19fQ==", + "client_configs": ["employee/APM_TRACING/valid_conf_path/config"], + "target_files": [{"path": "employee/APM_TRACING/valid_conf_path/config", "raw": "eyJmb28iOiAiYmFyIn0="}] +} diff --git a/fuzz/remote-configuration/corpus/input14.json b/fuzz/remote-configuration/corpus/input14.json new file mode 100644 index 00000000..bf7201d5 --- /dev/null +++ b/fuzz/remote-configuration/corpus/input14.json @@ -0,0 +1,5 @@ +{ + "targets": "eyJzaWduZWQiOiB7InZlcnNpb24iOiAyLCAidGFyZ2V0cyI6IHsiZW1wbG95ZWUvQVBNX1RSQUNJTkcvdmFsaWRfY29uZl9wYXRoL2NvbmZpZyI6IHt9LCAiYmFyIjoge319LCJjdXN0b20iOiB7Im9wYXF1ZV9iYWNrZW5kX3N0YXRlIjogIjE1In19fQ==", + "client_configs": ["foo"], + "target_files": [{"path": "foo", "raw": "eyJmb28iOiAiYmFyIn0="}] +} diff --git a/fuzz/remote-configuration/corpus/input15.json b/fuzz/remote-configuration/corpus/input15.json new file mode 100644 index 00000000..be1ee7d7 --- /dev/null +++ b/fuzz/remote-configuration/corpus/input15.json @@ -0,0 +1,22 @@ +{ + "targets": "ewogICAgInNpZ25lZCI6IHsKICAgICAgICAiY3VzdG9tIjogewogICAgICAgICAgICAiYWdlbnRfcmVmcmVzaF9pbnRlcnZhbCI6IDUsCiAgICAgICAgICAgICJvcGFxdWVfYmFja2VuZF9zdGF0ZSI6ICJleUoyWlhKemFXOXVJam95TENKemRHRjBaU0k2ZXlKbWFXeGxYMmhoYzJobGN5STZleUprWVhSaFpHOW5MekV3TURBeE1qVTROREF2UVZCTlgxUlNRVU5KVGtjdk9ESTNaV0ZqWmpoa1ltTXpZV0l4TkRNMFpETXlNV05pT0RGa1ptSm1OMkZtWlRZMU5HRTBZall4TVRGalpqRTJOakJpTnpGalkyWTRPVGM0TVRrek9DOHlPVEE0Tm1Ka1ltVTFNRFpsTmpoaU5UQm1NekExTlRneU0yRXpaR0UxWTJVd05USTRaakUyTkRCa05USmpaamc0TmpFNE1UWmhZV0U1Wm1ObFlXWTBJanBiSW05WVpESnBlVU16ZUM5b1JXc3hlWFZoWTFoR04xbHFjWEpwVGs5QldVdHVaekZ0V0UwMU5WWktUSGM5SWwxOWZYMD0iCiAgICAgICAgfSwKICAgICAgICAic3BlY192ZXJzaW9uIjogIjEuMC4wIiwKICAgICAgICAidGFyZ2V0cyI6IHsKICAgICAgICAgICAgImVtcGxveWVlL0FQTV9UUkFDSU5HL3Rlc3RfcmNfdXBkYXRlL2xpYl91cGRhdGUiOiB7CiAgICAgICAgICAgICAgICAiaGFzaGVzIjogewogICAgICAgICAgICAgICAgICAgICJzaGEyNTYiOiAiYTE3Nzc2OGIyMGI3YzdmODQ0OTM1Y2FlNjljNWM1ZWQ4OGVhYWUyMzRlMDE4MmE3ODM1OTk3MzM5ZTU1MjRiYyIKICAgICAgICAgICAgICAgIH0sCiAgICAgICAgICAgICAgICAibGVuZ3RoIjogMzc0LAoJCQkJImN1c3RvbSI6IHsgInYiOiAxMjQgfQogICAgICAgICAgICB9LAogICAgICAgICAgICAiZW1wbG95ZWUvQUdFTlRfVEFTSy90ZXN0X3JjX3VwZGF0ZS9mbGFyZV90YXNrIjogewogICAgICAgICAgICAgICAgImhhc2hlcyI6IHsKICAgICAgICAgICAgICAgICAgICAic2hhMjU2IjogIjQxOTRjZTZmNzExMzk1OTQ2YmU4MzdiZjVlYmE5NDg5MWI3YmRlNzk4OTExZWQ1ZWZmZjY1OTlkMjFhYjk2OTYiCiAgICAgICAgICAgICAgICB9LAogICAgICAgICAgICAgICAgImxlbmd0aCI6IDM3NCwKCQkJCSJjdXN0b20iOiB7ICJ2IjogMTI1IH0KICAgICAgICAgICAgfSwKICAgICAgICAgICAgImVtcGxveWVlL0FHRU5UX0NPTkZJRy90ZXN0X3JjX3VwZGF0ZS9mbGFyZV9jb25mIjogewogICAgICAgICAgICAgICAgImhhc2hlcyI6IHsKICAgICAgICAgICAgICAgICAgICAic2hhMjU2IjogIjJkNzhhZTczNmEzZmM0NTViNzIzMWRhZjk5NDVmOGRmNzA0ZjE3MjViNTBkZGU0NmQwY2JjZGMzZjBlMTFkNDEiCiAgICAgICAgICAgICAgICB9LAogICAgICAgICAgICAgICAgImxlbmd0aCI6IDM3NCwKCQkJCSJjdXN0b20iOiB7ICJ2IjogMTI1IH0KICAgICAgICAgICAgfQogICAgIAogICAgICAgIH0sCiAgICAgICAgInZlcnNpb24iOiA2NjIwNDMyMAogICAgfQp9Cg==", + "client_configs": [ + "employee/APM_TRACING/test_rc_update/lib_update", + "employee/AGENT_TASK/test_rc_update/flare_task", + "employee/AGENT_CONFIG/test_rc_update/flare_conf" + ], + "target_files": [ + { + "path": "employee/AGENT_CONFIG/test_rc_update/flare_conf", + "raw": "eyAiaWQiOiAiODI3ZWFjZjhkYmMzYWIxNDM0ZDMyMWNiODFkZmJmN2FmZTY1NGE0YjYxMTFjZjE2NjBiNzFjY2Y4OTc4MTkzOCIsICJyZXZpc2lvbiI6IDE2OTgxNjcxMjYwNjQsICJzY2hlbWFfdmVyc2lvbiI6ICJ2MS4wLjAiLCAiYWN0aW9uIjogImVuYWJsZSIsICJsaWJfY29uZmlnIjogeyAibGlicmFyeV9sYW5ndWFnZSI6ICJhbGwiLCAibGlicmFyeV92ZXJzaW9uIjogImxhdGVzdCIsICJzZXJ2aWNlX25hbWUiOiAidGVzdHN2YyIsICJlbnYiOiAidGVzdCIsICJ0cmFjaW5nX2VuYWJsZWQiOiB0cnVlLCAidHJhY2luZ19zYW1wbGluZ19yYXRlIjogMC42IH0sICJzZXJ2aWNlX3RhcmdldCI6IHsgInNlcnZpY2UiOiAidGVzdHN2YyIsICJlbnYiOiAidGVzdCIgfSB9" + }, + { + "path": "employee/APM_TRACING/test_rc_update/lib_update", + "raw": "eyAiaWQiOiAiODI3ZWFjZjhkYmMzYWIxNDM0ZDMyMWNiODFkZmJmN2FmZTY1NGE0YjYxMTFjZjE2NjBiNzFjY2Y4OTc4MTkzOCIsICJyZXZpc2lvbiI6IDE2OTgxNjcxMjYwNjQsICJzY2hlbWFfdmVyc2lvbiI6ICJ2MS4wLjAiLCAiYWN0aW9uIjogImVuYWJsZSIsICJsaWJfY29uZmlnIjogeyAibGlicmFyeV9sYW5ndWFnZSI6ICJhbGwiLCAibGlicmFyeV92ZXJzaW9uIjogImxhdGVzdCIsICJzZXJ2aWNlX25hbWUiOiAidGVzdHN2YyIsICJlbnYiOiAidGVzdCIsICJ0cmFjaW5nX2VuYWJsZWQiOiB0cnVlLCAidHJhY2luZ19zYW1wbGluZ19yYXRlIjogMC42IH0sICJzZXJ2aWNlX3RhcmdldCI6IHsgInNlcnZpY2UiOiAidGVzdHN2YyIsICJlbnYiOiAidGVzdCIgfSB9" + }, + { + "path": "employee/AGENT_TASK/test_rc_update/flare_task", + "raw": "eyAiaWQiOiAiODI3ZWFjZjhkYmMzYWIxNDM0ZDMyMWNiODFkZmJmN2FmZTY1NGE0YjYxMTFjZjE2NjBiNzFjY2Y4OTc4MTkzOCIsICJyZXZpc2lvbiI6IDE2OTgxNjcxMjYwNjQsICJzY2hlbWFfdmVyc2lvbiI6ICJ2MS4wLjAiLCAiYWN0aW9uIjogImVuYWJsZSIsICJsaWJfY29uZmlnIjogeyAibGlicmFyeV9sYW5ndWFnZSI6ICJhbGwiLCAibGlicmFyeV92ZXJzaW9uIjogImxhdGVzdCIsICJzZXJ2aWNlX25hbWUiOiAidGVzdHN2YyIsICJlbnYiOiAidGVzdCIsICJ0cmFjaW5nX2VuYWJsZWQiOiB0cnVlLCAidHJhY2luZ19zYW1wbGluZ19yYXRlIjogMC42IH0sICJzZXJ2aWNlX3RhcmdldCI6IHsgInNlcnZpY2UiOiAidGVzdHN2YyIsICJlbnYiOiAidGVzdCIgfSB9" + } + ] +} diff --git a/fuzz/remote-configuration/corpus/input16.json b/fuzz/remote-configuration/corpus/input16.json new file mode 100644 index 00000000..5b15bae9 --- /dev/null +++ b/fuzz/remote-configuration/corpus/input16.json @@ -0,0 +1,22 @@ +{ + "targets": "ewogICAgInNpZ25lZCI6IHsKICAgICAgICAiY3VzdG9tIjogewogICAgICAgICAgICAiYWdlbnRfcmVmcmVzaF9pbnRlcnZhbCI6IDUsCiAgICAgICAgICAgICJvcGFxdWVfYmFja2VuZF9zdGF0ZSI6ICJleUoyWlhKemFXOXVJam95TENKemRHRjBaU0k2ZXlKbWFXeGxYMmhoYzJobGN5STZleUprWVhSaFpHOW5MekV3TURBeE1qVTROREF2UVZCTlgxUlNRVU5KVGtjdk9ESTNaV0ZqWmpoa1ltTXpZV0l4TkRNMFpETXlNV05pT0RGa1ptSm1OMkZtWlRZMU5HRTBZall4TVRGalpqRTJOakJpTnpGalkyWTRPVGM0TVRrek9DOHlPVEE0Tm1Ka1ltVTFNRFpsTmpoaU5UQm1NekExTlRneU0yRXpaR0UxWTJVd05USTRaakUyTkRCa05USmpaamc0TmpFNE1UWmhZV0U1Wm1ObFlXWTBJanBiSW05WVpESnBlVU16ZUM5b1JXc3hlWFZoWTFoR04xbHFjWEpwVGs5QldVdHVaekZ0V0UwMU5WWktUSGM5SWwxOWZYMD0iCiAgICAgICAgfSwKICAgICAgICAic3BlY192ZXJzaW9uIjogIjEuMC4wIiwKICAgICAgICAidGFyZ2V0cyI6IHsKICAgICAgICAgICAgImVtcGxveWVlL0FQTV9UUkFDSU5HL3Rlc3RfcmNfdXBkYXRlL2xpYl91cGRhdGUiOiB7CiAgICAgICAgICAgICAgICAiaGFzaGVzIjogewogICAgICAgICAgICAgICAgICAgICJzaGEyNTYiOiAiM2I5NDIxY2FhYTVkNzUzMTg0NWY3YzMwN2FkN2M2MTU1ZDgxOTVkMjcwOTEzMzY0OTI2YzlmNjQxZTkyNDE0NyIKICAgICAgICAgICAgICAgIH0sCiAgICAgICAgICAgICAgICAibGVuZ3RoIjogMzc0LAoJCQkJImN1c3RvbSI6IHsgInYiOiAxNjAgfQogICAgICAgICAgICB9LAogICAgICAgICAgICAiZW1wbG95ZWUvQUdFTlRfVEFTSy90ZXN0X3JjX3VwZGF0ZS9mbGFyZV90YXNrIjogewogICAgICAgICAgICAgICAgImhhc2hlcyI6IHsKICAgICAgICAgICAgICAgICAgICAic2hhMjU2IjogIjU2Nzc0ODFhOGMyMWQ2Yzc0MDgyOWZkMTA2MTAwZjQ2ZjdjNTFmNTI2NWIwYmE1NDBiYzE5OGJkODMzOWY4NzIiCiAgICAgICAgICAgICAgICB9LAogICAgICAgICAgICAgICAgImxlbmd0aCI6IDM3NCwKCQkJCSJjdXN0b20iOiB7ICJ2IjogMTYxIH0KICAgICAgICAgICAgfSwKICAgICAgICAgICAgImVtcGxveWVlL0FHRU5UX0NPTkZJRy90ZXN0X3JjX3VwZGF0ZS9mbGFyZV9jb25mIjogewogICAgICAgICAgICAgICAgImhhc2hlcyI6IHsKICAgICAgICAgICAgICAgICAgICAic2hhMjU2IjogImU2OGVjOGQ5YjExYThjZDU4YzhjYTVlMTQyNWQ2MTYzZGI5NDdlYWEzNWY3Mzg1NjFjNDg2ZTE0NGU5NGZjNTIiCiAgICAgICAgICAgICAgICB9LAogICAgICAgICAgICAgICAgImxlbmd0aCI6IDM3NCwKCQkJCSJjdXN0b20iOiB7ICJ2IjogMTYyIH0KICAgICAgICAgICAgfQogICAgICAgIH0sCiAgICAgICAgInZlcnNpb24iOiA2NjIwNDMyMAogICAgfQp9Cg==", + "client_configs": [ + "employee/APM_TRACING/test_rc_update/lib_update", + "employee/AGENT_TASK/test_rc_update/flare_task", + "employee/AGENT_CONFIG/test_rc_update/flare_conf" + ], + "target_files": [ + { + "path": "employee/AGENT_CONFIG/test_rc_update/flare_conf", + "raw": "eyAiaWQiOiAiODI3ZWFjZjhkYmMzYWIxNDM0ZDMyMWNiODFkZmJmN2FmZTY1NGE0YjYxMTFjZjE2NjBiNzFjY2Y4OTc4MTkzOCIsICJyZXZpc2lvbiI6IDE2OTgxNjcxMjYwNjQsICJzY2hlbWFfdmVyc2lvbiI6ICJ2MS4wLjAiLCAiYWN0aW9uIjogImVuYWJsZSIsICJsaWJfY29uZmlnIjogeyAibGlicmFyeV9sYW5ndWFnZSI6ICJhbGwiLCAibGlicmFyeV92ZXJzaW9uIjogImxhdGVzdCIsICJzZXJ2aWNlX25hbWUiOiAidGVzdHN2YyIsICJlbnYiOiAidGVzdCIsICJ0cmFjaW5nX2VuYWJsZWQiOiB0cnVlLCAidHJhY2luZ19zYW1wbGluZ19yYXRlIjogMC42IH0sICJzZXJ2aWNlX3RhcmdldCI6IHsgInNlcnZpY2UiOiAidGVzdHN2YyIsICJlbnYiOiAidGVzdCIgfSB9" + }, + { + "path": "employee/APM_TRACING/test_rc_update/lib_update", + "raw": "eyAiaWQiOiAiODI3ZWFjZjhkYmMzYWIxNDM0ZDMyMWNiODFkZmJmN2FmZTY1NGE0YjYxMTFjZjE2NjBiNzFjY2Y4OTc4MTkzOCIsICJyZXZpc2lvbiI6IDE2OTgxNjcxMjYwNjQsICJzY2hlbWFfdmVyc2lvbiI6ICJ2MS4wLjAiLCAiYWN0aW9uIjogImVuYWJsZSIsICJsaWJfY29uZmlnIjogeyAibGlicmFyeV9sYW5ndWFnZSI6ICJhbGwiLCAibGlicmFyeV92ZXJzaW9uIjogImxhdGVzdCIsICJzZXJ2aWNlX25hbWUiOiAidGVzdHN2YyIsICJlbnYiOiAidGVzdCIsICJ0cmFjaW5nX2VuYWJsZWQiOiB0cnVlLCAidHJhY2luZ19zYW1wbGluZ19yYXRlIjogMC42IH0sICJzZXJ2aWNlX3RhcmdldCI6IHsgInNlcnZpY2UiOiAidGVzdHN2YyIsICJlbnYiOiAidGVzdCIgfSB9" + }, + { + "path": "employee/AGENT_TASK/test_rc_update/flare_task", + "raw": "eyAiaWQiOiAiODI3ZWFjZjhkYmMzYWIxNDM0ZDMyMWNiODFkZmJmN2FmZTY1NGE0YjYxMTFjZjE2NjBiNzFjY2Y4OTc4MTkzOCIsICJyZXZpc2lvbiI6IDE2OTgxNjcxMjYwNjQsICJzY2hlbWFfdmVyc2lvbiI6ICJ2MS4wLjAiLCAiYWN0aW9uIjogImVuYWJsZSIsICJsaWJfY29uZmlnIjogeyAibGlicmFyeV9sYW5ndWFnZSI6ICJhbGwiLCAibGlicmFyeV92ZXJzaW9uIjogImxhdGVzdCIsICJzZXJ2aWNlX25hbWUiOiAidGVzdHN2YyIsICJlbnYiOiAidGVzdCIsICJ0cmFjaW5nX2VuYWJsZWQiOiB0cnVlLCAidHJhY2luZ19zYW1wbGluZ19yYXRlIjogMC42IH0sICJzZXJ2aWNlX3RhcmdldCI6IHsgInNlcnZpY2UiOiAidGVzdHN2YyIsICJlbnYiOiAidGVzdCIgfSB9" + } + ] +} diff --git a/fuzz/remote-configuration/corpus/input17.json b/fuzz/remote-configuration/corpus/input17.json new file mode 100644 index 00000000..4ab75712 --- /dev/null +++ b/fuzz/remote-configuration/corpus/input17.json @@ -0,0 +1,17 @@ +{ + "targets": "ewogICAgInNpZ25lZCI6IHsKICAgICAgICAiY3VzdG9tIjogewogICAgICAgICAgICAiYWdlbnRfcmVmcmVzaF9pbnRlcnZhbCI6IDUsCiAgICAgICAgICAgICJvcGFxdWVfYmFja2VuZF9zdGF0ZSI6ICJleUoyWlhKemFXOXVJam95TENKemRHRjBaU0k2ZXlKbWFXeGxYMmhoYzJobGN5STZleUprWVhSaFpHOW5MekV3TURBeE1qVTROREF2UVZCTlgxUlNRVU5KVGtjdk9ESTNaV0ZqWmpoa1ltTXpZV0l4TkRNMFpETXlNV05pT0RGa1ptSm1OMkZtWlRZMU5HRTBZall4TVRGalpqRTJOakJpTnpGalkyWTRPVGM0TVRrek9DOHlPVEE0Tm1Ka1ltVTFNRFpsTmpoaU5UQm1NekExTlRneU0yRXpaR0UxWTJVd05USTRaakUyTkRCa05USmpaamc0TmpFNE1UWmhZV0U1Wm1ObFlXWTBJanBiSW05WVpESnBlVU16ZUM5b1JXc3hlWFZoWTFoR04xbHFjWEpwVGs5QldVdHVaekZ0V0UwMU5WWktUSGM5SWwxOWZYMD0iCiAgICAgICAgfSwKICAgICAgICAic3BlY192ZXJzaW9uIjogIjEuMC4wIiwKICAgICAgICAidGFyZ2V0cyI6IHsKICAgICAgICAgICAgImVtcGxveWVlL0FHRU5UX1RBU0svdGVzdF9yY191cGRhdGUvZmxhcmVfdGFzayI6IHsKICAgICAgICAgICAgICAgICJoYXNoZXMiOiB7CiAgICAgICAgICAgICAgICAgICAgInNoYTI1NiI6ICI0MTk0Y2U2ZjcxMTM5NTk0NmJlODM3YmY1ZWJhOTQ4OTFiN2JkZTc5ODkxMWVkNWVmZmY2NTk5ZDIxYWI5Njk2IgogICAgICAgICAgICAgICAgfSwKICAgICAgICAgICAgICAgICJsZW5ndGgiOiAzNzQKICAgICAgICAgICAgfSwKICAgICAgICAgICAgImVtcGxveWVlL0FHRU5UX0NPTkZJRy90ZXN0X3JjX3VwZGF0ZS9mbGFyZV9jb25mIjogewogICAgICAgICAgICAgICAgImhhc2hlcyI6IHsKICAgICAgICAgICAgICAgICAgICAic2hhMjU2IjogIjJkNzhhZTczNmEzZmM0NTViNzIzMWRhZjk5NDVmOGRmNzA0ZjE3MjViNTBkZGU0NmQwY2JjZGMzZjBlMTFkNDEiCiAgICAgICAgICAgICAgICB9LAogICAgICAgICAgICAgICAgImxlbmd0aCI6IDM3NAogICAgICAgICAgICB9CiAgICAgCiAgICAgICAgfSwKICAgICAgICAidmVyc2lvbiI6IDY2MjA0MzIwCiAgICB9Cn0=", + "client_configs": [ + "employee/AGENT_TASK/test_rc_update/flare_task", + "employee/AGENT_CONFIG/test_rc_update/flare_conf" + ], + "target_files": [ + { + "path": "employee/AGENT_CONFIG/test_rc_update/flare_conf", + "raw": "eyAiaWQiOiAiODI3ZWFjZjhkYmMzYWIxNDM0ZDMyMWNiODFkZmJmN2FmZTY1NGE0YjYxMTFjZjE2NjBiNzFjY2Y4OTc4MTkzOCIsICJyZXZpc2lvbiI6IDE2OTgxNjcxMjYwNjQsICJzY2hlbWFfdmVyc2lvbiI6ICJ2MS4wLjAiLCAiYWN0aW9uIjogImVuYWJsZSIsICJsaWJfY29uZmlnIjogeyAibGlicmFyeV9sYW5ndWFnZSI6ICJhbGwiLCAibGlicmFyeV92ZXJzaW9uIjogImxhdGVzdCIsICJzZXJ2aWNlX25hbWUiOiAidGVzdHN2YyIsICJlbnYiOiAidGVzdCIsICJ0cmFjaW5nX2VuYWJsZWQiOiB0cnVlLCAidHJhY2luZ19zYW1wbGluZ19yYXRlIjogMC42IH0sICJzZXJ2aWNlX3RhcmdldCI6IHsgInNlcnZpY2UiOiAidGVzdHN2YyIsICJlbnYiOiAidGVzdCIgfSB9" + }, + { + "path": "employee/AGENT_TASK/test_rc_update/flare_task", + "raw": "eyAiaWQiOiAiODI3ZWFjZjhkYmMzYWIxNDM0ZDMyMWNiODFkZmJmN2FmZTY1NGE0YjYxMTFjZjE2NjBiNzFjY2Y4OTc4MTkzOCIsICJyZXZpc2lvbiI6IDE2OTgxNjcxMjYwNjQsICJzY2hlbWFfdmVyc2lvbiI6ICJ2MS4wLjAiLCAiYWN0aW9uIjogImVuYWJsZSIsICJsaWJfY29uZmlnIjogeyAibGlicmFyeV9sYW5ndWFnZSI6ICJhbGwiLCAibGlicmFyeV92ZXJzaW9uIjogImxhdGVzdCIsICJzZXJ2aWNlX25hbWUiOiAidGVzdHN2YyIsICJlbnYiOiAidGVzdCIsICJ0cmFjaW5nX2VuYWJsZWQiOiB0cnVlLCAidHJhY2luZ19zYW1wbGluZ19yYXRlIjogMC42IH0sICJzZXJ2aWNlX3RhcmdldCI6IHsgInNlcnZpY2UiOiAidGVzdHN2YyIsICJlbnYiOiAidGVzdCIgfSB9" + } + ] +} diff --git a/fuzz/remote-configuration/corpus/input18.json b/fuzz/remote-configuration/corpus/input18.json new file mode 100644 index 00000000..704c14bb --- /dev/null +++ b/fuzz/remote-configuration/corpus/input18.json @@ -0,0 +1,4 @@ +{ +"targets": "ewogICAgInNpZ25lZCI6IHsKICAgICAgICAiY3VzdG9tIjogewogICAgICAgICAgICAiYWdlbnRfcmVmcmVzaF9pbnRlcnZhbCI6IDUsCiAgICAgICAgICAgICJvcGFxdWVfYmFja2VuZF9zdGF0ZSI6ICJleUoyWlhKemFXOXVJam95TENKemRHRjBaU0k2ZXlKbWFXeGxYMmhoYzJobGN5STZleUprWVhSaFpHOW5MekV3TURBeE1qVTROREF2UVZCTlgxUlNRVU5KVGtjdk9ESTNaV0ZqWmpoa1ltTXpZV0l4TkRNMFpETXlNV05pT0RGa1ptSm1OMkZtWlRZMU5HRTBZall4TVRGalpqRTJOakJpTnpGalkyWTRPVGM0TVRrek9DOHlPVEE0Tm1Ka1ltVTFNRFpsTmpoaU5UQm1NekExTlRneU0yRXpaR0UxWTJVd05USTRaakUyTkRCa05USmpaamc0TmpFNE1UWmhZV0U1Wm1ObFlXWTBJanBiSW05WVpESnBlVU16ZUM5b1JXc3hlWFZoWTFoR04xbHFjWEpwVGs5QldVdHVaekZ0V0UwMU5WWktUSGM5SWwxOWZYMD0iCiAgICAgICAgfSwKICAgICAgICAic3BlY192ZXJzaW9uIjogIjEuMC4wIiwKICAgICAgICAidGFyZ2V0cyI6IHsKICAgICAgICAgICAgImVtcGxveWVlL0FQTV9UUkFDSU5HL3Rlc3RfcmNfdXBkYXRlL2xpYl91cGRhdGUiOiB7CiAgICAgICAgICAgICAgICAiaGFzaGVzIjogewogICAgICAgICAgICAgICAgICAgICJzaGEyNTYiOiAiYTE3Nzc2OGIyMGI3YzdmODQ0OTM1Y2FlNjljNWM1ZWQ4OGVhYWUyMzRlMDE4MmE3ODM1OTk3MzM5ZTU1MjRiYyIKICAgICAgICAgICAgICAgIH0sCiAgICAgICAgICAgICAgICAibGVuZ3RoIjogMzc0CiAgICAgICAgICAgIH0sCiAgICAgICAgICAgICJlbXBsb3llZS9BR0VOVF9UQVNLL3Rlc3RfcmNfdXBkYXRlL2ZsYXJlX3Rhc2siOiB7CiAgICAgICAgICAgICAgICAiaGFzaGVzIjogewogICAgICAgICAgICAgICAgICAgICJzaGEyNTYiOiAiNDE5NGNlNmY3MTEzOTU5NDZiZTgzN2JmNWViYTk0ODkxYjdiZGU3OTg5MTFlZDVlZmZmNjU5OWQyMWFiOTY5NiIKICAgICAgICAgICAgICAgIH0sCiAgICAgICAgICAgICAgICAibGVuZ3RoIjogMzc0CiAgICAgICAgICAgIH0sCiAgICAgICAgICAgICJlbXBsb3llZS9BR0VOVF9DT05GSUcvdGVzdF9yY191cGRhdGUvZmxhcmVfY29uZiI6IHsKICAgICAgICAgICAgICAgICJoYXNoZXMiOiB7CiAgICAgICAgICAgICAgICAgICAgInNoYTI1NiI6ICIyZDc4YWU3MzZhM2ZjNDU1YjcyMzFkYWY5OTQ1ZjhkZjcwNGYxNzI1YjUwZGRlNDZkMGNiY2RjM2YwZTExZDQxIgogICAgICAgICAgICAgICAgfSwKICAgICAgICAgICAgICAgICJsZW5ndGgiOiAzNzQKICAgICAgICAgICAgfQogICAgIAogICAgICAgIH0sCiAgICAgICAgInZlcnNpb24iOiA2NjIwNDMyMAogICAgfQp9", +"target_files": [{}] +} diff --git a/fuzz/remote-configuration/corpus/input2.json b/fuzz/remote-configuration/corpus/input2.json new file mode 100644 index 00000000..1a899a40 --- /dev/null +++ b/fuzz/remote-configuration/corpus/input2.json @@ -0,0 +1 @@ +{ "targets": "" } diff --git a/fuzz/remote-configuration/corpus/input3.json b/fuzz/remote-configuration/corpus/input3.json new file mode 100644 index 00000000..d9645f65 --- /dev/null +++ b/fuzz/remote-configuration/corpus/input3.json @@ -0,0 +1 @@ +{ "targets": "Hello, Mars!" } diff --git a/fuzz/remote-configuration/corpus/input4.json b/fuzz/remote-configuration/corpus/input4.json new file mode 100644 index 00000000..57823a51 --- /dev/null +++ b/fuzz/remote-configuration/corpus/input4.json @@ -0,0 +1 @@ +{ "targets": "bm90IGpzb24=" } diff --git a/fuzz/remote-configuration/corpus/input5.json b/fuzz/remote-configuration/corpus/input5.json new file mode 100644 index 00000000..74376bac --- /dev/null +++ b/fuzz/remote-configuration/corpus/input5.json @@ -0,0 +1 @@ +{ "targets": "eyJmb28iOiAiYmFyIn0=" } diff --git a/fuzz/remote-configuration/corpus/input6.json b/fuzz/remote-configuration/corpus/input6.json new file mode 100644 index 00000000..b0f862fd --- /dev/null +++ b/fuzz/remote-configuration/corpus/input6.json @@ -0,0 +1,4 @@ +{ + "targets": "eyJzaWduZWQiOiB7InZlcnNpb24iOiAyLCAiY3VzdG9tIjogeyJvcGFxdWVfYmFja2VuZF9zdGF0ZSI6ICIxNSJ9fX0=", + "client_configs": ["employee/APM_TRACING/missing_target/conf"] +} diff --git a/fuzz/remote-configuration/corpus/input7.json b/fuzz/remote-configuration/corpus/input7.json new file mode 100644 index 00000000..9e031e82 --- /dev/null +++ b/fuzz/remote-configuration/corpus/input7.json @@ -0,0 +1,4 @@ +{ + "targets": "eyJzaWduZWQiOiB7InZlcnNpb24iOiAyLCAidGFyZ2V0cyI6IHsiZm9vIjoge30sICJiYXIiOiB7fX0sImN1c3RvbSI6IHsib3BhcXVlX2JhY2tlbmRfc3RhdGUiOiAiMTUifX19", + "client_configs": ["employee/APM_TRACING/missing_client_entry/conf"] +} diff --git a/fuzz/remote-configuration/corpus/input8.json b/fuzz/remote-configuration/corpus/input8.json new file mode 100644 index 00000000..33fac8e7 --- /dev/null +++ b/fuzz/remote-configuration/corpus/input8.json @@ -0,0 +1,4 @@ +{ + "targets": "eyJzaWduZWQiOiB7InZlcnNpb24iOiAyLCAidGFyZ2V0cyI6IHsiZW1wbG95ZWUvQVBNX1RSQUNJTkcvdmFsaWRfY29uZl9wYXRoL2NvbmZpZyI6IHt9LCAiYmFyIjoge319LCJjdXN0b20iOiB7Im9wYXF1ZV9iYWNrZW5kX3N0YXRlIjogIjE1In19fQ==", + "client_configs": ["employee/APM_TRACING/valid_conf_path/config"] +} diff --git a/fuzz/remote-configuration/corpus/input9.json b/fuzz/remote-configuration/corpus/input9.json new file mode 100644 index 00000000..d6518d99 --- /dev/null +++ b/fuzz/remote-configuration/corpus/input9.json @@ -0,0 +1,5 @@ +{ + "targets": "eyJzaWduZWQiOiB7InZlcnNpb24iOiAyLCAidGFyZ2V0cyI6IHsiZW1wbG95ZWUvQVBNX1RSQUNJTkcvdmFsaWRfY29uZl9wYXRoL2NvbmZpZyI6IHt9LCAiYmFyIjoge319LCJjdXN0b20iOiB7Im9wYXF1ZV9iYWNrZW5kX3N0YXRlIjogIjE1In19fQ==", + "client_configs": ["employee/APM_TRACING/valid_conf_path/config"], + "target_files": [] +} diff --git a/fuzz/remote-configuration/main.cpp b/fuzz/remote-configuration/main.cpp new file mode 100644 index 00000000..da1e6c25 --- /dev/null +++ b/fuzz/remote-configuration/main.cpp @@ -0,0 +1,36 @@ +#include +#include +#include + +#include +#include +#include + +namespace dd = datadog; + +class NoopLogger : public dd::tracing::Logger { + std::mutex mutex_; + std::ostringstream stream_; + + public: + void log_error(const LogFunc&) override{}; + void log_startup(const LogFunc&) override{}; + using Logger::log_error; // expose the non-virtual overloads + + private: + void log(const LogFunc&){}; +}; + +extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t* data, size_t size) { + dd::tracing::TracerSignature signature(dd::tracing::RuntimeID::generate(), + "foosvc", "fooenv"); + std::vector> listener; + auto logger = std::make_shared(); + datadog::remote_config::Manager manager(signature, listener, logger); + std::string in((char*)data, size); + auto j = nlohmann::json::parse(in, nullptr, false); + if (j.is_discarded()) return 0; + + manager.process_response(j); + return 0; +} diff --git a/include/datadog/error.h b/include/datadog/error.h index a0c103a2..af6825dc 100644 --- a/include/datadog/error.h +++ b/include/datadog/error.h @@ -78,6 +78,7 @@ struct Error { REMOTE_CONFIGURATION_INVALID_INPUT = 53, BAGGAGE_MAXIMUM_BYTES_REACHED = 54, BAGGAGE_MAXIMUM_ITEMS_REACHED = 55, + REMOTE_CONFIGURATION_INVALID_JSON = 56, }; Code code; diff --git a/src/datadog/config_manager.cpp b/src/datadog/config_manager.cpp index 6b6462f8..59500994 100644 --- a/src/datadog/config_manager.cpp +++ b/src/datadog/config_manager.cpp @@ -24,6 +24,15 @@ nlohmann::json to_json(const SpanDefaults& defaults) { return result; } +nlohmann::json to_json(const std::vector& rules) { + std::vector j; + for (const auto& rule : rules) { + j.push_back(to_json(rule)); + } + + return j; +} + using Rules = std::vector; using Tags = std::unordered_map; @@ -35,7 +44,8 @@ Expected parse_tags_from_sampling_rules(const nlohmann::json& json_tags) { auto key = json_tag_entry.find("key"); if (key == json_tag_entry.cend() || key->is_string() == false) { std::string err_msg = - "Failed to parse tags: the required \"key\" field is either missing " + "failed to parse tags from sampling rule: the required \"key\" field " + "is either missing " "or incorrectly formatted. (input: "; err_msg += json_tags.dump(); err_msg += ")"; @@ -46,7 +56,8 @@ Expected parse_tags_from_sampling_rules(const nlohmann::json& json_tags) { auto value = json_tag_entry.find("value_glob"); if (value == json_tag_entry.cend() || value->is_string() == false) { std::string err_msg = - "Failed to parse tags: the required \"value_glob\" field is either " + "failed to parse tags from sampling rule: the required " + "\"value_glob\" field is either " "missing or incorrectly formatted. (input: "; err_msg += json_tags.dump(); err_msg += ")"; @@ -61,10 +72,18 @@ Expected parse_tags_from_sampling_rules(const nlohmann::json& json_tags) { } Expected parse_rule(const nlohmann::json& json_rule) { - assert(json_rule.is_object()); + if (!json_rule.is_object()) { + std::string err_msg = + "failed to parse sampling rule: expected a JSON object but got a "; + err_msg += json_rule.type_name(); + err_msg += " instead. (input: "; + err_msg += json_rule.dump(); + err_msg += ")"; + return Error{Error::TRACE_SAMPLING_RULES_INVALID_JSON, std::move(err_msg)}; + } auto make_error = [&json_rule](StringView field_name) { - std::string err_msg = "Failed to parse sampling rule: the required \""; + std::string err_msg = "failed to parse sampling rule: the required \""; append(err_msg, field_name); err_msg += "\" field is missing. (input: "; err_msg += json_rule.dump(); @@ -76,7 +95,7 @@ Expected parse_rule(const nlohmann::json& json_rule) { const nlohmann::json& value, StringView expected_type) { std::string message; - message += "Rule property \""; + message += "rule property \""; append(message, property); message += "\" should have type \""; append(message, expected_type); @@ -139,7 +158,7 @@ Expected parse_rule(const nlohmann::json& json_rule) { } else if (provenance == "dynamic") { rule.mechanism = SamplingMechanism::REMOTE_ADAPTIVE_RULE; } else { - std::string err_msg = "Failed to parse sampling rule: unknown \""; + std::string err_msg = "failed to parse sampling rule: unknown \""; err_msg += provenance; err_msg += "\" value. Expected either \"customer\" or \"dynamic\""; return Error{Error::TRACE_SAMPLING_RULES_UNKNOWN_PROPERTY, @@ -174,11 +193,6 @@ Expected parse_rule(const nlohmann::json& json_rule) { } Expected parse_trace_sampling_rules(const nlohmann::json& json_rules) { - if (json_rules.is_array() == false) { - std::string message; - return Error{Error::TRACE_SAMPLING_RULES_WRONG_TYPE, std::move(message)}; - } - Rules parsed_rules; for (const auto& json_rule : json_rules) { auto maybe_rule = parse_rule(json_rule); @@ -192,28 +206,74 @@ Expected parse_trace_sampling_rules(const nlohmann::json& json_rules) { return parsed_rules; } -ConfigManager::Update parse_dynamic_config(const nlohmann::json& j) { +Expected parse_dynamic_config(const nlohmann::json& j) { + auto make_err_property_msg = [](StringView field_name, + StringView unexpected_type) { + std::string err_msg{"field \""}; + append(err_msg, field_name); + err_msg += "\" expected a float number value but got "; + append(err_msg, unexpected_type); + err_msg += " instead."; + return Error{Error::Code::REMOTE_CONFIGURATION_INVALID_JSON, + std::move(err_msg)}; + }; + ConfigManager::Update config_update; if (auto sampling_rate_it = j.find("tracing_sampling_rate"); - sampling_rate_it != j.cend() && sampling_rate_it->is_number()) { - config_update.trace_sampling_rate = sampling_rate_it->get(); + sampling_rate_it != j.cend()) { + if (!sampling_rate_it->is_number_float()) { + return make_err_property_msg("tracing_sampling_rate", + sampling_rate_it->type_name()); + } + + auto maybe_rate = Rate::from(sampling_rate_it->get()); + if (auto* error = maybe_rate.if_error()) { + return error->with_prefix( + "\"tracing_sampling_rate\" field got an invalid input. "); + } + + config_update.trace_sampling_rate = *maybe_rate; } - if (auto tags_it = j.find("tracing_tags"); - tags_it != j.cend() && tags_it->is_array()) { - config_update.tags = tags_it->get>(); + if (auto tags_it = j.find("tracing_tags"); tags_it != j.cend()) { + if (!tags_it->is_array()) { + return make_err_property_msg("tracing_tags", tags_it->type_name()); + } + + auto maybe_tags = parse_tags(tags_it->get>()); + if (auto error = maybe_tags.if_error()) { + return *error; + } + + config_update.tags = std::move(*maybe_tags); } if (auto tracing_enabled_it = j.find("tracing_enabled"); - tracing_enabled_it != j.cend() && tracing_enabled_it->is_boolean()) { + tracing_enabled_it != j.cend()) { + if (!tracing_enabled_it->is_boolean()) { + return make_err_property_msg("tracing_enabled", + tracing_enabled_it->type_name()); + } + config_update.report_traces = tracing_enabled_it->get(); } if (auto tracing_sampling_rules_it = j.find("tracing_sampling_rules"); - tracing_sampling_rules_it != j.cend() && - tracing_sampling_rules_it->is_array()) { - config_update.trace_sampling_rules = &(*tracing_sampling_rules_it); + tracing_sampling_rules_it != j.cend()) { + if (!tracing_sampling_rules_it->is_array()) { + return make_err_property_msg("tracing_sampling_rules", + tracing_sampling_rules_it->type_name()); + } + + auto maybe_sampling_rules = + parse_trace_sampling_rules(*tracing_sampling_rules_it); + + if (auto error = maybe_sampling_rules.if_error()) { + return *error; + } + + config_update.trace_sampling_rules = std::move(*maybe_sampling_rules); } return config_update; @@ -247,19 +307,18 @@ Optional ConfigManager::on_update(const Configuration& config) { const auto config_json = nlohmann::json::parse(config.content); - auto config_update = parse_dynamic_config(config_json.at("lib_config")); - - auto config_metadata = apply_update(config_update); - telemetry::capture_configuration_change(config_metadata); + auto maybe_config_update = parse_dynamic_config(config_json.at("lib_config")); + if (auto err = maybe_config_update.if_error()) { + return err + ->with_prefix("Failed to parse APM remote configuration payload: ") + .message; + } - // TODO: + apply_update(*maybe_config_update); return nullopt; } -void ConfigManager::on_revert(const Configuration&) { - auto config_metadata = apply_update({}); - telemetry::capture_configuration_change(config_metadata); -} +void ConfigManager::on_revert(const Configuration&) { apply_update({}); } std::shared_ptr ConfigManager::trace_sampler() { std::lock_guard lock(mutex_); @@ -276,115 +335,107 @@ bool ConfigManager::report_traces() { return report_traces_.value(); } -std::vector ConfigManager::apply_update( - const ConfigManager::Update& conf) { +void ConfigManager::apply_update(const ConfigManager::Update& conf) { std::vector metadata; - std::lock_guard lock(mutex_); - - // NOTE(@dmehala): Sampling rules are generally not well specified. - // - // Rules are evaluated in the order they are inserted, which means the most - // specific matching rule might not be evaluated, even though it should be. - // For now, we must follow this legacy behavior. - // - // Additionally, I exploit this behavior to avoid a merge operation. - // The resulting array can contain duplicate `SpanMatcher`, but only the first - // encountered one will be evaluated, acting as an override. - // - // Remote Configuration rules will/should always be placed at the begining of - // the array, ensuring they are evaluated first. - auto rules = rules_; - - if (!conf.trace_sampling_rate) { - auto found = default_metadata_.find(ConfigName::TRACE_SAMPLING_RATE); - if (found != default_metadata_.cend()) { - metadata.push_back(found->second); - } - } else { - ConfigMetadata trace_sampling_metadata( - ConfigName::TRACE_SAMPLING_RATE, - to_string(*conf.trace_sampling_rate, 1), - ConfigMetadata::Origin::REMOTE_CONFIG); - - auto rate = Rate::from(*conf.trace_sampling_rate); - - TraceSamplerRule rule; - rule.rate = *rate; - rule.matcher = catch_all; - rule.mechanism = SamplingMechanism::RULE; - - // Convention: Catch-all rules should ALWAYS be the last in the list. - // If a catch-all rule already exists, replace it. - // If NOT, add the new one at the end of the rules list. - if (rules.empty()) { - rules.emplace_back(std::move(rule)); + // TODO: We're doing too much while holding the lock. Refactor to reduce lock + // contention. + { + std::lock_guard lock(mutex_); + + // NOTE(@dmehala): Sampling rules are generally not well specified. + // + // Rules are evaluated in the order they are inserted, which means the most + // specific matching rule might not be evaluated, even though it should be. + // For now, we must follow this legacy behavior. + // + // Additionally, I exploit this behavior to avoid a merge operation. + // The resulting array can contain duplicate `SpanMatcher`, but only the + // first encountered one will be evaluated, acting as an override. + // + // Remote Configuration rules will/should always be placed at the begining + // of the array, ensuring they are evaluated first. + auto rules = rules_; + + if (!conf.trace_sampling_rate) { + auto found = default_metadata_.find(ConfigName::TRACE_SAMPLING_RATE); + if (found != default_metadata_.cend()) { + metadata.push_back(found->second); + } } else { - if (auto& last_rule = rules.back(); last_rule.matcher == catch_all) { - last_rule = rule; - } else { + ConfigMetadata trace_sampling_metadata( + ConfigName::TRACE_SAMPLING_RATE, + to_string(*conf.trace_sampling_rate, 1), + ConfigMetadata::Origin::REMOTE_CONFIG); + + TraceSamplerRule rule; + rule.rate = *conf.trace_sampling_rate; + rule.matcher = catch_all; + rule.mechanism = SamplingMechanism::RULE; + + // Convention: Catch-all rules should ALWAYS be the last in the list. + // If a catch-all rule already exists, replace it. + // If NOT, add the new one at the end of the rules list. + if (rules.empty()) { rules.emplace_back(std::move(rule)); + } else { + if (auto& last_rule = rules.back(); last_rule.matcher == catch_all) { + last_rule = rule; + } else { + rules.emplace_back(std::move(rule)); + } } - } - - metadata.emplace_back(std::move(trace_sampling_metadata)); - } - if (!conf.trace_sampling_rules) { - auto found = default_metadata_.find(ConfigName::TRACE_SAMPLING_RULES); - if (found != default_metadata_.cend()) { - metadata.emplace_back(found->second); + metadata.emplace_back(std::move(trace_sampling_metadata)); } - } else { - ConfigMetadata trace_sampling_rules_metadata( - ConfigName::TRACE_SAMPLING_RULES, conf.trace_sampling_rules->dump(), - ConfigMetadata::Origin::REMOTE_CONFIG); - auto maybe_rules = parse_trace_sampling_rules(*conf.trace_sampling_rules); - if (auto error = maybe_rules.if_error()) { - trace_sampling_rules_metadata.error = std::move(*error); + if (!conf.trace_sampling_rules) { + auto found = default_metadata_.find(ConfigName::TRACE_SAMPLING_RULES); + if (found != default_metadata_.cend()) { + metadata.emplace_back(found->second); + } } else { - rules.insert(rules.cbegin(), maybe_rules->begin(), maybe_rules->end()); + rules.insert(rules.cbegin(), conf.trace_sampling_rules->begin(), + conf.trace_sampling_rules->end()); + + ConfigMetadata trace_sampling_rules_metadata( + ConfigName::TRACE_SAMPLING_RULES, + to_json(*conf.trace_sampling_rules).dump(), + ConfigMetadata::Origin::REMOTE_CONFIG); + metadata.emplace_back(std::move(trace_sampling_rules_metadata)); } - metadata.emplace_back(std::move(trace_sampling_rules_metadata)); - } - - trace_sampler_->set_rules(std::move(rules)); - - if (!conf.tags) { - reset_config(ConfigName::TAGS, span_defaults_, metadata); - } else { - ConfigMetadata tags_metadata(ConfigName::TAGS, join(*conf.tags, ","), - ConfigMetadata::Origin::REMOTE_CONFIG); + trace_sampler_->set_rules(std::move(rules)); - auto parsed_tags = parse_tags(*conf.tags); - if (auto error = parsed_tags.if_error()) { - tags_metadata.error = *error; - } + if (!conf.tags) { + reset_config(ConfigName::TAGS, span_defaults_, metadata); + } else { + ConfigMetadata tags_metadata(ConfigName::TAGS, join_tags(*conf.tags), + ConfigMetadata::Origin::REMOTE_CONFIG); - if (*parsed_tags != span_defaults_.value()->tags) { - auto new_span_defaults = - std::make_shared(*span_defaults_.value()); - new_span_defaults->tags = std::move(*parsed_tags); + if (*conf.tags != span_defaults_.value()->tags) { + auto new_span_defaults = + std::make_shared(*span_defaults_.value()); + new_span_defaults->tags = std::move(*conf.tags); - span_defaults_ = new_span_defaults; - metadata.emplace_back(std::move(tags_metadata)); + span_defaults_ = new_span_defaults; + metadata.emplace_back(std::move(tags_metadata)); + } } - } - if (!conf.report_traces) { - reset_config(ConfigName::REPORT_TRACES, report_traces_, metadata); - } else { - if (conf.report_traces != report_traces_.value()) { - report_traces_ = *conf.report_traces; - metadata.emplace_back(ConfigName::REPORT_TRACES, - to_string(*conf.report_traces), - ConfigMetadata::Origin::REMOTE_CONFIG); + if (!conf.report_traces) { + reset_config(ConfigName::REPORT_TRACES, report_traces_, metadata); + } else { + if (conf.report_traces != report_traces_.value()) { + report_traces_ = *conf.report_traces; + metadata.emplace_back(ConfigName::REPORT_TRACES, + to_string(*conf.report_traces), + ConfigMetadata::Origin::REMOTE_CONFIG); + } } } - return metadata; + telemetry::capture_configuration_change(metadata); } template diff --git a/src/datadog/config_manager.h b/src/datadog/config_manager.h index a19824a4..997f4036 100644 --- a/src/datadog/config_manager.h +++ b/src/datadog/config_manager.h @@ -25,11 +25,11 @@ class ConfigManager : public remote_config::Listener { // // Configurations can be `nullopt` to signal the absence of a value from the // remote configuration value. - struct Update { + struct Update final { Optional report_traces; - Optional trace_sampling_rate; - Optional> tags; - const nlohmann::json* trace_sampling_rules = nullptr; + Optional trace_sampling_rate; + Optional> tags; + Optional> trace_sampling_rules; }; private: @@ -105,7 +105,7 @@ class ConfigManager : public remote_config::Listener { // object. nlohmann::json config_json() const; - std::vector apply_update(const ConfigManager::Update& conf); + void apply_update(const ConfigManager::Update& conf); }; } // namespace tracing diff --git a/src/datadog/telemetry/telemetry.cpp b/src/datadog/telemetry/telemetry.cpp index 95a4433a..3cd51545 100644 --- a/src/datadog/telemetry/telemetry.cpp +++ b/src/datadog/telemetry/telemetry.cpp @@ -35,16 +35,16 @@ struct Ctor_param final { tracing::Clock clock = tracing::default_clock; }; -TelemetryProxy make_telemetry(const Ctor_param& init) { - if (!init.configuration.enabled) return NoopTelemetry{}; - return Telemetry{init.configuration, init.tracer_signature, init.logger, - init.client, init.scheduler, init.agent_url, - init.clock}; +TelemetryProxy make_telemetry(const tracing::Optional& init) { + if (!init || !init->configuration.enabled) return NoopTelemetry{}; + return Telemetry{init->configuration, init->tracer_signature, init->logger, + init->client, init->scheduler, init->agent_url, + init->clock}; } TelemetryProxy& instance( const tracing::Optional& init = tracing::nullopt) { - static TelemetryProxy telemetry = make_telemetry(*init); + static TelemetryProxy telemetry = make_telemetry(init); return telemetry; } diff --git a/src/datadog/trace_sampler.cpp b/src/datadog/trace_sampler.cpp index 153b2610..1831581f 100644 --- a/src/datadog/trace_sampler.cpp +++ b/src/datadog/trace_sampler.cpp @@ -14,7 +14,6 @@ namespace datadog { namespace tracing { -namespace { nlohmann::json to_json(const TraceSamplerRule& rule) { nlohmann::json j = rule.matcher; @@ -22,8 +21,6 @@ nlohmann::json to_json(const TraceSamplerRule& rule) { return j; } -} // namespace - TraceSampler::TraceSampler(const FinalizedTraceSamplerConfig& config, const Clock& clock) : rules_(config.rules), diff --git a/src/datadog/trace_sampler.h b/src/datadog/trace_sampler.h index 034e6f31..78b27cd7 100644 --- a/src/datadog/trace_sampler.h +++ b/src/datadog/trace_sampler.h @@ -127,5 +127,7 @@ class TraceSampler { nlohmann::json config_json() const; }; +nlohmann::json to_json(const TraceSamplerRule&); + } // namespace tracing } // namespace datadog diff --git a/test/test_config_manager.cpp b/test/test_config_manager.cpp index 4443cb64..a9a257d1 100644 --- a/test/test_config_manager.cpp +++ b/test/test_config_manager.cpp @@ -1,3 +1,5 @@ +#include + #include "catch.hpp" #include "datadog/config_manager.h" #include "datadog/remote_config/listener.h" @@ -38,50 +40,112 @@ CONFIG_MANAGER_TEST("remote configuration handling") { rc::product::Flag::APM_TRACING}; SECTION("handling of `tracing_sampling_rate`") { - // SECTION("invalid value") { - // config_update.content = R"({ - // "lib_config": { - // "library_language": "all", - // "library_version": "latest", - // "service_name": "testsvc", - // "env": "test", - // "tracing_enabled": false, - // "tracing_sampling_rate": 100.0, - // "tracing_tags": [ - // "hello:world", - // "foo:bar" - // ] - // }, - // "service_target": { - // "service": "testsvc", - // "env": "test" - // } - // })"; - // - // const auto old_trace_sampler_config = - // config_manager.trace_sampler()->config_json(); - // - // const auto err = config_manager.on_update(config_update); - // CHECK(!err); - // - // const auto new_trace_sampler_config = - // config_manager.trace_sampler()->config_json(); - // - // CHECK(old_trace_sampler_config == new_trace_sampler_config); - // } - SECTION("valid value") { + SECTION("assess field validation") { + struct TestCase { + size_t line; + std::string_view name; + std::string_view input; + }; + + const auto test_case = GENERATE(values({ + { + __LINE__, + "rate outside of [0;1] range 1/2", + R"("tracing_sampling_rate": 100)", + }, + { + __LINE__, + "rate outside of [0;1] range 2/2", + R"("tracing_sampling_rate": -0.2)", + }, + { + __LINE__, + "not a number 1/2", + R"("tracing_sampling_rate": "quarante-deux")", + }, + { + __LINE__, + "not a number 2/2", + R"("tracing_sampling_rate": true)", + }, + { + __LINE__, + "not a number 2/2", + R"("tracing_sampling_rate": {"value": 0.5})", + }, + })); + + char payload[1024]; + std::snprintf(payload, 1024, R"({ + "lib_config": { + "library_language": "all", + "library_version": "latest", + "service_name": "testsvc", + "env": "test", + %s + }, + "service_target": { + "service": "testsvc", + "env": "test" + } + })", + test_case.input.data()); + + config_update.content = payload; + + CAPTURE(test_case.line); + CAPTURE(test_case.name); + + const auto old_trace_sampler_config = + config_manager.trace_sampler()->config_json(); + + const auto err = config_manager.on_update(config_update); + CHECK(err); + + const auto new_trace_sampler_config = + config_manager.trace_sampler()->config_json(); + + CHECK(old_trace_sampler_config == new_trace_sampler_config); + } + + SECTION( + "an RC payload without the `tracing_sampling_rate` does not raise an " + "error nor update the sampling rate") { + config_update.content = R"({ + "lib_config": { + "library_language": "all", + "library_version": "latest", + "service_name": "testsvc", + "env": "test" + }, + "service_target": { + "service": "testsvc", + "env": "test" + } + })"; + + const auto old_trace_sampler_config = + config_manager.trace_sampler()->config_json(); + + const auto err = config_manager.on_update(config_update); + CHECK(!err); + + const auto new_trace_sampler_config = + config_manager.trace_sampler()->config_json(); + + CHECK(old_trace_sampler_config == new_trace_sampler_config); + } + + SECTION( + "A valid RC payload update the sampling rules and reverting restore " + "the initial value") { config_update.content = R"({ "lib_config": { "library_language": "all", "library_version": "latest", "service_name": "testsvc", "env": "test", - "tracing_enabled": false, - "tracing_sampling_rate": 0.6, - "tracing_tags": [ - "hello:world", - "foo:bar" - ] + "tracing_sampling_rate": 0.6 }, "service_target": { "service": "testsvc", @@ -110,14 +174,99 @@ CONFIG_MANAGER_TEST("remote configuration handling") { } SECTION("handling of `tracing_tags`") { - config_update.content = R"({ + SECTION("assess field validation returns an error") { + struct TestCase { + size_t line; + std::string_view name; + std::string_view input; + }; + + const auto test_case = GENERATE(values({ + { + __LINE__, + "not an array", + R"("tracing_tags": 15)", + }, + { + __LINE__, + "not an array", + R"("tracing_tags": "foo")", + }, + { + __LINE__, + "not an array", + R"("tracing_tags": {"key": "a", "value": "b"})", + }, + })); + + CAPTURE(test_case.line); + CAPTURE(test_case.name); + + char payload[1024]; + std::snprintf(payload, 1024, R"({ + "lib_config": { + "library_language": "all", + "library_version": "latest", + "service_name": "testsvc", + "env": "test", + %s + }, + "service_target": { + "service": "testsvc", + "env": "test" + } + })", + test_case.input.data()); + + config_update.content = payload; + + const auto old_tags = config_manager.span_defaults()->tags; + + const auto err = config_manager.on_update(config_update); + CHECK(err); + + const auto new_tags = config_manager.span_defaults()->tags; + + // Make sure tags are not updated + CHECK(old_tags == new_tags); + } + + SECTION( + "payload without `tracing_tags` does not raise an error nor update the " + "list of tags") { + config_update.content = R"({ + "lib_config": { + "library_language": "all", + "library_version": "latest", + "service_name": "testsvc", + "env": "test" + }, + "service_target": { + "service": "testsvc", + "env": "test" + } + })"; + + const auto old_tags = config_manager.span_defaults()->tags; + + const auto err = config_manager.on_update(config_update); + CHECK(!err); + + const auto new_tags = config_manager.span_defaults()->tags; + + // Make sure tags are not updated + CHECK(old_tags == new_tags); + } + + SECTION( + "A valid RC payload does update the list of tags and reverting restore " + "the initial list of tags") { + config_update.content = R"({ "lib_config": { "library_language": "all", "library_version": "latest", "service_name": "testsvc", "env": "test", - "tracing_enabled": false, - "tracing_sampling_rate": 0.6, "tracing_tags": [ "hello:world", "foo:bar" @@ -129,28 +278,112 @@ CONFIG_MANAGER_TEST("remote configuration handling") { } })"; - const std::unordered_map expected_tags{ - {"hello", "world"}, {"foo", "bar"}}; + const std::unordered_map expected_tags{ + {"hello", "world"}, {"foo", "bar"}}; - const auto old_tags = config_manager.span_defaults()->tags; + const auto old_tags = config_manager.span_defaults()->tags; - const auto err = config_manager.on_update(config_update); - CHECK(!err); + const auto err = config_manager.on_update(config_update); + CHECK(!err); - const auto new_tags = config_manager.span_defaults()->tags; + const auto new_tags = config_manager.span_defaults()->tags; - CHECK(old_tags != new_tags); - CHECK(new_tags == expected_tags); + CHECK(old_tags != new_tags); + CHECK(new_tags == expected_tags); - config_manager.on_revert(config_update); + config_manager.on_revert(config_update); - const auto reverted_tags = config_manager.span_defaults()->tags; + const auto reverted_tags = config_manager.span_defaults()->tags; - CHECK(old_tags == reverted_tags); + CHECK(old_tags == reverted_tags); + } } SECTION("handling of `tracing_enabled`") { - config_update.content = R"({ + SECTION("validation") { + struct TestCase { + size_t line; + std::string_view name; + std::string_view input; + }; + + const auto test_case = GENERATE(values({ + { + __LINE__, + "not a boolean", + R"("tracing_enabled": "false")", + }, + { + __LINE__, + "not a boolean 2/x", + R"("tracing_enabled": ["false"])", + }, + { + __LINE__, + "not a boolean 2/x", + R"("tracing_enabled": 26)", + }, + })); + + CAPTURE(test_case.line); + CAPTURE(test_case.name); + + char payload[1024]; + std::snprintf(payload, 1024, R"({ + "lib_config": { + "library_language": "all", + "library_version": "latest", + "service_name": "testsvc", + "env": "test", + %s + }, + "service_target": { + "service": "testsvc", + "env": "test" + } + })", + test_case.input.data()); + + config_update.content = payload; + + const auto old_tracing_status = config_manager.report_traces(); + + const auto err = config_manager.on_update(config_update); + CHECK(err); + + const auto new_tracing_status = config_manager.report_traces(); + + CHECK(old_tracing_status == new_tracing_status); + } + + SECTION( + "An RC payload without `tracing_enabled` does not raise an error nor " + "update the value") { + config_update.content = R"({ + "lib_config": { + "library_language": "all", + "library_version": "latest", + "service_name": "testsvc", + "env": "test" + }, + "service_target": { + "service": "testsvc", + "env": "test" + } + })"; + + const auto old_tracing_status = config_manager.report_traces(); + + const auto err = config_manager.on_update(config_update); + CHECK(!err); + + const auto new_tracing_status = config_manager.report_traces(); + + CHECK(old_tracing_status == new_tracing_status); + } + + SECTION("valid") { + config_update.content = R"({ "lib_config": { "library_language": "all", "library_version": "latest", @@ -169,23 +402,154 @@ CONFIG_MANAGER_TEST("remote configuration handling") { } })"; - const auto old_tracing_status = config_manager.report_traces(); + const auto old_tracing_status = config_manager.report_traces(); - const auto err = config_manager.on_update(config_update); - CHECK(!err); + const auto err = config_manager.on_update(config_update); + CHECK(!err); - const auto new_tracing_status = config_manager.report_traces(); + const auto new_tracing_status = config_manager.report_traces(); - CHECK(old_tracing_status != new_tracing_status); - CHECK(new_tracing_status == false); + CHECK(old_tracing_status != new_tracing_status); + CHECK(new_tracing_status == false); - config_manager.on_revert(config_update); + config_manager.on_revert(config_update); - const auto reverted_tracing_status = config_manager.report_traces(); - CHECK(old_tracing_status == reverted_tracing_status); + const auto reverted_tracing_status = config_manager.report_traces(); + CHECK(old_tracing_status == reverted_tracing_status); + } } SECTION("handling of `tracing_sampling_rules`") { + SECTION("validation") { + struct TestCase { + size_t line; + std::string_view name; + std::string_view input; + }; + + const auto test_case = GENERATE(values({ + { + __LINE__, + "not an array", + R"("tracing_sampling_rules": "service:a,sample_rate:12")", + }, + { + __LINE__, + "not an array 2/x", + R"("tracing_sampling_rules": 28)", + }, + { + __LINE__, + "not a valid sampling rules 1/x", + R"("tracing_sampling_rules": ["foo", "bar"])", + }, + { + __LINE__, + "missing required fields 1/x", + R"("tracing_sampling_rules": [{"foo": "bar"}])", + }, + { + __LINE__, + "missing required fields 2/x", + R"("tracing_sampling_rules": [{"service": "bar"}])", + }, + { + __LINE__, + "missing required fields 3/x", + R"("tracing_sampling_rules": [{"service": "bar", "resource": "yo"}])", + }, + { + __LINE__, + "missing required fields 3/x", + R"("tracing_sampling_rules": [{"service": "bar", "resource": "yo", "sample_rate": 0.2}])", + }, + { + __LINE__, + "invalid value for `service` field", + R"("tracing_sampling_rules": [{"service": ["a", "b"], "resource": "yo", "sample_rate": 0.2, "provenance": "customer"}])", + }, + { + __LINE__, + "invalid value for `resource` field", + R"("tracing_sampling_rules": [{"service": "a", "resource": true, "sample_rate": 0.2, "provenance": "customer"}])", + }, + { + __LINE__, + "invalid value for `provenance` field", + R"("tracing_sampling_rules": [{"service": "bar", "resource": "yo", "sample_rate": 0.2, "provenance": "ui"}])", + }, + { + __LINE__, + "invalid value for `sample_rate` field", + R"("tracing_sampling_rules": [{"service": "bar", "resource": "yo", "sample_rate": "0.5", "provenance": "customer"}])", + }, + { + __LINE__, + "invalid value for `tags` field", + R"("tracing_sampling_rules": [{"service": "bar", "resource": "yo", "sample_rate": "0.5", "provenance": "customer", "tags": "tag1"}])", + }, + { + __LINE__, + "invalid second rules", + R"("tracing_sampling_rules": [{"service": "bar", "resource": "yo", "sample_rate": "0.5", "provenance": "customer"}, {"foo": "bar"}])", + }, + })); + + CAPTURE(test_case.line); + CAPTURE(test_case.name); + + char payload[1024]; + std::snprintf(payload, 1024, R"({ + "lib_config": { + "library_language": "all", + "library_version": "latest", + "service_name": "testsvc", + "env": "test", + %s + }, + "service_target": { + "service": "testsvc", + "env": "test" + } + })", + test_case.input.data()); + + config_update.content = payload; + + const auto old_sampler_cfg = + config_manager.trace_sampler()->config_json(); + const auto err = config_manager.on_update(config_update); + CHECK(err); + const auto new_sampler_cfg = + config_manager.trace_sampler()->config_json(); + + CHECK(old_sampler_cfg == new_sampler_cfg); + } + + SECTION("empty") { + config_update.content = R"({ + "lib_config": { + "library_language": "all", + "library_version": "latest", + "service_name": "testsvc", + "env": "test" + }, + "service_target": { + "service": "testsvc", + "env": "test" + } + })"; + + const auto old_sampler_cfg = + config_manager.trace_sampler()->config_json(); + const auto err = config_manager.on_update(config_update); + CHECK(!err); + const auto new_sampler_cfg = + config_manager.trace_sampler()->config_json(); + + CHECK(old_sampler_cfg == new_sampler_cfg); + } + SECTION("valid input") { config_update.content = R"({ "lib_config": { @@ -215,10 +579,18 @@ CONFIG_MANAGER_TEST("remote configuration handling") { const auto old_sampler_cfg = config_manager.trace_sampler()->config_json(); const auto err = config_manager.on_update(config_update); + CHECK(!err); const auto new_sampler_cfg = config_manager.trace_sampler()->config_json(); CHECK(old_sampler_cfg != new_sampler_cfg); + + config_manager.on_revert({}); + + const auto reverted_sampler_cfg = + config_manager.trace_sampler()->config_json(); + + CHECK(old_sampler_cfg == reverted_sampler_cfg); } } } From 2fb3270ea532994f07afeaedd395c7d8f0453a30 Mon Sep 17 00:00:00 2001 From: Damien Mehala Date: Wed, 3 Sep 2025 10:35:55 +0200 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Zach Montoya --- test/test_config_manager.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_config_manager.cpp b/test/test_config_manager.cpp index a9a257d1..9c67133c 100644 --- a/test/test_config_manager.cpp +++ b/test/test_config_manager.cpp @@ -486,12 +486,12 @@ CONFIG_MANAGER_TEST("remote configuration handling") { { __LINE__, "invalid value for `tags` field", - R"("tracing_sampling_rules": [{"service": "bar", "resource": "yo", "sample_rate": "0.5", "provenance": "customer", "tags": "tag1"}])", + R"("tracing_sampling_rules": [{"service": "bar", "resource": "yo", "sample_rate": 0.2, "provenance": "customer", "tags": "tag1"}])", }, { __LINE__, "invalid second rules", - R"("tracing_sampling_rules": [{"service": "bar", "resource": "yo", "sample_rate": "0.5", "provenance": "customer"}, {"foo": "bar"}])", + R"("tracing_sampling_rules": [{"service": "bar", "resource": "yo", "sample_rate": 0.2, "provenance": "customer"}, {"foo": "bar"}])", }, }));