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

Requesting clarification for non-probabilistic samplers #2253

Open
iNikem opened this issue Jan 9, 2022 · 1 comment
Open

Requesting clarification for non-probabilistic samplers #2253

iNikem opened this issue Jan 9, 2022 · 1 comment
Assignees
Labels
spec:trace Related to the specification/trace directory

Comments

@iNikem
Copy link
Contributor

iNikem commented Jan 9, 2022

As was already discussed in #1844 (comment) current specification says very little about non-probabilistic samplers. In this issue I would like to talk about non-probabilistic not ParentBased samplers.

E.g. if I want to implement a healthcheck filtering using custom Sampler current spec allows me to implement it like this (in pseudocode):

if (attributes.url == '/healthcheck')
  return DROP
else
  return RECORD_AND_SAMPLE

This most probably will result in broken traces, where healthcheck span will not be exported, but its child will be and it will have parent span context pointing to non-existing span.

Should we augment the spec and require, or at least strongly recommend, that any non-probabilistic traces should be ParentBased ?

Alternatively, should we forbid setting span as a parent if that span has Sampled flag unset?

@iNikem iNikem added the spec:trace Related to the specification/trace directory label Jan 9, 2022
@arminru arminru assigned jmacd and unassigned arminru Jan 10, 2022
@jmacd
Copy link
Contributor

jmacd commented Jan 12, 2022

The problem of trace incompleteness is discussed in #2047 (which has not merged, but it seems we're using terminology defined there). Imagine instead of an AlwaysOff decision for your healthcheck Span, suppose you used a probability sampler with with probability 2**-62 leading to practically the same outcome. There are a number of recommendations of ways to avoid incomplete traces in #2047 meant to address this matter.

In the setup, you wrote:

This most probably will result in broken traces, where healthcheck span will not be exported, but its child will be and it will have parent span context pointing to non-existing span.

I don't follow why this is "most probable". It depends on the configuration of the child span. If you want no incomplete traces, and your healthcheck span has children, then you should configure them either with ParentBased (no broken traces) or with AlwaysOff (which follows the recommendation to use non-ascending probabilities from parent to child).

Should we augment the spec and require, or at least strongly recommend, that any non-probabilistic traces should be ParentBased ?

I believe you're proposing that we should recommend non-probability samplers be configured as the root sampler of a ParentBased sampler (i.e., so that DROPs only affect roots thus no broken traces). That's reasonable. There's a related issue described in #2179.

Alternatively, should we forbid setting span as a parent if that span has Sampled flag unset?

At least you can detect when the parent span is not collected because you have no span for your parent span ID.

OTel has a first-class problem with detecting incompleteness anyway. We can't detect when a parent is missing its child, and we have no recommendation for this. If you want recommendations that avoid incomplete traces, we have:

  1. Use ParentBased everywhere
  2. Make non-probability sampling decisions at the root
  3. Make consistent probability sampling decisions anywhere, also (a) use non-ascending probabilities for sub-trace completeness, (b) know your system-wide minimum probability (which ought to be a power-of-two, as discussed in Probability sampling in tracestate specification #2047).

Another direction has been proposed by @oertl, basically that we could keep track of how may spans have not recorded between the last parent that did record and the descendant that decides to record, this would lead to connected traces that are still incomplete. This could be done using a future extension to the OTel tracestate.

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

No branches or pull requests

3 participants