Skip to content
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

Threshold propagation in trace state #235

Merged
merged 39 commits into from
Jan 30, 2024

Conversation

kentquirk
Copy link
Member

@kentquirk kentquirk commented Aug 9, 2023

This is intended to capture the proposal developed by the Sampling SIG.

@jmacd jmacd self-requested a review August 31, 2023 15:07
@kentquirk kentquirk marked this pull request as ready for review September 5, 2023 12:57
@kentquirk kentquirk requested review from a team September 5, 2023 12:57
@kentquirk kentquirk changed the title First draft of threshold propagation proposal Threshold propagation in trace state Sep 5, 2023
@oertl
Copy link
Contributor

oertl commented 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?

@kentquirk
Copy link
Member Author

@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.

text/trace/0235-sampling-threshold-in-trace-state.md Outdated Show resolved Hide resolved
text/trace/0235-sampling-threshold-in-trace-state.md Outdated Show resolved Hide resolved
text/trace/0235-sampling-threshold-in-trace-state.md Outdated Show resolved Hide resolved
text/trace/0235-sampling-threshold-in-trace-state.md Outdated Show resolved Hide resolved
text/trace/0235-sampling-threshold-in-trace-state.md Outdated Show resolved Hide resolved
text/trace/0235-sampling-threshold-in-trace-state.md Outdated Show resolved Hide resolved
text/trace/0235-sampling-threshold-in-trace-state.md Outdated Show resolved Hide resolved
kentquirk and others added 5 commits January 25, 2024 11:13
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>
@jmacd
Copy link
Contributor

jmacd commented Jan 30, 2024

Thank you @kentquirk!

@jmacd jmacd merged commit 4d54d97 into open-telemetry:main Jan 30, 2024
2 checks passed
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>
@codeboten codeboten deleted the threshold-propagation branch July 16, 2024 15:48
carlosalberto pushed a commit to carlosalberto/oteps that referenced this pull request Oct 23, 2024
This is intended to capture the proposal developed by [the Sampling
SIG](https://docs.google.com/document/d/1gASMhmxNt9qCa8czEMheGlUW2xpORiYoD7dBD7aNtbQ/edit#heading=h.mngoctm0c84w).

---------

Co-authored-by: Joshua MacDonald <jmacd@users.noreply.github.com>
Co-authored-by: Spencer Wilson <5624115+spencerwilson@users.noreply.github.com>
Co-authored-by: Otmar Ertl <otmar.ertl@gmail.com>
Co-authored-by: Armin Ruech <7052238+arminru@users.noreply.github.com>
carlosalberto pushed a commit to carlosalberto/oteps that referenced this pull request Oct 23, 2024
This is intended to capture the proposal developed by [the Sampling
SIG](https://docs.google.com/document/d/1gASMhmxNt9qCa8czEMheGlUW2xpORiYoD7dBD7aNtbQ/edit#heading=h.mngoctm0c84w).

---------

Co-authored-by: Joshua MacDonald <jmacd@users.noreply.github.com>
Co-authored-by: Spencer Wilson <5624115+spencerwilson@users.noreply.github.com>
Co-authored-by: Otmar Ertl <otmar.ertl@gmail.com>
Co-authored-by: Armin Ruech <7052238+arminru@users.noreply.github.com>
carlosalberto pushed a commit to carlosalberto/oteps that referenced this pull request Oct 30, 2024
This is intended to capture the proposal developed by [the Sampling
SIG](https://docs.google.com/document/d/1gASMhmxNt9qCa8czEMheGlUW2xpORiYoD7dBD7aNtbQ/edit#heading=h.mngoctm0c84w).

---------

Co-authored-by: Joshua MacDonald <jmacd@users.noreply.github.com>
Co-authored-by: Spencer Wilson <5624115+spencerwilson@users.noreply.github.com>
Co-authored-by: Otmar Ertl <otmar.ertl@gmail.com>
Co-authored-by: Armin Ruech <7052238+arminru@users.noreply.github.com>
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
This is intended to capture the proposal developed by [the Sampling
SIG](https://docs.google.com/document/d/1gASMhmxNt9qCa8czEMheGlUW2xpORiYoD7dBD7aNtbQ/edit#heading=h.mngoctm0c84w).

---------

Co-authored-by: Joshua MacDonald <jmacd@users.noreply.github.com>
Co-authored-by: Spencer Wilson <5624115+spencerwilson@users.noreply.github.com>
Co-authored-by: Otmar Ertl <otmar.ertl@gmail.com>
Co-authored-by: Armin Ruech <7052238+arminru@users.noreply.github.com>
carlosalberto pushed a commit to carlosalberto/oteps that referenced this pull request Oct 31, 2024
This is intended to capture the proposal developed by [the Sampling
SIG](https://docs.google.com/document/d/1gASMhmxNt9qCa8czEMheGlUW2xpORiYoD7dBD7aNtbQ/edit#heading=h.mngoctm0c84w).

---------

Co-authored-by: Joshua MacDonald <jmacd@users.noreply.github.com>
Co-authored-by: Spencer Wilson <5624115+spencerwilson@users.noreply.github.com>
Co-authored-by: Otmar Ertl <otmar.ertl@gmail.com>
Co-authored-by: Armin Ruech <7052238+arminru@users.noreply.github.com>
carlosalberto pushed a commit to carlosalberto/oteps that referenced this pull request Nov 1, 2024
This is intended to capture the proposal developed by [the Sampling
SIG](https://docs.google.com/document/d/1gASMhmxNt9qCa8czEMheGlUW2xpORiYoD7dBD7aNtbQ/edit#heading=h.mngoctm0c84w).

---------

Co-authored-by: Joshua MacDonald <jmacd@users.noreply.github.com>
Co-authored-by: Spencer Wilson <5624115+spencerwilson@users.noreply.github.com>
Co-authored-by: Otmar Ertl <otmar.ertl@gmail.com>
Co-authored-by: Armin Ruech <7052238+arminru@users.noreply.github.com>
carlosalberto pushed a commit to open-telemetry/opentelemetry-specification that referenced this pull request Nov 8, 2024
This is intended to capture the proposal developed by [the Sampling
SIG](https://docs.google.com/document/d/1gASMhmxNt9qCa8czEMheGlUW2xpORiYoD7dBD7aNtbQ/edit#heading=h.mngoctm0c84w).

---------

Co-authored-by: Joshua MacDonald <jmacd@users.noreply.github.com>
Co-authored-by: Spencer Wilson <5624115+spencerwilson@users.noreply.github.com>
Co-authored-by: Otmar Ertl <otmar.ertl@gmail.com>
Co-authored-by: Armin Ruech <7052238+arminru@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.