fix(config visibility): fix system test related to reporting of trace sampler and span sampler by anna-git · Pull Request #272 · DataDog/dd-trace-cpp

anna-git · 2026-01-05T23:20:55Z

Description

Previous PR #268 accidentally broke the system test library_settings as we're not filling the telemetry vector with final.metadata anymore but from a separate array and this array wasn't passed by to finalize_config for span and trace sampler.

Motivation

Fix system test Test_Environment::test_library_settings

Additional Notes

Ran parametric tests locally and they all pass. (I thought they did with the previous PR as well but turned out I had a cached docker image)

https://datadoghq.atlassian.net/browse/APMAPI-1784

pr-commenter · 2026-01-05T23:21:59Z

Benchmarks

Benchmark execution time: 2026-01-21 10:14:53

Comparing candidate commit 9000fef in PR branch anna/config-visibility-fix-sample-reporting with baseline commit ef6d81e in branch main.

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

datadog-datadog-prod-us1 · 2026-01-05T23:27:31Z

🎯 Code Coverage
• Patch Coverage: 94.12%
• Overall Coverage: 87.77%

View detailed report

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: 9000fef | Docs | Datadog PR Page | Was this helpful? Give us feedback!}

codecov-commenter · 2026-01-05T23:28:11Z

Codecov Report

❌ Patch coverage is 94.11765% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.76%. Comparing base (7a55843) to head (9000fef).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/datadog/trace_sampler_config.cpp	80.00%	2 Missing ⚠️
src/datadog/datadog_agent_config.cpp	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #272      +/-   ##
==========================================
- Coverage   87.83%   87.76%   -0.07%     
==========================================
  Files          84       84              
  Lines        5664     5658       -6     
==========================================
- Hits         4975     4966       -9     
- Misses        689      692       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

dmehala

The fix works as intended. That said, it will be cleaner to populate all_sources_configs by merging the configuration sources from the trace and span samplers using the results of finalize_config, leveraging the same mechanism used for metadata. This would help keep the approach consistent and easier to maintain.

dmehala · 2026-01-06T13:54:26Z

src/datadog/tracer_config.cpp

