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

Refactor the built-in Samplers for probability sampling #2179

Closed
jmacd opened this issue Nov 30, 2021 · 1 comment
Closed

Refactor the built-in Samplers for probability sampling #2179

jmacd opened this issue Nov 30, 2021 · 1 comment
Assignees
Labels
area:sampling Related to trace sampling area:sdk Related to the SDK spec:trace Related to the specification/trace directory

Comments

@jmacd
Copy link
Contributor

jmacd commented Nov 30, 2021

What are you trying to achieve?

For the probability sampling specification, a review comment in #2047 points to a potential for inconsistency stemming from the ParentBased Sampler API, which permits configuring different samplers based on the parent context's sampled flag.

The comment #2047 (comment) thread points out that ParentBased which is configured with up to 5 Samplers can be replaced by two primitives, DelegatingSampler (configured with up to 3 Samplers) and FlagSampler (with zero parameters, only used for non-roots).

What would we like to see?

As we introduce probability sampling and wish to avoid inconsistencies resulting from improper Sampler configuration, the ideal state seems to be the following set of built-in samplers:

ConsistentParentProbabilitySampler (0 parameters) that validates the OpenTelemetry tracestate fields in #2047 and otherwise does what the hypothetical FlagSampler described above would do.
ConsistentProbabilitySampler (1 parameter: probability) that implements the sampling logic in #2047 and records OpenTelemetry tracestate
DelegatingSampler (1 required parameter: root Sampler, 2 optional parameters: localParent, remoteParent Samplers): implements the former ParentBased style of delegation and enforces consistent adjusted counts when delegating for non-root spans

with two built-in aliases:

AlwaysOn (0 parameters) equivalent to ConsistentProbabilitySampler(1)
AlwaysOff (0 parameters) equivalent to ConsistentProbabilitySampler(0)

@jmacd
Copy link
Contributor Author

jmacd commented Mar 18, 2024

Closed, see open-telemetry/oteps#250

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:sampling Related to trace sampling area:sdk Related to the SDK spec:trace Related to the specification/trace directory
Projects
None yet
Development

No branches or pull requests

2 participants