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

Implement Sampling Seed Propagation #3921

Open
bitsandfoxes opened this issue Jan 28, 2025 · 4 comments · May be fixed by #3951
Open

Implement Sampling Seed Propagation #3921

bitsandfoxes opened this issue Jan 28, 2025 · 4 comments · May be fixed by #3951
Assignees

Comments

@bitsandfoxes
Copy link
Contributor

bitsandfoxes commented Jan 28, 2025

See https://develop.sentry.dev/sdk/telemetry/traces/#propagated-random-value

The goal is to propagate the random value in the context of distributed tracing and aims to improve trace quality when using a custom tracesSampler.

Motivation

A trace is complete when all of its members are sampled. A "sub-trace" is complete when all of its descendents are sampled.

Ordinarily, Trace and Logging SDKs configure parent-based samplers which decide to sample based on the Context, because it leads to completeness. However, when non-root spans or logs make independent sampling decisions, instead of using the parent-based approach, incompleteness may result.

Consistent probability sampling requires that for any span in a given trace, if a Sampler with lesser sampling probability selects the span for sampling, then the span would also be selected by a Sampler configured with greater sampling probability.

This is achieved by propagating not only the sampling decision, but also the inputs used to make that decision, in the Dynamic Sampling Context so that downstream nodes can make consistent sampling decisions.

Example

Imagine a trace from machines A -> B -> C.

The respective sample rates are 0.5 -> 0.1 -> 0.3

On machine A (the start of the trace) a random number is generated in the range [0, 1) and assigns this to sample_rand, which is then propagated and used by all members of the trace for their sampling decision. Let's assume that number is 0.2345.
Machine A: Sets sampled = 0.2345 < 0.5 = true
Machine B: Sets sampled = 0.2345 < 0.1 = false
Machine C: Sets sampled = 0.2345 < 0.3 = true

Propagating the sample_rand and having all members use this in sampling decisions ensures that whenever a member with a lower sampling rate (like machine C in this case) sample in, members with higher sample rates (like machine A in this case) also sample in.

In this example, Machine B didn't sample in and so we don't get a complete trace. However if machine B did sample in, Machines A and C would also be guaranteed to... so we always get complete traces whenever the member with the lowest sample rate samples in.

The only other way to guarantee complete traces is by having all members sample at the same rate (e.g. 0.5 or 0.1)... which is basically what you get when the parent decides for everyone. Seed propagation doesn't guarantee complete traces - it maximises the chances of a complete trace when different members participating in a trace are all maxing their own sampling decisions (either with a custom sample rate or a custom trace sampling function).

References

@jamescrosswell
Copy link
Collaborator

jamescrosswell commented Jan 28, 2025

@bitsandfoxes I'm not sure I get it. There's already a parentSampled right (indicating whether the trace was sampled in or out by the parent)... So if there is a parent, the sampling decision gets made by the parent right? And if there isn't a parent then you'd get neither parentSampled nor sample_rand right?

So why/when does sample_rand ever get used?

@bitsandfoxes
Copy link
Contributor Author

I've updated the issue description based on the slack conversation you had.

@schmitch
Copy link

schmitch commented Feb 5, 2025

it would actually be even greater if sentry would allow to ingest data from otel directly, it would or at least provide a exporter in https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/exporter so that we can do Dotnet -> OTEL -> Sentry.

That would allow Tail Sampling.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Feb 5, 2025
@jamescrosswell
Copy link
Collaborator

it would actually be even greater if sentry would allow to ingest data from otel directly

@schmitch you're aware of the Sentry.OpenTelemetry package right? Does that integration not do what you want?

@getsantry getsantry bot removed the status in GitHub Issues with 👀 3 Feb 6, 2025
@jamescrosswell jamescrosswell linked a pull request Feb 10, 2025 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Status: No status
Development

Successfully merging a pull request may close this issue.

3 participants