feat: support Remote Config sampling rules#116
Conversation
pablomartinezbernardo
left a comment
There was a problem hiding this comment.
One small comment, otherwise LGTM
| if (auto error = maybe_rules.if_error()) { | ||
| trace_sampling_rules_metadata.error = std::move(*error); | ||
| } else { | ||
| rules.merge(*maybe_rules); |
There was a problem hiding this comment.
I'm looking at the docs for this and if I'm understanding correctly, items already in rules will not be overriden by those in maybe_rules. Is that the intended behavior?
bdf01a0 to
f26dd83
Compare
BenchmarksBenchmark execution time: 2024-05-23 15:29:15 Comparing candidate commit 3df7c9f in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics. |
According to the Remote Configuration specification, `DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS` support floating point input and can be set to `0`.
Now, sampling remote configuration do not create a new trace sampler which has the side effect to reset the rate limiter.
- report remote trace sample rate as RULE instead of REMOTE_RULE for legacy reasons - update REMOTE_RULES and REMOTE_ADAPTIVE_RULE values to match the spec - report default sample rate for telemetry - add _dd.psr for new remote rules
05c0459 to
23bb27c
Compare
|
Since your review I added 3 commits: I fixed the rule override issue you mentioned but also remote configuration system tests revealed part of the implementation I overlooked. |
| auto j = r.to_json(); | ||
| j["sample_rate"] = r.sample_rate; | ||
| if (r.max_per_second) { | ||
| j["max_per_second"] = *r.max_per_second; | ||
| } | ||
| res.emplace_back(std::move(j)); |
There was a problem hiding this comment.
Curious to know, as they are being set in to_json, why is it necessary to set them again?
There was a problem hiding this comment.
to_json is not overloaded by SpanSamplerConfig::Rule, meaning SpanMatcher::to_json is called instead.
This pull request introduces support for remote configuration sampling rules. With this enhancement, users can dynamically adjust sampling rates and rules without redeploying or restarting their applications, providing flexibility and control over ingestion cost.