From e4d44b9c78485416f2947fa717c09998acbfc74e Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Thu, 7 Dec 2023 11:32:25 -0500 Subject: [PATCH 01/21] update fuzz documentation --- bin/cmake-build | 2 +- fuzz/README.md | 21 ++++++++------------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/bin/cmake-build b/bin/cmake-build index 1ba61e27..34926942 100755 --- a/bin/cmake-build +++ b/bin/cmake-build @@ -6,5 +6,5 @@ cd "$(dirname "$0")"/.. mkdir -p .build cd .build -cmake .. +cmake .. "$@" make -j "$(nproc)" diff --git a/fuzz/README.md b/fuzz/README.md index 3f0181c3..648d3e40 100644 --- a/fuzz/README.md +++ b/fuzz/README.md @@ -3,24 +3,19 @@ Fuzzers Each subdirectory here contains the source of an executable that [fuzz tests][1] some part of the library using [LLVM's libfuzzer][2]. -There is a toplevel CMake boolean option associated with each fuzzer. The naming -convention is `FUZZ_`, e.g. -`FUZZ_W3C_PROPAGATION` for the fuzzer defined in -[fuzz/w3c-propagation/](./w3c-propagation/). The resulting binary is called -`fuzz` by convention. +There is a toplevel CMake boolean option that adds all of the fuzzer +executables to the build: `BUILD_FUZZERS`. -When building a fuzzer, the toolchain must be clang-based. For example, this -is how to build the fuzzer in [fuzz/w3c-propagation](./w3c-propagation/) from -the root of the repository: +When building the fuzzers, the toolchain must be clang-based. For example: ```console -$ rm -rf .build && mkdir .build # if toolchain or test setup need clearing -$ cd .build -$ CC=clang CXX=clang++ cmake .. -DFUZZ_W3C_PROPAGATION=ON -$ make -j $(nproc) -$ fuzz/w3c-propagation/fuzz +$ rm -rf .build # if toolchain needs clearing +$ bin/with-toolchain llvm bin/cmake-build -DBUILD_FUZZERS=1 +$ .build/fuzz/w3c-propagation/w3c-propagation-fuzz [... fuzzer output ...] ``` +The fuzzer executables are named `.build/fuzz/*/*-fuzz` by convention. + [1]: https://en.wikipedia.org/wiki/Fuzzing [2]: https://llvm.org/docs/LibFuzzer.html From b7c0d0e39a6260ae82fbeb784d44bf666b6e28b9 Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Thu, 7 Dec 2023 12:08:15 -0500 Subject: [PATCH 02/21] revise the base64 decoder --- fuzz/base64/main.cpp | 4 +- src/datadog/base64.cpp | 133 ++++++++++++++++------------------ src/datadog/base64.h | 14 ++-- src/datadog/remote_config.cpp | 4 +- test/test_base64.cpp | 18 ++--- 5 files changed, 81 insertions(+), 92 deletions(-) diff --git a/fuzz/base64/main.cpp b/fuzz/base64/main.cpp index 805a36c0..bd0e9712 100644 --- a/fuzz/base64/main.cpp +++ b/fuzz/base64/main.cpp @@ -1,9 +1,11 @@ #include #include +#include + namespace dd = datadog::tracing; extern "C" int LLVMFuzzerTestOneInput(const std::uint8_t* data, size_t size) { - dd::base64::decode(dd::StringView{(char*)data, size}); + dd::base64_decode(dd::StringView{(const char*)data, size}); return 0; } diff --git a/src/datadog/base64.cpp b/src/datadog/base64.cpp index c134a7dd..515dab67 100644 --- a/src/datadog/base64.cpp +++ b/src/datadog/base64.cpp @@ -1,109 +1,100 @@ #include "base64.h" -#include +#include +#include namespace datadog { namespace tracing { -namespace base64 { -#define _ 255 -#define SENTINEL_VALUE _ -#define EOL 0 +constexpr uint8_t k_sentinel = 255; +constexpr uint8_t _ = k_sentinel; // for brevity +constexpr uint8_t k_eol = 0; -/* - * Lookup table mapping the base64 table. Invalid inputs are mapped - * to the value 255. - * `=` map to 0. - */ +// Invalid inputs are mapped to the value 255. '=' maps to 0. constexpr uint8_t k_base64_table[]{ - _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, - _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, - _, _, _, _, _, 62, _, _, _, 63, 52, 53, 54, 55, 56, 57, 58, 59, 60, - 61, _, _, _, EOL, _, _, _, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, - 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, _, _, _, _, - _, _, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, - 43, 44, 45, 46, 47, 48, 49, 50, 51, _, _, _, _, _, _, _, _, _, _, - _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, - _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, - _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, - _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, - _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, - _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, - _, _, _, _, _, _, _, _, _}; - -// TODO: support input without padding? -std::string decode(StringView in) { - const std::size_t in_size = in.size(); - - std::string out; - out.reserve(in_size); + _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, + _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, + _, _, _, _, _, _, _, 62, _, _, _, 63, 52, 53, 54, 55, 56, 57, + 58, 59, 60, 61, _, _, _, k_eol, _, _, _, 0, 1, 2, 3, 4, 5, 6, + 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, + 25, _, _, _, _, _, _, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, + 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, _, _, _, + _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, + _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, + _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, + _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, + _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, + _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, + _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, + _, _, _, _}; + +std::string base64_decode(StringView input) { + const size_t in_size = input.size(); + + std::string output; + output.reserve(in_size); union { uint32_t buffer; uint8_t bytes[4]; } decoder; - std::size_t i = 0; + size_t i = 0; for (; i + 4 < in_size;) { - auto c0 = k_base64_table[static_cast(in[i++])]; - auto c1 = k_base64_table[static_cast(in[i++])]; - auto c2 = k_base64_table[static_cast(in[i++])]; - auto c3 = k_base64_table[static_cast(in[i++])]; + uint32_t c0 = k_base64_table[static_cast(input[i++])]; + uint32_t c1 = k_base64_table[static_cast(input[i++])]; + uint32_t c2 = k_base64_table[static_cast(input[i++])]; + uint32_t c3 = k_base64_table[static_cast(input[i++])]; - if (c0 == SENTINEL_VALUE || c1 == SENTINEL_VALUE || c2 == SENTINEL_VALUE || - c3 == SENTINEL_VALUE) { + if (c0 == k_sentinel || c1 == k_sentinel || c2 == k_sentinel || + c3 == k_sentinel) { return ""; } - decoder.buffer = - static_cast(0) | static_cast(c0) << 26 | - static_cast(c1) << 20 | static_cast(c2) << 14 | - static_cast(c3) << 8; + decoder.buffer = 0 | c0 << (32 - 6 * 1) | c1 << (32 - 6 * 2) | + c2 << (32 - 6 * 3) | c3 << (32 - 6 * 4); - // NOTE(@dmehala): It might seem confusion to read those bytes in reverse + // NOTE(@dmehala): It might seem confusing to read those bytes input reverse // order. It is related to the architecture endianess. For now the set of // architecture we support (x86_64 and arm64) are all little-endian. - out.push_back(decoder.bytes[3]); - out.push_back(decoder.bytes[2]); - out.push_back(decoder.bytes[1]); + // TODO(@dgoffredo): I'd prefer an endian-agnostic implementation. + // nginx-datadog targets x86_64 and arm64 in its binary releases, but + // dd-trace-cpp targets any standard C++17 compiler. + output.push_back(decoder.bytes[3]); + output.push_back(decoder.bytes[2]); + output.push_back(decoder.bytes[1]); } - if ((in_size - i) < 4) return ""; // not padded input is not supported + // If padding is missing, return the empty string in lieu of an Error. + if ((in_size - i) < 4) return ""; - auto c0 = k_base64_table[static_cast(in[i++])]; - auto c1 = k_base64_table[static_cast(in[i++])]; - auto c2 = k_base64_table[static_cast(in[i++])]; - auto c3 = k_base64_table[static_cast(in[i++])]; + uint32_t c0 = k_base64_table[static_cast(input[i++])]; + uint32_t c1 = k_base64_table[static_cast(input[i++])]; + uint32_t c2 = k_base64_table[static_cast(input[i++])]; + uint32_t c3 = k_base64_table[static_cast(input[i++])]; - if (c0 == SENTINEL_VALUE || c1 == SENTINEL_VALUE || c2 == SENTINEL_VALUE || - c3 == SENTINEL_VALUE) { + if (c0 == k_sentinel || c1 == k_sentinel || c2 == k_sentinel || + c3 == k_sentinel) { return ""; } - decoder.buffer = static_cast(0) | static_cast(c0) << 26 | - static_cast(c1) << 20 | - static_cast(c2) << 14 | - static_cast(c3) << 8; + decoder.buffer = 0 | c0 << (32 - 6 * 1) | c1 << (32 - 6 * 2) | + c2 << (32 - 6 * 3) | c3 << (32 - 6 * 4); - if (c2 == EOL) { - out.push_back(decoder.bytes[3]); - } else if (c3 == EOL) { - out.push_back(decoder.bytes[3]); - out.push_back(decoder.bytes[2]); + if (c2 == k_eol) { + output.push_back(decoder.bytes[3]); + } else if (c3 == k_eol) { + output.push_back(decoder.bytes[3]); + output.push_back(decoder.bytes[2]); } else { - out.push_back(decoder.bytes[3]); - out.push_back(decoder.bytes[2]); - out.push_back(decoder.bytes[1]); + output.push_back(decoder.bytes[3]); + output.push_back(decoder.bytes[2]); + output.push_back(decoder.bytes[1]); } - return out; + return output; } -#undef EOL -#undef SENTINEL_VALUE -#undef _ - -} // namespace base64 } // namespace tracing } // namespace datadog diff --git a/src/datadog/base64.h b/src/datadog/base64.h index a594f1df..f2539872 100644 --- a/src/datadog/base64.h +++ b/src/datadog/base64.h @@ -1,19 +1,15 @@ #pragma once +#include + #include "string_view.h" namespace datadog { namespace tracing { -namespace base64 { -/* - * Decode a base64-encoded string and returns the decoded data. - * - * It only supported padded base64-encoded string. Providing an unpadded - * input will return an empty string. - */ -std::string decode(StringView in); +// Return the result of decoding the specified padded base64-encoded `input`. If +// `input` is not padded, then return the empty string instead. +std::string base64_decode(StringView input); -} // namespace base64 } // namespace tracing } // namespace datadog diff --git a/src/datadog/remote_config.cpp b/src/datadog/remote_config.cpp index 58784c84..f744103e 100644 --- a/src/datadog/remote_config.cpp +++ b/src/datadog/remote_config.cpp @@ -109,7 +109,7 @@ void RemoteConfigurationManager::process_response(const nlohmann::json& json) { try { const auto targets = nlohmann::json::parse( - base64::decode(json.at("targets").get())); + base64_decode(json.at("targets").get())); state_.targets_version = targets.at("/signed/version"_json_pointer); state_.opaque_backend_state = @@ -159,7 +159,7 @@ void RemoteConfigurationManager::process_response(const nlohmann::json& json) { } const auto config_json = nlohmann::json::parse( - base64::decode(target_it.value().at("raw").get())); + base64_decode(target_it.value().at("raw").get())); const auto& targeted_service = config_json.at("service_target"); if (targeted_service.at("service").get() != diff --git a/test/test_base64.cpp b/test/test_base64.cpp index 7c2b1f13..298b6577 100644 --- a/test/test_base64.cpp +++ b/test/test_base64.cpp @@ -5,28 +5,28 @@ #define BASE64_TEST(x) TEST_CASE(x, "[base64]") -namespace base64 = datadog::tracing::base64; +using namespace datadog::tracing; -BASE64_TEST("empty input") { CHECK(base64::decode("") == ""); } +BASE64_TEST("empty input") { CHECK(base64_decode("") == ""); } BASE64_TEST("invalid inputs") { SECTION("invalid characters") { - CHECK(base64::decode("InvalidData@") == ""); - CHECK(base64::decode("In@#*!^validData") == ""); + CHECK(base64_decode("InvalidData@") == ""); + CHECK(base64_decode("In@#*!^validData") == ""); } SECTION("single character without padding") { - CHECK(base64::decode("V") == ""); + CHECK(base64_decode("V") == ""); } } BASE64_TEST("unpadded input") { - CHECK(base64::decode("VGVzdGluZyBtdWx0aXBsZSBvZiA0IHBhZGRpbmcu") == + CHECK(base64_decode("VGVzdGluZyBtdWx0aXBsZSBvZiA0IHBhZGRpbmcu") == "Testing multiple of 4 padding."); } BASE64_TEST("padding") { - CHECK(base64::decode("bGlnaHQgdw==") == "light w"); - CHECK(base64::decode("bGlnaHQgd28=") == "light wo"); - CHECK(base64::decode("bGlnaHQgd29y") == "light wor"); + CHECK(base64_decode("bGlnaHQgdw==") == "light w"); + CHECK(base64_decode("bGlnaHQgd28=") == "light wo"); + CHECK(base64_decode("bGlnaHQgd29y") == "light wor"); } From e3f0ce34f3794127147d56f7f34b9afd7d3d603a Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Thu, 7 Dec 2023 12:40:55 -0500 Subject: [PATCH 03/21] move ConfigManager into its own .cpp --- BUILD.bazel | 1 + CMakeLists.txt | 1 + src/datadog/config_manager.cpp | 45 +++++++++++++++++++++++++++++ src/datadog/config_manager.h | 52 +++++++++++----------------------- src/datadog/remote_config.cpp | 4 +-- src/datadog/remote_config.h | 2 +- 6 files changed, 66 insertions(+), 39 deletions(-) create mode 100644 src/datadog/config_manager.cpp diff --git a/BUILD.bazel b/BUILD.bazel index 500c56e5..91fd94d5 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -4,6 +4,7 @@ cc_library( "src/datadog/base64.cpp", "src/datadog/cerr_logger.cpp", "src/datadog/clock.cpp", + "src/datadog/config_manager.cpp", "src/datadog/collector_response.cpp", # "src/datadog/curl.cpp", no libcurl "src/datadog/datadog_agent_config.cpp", diff --git a/CMakeLists.txt b/CMakeLists.txt index b03ad992..29fa8618 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -104,6 +104,7 @@ target_sources(dd_trace_cpp-objects PRIVATE src/datadog/base64.cpp src/datadog/cerr_logger.cpp src/datadog/clock.cpp + src/datadog/config_manager.cpp src/datadog/collector_response.cpp src/datadog/curl.cpp src/datadog/datadog_agent_config.cpp diff --git a/src/datadog/config_manager.cpp b/src/datadog/config_manager.cpp new file mode 100644 index 00000000..cb441bf7 --- /dev/null +++ b/src/datadog/config_manager.cpp @@ -0,0 +1,45 @@ +#include "config_manager.h" + +namespace datadog { +namespace tracing { + +ConfigManager::ConfigManager(const FinalizedTracerConfig& config) + : clock_(config.clock), + default_trace_sampler_( + std::make_shared(config.trace_sampler, clock_)), + current_trace_sampler_(default_trace_sampler_) {} + +std::shared_ptr ConfigManager::get_trace_sampler() { + std::lock_guard lock(mutex_); + return current_trace_sampler_; +} + +void ConfigManager::update(const ConfigManager::Update& conf) { + std::lock_guard lock(mutex_); + + if (conf.trace_sampler) { + if (auto finalized_trace_sampler_cfg = + finalize_config(*conf.trace_sampler)) { + current_trace_sampler_ = + std::make_shared(*finalized_trace_sampler_cfg, clock_); + } else { + // TODO: report error + } + } else { + current_trace_sampler_ = default_trace_sampler_; + } +} + +void ConfigManager::reset() { + std::lock_guard lock(mutex_); + current_trace_sampler_ = default_trace_sampler_; +} + +nlohmann::json ConfigManager::config_json() const { + std::lock_guard lock(mutex_); + return nlohmann::json{ + {"trace_sampler", current_trace_sampler_->config_json()}}; +} + +} // namespace tracing +} // namespace datadog diff --git a/src/datadog/config_manager.h b/src/datadog/config_manager.h index 61fa88cd..f1dcf98d 100644 --- a/src/datadog/config_manager.h +++ b/src/datadog/config_manager.h @@ -1,5 +1,7 @@ #pragma once +// TODO: Document + #include #include "clock.h" @@ -10,10 +12,6 @@ namespace datadog { namespace tracing { -struct ConfigUpdate { - Optional trace_sampler; -}; - class ConfigManager { mutable std::mutex mutex_; Clock clock_; @@ -21,43 +19,25 @@ class ConfigManager { std::shared_ptr current_trace_sampler_; public: - ConfigManager(const FinalizedTracerConfig& config) - : clock_(config.clock), - default_trace_sampler_( - std::make_shared(config.trace_sampler, clock_)), - current_trace_sampler_(default_trace_sampler_) {} + struct Update { + Optional trace_sampler; + }; - std::shared_ptr get_trace_sampler() { - std::lock_guard lock(mutex_); - return current_trace_sampler_; - } + ConfigManager(const FinalizedTracerConfig& config); - void update(const ConfigUpdate& conf) { - std::lock_guard lock(mutex_); + // Return the `TraceSampler` consistent with the most recent configuration. + std::shared_ptr get_trace_sampler(); - if (conf.trace_sampler) { - if (auto finalized_trace_sampler_cfg = - finalize_config(*conf.trace_sampler)) { - current_trace_sampler_ = std::make_shared( - *finalized_trace_sampler_cfg, clock_); - } else { - // TODO: report error - } - } else { - current_trace_sampler_ = default_trace_sampler_; - } - } + // Apply the specified `conf` update. + void update(const Update& conf); - void reset() { - std::lock_guard lock(mutex_); - current_trace_sampler_ = default_trace_sampler_; - } + // Restore the configuration that was passed to this object's constructor, + // overriding any previous calls to `update`. + void reset(); - nlohmann::json config_json() const { - std::lock_guard lock(mutex_); - return nlohmann::json{ - {"trace_sampler", current_trace_sampler_->config_json()}}; - } + // Return a JSON representation of the current configuration managed by this + // object. + nlohmann::json config_json() const; }; } // namespace tracing diff --git a/src/datadog/remote_config.cpp b/src/datadog/remote_config.cpp index f744103e..2a23397d 100644 --- a/src/datadog/remote_config.cpp +++ b/src/datadog/remote_config.cpp @@ -37,7 +37,7 @@ constexpr std::array k_apm_capabilities = } // namespace -void from_json(const nlohmann::json& j, ConfigUpdate& out) { +void from_json(const nlohmann::json& j, ConfigManager::Update& out) { if (auto sampling_rate_it = j.find("tracing_sampling_rate"); sampling_rate_it != j.cend()) { TraceSamplerConfig trace_sampler_cfg; @@ -173,7 +173,7 @@ void RemoteConfigurationManager::process_response(const nlohmann::json& json) { new_config.hash = config_metadata.at("/hashes/sha256"_json_pointer); new_config.id = config_json.at("id"); new_config.version = config_json.at("revision"); - new_config.content = ConfigUpdate(config_json.at("lib_config")); + new_config.content = ConfigManager::Update(config_json.at("lib_config")); apply_config(new_config); applied_config_[std::string{config_path}] = new_config; diff --git a/src/datadog/remote_config.h b/src/datadog/remote_config.h index b30ce7f6..36709f3d 100644 --- a/src/datadog/remote_config.h +++ b/src/datadog/remote_config.h @@ -24,7 +24,7 @@ class RemoteConfigurationManager { std::string id; std::string hash; std::size_t version; - ConfigUpdate content; + ConfigManager::Update content; }; TracerId tracer_id_; From 6d1553b13d4a2e05a755935211f5c15ca8651a24 Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Thu, 7 Dec 2023 12:46:31 -0500 Subject: [PATCH 04/21] remove namespace httputil::header --- src/datadog/datadog_agent.cpp | 31 +++++++++++++++++-------------- src/datadog/http_client.h | 8 -------- 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/src/datadog/datadog_agent.cpp b/src/datadog/datadog_agent.cpp index ca8d6d2d..a544c9df 100644 --- a/src/datadog/datadog_agent.cpp +++ b/src/datadog/datadog_agent.cpp @@ -28,6 +28,10 @@ constexpr StringView traces_api_path = "/v0.4/traces"; constexpr StringView telemetry_v2_path = "/telemetry/proxy/api/v2/apmtelemetry"; constexpr StringView remote_configuration_path = "/v0.7/config"; +void set_content_type_json(DictWriter& headers) { + headers.set("Content-Type", "application/json"); +} + HTTPClient::URL traces_endpoint(const HTTPClient::URL& agent_url) { auto traces_url = agent_url; append(traces_url.path, traces_api_path); @@ -357,10 +361,10 @@ void DatadogAgent::flush() { void DatadogAgent::send_app_started() { auto payload = tracer_telemetry_->app_started(); - auto post_result = http_client_->post( - telemetry_endpoint_, httputil::header::json_content_type_setter, - std::move(payload), telemetry_on_response_, telemetry_on_error_, - clock_().tick + request_timeout_); + auto post_result = + http_client_->post(telemetry_endpoint_, set_content_type_json, + std::move(payload), telemetry_on_response_, + telemetry_on_error_, clock_().tick + request_timeout_); if (auto* error = post_result.if_error()) { logger_->log_error(error->with_prefix( "Unexpected error submitting telemetry app-started event: ")); @@ -369,10 +373,10 @@ void DatadogAgent::send_app_started() { void DatadogAgent::send_heartbeat_and_telemetry() { auto payload = tracer_telemetry_->heartbeat_and_telemetry(); - auto post_result = http_client_->post( - telemetry_endpoint_, httputil::header::json_content_type_setter, - std::move(payload), telemetry_on_response_, telemetry_on_error_, - clock_().tick + request_timeout_); + auto post_result = + http_client_->post(telemetry_endpoint_, set_content_type_json, + std::move(payload), telemetry_on_response_, + telemetry_on_error_, clock_().tick + request_timeout_); if (auto* error = post_result.if_error()) { logger_->log_error(error->with_prefix( "Unexpected error submitting telemetry app-heartbeat event: ")); @@ -381,10 +385,10 @@ void DatadogAgent::send_heartbeat_and_telemetry() { void DatadogAgent::send_app_closing() { auto payload = tracer_telemetry_->app_closing(); - auto post_result = http_client_->post( - telemetry_endpoint_, httputil::header::json_content_type_setter, - std::move(payload), telemetry_on_response_, telemetry_on_error_, - clock_().tick + request_timeout_); + auto post_result = + http_client_->post(telemetry_endpoint_, set_content_type_json, + std::move(payload), telemetry_on_response_, + telemetry_on_error_, clock_().tick + request_timeout_); if (auto* error = post_result.if_error()) { logger_->log_error(error->with_prefix( "Unexpected error submitting telemetry app-closing event: ")); @@ -441,8 +445,7 @@ void DatadogAgent::get_and_apply_remote_configuration_updates() { }; auto post_result = http_client_->post( - remote_configuration_endpoint_, - httputil::header::json_content_type_setter, + remote_configuration_endpoint_, set_content_type_json, remote_config_.make_request_payload().dump(), remote_configuration_on_response, remote_configuration_on_error, clock_().tick + request_timeout_); diff --git a/src/datadog/http_client.h b/src/datadog/http_client.h index 5d77610d..56eaa287 100644 --- a/src/datadog/http_client.h +++ b/src/datadog/http_client.h @@ -67,13 +67,5 @@ class HTTPClient { virtual ~HTTPClient() = default; }; -namespace httputil::header { - -inline void json_content_type_setter(DictWriter& headers) { - headers.set("Content-Type", "application/json"); -} - -} // namespace httputil::header - } // namespace tracing } // namespace datadog From ea1611f65837d6056c45d89eb4f0c98dcb05dc35 Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Thu, 7 Dec 2023 12:52:02 -0500 Subject: [PATCH 05/21] more specific error code --- src/datadog/datadog_agent_config.cpp | 2 +- src/datadog/error.h | 2 +- test/test_tracer_config.cpp | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/datadog/datadog_agent_config.cpp b/src/datadog/datadog_agent_config.cpp index f19f356e..ad7154c9 100644 --- a/src/datadog/datadog_agent_config.cpp +++ b/src/datadog/datadog_agent_config.cpp @@ -133,7 +133,7 @@ Expected finalize_config( } if (rc_poll_interval_seconds <= 0) { - return Error{Error::DATADOG_AGENT_INVALID_CONFIGURATION, + return Error{Error::DATADOG_AGENT_INVALID_REMOTE_CONFIG_POLL_INTERVAL, "DatadogAgent: Remote Configuration poll interval must be a " "positive number of seconds."}; } diff --git a/src/datadog/error.h b/src/datadog/error.h index cce99e23..67accc6a 100644 --- a/src/datadog/error.h +++ b/src/datadog/error.h @@ -73,7 +73,7 @@ struct Error { CURL_DEADLINE_EXCEEDED_BEFORE_REQUEST_START = 48, DATADOG_AGENT_INVALID_REQUEST_TIMEOUT = 49, DATADOG_AGENT_INVALID_SHUTDOWN_TIMEOUT = 50, - DATADOG_AGENT_INVALID_CONFIGURATION = 51, + DATADOG_AGENT_INVALID_REMOTE_CONFIG_POLL_INTERVAL = 51, }; Code code; diff --git a/test/test_tracer_config.cpp b/test/test_tracer_config.cpp index 8ec4e300..7f0df1a1 100644 --- a/test/test_tracer_config.cpp +++ b/test/test_tracer_config.cpp @@ -410,7 +410,7 @@ TEST_CASE("TracerConfig::agent") { auto finalized = finalize_config(config); REQUIRE(!finalized); REQUIRE(finalized.error().code == - Error::DATADOG_AGENT_INVALID_CONFIGURATION); + Error::DATADOG_AGENT_INVALID_REMOTE_CONFIG_POLL_INTERVAL); } SECTION("cannot be negative") { @@ -418,7 +418,7 @@ TEST_CASE("TracerConfig::agent") { auto finalized = finalize_config(config); REQUIRE(!finalized); REQUIRE(finalized.error().code == - Error::DATADOG_AGENT_INVALID_CONFIGURATION); + Error::DATADOG_AGENT_INVALID_REMOTE_CONFIG_POLL_INTERVAL); } SECTION("override default value") { From a1567031a28eda55e69dceacadb4feabea17273a Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Thu, 7 Dec 2023 12:56:38 -0500 Subject: [PATCH 06/21] RuntimeID rc_id_ --> std::string client_id_ --- src/datadog/remote_config.cpp | 5 +++-- src/datadog/remote_config.h | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/datadog/remote_config.cpp b/src/datadog/remote_config.cpp index 2a23397d..e888cfe1 100644 --- a/src/datadog/remote_config.cpp +++ b/src/datadog/remote_config.cpp @@ -7,6 +7,7 @@ #include "base64.h" #include "json.hpp" +#include "random.h" #include "version.h" using namespace nlohmann::literals; @@ -50,7 +51,7 @@ RemoteConfigurationManager::RemoteConfigurationManager( const TracerId& tracer_id, ConfigManager& config_manager) : tracer_id_(tracer_id), config_manager_(config_manager), - rc_id_(RuntimeID::generate()) {} + client_id_(uuid()) {} bool RemoteConfigurationManager::is_new_config( StringView config_path, const nlohmann::json& config_meta) { @@ -65,7 +66,7 @@ nlohmann::json RemoteConfigurationManager::make_request_payload() { // clang-format off auto j = nlohmann::json{ {"client", { - {"id", rc_id_.string()}, + {"id", client_id_}, {"products", nlohmann::json::array({k_apm_product})}, {"is_tracer", true}, {"capabilities", k_apm_capabilities}, diff --git a/src/datadog/remote_config.h b/src/datadog/remote_config.h index 36709f3d..23327ecc 100644 --- a/src/datadog/remote_config.h +++ b/src/datadog/remote_config.h @@ -1,5 +1,7 @@ #pragma once +// TODO: Document + #include #include "config_manager.h" @@ -29,7 +31,7 @@ class RemoteConfigurationManager { TracerId tracer_id_; ConfigManager& config_manager_; - RuntimeID rc_id_; + std::string client_id_; State state_; std::unordered_map applied_config_; From 23531711aff18ff6301a4d3f822f75ef641ba654 Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Thu, 7 Dec 2023 13:03:42 -0500 Subject: [PATCH 07/21] TracerId --> TracerID --- src/datadog/datadog_agent.cpp | 2 +- src/datadog/datadog_agent.h | 4 ++-- src/datadog/remote_config.cpp | 2 +- src/datadog/remote_config.h | 4 ++-- src/datadog/tracer.h | 4 ++-- src/datadog/tracer_id.h | 2 +- src/datadog/tracer_telemetry.cpp | 2 +- src/datadog/tracer_telemetry.h | 4 ++-- test/test_remote_config.cpp | 4 ++-- test/test_tracer_telemetry.cpp | 2 +- 10 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/datadog/datadog_agent.cpp b/src/datadog/datadog_agent.cpp index a544c9df..0d5adcad 100644 --- a/src/datadog/datadog_agent.cpp +++ b/src/datadog/datadog_agent.cpp @@ -149,7 +149,7 @@ std::variant parse_agent_traces_response( DatadogAgent::DatadogAgent( const FinalizedDatadogAgentConfig& config, const std::shared_ptr& tracer_telemetry, - const std::shared_ptr& logger, const TracerId& tracer_id, + const std::shared_ptr& logger, const TracerID& tracer_id, ConfigManager& config_manager) : tracer_telemetry_(tracer_telemetry), clock_(config.clock), diff --git a/src/datadog/datadog_agent.h b/src/datadog/datadog_agent.h index cfc8ddf0..6aa11440 100644 --- a/src/datadog/datadog_agent.h +++ b/src/datadog/datadog_agent.h @@ -26,7 +26,7 @@ class FinalizedDatadogAgentConfig; class Logger; struct SpanData; class TraceSampler; -struct TracerId; +struct TracerID; class DatadogAgent : public Collector { public: @@ -65,7 +65,7 @@ class DatadogAgent : public Collector { public: DatadogAgent(const FinalizedDatadogAgentConfig&, const std::shared_ptr&, - const std::shared_ptr&, const TracerId& id, + const std::shared_ptr&, const TracerID& id, ConfigManager& config_manager); ~DatadogAgent(); diff --git a/src/datadog/remote_config.cpp b/src/datadog/remote_config.cpp index e888cfe1..10d00b38 100644 --- a/src/datadog/remote_config.cpp +++ b/src/datadog/remote_config.cpp @@ -48,7 +48,7 @@ void from_json(const nlohmann::json& j, ConfigManager::Update& out) { } RemoteConfigurationManager::RemoteConfigurationManager( - const TracerId& tracer_id, ConfigManager& config_manager) + const TracerID& tracer_id, ConfigManager& config_manager) : tracer_id_(tracer_id), config_manager_(config_manager), client_id_(uuid()) {} diff --git a/src/datadog/remote_config.h b/src/datadog/remote_config.h index 23327ecc..7e29da53 100644 --- a/src/datadog/remote_config.h +++ b/src/datadog/remote_config.h @@ -29,7 +29,7 @@ class RemoteConfigurationManager { ConfigManager::Update content; }; - TracerId tracer_id_; + TracerID tracer_id_; ConfigManager& config_manager_; std::string client_id_; @@ -37,7 +37,7 @@ class RemoteConfigurationManager { std::unordered_map applied_config_; public: - RemoteConfigurationManager(const TracerId& tracer_id, + RemoteConfigurationManager(const TracerID& tracer_id, ConfigManager& config_manager); nlohmann::json make_request_payload(); diff --git a/src/datadog/tracer.h b/src/datadog/tracer.h index d2556ee9..6c72faea 100644 --- a/src/datadog/tracer.h +++ b/src/datadog/tracer.h @@ -28,14 +28,14 @@ class DictReader; struct SpanConfig; class TraceSampler; class SpanSampler; -struct TracerId; +struct TracerID; class Tracer { std::shared_ptr logger_; std::shared_ptr collector_; std::shared_ptr defaults_; RuntimeID runtime_id_; - TracerId id_; + TracerID id_; std::shared_ptr tracer_telemetry_; std::shared_ptr span_sampler_; std::shared_ptr generator_; diff --git a/src/datadog/tracer_id.h b/src/datadog/tracer_id.h index e89f5534..dd4caf00 100644 --- a/src/datadog/tracer_id.h +++ b/src/datadog/tracer_id.h @@ -6,7 +6,7 @@ namespace datadog { namespace tracing { // Identify a tracer -struct TracerId { +struct TracerID { RuntimeID runtime_id; std::string service; std::string environment; diff --git a/src/datadog/tracer_telemetry.cpp b/src/datadog/tracer_telemetry.cpp index 88a01b6c..79bbb01c 100644 --- a/src/datadog/tracer_telemetry.cpp +++ b/src/datadog/tracer_telemetry.cpp @@ -10,7 +10,7 @@ namespace tracing { TracerTelemetry::TracerTelemetry(bool enabled, const Clock& clock, const std::shared_ptr& logger, - const TracerId& tracer_id) + const TracerID& tracer_id) : enabled_(enabled), clock_(clock), logger_(logger), diff --git a/src/datadog/tracer_telemetry.h b/src/datadog/tracer_telemetry.h index f40ec878..3cc64e57 100644 --- a/src/datadog/tracer_telemetry.h +++ b/src/datadog/tracer_telemetry.h @@ -44,7 +44,7 @@ class TracerTelemetry { bool debug_ = false; Clock clock_; std::shared_ptr logger_; - TracerId tracer_id_; + TracerID tracer_id_; std::string hostname_; uint64_t seq_id_ = 0; // This structure contains all the metrics that are exposed by tracer @@ -100,7 +100,7 @@ class TracerTelemetry { public: TracerTelemetry(bool enabled, const Clock& clock, const std::shared_ptr& logger, - const TracerId& tracer_id); + const TracerID& tracer_id); bool enabled() { return enabled_; }; // Provides access to the telemetry metrics for updating the values. // This value should not be stored. diff --git a/test/test_remote_config.cpp b/test/test_remote_config.cpp index 3b15ee1a..4e015c46 100644 --- a/test/test_remote_config.cpp +++ b/test/test_remote_config.cpp @@ -11,7 +11,7 @@ using namespace datadog::tracing; #define REMOTE_CONFIG_TEST(x) TEST_CASE(x, "[remote_config]") REMOTE_CONFIG_TEST("first payload") { - const TracerId tracer_id{/* runtime_id = */ RuntimeID::generate(), + const TracerID tracer_id{/* runtime_id = */ RuntimeID::generate(), /* service = */ "testsvc", /* environment = */ "test"}; @@ -43,7 +43,7 @@ REMOTE_CONFIG_TEST("first payload") { } REMOTE_CONFIG_TEST("response processing") { - const TracerId tracer_id{/* runtime_id = */ RuntimeID::generate(), + const TracerID tracer_id{/* runtime_id = */ RuntimeID::generate(), /* service = */ "testsvc", /* environment = */ "test"}; diff --git a/test/test_tracer_telemetry.cpp b/test/test_tracer_telemetry.cpp index 8affb195..9e691ad3 100644 --- a/test/test_tracer_telemetry.cpp +++ b/test/test_tracer_telemetry.cpp @@ -22,7 +22,7 @@ TEST_CASE("Tracer telemetry") { }; auto logger = std::make_shared(); - const TracerId tracer_id{/* runtime_id = */ RuntimeID::generate(), + const TracerID tracer_id{/* runtime_id = */ RuntimeID::generate(), /* service = */ "testsvc", /* environment = */ "test"}; From 34ad10fd4fa698ef1d175cee09b1230e061cbc8d Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Thu, 7 Dec 2023 13:18:05 -0500 Subject: [PATCH 08/21] remove disambiguating "template" keyword --- src/datadog/remote_config.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/datadog/remote_config.cpp b/src/datadog/remote_config.cpp index 10d00b38..dc9b458c 100644 --- a/src/datadog/remote_config.cpp +++ b/src/datadog/remote_config.cpp @@ -148,9 +148,8 @@ void RemoteConfigurationManager::process_response(const nlohmann::json& json) { const auto& target_files = json.at("/target_files"_json_pointer); auto target_it = std::find_if( target_files.cbegin(), target_files.cend(), - [&config_path](const auto& j) { - return j.at("/path"_json_pointer).template get() == - config_path; + [&config_path](const nlohmann::json& j) { + return j.at("/path"_json_pointer).get() == config_path; }); if (target_it == target_files.cend()) { From fd76ec468cfaf9419b6790ca652f81a59a2b2fcd Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Thu, 7 Dec 2023 13:32:15 -0500 Subject: [PATCH 09/21] document k_apm_capabilities --- src/datadog/remote_config.cpp | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/datadog/remote_config.cpp b/src/datadog/remote_config.cpp index dc9b458c..8dee5110 100644 --- a/src/datadog/remote_config.cpp +++ b/src/datadog/remote_config.cpp @@ -1,7 +1,6 @@ #include "remote_config.h" -#include - +#include #include #include @@ -16,6 +15,16 @@ namespace datadog { namespace tracing { namespace { +// The ".client.capabilities" field of the remote config request payload +// describes which parts of the library's configuration are supported for remote +// configuration. +// +// It's a bitset, 64 bits wide, where each bit indicates whether the library +// supports a particular feature for remote configuration. +// +// The bitset is encoded in the request as a JSON array of 8 integers, where +// each integer is one byte from the 64 bits. The bytes are in big-endian order +// within the array. enum CapabilitiesFlag : uint64_t { APM_TRACING_SAMPLE_RATE = 1 << 12, }; @@ -31,10 +40,10 @@ constexpr std::array capabilities_byte_array( return res; } -constexpr StringView k_apm_product = "APM_TRACING"; - constexpr std::array k_apm_capabilities = - capabilities_byte_array((uint64_t)0 | APM_TRACING_SAMPLE_RATE); + capabilities_byte_array(APM_TRACING_SAMPLE_RATE); + +constexpr StringView k_apm_product = "APM_TRACING"; } // namespace From 7c421ba69795c55654d9786c77cc37cb1247c7b2 Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Thu, 7 Dec 2023 13:43:10 -0500 Subject: [PATCH 10/21] missed a spot when removing namespace httputil::header --- src/datadog/http_client.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/datadog/http_client.h b/src/datadog/http_client.h index 56eaa287..2931dde3 100644 --- a/src/datadog/http_client.h +++ b/src/datadog/http_client.h @@ -11,7 +11,6 @@ #include #include -#include "dict_writer.h" #include "error.h" #include "expected.h" #include "json_fwd.hpp" From 197063b4ac09f324d4850052815a490b1367c0c4 Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Thu, 7 Dec 2023 14:03:02 -0500 Subject: [PATCH 11/21] no need to optimize target file lookup --- src/datadog/remote_config.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/datadog/remote_config.cpp b/src/datadog/remote_config.cpp index 8dee5110..ca3b3065 100644 --- a/src/datadog/remote_config.cpp +++ b/src/datadog/remote_config.cpp @@ -153,7 +153,6 @@ void RemoteConfigurationManager::process_response(const nlohmann::json& json) { continue; } - // NOTE(@dmehala): is it worth indexing first? const auto& target_files = json.at("/target_files"_json_pointer); auto target_it = std::find_if( target_files.cbegin(), target_files.cend(), From 06aace8b0f101a0e49312a8e648094e52517b458 Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Thu, 7 Dec 2023 14:35:13 -0500 Subject: [PATCH 12/21] slightly safer product parsing --- src/datadog/remote_config.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/datadog/remote_config.cpp b/src/datadog/remote_config.cpp index ca3b3065..d81dd6e8 100644 --- a/src/datadog/remote_config.cpp +++ b/src/datadog/remote_config.cpp @@ -43,6 +43,7 @@ constexpr std::array capabilities_byte_array( constexpr std::array k_apm_capabilities = capabilities_byte_array(APM_TRACING_SAMPLE_RATE); +constexpr StringView k_apm_product_path_substring = "/APM_TRACING/"; constexpr StringView k_apm_product = "APM_TRACING"; } // namespace @@ -148,7 +149,7 @@ void RemoteConfigurationManager::process_response(const nlohmann::json& json) { const auto& config_metadata = targets.at("/signed/targets"_json_pointer).at(config_path); - if (!contains(config_path, k_apm_product) || + if (!contains(config_path, k_apm_product_path_substring) || !is_new_config(config_path, config_metadata)) { continue; } From b1dc9cfc8b8b2dafe6f4a2addfc48b1195d5849e Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Thu, 7 Dec 2023 15:05:13 -0500 Subject: [PATCH 13/21] mention missing config path in error message --- src/datadog/remote_config.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/datadog/remote_config.cpp b/src/datadog/remote_config.cpp index d81dd6e8..4717a1ef 100644 --- a/src/datadog/remote_config.cpp +++ b/src/datadog/remote_config.cpp @@ -163,7 +163,10 @@ void RemoteConfigurationManager::process_response(const nlohmann::json& json) { if (target_it == target_files.cend()) { state_.error_message = - "Missing configuration from Remote Configuration response"; + "Missing configuration from Remote Configuration response: No " + "target file having path \""; + append(*state_.error_message, config_path); + *state_.error_message += '\"'; return; } From c32d861435e34fc3af137eef142338adede09a03 Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Thu, 7 Dec 2023 15:31:25 -0500 Subject: [PATCH 14/21] use line comments --- src/datadog/datadog_agent.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/datadog/datadog_agent.cpp b/src/datadog/datadog_agent.cpp index 0d5adcad..21cf41f7 100644 --- a/src/datadog/datadog_agent.cpp +++ b/src/datadog/datadog_agent.cpp @@ -401,12 +401,11 @@ void DatadogAgent::get_and_apply_remote_configuration_updates() { std::string response_body) { if (response_status < 200 || response_status >= 300) { if (response_status == 404) { - /* - * 404 is not considered as an error as the agent use it to - * signal remote configuration is disabled. At any point, the - * feature could be enabled, so the tracer must continuously check - * for new remote configuration. - */ + // If the Datadog Agent doesn't understand remote configuration, + // or if remote configuration is disabled in the agent, then it + // will return 404. This is not an error. + // We'll keep polling, though, because the agent's configuration + // might change. return; } From 1c8a6652f7cb8f4600285abb76c00477a5c601cb Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Thu, 7 Dec 2023 15:31:33 -0500 Subject: [PATCH 15/21] remove trailing whitespace --- test/test_remote_config.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/test_remote_config.cpp b/test/test_remote_config.cpp index 4e015c46..a34b488c 100644 --- a/test/test_remote_config.cpp +++ b/test/test_remote_config.cpp @@ -83,39 +83,39 @@ REMOTE_CONFIG_TEST("response processing") { R"({ "targets": "eyJmb28iOiAiYmFyIn0=" })", // `targets` is missing the `targets` field. // decode("eyJzaWduZWQiOiB7InZlcnNpb24iOiAyLCAiY3VzdG9tIjogeyJvcGFxdWVfYmFja2VuZF9zdGF0ZSI6ICIxNSJ9fX0=") == "{"signed": {"version": 2, "custom": {"opaque_backend_state": "15"}}}" - R"({ + R"({ "targets": "eyJzaWduZWQiOiB7InZlcnNpb24iOiAyLCAiY3VzdG9tIjogeyJvcGFxdWVfYmFja2VuZF9zdGF0ZSI6ICIxNSJ9fX0=", "client_configs": ["datadog"] })", // `/targets/targets` have no `datadog` entry // {"signed": {"version": 2, "targets": {"foo": {}, "bar": {}},"custom": {"opaque_backend_state": "15"}}} - R"({ + R"({ "targets": "eyJzaWduZWQiOiB7InZlcnNpb24iOiAyLCAidGFyZ2V0cyI6IHsiZm9vIjoge30sICJiYXIiOiB7fX0sImN1c3RvbSI6IHsib3BhcXVlX2JhY2tlbmRfc3RhdGUiOiAiMTUifX19", "client_configs": ["datadog"] })", // `targets` OK but no `target_files` field. // {"signed": {"version": 2, "targets": {"foo/APM_TRACING/30": {}, "bar": {}},"custom": {"opaque_backend_state": "15"}}} - R"({ + R"({ "targets": "eyJzaWduZWQiOiB7InZlcnNpb24iOiAyLCAidGFyZ2V0cyI6IHsiZm9vL0FQTV9UUkFDSU5HLzMwIjoge30sICJiYXIiOiB7fX0sImN1c3RvbSI6IHsib3BhcXVlX2JhY2tlbmRfc3RhdGUiOiAiMTUifX19", "client_configs": ["foo/APM_TRACING/30"] })", // `targets` OK. `target_files` field is empty. // {"signed": {"version": 2, "targets": {"foo/APM_TRACING/30": {}, "bar": {}},"custom": {"opaque_backend_state": "15"}}} - R"({ + R"({ "targets": "eyJzaWduZWQiOiB7InZlcnNpb24iOiAyLCAidGFyZ2V0cyI6IHsiZm9vL0FQTV9UUkFDSU5HLzMwIjoge30sICJiYXIiOiB7fX0sImN1c3RvbSI6IHsib3BhcXVlX2JhY2tlbmRfc3RhdGUiOiAiMTUifX19", "client_configs": ["foo/APM_TRACING/30"], "target_files": [] })", // `targets` OK. `target_files` field is not an array. // {"signed": {"version": 2, "targets": {"foo/APM_TRACING/30": {}, "bar": {}},"custom": {"opaque_backend_state": "15"}}} - R"({ + R"({ "targets": "eyJzaWduZWQiOiB7InZlcnNpb24iOiAyLCAidGFyZ2V0cyI6IHsiZm9vL0FQTV9UUkFDSU5HLzMwIjoge30sICJiYXIiOiB7fX0sImN1c3RvbSI6IHsib3BhcXVlX2JhY2tlbmRfc3RhdGUiOiAiMTUifX19", "client_configs": ["foo/APM_TRACING/30"], "target_files": 15 })", // `targets` OK. `target_files` field content is not base64 encoded. // {"signed": {"version": 2, "targets": {"foo/APM_TRACING/30": {}, "bar": {}},"custom": {"opaque_backend_state": "15"}}} - R"({ + R"({ "targets": "eyJzaWduZWQiOiB7InZlcnNpb24iOiAyLCAidGFyZ2V0cyI6IHsiZm9vL0FQTV9UUkFDSU5HLzMwIjoge30sICJiYXIiOiB7fX0sImN1c3RvbSI6IHsib3BhcXVlX2JhY2tlbmRfc3RhdGUiOiAiMTUifX19", "client_configs": ["foo/APM_TRACING/30"], "target_files": [{"path": "foo/APM_TRACING/30", "raw": "Hello, Uranus!"}] @@ -123,7 +123,7 @@ REMOTE_CONFIG_TEST("response processing") { // `targets` OK. `target_files` field content is not a JSON base64 encoded. // decode("bm90IGpzb24=") == "not json" // {"signed": {"version": 2, "targets": {"foo/APM_TRACING/30": {}, "bar": {}},"custom": {"opaque_backend_state": "15"}}} - R"({ + R"({ "targets": "eyJzaWduZWQiOiB7InZlcnNpb24iOiAyLCAidGFyZ2V0cyI6IHsiZm9vL0FQTV9UUkFDSU5HLzMwIjoge30sICJiYXIiOiB7fX0sImN1c3RvbSI6IHsib3BhcXVlX2JhY2tlbmRfc3RhdGUiOiAiMTUifX19", "client_configs": ["foo/APM_TRACING/30"], "target_files": [{"path": "foo/APM_TRACING/30", "raw": "bm90IGpzb24="}] @@ -131,7 +131,7 @@ REMOTE_CONFIG_TEST("response processing") { // `targets` OK. `target_files` field JSON base64 content do not follow the expected schema. // decode("eyJmb28iOiAiYmFyIn0=") == "{"foo": "bar"}" // {"signed": {"version": 2, "targets": {"foo/APM_TRACING/30": {}, "bar": {}},"custom": {"opaque_backend_state": "15"}}} - R"({ + R"({ "targets": "eyJzaWduZWQiOiB7InZlcnNpb24iOiAyLCAidGFyZ2V0cyI6IHsiZm9vL0FQTV9UUkFDSU5HLzMwIjoge30sICJiYXIiOiB7fX0sImN1c3RvbSI6IHsib3BhcXVlX2JhY2tlbmRfc3RhdGUiOiAiMTUifX19", "client_configs": ["foo/APM_TRACING/30"], "target_files": [{"path": "foo/APM_TRACING/30", "raw": "eyJmb28iOiAiYmFyIn0="}] From 5dfe43eb75ea4fc3edf31bf731858e3dbab1926c Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Thu, 7 Dec 2023 15:34:01 -0500 Subject: [PATCH 16/21] question about remote config response --- src/datadog/datadog_agent.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/datadog/datadog_agent.cpp b/src/datadog/datadog_agent.cpp index 21cf41f7..83dd4927 100644 --- a/src/datadog/datadog_agent.cpp +++ b/src/datadog/datadog_agent.cpp @@ -430,6 +430,7 @@ void DatadogAgent::get_and_apply_remote_configuration_updates() { return; } + // TODO(@dgoffredo): When is the parsed JSON object empty? if (!response_json.empty()) { // TODO (during Active Configuration): `process_response` should // return a list of configuration update and should be consumed by From 322061d114b0ff68e6cb5c50c77e9ed63d86293a Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Wed, 3 Jan 2024 15:19:06 -0500 Subject: [PATCH 17/21] TracerID --> TracerSignature --- BUILD.bazel | 3 +- CMakeLists.txt | 3 +- src/datadog/datadog_agent.cpp | 4 +- src/datadog/datadog_agent.h | 4 +- src/datadog/remote_config.cpp | 18 ++++----- src/datadog/remote_config.h | 6 +-- src/datadog/tracer.cpp | 8 ++-- src/datadog/tracer.h | 4 +- src/datadog/tracer_id.h | 16 -------- src/datadog/tracer_signature.cpp | 33 +++++++++++++++++ src/datadog/tracer_signature.h | 63 ++++++++++++++++++++++++++++++++ src/datadog/tracer_telemetry.cpp | 16 ++++---- src/datadog/tracer_telemetry.h | 6 +-- test/test_remote_config.cpp | 8 ++-- test/test_tracer_telemetry.cpp | 4 +- 15 files changed, 139 insertions(+), 57 deletions(-) delete mode 100644 src/datadog/tracer_id.h create mode 100644 src/datadog/tracer_signature.cpp create mode 100644 src/datadog/tracer_signature.h diff --git a/BUILD.bazel b/BUILD.bazel index b74cbabe..265436e9 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -38,6 +38,7 @@ cc_library( "src/datadog/tags.cpp", "src/datadog/threaded_event_scheduler.cpp", "src/datadog/tracer_config.cpp", + "src/datadog/tracer_signature.cpp", "src/datadog/tracer_telemetry.cpp", "src/datadog/tracer.cpp", "src/datadog/trace_id.cpp", @@ -102,10 +103,10 @@ cc_library( "src/datadog/tags.h", "src/datadog/threaded_event_scheduler.h", "src/datadog/tracer_config.h", + "src/datadog/tracer_signature.h", "src/datadog/tracer_telemetry.h", "src/datadog/tracer.h", "src/datadog/trace_id.h", - "src/datadog/tracer_id.h", "src/datadog/trace_sampler_config.h", "src/datadog/trace_sampler.h", "src/datadog/trace_segment.h", diff --git a/CMakeLists.txt b/CMakeLists.txt index ad5aeabf..deaf136d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -138,6 +138,7 @@ target_sources(dd_trace_cpp-objects PRIVATE src/datadog/tags.cpp src/datadog/threaded_event_scheduler.cpp src/datadog/tracer_config.cpp + src/datadog/tracer_signature.cpp src/datadog/tracer_telemetry.cpp src/datadog/tracer.cpp src/datadog/trace_id.cpp @@ -208,10 +209,10 @@ target_sources(dd_trace_cpp-objects PUBLIC src/datadog/tags.h src/datadog/threaded_event_scheduler.h src/datadog/tracer_config.h + src/datadog/tracer_signature.h src/datadog/tracer_telemetry.h src/datadog/tracer.h src/datadog/trace_id.h - src/datadog/tracer_id.h src/datadog/trace_sampler_config.h src/datadog/trace_sampler.h src/datadog/trace_segment.h diff --git a/src/datadog/datadog_agent.cpp b/src/datadog/datadog_agent.cpp index 83dd4927..fa9e3592 100644 --- a/src/datadog/datadog_agent.cpp +++ b/src/datadog/datadog_agent.cpp @@ -149,7 +149,7 @@ std::variant parse_agent_traces_response( DatadogAgent::DatadogAgent( const FinalizedDatadogAgentConfig& config, const std::shared_ptr& tracer_telemetry, - const std::shared_ptr& logger, const TracerID& tracer_id, + const std::shared_ptr& logger, const TracerSignature& tracer_signature, ConfigManager& config_manager) : tracer_telemetry_(tracer_telemetry), clock_(config.clock), @@ -164,7 +164,7 @@ DatadogAgent::DatadogAgent( flush_interval_(config.flush_interval), request_timeout_(config.request_timeout), shutdown_timeout_(config.shutdown_timeout), - remote_config_(tracer_id, config_manager) { + remote_config_(tracer_signature, config_manager) { assert(logger_); assert(tracer_telemetry_); if (tracer_telemetry_->enabled()) { diff --git a/src/datadog/datadog_agent.h b/src/datadog/datadog_agent.h index 6aa11440..9cefe059 100644 --- a/src/datadog/datadog_agent.h +++ b/src/datadog/datadog_agent.h @@ -26,7 +26,7 @@ class FinalizedDatadogAgentConfig; class Logger; struct SpanData; class TraceSampler; -struct TracerID; +class TracerSignature; class DatadogAgent : public Collector { public: @@ -65,7 +65,7 @@ class DatadogAgent : public Collector { public: DatadogAgent(const FinalizedDatadogAgentConfig&, const std::shared_ptr&, - const std::shared_ptr&, const TracerID& id, + const std::shared_ptr&, const TracerSignature& id, ConfigManager& config_manager); ~DatadogAgent(); diff --git a/src/datadog/remote_config.cpp b/src/datadog/remote_config.cpp index a148d4cf..cdcc3b8b 100644 --- a/src/datadog/remote_config.cpp +++ b/src/datadog/remote_config.cpp @@ -64,8 +64,8 @@ ConfigUpdate parse_dynamic_config(const nlohmann::json& j) { } // namespace RemoteConfigurationManager::RemoteConfigurationManager( - const TracerID& tracer_id, ConfigManager& config_manager) - : tracer_id_(tracer_id), + const TracerSignature& tracer_signature, ConfigManager& config_manager) + : tracer_signature_(tracer_signature), config_manager_(config_manager), client_id_(uuid()) {} @@ -87,11 +87,11 @@ nlohmann::json RemoteConfigurationManager::make_request_payload() { {"is_tracer", true}, {"capabilities", k_apm_capabilities}, {"client_tracer", { - {"runtime_id", tracer_id_.runtime_id.string()}, - {"language", "cpp"}, - {"tracer_version", tracer_version}, - {"service", tracer_id_.service}, - {"env", tracer_id_.environment} + {"runtime_id", tracer_signature_.runtime_id().string()}, + {"language", tracer_signature_.library_language()}, + {"tracer_version", tracer_signature_.library_version()}, + {"service", tracer_signature_.default_service()}, + {"env", tracer_signature_.default_environment()} }}, {"state", { {"root_version", 1}, @@ -178,9 +178,9 @@ void RemoteConfigurationManager::process_response(const nlohmann::json& json) { const auto& targeted_service = config_json.at("service_target"); if (targeted_service.at("service").get() != - tracer_id_.service || + tracer_signature_.default_service() || targeted_service.at("env").get() != - tracer_id_.environment) { + tracer_signature_.default_environment()) { continue; } diff --git a/src/datadog/remote_config.h b/src/datadog/remote_config.h index 0359f014..6ec93608 100644 --- a/src/datadog/remote_config.h +++ b/src/datadog/remote_config.h @@ -10,7 +10,7 @@ #include "runtime_id.h" #include "string_view.h" #include "trace_sampler_config.h" -#include "tracer_id.h" +#include "tracer_signature.h" namespace datadog { namespace tracing { @@ -29,7 +29,7 @@ class RemoteConfigurationManager { ConfigUpdate content; }; - TracerID tracer_id_; + TracerSignature tracer_signature_; ConfigManager& config_manager_; std::string client_id_; @@ -37,7 +37,7 @@ class RemoteConfigurationManager { std::unordered_map applied_config_; public: - RemoteConfigurationManager(const TracerID& tracer_id, + RemoteConfigurationManager(const TracerSignature& tracer_signature, ConfigManager& config_manager); nlohmann::json make_request_payload(); diff --git a/src/datadog/tracer.cpp b/src/datadog/tracer.cpp index efc2b6e7..887bfa8c 100644 --- a/src/datadog/tracer.cpp +++ b/src/datadog/tracer.cpp @@ -23,7 +23,7 @@ #include "tags.h" #include "trace_sampler.h" #include "trace_segment.h" -#include "tracer_id.h" +#include "tracer_signature.h" #include "version.h" #include "w3c_propagation.h" @@ -40,9 +40,9 @@ Tracer::Tracer(const FinalizedTracerConfig& config, defaults_(std::make_shared(config.defaults)), runtime_id_(config.runtime_id ? *config.runtime_id : RuntimeID::generate()), - id_{runtime_id_, config.defaults.service, config.defaults.environment}, + signature_{runtime_id_, config.defaults.service, config.defaults.environment}, tracer_telemetry_(std::make_shared( - config.report_telemetry, config.clock, logger_, id_)), + config.report_telemetry, config.clock, logger_, signature_)), span_sampler_( std::make_shared(config.span_sampler, config.clock)), generator_(generator), @@ -60,7 +60,7 @@ Tracer::Tracer(const FinalizedTracerConfig& config, std::get(config.collector); auto agent = std::make_shared( - agent_config, tracer_telemetry_, config.logger, id_, config_manager_); + agent_config, tracer_telemetry_, config.logger, signature_, config_manager_); collector_ = agent; if (tracer_telemetry_->enabled()) { diff --git a/src/datadog/tracer.h b/src/datadog/tracer.h index 6c72faea..8f1f5334 100644 --- a/src/datadog/tracer.h +++ b/src/datadog/tracer.h @@ -19,6 +19,7 @@ #include "optional.h" #include "span.h" #include "tracer_config.h" +#include "tracer_signature.h" #include "tracer_telemetry.h" namespace datadog { @@ -28,14 +29,13 @@ class DictReader; struct SpanConfig; class TraceSampler; class SpanSampler; -struct TracerID; class Tracer { std::shared_ptr logger_; std::shared_ptr collector_; std::shared_ptr defaults_; RuntimeID runtime_id_; - TracerID id_; + TracerSignature signature_; std::shared_ptr tracer_telemetry_; std::shared_ptr span_sampler_; std::shared_ptr generator_; diff --git a/src/datadog/tracer_id.h b/src/datadog/tracer_id.h deleted file mode 100644 index dd4caf00..00000000 --- a/src/datadog/tracer_id.h +++ /dev/null @@ -1,16 +0,0 @@ -#pragma once - -#include "runtime_id.h" - -namespace datadog { -namespace tracing { - -// Identify a tracer -struct TracerID { - RuntimeID runtime_id; - std::string service; - std::string environment; -}; - -} // namespace tracing -} // namespace datadog diff --git a/src/datadog/tracer_signature.cpp b/src/datadog/tracer_signature.cpp new file mode 100644 index 00000000..83a805c3 --- /dev/null +++ b/src/datadog/tracer_signature.cpp @@ -0,0 +1,33 @@ +#include "tracer_signature.h" + +#include "version.h" + +namespace datadog { +namespace tracing { + +TracerSignature::TracerSignature(const RuntimeID& runtime_id, + const std::string& default_service, + const std::string& default_environment) + : runtime_id_(runtime_id), + default_service_(default_service), + default_environment_(default_environment) {} + +const RuntimeID& TracerSignature::runtime_id() const { return runtime_id_; } + +StringView TracerSignature::default_service() const { return default_service_; } + +StringView TracerSignature::default_environment() const { + return default_environment_; +} + +StringView TracerSignature::library_version() const { return tracer_version; } + +StringView TracerSignature::library_language() const { return "cpp"; } + +StringView TracerSignature::library_language_version() const { + static const std::string value = std::to_string(__cplusplus); + return value; +} + +} // namespace tracing +} // namespace datadog diff --git a/src/datadog/tracer_signature.h b/src/datadog/tracer_signature.h new file mode 100644 index 00000000..be86d94d --- /dev/null +++ b/src/datadog/tracer_signature.h @@ -0,0 +1,63 @@ +#pragma once + +// This component provides a class, `TracerSignature`, that contains the parts +// of a tracer's configuration that are used to refer to the tracer in Datadog's +// telemetry and remote configuration APIs. +// +// `TracerSignature` is used in three contexts: +// +// 1. When telemetry is sent to the Datadog Agent, the tracer signature is +// included in the request payload. See +// `TracerTelemetry::generate_telemetry_body` in `tracer_telemetry.cpp`. +// 2. When the Datadog Agent is polled for configuration updates, part of the +// tracer signature (all but the language version) is included in the request +// payload. See `RemoteConfigurationManager::make_request_payload` in +// `remote_config.h`. +// 3. When the Datadog Agent responds with configuration updates, the service +// and environment of the tracer signature are used to determine whether the +// updates are relevant to the `Tracer` that created the collector that is +// polling the Datadog Agent. See +// `RemoteConfigurationManager::process_response` in `remote_config.h`. + +#include + +#include "runtime_id.h" +#include "string_view.h" + +namespace datadog { +namespace tracing { + +class TracerSignature { + RuntimeID runtime_id_; + std::string default_service_; + std::string default_environment_; + + public: + // Create a tracer signature having the specified `runtime_id`, + // `default_service`, and `default_environment`. `default_service` and + // `default_environment` refer to the corresponding fields from + // `SpanDefaults`. + TracerSignature(const RuntimeID& runtime_id, + const std::string& default_service, + const std::string& default_environment); + + // Return the runtime ID with which the tracer was configured. + const RuntimeID& runtime_id() const; + // Return the `SpanDefaults::service` with which the tracer was configured. + StringView default_service() const; + // Return the `SpanDefaults::environment` with which the tracer was + // configured. + StringView default_environment() const; + // Return the version of this tracing library (`tracer_version` from + // `version.h`). + StringView library_version() const; + // Return the name of the programming language in which this library is + // written: "cpp". + StringView library_language() const; + // Return the version of C++ standard used to compile this library. It should + // be "201703". + StringView library_language_version() const; +}; + +} // namespace tracing +} // namespace datadog diff --git a/src/datadog/tracer_telemetry.cpp b/src/datadog/tracer_telemetry.cpp index 79bbb01c..14e12c78 100644 --- a/src/datadog/tracer_telemetry.cpp +++ b/src/datadog/tracer_telemetry.cpp @@ -10,11 +10,11 @@ namespace tracing { TracerTelemetry::TracerTelemetry(bool enabled, const Clock& clock, const std::shared_ptr& logger, - const TracerID& tracer_id) + const TracerSignature& tracer_signature) : enabled_(enabled), clock_(clock), logger_(logger), - tracer_id_(tracer_id), + tracer_signature_(tracer_signature), hostname_(get_hostname().value_or("hostname-unavailable")) { if (enabled_) { // Register all the metrics that we're tracking by adding them to the @@ -62,14 +62,14 @@ nlohmann::json TracerTelemetry::generate_telemetry_body( {"seq_id", seq_id_}, {"request_type", request_type}, {"tracer_time", tracer_time}, - {"runtime_id", tracer_id_.runtime_id.string()}, + {"runtime_id", tracer_signature_.runtime_id().string()}, {"debug", debug_}, {"application", nlohmann::json::object({ - {"service_name", tracer_id_.service}, - {"env", tracer_id_.environment}, - {"tracer_version", tracer_version}, - {"language_name", "cpp"}, - {"language_version", std::to_string(__cplusplus)}, + {"service_name", tracer_signature_.default_service()}, + {"env", tracer_signature_.default_environment()}, + {"tracer_version", tracer_signature_.library_version()}, + {"language_name", tracer_signature_.library_language()}, + {"language_version", tracer_signature_.library_language_version()}, })}, // TODO: host information (os, os_version, kernel, etc) {"host", nlohmann::json::object({ diff --git a/src/datadog/tracer_telemetry.h b/src/datadog/tracer_telemetry.h index 3cc64e57..e9488b94 100644 --- a/src/datadog/tracer_telemetry.h +++ b/src/datadog/tracer_telemetry.h @@ -31,7 +31,7 @@ #include "json.hpp" #include "metrics.h" #include "runtime_id.h" -#include "tracer_id.h" +#include "tracer_signature.h" namespace datadog { namespace tracing { @@ -44,7 +44,7 @@ class TracerTelemetry { bool debug_ = false; Clock clock_; std::shared_ptr logger_; - TracerID tracer_id_; + TracerSignature tracer_signature_; std::string hostname_; uint64_t seq_id_ = 0; // This structure contains all the metrics that are exposed by tracer @@ -100,7 +100,7 @@ class TracerTelemetry { public: TracerTelemetry(bool enabled, const Clock& clock, const std::shared_ptr& logger, - const TracerID& tracer_id); + const TracerSignature& tracer_signature); bool enabled() { return enabled_; }; // Provides access to the telemetry metrics for updating the values. // This value should not be stored. diff --git a/test/test_remote_config.cpp b/test/test_remote_config.cpp index a34b488c..7c916d7a 100644 --- a/test/test_remote_config.cpp +++ b/test/test_remote_config.cpp @@ -11,7 +11,7 @@ using namespace datadog::tracing; #define REMOTE_CONFIG_TEST(x) TEST_CASE(x, "[remote_config]") REMOTE_CONFIG_TEST("first payload") { - const TracerID tracer_id{/* runtime_id = */ RuntimeID::generate(), + const TracerSignature tracer_signature{/* runtime_id = */ RuntimeID::generate(), /* service = */ "testsvc", /* environment = */ "test"}; @@ -29,7 +29,7 @@ REMOTE_CONFIG_TEST("first payload") { config.defaults.environment = "test"; ConfigManager config_manager(*finalize_config(config)); - RemoteConfigurationManager rc(tracer_id, config_manager); + RemoteConfigurationManager rc(tracer_signature, config_manager); const auto payload = rc.make_request_payload(); @@ -43,7 +43,7 @@ REMOTE_CONFIG_TEST("first payload") { } REMOTE_CONFIG_TEST("response processing") { - const TracerID tracer_id{/* runtime_id = */ RuntimeID::generate(), + const TracerSignature tracer_signature{/* runtime_id = */ RuntimeID::generate(), /* service = */ "testsvc", /* environment = */ "test"}; @@ -62,7 +62,7 @@ REMOTE_CONFIG_TEST("response processing") { config.trace_sampler.sample_rate = 1.0; ConfigManager config_manager(*finalize_config(config)); - RemoteConfigurationManager rc(tracer_id, config_manager); + RemoteConfigurationManager rc(tracer_signature, config_manager); SECTION("ill formatted input", "inputs not following the Remote Configuration JSON schema should " diff --git a/test/test_tracer_telemetry.cpp b/test/test_tracer_telemetry.cpp index 9e691ad3..cc0a9655 100644 --- a/test/test_tracer_telemetry.cpp +++ b/test/test_tracer_telemetry.cpp @@ -22,11 +22,11 @@ TEST_CASE("Tracer telemetry") { }; auto logger = std::make_shared(); - const TracerID tracer_id{/* runtime_id = */ RuntimeID::generate(), + const TracerSignature tracer_signature{/* runtime_id = */ RuntimeID::generate(), /* service = */ "testsvc", /* environment = */ "test"}; - TracerTelemetry tracer_telemetry = {true, clock, logger, tracer_id}; + TracerTelemetry tracer_telemetry = {true, clock, logger, tracer_signature}; SECTION("generates app-started message") { auto app_started_message = tracer_telemetry.app_started(); From 4e7f4ac8f952882837aca964c209d007c4786490 Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Wed, 3 Jan 2024 15:19:33 -0500 Subject: [PATCH 18/21] bin/format --- src/datadog/datadog_agent.cpp | 4 ++-- src/datadog/tracer.cpp | 8 +++++--- src/datadog/tracer_telemetry.cpp | 15 ++++++++------- test/test_remote_config.cpp | 14 ++++++++------ test/test_tracer_telemetry.cpp | 7 ++++--- 5 files changed, 27 insertions(+), 21 deletions(-) diff --git a/src/datadog/datadog_agent.cpp b/src/datadog/datadog_agent.cpp index fa9e3592..287a19b2 100644 --- a/src/datadog/datadog_agent.cpp +++ b/src/datadog/datadog_agent.cpp @@ -149,8 +149,8 @@ std::variant parse_agent_traces_response( DatadogAgent::DatadogAgent( const FinalizedDatadogAgentConfig& config, const std::shared_ptr& tracer_telemetry, - const std::shared_ptr& logger, const TracerSignature& tracer_signature, - ConfigManager& config_manager) + const std::shared_ptr& logger, + const TracerSignature& tracer_signature, ConfigManager& config_manager) : tracer_telemetry_(tracer_telemetry), clock_(config.clock), logger_(logger), diff --git a/src/datadog/tracer.cpp b/src/datadog/tracer.cpp index 887bfa8c..4d0da254 100644 --- a/src/datadog/tracer.cpp +++ b/src/datadog/tracer.cpp @@ -40,7 +40,8 @@ Tracer::Tracer(const FinalizedTracerConfig& config, defaults_(std::make_shared(config.defaults)), runtime_id_(config.runtime_id ? *config.runtime_id : RuntimeID::generate()), - signature_{runtime_id_, config.defaults.service, config.defaults.environment}, + signature_{runtime_id_, config.defaults.service, + config.defaults.environment}, tracer_telemetry_(std::make_shared( config.report_telemetry, config.clock, logger_, signature_)), span_sampler_( @@ -59,8 +60,9 @@ Tracer::Tracer(const FinalizedTracerConfig& config, auto& agent_config = std::get(config.collector); - auto agent = std::make_shared( - agent_config, tracer_telemetry_, config.logger, signature_, config_manager_); + auto agent = std::make_shared(agent_config, tracer_telemetry_, + config.logger, signature_, + config_manager_); collector_ = agent; if (tracer_telemetry_->enabled()) { diff --git a/src/datadog/tracer_telemetry.cpp b/src/datadog/tracer_telemetry.cpp index 14e12c78..2ef26a74 100644 --- a/src/datadog/tracer_telemetry.cpp +++ b/src/datadog/tracer_telemetry.cpp @@ -64,13 +64,14 @@ nlohmann::json TracerTelemetry::generate_telemetry_body( {"tracer_time", tracer_time}, {"runtime_id", tracer_signature_.runtime_id().string()}, {"debug", debug_}, - {"application", nlohmann::json::object({ - {"service_name", tracer_signature_.default_service()}, - {"env", tracer_signature_.default_environment()}, - {"tracer_version", tracer_signature_.library_version()}, - {"language_name", tracer_signature_.library_language()}, - {"language_version", tracer_signature_.library_language_version()}, - })}, + {"application", + nlohmann::json::object({ + {"service_name", tracer_signature_.default_service()}, + {"env", tracer_signature_.default_environment()}, + {"tracer_version", tracer_signature_.library_version()}, + {"language_name", tracer_signature_.library_language()}, + {"language_version", tracer_signature_.library_language_version()}, + })}, // TODO: host information (os, os_version, kernel, etc) {"host", nlohmann::json::object({ {"hostname", hostname_}, diff --git a/test/test_remote_config.cpp b/test/test_remote_config.cpp index 7c916d7a..e0406da2 100644 --- a/test/test_remote_config.cpp +++ b/test/test_remote_config.cpp @@ -11,9 +11,10 @@ using namespace datadog::tracing; #define REMOTE_CONFIG_TEST(x) TEST_CASE(x, "[remote_config]") REMOTE_CONFIG_TEST("first payload") { - const TracerSignature tracer_signature{/* runtime_id = */ RuntimeID::generate(), - /* service = */ "testsvc", - /* environment = */ "test"}; + const TracerSignature tracer_signature{ + /* runtime_id = */ RuntimeID::generate(), + /* service = */ "testsvc", + /* environment = */ "test"}; TracerConfig tracer_cfg; @@ -43,9 +44,10 @@ REMOTE_CONFIG_TEST("first payload") { } REMOTE_CONFIG_TEST("response processing") { - const TracerSignature tracer_signature{/* runtime_id = */ RuntimeID::generate(), - /* service = */ "testsvc", - /* environment = */ "test"}; + const TracerSignature tracer_signature{ + /* runtime_id = */ RuntimeID::generate(), + /* service = */ "testsvc", + /* environment = */ "test"}; TracerConfig tracer_cfg; diff --git a/test/test_tracer_telemetry.cpp b/test/test_tracer_telemetry.cpp index cc0a9655..05cd00ba 100644 --- a/test/test_tracer_telemetry.cpp +++ b/test/test_tracer_telemetry.cpp @@ -22,9 +22,10 @@ TEST_CASE("Tracer telemetry") { }; auto logger = std::make_shared(); - const TracerSignature tracer_signature{/* runtime_id = */ RuntimeID::generate(), - /* service = */ "testsvc", - /* environment = */ "test"}; + const TracerSignature tracer_signature{ + /* runtime_id = */ RuntimeID::generate(), + /* service = */ "testsvc", + /* environment = */ "test"}; TracerTelemetry tracer_telemetry = {true, clock, logger, tracer_signature}; From 76182918a73bcaaa8737b0e8a33337b9cc3aa3f4 Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Wed, 3 Jan 2024 15:23:35 -0500 Subject: [PATCH 19/21] add documentation TODOs --- src/datadog/config_update.h | 3 ++- src/datadog/remote_config.h | 9 +++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/datadog/config_update.h b/src/datadog/config_update.h index 9dc56648..00e8ba33 100644 --- a/src/datadog/config_update.h +++ b/src/datadog/config_update.h @@ -1,10 +1,11 @@ +// TODO: Document + #include "optional" #include "trace_sampler.h" namespace datadog { namespace tracing { -// TODO: Document struct ConfigUpdate { Optional trace_sampler; }; diff --git a/src/datadog/remote_config.h b/src/datadog/remote_config.h index 6ec93608..5441e76d 100644 --- a/src/datadog/remote_config.h +++ b/src/datadog/remote_config.h @@ -16,12 +16,14 @@ namespace datadog { namespace tracing { class RemoteConfigurationManager { + // TODO: document struct State { uint64_t targets_version = 1; std::string opaque_backend_state; Optional error_message; }; + // TODO: document struct Configuration { std::string id; std::string hash; @@ -31,23 +33,30 @@ class RemoteConfigurationManager { TracerSignature tracer_signature_; ConfigManager& config_manager_; + // TODO: document std::string client_id_; State state_; + // TODO: document std::unordered_map applied_config_; public: RemoteConfigurationManager(const TracerSignature& tracer_signature, ConfigManager& config_manager); + // TODO: document nlohmann::json make_request_payload(); + // TODO: document void process_response(const nlohmann::json& json); private: + // TODO: document bool is_new_config(StringView config_path, const nlohmann::json& config_meta); + // TODO: document void apply_config(Configuration config); + // TODO: document void revert_config(Configuration config); }; From 270d376b06a8676e0f4d465d0a25ecacc6fc837a Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Wed, 3 Jan 2024 15:25:58 -0500 Subject: [PATCH 20/21] fix up pragmas and includes --- src/datadog/config_manager.cpp | 1 + src/datadog/config_update.h | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/datadog/config_manager.cpp b/src/datadog/config_manager.cpp index 90510789..7ef99b56 100644 --- a/src/datadog/config_manager.cpp +++ b/src/datadog/config_manager.cpp @@ -1,4 +1,5 @@ #include "config_manager.h" +#include "trace_sampler.h" namespace datadog { namespace tracing { diff --git a/src/datadog/config_update.h b/src/datadog/config_update.h index 00e8ba33..48704d8f 100644 --- a/src/datadog/config_update.h +++ b/src/datadog/config_update.h @@ -1,7 +1,9 @@ +#pragma once + // TODO: Document #include "optional" -#include "trace_sampler.h" +#include "trace_sampler_config.h" namespace datadog { namespace tracing { From 5b4d1acc0a87e8f1818a08ab4de4abbe5ce2ecab Mon Sep 17 00:00:00 2001 From: David Goffredo Date: Wed, 3 Jan 2024 15:26:10 -0500 Subject: [PATCH 21/21] bin/format --- src/datadog/config_manager.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/datadog/config_manager.cpp b/src/datadog/config_manager.cpp index 7ef99b56..a13ac346 100644 --- a/src/datadog/config_manager.cpp +++ b/src/datadog/config_manager.cpp @@ -1,4 +1,5 @@ #include "config_manager.h" + #include "trace_sampler.h" namespace datadog {