opentelemetry-js icon indicating copy to clipboard operation
opentelemetry-js copied to clipboard

Reuse of parentSpanId for NoopSpans

Open Flarna opened this issue 3 years ago • 3 comments

  • [x] This only affects the JavaScript OpenTelemetry library
  • [ ] This may affect other libraries, but I would like to get opinions here first

In case a span is dropped because of sampling decision the Tracer returns a NoopSpan with a new, unique SpanId as described in spec.

But if the Tracer returns a NoopSpan because of disabled instrumentation the SpanId from parent is reused (see here).

Should we create a new SpanId also in that case? Or is this anyway something not relevant in real world?

Flarna avatar Mar 18 '21 16:03 Flarna

If i'm not mistaken this is mostly to propagate the correct info across process boundary, even though the span shouldn't noop doesn't mean it isn't part of a bigger trace

vmarchaud avatar Mar 24 '21 20:03 vmarchaud

The difference of the two cases is that non sampled spans get an unique ID and backend will show that a span is missing (assuming a non parent based sampler here). The unique spanId ensures also that users can use the spanId for logging, as a key in a map,.. without the risk of having duplicates.

On the other hand a span suppressed via disable instrumentation has the same spanId as it's parent. It's a NoopSpan so it will be never exported but the case above with a user doing logging use spanId as key in a map would be broken. a propagator would inject this span so for the backend it would look like the suppressed span doesn't exist at all.

suppress is used by HTTP based exporters. My gut feeling is that such spans should either have a unique spanId as others or an invalid spanId. And we should suppress inject of invalid spanIds.

Flarna avatar Mar 24 '21 22:03 Flarna

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

github-actions[bot] avatar Aug 15 '22 06:08 github-actions[bot]