Threshold propagation in trace state #235
Merged
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
PeterF778
reviewed
Aug 10, 2023
oertl
reviewed
Aug 10, 2023
oertl
reviewed
Aug 10, 2023
kentquirk
changed the title
yurishkuro
reviewed
Sep 5, 2023
yurishkuro
reviewed
Sep 5, 2023
yurishkuro
reviewed
Sep 5, 2023
yurishkuro
reviewed
Sep 5, 2023
yurishkuro
reviewed
Sep 5, 2023
PeterF778
reviewed
Sep 6, 2023
jpkrohling
reviewed
Sep 6, 2023
|
Currently, everything is very weakly worded, because the text contains many SHALLs and SHOULDs. Shouldn't we be stricter and require that any compliant consistent sampler implementation MUST follow all the rules defined here? |
oertl
reviewed
Sep 6, 2023
|
@oertl According to this RFChttps://www.[ietf.org/rfc/rfc2119.txt](https://www.ietf.org/rfc/rfc2119.txt), MUST and SHALL are equivalent. I thought I had read a document that said to prefer SHALL, but now I can't find it, and I'm finding lots of other ones that prefer MUST, so yeah, I've changed it. I've tried to address most of the comments. |
pyohannes
reviewed
Sep 6, 2023
yurishkuro
reviewed
Sep 6, 2023
kalyanaj
reviewed
Sep 6, 2023
oertl
approved these changes
Jan 18, 2024
Co-authored-by: Otmar Ertl <otmar.ertl@gmail.com>
jmacd
reviewed
Jan 18, 2024
jmacd
approved these changes
Jan 18, 2024
|
@yurishkuro will you please review and consider approving? We have reached agreement in the Sampling SIG and have begun to press for approvals. Thank you. |
|
@bogdandrutu will you please review and consider approving? We have reached agreement in the Sampling SIG and have begun to press for approvals. Thank you. |
arminru
reviewed
Jan 25, 2024
Co-authored-by: Armin Ruech <7052238+arminru@users.noreply.github.com>
Co-authored-by: Armin Ruech <7052238+arminru@users.noreply.github.com>
Co-authored-by: Armin Ruech <7052238+arminru@users.noreply.github.com>
Co-authored-by: Armin Ruech <7052238+arminru@users.noreply.github.com>
Co-authored-by: Armin Ruech <7052238+arminru@users.noreply.github.com>
arminru
reviewed
Jan 25, 2024
arminru
approved these changes
Jan 25, 2024
Co-authored-by: Armin Ruech <7052238+arminru@users.noreply.github.com>
|
Thank you @kentquirk! |
5 tasks
jpkrohling
added a commit
to open-telemetry/opentelemetry-collector-contrib
that referenced
this pull request
Jun 13, 2024
…rt OTEP 235) (#31894) **Description:** Creates new sampler modes named "equalizing" and "proportional". Preserves existing functionality under the mode named "hash_seed". Fixes #31918 This is the final step in a sequence, the whole of this work was factored into 3+ PRs, including the new `pkg/sampling` and the previous step, #31946. The two new Sampler modes enable mixing OTel sampling SDKs with Collectors in a consistent way. The existing hash_seed mode is also a consistent sampling mode, which makes it possible to have a 1:1 mapping between its decisions and the OTEP 235 randomness and threshold values. Specifically, the 14-bit hash value and sampling probability are mapped into 56-bit R-value and T-value encodings, so that all sampling decisions in all modes include threshold information. This implements the semantic conventions of open-telemetry/semantic-conventions#793, namely the `sampling.randomness` and `sampling.threshold` attributes used for logs where there is no tracestate. The default sampling mode remains HashSeed. We consider a future change of default to Proportional to be desirable, because: 1. Sampling probability is the same, only the hashing algorithm changes 2. Proportional respects and preserves information about earlier sampling decisions, which HashSeed can't do, so it has greater interoperability with OTel SDKs which may also adopt OTEP 235 samplers. **Link to tracking Issue:** Draft for open-telemetry/opentelemetry-specification#3602. Previously #24811, see also open-telemetry/oteps#235 Part of #29738 **Testing:** New testing has been added. **Documentation:** ✅ --------- Co-authored-by: Juraci Paixão Kröhling <juraci.github@kroehling.de>
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.
This is intended to capture the proposal developed by the Sampling SIG.