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

HttpClient should propagate parent context even when Filter or Sampler is in play #1561

Closed
cijothomas opened this issue Nov 17, 2020 · 1 comment · Fixed by #1707
Closed
Labels
bug Something isn't working
Milestone

Comments

@cijothomas
Copy link
Member

cijothomas commented Nov 17, 2020

  1. For HttpClient (.NET Framework) via HttpWebRequest
    As mentioned here, HttpWebRequest instrumentation must propagate Current context #1445 (comment), the instrumentation should propagate the context from parent, when the request itself is filtered out. Filtering/Sampling causes the activity to be not created/recorded, but there might be a parent context, which must be propagated downstream, otherwise downstream services thinks they are root and starts generating fresh TraceIds, leading to broken traces.

  2. For HttpClient (.NET Core)
    As this library has legacy instrumentation, there is no situation causing Activity to be null, so propagation is almost always correct. The exception is the Filter API, which is used to short-circuit the DiagnosticSourceSubscription. This means, whenever Filter is applied, no callback is fired, and hence no context is propagated. This would NOT lead to broken trace always, as HttpClient has built-in TraceContext propagation. But it does not work if propagator is custom. Also the baggage won't be propagated natively by HttpClient. This can also be an issue, but not as bad as broken trace when downstream service starts fresh TraceId.

  3. For GrpcClient - not an issue, as grpcClient relies on underlying HttpClient to handle propagation in most cases. The exception is when downsteam is suppressed, and in that case grpcClient is propagating correctly now, as there is no Filter option available in grpcClient. Once it is added (Add Filter options to gRPCClient instrumentation opentelemetry-dotnet-contrib#1781), we need to ensure it doesn't prevent context propagation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant