fix(rc): rework remote configuration handling #234
Conversation
dmehala
requested review from
Anilm3 and
zacharycmontoya
and removed request for
a team
BenchmarksBenchmark execution time: 2025-09-04 09:15:30 Comparing candidate commit 2fb3270 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics. |
zacharycmontoya
left a comment
zacharycmontoya
left a comment
There was a problem hiding this comment.
LGTM, thanks for expanding the tests
dubloom
left a comment
dubloom
left a comment
There was a problem hiding this comment.
LGTM, i'd be interested to know if all those PRs fix the segmentation fault in istio.
|
@dubloom I doubt the segmentation fault observed in Istio are related. It's worth trying tho. |
This commit refactors the way remote configurations are applied
to the tracer. Previously, if a remote configuration contained both
valid and invalid fields, the valid parts could be partially applied,
potentially leading to inconsistent behavior.
Now, the configuration is only applied if it is fully valid. Any invalid
configuration is entirely rejected. This constitutes a breaking change.
This also report errors when an invalid configuration is rejected.
Changes:
- Add fuzz test and input corpus for Remote Configuration.
- Fix a crash when telemetry methods are invoked before intializing
the module.
- Improve unit test coverage of remote configuration.
Co-authored-by: Zach Montoya <zach.montoya@datadoghq.com>
dmehala
force-pushed
the
dmehala/fix-rc-report-conf-status
branch
from
6d37940 to
2fb3270
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #234 +/- ##
==========================================
+ Coverage 86.63% 87.44% +0.81%
==========================================
Files 83 83
Lines 5425 5458 +33
==========================================
+ Hits 4700 4773 +73
+ Misses 725 685 -40 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
|
|
||
| nlohmann::json to_json(const std::vector<TraceSamplerRule>& rules) { | ||
| std::vector<nlohmann::json> j; |
There was a problem hiding this comment.
Maybe use j.reserve(rules.size()); to avoid possible reallocations?
This commit refactors the way remote configurations are applied to the tracer. Previously, if a remote configuration contained both valid and invalid fields, the valid parts could be partially applied, potentially leading to inconsistent behavior.
Now, the configuration is only applied if it is fully valid. Any invalid configuration is entirely rejected. This constitutes a breaking change.
This also report errors when an invalid configuration is rejected.
Changes: