-
Notifications
You must be signed in to change notification settings - Fork 318
[WIP] Merge of remote sampling rules and new sampling mechanism #6991
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Changed RULE -> LOCAL_USER_RULE Added REMOTE_USER_RULE and REMOTE_ADAPTIVE_RULE
Adding mechanism to SamplingRule Relaying mechanism from rule to the span
Adding comments explaining the metainfo logic Since I found the logic less than intuitive, I wanted to make it easier on the next person
… mcculls/handle-remote-sampling-rules_MERGE
aedbe1b to
cb12ad7
Compare
| rule.getTags(), | ||
| new DeterministicSampler.TraceSampler(rule.getSampleRate())); | ||
| new DeterministicSampler.TraceSampler(rule.getSampleRate()), | ||
| samplingMechanism(rule.getProvenance())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
| final SamplingRule samplingRule = | ||
| new ServiceSamplingRule( | ||
| final RateSamplingRule samplingRule = | ||
| new RateSamplingRule.ServiceSamplingRule( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't quite decide what I wanted to do with ServiceSamplingRule and OperationSamplingRule. I thought about explicitly specifying the mechanism just to make the logic more clear, but I'll leave it to you to decide.
|
Testing complete, PRs will be merged separately |
What Does This Do
Merge of #6987 and #6989 for testing purposes
Motivation
Forthcoming changes will allow rules to be set via remote configuration.
Once rules can be either locally or remotely, we want the decision maker to reflect the source of the rule.
Additional Notes
Change makes extensive changes to the sampling rule tests - previously these tests weren't checking the decision maker tag
Jira ticket: AIT-9975