Skip to content

feat: implement stable configuration#294

Draft
bm1549 wants to merge 27 commits intomainfrom
bm1549/stable-configuration
Draft

feat: implement stable configuration#294
bm1549 wants to merge 27 commits intomainfrom
bm1549/stable-configuration

Conversation

@bm1549
Copy link
Contributor

@bm1549 bm1549 commented Mar 20, 2026

Description

Implement Phase 1 Stable Configuration support for dd-trace-cpp, as defined in the RFC.

Reads application_monitoring.yaml files from disk at tracer initialization, parses the apm_configuration_default map, and applies values with correct configuration precedence across all supported tracer configs.

Key changes

  • YAML parsing via yaml-cpp (yaml_parser.h/cpp): Clean interface — parse_yaml(string) → {config_id, values map}. Uses yaml-cpp 0.8.0 (FetchContent, static build). Handles all YAML features correctly.
  • Stable config loading (stable_config.h/cpp): Reads two files (user-managed local + fleet-managed), 256KB size limit. Platform paths for Linux and Windows (registry-based agent directory with C:\ProgramData\Datadog\ fallback).
  • Config precedence: fleet_stable > env > user/code > local_stable > default — matches all other tracer implementations. All tracer configs participate in stable config precedence, including tags, propagation styles, sampling rules, baggage, and resource renaming.
  • Telemetry: Two new ConfigMetadata::Origin values (LOCAL_STABLE_CONFIG, FLEET_STABLE_CONFIG). Fleet config_id attached at serialization time.
  • Extended resolve_and_record_config(): 5-parameter overload with fleet/local stable sources. The 3-parameter version delegates to it.
  • Parametric test server: /config endpoint dynamically exposes all stable config values.

Files

File Change
cmake/deps/yaml.cmake New: yaml-cpp 0.8.0 FetchContent dependency
src/datadog/yaml_parser.h/cpp New: YAML parser interface using yaml-cpp
src/datadog/stable_config.h/cpp New: StableConfig structs, file loading, path resolution, parse_stable_config_rules template
include/datadog/config.h New origin enum values, 5-param resolve_and_record_config
include/datadog/tracer_config.h Stable config value maps on FinalizedTracerConfig (for parametric test server)
include/datadog/telemetry/configuration.h fleet_stable_config_id on FinalizedConfiguration
include/datadog/trace_sampler_config.h StableConfigs* parameter on finalize_config
include/datadog/span_sampler_config.h StableConfigs* parameter on finalize_config
src/datadog/tracer_config.cpp StablePair helpers, all configs wired to 5-param resolve
src/datadog/trace_sampler_config.cpp Stable config for sample rate, rate limit, sampling rules
src/datadog/span_sampler_config.cpp Stable config for span sampling rules
src/datadog/telemetry/telemetry_impl.cpp Origin serialization, fleet config_id at serialization time
test/test_yaml_parser.cpp 26 YAML parser tests
test/test_stable_config.cpp 16 stable config integration tests
test/system-tests/request_handler.cpp/h Parametric server /config endpoint

Testing

  • 148 total tests (42 new), all passing
  • 26 YAML parser tests: valid/invalid input, quoting, document markers, anchors, JSON-in-YAML
  • 16 stable config tests: lookup, precedence, path resolution, finalize_config integration
  • System tests pass locally: Test_Stable_Config_Default (20/20) and Test_Stable_Configuration_Origin (all passing)
  • Related system-tests PR: feat(cpp): enable stable config telemetry tests system-tests#6554

Motivation

Enable fleet-managed and user-managed APM product configuration for C++ tracers (dd-trace-cpp, nginx-datadog, httpd-datadog). This is the last major language to get stable config support. nginx-datadog and httpd-datadog inherit the capability transparently through finalize_config().

Additional Notes

Jira ticket: [PROJ-IDENT]

🤖 Generated with Claude Code

bm1549 and others added 3 commits March 20, 2026 12:45
Read `application_monitoring.yaml` files at tracer init from user-managed
(local) and fleet-managed paths. Parse the `apm_configuration_default` map
using a minimal in-language YAML parser. Apply values with correct
precedence: fleet_stable > env > user/code > local_stable > default.

New files:
- src/datadog/stable_config.{h,cpp}: StableConfig structs, YAML parser,
  file reading, platform path resolution (Linux + Windows)

Modified:
- config.h: Add LOCAL_STABLE_CONFIG and FLEET_STABLE_CONFIG origins,
  add 5-parameter resolve_and_record_config overload
- tracer_config.h/cpp: Load stable configs in finalize_config(), store
  values on FinalizedTracerConfig for telemetry
- telemetry_impl.cpp: Serialize new origin values
- request_handler.cpp/h: Expose stable config values via /config endpoint

