[dynamic control] only change the sampling rate if it has changed value#2957
Open
jackshirazi wants to merge 2 commits into
Open
[dynamic control] only change the sampling rate if it has changed value#2957jackshirazi wants to merge 2 commits into
jackshirazi wants to merge 2 commits into
Conversation
15 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
Optimizes the dynamic-control trace sampling policy application path by avoiding reapplying an equivalent sampling probability, reducing unnecessary sampler churn and log noise when policy updates repeat the same effective value.
Changes:
- Update
TraceSamplingRatePolicyImplementerto apply sampling probability changes viaDelegatingSampler.setSamplingProbability(...)and only log when an effective change occurs. - Add probability tracking to
DelegatingSamplerto atomically skip repeated equivalent probability updates. - Add a unit test to verify that repeated equivalent probability updates do not cause repeated delegate changes.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/tracesampling/TraceSamplingRatePolicyImplementer.java | Applies sampling probability updates via a helper that only logs when the delegate actually changes. |
| dynamic-control/src/main/java/io/opentelemetry/contrib/dynamic/policy/tracesampling/DelegatingSampler.java | Tracks last-applied probability and adds setSamplingProbability to skip equivalent updates. |
| dynamic-control/src/test/java/io/opentelemetry/contrib/dynamic/policy/tracesampling/TraceSamplingRatePolicyImplementerTest.java | Adds coverage ensuring repeated equivalent probability updates only apply once. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
The policy aggregator spec doesn't require already applied policy changes to be eliminated, it delegates that optimization to the implementation. I've added the optimization that if there is no effective change to the sampling rate, then no actual change is applied
Existing Issue(s):
#2868
Testing:
added
Documentation:
n/a
Outstanding items:
#2868