opentelemetry-dotnet
opentelemetry-dotnet copied to clipboard
TraceContextPropagator uses ActivityContext in input PropagationContext as override instead of fallback
Affected code: https://github.com/open-telemetry/opentelemetry-dotnet/blob/aad309b9445e19c437f05df150af92227e1b9eb3/src/OpenTelemetry.Api/Context/Propagation/TraceContextPropagator.cs#L53-L59
This will not extract a Context from the carrier if there already is one in the input context.
This is against the base class documentation which says:
https://github.com/open-telemetry/opentelemetry-dotnet/blob/aad309b9445e19c437f05df150af92227e1b9eb3/src/OpenTelemetry.Api/Context/Propagation/TextMapPropagator.cs#L45-L53
The default context to be used if Extract fails.
The base class docs conform to the OTel spec (or at least my understanding of it and the Java implementation), the TraceContextPropagator implementation does not. https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/context/api-propagators.md#extract
@Oberon00 - This appears to be by design (added in #1048) since ASP.NET and ASP.NET Core automatically extract the W3C trace context when handling incoming requests, so there's no need to parse the headers a second time, adding a small amount of overhead. It, therefore, handles cases when an ActivityContext hasn't already been parsed (i.e. when ASP.NET (Core) is not involved). This seems reasonable, and I don't think the spec prohibits this. @martinjt, if @CodeBlanch agrees, I think we can close this.
This issue was marked stale due to lack of activity and will be closed in 7 days. Commenting will instruct the bot to automatically remove the label. This bot runs once per day.
Closed as inactive. Feel free to reopen if this issue is still a concern.