Supressed spans get injected

[Flarna]

What version of OpenTelemetry are you using?

0.18.2

What version of Node are you using?

14.16.0

Please provide the code you used to setup the OpenTelemetry SDK

const tp = new NodeTracerProvider();
tp.addSpanProcessor(new SimpleSpanProcessor(new ZipkinExporter({ serviceName: "Foo" })));
tp.register();

What did you do?

Create spans and watch HTTP messages sent by exporter

What did you expect to see?

HTTP messages without traceparent HTTP header

What did you see instead?

HTTP messages contain traceparent HTTP header even the span for the HTTP operation is suppressed.

Additional context

Add any other context about the problem here.

[dyladan]

To be clear, this is saying the trace context is injected but not that they're actually traced and exported (infinite loop) correct?

[Flarna]

The traceparent, tracestate headers are added to the HTTP requests exporting data to zipkin.
The spans itself are suppressed as expected. traceId/spanId from the place where export is called are used.

[dyladan]

I suggest we update the propagators to check the context for isSuppressed

[Flarna]

Another option would be to adapt suppressInstrumentation() to use ROOT_CONTEXT instead context.active() as base.
But well, this would likely break unsuppressInstrumentation()... or at least modify it's behavior.

[dyladan]

so a suppressed instrumentation would always be empty?

I think this could be confusing. If suppressing a context returns an empty context, nothing stops the user from adding new properties to it later on. Having some properties missing and others not would be hard to debug for users who didn't know what was going on.

[Flarna]

Yep, agree that your proposal fits better.

[dyladan]

More work and more places to mess up unfortunately, but it's more straightforward