Suppressing Untraced Spans

[arielvalentin]

In order to debug an issue with an upstream sampling decision, I set the OTEL_TRACES_SAMPLER=always_on and noticed that some instrumentations where generating spans even though they were wrapped in an untraced block.

Here is an example of traces generated by Net::HTTP instrumentation:

https://github.com/open-telemetry/opentelemetry-ruby/blob/main/exporter/otlp/lib/opentelemetry/exporter/otlp/exporter.rb#L131

We programmatically disable tracing in this case because it is non-trivial for us to alter auto-instrumentation. I was surprised to see that the instrumentation generated spans ignored the non-recording parent.

Is this the behavior we should expect when using the Always On sampler with the untraced block?

cc: @fbogsany our resident sampling expert

[ahayworth]

From what I can tell, the way the untraced helper works is by wrapping the block to be executed in an API span (rather than in an SDK span) - and the recording? method there always returns false. Additionally, because we are bypassing a lot of the normal span-creation machinery, I think the trace flags will be set to default (ie: not sampled).

So normally, this invisible span should not be exported, and any children won't be exported so long as the sampler takes the parent span's trace flags.

However.

The always_on sampler is a ConstantSampler, configured with a decision to always sample. It does not consider the parent span's trace flags whatsoever.

So, I think I understand both what's happening under the hood, and why you are surprised to see the spans. Intuitively, untraced should suppress anything within its block, no matter how many layers deep we go. But untraced itself is implemented in a way that the always_on sampler will never, ever care about.

We might be able to fix this somehow, perhaps with an internal flag of some kind on the span which gets propagated unilaterally downwards, indicating that we are within an untraced block, and ensuring that such spans are always recording? false and that their trace flags always indicate that they are not sampled. It probably wouldn't be an incredibly invasive change, either ... but I'm not sure if we truly want that.

Have you tried parent_based_always_on? In my tests, that seemed to do the right thing - I didn't see the internal net_http exporter spans when I used that sampler.

[arielvalentin]

what I think that untraced means is forcing the sampling decision to always_off and disabling context propagation in instrumentations for the duration of the block.

Assuming instrumentations do not propagate context, then any downstream dependencies can use their own sampling algorithm without needing to take into consideration the inbound request.

[ahayworth]

Yeah, I don't know honestly. I will defer to the wisdom of the group.

[fbogsany]

Sorry, I didn't see this earlier. Our implementation of untraced will only work with the ParentBased sampler. This problem impacts Shopify as well when we don't wrap our samplers properly.

untraced works entirely with the public API, with the assumption of a ParentBased sampler - i.e. it isn't part of the public API and can be trivially implemented on top of the public API. Other language SIGs (e.g. JS) adopted a different approach using (IIRC) the Context API. This requires an extended API that sets a Context key internally that start_span knows about. This allows new Samplers to be developed without knowledge of the "untracing" mechanism, at the cost of a non-standard API extension.

I don't have links handy for the JS implementation. I'm a little uncomfortable with requiring an API extension for this - I'd much prefer it if we could get this added to the spec (I'm sure I've seen issues floating around the spec repo for this in the past). In the meantime, I plan to fix our bug at Shopify by wrapping our VerbositySampler in a ParentBased sampler.

[tsloughter]

@fbogsany I'm trying to find the solution JS uses, can you share where the code is for it?

[fbogsany]

https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-core/src/trace/suppress-tracing.ts