Tests:
- 16 new Catch2 unit tests covering precedence, lookup, path resolution
- System tests Test_Stable_Config_Default: all 16 parametric tests pass

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Add support for reporting all stable configuration values in telemetry,
including keys that dd-trace-cpp does not natively consume (e.g.
DD_LOGS_INJECTION, DD_PROFILING_ENABLED, DD_DYNAMIC_INSTRUMENTATION_ENABLED).

For each non-native key from stable config, determine the winning source
(fleet > local) and create a telemetry configuration entry with the DD_*
key name, the value, the correct origin, and (for fleet) the config_id.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- Fix stable_config_bool treating empty string as true (add empty check)
- Remove dead code loop in request_handler.cpp on_trace_config
- Use unordered_set instead of unordered_map<string,int> for key sets
- Fix misleading precedence comment for non-native key resolution

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@bm1549 bm1549 added the AI Generated Largely based on code generated by an AI or LLM. This label is the same across all dd-trace-* repos label Mar 20, 2026
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@pr-commenter
Copy link

pr-commenter bot commented Mar 20, 2026

Benchmarks

Benchmark execution time: 2026-03-21 14:54:31

Comparing candidate commit 20488e9 in PR branch bm1549/stable-configuration with baseline commit 14280af in branch main.

Found 0 performance improvements and 1 performance regressions! Performance is the same for 0 metrics, 0 unstable metrics.

Explanation

This is an A/B test comparing a candidate commit's performance against that of a baseline commit. Performance changes are noted in the tables below as:

  • 🟩 = significantly better candidate vs. baseline
  • 🟥 = significantly worse candidate vs. baseline

We compute a confidence interval (CI) over the relative difference of means between metrics from the candidate and baseline commits, considering the baseline as the reference.

If the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD), the change is considered significant.

Feel free to reach out to #apm-benchmarking-platform on Slack if you have any questions.

More details about the CI and significant changes

You can imagine this CI as a range of values that is likely to contain the true difference of means between the candidate and baseline commits.

CIs of the difference of means are often centered around 0%, because often changes are not that big:

---------------------------------(------|---^--------)-------------------------------->
                              -0.6%    0%  0.3%     +1.2%
                                 |          |        |
         lower bound of the CI --'          |        |
sample mean (center of the CI) -------------'        |
         upper bound of the CI ----------------------'

As described above, a change is considered significant if the CI is entirely outside the configured SIGNIFICANT_IMPACT_THRESHOLD (or the deprecated UNCONFIDENCE_THRESHOLD).

For instance, for an execution time metric, this confidence interval indicates a significantly worse performance:

----------------------------------------|---------|---(---------^---------)---------->
                                       0%        1%  1.3%      2.2%      3.1%
                                                  |   |         |         |
       significant impact threshold --------------'   |         |         |
                      lower bound of CI --------------'         |         |
       sample mean (center of the CI) --------------------------'         |
                      upper bound of CI ----------------------------------'

scenario:BM_TraceTinyCCSource

  • 🟥 execution_time [+3.170ms; +3.334ms] or [+4.224%; +4.442%]

- Add stable_config.cpp and stable_config.h to Bazel build
- Remove hardcoded product defaults from parametric server — C++ should
  not fake values for products it doesn't support (profiling, runtime
  metrics, data streams, logs injection, dynamic instrumentation).
  The system tests are updated to not enforce those defaults for C++.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@datadog-datadog-prod-us1-2
Copy link

datadog-datadog-prod-us1-2 bot commented Mar 20, 2026

