From 7f540b3bbd3202cd46fda9ab14b88e38a58465f9 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 19 Feb 2026 16:47:14 +0100 Subject: [PATCH 01/11] chore: add supported configurations The configuration macro is changed to a registry that includes types and default values next to the configuration names. This is used for compile time generation instead of using runtime checks for the configurations (environment variables). For safety, failing to parse configurations is now going to log a warning instead. This aligns with other platforms / tracers to not crash a customer in production where their staging configuration might be valid and their production one not. The macro must now be used for any new configuration added and a CI job verifies that all entries do exist. If a new env is added otherwise it should fail. The CI will also verify that the generated file is up to date and matches the remote registry. It would fail, if either is not the case. The behavior change is documented in the README.md. --- .github/workflows/dev.yml | 13 + .gitignore | 3 +- bin/README.md | 16 + bin/check-environment-variables | 6 + bin/generate-supported-configurations | 6 + bin/supported-configurations | 21 + bin/supported-configurations.cpp | 648 ++++++++++++++++++++++++ include/datadog/environment.h | 180 ++++--- include/datadog/environment_registry.h | 70 +++ metadata/supported-configurations.json | 286 +++++++++++ src/datadog/datadog_agent_config.cpp | 48 +- src/datadog/environment.cpp | 49 +- src/datadog/span_sampler_config.cpp | 22 +- src/datadog/telemetry/configuration.cpp | 52 +- src/datadog/trace_sampler_config.cpp | 42 +- src/datadog/tracer_config.cpp | 158 +++--- test/test_tracer_config.cpp | 131 ++--- 17 files changed, 1461 insertions(+), 290 deletions(-) create mode 100755 bin/check-environment-variables create mode 100755 bin/generate-supported-configurations create mode 100755 bin/supported-configurations create mode 100644 bin/supported-configurations.cpp create mode 100644 include/datadog/environment_registry.h create mode 100644 metadata/supported-configurations.json diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 9c65e53f..a023eb21 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -12,6 +12,19 @@ jobs: run: bin/check-format - name: Shellcheck run: find bin/ -executable -type f -print0 | xargs -0 shellcheck + - name: Verify environment variable allowlist + run: bin/check-environment-variables + - name: Verify supported configurations metadata + run: | + tmp_dir="$(mktemp -d)" + trap 'rm -rf "$tmp_dir"' EXIT + cp metadata/supported-configurations.json "$tmp_dir/supported-configurations.json" + bin/generate-supported-configurations + if ! diff -q "$tmp_dir/supported-configurations.json" metadata/supported-configurations.json >/dev/null 2>&1; then + echo "ERROR: metadata/supported-configurations.json got out of sync with implemented configurations. Please run bin/generate-supported-configurations locally." + diff -u "$tmp_dir/supported-configurations.json" metadata/supported-configurations.json || true + exit 1 + fi build-linux-cmake: strategy: diff --git a/.gitignore b/.gitignore index 60c94744..409fd98a 100644 --- a/.gitignore +++ b/.gitignore @@ -11,4 +11,5 @@ MODULE.bazel.lock .vscode .cache/ .cursor/ -.DS_Store \ No newline at end of file +.DS_Store +bin/.supported-configurations \ No newline at end of file diff --git a/bin/README.md b/bin/README.md index d4bc645a..a66f23d8 100644 --- a/bin/README.md +++ b/bin/README.md @@ -1,5 +1,6 @@ Scripts ======= + This directory contains scripts that are useful during development. - [bazel-build](bazel-build) builds the library using [Bazel][1] via [bazelisk][2]. @@ -8,6 +9,12 @@ This directory contains scripts that are useful during development. - [check](check) performs some of the checks that [continuous integration](../.circleci) performs. It's convenient to run this script before pushing changes. +- [check-environment-variables](check-environment-variables) validates that + all `DD_`/`OTEL_` environment variables used by the codebase are allowlisted + in + [include/datadog/environment_registry.h](../include/datadog/environment_registry.h) + (the source of truth used by + [include/datadog/environment.h](../include/datadog/environment.h)). - [check-format](check-format) verifies that the source code is formatted as [format](format) prefers. - [check-version](check-version) accepts a version string as a command line @@ -16,6 +23,15 @@ This directory contains scripts that are useful during development. - [cmake-build](cmake-build) builds the library using [CMake][3]. - [format](format) formats all of the C++ source code using [clang-format-14][4]. +- [generate-supported-configurations](generate-supported-configurations) + regenerates + [metadata/supported-configurations.json](../metadata/supported-configurations.json) + from + [include/datadog/environment_registry.h](../include/datadog/environment_registry.h). + This script (and [check-environment-variables](check-environment-variables)) + compiles and executes + [supported-configurations.cpp](supported-configurations.cpp) via + [supported-configurations](supported-configurations). - [hasher-example](hasher-example) builds the library, including the [command line example](../examples/hasher) program, and then runs the example. - [http-server-example](http-server-example) runs the docker compose based [HTTP diff --git a/bin/check-environment-variables b/bin/check-environment-variables new file mode 100755 index 00000000..69a4d6c2 --- /dev/null +++ b/bin/check-environment-variables @@ -0,0 +1,6 @@ +#!/bin/sh + +set -eu + +SCRIPT_DIR=$(cd "$(dirname "$0")" && pwd) +exec "$SCRIPT_DIR/supported-configurations" check diff --git a/bin/generate-supported-configurations b/bin/generate-supported-configurations new file mode 100755 index 00000000..5d0d061c --- /dev/null +++ b/bin/generate-supported-configurations @@ -0,0 +1,6 @@ +#!/bin/sh + +set -eu + +SCRIPT_DIR=$(cd "$(dirname "$0")" && pwd) +exec "$SCRIPT_DIR/supported-configurations" generate diff --git a/bin/supported-configurations b/bin/supported-configurations new file mode 100755 index 00000000..664883ec --- /dev/null +++ b/bin/supported-configurations @@ -0,0 +1,21 @@ +#!/bin/sh + +set -eu + +SCRIPT_DIR=$(cd "$(dirname "$0")" && pwd) +REPO_ROOT=$(cd "$SCRIPT_DIR/.." && pwd) +SOURCE="$SCRIPT_DIR/supported-configurations.cpp" +BINARY="$SCRIPT_DIR/.supported-configurations" + +compiler=${CXX:-c++} + +if [ ! -x "$BINARY" ] || [ "$SOURCE" -nt "$BINARY" ]; then + # `CXX` may contain wrappers/flags (e.g. "ccache c++"). + ( + # shellcheck disable=SC2086 + set -- $compiler + "$@" -std=c++17 -O2 -I"$REPO_ROOT/src/datadog" "$SOURCE" -o "$BINARY" + ) +fi + +exec "$BINARY" "$@" diff --git a/bin/supported-configurations.cpp b/bin/supported-configurations.cpp new file mode 100644 index 00000000..4472eeb7 --- /dev/null +++ b/bin/supported-configurations.cpp @@ -0,0 +1,648 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "json.hpp" + +namespace fs = std::filesystem; +using nlohmann::json; + +namespace { + +const std::vector kScanDirs = {"include", "src", "test", "examples", + "fuzz"}; +const std::set kScanSuffixes = {".h", ".hh", ".hpp", ".c", + ".cc", ".cpp", ".cxx"}; +const std::vector kDynamicScanDirs = {"include", "src"}; +const std::set kAllowedDynamicGetenvPaths = { + "include/datadog/environment.h", + "src/datadog/environment.cpp", +}; + +const std::regex kEnvNameRe("^(?:DD|OTEL)_[A-Z0-9_]+$"); +const std::regex kEnvEnumRe("\\b(?:environment|env)::((?:DD|OTEL)_[A-Z0-9_]+)\\b"); +const std::regex kGetenvRe( + "\\b(?:std::)?getenv\\s*\\(\\s*\"((?:DD|OTEL)_[A-Z0-9_]+)\"\\s*\\)"); +const std::regex kGetenvCallRe("\\b(?:std::)?getenv\\s*\\(\\s*([^)]+?)\\s*\\)"); +const std::regex kSetenvRe( + "\\b(?:setenv|setenv_s|_putenv_s?)\\s*\\(\\s*\"((?:DD|OTEL)_[A-Z0-9_]+)"); +const std::regex kEnvGuardRe( + "\\bEnvGuard\\b[\\w\\s:<>&,*]*[\\(\\{]\\s*\"((?:DD|OTEL)_[A-Z0-9_]+)\""); +const std::regex kStringEnvRe("\"((?:DD|OTEL)_[A-Z0-9_]+)\""); +const std::regex kDefaultResolvedInCodeRe( + "^ENV_DEFAULT_RESOLVED_IN_CODE\\s*\\((.+)\\)$"); +const std::regex kNumericRe("-?(?:[0-9]+(?:\\.[0-9]+)?|\\.[0-9]+)"); + +struct EnvVarDefinition { + std::string name; + std::string type_token; + std::string default_token; +}; + +struct Context { + fs::path repo_root; + fs::path registry_header; + fs::path supported_config_path; +}; + +std::string trim(std::string input) { + const auto not_space = [](unsigned char c) { return !std::isspace(c); }; + auto begin = std::find_if(input.begin(), input.end(), not_space); + auto end = std::find_if(input.rbegin(), input.rend(), not_space).base(); + if (begin >= end) { + return ""; + } + return std::string(begin, end); +} + +void replace_all(std::string &text, const std::string &from, + const std::string &to) { + if (from.empty()) { + return; + } + std::size_t start = 0; + while ((start = text.find(from, start)) != std::string::npos) { + text.replace(start, from.size(), to); + start += to.size(); + } +} + +std::string path_to_posix(const fs::path &path) { return path.generic_string(); } + +bool is_quoted_string_literal(const std::string &text) { + return text.size() >= 2 && text.front() == '"' && text.back() == '"'; +} + +std::string relative_posix_path(const Context &context, const fs::path &path) { + return path_to_posix(fs::relative(path, context.repo_root)); +} + +std::string read_text(const fs::path &path) { + std::ifstream file(path, std::ios::binary); + if (!file) { + throw std::runtime_error("Unable to read file: " + path_to_posix(path)); + } + return std::string(std::istreambuf_iterator(file), + std::istreambuf_iterator()); +} + +std::vector find_regex_matches(const std::string &content, + const std::regex &pattern) { + std::vector matches; + for (auto it = std::sregex_iterator(content.begin(), content.end(), pattern); + it != std::sregex_iterator(); ++it) { + matches.push_back((*it)[1].str()); + } + return matches; +} + +std::optional> parse_macro_arguments( + const std::string &source, std::size_t open_paren, std::size_t &close_paren) { + std::array arguments; + std::size_t argument_index = 0; + std::size_t argument_start = open_paren + 1; + int paren_depth = 0; + int brace_depth = 0; + int bracket_depth = 0; + bool in_single = false; + bool in_double = false; + bool in_line_comment = false; + bool in_block_comment = false; + bool escape = false; + + for (std::size_t index = open_paren; index < source.size(); ++index) { + const char ch = source[index]; + const char next = (index + 1 < source.size()) ? source[index + 1] : '\0'; + + if (in_line_comment) { + if (ch == '\n') in_line_comment = false; + continue; + } + if (in_block_comment) { + if (ch == '*' && next == '/') in_block_comment = false; + continue; + } + if (in_single) { + if (escape) { + escape = false; + } else if (ch == '\\') { + escape = true; + } else if (ch == '\'') { + in_single = false; + } + continue; + } + if (in_double) { + if (escape) { + escape = false; + } else if (ch == '\\') { + escape = true; + } else if (ch == '"') { + in_double = false; + } + continue; + } + + if (ch == '/' && next == '/') { + in_line_comment = true; + continue; + } + if (ch == '/' && next == '*') { + in_block_comment = true; + continue; + } + if (ch == '\'') { + in_single = true; + continue; + } + if (ch == '"') { + in_double = true; + continue; + } + + if (ch == '(') { + ++paren_depth; + continue; + } + if (ch == ')') { + --paren_depth; + if (paren_depth == 0) { + close_paren = index; + if (argument_index < arguments.size()) { + arguments[argument_index++] = + trim(source.substr(argument_start, index - argument_start)); + } + break; + } + continue; + } + if (ch == '{') { + ++brace_depth; + continue; + } + if (ch == '}') { + --brace_depth; + continue; + } + if (ch == '[') { + ++bracket_depth; + continue; + } + if (ch == ']') { + --bracket_depth; + continue; + } + + if (ch == ',' && paren_depth == 1 && brace_depth == 0 && bracket_depth == 0) { + if (argument_index >= arguments.size()) return std::nullopt; + arguments[argument_index++] = + trim(source.substr(argument_start, index - argument_start)); + argument_start = index + 1; + } + } + + if (argument_index != arguments.size()) return std::nullopt; + return arguments; +} + +std::string decode_cpp_single_string_literal(std::string text) { + std::string body = trim(std::move(text)); + if (!is_quoted_string_literal(body)) { + throw std::runtime_error("Expected quoted C++ string literal"); + } + body = body.substr(1, body.size() - 2); + replace_all(body, "\\\\", "\\"); + replace_all(body, "\\\"", "\""); + replace_all(body, "\\n", "\n"); + replace_all(body, "\\t", "\t"); + return body; +} + +std::string decode_cpp_string_literal_sequence(const std::string &text) { + const std::string input = trim(text); + std::string decoded; + std::size_t index = 0; + while (index < input.size()) { + while (index < input.size() && + std::isspace(static_cast(input[index]))) { + ++index; + } + if (index == input.size()) { + break; + } + if (input[index] != '"') { + throw std::runtime_error( + "Expected one or more quoted C++ string literals"); + } + + std::size_t end = index + 1; + bool escaped = false; + for (; end < input.size(); ++end) { + const char ch = input[end]; + if (escaped) { + escaped = false; + continue; + } + if (ch == '\\') { + escaped = true; + continue; + } + if (ch == '"') { + break; + } + } + if (end >= input.size()) { + throw std::runtime_error("Unterminated C++ string literal"); + } + + decoded += decode_cpp_single_string_literal( + input.substr(index, end - index + 1)); + index = end + 1; + } + return decoded; +} + +std::vector parse_registry_definitions(const Context &context) { + std::string source = read_text(context.registry_header); + replace_all(source, "\\\n", " "); + + const std::string token = "MACRO("; + std::size_t search_index = 0; + std::vector definitions; + std::set seen; + + while (true) { + const std::size_t match_index = source.find(token, search_index); + if (match_index == std::string::npos) { + break; + } + const std::size_t open_paren = match_index + std::string("MACRO").size(); + std::size_t close_paren = open_paren; + const auto arguments = parse_macro_arguments(source, open_paren, close_paren); + search_index = close_paren + 1; + if (!arguments) { + continue; + } + + const std::string name = trim((*arguments)[1]); + const std::string type_token = trim((*arguments)[2]); + const std::string default_token = trim((*arguments)[3]); + + if (!std::regex_match(name, kEnvNameRe)) { + continue; + } + if (seen.count(name)) { + throw std::runtime_error("Duplicate environment variable entry: " + name); + } + seen.insert(name); + definitions.push_back(EnvVarDefinition{name, type_token, default_token}); + } + + if (definitions.empty()) { + throw std::runtime_error("No environment variable definitions found in " + + path_to_posix(context.registry_header)); + } + return definitions; +} + +std::string normalize_default_token(const std::string &token, + const Context &context) { + const std::string value = trim(token); + std::smatch marker; + if (std::regex_match(value, marker, kDefaultResolvedInCodeRe)) { + std::string message = trim(marker[1].str()); + if (message.empty() || message.front() != '"') { + throw std::runtime_error( + "ENV_DEFAULT_RESOLVED_IN_CODE(...) must contain a quoted string in " + + path_to_posix(context.registry_header)); + } + return decode_cpp_string_literal_sequence(message); + } + + if (is_quoted_string_literal(value)) { + return decode_cpp_string_literal_sequence(value); + } + if (value == "true" || value == "false") { + return value; + } + if (value == "[]") { + return value; + } + if (std::regex_match(value, kNumericRe)) { + return value; + } + throw std::runtime_error("Unsupported default token '" + value + "' in " + + path_to_posix(context.registry_header)); +} + +std::vector iter_source_files(const Context &context, + const std::vector &dirs) { + std::vector paths; + for (const auto &directory : dirs) { + const fs::path root = context.repo_root / directory; + if (!fs::exists(root)) { + continue; + } + for (auto it = fs::recursive_directory_iterator(root); + it != fs::recursive_directory_iterator(); ++it) { + if (!it->is_regular_file()) { + continue; + } + if (kScanSuffixes.count(it->path().extension().string()) == 0) { + continue; + } + paths.push_back(it->path()); + } + } + std::sort(paths.begin(), paths.end()); + return paths; +} + +using UsedVars = std::map>; + +UsedVars discover_used_env_variables(const Context &context) { + UsedVars used; + for (const auto &path : iter_source_files(context, kScanDirs)) { + const std::string content = read_text(path); + const std::string relative = relative_posix_path(context, path); + + for (const auto &match : find_regex_matches(content, kEnvEnumRe)) { + used[match].insert(relative); + } + for (const auto &match : find_regex_matches(content, kGetenvRe)) { + used[match].insert(relative); + } + for (const auto &match : find_regex_matches(content, kSetenvRe)) { + used[match].insert(relative); + } + for (const auto &match : find_regex_matches(content, kEnvGuardRe)) { + used[match].insert(relative); + } + if (content.find("EnvGuard") != std::string::npos) { + for (const auto &match : find_regex_matches(content, kStringEnvRe)) { + used[match].insert(relative); + } + } + } + return used; +} + +using DynamicCalls = std::map>; + +DynamicCalls discover_disallowed_dynamic_getenv_calls(const Context &context) { + DynamicCalls calls; + for (const auto &path : iter_source_files(context, kDynamicScanDirs)) { + const std::string relative = relative_posix_path(context, path); + if (kAllowedDynamicGetenvPaths.count(relative) > 0) { + continue; + } + const std::string content = read_text(path); + for (const auto &argument : find_regex_matches(content, kGetenvCallRe)) { + const auto trimmed = trim(argument); + if (is_quoted_string_literal(trimmed)) { + continue; + } + calls[relative].insert(trimmed); + } + } + return calls; +} + +struct Verification { + bool ok; + std::vector errors; +}; + +struct PreflightResult { + Verification allowlist; + Verification dynamic_getenv; +}; + +Verification verify_used_variables_are_allowlisted( + const std::set &allowlisted, const UsedVars &used) { + std::vector missing; + for (const auto &[var, _] : used) { + if (!allowlisted.count(var)) { + missing.push_back(var); + } + } + if (missing.empty()) { + return {true, {}}; + } + + std::vector lines; + lines.push_back( + "Found DD_/OTEL_ environment variables used in code that are not declared in " + "include/datadog/environment_registry.h:"); + for (const auto &variable : missing) { + std::string locations; + bool first = true; + for (const auto &path : used.at(variable)) { + if (!first) { + locations += ", "; + } + first = false; + locations += path; + } + lines.push_back(" - " + variable + " (" + locations + ")"); + } + lines.push_back( + "Add missing variables to include/datadog/environment_registry.h (which " + "drives include/datadog/environment.h)."); + return {false, lines}; +} + +Verification verify_no_disallowed_dynamic_getenv_calls( + const DynamicCalls &dynamic_calls) { + if (dynamic_calls.empty()) { + return {true, {}}; + } + + std::vector lines; + lines.push_back( + "Found dynamic getenv(...) access in include/src. Dynamic environment " + "variable access is prohibited for tracer configuration paths."); + for (const auto &[path, arguments] : dynamic_calls) { + std::string joined; + bool first = true; + for (const auto &arg : arguments) { + if (!first) { + joined += ", "; + } + first = false; + joined += arg; + } + lines.push_back(" - " + path + " (arguments: " + joined + ")"); + } + lines.push_back( + "Use include/datadog/environment_registry.h plus environment::lookup<...>() " + "instead."); + return {false, lines}; +} + +PreflightResult run_preflight(const Context &context, + const std::vector &definitions) { + std::set allowlisted; + for (const auto &definition : definitions) { + allowlisted.insert(definition.name); + } + + return { + verify_used_variables_are_allowlisted(allowlisted, + discover_used_env_variables(context)), + verify_no_disallowed_dynamic_getenv_calls( + discover_disallowed_dynamic_getenv_calls(context)), + }; +} + +void print_verification_errors(const Verification &verification) { + for (const auto &line : verification.errors) { + std::cout << line << '\n'; + } +} + +json build_supported_configurations(const std::vector &definitions, + const Context &context) { + const std::map type_map = { + {"STRING", "string"}, {"BOOLEAN", "boolean"}, {"INT", "int"}, + {"DECIMAL", "decimal"}, {"ARRAY", "array"}, {"MAP", "map"}}; + + std::vector sorted = definitions; + std::sort(sorted.begin(), sorted.end(), + [](const EnvVarDefinition &a, const EnvVarDefinition &b) { + return a.name < b.name; + }); + + json supported = json::object(); + for (const auto &definition : sorted) { + const auto type_it = type_map.find(definition.type_token); + if (type_it == type_map.end()) { + throw std::runtime_error("Unsupported type token '" + definition.type_token + + "' for " + definition.name); + } + supported[definition.name] = json::array( + {json{{"implementation", "A"}, + {"type", type_it->second}, + {"default", normalize_default_token(definition.default_token, context)}}}); + } + + if (supported.size() != definitions.size()) { + throw std::runtime_error( + "Internal error: every registry definition must map to exactly one " + "supported configuration entry"); + } + return supported; +} + +json load_existing_deprecations(const Context &context) { + if (!fs::exists(context.supported_config_path)) { + return json::object(); + } + try { + const auto parsed = json::parse(read_text(context.supported_config_path)); + if (parsed.contains("deprecations") && parsed["deprecations"].is_object()) { + return parsed["deprecations"]; + } + return json::object(); + } catch (...) { + return json::object(); + } +} + +void write_supported_configurations_json(const Context &context, + const json &supported_configurations) { + json output = json::object(); + output["version"] = "2"; + output["supportedConfigurations"] = supported_configurations; + output["deprecations"] = load_existing_deprecations(context); + + fs::create_directories(context.supported_config_path.parent_path()); + std::ofstream file(context.supported_config_path, std::ios::binary); + if (!file) { + throw std::runtime_error("Unable to write " + + path_to_posix(context.supported_config_path)); + } + file << output.dump(2) << '\n'; +} + +int run_check(const Context &context) { + const auto definitions = parse_registry_definitions(context); + const auto preflight = run_preflight(context, definitions); + + if (preflight.allowlist.ok && preflight.dynamic_getenv.ok) { + std::cout << "Environment variable allowlist check passed.\n"; + return 0; + } + print_verification_errors(preflight.allowlist); + print_verification_errors(preflight.dynamic_getenv); + return 1; +} + +int run_generate(const Context &context) { + const auto definitions = parse_registry_definitions(context); + const auto preflight = run_preflight(context, definitions); + if (!preflight.allowlist.ok || !preflight.dynamic_getenv.ok) { + print_verification_errors(preflight.allowlist); + print_verification_errors(preflight.dynamic_getenv); + return 1; + } + + const auto supported = build_supported_configurations(definitions, context); + if (supported.empty()) { + std::cout << "Error: no supported configurations were generated.\n"; + return 1; + } + + write_supported_configurations_json(context, supported); + std::cout << "Wrote " + << path_to_posix(fs::relative(context.supported_config_path, + context.repo_root)) + << '\n'; + return 0; +} + +Context make_context(const char *argv0) { + fs::path executable = fs::absolute(argv0); + fs::path script_dir = executable.parent_path(); + fs::path repo_root = script_dir.parent_path(); + return Context{ + repo_root, + repo_root / "include/datadog/environment_registry.h", + repo_root / "metadata/supported-configurations.json", + }; +} + +} // namespace + +int main(int argc, char **argv) { + try { + constexpr const char kUsage[] = "Usage: supported-configurations \n"; + if (argc != 2) { + std::cerr << kUsage; + return 1; + } + const std::string command = argv[1]; + const Context context = make_context(argv[0]); + if (command == "check") { + return run_check(context); + } + if (command == "generate") { + return run_generate(context); + } + std::cerr << kUsage; + return 1; + } catch (const std::exception &error) { + std::cerr << error.what() << '\n'; + return 1; + } +} diff --git a/include/datadog/environment.h b/include/datadog/environment.h index 358d0bcb..baa33f58 100644 --- a/include/datadog/environment.h +++ b/include/datadog/environment.h @@ -6,91 +6,149 @@ // Each `enum Variable` denotes an environment variable. The enum value names // are the same as the names of the environment variables. // -// `variable_names` is an array of the names of the environment variables. Nginx -// uses `variable_names` as an allow list of environment variables to forward to -// worker processes. -// // `name` returns the name of a specified `Variable`. // // `lookup` retrieves the value of `Variable` in the environment. +#include +#include #include #include +#include +#include + namespace datadog { namespace tracing { namespace environment { -// To enforce correspondence between `enum Variable` and `variable_names`, the -// preprocessor is used so that the DD_* symbols are listed exactly once. -#define LIST_ENVIRONMENT_VARIABLES(MACRO) \ - MACRO(DD_AGENT_HOST) \ - MACRO(DD_ENV) \ - MACRO(DD_INSTRUMENTATION_TELEMETRY_ENABLED) \ - MACRO(DD_PROPAGATION_STYLE_EXTRACT) \ - MACRO(DD_PROPAGATION_STYLE_INJECT) \ - MACRO(DD_REMOTE_CONFIGURATION_ENABLED) \ - MACRO(DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS) \ - MACRO(DD_SERVICE) \ - MACRO(DD_SPAN_SAMPLING_RULES) \ - MACRO(DD_SPAN_SAMPLING_RULES_FILE) \ - MACRO(DD_TRACE_PROPAGATION_STYLE_EXTRACT) \ - MACRO(DD_TRACE_PROPAGATION_STYLE_INJECT) \ - MACRO(DD_TRACE_PROPAGATION_STYLE) \ - MACRO(DD_TAGS) \ - MACRO(DD_TRACE_AGENT_PORT) \ - MACRO(DD_TRACE_AGENT_URL) \ - MACRO(DD_TRACE_DEBUG) \ - MACRO(DD_TRACE_ENABLED) \ - MACRO(DD_TRACE_RATE_LIMIT) \ - MACRO(DD_TRACE_REPORT_HOSTNAME) \ - MACRO(DD_TRACE_SAMPLE_RATE) \ - MACRO(DD_TRACE_SAMPLING_RULES) \ - MACRO(DD_TRACE_STARTUP_LOGS) \ - MACRO(DD_TRACE_TAGS_PROPAGATION_MAX_LENGTH) \ - MACRO(DD_VERSION) \ - MACRO(DD_TRACE_128_BIT_TRACEID_GENERATION_ENABLED) \ - MACRO(DD_TELEMETRY_HEARTBEAT_INTERVAL) \ - MACRO(DD_TELEMETRY_METRICS_ENABLED) \ - MACRO(DD_TELEMETRY_METRICS_INTERVAL_SECONDS) \ - MACRO(DD_TELEMETRY_DEBUG) \ - MACRO(DD_TRACE_BAGGAGE_MAX_ITEMS) \ - MACRO(DD_TRACE_BAGGAGE_MAX_BYTES) \ - MACRO(DD_TELEMETRY_LOG_COLLECTION_ENABLED) \ - MACRO(DD_INSTRUMENTATION_INSTALL_ID) \ - MACRO(DD_INSTRUMENTATION_INSTALL_TYPE) \ - MACRO(DD_INSTRUMENTATION_INSTALL_TIME) \ - MACRO(DD_APM_TRACING_ENABLED) \ - MACRO(DD_TRACE_RESOURCE_RENAMING_ENABLED) \ - MACRO(DD_TRACE_RESOURCE_RENAMING_ALWAYS_SIMPLIFIED_ENDPOINT) \ - MACRO(DD_EXTERNAL_ENV) - -#define WITH_COMMA(ARG) ARG, - -enum Variable { LIST_ENVIRONMENT_VARIABLES(WITH_COMMA) }; +enum class VariableType { + STRING, + BOOLEAN, + INT, + DECIMAL, + ARRAY, + MAP, +}; + +struct VariableSpec { + StringView name; + VariableType type; +}; + +#define VARIABLE_ENUM_VALUE(DATA, NAME, TYPE, DEFAULT_VALUE) NAME, + +enum Variable { DD_ENVIRONMENT_VARIABLES(VARIABLE_ENUM_VALUE, ~) }; // Quoting a macro argument requires this two-step. #define QUOTED_IMPL(ARG) #ARG #define QUOTED(ARG) QUOTED_IMPL(ARG) -#define QUOTED_WITH_COMMA(ARG) WITH_COMMA(QUOTED(ARG)) +#define VARIABLE_SPEC_WITH_COMMA(DATA, NAME, TYPE, DEFAULT_VALUE) \ + VariableSpec{StringView{QUOTED(NAME)}, VariableType::TYPE}, + +inline const VariableSpec variable_specs[] = { + DD_ENVIRONMENT_VARIABLES(VARIABLE_SPEC_WITH_COMMA, ~)}; + +template +struct LookupResultByType; + +template <> +struct LookupResultByType { + using type = Optional; +}; + +template <> +struct LookupResultByType { + using type = Optional; +}; + +template <> +struct LookupResultByType { + using type = Expected>; +}; + +template <> +struct LookupResultByType { + using type = Expected>; +}; -inline const char *const variable_names[] = { - LIST_ENVIRONMENT_VARIABLES(QUOTED_WITH_COMMA)}; +template <> +struct LookupResultByType { + using type = Optional; +}; -#undef QUOTED_WITH_COMMA +template <> +struct LookupResultByType { + using type = Optional; +}; + +template +struct VariableTraits; + +#define VARIABLE_TRAITS_VALUE(DATA, NAME, TYPE, DEFAULT_VALUE) \ + template <> \ + struct VariableTraits { \ + static constexpr VariableType variable_type = VariableType::TYPE; \ + static constexpr const char *name() { return QUOTED(NAME); } \ + using lookup_result = typename LookupResultByType::type; \ + }; + +DD_ENVIRONMENT_VARIABLES(VARIABLE_TRAITS_VALUE, ~) + +template +using LookupResult = typename VariableTraits::lookup_result; + +namespace detail { +template +inline constexpr bool unsupported_variable_type_v = false; + +template +Optional lookup_raw() { + const char *value = std::getenv(VariableTraits::name()); + if (!value) { + return nullopt; + } + return StringView{value}; +} + +Optional lookup_bool_from_raw(Optional value); +Expected> lookup_uint64_from_raw( + Optional value); +Expected> lookup_double_from_raw(Optional value); +} // namespace detail + +template +LookupResult lookup() { + constexpr VariableType type = VariableTraits::variable_type; + const auto raw = detail::lookup_raw(); + if constexpr (type == VariableType::STRING || type == VariableType::ARRAY || + type == VariableType::MAP) { + return raw; + } else if constexpr (type == VariableType::BOOLEAN) { + return detail::lookup_bool_from_raw(raw); + } else if constexpr (type == VariableType::INT) { + return detail::lookup_uint64_from_raw(raw); + } else if constexpr (type == VariableType::DECIMAL) { + return detail::lookup_double_from_raw(raw); + } else { + static_assert(detail::unsupported_variable_type_v, + "Unsupported environment variable type"); + } +} + +#undef VARIABLE_SPEC_WITH_COMMA +#undef VARIABLE_TRAITS_VALUE #undef QUOTED #undef QUOTED_IMPL -#undef WITH_COMMA -#undef LIST_ENVIRONMENT_VARIABLES +#undef VARIABLE_ENUM_VALUE + +// Return the metadata for the specified environment `variable`. +const VariableSpec &spec(Variable variable); // Return the name of the specified environment `variable`. StringView name(Variable variable); -// Return the value of the specified environment `variable`, or return -// `nullopt` if that variable is not set in the environment. -Optional lookup(Variable variable); - std::string to_json(); } // namespace environment diff --git a/include/datadog/environment_registry.h b/include/datadog/environment_registry.h new file mode 100644 index 00000000..c519bb0a --- /dev/null +++ b/include/datadog/environment_registry.h @@ -0,0 +1,70 @@ +#pragma once + +// Central registry for supported environment variables. +// +// Each entry has: +// - NAME: environment variable symbol (e.g. DD_SERVICE) +// - TYPE: STRING | BOOLEAN | INT | DECIMAL | ARRAY | MAP +// - DEFAULT: literal default value or a marker token +// +// Marker tokens: +// - ENV_DEFAULT_RESOLVED_IN_CODE("...description...") +// The runtime default is resolved in C++ configuration finalization +// logic. The description is emitted as the "default" field in +// metadata/supported-configurations.json. +// +// This registry is the single source of truth for: +// - env variable name allowlist (`include/datadog/environment.h`) +// - generated metadata (`metadata/supported-configurations.json`) + +#define DD_ENVIRONMENT_VARIABLES(MACRO, DATA) \ + MACRO(DATA, DD_AGENT_HOST, STRING, "localhost") \ + MACRO(DATA, DD_ENV, STRING, "") \ + MACRO(DATA, DD_INSTRUMENTATION_TELEMETRY_ENABLED, BOOLEAN, true) \ + MACRO(DATA, DD_PROPAGATION_STYLE_EXTRACT, ARRAY, \ + "datadog,tracecontext,baggage") \ + MACRO(DATA, DD_PROPAGATION_STYLE_INJECT, ARRAY, \ + "datadog,tracecontext,baggage") \ + MACRO(DATA, DD_REMOTE_CONFIGURATION_ENABLED, BOOLEAN, true) \ + MACRO(DATA, DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS, DECIMAL, 5.0) \ + MACRO(DATA, DD_SERVICE, STRING, \ + ENV_DEFAULT_RESOLVED_IN_CODE("Defaults to process name when unset.")) \ + MACRO(DATA, DD_SPAN_SAMPLING_RULES, ARRAY, "[]") \ + MACRO(DATA, DD_SPAN_SAMPLING_RULES_FILE, STRING, "") \ + MACRO(DATA, DD_TRACE_PROPAGATION_STYLE_EXTRACT, ARRAY, \ + "datadog,tracecontext,baggage") \ + MACRO(DATA, DD_TRACE_PROPAGATION_STYLE_INJECT, ARRAY, \ + "datadog,tracecontext,baggage") \ + MACRO(DATA, DD_TRACE_PROPAGATION_STYLE, ARRAY, \ + "datadog,tracecontext,baggage") \ + MACRO(DATA, DD_TAGS, MAP, "") \ + MACRO(DATA, DD_TRACE_AGENT_PORT, INT, 8126) \ + MACRO(DATA, DD_TRACE_AGENT_URL, STRING, \ + ENV_DEFAULT_RESOLVED_IN_CODE( \ + "If unset, built from DD_AGENT_HOST and DD_TRACE_AGENT_PORT, " \ + "then defaults to http://localhost:8126.")) \ + MACRO(DATA, DD_TRACE_DEBUG, BOOLEAN, false) \ + MACRO(DATA, DD_TRACE_ENABLED, BOOLEAN, true) \ + MACRO(DATA, DD_TRACE_RATE_LIMIT, DECIMAL, 100.0) \ + MACRO(DATA, DD_TRACE_REPORT_HOSTNAME, BOOLEAN, false) \ + MACRO(DATA, DD_TRACE_SAMPLE_RATE, DECIMAL, 1.0) \ + MACRO(DATA, DD_TRACE_SAMPLING_RULES, ARRAY, "[]") \ + MACRO(DATA, DD_TRACE_STARTUP_LOGS, BOOLEAN, true) \ + MACRO(DATA, DD_TRACE_TAGS_PROPAGATION_MAX_LENGTH, INT, 512) \ + MACRO(DATA, DD_VERSION, STRING, "") \ + MACRO(DATA, DD_TRACE_128_BIT_TRACEID_GENERATION_ENABLED, BOOLEAN, true) \ + MACRO(DATA, DD_TELEMETRY_HEARTBEAT_INTERVAL, DECIMAL, 10) \ + MACRO(DATA, DD_TELEMETRY_METRICS_ENABLED, BOOLEAN, true) \ + MACRO(DATA, DD_TELEMETRY_METRICS_INTERVAL_SECONDS, DECIMAL, 60) \ + MACRO(DATA, DD_TELEMETRY_DEBUG, BOOLEAN, false) \ + MACRO(DATA, DD_TRACE_BAGGAGE_MAX_ITEMS, INT, 64) \ + MACRO(DATA, DD_TRACE_BAGGAGE_MAX_BYTES, INT, 8192) \ + MACRO(DATA, DD_TELEMETRY_LOG_COLLECTION_ENABLED, BOOLEAN, true) \ + MACRO(DATA, DD_INSTRUMENTATION_INSTALL_ID, STRING, "") \ + MACRO(DATA, DD_INSTRUMENTATION_INSTALL_TYPE, STRING, "") \ + MACRO(DATA, DD_INSTRUMENTATION_INSTALL_TIME, STRING, "") \ + MACRO(DATA, DD_APM_TRACING_ENABLED, BOOLEAN, true) \ + MACRO(DATA, DD_TRACE_RESOURCE_RENAMING_ENABLED, BOOLEAN, false) \ + MACRO(DATA, DD_TRACE_RESOURCE_RENAMING_ALWAYS_SIMPLIFIED_ENDPOINT, BOOLEAN, \ + false) \ + MACRO(DATA, DD_EXTERNAL_ENV, STRING, "") diff --git a/metadata/supported-configurations.json b/metadata/supported-configurations.json new file mode 100644 index 00000000..1a67bfac --- /dev/null +++ b/metadata/supported-configurations.json @@ -0,0 +1,286 @@ +{ + "deprecations": {}, + "supportedConfigurations": { + "DD_AGENT_HOST": [ + { + "default": "localhost", + "implementation": "A", + "type": "string" + } + ], + "DD_APM_TRACING_ENABLED": [ + { + "default": "true", + "implementation": "A", + "type": "boolean" + } + ], + "DD_ENV": [ + { + "default": "", + "implementation": "A", + "type": "string" + } + ], + "DD_EXTERNAL_ENV": [ + { + "default": "", + "implementation": "A", + "type": "string" + } + ], + "DD_INSTRUMENTATION_INSTALL_ID": [ + { + "default": "", + "implementation": "A", + "type": "string" + } + ], + "DD_INSTRUMENTATION_INSTALL_TIME": [ + { + "default": "", + "implementation": "A", + "type": "string" + } + ], + "DD_INSTRUMENTATION_INSTALL_TYPE": [ + { + "default": "", + "implementation": "A", + "type": "string" + } + ], + "DD_INSTRUMENTATION_TELEMETRY_ENABLED": [ + { + "default": "true", + "implementation": "A", + "type": "boolean" + } + ], + "DD_PROPAGATION_STYLE_EXTRACT": [ + { + "default": "datadog,tracecontext,baggage", + "implementation": "A", + "type": "array" + } + ], + "DD_PROPAGATION_STYLE_INJECT": [ + { + "default": "datadog,tracecontext,baggage", + "implementation": "A", + "type": "array" + } + ], + "DD_REMOTE_CONFIGURATION_ENABLED": [ + { + "default": "true", + "implementation": "A", + "type": "boolean" + } + ], + "DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS": [ + { + "default": "5.0", + "implementation": "A", + "type": "decimal" + } + ], + "DD_SERVICE": [ + { + "default": "Defaults to process name when unset.", + "implementation": "A", + "type": "string" + } + ], + "DD_SPAN_SAMPLING_RULES": [ + { + "default": "[]", + "implementation": "A", + "type": "array" + } + ], + "DD_SPAN_SAMPLING_RULES_FILE": [ + { + "default": "", + "implementation": "A", + "type": "string" + } + ], + "DD_TAGS": [ + { + "default": "", + "implementation": "A", + "type": "map" + } + ], + "DD_TELEMETRY_DEBUG": [ + { + "default": "false", + "implementation": "A", + "type": "boolean" + } + ], + "DD_TELEMETRY_HEARTBEAT_INTERVAL": [ + { + "default": "10", + "implementation": "A", + "type": "decimal" + } + ], + "DD_TELEMETRY_LOG_COLLECTION_ENABLED": [ + { + "default": "true", + "implementation": "A", + "type": "boolean" + } + ], + "DD_TELEMETRY_METRICS_ENABLED": [ + { + "default": "true", + "implementation": "A", + "type": "boolean" + } + ], + "DD_TELEMETRY_METRICS_INTERVAL_SECONDS": [ + { + "default": "60", + "implementation": "A", + "type": "decimal" + } + ], + "DD_TRACE_128_BIT_TRACEID_GENERATION_ENABLED": [ + { + "default": "true", + "implementation": "A", + "type": "boolean" + } + ], + "DD_TRACE_AGENT_PORT": [ + { + "default": "8126", + "implementation": "A", + "type": "int" + } + ], + "DD_TRACE_AGENT_URL": [ + { + "default": "If unset, built from DD_AGENT_HOST and DD_TRACE_AGENT_PORT, then defaults to http://localhost:8126.", + "implementation": "A", + "type": "string" + } + ], + "DD_TRACE_BAGGAGE_MAX_BYTES": [ + { + "default": "8192", + "implementation": "A", + "type": "int" + } + ], + "DD_TRACE_BAGGAGE_MAX_ITEMS": [ + { + "default": "64", + "implementation": "A", + "type": "int" + } + ], + "DD_TRACE_DEBUG": [ + { + "default": "false", + "implementation": "A", + "type": "boolean" + } + ], + "DD_TRACE_ENABLED": [ + { + "default": "true", + "implementation": "A", + "type": "boolean" + } + ], + "DD_TRACE_PROPAGATION_STYLE": [ + { + "default": "datadog,tracecontext,baggage", + "implementation": "A", + "type": "array" + } + ], + "DD_TRACE_PROPAGATION_STYLE_EXTRACT": [ + { + "default": "datadog,tracecontext,baggage", + "implementation": "A", + "type": "array" + } + ], + "DD_TRACE_PROPAGATION_STYLE_INJECT": [ + { + "default": "datadog,tracecontext,baggage", + "implementation": "A", + "type": "array" + } + ], + "DD_TRACE_RATE_LIMIT": [ + { + "default": "100.0", + "implementation": "A", + "type": "decimal" + } + ], + "DD_TRACE_REPORT_HOSTNAME": [ + { + "default": "false", + "implementation": "A", + "type": "boolean" + } + ], + "DD_TRACE_RESOURCE_RENAMING_ALWAYS_SIMPLIFIED_ENDPOINT": [ + { + "default": "false", + "implementation": "A", + "type": "boolean" + } + ], + "DD_TRACE_RESOURCE_RENAMING_ENABLED": [ + { + "default": "false", + "implementation": "A", + "type": "boolean" + } + ], + "DD_TRACE_SAMPLE_RATE": [ + { + "default": "1.0", + "implementation": "A", + "type": "decimal" + } + ], + "DD_TRACE_SAMPLING_RULES": [ + { + "default": "[]", + "implementation": "A", + "type": "array" + } + ], + "DD_TRACE_STARTUP_LOGS": [ + { + "default": "true", + "implementation": "A", + "type": "boolean" + } + ], + "DD_TRACE_TAGS_PROPAGATION_MAX_LENGTH": [ + { + "default": "512", + "implementation": "A", + "type": "int" + } + ], + "DD_VERSION": [ + { + "default": "", + "implementation": "A", + "type": "string" + } + ] + }, + "version": "2" +} diff --git a/src/datadog/datadog_agent_config.cpp b/src/datadog/datadog_agent_config.cpp index 05b3b065..6cf85b72 100644 --- a/src/datadog/datadog_agent_config.cpp +++ b/src/datadog/datadog_agent_config.cpp @@ -1,46 +1,55 @@ #include #include +#include #include #include #include +#include #include "default_http_client.h" -#include "parse_util.h" #include "platform_util.h" #include "threaded_event_scheduler.h" namespace datadog { namespace tracing { -Expected load_datadog_agent_env_config() { +namespace env = environment; + +Expected load_datadog_agent_env_config(Logger& logger) { DatadogAgentConfig env_config; - if (auto rc_enabled = lookup(environment::DD_REMOTE_CONFIGURATION_ENABLED)) { - env_config.remote_configuration_enabled = !falsy(*rc_enabled); + if (auto rc_enabled = env::lookup()) { + env_config.remote_configuration_enabled = *rc_enabled; } - if (auto raw_rc_poll_interval_value = - lookup(environment::DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS)) { - auto res = parse_double(*raw_rc_poll_interval_value); - if (auto error = res.if_error()) { - return error->with_prefix( - "DatadogAgent: Remote Configuration poll interval error "); - } - - env_config.remote_configuration_poll_interval_seconds = *res; + auto raw_rc_poll_interval_value = + env::lookup(); + if (auto error = raw_rc_poll_interval_value.if_error()) { + logger.log_error(error->with_prefix( + "DatadogAgent: Remote Configuration poll interval error ")); + } else if (*raw_rc_poll_interval_value) { + env_config.remote_configuration_poll_interval_seconds = + **raw_rc_poll_interval_value; } - auto env_host = lookup(environment::DD_AGENT_HOST); - auto env_port = lookup(environment::DD_TRACE_AGENT_PORT); + const auto env_host = env::lookup(); + Optional env_port = nullopt; + const auto raw_env_port = env::lookup(); + if (auto* error = raw_env_port.if_error()) { + logger.log_error( + error->with_prefix("DatadogAgent: Agent port parsing error ")); + } else { + env_port = *raw_env_port; + } - if (auto url_env = lookup(environment::DD_TRACE_AGENT_URL)) { + if (auto url_env = env::lookup()) { env_config.url = std::string{*url_env}; } else if (env_host || env_port) { std::string configured_url = "http://"; append(configured_url, env_host.value_or("localhost")); configured_url += ':'; - append(configured_url, env_port.value_or("8126")); + configured_url += std::to_string(env_port.value_or(8126)); env_config.url = std::move(configured_url); } @@ -51,7 +60,8 @@ Expected load_datadog_agent_env_config() { Expected finalize_config( const DatadogAgentConfig& user_config, const std::shared_ptr& logger, const Clock& clock) { - Expected env_config = load_datadog_agent_env_config(); + Expected env_config = + load_datadog_agent_env_config(*logger); if (auto error = env_config.if_error()) { return *error; } @@ -148,7 +158,7 @@ Expected finalize_config( // Starting Datadog Agent 7.62.0, the admission controller inject a unique // identifier through `DD_EXTERNAL_ENV`. This uid is used for origin // detection. - if (auto external_env = lookup(environment::DD_EXTERNAL_ENV)) { + if (auto external_env = env::lookup()) { result.admission_controller_uid = std::string(*external_env); } diff --git a/src/datadog/environment.cpp b/src/datadog/environment.cpp index 4dc3eb3f..c48c9de8 100644 --- a/src/datadog/environment.cpp +++ b/src/datadog/environment.cpp @@ -3,31 +3,60 @@ #include #include "json.hpp" +#include "parse_util.h" namespace datadog { namespace tracing { namespace environment { -StringView name(Variable variable) { return variable_names[variable]; } +namespace detail { +Optional lookup_bool_from_raw(Optional value) { + if (value) { + return !falsy(*value); + } + return nullopt; +} + +Expected> lookup_uint64_from_raw( + Optional value) { + if (!value) { + return Optional{}; + } + auto parsed = parse_uint64(*value, 10); + if (auto *error = parsed.if_error()) { + return *error; + } + return Optional{*parsed}; +} -Optional lookup(Variable variable) { - const char *name = variable_names[variable]; - const char *value = std::getenv(name); +Expected> lookup_double_from_raw(Optional value) { if (!value) { - return nullopt; + return Optional{}; + } + auto parsed = parse_double(*value); + if (auto *error = parsed.if_error()) { + return *error; } - return StringView{value}; + return Optional{*parsed}; } +} // namespace detail + +const VariableSpec &spec(Variable variable) { return variable_specs[variable]; } + +StringView name(Variable variable) { return spec(variable).name; } std::string to_json() { auto result = nlohmann::json::object({}); - for (const char *name : variable_names) { - if (const char *value = std::getenv(name)) { - result[name] = value; - } +#define ADD_ENV_TO_JSON_IF_SET(DATA, NAME, TYPE, DEFAULT_VALUE) \ + if (const char *value = std::getenv(VariableTraits::name())) { \ + result[VariableTraits::name()] = value; \ } + DD_ENVIRONMENT_VARIABLES(ADD_ENV_TO_JSON_IF_SET, ~) + +#undef ADD_ENV_TO_JSON_IF_SET + return result.dump(); } diff --git a/src/datadog/span_sampler_config.cpp b/src/datadog/span_sampler_config.cpp index cf1b60fb..b9288cad 100644 --- a/src/datadog/span_sampler_config.cpp +++ b/src/datadog/span_sampler_config.cpp @@ -12,6 +12,9 @@ namespace datadog { namespace tracing { + +namespace env = environment; + namespace { std::string to_string(const std::vector &rules) { @@ -146,21 +149,20 @@ Expected> parse_rules(StringView rules_raw, Expected load_span_sampler_env_config(Logger &logger) { SpanSamplerConfig env_config; - auto rules_env = lookup(environment::DD_SPAN_SAMPLING_RULES); + auto rules_env = env::lookup(); if (rules_env) { auto maybe_rules = - parse_rules(*rules_env, name(environment::DD_SPAN_SAMPLING_RULES)); + parse_rules(*rules_env, name(env::DD_SPAN_SAMPLING_RULES)); if (auto *error = maybe_rules.if_error()) { return std::move(*error); } env_config.rules = std::move(*maybe_rules); } - if (auto file_env = lookup(environment::DD_SPAN_SAMPLING_RULES_FILE)) { + if (auto file_env = env::lookup()) { if (rules_env) { - const auto rules_file_name = - name(environment::DD_SPAN_SAMPLING_RULES_FILE); - const auto rules_name = name(environment::DD_SPAN_SAMPLING_RULES); + const auto rules_file_name = name(env::DD_SPAN_SAMPLING_RULES_FILE); + const auto rules_name = name(env::DD_SPAN_SAMPLING_RULES); std::string message; append(message, rules_file_name); message += " is overridden by "; @@ -181,7 +183,7 @@ Expected load_span_sampler_env_config(Logger &logger) { message += " file \""; message += span_rules_file; message += "\" specified as value of environment variable "; - append(message, name(environment::DD_SPAN_SAMPLING_RULES_FILE)); + append(message, name(env::DD_SPAN_SAMPLING_RULES_FILE)); return Error{Error::SPAN_SAMPLING_RULES_FILE_IO, std::move(message)}; }; @@ -197,12 +199,12 @@ Expected load_span_sampler_env_config(Logger &logger) { return file_error("read"); } - auto maybe_rules = parse_rules( - rules_stream.str(), name(environment::DD_SPAN_SAMPLING_RULES_FILE)); + auto maybe_rules = parse_rules(rules_stream.str(), + name(env::DD_SPAN_SAMPLING_RULES_FILE)); if (auto *error = maybe_rules.if_error()) { std::string prefix; prefix += "With "; - append(prefix, name(environment::DD_SPAN_SAMPLING_RULES_FILE)); + append(prefix, name(env::DD_SPAN_SAMPLING_RULES_FILE)); prefix += '='; append(prefix, *file_env); prefix += ": "; diff --git a/src/datadog/telemetry/configuration.cpp b/src/datadog/telemetry/configuration.cpp index cc8d2e85..1836dba8 100644 --- a/src/datadog/telemetry/configuration.cpp +++ b/src/datadog/telemetry/configuration.cpp @@ -3,49 +3,41 @@ #include #include -#include "parse_util.h" - using namespace datadog::tracing; namespace datadog::telemetry { +namespace env = tracing::environment; + namespace { tracing::Expected load_telemetry_env_config() { Configuration env_cfg; if (auto enabled_env = - lookup(environment::DD_INSTRUMENTATION_TELEMETRY_ENABLED)) { - env_cfg.enabled = !falsy(*enabled_env); + env::lookup()) { + env_cfg.enabled = *enabled_env; } - if (auto metrics_enabled = - lookup(environment::DD_TELEMETRY_METRICS_ENABLED)) { - env_cfg.report_metrics = !falsy(*metrics_enabled); + if (auto metrics_enabled = env::lookup()) { + env_cfg.report_metrics = *metrics_enabled; } if (auto logs_enabled = - lookup(environment::DD_TELEMETRY_LOG_COLLECTION_ENABLED)) { - env_cfg.report_logs = !falsy(*logs_enabled); + env::lookup()) { + env_cfg.report_logs = *logs_enabled; } - if (auto metrics_interval_seconds = - lookup(environment::DD_TELEMETRY_METRICS_INTERVAL_SECONDS)) { - auto maybe_value = parse_double(*metrics_interval_seconds); - if (auto error = maybe_value.if_error()) { - return *error; - } - env_cfg.metrics_interval_seconds = *maybe_value; + auto metrics_interval_seconds = + env::lookup(); + if (!metrics_interval_seconds.if_error() && *metrics_interval_seconds) { + env_cfg.metrics_interval_seconds = **metrics_interval_seconds; } - if (auto heartbeat_interval_seconds = - lookup(environment::DD_TELEMETRY_HEARTBEAT_INTERVAL)) { - auto maybe_value = parse_double(*heartbeat_interval_seconds); - if (auto error = maybe_value.if_error()) { - return *error; - } - - env_cfg.heartbeat_interval_seconds = *maybe_value; + auto heartbeat_interval_seconds = + env::lookup(); + if (!heartbeat_interval_seconds.if_error() && *heartbeat_interval_seconds) { + env_cfg.heartbeat_interval_seconds = **heartbeat_interval_seconds; } return env_cfg; @@ -83,8 +75,8 @@ tracing::Expected finalize_config( } // debug - if (auto enabled_debug_env = lookup(environment::DD_TELEMETRY_DEBUG)) { - result.debug = !falsy(*enabled_debug_env); + if (auto enabled_debug_env = env::lookup()) { + result.debug = *enabled_debug_env; } else { result.debug = false; } @@ -126,15 +118,13 @@ tracing::Expected finalize_config( result.products = user_config.products; // onboarding data - if (auto install_id = lookup(environment::DD_INSTRUMENTATION_INSTALL_ID)) { + if (auto install_id = env::lookup()) { result.install_id = std::string(*install_id); } - if (auto install_type = - lookup(environment::DD_INSTRUMENTATION_INSTALL_TYPE)) { + if (auto install_type = env::lookup()) { result.install_type = std::string(*install_type); } - if (auto install_time = - lookup(environment::DD_INSTRUMENTATION_INSTALL_TIME)) { + if (auto install_time = env::lookup()) { result.install_time = std::string(*install_time); } diff --git a/src/datadog/trace_sampler_config.cpp b/src/datadog/trace_sampler_config.cpp index 698731db..0c1834ca 100644 --- a/src/datadog/trace_sampler_config.cpp +++ b/src/datadog/trace_sampler_config.cpp @@ -7,25 +7,27 @@ #include "json.hpp" #include "json_serializer.h" -#include "parse_util.h" #include "string_util.h" #include "tags.h" namespace datadog { namespace tracing { + +namespace env = environment; + namespace { Expected load_trace_sampler_env_config() { TraceSamplerConfig env_config; - if (auto rules_env = lookup(environment::DD_TRACE_SAMPLING_RULES)) { + if (auto rules_env = env::lookup()) { nlohmann::json json_rules; try { json_rules = nlohmann::json::parse(*rules_env); } catch (const nlohmann::json::parse_error &error) { std::string message; message += "Unable to parse JSON from "; - append(message, name(environment::DD_TRACE_SAMPLING_RULES)); + append(message, name(env::DD_TRACE_SAMPLING_RULES)); message += " value "; append(message, *rules_env); message += ": "; @@ -38,7 +40,7 @@ Expected load_trace_sampler_env_config() { if (type != "array") { std::string message; message += "Trace sampling rules must be an array, but "; - append(message, name(environment::DD_TRACE_SAMPLING_RULES)); + append(message, name(env::DD_TRACE_SAMPLING_RULES)); message += " has JSON type \""; message += type; message += "\": "; @@ -54,7 +56,7 @@ Expected load_trace_sampler_env_config() { if (auto *error = matcher.if_error()) { std::string prefix; prefix += "Unable to create a rule from "; - append(prefix, name(environment::DD_TRACE_SAMPLING_RULES)); + append(prefix, name(env::DD_TRACE_SAMPLING_RULES)); prefix += " value "; append(prefix, *rules_env); prefix += ": "; @@ -69,7 +71,7 @@ Expected load_trace_sampler_env_config() { if (type != "number") { std::string message; message += "Unable to parse a rule from "; - append(message, name(environment::DD_TRACE_SAMPLING_RULES)); + append(message, name(env::DD_TRACE_SAMPLING_RULES)); message += " value "; append(message, *rules_env); message += ". The \"sample_rate\" property of the rule "; @@ -96,7 +98,7 @@ Expected load_trace_sampler_env_config() { message += " in trace sampling rule "; message += json_rule.dump(); message += ". Error occurred while parsing "; - append(message, name(environment::DD_TRACE_SAMPLING_RULES)); + append(message, name(env::DD_TRACE_SAMPLING_RULES)); message += ": "; append(message, *rules_env); return Error{Error::TRACE_SAMPLING_RULES_UNKNOWN_PROPERTY, @@ -107,28 +109,14 @@ Expected load_trace_sampler_env_config() { } } - if (auto sample_rate_env = lookup(environment::DD_TRACE_SAMPLE_RATE)) { - auto maybe_sample_rate = parse_double(*sample_rate_env); - if (auto *error = maybe_sample_rate.if_error()) { - std::string prefix; - prefix += "While parsing "; - append(prefix, name(environment::DD_TRACE_SAMPLE_RATE)); - prefix += ": "; - return error->with_prefix(prefix); - } - env_config.sample_rate = *maybe_sample_rate; + const auto sample_rate_env = env::lookup(); + if (!sample_rate_env.if_error() && *sample_rate_env) { + env_config.sample_rate = **sample_rate_env; } - if (auto limit_env = lookup(environment::DD_TRACE_RATE_LIMIT)) { - auto maybe_max_per_second = parse_double(*limit_env); - if (auto *error = maybe_max_per_second.if_error()) { - std::string prefix; - prefix += "While parsing "; - append(prefix, name(environment::DD_TRACE_RATE_LIMIT)); - prefix += ": "; - return error->with_prefix(prefix); - } - env_config.max_per_second = *maybe_max_per_second; + const auto limit_env = env::lookup(); + if (!limit_env.if_error() && *limit_env) { + env_config.max_per_second = **limit_env; } return env_config; diff --git a/src/datadog/tracer_config.cpp b/src/datadog/tracer_config.cpp index a10b17a8..d4514d7a 100644 --- a/src/datadog/tracer_config.cpp +++ b/src/datadog/tracer_config.cpp @@ -19,6 +19,9 @@ namespace datadog { namespace tracing { + +namespace env = environment; + namespace { Expected> parse_propagation_styles( @@ -65,12 +68,35 @@ Expected> parse_propagation_styles( return styles; } +std::string json_quoted(StringView text) { + std::string unquoted; + assign(unquoted, text); + return nlohmann::json(std::move(unquoted)).dump(); +} + +Optional lookup_propagation_env(environment::Variable variable) { + switch (variable) { + case environment::DD_TRACE_PROPAGATION_STYLE: + return env::lookup(); + case environment::DD_TRACE_PROPAGATION_STYLE_EXTRACT: + return env::lookup(); + case environment::DD_PROPAGATION_STYLE_EXTRACT: + return env::lookup(); + case environment::DD_TRACE_PROPAGATION_STYLE_INJECT: + return env::lookup(); + case environment::DD_PROPAGATION_STYLE_INJECT: + return env::lookup(); + default: + return {}; + } +} + // Return a `std::vector` parsed from the specified `env_var`. -// If `env_var` is not in the environment, return `nullopt`. If an error occurs, -// throw an `Error`. +// If `env_var` is not in the environment, return `nullopt`. +// If parsing fails, log and ignore the environment variable. Optional> styles_from_env( - environment::Variable env_var) { - const auto styles_env = lookup(env_var); + environment::Variable env_var, Logger &logger) { + const auto styles_env = lookup_propagation_env(env_var); if (!styles_env) { return {}; } @@ -81,87 +107,79 @@ Optional> styles_from_env( prefix += "Unable to parse "; append(prefix, name(env_var)); prefix += " environment variable: "; - throw error->with_prefix(prefix); + logger.log_error(error->with_prefix(prefix)); + return {}; } return *styles; } -std::string json_quoted(StringView text) { - std::string unquoted; - assign(unquoted, text); - return nlohmann::json(std::move(unquoted)).dump(); -} - Expected load_tracer_env_config(Logger &logger) { TracerConfig env_cfg; - if (auto service_env = lookup(environment::DD_SERVICE)) { + if (auto service_env = env::lookup()) { env_cfg.service = std::string{*service_env}; } - if (auto environment_env = lookup(environment::DD_ENV)) { + if (auto environment_env = env::lookup()) { env_cfg.environment = std::string{*environment_env}; } - if (auto version_env = lookup(environment::DD_VERSION)) { + if (auto version_env = env::lookup()) { env_cfg.version = std::string{*version_env}; } - if (auto tags_env = lookup(environment::DD_TAGS)) { + if (auto tags_env = env::lookup()) { auto tags = parse_tags(*tags_env); if (auto *error = tags.if_error()) { std::string prefix; prefix += "Unable to parse "; - append(prefix, name(environment::DD_TAGS)); + append(prefix, name(env::DD_TAGS)); prefix += " environment variable: "; - return error->with_prefix(prefix); + logger.log_error(error->with_prefix(prefix)); + } else { + env_cfg.tags = std::move(*tags); } - env_cfg.tags = std::move(*tags); } - if (auto startup_env = lookup(environment::DD_TRACE_STARTUP_LOGS)) { - env_cfg.log_on_startup = !falsy(*startup_env); + if (auto startup_env = env::lookup()) { + env_cfg.log_on_startup = *startup_env; } - if (auto enabled_env = lookup(environment::DD_TRACE_ENABLED)) { - env_cfg.report_traces = !falsy(*enabled_env); + if (auto enabled_env = env::lookup()) { + env_cfg.report_traces = *enabled_env; } if (auto enabled_env = - lookup(environment::DD_TRACE_128_BIT_TRACEID_GENERATION_ENABLED)) { - env_cfg.generate_128bit_trace_ids = !falsy(*enabled_env); + env::lookup()) { + env_cfg.generate_128bit_trace_ids = *enabled_env; } - if (auto apm_enabled_env = lookup(environment::DD_APM_TRACING_ENABLED)) { - env_cfg.tracing_enabled = !falsy(*apm_enabled_env); + if (auto apm_enabled_env = env::lookup()) { + env_cfg.tracing_enabled = *apm_enabled_env; } if (auto resource_renaming_enabled_env = - lookup(environment::DD_TRACE_RESOURCE_RENAMING_ENABLED)) { - env_cfg.resource_renaming_enabled = !falsy(*resource_renaming_enabled_env); + env::lookup()) { + env_cfg.resource_renaming_enabled = *resource_renaming_enabled_env; } - if (auto resource_renaming_always_simplified_endpoint_env = lookup( - environment::DD_TRACE_RESOURCE_RENAMING_ALWAYS_SIMPLIFIED_ENDPOINT)) { + if (auto resource_renaming_always_simplified_endpoint_env = env::lookup< + env::DD_TRACE_RESOURCE_RENAMING_ALWAYS_SIMPLIFIED_ENDPOINT>()) { env_cfg.resource_renaming_always_simplified_endpoint = - !falsy(*resource_renaming_always_simplified_endpoint_env); + *resource_renaming_always_simplified_endpoint_env; } // Baggage - if (auto baggage_items_env = - lookup(environment::DD_TRACE_BAGGAGE_MAX_ITEMS)) { - auto maybe_value = parse_uint64(*baggage_items_env, 10); - if (auto *error = maybe_value.if_error()) { - return *error; - } - - env_cfg.baggage_max_items = std::move(*maybe_value); + const auto baggage_items_env = env::lookup(); + if (auto *error = baggage_items_env.if_error()) { + logger.log_error(error->with_prefix( + "Unable to parse DD_TRACE_BAGGAGE_MAX_ITEMS environment variable: ")); + } else if (*baggage_items_env) { + env_cfg.baggage_max_items = std::move(**baggage_items_env); } - if (auto baggage_bytes_env = - lookup(environment::DD_TRACE_BAGGAGE_MAX_BYTES)) { - auto maybe_value = parse_uint64(*baggage_bytes_env, 10); - if (auto *error = maybe_value.if_error()) { - return *error; - } - - env_cfg.baggage_max_bytes = std::move(*maybe_value); + const auto baggage_bytes_env = env::lookup(); + if (auto *error = baggage_bytes_env.if_error()) { + logger.log_error(error->with_prefix( + "Unable to parse DD_TRACE_BAGGAGE_MAX_BYTES environment variable: ")); + } else if (*baggage_bytes_env) { + env_cfg.baggage_max_bytes = std::move(**baggage_bytes_env); } // PropagationStyle @@ -217,11 +235,11 @@ Expected load_tracer_env_config(Logger &logger) { }; for (const auto &[var, var_override] : questionable_combinations) { - const auto value = lookup(var); + const auto value = lookup_propagation_env(var); if (!value) { continue; } - const auto value_override = lookup(var_override); + const auto value_override = lookup_propagation_env(var_override); if (!value_override) { continue; } @@ -234,31 +252,27 @@ Expected load_tracer_env_config(Logger &logger) { warn_message(var_name, *value, var_name_override, *value_override)}); } - try { - const auto global_styles = - styles_from_env(environment::DD_TRACE_PROPAGATION_STYLE); + const auto global_styles = + styles_from_env(environment::DD_TRACE_PROPAGATION_STYLE, logger); - if (auto trace_extraction_styles = - styles_from_env(environment::DD_TRACE_PROPAGATION_STYLE_EXTRACT)) { - env_cfg.extraction_styles = std::move(*trace_extraction_styles); - } else if (auto extraction_styles = - styles_from_env(environment::DD_PROPAGATION_STYLE_EXTRACT)) { - env_cfg.extraction_styles = std::move(*extraction_styles); - } else { - env_cfg.extraction_styles = global_styles; - } + if (auto trace_extraction_styles = styles_from_env( + environment::DD_TRACE_PROPAGATION_STYLE_EXTRACT, logger)) { + env_cfg.extraction_styles = std::move(*trace_extraction_styles); + } else if (auto extraction_styles = styles_from_env( + environment::DD_PROPAGATION_STYLE_EXTRACT, logger)) { + env_cfg.extraction_styles = std::move(*extraction_styles); + } else { + env_cfg.extraction_styles = global_styles; + } - if (auto trace_injection_styles = - styles_from_env(environment::DD_TRACE_PROPAGATION_STYLE_INJECT)) { - env_cfg.injection_styles = std::move(*trace_injection_styles); - } else if (auto injection_styles = - styles_from_env(environment::DD_PROPAGATION_STYLE_INJECT)) { - env_cfg.injection_styles = std::move(*injection_styles); - } else { - env_cfg.injection_styles = global_styles; - } - } catch (Error &error) { - return std::move(error); + if (auto trace_injection_styles = styles_from_env( + environment::DD_TRACE_PROPAGATION_STYLE_INJECT, logger)) { + env_cfg.injection_styles = std::move(*trace_injection_styles); + } else if (auto injection_styles = styles_from_env( + environment::DD_PROPAGATION_STYLE_INJECT, logger)) { + env_cfg.injection_styles = std::move(*injection_styles); + } else { + env_cfg.injection_styles = global_styles; } return env_cfg; diff --git a/test/test_tracer_config.cpp b/test/test_tracer_config.cpp index b6407f72..46a0c768 100644 --- a/test/test_tracer_config.cpp +++ b/test/test_tracer_config.cpp @@ -438,12 +438,16 @@ TRACER_CONFIG_TEST("TracerConfig::agent") { std::chrono::seconds(15)); } - SECTION("ill-formated environment variable is an error") { + SECTION("ill-formatted environment variable falls back to default") { const EnvGuard env_guard{"DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS", "ddog"}; auto finalized = finalize_config(config); - REQUIRE(!finalized); - REQUIRE(finalized.error().code == Error::INVALID_DOUBLE); + REQUIRE(finalized); + const auto* const agent = + std::get_if(&finalized->collector); + REQUIRE(agent); + REQUIRE(agent->remote_configuration_poll_interval == + std::chrono::seconds(5)); } } } @@ -508,11 +512,10 @@ TRACER_CONFIG_TEST("TracerConfig::agent") { "dd-agent:8080"}, {"override port with default host", nullopt, "8080", nullopt, "http", "localhost:8080"}, - // A bogus port number will cause an error in the TCPClient, not - // during configuration. For the purposes of configuration, any - // value is accepted. + // A bogus port value is ignored during configuration parsing and + // defaults are used. {"we don't parse port", nullopt, "bogus", nullopt, "http", - "localhost:bogus"}, + "localhost:8126"}, {"URL", nullopt, nullopt, "http://dd-agent:8080", "http", "dd-agent:8080"}, {"URL overrides scheme", nullopt, nullopt, "https://dd-agent:8080", @@ -646,38 +649,44 @@ TRACER_CONFIG_TEST("TracerConfig::trace_sampler") { REQUIRE(finalized->trace_sampler.rules.front().rate == 0.5); } - SECTION("has to have a valid value") { + SECTION("invalid values either fallback or fail range checks") { struct TestCase { std::string name; std::string env_value; - std::vector allowed_errors; + Optional expected_error; + bool allow_success = false; }; auto test_case = GENERATE(values({ - {"nonsense", "nonsense", {Error::INVALID_DOUBLE}}, - {"trailing space", "0.23 ", {Error::INVALID_DOUBLE}}, - {"out of range of double", "123e9999999999", {Error::INVALID_DOUBLE}}, + {"nonsense", "nonsense", nullopt}, + {"trailing space", "0.23 ", nullopt}, + {"out of range of double", "123e9999999999", nullopt}, // Some C++ standard libraries parse "nan" and "inf" as the // corresponding special floating point values. Other standard // libraries consider "nan" and "inf" invalid. - // So, either the double will fail to parse, or parsing will succeed - // but the resulting value will be outside of the inclusive range - // [0.0, 1.0] of the `Rate` type. - {"NaN", "NaN", {Error::INVALID_DOUBLE, Error::RATE_OUT_OF_RANGE}}, - {"nan", "nan", {Error::INVALID_DOUBLE, Error::RATE_OUT_OF_RANGE}}, - {"inf", "inf", {Error::INVALID_DOUBLE, Error::RATE_OUT_OF_RANGE}}, - {"Inf", "Inf", {Error::INVALID_DOUBLE, Error::RATE_OUT_OF_RANGE}}, - {"below range", "-0.1", {Error::RATE_OUT_OF_RANGE}}, - {"above range", "1.1", {Error::RATE_OUT_OF_RANGE}}, + // If parsing fails, the value is ignored. If parsing succeeds, range + // checks still apply. + {"NaN", "NaN", Error::RATE_OUT_OF_RANGE, true}, + {"nan", "nan", Error::RATE_OUT_OF_RANGE, true}, + {"inf", "inf", Error::RATE_OUT_OF_RANGE, true}, + {"Inf", "Inf", Error::RATE_OUT_OF_RANGE, true}, + {"below range", "-0.1", Error::RATE_OUT_OF_RANGE}, + {"above range", "1.1", Error::RATE_OUT_OF_RANGE}, })); CAPTURE(test_case.name); const EnvGuard guard{"DD_TRACE_SAMPLE_RATE", test_case.env_value}; auto finalized = finalize_config(config); - REQUIRE(!finalized); - REQUIRE_THAT(test_case.allowed_errors, - Catch::Matchers::VectorContains(finalized.error().code)); + if (test_case.expected_error) { + if (finalized) { + REQUIRE(test_case.allow_success); + } else { + REQUIRE(finalized.error().code == *test_case.expected_error); + } + } else { + REQUIRE(finalized); + } } } @@ -711,48 +720,44 @@ TRACER_CONFIG_TEST("TracerConfig::trace_sampler") { REQUIRE(finalized->trace_sampler.max_per_second == 120); } - SECTION("has to have a valid value") { + SECTION("invalid values either fallback or fail range checks") { struct TestCase { std::string name; std::string env_value; - std::vector allowed_errors; + Optional expected_error; + bool allow_success = false; }; auto test_case = GENERATE(values({ - {"nonsense", "nonsense", {Error::INVALID_DOUBLE}}, - {"trailing space", "23 ", {Error::INVALID_DOUBLE}}, - {"out of range of double", "123e9999999999", {Error::INVALID_DOUBLE}}, + {"nonsense", "nonsense", nullopt}, + {"trailing space", "23 ", nullopt}, + {"out of range of double", "123e9999999999", nullopt}, // Some C++ standard libraries parse "nan" and "inf" as the // corresponding special floating point values. Other standard // libraries consider "nan" and "inf" invalid. - // So, either the double will fail to parse, or parsing will succeed - // but the resulting value will be outside of the exclusive range - // (0.0, Inf) allowed. - {"NaN", - "NaN", - {Error::INVALID_DOUBLE, Error::MAX_PER_SECOND_OUT_OF_RANGE}}, - {"nan", - "nan", - {Error::INVALID_DOUBLE, Error::MAX_PER_SECOND_OUT_OF_RANGE}}, - {"inf", - "inf", - {Error::INVALID_DOUBLE, Error::MAX_PER_SECOND_OUT_OF_RANGE}}, - {"Inf", - "Inf", - {Error::INVALID_DOUBLE, Error::MAX_PER_SECOND_OUT_OF_RANGE}}, - {"below range", "-0.1", {Error::MAX_PER_SECOND_OUT_OF_RANGE}}, - {"zero (also below range)", - "0", - {Error::MAX_PER_SECOND_OUT_OF_RANGE}}, + // If parsing fails, the value is ignored. If parsing succeeds, range + // checks still apply. + {"NaN", "NaN", Error::MAX_PER_SECOND_OUT_OF_RANGE, true}, + {"nan", "nan", Error::MAX_PER_SECOND_OUT_OF_RANGE, true}, + {"inf", "inf", Error::MAX_PER_SECOND_OUT_OF_RANGE, true}, + {"Inf", "Inf", Error::MAX_PER_SECOND_OUT_OF_RANGE, true}, + {"below range", "-0.1", Error::MAX_PER_SECOND_OUT_OF_RANGE}, + {"zero (also below range)", "0", Error::MAX_PER_SECOND_OUT_OF_RANGE}, })); CAPTURE(test_case.name); const EnvGuard guard{"DD_TRACE_RATE_LIMIT", test_case.env_value}; auto finalized = finalize_config(config); - REQUIRE(!finalized); - REQUIRE_THAT(test_case.allowed_errors, - Catch::Matchers::VectorContains(finalized.error().code)); + if (test_case.expected_error) { + if (finalized) { + REQUIRE(test_case.allow_success); + } else { + REQUIRE(finalized.error().code == *test_case.expected_error); + } + } else { + REQUIRE(finalized); + } } } @@ -1180,8 +1185,11 @@ TRACER_CONFIG_TEST("TracerConfig propagation styles") { test_case.env_value}; auto finalized = finalize_config(config); if (test_case.expected_error) { - REQUIRE(!finalized); - REQUIRE(finalized.error().code == *test_case.expected_error); + REQUIRE(finalized); + const std::vector default_styles = { + PropagationStyle::DATADOG, PropagationStyle::W3C, + PropagationStyle::BAGGAGE}; + REQUIRE(finalized->injection_styles == default_styles); } else { REQUIRE(finalized); REQUIRE(finalized->injection_styles == test_case.expected_styles); @@ -1237,8 +1245,11 @@ TRACER_CONFIG_TEST("TracerConfig propagation styles") { SECTION("parsing failure") { const EnvGuard guard{"DD_PROPAGATION_STYLE_EXTRACT", "b3,,datadog"}; auto finalized = finalize_config(config); - REQUIRE(!finalized); - REQUIRE(finalized.error().code == Error::UNKNOWN_PROPAGATION_STYLE); + REQUIRE(finalized); + const std::vector default_styles = { + PropagationStyle::DATADOG, PropagationStyle::W3C, + PropagationStyle::BAGGAGE}; + REQUIRE(finalized->extraction_styles == default_styles); } } } @@ -1381,16 +1392,18 @@ TRACER_CONFIG_TEST("baggage") { } SECTION("value overriden by environment variables") { - SECTION("invalid BAGGAGE_MAX_ITEMS is reported") { + SECTION("invalid BAGGAGE_MAX_ITEMS is ignored") { EnvGuard guard{"DD_TRACE_BAGGAGE_MAX_ITEMS", "ten"}; auto finalized = finalize_config(config); - CHECK(!finalized); + REQUIRE(finalized); + CHECK(finalized->baggage_opts.max_items == 64); } - SECTION("invalid BAGGAGE_MAX_BYTES is reported") { + SECTION("invalid BAGGAGE_MAX_BYTES is ignored") { EnvGuard guard{"DD_TRACE_BAGGAGE_MAX_BYTES", "2kib"}; auto finalized = finalize_config(config); - CHECK(!finalized); + REQUIRE(finalized); + CHECK(finalized->baggage_opts.max_bytes == 8192); } EnvGuard guard{"DD_TRACE_BAGGAGE_MAX_ITEMS", "128"}; From 66a1481da623ebc2e6b4f6ee101f2abbe15dbbdb Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 19 Feb 2026 19:08:27 +0100 Subject: [PATCH 02/11] fixup! --- .gitlab-ci.yml | 18 +++++++++++++++++- .gitlab/one-pipeline.locked.yml | 3 +++ BUILD.bazel | 1 + bin/supported-configurations | 12 +++++------- 4 files changed, 26 insertions(+), 8 deletions(-) create mode 100644 .gitlab/one-pipeline.locked.yml diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 7f9babbc..094bbd49 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,5 +1,21 @@ stages: + - shared-pipeline - benchmarks - benchmarks-report -include: ".gitlab/benchmarks.yml" +validate_supported_configurations_local_file: + extends: .validate_supported_configurations_local_file + variables: + LOCAL_JSON_PATH: "metadata/supported-configurations.json" + +update_central_configurations_version_range: + extends: .update_central_configurations_version_range + variables: + LOCAL_REPO_NAME: "dd-trace-cpp" + LOCAL_JSON_PATH: "metadata/supported-configurations.json" + LANGUAGE_NAME: "cpp" + MULTIPLE_RELEASE_LINES: "false" + +include: + - local: ".gitlab/one-pipeline.locked.yml" + - local: ".gitlab/benchmarks.yml" diff --git a/.gitlab/one-pipeline.locked.yml b/.gitlab/one-pipeline.locked.yml new file mode 100644 index 00000000..50ca4f4a --- /dev/null +++ b/.gitlab/one-pipeline.locked.yml @@ -0,0 +1,3 @@ +# This file should be auto-generated soon. +include: + - remote: https://gitlab-templates.ddbuild.io/libdatadog/one-pipeline/ca/fbfa24e9dd887ed24ce65e71f2e41562c809f40cfc26489705b32406de7e096f/one-pipeline.yml diff --git a/BUILD.bazel b/BUILD.bazel index 5c5b72a5..7327369e 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -109,6 +109,7 @@ cc_library( "include/datadog/dict_reader.h", "include/datadog/dict_writer.h", "include/datadog/environment.h", + "include/datadog/environment_registry.h", "include/datadog/error.h", "include/datadog/event_scheduler.h", "include/datadog/expected.h", diff --git a/bin/supported-configurations b/bin/supported-configurations index 664883ec..d0cec8a9 100755 --- a/bin/supported-configurations +++ b/bin/supported-configurations @@ -9,13 +9,11 @@ BINARY="$SCRIPT_DIR/.supported-configurations" compiler=${CXX:-c++} -if [ ! -x "$BINARY" ] || [ "$SOURCE" -nt "$BINARY" ]; then +( # `CXX` may contain wrappers/flags (e.g. "ccache c++"). - ( - # shellcheck disable=SC2086 - set -- $compiler - "$@" -std=c++17 -O2 -I"$REPO_ROOT/src/datadog" "$SOURCE" -o "$BINARY" - ) -fi + # shellcheck disable=SC2086 + set -- $compiler + "$@" -std=c++17 -O2 -I"$REPO_ROOT/src/datadog" "$SOURCE" -o "$BINARY" +) exec "$BINARY" "$@" From e9e26176254d4df1c803d0065f4e1561da58d005 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 19 Feb 2026 21:52:15 +0100 Subject: [PATCH 03/11] fixup! trigger workflows --- .gitlab-ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 094bbd49..5bb983df 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -3,6 +3,10 @@ stages: - benchmarks - benchmarks-report +workflow: + rules: + - when: always + validate_supported_configurations_local_file: extends: .validate_supported_configurations_local_file variables: From e994b4d5f2b33923252b39d4ad70d1e906ffd8ff Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 19 Feb 2026 21:56:13 +0100 Subject: [PATCH 04/11] fixup! add cancel and do not trigger for releases --- .gitlab-ci.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 5bb983df..51efd49a 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -4,7 +4,12 @@ stages: - benchmarks-report workflow: + auto_cancel: + on_new_commit: interruptible rules: + - if: $CI_COMMIT_TAG =~ /^v?[0-9]+\.[0-9]+\.[0-9]+$/ + variables: + DANGEROUSLY_SKIP_SHARED_PIPELINE_TESTS: "true" - when: always validate_supported_configurations_local_file: From 453368bba79abf1b1136454bcb8dedf055b050b5 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 19 Feb 2026 22:43:11 +0100 Subject: [PATCH 05/11] fixup! --- .github/workflows/dev.yml | 2 +- .gitlab-ci.yml | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index a023eb21..0292d6e0 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -2,7 +2,7 @@ name: "Development" on: [pull_request, workflow_dispatch, workflow_call] jobs: - format: + format-and-verify-configurations: runs-on: ubuntu-22.04-arm container: image: datadog/docker-library:dd-trace-cpp-ci-23768e9-arm64 diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 51efd49a..780a5d60 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -12,13 +12,14 @@ workflow: DANGEROUSLY_SKIP_SHARED_PIPELINE_TESTS: "true" - when: always -validate_supported_configurations_local_file: - extends: .validate_supported_configurations_local_file +validate_supported_configurations_v2_local_file: + extends: .validate_supported_configurations_v2_local_file variables: LOCAL_JSON_PATH: "metadata/supported-configurations.json" + BACKFILLED: "false" -update_central_configurations_version_range: - extends: .update_central_configurations_version_range +update_central_configurations_version_range_v2: + extends: .update_central_configurations_version_range_v2 variables: LOCAL_REPO_NAME: "dd-trace-cpp" LOCAL_JSON_PATH: "metadata/supported-configurations.json" From 2bc6ca3a8cb4c500c929971ed1ae25173efa6c49 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 19 Feb 2026 22:51:05 +0100 Subject: [PATCH 06/11] fixup! skip regular pipeline --- .gitlab-ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 780a5d60..d15a6c27 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -3,6 +3,9 @@ stages: - benchmarks - benchmarks-report +variables: + SKIP_SHARED_PIPELINE: "true" + workflow: auto_cancel: on_new_commit: interruptible From 95189cabaf218716540760a9324639e37fc0ced6 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Fri, 20 Feb 2026 10:26:57 +0100 Subject: [PATCH 07/11] fixup! --- .github/workflows/dev.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 0292d6e0..2ae86d68 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -39,7 +39,7 @@ jobs: - runner: ubuntu-22.04 arch: x64 docker-arch: amd64 - needs: format + needs: format-and-verify-configurations runs-on: ${{ matrix.runner }} container: image: datadog/docker-library:dd-trace-cpp-ci-23768e9-${{matrix.docker-arch}} @@ -67,7 +67,7 @@ jobs: datadog-ci junit upload --service dd-trace-cpp --tags test.source.file:test/*.cpp .build/report.xml build-linux-bazel: - needs: format + needs: format-and-verify-configurations strategy: fail-fast: false matrix: @@ -90,7 +90,7 @@ jobs: run: bin/with-toolchain ${{ matrix.toolchain }} bazelisk --bazelrc=${{ matrix.bazelrc }} build dd_trace_cpp build-windows-bazel: - needs: format + needs: format-and-verify-configurations runs-on: windows-2022 defaults: run: @@ -111,7 +111,7 @@ jobs: run: bazelisk.exe --bazelrc=${{ matrix.bazelrc }} build dd_trace_cpp build-windows-cmake: - needs: format + needs: format-and-verify-configurations strategy: fail-fast: false matrix: From 3ed7d557b7278c1de659c151aba0490f1f3c4c66 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Fri, 20 Feb 2026 15:23:17 +0100 Subject: [PATCH 08/11] fixup! address comments --- bin/README.md | 10 ++++++++-- include/datadog/environment_registry.h | 9 +++++---- include/datadog/trace_sampler_config.h | 5 +++-- src/datadog/datadog_agent_config.cpp | 20 ++++++++------------ src/datadog/telemetry/configuration.cpp | 6 +----- src/datadog/trace_sampler_config.cpp | 17 ++++++++++++----- src/datadog/tracer_config.cpp | 3 ++- 7 files changed, 39 insertions(+), 31 deletions(-) diff --git a/bin/README.md b/bin/README.md index a66f23d8..f0c48c41 100644 --- a/bin/README.md +++ b/bin/README.md @@ -26,8 +26,14 @@ This directory contains scripts that are useful during development. - [generate-supported-configurations](generate-supported-configurations) regenerates [metadata/supported-configurations.json](../metadata/supported-configurations.json) - from - [include/datadog/environment_registry.h](../include/datadog/environment_registry.h). + using + [include/datadog/environment_registry.h](../include/datadog/environment_registry.h) + as the source of truth. + To add a new supported environment variable: + 1. update + [include/datadog/environment_registry.h](../include/datadog/environment_registry.h), + 2. run [generate-supported-configurations](generate-supported-configurations), + 3. run [check-environment-variables](check-environment-variables). This script (and [check-environment-variables](check-environment-variables)) compiles and executes [supported-configurations.cpp](supported-configurations.cpp) via diff --git a/include/datadog/environment_registry.h b/include/datadog/environment_registry.h index c519bb0a..54118e29 100644 --- a/include/datadog/environment_registry.h +++ b/include/datadog/environment_registry.h @@ -1,6 +1,11 @@ #pragma once // Central registry for supported environment variables. +// All configurations must be registered here. +// +// This registry is the single source of truth for: +// - env variable name allowlist (`include/datadog/environment.h`) +// - generated metadata (`metadata/supported-configurations.json`) // // Each entry has: // - NAME: environment variable symbol (e.g. DD_SERVICE) @@ -12,10 +17,6 @@ // The runtime default is resolved in C++ configuration finalization // logic. The description is emitted as the "default" field in // metadata/supported-configurations.json. -// -// This registry is the single source of truth for: -// - env variable name allowlist (`include/datadog/environment.h`) -// - generated metadata (`metadata/supported-configurations.json`) #define DD_ENVIRONMENT_VARIABLES(MACRO, DATA) \ MACRO(DATA, DD_AGENT_HOST, STRING, "localhost") \ diff --git a/include/datadog/trace_sampler_config.h b/include/datadog/trace_sampler_config.h index 685eeb82..1d4e393b 100644 --- a/include/datadog/trace_sampler_config.h +++ b/include/datadog/trace_sampler_config.h @@ -12,6 +12,7 @@ #include "config.h" #include "expected.h" +#include "logger.h" #include "optional.h" #include "rate.h" #include "sampling_mechanism.h" @@ -42,7 +43,7 @@ struct TraceSamplerConfig { class FinalizedTraceSamplerConfig { friend Expected finalize_config( - const TraceSamplerConfig& config); + const TraceSamplerConfig& config, Logger& logger); friend class FinalizedTracerConfig; FinalizedTraceSamplerConfig() = default; @@ -58,7 +59,7 @@ class FinalizedTraceSamplerConfig { }; Expected finalize_config( - const TraceSamplerConfig& config); + const TraceSamplerConfig& config, Logger& logger); } // namespace tracing } // namespace datadog diff --git a/src/datadog/datadog_agent_config.cpp b/src/datadog/datadog_agent_config.cpp index 6cf85b72..89fa472a 100644 --- a/src/datadog/datadog_agent_config.cpp +++ b/src/datadog/datadog_agent_config.cpp @@ -16,7 +16,7 @@ namespace tracing { namespace env = environment; -Expected load_datadog_agent_env_config(Logger& logger) { +DatadogAgentConfig load_datadog_agent_env_config(Logger& logger) { DatadogAgentConfig env_config; if (auto rc_enabled = env::lookup()) { @@ -60,11 +60,7 @@ Expected load_datadog_agent_env_config(Logger& logger) { Expected finalize_config( const DatadogAgentConfig& user_config, const std::shared_ptr& logger, const Clock& clock) { - Expected env_config = - load_datadog_agent_env_config(*logger); - if (auto error = env_config.if_error()) { - return *error; - } + DatadogAgentConfig env_config = load_datadog_agent_env_config(*logger); FinalizedDatadogAgentConfig result; @@ -93,7 +89,7 @@ Expected finalize_config( user_config.remote_configuration_listeners; if (auto flush_interval_milliseconds = - value_or(env_config->flush_interval_milliseconds, + value_or(env_config.flush_interval_milliseconds, user_config.flush_interval_milliseconds, 2000); flush_interval_milliseconds > 0) { result.flush_interval = @@ -105,7 +101,7 @@ Expected finalize_config( } if (auto request_timeout_milliseconds = - value_or(env_config->request_timeout_milliseconds, + value_or(env_config.request_timeout_milliseconds, user_config.request_timeout_milliseconds, 2000); request_timeout_milliseconds > 0) { result.request_timeout = @@ -117,7 +113,7 @@ Expected finalize_config( } if (auto shutdown_timeout_milliseconds = - value_or(env_config->shutdown_timeout_milliseconds, + value_or(env_config.shutdown_timeout_milliseconds, user_config.shutdown_timeout_milliseconds, 2000); shutdown_timeout_milliseconds > 0) { result.shutdown_timeout = @@ -129,7 +125,7 @@ Expected finalize_config( } if (double rc_poll_interval_seconds = - value_or(env_config->remote_configuration_poll_interval_seconds, + value_or(env_config.remote_configuration_poll_interval_seconds, user_config.remote_configuration_poll_interval_seconds, 5.0); rc_poll_interval_seconds >= 0.0) { result.remote_configuration_poll_interval = @@ -142,11 +138,11 @@ Expected finalize_config( } result.remote_configuration_enabled = - value_or(env_config->remote_configuration_enabled, + value_or(env_config.remote_configuration_enabled, user_config.remote_configuration_enabled, true); const auto [origin, url] = - pick(env_config->url, user_config.url, "http://localhost:8126"); + pick(env_config.url, user_config.url, "http://localhost:8126"); auto parsed_url = HTTPClient::URL::parse(url); if (auto* error = parsed_url.if_error()) { return std::move(*error); diff --git a/src/datadog/telemetry/configuration.cpp b/src/datadog/telemetry/configuration.cpp index 1836dba8..93c3c1d3 100644 --- a/src/datadog/telemetry/configuration.cpp +++ b/src/datadog/telemetry/configuration.cpp @@ -75,11 +75,7 @@ tracing::Expected finalize_config( } // debug - if (auto enabled_debug_env = env::lookup()) { - result.debug = *enabled_debug_env; - } else { - result.debug = false; - } + result.debug = env::lookup().value_or(false); // metrics_interval_seconds auto metrics_interval = pick(env_config->metrics_interval_seconds, diff --git a/src/datadog/trace_sampler_config.cpp b/src/datadog/trace_sampler_config.cpp index 0c1834ca..445320ab 100644 --- a/src/datadog/trace_sampler_config.cpp +++ b/src/datadog/trace_sampler_config.cpp @@ -17,7 +17,7 @@ namespace env = environment; namespace { -Expected load_trace_sampler_env_config() { +Expected load_trace_sampler_env_config(Logger &logger) { TraceSamplerConfig env_config; if (auto rules_env = env::lookup()) { @@ -110,12 +110,18 @@ Expected load_trace_sampler_env_config() { } const auto sample_rate_env = env::lookup(); - if (!sample_rate_env.if_error() && *sample_rate_env) { + if (auto *error = sample_rate_env.if_error()) { + logger.log_error( + error->with_prefix("Unable to parse DD_TRACE_SAMPLE_RATE: ")); + } else if (*sample_rate_env) { env_config.sample_rate = **sample_rate_env; } const auto limit_env = env::lookup(); - if (!limit_env.if_error() && *limit_env) { + if (auto *error = limit_env.if_error()) { + logger.log_error( + error->with_prefix("Unable to parse DD_TRACE_RATE_LIMIT: ")); + } else if (*limit_env) { env_config.max_per_second = **limit_env; } @@ -138,8 +144,9 @@ std::string to_string(const std::vector &rules) { TraceSamplerConfig::Rule::Rule(const SpanMatcher &base) : SpanMatcher(base) {} Expected finalize_config( - const TraceSamplerConfig &config) { - Expected env_config = load_trace_sampler_env_config(); + const TraceSamplerConfig &config, Logger &logger) { + Expected env_config = + load_trace_sampler_env_config(logger); if (auto error = env_config.if_error()) { return *error; } diff --git a/src/datadog/tracer_config.cpp b/src/datadog/tracer_config.cpp index d4514d7a..3d0b95fe 100644 --- a/src/datadog/tracer_config.cpp +++ b/src/datadog/tracer_config.cpp @@ -429,7 +429,8 @@ Expected finalize_config(const TracerConfig &user_config, return std::move(*error); } - if (auto trace_sampler_config = finalize_config(user_config.trace_sampler)) { + if (auto trace_sampler_config = + finalize_config(user_config.trace_sampler, *logger)) { // Merge metadata vectors for (auto &[key, values] : trace_sampler_config->metadata) { auto &dest = final_config.metadata[key]; From 08d7a0715ab95f816e724923f20c718dd642e4d7 Mon Sep 17 00:00:00 2001 From: Benjamin De Bernardi Date: Mon, 23 Feb 2026 18:05:34 +0100 Subject: [PATCH 09/11] ci: change supported-configurations CI implementation (#283) --- .gitlab-ci.yml | 31 +++++++++++++------------ .gitlab/config-registry.yml | 41 +++++++++++++++++++++++++++++++++ .gitlab/one-pipeline.locked.yml | 3 --- 3 files changed, 57 insertions(+), 18 deletions(-) create mode 100644 .gitlab/config-registry.yml delete mode 100644 .gitlab/one-pipeline.locked.yml diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index d15a6c27..5978566f 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,34 +1,35 @@ stages: - - shared-pipeline + - config-validation - benchmarks - benchmarks-report variables: SKIP_SHARED_PIPELINE: "true" -workflow: - auto_cancel: - on_new_commit: interruptible - rules: - - if: $CI_COMMIT_TAG =~ /^v?[0-9]+\.[0-9]+\.[0-9]+$/ - variables: - DANGEROUSLY_SKIP_SHARED_PIPELINE_TESTS: "true" - - when: always +include: + - local: ".gitlab/config-registry.yml" + - local: ".gitlab/benchmarks.yml" + +# Config Registry CI Jobs validate_supported_configurations_v2_local_file: + stage: config-validation + image: registry.ddbuild.io/ci/libdatadog-build/packaging:82290795 + tags: ["runner:apm-k8s-tweaked-metal"] + rules: + - when: on_success extends: .validate_supported_configurations_v2_local_file variables: LOCAL_JSON_PATH: "metadata/supported-configurations.json" - BACKFILLED: "false" + BACKFILLED: false update_central_configurations_version_range_v2: + stage: config-validation + image: registry.ddbuild.io/ci/libdatadog-build/packaging:82290795 + tags: ["runner:apm-k8s-tweaked-metal"] extends: .update_central_configurations_version_range_v2 variables: LOCAL_REPO_NAME: "dd-trace-cpp" LOCAL_JSON_PATH: "metadata/supported-configurations.json" LANGUAGE_NAME: "cpp" - MULTIPLE_RELEASE_LINES: "false" - -include: - - local: ".gitlab/one-pipeline.locked.yml" - - local: ".gitlab/benchmarks.yml" + MULTIPLE_RELEASE_LINES: "false" \ No newline at end of file diff --git a/.gitlab/config-registry.yml b/.gitlab/config-registry.yml new file mode 100644 index 00000000..1d3bf2f3 --- /dev/null +++ b/.gitlab/config-registry.yml @@ -0,0 +1,41 @@ +# This CI jobs are copied from libdatadog-build one-pipeline.yml gitlab template. +# This URL is available in the dd-gitlab/publish-content-addresable-templates job whenever +# a change is made on the one-pipeline template. +variables: + SCRIPTS_BASE_URL: https://gitlab-templates.ddbuild.io/libdatadog/one-pipeline/ca/f14ac28614630d12bcfe6cba4fd8d72dce142c62ff0b053ba7c323622104ebd7/scripts/config-inversion/ + +.download-scripts-from-template: &download-scripts-from-template + - mkdir -p scripts + - | + for script_file in "config-inversion-local-validation.py" "config-inversion-update-supported-range.py" + do + curl --location --fail --show-error --output "scripts/${script_file}" "${SCRIPTS_BASE_URL}/${script_file}" + chmod +x scripts/$script_file + done + +.validate_supported_configurations_v2_local_file: + allow_failure: false + rules: + - when: on_success + variables: + LOCAL_JSON_PATH: "" + BACKFILLED: "" + before_script: + - *download-scripts-from-template + script: + - scripts/config-inversion-local-validation.py + +.update_central_configurations_version_range_v2: + allow_failure: false + rules: + - if: '$CI_COMMIT_TAG =~ /^v?[0-9]+\.[0-9]+\.[0-9]+$/' + when: always + variables: + LOCAL_JSON_PATH: "" + LANGUAGE_NAME: "" + MULTIPLE_RELEASE_LINES: "false" # expect "true" or "false" as a value to determine if a new "branch" identifier is needed to differentiate between multiple release lines + before_script: + - *download-scripts-from-template + - export FP_API_KEY=$(aws ssm get-parameter --region us-east-1 --name ci.$CI_PROJECT_NAME.FP_API_KEY --with-decryption --query "Parameter.Value" --out text) + script: + - scripts/config-inversion-update-supported-range.py \ No newline at end of file diff --git a/.gitlab/one-pipeline.locked.yml b/.gitlab/one-pipeline.locked.yml deleted file mode 100644 index 50ca4f4a..00000000 --- a/.gitlab/one-pipeline.locked.yml +++ /dev/null @@ -1,3 +0,0 @@ -# This file should be auto-generated soon. -include: - - remote: https://gitlab-templates.ddbuild.io/libdatadog/one-pipeline/ca/fbfa24e9dd887ed24ce65e71f2e41562c809f40cfc26489705b32406de7e096f/one-pipeline.yml From 75f564e33f2a45fb3063bc925f7d27263d640114 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 25 Feb 2026 17:44:39 +0100 Subject: [PATCH 10/11] fixup! do not log, instead just bail instrumentation --- include/datadog/trace_sampler_config.h | 5 +- src/datadog/datadog_agent_config.cpp | 26 +++-- src/datadog/telemetry/configuration.cpp | 8 +- src/datadog/trace_sampler_config.cpp | 21 ++-- src/datadog/tracer_config.cpp | 65 +++++------ test/telemetry/test_configuration.cpp | 14 +++ test/test_tracer_config.cpp | 148 +++++++++++------------- 7 files changed, 150 insertions(+), 137 deletions(-) diff --git a/include/datadog/trace_sampler_config.h b/include/datadog/trace_sampler_config.h index 1d4e393b..685eeb82 100644 --- a/include/datadog/trace_sampler_config.h +++ b/include/datadog/trace_sampler_config.h @@ -12,7 +12,6 @@ #include "config.h" #include "expected.h" -#include "logger.h" #include "optional.h" #include "rate.h" #include "sampling_mechanism.h" @@ -43,7 +42,7 @@ struct TraceSamplerConfig { class FinalizedTraceSamplerConfig { friend Expected finalize_config( - const TraceSamplerConfig& config, Logger& logger); + const TraceSamplerConfig& config); friend class FinalizedTracerConfig; FinalizedTraceSamplerConfig() = default; @@ -59,7 +58,7 @@ class FinalizedTraceSamplerConfig { }; Expected finalize_config( - const TraceSamplerConfig& config, Logger& logger); + const TraceSamplerConfig& config); } // namespace tracing } // namespace datadog diff --git a/src/datadog/datadog_agent_config.cpp b/src/datadog/datadog_agent_config.cpp index 89fa472a..1dfc11d4 100644 --- a/src/datadog/datadog_agent_config.cpp +++ b/src/datadog/datadog_agent_config.cpp @@ -16,7 +16,7 @@ namespace tracing { namespace env = environment; -DatadogAgentConfig load_datadog_agent_env_config(Logger& logger) { +Expected load_datadog_agent_env_config() { DatadogAgentConfig env_config; if (auto rc_enabled = env::lookup()) { @@ -26,8 +26,8 @@ DatadogAgentConfig load_datadog_agent_env_config(Logger& logger) { auto raw_rc_poll_interval_value = env::lookup(); if (auto error = raw_rc_poll_interval_value.if_error()) { - logger.log_error(error->with_prefix( - "DatadogAgent: Remote Configuration poll interval error ")); + return error->with_prefix( + "DatadogAgent: Remote Configuration poll interval error "); } else if (*raw_rc_poll_interval_value) { env_config.remote_configuration_poll_interval_seconds = **raw_rc_poll_interval_value; @@ -37,8 +37,7 @@ DatadogAgentConfig load_datadog_agent_env_config(Logger& logger) { Optional env_port = nullopt; const auto raw_env_port = env::lookup(); if (auto* error = raw_env_port.if_error()) { - logger.log_error( - error->with_prefix("DatadogAgent: Agent port parsing error ")); + return error->with_prefix("DatadogAgent: Agent port parsing error "); } else { env_port = *raw_env_port; } @@ -60,7 +59,10 @@ DatadogAgentConfig load_datadog_agent_env_config(Logger& logger) { Expected finalize_config( const DatadogAgentConfig& user_config, const std::shared_ptr& logger, const Clock& clock) { - DatadogAgentConfig env_config = load_datadog_agent_env_config(*logger); + Expected env_config = load_datadog_agent_env_config(); + if (auto error = env_config.if_error()) { + return *error; + } FinalizedDatadogAgentConfig result; @@ -89,7 +91,7 @@ Expected finalize_config( user_config.remote_configuration_listeners; if (auto flush_interval_milliseconds = - value_or(env_config.flush_interval_milliseconds, + value_or(env_config->flush_interval_milliseconds, user_config.flush_interval_milliseconds, 2000); flush_interval_milliseconds > 0) { result.flush_interval = @@ -101,7 +103,7 @@ Expected finalize_config( } if (auto request_timeout_milliseconds = - value_or(env_config.request_timeout_milliseconds, + value_or(env_config->request_timeout_milliseconds, user_config.request_timeout_milliseconds, 2000); request_timeout_milliseconds > 0) { result.request_timeout = @@ -113,7 +115,7 @@ Expected finalize_config( } if (auto shutdown_timeout_milliseconds = - value_or(env_config.shutdown_timeout_milliseconds, + value_or(env_config->shutdown_timeout_milliseconds, user_config.shutdown_timeout_milliseconds, 2000); shutdown_timeout_milliseconds > 0) { result.shutdown_timeout = @@ -125,7 +127,7 @@ Expected finalize_config( } if (double rc_poll_interval_seconds = - value_or(env_config.remote_configuration_poll_interval_seconds, + value_or(env_config->remote_configuration_poll_interval_seconds, user_config.remote_configuration_poll_interval_seconds, 5.0); rc_poll_interval_seconds >= 0.0) { result.remote_configuration_poll_interval = @@ -138,11 +140,11 @@ Expected finalize_config( } result.remote_configuration_enabled = - value_or(env_config.remote_configuration_enabled, + value_or(env_config->remote_configuration_enabled, user_config.remote_configuration_enabled, true); const auto [origin, url] = - pick(env_config.url, user_config.url, "http://localhost:8126"); + pick(env_config->url, user_config.url, "http://localhost:8126"); auto parsed_url = HTTPClient::URL::parse(url); if (auto* error = parsed_url.if_error()) { return std::move(*error); diff --git a/src/datadog/telemetry/configuration.cpp b/src/datadog/telemetry/configuration.cpp index 93c3c1d3..c9681669 100644 --- a/src/datadog/telemetry/configuration.cpp +++ b/src/datadog/telemetry/configuration.cpp @@ -30,13 +30,17 @@ tracing::Expected load_telemetry_env_config() { auto metrics_interval_seconds = env::lookup(); - if (!metrics_interval_seconds.if_error() && *metrics_interval_seconds) { + if (auto error = metrics_interval_seconds.if_error()) { + return *error; + } else if (*metrics_interval_seconds) { env_cfg.metrics_interval_seconds = **metrics_interval_seconds; } auto heartbeat_interval_seconds = env::lookup(); - if (!heartbeat_interval_seconds.if_error() && *heartbeat_interval_seconds) { + if (auto error = heartbeat_interval_seconds.if_error()) { + return *error; + } else if (*heartbeat_interval_seconds) { env_cfg.heartbeat_interval_seconds = **heartbeat_interval_seconds; } diff --git a/src/datadog/trace_sampler_config.cpp b/src/datadog/trace_sampler_config.cpp index 445320ab..34daf07f 100644 --- a/src/datadog/trace_sampler_config.cpp +++ b/src/datadog/trace_sampler_config.cpp @@ -17,7 +17,7 @@ namespace env = environment; namespace { -Expected load_trace_sampler_env_config(Logger &logger) { +Expected load_trace_sampler_env_config() { TraceSamplerConfig env_config; if (auto rules_env = env::lookup()) { @@ -111,16 +111,22 @@ Expected load_trace_sampler_env_config(Logger &logger) { const auto sample_rate_env = env::lookup(); if (auto *error = sample_rate_env.if_error()) { - logger.log_error( - error->with_prefix("Unable to parse DD_TRACE_SAMPLE_RATE: ")); + std::string prefix; + prefix += "While parsing "; + append(prefix, name(env::DD_TRACE_SAMPLE_RATE)); + prefix += ": "; + return error->with_prefix(prefix); } else if (*sample_rate_env) { env_config.sample_rate = **sample_rate_env; } const auto limit_env = env::lookup(); if (auto *error = limit_env.if_error()) { - logger.log_error( - error->with_prefix("Unable to parse DD_TRACE_RATE_LIMIT: ")); + std::string prefix; + prefix += "While parsing "; + append(prefix, name(env::DD_TRACE_RATE_LIMIT)); + prefix += ": "; + return error->with_prefix(prefix); } else if (*limit_env) { env_config.max_per_second = **limit_env; } @@ -144,9 +150,8 @@ std::string to_string(const std::vector &rules) { TraceSamplerConfig::Rule::Rule(const SpanMatcher &base) : SpanMatcher(base) {} Expected finalize_config( - const TraceSamplerConfig &config, Logger &logger) { - Expected env_config = - load_trace_sampler_env_config(logger); + const TraceSamplerConfig &config) { + Expected env_config = load_trace_sampler_env_config(); if (auto error = env_config.if_error()) { return *error; } diff --git a/src/datadog/tracer_config.cpp b/src/datadog/tracer_config.cpp index 3d0b95fe..901c789e 100644 --- a/src/datadog/tracer_config.cpp +++ b/src/datadog/tracer_config.cpp @@ -92,10 +92,10 @@ Optional lookup_propagation_env(environment::Variable variable) { } // Return a `std::vector` parsed from the specified `env_var`. -// If `env_var` is not in the environment, return `nullopt`. -// If parsing fails, log and ignore the environment variable. +// If `env_var` is not in the environment, return `nullopt`. If an error occurs, +// throw an `Error`. Optional> styles_from_env( - environment::Variable env_var, Logger &logger) { + environment::Variable env_var) { const auto styles_env = lookup_propagation_env(env_var); if (!styles_env) { return {}; @@ -107,8 +107,7 @@ Optional> styles_from_env( prefix += "Unable to parse "; append(prefix, name(env_var)); prefix += " environment variable: "; - logger.log_error(error->with_prefix(prefix)); - return {}; + throw error->with_prefix(prefix); } return *styles; } @@ -134,10 +133,9 @@ Expected load_tracer_env_config(Logger &logger) { prefix += "Unable to parse "; append(prefix, name(env::DD_TAGS)); prefix += " environment variable: "; - logger.log_error(error->with_prefix(prefix)); - } else { - env_cfg.tags = std::move(*tags); + return error->with_prefix(prefix); } + env_cfg.tags = std::move(*tags); } if (auto startup_env = env::lookup()) { @@ -168,16 +166,14 @@ Expected load_tracer_env_config(Logger &logger) { // Baggage const auto baggage_items_env = env::lookup(); if (auto *error = baggage_items_env.if_error()) { - logger.log_error(error->with_prefix( - "Unable to parse DD_TRACE_BAGGAGE_MAX_ITEMS environment variable: ")); + return *error; } else if (*baggage_items_env) { env_cfg.baggage_max_items = std::move(**baggage_items_env); } const auto baggage_bytes_env = env::lookup(); if (auto *error = baggage_bytes_env.if_error()) { - logger.log_error(error->with_prefix( - "Unable to parse DD_TRACE_BAGGAGE_MAX_BYTES environment variable: ")); + return *error; } else if (*baggage_bytes_env) { env_cfg.baggage_max_bytes = std::move(**baggage_bytes_env); } @@ -252,27 +248,31 @@ Expected load_tracer_env_config(Logger &logger) { warn_message(var_name, *value, var_name_override, *value_override)}); } - const auto global_styles = - styles_from_env(environment::DD_TRACE_PROPAGATION_STYLE, logger); + try { + const auto global_styles = + styles_from_env(environment::DD_TRACE_PROPAGATION_STYLE); - if (auto trace_extraction_styles = styles_from_env( - environment::DD_TRACE_PROPAGATION_STYLE_EXTRACT, logger)) { - env_cfg.extraction_styles = std::move(*trace_extraction_styles); - } else if (auto extraction_styles = styles_from_env( - environment::DD_PROPAGATION_STYLE_EXTRACT, logger)) { - env_cfg.extraction_styles = std::move(*extraction_styles); - } else { - env_cfg.extraction_styles = global_styles; - } + if (auto trace_extraction_styles = + styles_from_env(environment::DD_TRACE_PROPAGATION_STYLE_EXTRACT)) { + env_cfg.extraction_styles = std::move(*trace_extraction_styles); + } else if (auto extraction_styles = + styles_from_env(environment::DD_PROPAGATION_STYLE_EXTRACT)) { + env_cfg.extraction_styles = std::move(*extraction_styles); + } else { + env_cfg.extraction_styles = global_styles; + } - if (auto trace_injection_styles = styles_from_env( - environment::DD_TRACE_PROPAGATION_STYLE_INJECT, logger)) { - env_cfg.injection_styles = std::move(*trace_injection_styles); - } else if (auto injection_styles = styles_from_env( - environment::DD_PROPAGATION_STYLE_INJECT, logger)) { - env_cfg.injection_styles = std::move(*injection_styles); - } else { - env_cfg.injection_styles = global_styles; + if (auto trace_injection_styles = + styles_from_env(environment::DD_TRACE_PROPAGATION_STYLE_INJECT)) { + env_cfg.injection_styles = std::move(*trace_injection_styles); + } else if (auto injection_styles = + styles_from_env(environment::DD_PROPAGATION_STYLE_INJECT)) { + env_cfg.injection_styles = std::move(*injection_styles); + } else { + env_cfg.injection_styles = global_styles; + } + } catch (Error &error) { + return std::move(error); } return env_cfg; @@ -429,8 +429,7 @@ Expected finalize_config(const TracerConfig &user_config, return std::move(*error); } - if (auto trace_sampler_config = - finalize_config(user_config.trace_sampler, *logger)) { + if (auto trace_sampler_config = finalize_config(user_config.trace_sampler)) { // Merge metadata vectors for (auto &[key, values] : trace_sampler_config->metadata) { auto &dest = final_config.metadata[key]; diff --git a/test/telemetry/test_configuration.cpp b/test/telemetry/test_configuration.cpp index 24373e38..aae06613 100644 --- a/test/telemetry/test_configuration.cpp +++ b/test/telemetry/test_configuration.cpp @@ -130,6 +130,13 @@ TELEMETRY_CONFIGURATION_TEST("validation") { auto final_cfg = telemetry::finalize_config(); REQUIRE(!final_cfg); } + + SECTION("environment variable parse error") { + ddtest::EnvGuard env("DD_TELEMETRY_METRICS_INTERVAL_SECONDS", "nope"); + auto final_cfg = telemetry::finalize_config(); + REQUIRE(!final_cfg); + REQUIRE(final_cfg.error().code == tracing::Error::INVALID_DOUBLE); + } } SECTION("heartbeat interval validation") { @@ -146,6 +153,13 @@ TELEMETRY_CONFIGURATION_TEST("validation") { auto final_cfg = telemetry::finalize_config(); REQUIRE(!final_cfg); } + + SECTION("environment variable parse error") { + ddtest::EnvGuard env("DD_TELEMETRY_HEARTBEAT_INTERVAL", "bogus"); + auto final_cfg = telemetry::finalize_config(); + REQUIRE(!final_cfg); + REQUIRE(final_cfg.error().code == tracing::Error::INVALID_DOUBLE); + } } } diff --git a/test/test_tracer_config.cpp b/test/test_tracer_config.cpp index 46a0c768..81c51dd4 100644 --- a/test/test_tracer_config.cpp +++ b/test/test_tracer_config.cpp @@ -438,16 +438,12 @@ TRACER_CONFIG_TEST("TracerConfig::agent") { std::chrono::seconds(15)); } - SECTION("ill-formatted environment variable falls back to default") { + SECTION("ill-formated environment variable is an error") { const EnvGuard env_guard{"DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS", "ddog"}; auto finalized = finalize_config(config); - REQUIRE(finalized); - const auto* const agent = - std::get_if(&finalized->collector); - REQUIRE(agent); - REQUIRE(agent->remote_configuration_poll_interval == - std::chrono::seconds(5)); + REQUIRE(!finalized); + REQUIRE(finalized.error().code == Error::INVALID_DOUBLE); } } } @@ -501,6 +497,7 @@ TRACER_CONFIG_TEST("TracerConfig::agent") { Optional env_host; Optional env_port; Optional env_url; + Optional expected_error = nullopt; std::string expected_scheme; std::string expected_authority; }; @@ -512,10 +509,8 @@ TRACER_CONFIG_TEST("TracerConfig::agent") { "dd-agent:8080"}, {"override port with default host", nullopt, "8080", nullopt, "http", "localhost:8080"}, - // A bogus port value is ignored during configuration parsing and - // defaults are used. - {"we don't parse port", nullopt, "bogus", nullopt, "http", - "localhost:8126"}, + {"invalid port", nullopt, "bogus", nullopt, Error::INVALID_INTEGER, + "", ""}, {"URL", nullopt, nullopt, "http://dd-agent:8080", "http", "dd-agent:8080"}, {"URL overrides scheme", nullopt, nullopt, "https://dd-agent:8080", @@ -543,12 +538,17 @@ TRACER_CONFIG_TEST("TracerConfig::agent") { } auto finalized = finalize_config(config); - REQUIRE(finalized); - const auto* const agent = - std::get_if(&finalized->collector); - REQUIRE(agent); - REQUIRE(agent->url.scheme == test_case.expected_scheme); - REQUIRE(agent->url.authority == test_case.expected_authority); + if (test_case.expected_error) { + REQUIRE(!finalized); + REQUIRE(finalized.error().code == *test_case.expected_error); + } else { + REQUIRE(finalized); + const auto* const agent = + std::get_if(&finalized->collector); + REQUIRE(agent); + REQUIRE(agent->url.scheme == test_case.expected_scheme); + REQUIRE(agent->url.authority == test_case.expected_authority); + } } } @@ -649,44 +649,38 @@ TRACER_CONFIG_TEST("TracerConfig::trace_sampler") { REQUIRE(finalized->trace_sampler.rules.front().rate == 0.5); } - SECTION("invalid values either fallback or fail range checks") { + SECTION("has to have a valid value") { struct TestCase { std::string name; std::string env_value; - Optional expected_error; - bool allow_success = false; + std::vector allowed_errors; }; auto test_case = GENERATE(values({ - {"nonsense", "nonsense", nullopt}, - {"trailing space", "0.23 ", nullopt}, - {"out of range of double", "123e9999999999", nullopt}, + {"nonsense", "nonsense", {Error::INVALID_DOUBLE}}, + {"trailing space", "0.23 ", {Error::INVALID_DOUBLE}}, + {"out of range of double", "123e9999999999", {Error::INVALID_DOUBLE}}, // Some C++ standard libraries parse "nan" and "inf" as the // corresponding special floating point values. Other standard // libraries consider "nan" and "inf" invalid. - // If parsing fails, the value is ignored. If parsing succeeds, range - // checks still apply. - {"NaN", "NaN", Error::RATE_OUT_OF_RANGE, true}, - {"nan", "nan", Error::RATE_OUT_OF_RANGE, true}, - {"inf", "inf", Error::RATE_OUT_OF_RANGE, true}, - {"Inf", "Inf", Error::RATE_OUT_OF_RANGE, true}, - {"below range", "-0.1", Error::RATE_OUT_OF_RANGE}, - {"above range", "1.1", Error::RATE_OUT_OF_RANGE}, + // So, either the double will fail to parse, or parsing will succeed + // but the resulting value will be outside of the inclusive range + // [0.0, 1.0] of the `Rate` type. + {"NaN", "NaN", {Error::INVALID_DOUBLE, Error::RATE_OUT_OF_RANGE}}, + {"nan", "nan", {Error::INVALID_DOUBLE, Error::RATE_OUT_OF_RANGE}}, + {"inf", "inf", {Error::INVALID_DOUBLE, Error::RATE_OUT_OF_RANGE}}, + {"Inf", "Inf", {Error::INVALID_DOUBLE, Error::RATE_OUT_OF_RANGE}}, + {"below range", "-0.1", {Error::RATE_OUT_OF_RANGE}}, + {"above range", "1.1", {Error::RATE_OUT_OF_RANGE}}, })); CAPTURE(test_case.name); const EnvGuard guard{"DD_TRACE_SAMPLE_RATE", test_case.env_value}; auto finalized = finalize_config(config); - if (test_case.expected_error) { - if (finalized) { - REQUIRE(test_case.allow_success); - } else { - REQUIRE(finalized.error().code == *test_case.expected_error); - } - } else { - REQUIRE(finalized); - } + REQUIRE(!finalized); + REQUIRE_THAT(test_case.allowed_errors, + Catch::Matchers::VectorContains(finalized.error().code)); } } @@ -720,44 +714,48 @@ TRACER_CONFIG_TEST("TracerConfig::trace_sampler") { REQUIRE(finalized->trace_sampler.max_per_second == 120); } - SECTION("invalid values either fallback or fail range checks") { + SECTION("has to have a valid value") { struct TestCase { std::string name; std::string env_value; - Optional expected_error; - bool allow_success = false; + std::vector allowed_errors; }; auto test_case = GENERATE(values({ - {"nonsense", "nonsense", nullopt}, - {"trailing space", "23 ", nullopt}, - {"out of range of double", "123e9999999999", nullopt}, + {"nonsense", "nonsense", {Error::INVALID_DOUBLE}}, + {"trailing space", "23 ", {Error::INVALID_DOUBLE}}, + {"out of range of double", "123e9999999999", {Error::INVALID_DOUBLE}}, // Some C++ standard libraries parse "nan" and "inf" as the // corresponding special floating point values. Other standard // libraries consider "nan" and "inf" invalid. - // If parsing fails, the value is ignored. If parsing succeeds, range - // checks still apply. - {"NaN", "NaN", Error::MAX_PER_SECOND_OUT_OF_RANGE, true}, - {"nan", "nan", Error::MAX_PER_SECOND_OUT_OF_RANGE, true}, - {"inf", "inf", Error::MAX_PER_SECOND_OUT_OF_RANGE, true}, - {"Inf", "Inf", Error::MAX_PER_SECOND_OUT_OF_RANGE, true}, - {"below range", "-0.1", Error::MAX_PER_SECOND_OUT_OF_RANGE}, - {"zero (also below range)", "0", Error::MAX_PER_SECOND_OUT_OF_RANGE}, + // So, either the double will fail to parse, or parsing will succeed + // but the resulting value will be outside of the exclusive range + // (0.0, Inf) allowed. + {"NaN", + "NaN", + {Error::INVALID_DOUBLE, Error::MAX_PER_SECOND_OUT_OF_RANGE}}, + {"nan", + "nan", + {Error::INVALID_DOUBLE, Error::MAX_PER_SECOND_OUT_OF_RANGE}}, + {"inf", + "inf", + {Error::INVALID_DOUBLE, Error::MAX_PER_SECOND_OUT_OF_RANGE}}, + {"Inf", + "Inf", + {Error::INVALID_DOUBLE, Error::MAX_PER_SECOND_OUT_OF_RANGE}}, + {"below range", "-0.1", {Error::MAX_PER_SECOND_OUT_OF_RANGE}}, + {"zero (also below range)", + "0", + {Error::MAX_PER_SECOND_OUT_OF_RANGE}}, })); CAPTURE(test_case.name); const EnvGuard guard{"DD_TRACE_RATE_LIMIT", test_case.env_value}; auto finalized = finalize_config(config); - if (test_case.expected_error) { - if (finalized) { - REQUIRE(test_case.allow_success); - } else { - REQUIRE(finalized.error().code == *test_case.expected_error); - } - } else { - REQUIRE(finalized); - } + REQUIRE(!finalized); + REQUIRE_THAT(test_case.allowed_errors, + Catch::Matchers::VectorContains(finalized.error().code)); } } @@ -1185,11 +1183,8 @@ TRACER_CONFIG_TEST("TracerConfig propagation styles") { test_case.env_value}; auto finalized = finalize_config(config); if (test_case.expected_error) { - REQUIRE(finalized); - const std::vector default_styles = { - PropagationStyle::DATADOG, PropagationStyle::W3C, - PropagationStyle::BAGGAGE}; - REQUIRE(finalized->injection_styles == default_styles); + REQUIRE(!finalized); + REQUIRE(finalized.error().code == *test_case.expected_error); } else { REQUIRE(finalized); REQUIRE(finalized->injection_styles == test_case.expected_styles); @@ -1245,11 +1240,8 @@ TRACER_CONFIG_TEST("TracerConfig propagation styles") { SECTION("parsing failure") { const EnvGuard guard{"DD_PROPAGATION_STYLE_EXTRACT", "b3,,datadog"}; auto finalized = finalize_config(config); - REQUIRE(finalized); - const std::vector default_styles = { - PropagationStyle::DATADOG, PropagationStyle::W3C, - PropagationStyle::BAGGAGE}; - REQUIRE(finalized->extraction_styles == default_styles); + REQUIRE(!finalized); + REQUIRE(finalized.error().code == Error::UNKNOWN_PROPAGATION_STYLE); } } } @@ -1392,18 +1384,16 @@ TRACER_CONFIG_TEST("baggage") { } SECTION("value overriden by environment variables") { - SECTION("invalid BAGGAGE_MAX_ITEMS is ignored") { + SECTION("invalid BAGGAGE_MAX_ITEMS is reported") { EnvGuard guard{"DD_TRACE_BAGGAGE_MAX_ITEMS", "ten"}; auto finalized = finalize_config(config); - REQUIRE(finalized); - CHECK(finalized->baggage_opts.max_items == 64); + CHECK(!finalized); } - SECTION("invalid BAGGAGE_MAX_BYTES is ignored") { + SECTION("invalid BAGGAGE_MAX_BYTES is reported") { EnvGuard guard{"DD_TRACE_BAGGAGE_MAX_BYTES", "2kib"}; auto finalized = finalize_config(config); - REQUIRE(finalized); - CHECK(finalized->baggage_opts.max_bytes == 8192); + CHECK(!finalized); } EnvGuard guard{"DD_TRACE_BAGGAGE_MAX_ITEMS", "128"}; From c232c3520bfb02fb9f401624d1cf4f7be5e1d852 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 25 Feb 2026 18:17:23 +0100 Subject: [PATCH 11/11] fixup! --- test/test_tracer_config.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/test_tracer_config.cpp b/test/test_tracer_config.cpp index 81c51dd4..ebe81d81 100644 --- a/test/test_tracer_config.cpp +++ b/test/test_tracer_config.cpp @@ -497,9 +497,9 @@ TRACER_CONFIG_TEST("TracerConfig::agent") { Optional env_host; Optional env_port; Optional env_url; - Optional expected_error = nullopt; std::string expected_scheme; std::string expected_authority; + Optional expected_error = nullopt; }; auto test_case = GENERATE(values({ @@ -509,8 +509,8 @@ TRACER_CONFIG_TEST("TracerConfig::agent") { "dd-agent:8080"}, {"override port with default host", nullopt, "8080", nullopt, "http", "localhost:8080"}, - {"invalid port", nullopt, "bogus", nullopt, Error::INVALID_INTEGER, - "", ""}, + {"invalid port", nullopt, "bogus", nullopt, "", "", + Error::INVALID_INTEGER}, {"URL", nullopt, nullopt, "http://dd-agent:8080", "http", "dd-agent:8080"}, {"URL overrides scheme", nullopt, nullopt, "https://dd-agent:8080",