+  if (auto trace_sampler_config =
+          finalize_config(user_config.trace_sampler, &all_sources_configs)) {


I prefer the current behaviour where the configuration source/data are returned by finalize_config, then merged into the final all_sources_configs. Instead of adding a superfluous argument to finalize_config.

good point thank you, I got worried about creating a new hash table in these types, but in the end not changing signatures is definitely cleaner, I changed it in bcc8bd6

Also added a few more asserts in commit 5dec43d

dmehala

Thanks for addressing my comments. I’ve left a few additional comments that still need to be addressed; aside from those, LGTM.

dmehala · 2026-01-07T15:26:31Z

include/datadog/span_sampler_config.h

@@ -50,6 +50,7 @@ class FinalizedSpanSamplerConfig {

  std::vector<Rule> rules;
  std::unordered_map<ConfigName, ConfigMetadata> metadata;


Since telemetry_configs already stores all versions, is this still necessary? If not, I recommend removing it and renaming telemetry_configs to metadata for clarity.

dmehala · 2026-01-09T10:40:10Z

src/datadog/span_sampler_config.cpp

  FinalizedSpanSamplerConfig result;
-
-  std::vector<SpanSamplerConfig::Rule> rules;
+  // Convert to Optional for resolve_and_record_config


These comments don’t provide long-term value and are unlikely to remain accurate over time. Can we remove it?

removed in 5e4d60b

dmehala · 2026-01-09T10:43:15Z

src/datadog/trace_sampler_config.cpp

  // appended to the end of `rules`. First, though, we have to make sure the
  // sample rate is valid.
-  if (sample_rate) {
+  if (sample_rate && (env_config->sample_rate || config.sample_rate)) {


I’m not sure I understand why this is required. Could you provide some context?

I created a variable and store5e4d60b, hopefully with the var name i't

anna-git temporarily deployed to dev January 5, 2026 23:21 — with GitHub Actions Inactive

anna-git changed the title ~~Config Visibility: fix reporting of trace sampler and span sampler~~ Config Visibility: fix system test related to reporting of trace sampler and span sampler Jan 5, 2026

anna-git temporarily deployed to dev January 5, 2026 23:24 — with GitHub Actions Inactive

anna-git force-pushed the anna/config-visibility-fix-sample-reporting branch from 047a5b5 to 28c4f6d Compare January 5, 2026 23:34

anna-git had a problem deploying to dev January 5, 2026 23:35 — with GitHub Actions Failure

anna-git temporarily deployed to dev January 5, 2026 23:35 — with GitHub Actions Inactive

anna-git temporarily deployed to dev January 5, 2026 23:38 — with GitHub Actions Inactive

anna-git temporarily deployed to dev January 5, 2026 23:44 — with GitHub Actions Inactive

anna-git marked this pull request as ready for review January 6, 2026 00:08

anna-git requested a review from a team as a code owner January 6, 2026 00:08

anna-git requested review from Anilm3 and removed request for a team January 6, 2026 00:08

dmehala changed the title ~~Config Visibility: fix system test related to reporting of trace sampler and span sampler~~ fix(config visibility): fix system test related to reporting of trace sampler and span sampler Jan 6, 2026

dmehala reviewed Jan 6, 2026

View reviewed changes

anna-git force-pushed the anna/config-visibility-fix-sample-reporting branch from 46f073e to 7e00b6a Compare January 7, 2026 10:22

anna-git temporarily deployed to dev January 7, 2026 10:23 — with GitHub Actions Inactive

anna-git temporarily deployed to dev January 7, 2026 13:44 — with GitHub Actions Inactive

anna-git temporarily deployed to dev January 7, 2026 13:47 — with GitHub Actions Inactive

anna-git requested a review from dmehala January 8, 2026 10:31

dmehala approved these changes Jan 9, 2026

View reviewed changes

anna-git temporarily deployed to dev January 12, 2026 11:18 — with GitHub Actions Inactive

anna-git temporarily deployed to dev January 12, 2026 11:21 — with GitHub Actions Inactive

anna-git temporarily deployed to dev January 16, 2026 13:23 — with GitHub Actions Inactive

anna-git temporarily deployed to dev January 16, 2026 13:26 — with GitHub Actions Inactive

anna-git added 6 commits January 21, 2026 11:11

Report trace sampler

a144119

Report span sampler values

85a5977

answer to comment, merge instead of passing by ref

d26be77

Add more asserts in unit tests

c0a0489

answer to comments

842d4e8

rename telemetry_configs to metadata and remove old metadata

9000fef

anna-git force-pushed the anna/config-visibility-fix-sample-reporting branch from 5896adc to 9000fef Compare January 21, 2026 10:11

anna-git mentioned this pull request Jan 21, 2026

[cpp] revert skip failing test for APMAPI-1784 DataDog/system-tests#6077

Merged

5 tasks

		if (auto trace_sampler_config =
		finalize_config(user_config.trace_sampler, &all_sources_configs)) {

		@@ -50,6 +50,7 @@ class FinalizedSpanSamplerConfig {

		std::vector<Rule> rules;
		std::unordered_map<ConfigName, ConfigMetadata> metadata;

Conversation

anna-git commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation

Additional Notes

Uh oh!

pr-commenter bot commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

Uh oh!

datadog-datadog-prod-us1 bot commented Jan 5, 2026 • edited by datadog-official bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dmehala left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmehala left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

anna-git commented Jan 5, 2026 •

edited

Loading

pr-commenter bot commented Jan 5, 2026 •

edited

Loading

datadog-datadog-prod-us1 bot commented Jan 5, 2026 •

edited by datadog-official bot

Loading

codecov-commenter commented Jan 5, 2026 •

edited

Loading