Conversation
BenchmarksBenchmark execution time: 2026-01-21 10:14:53 Comparing candidate commit 9000fef in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics. |
|
🎯 Code Coverage 🔗 Commit SHA: 9000fef | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
Codecov Report❌ Patch coverage is
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. 🚀 New features to boost your workflow:
|
047a5b5 to
28c4f6d
Compare
dmehala
left a comment
There was a problem hiding this comment.
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.
src/datadog/tracer_config.cpp
Outdated
| if (auto trace_sampler_config = | ||
| finalize_config(user_config.trace_sampler, &all_sources_configs)) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Also added a few more asserts in commit 5dec43d
46f073e to
7e00b6a
Compare
5896adc to
9000fef
Compare
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