🎯 Code Coverage (details)
Patch Coverage: 61.70%
Overall Coverage: 88.22% (-2.61%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 20488e9 | Docs | Datadog PR Page | Was this helpful? React with 👍/👎 or give us feedback!

windows.h defines ERROR as a macro which conflicts with enum values.
Rename to PARSE_ERROR and add #undef ERROR after the include.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…onfig precedence

Task 1: Separate the YAML parser from stable_config.cpp into its own
module (yaml_parser.h/cpp) with a clean interface that takes a string
and returns parsed data without depending on Logger or StableConfig.
Add comprehensive parser tests in test_yaml_parser.cpp.

Task 2: Update all remaining configs to use the 5-parameter
resolve_and_record_config with stable config sources:
- DD_TAGS (tags map type)
- DD_TRACE_PROPAGATION_STYLE_EXTRACT/INJECT and variants (propagation styles)
- DD_TRACE_BAGGAGE_MAX_ITEMS/BYTES (uint64)
- DD_TRACE_SAMPLE_RATE, DD_TRACE_RATE_LIMIT (double, via trace_sampler_config)
- DD_TRACE_RESOURCE_RENAMING_ENABLED (bool)
- DD_TRACE_RESOURCE_RENAMING_ALWAYS_SIMPLIFIED_ENDPOINT (bool)

JSON array configs (DD_TRACE_SAMPLING_RULES, DD_SPAN_SAMPLING_RULES)
are left on the old path with TODO comments since the YAML parser
skips non-scalar values.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Move kMaxYamlFileSize from yaml_parser.h to stable_config.cpp (file I/O concern)
- Add comment explaining why trim() is local in yaml_parser.cpp (handles \r\n)
- Skip YAML document markers --- and ... in parse_yaml()
- Use content.data() instead of &content[0] in stable_config.cpp
- Deduplicate resolve_and_record_config: 3-param delegates to 5-param overload
- Move config_id from ConfigMetadata to serialization time via
  FinalizedConfiguration::fleet_stable_config_id
- Extract StablePair<T> helpers to reduce repetitive fleet/local lookups
- Add TODO comments for catch(...) blocks lacking logger access
- Factor out shared JSON rule parsing into parse_stable_config_rules template
- Fix test to use parse_yaml() directly instead of manual struct population
- Use PID-based unique TempDir path for test isolation
- Fix env var check in request_handler.cpp to cover all tracked keys
- Add comment clarifying raw stable config maps are for parametric test server

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Collaborator

@dmehala dmehala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a quick look, and this PR is a hard no. There are many problems, with the most obvious being the custom YAML parser. I sincerely appreciate your effort to add feature to the tracer. Coding agents can be useful, but they need an experienced person to guide them properly. Without that, the result is just a pile of code that is hard to maintain and fix.

bm1549 and others added 8 commits March 21, 2026 08:22
unistd.h is not available on Windows. Use std::hash instead.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Replace the hand-rolled line-by-line YAML parser with the yaml-cpp 0.8.0
library. This correctly handles all YAML features (anchors, multi-line
strings, flow sequences, document markers, quoted values, inline
comments) without custom code. The parse_yaml() interface is unchanged
so no consumers need modification.

- Add yaml-cpp 0.8.0 as a CMake FetchContent dependency
- Rewrite yaml_parser.cpp to use YAML::Load and node iteration
- Remove custom trim(), unquote(), strip_inline_comment() helpers
- Update tests to match yaml-cpp behavior; add new tests for anchors,
  document markers, and quoted JSON strings
- Add TODO in BUILD.bazel for Bazel yaml-cpp dependency wiring

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add yaml-cpp to MODULE.bazel and WORKSPACE for Bazel builds
- Add @yaml-cpp dep to BUILD.bazel
- Move yaml.cmake include after compiler setup in CMakeLists.txt so
  yaml-cpp inherits -stdlib=libc++ from clang.cmake (fixes LLVM CI)

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Move stable config map loading from FinalizedTracerConfig into the
parametric test server directly via load_stable_configs(). This removes
4 fields (local/fleet values + config IDs) that were only needed for
test infrastructure from the public API surface.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@yaml-cpp refers to the repo, not the target. The correct label is
@yaml-cpp//:yaml-cpp which references the cc_library target.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Bazel's bzlmod may have issues with hyphens in module names when
resolving targets. Use repo_name = "yaml_cpp" to create a clean alias.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
yaml-cpp 0.8.0 uses cc_library without a load() statement, which was
removed in Bazel 9. The 0.8.0.bcr.1 BCR release adds the required
load("@rules_cc//cc:defs.bzl", "cc_library") patch.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
bm1549 and others added 6 commits March 21, 2026 10:15
MSVC ASAN requires all linked objects to have matching annotation flags.
yaml-cpp was compiled without ASAN while the main project had it enabled,
causing LNK2038 mismatch errors. Propagate sanitizer flags from
dd-trace-cpp-specs to yaml-cpp when DD_TRACE_ENABLE_SANITIZE is ON.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Previous commits used clang-format-22 which uses left-aligned references
(Google style). The project uses clang-format-14 via ./bin/format which
keeps right-aligned references. This reverts the spurious & formatting
changes to existing code.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…, catch specifics

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The previous approach propagated all INTERFACE_COMPILE_OPTIONS from
dd-trace-cpp-specs to yaml-cpp, which included -WX and -W4. This caused
yaml-cpp's own code (binary.cpp) to fail with C4244 warnings treated as
errors. Now we add only the -fsanitize flags directly.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@bm1549
Copy link
Contributor Author

bm1549 commented Mar 21, 2026

I took a quick look, and this PR is a hard no. There are many problems, with the most obvious being the custom YAML parser. I sincerely appreciate your effort to add feature to the tracer. Coding agents can be useful, but they need an experienced person to guide them properly. Without that, the result is just a pile of code that is hard to maintain and fix.

@dmehala I was the one that nudged it towards hand-rolled yaml 😅 justification was because it was a 600KB dependency (8% of library size). After reviewing further you're right

Made a bunch of tweaks that improve it, but still happy to close if you think this is far from solving the problem correctly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI Generated Largely based on code generated by an AI or LLM. This label is the same across all dd-trace-* repos

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants