ddtrace/tracer: have RuleSampler use User{Keep,Reject} instead of new values #1030
Conversation
This file contains 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
… library rules to use USER_ACCEPT and USER_REJECT
j-rufino
changed the title
gbbr
changed the title
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.
In this previous PR we changed priority to 3,-3 for traces sampled/not based on sampling rate configuration, to solve two problems:
Problem 1: The agent will always reject spans/traces with negative priority, but it might decide to keep some with priority 0. This behaviour is not intuitive for users that intend to reject a specific % of spans/traces.
Problem 2: In the backend it's not possible to differentiate and produce stats on which spans/traces were sampled based on one mechanism vs the other.
However we discovered that PHP, C++ and .Net are doing a hard validation on the priority (accept only -1,0,1,2) when they receive it via http headers and anything else is considered as if priority is not set. See here, here.
Based on that, we can not modify the sampling priority values in a backwards compatible way and the only way to obtain the desired behaviour to solve Problem 1 with the existing values is using (-1,2).
For Problem 2, we will not be able to encode the specific "reason" or mechanism used by the user to make the sampling decision via samplingPriority, and we will need to introduce (together with the trace format changes we are making) a trace-level tag to track that reason separately.
Updated documentation here: https://docs.google.com/document/d/1lVhgeG6uS0C9DVmN3t74tBDL7h44duwynIQqExljxRc/edit#