fix(telemetry): stop a filtered span from becoming the parent a remot… - #107
Merged
Merged
Conversation
…e service sees Filtering a span does not remove its Activity. ShouldTraceHttpRequest drops the HttpClient span for Dapr state-store, lock, secret and configuration calls in every profile below Verbose — but the activity is still created, its id is still written into the outgoing traceparent, and the receiver has no way to know that id names a span nobody will export. A Dapr sidecar with samplingRate "1" samples on its own terms and records a span whose parent document never arrives. Elastic APM resolves nesting strictly through parent.id and re-roots the orphan to the trace root; OpenObserve groups by trace id and hides it, which is why the same deployment looks healthy on one backend and littered on the other. Measured in a three-domain lab: 248 of 300 sidecar spans orphaned, and the orphan set matched the filter's method list exactly — every GetState, SaveState, TryLockAlpha1 and UnlockAlpha1, and none of the PublishEvent or Jobs calls, which are not on it. The obvious fix does not work, and neither do the two next-most-obvious ones. "If this context is not recorded, propagate its nearest recorded ancestor" fails because the flag has not been cleared yet: logging every injection showed the context arriving as Recorded, with Activity.Current = System.Net.Http.HttpRequestOut. OpenTelemetry injects the headers first and applies FilterHttpRequestMessage afterwards, so at injection time nothing distinguishes a span that will be exported from one that will not. GrpcNetClient's SuppressDownstreamInstrumentation changes nothing in either position — the HTTP activity is created regardless. And a sidecar-side samplingRate of "0" is an off switch rather than a parent-based sampler: it silenced spans whose parents were exported and sampled. What works is applying the filter's own predicate at injection time. The carrier is the HttpRequestMessage, so FilteredSpanRedirect can ask the same question the filter will ask and redirect the parent to the enclosing activity — for a Dapr state or lock call, the dapr.proto.runtime.v1.Dapr/GetState gRPC client span the framework already exports. IsDaprDiagnosticRequest moves into its own type for exactly this reason: if the two lists ever disagreed, a request would be filtered but still propagated, which is the defect itself. Both injection layers are wrapped because both run and OpenTelemetry's runs last, overwriting what .NET wrote — wrapping only the DistributedContextPropagator left the orphans in place, verified rather than assumed. Verified end to end against vnext through the local feed: sidecar spans now nest under the exported gRPC client span (Cache.Get -> dapr.proto…/GetState -> /dapr.proto…/GetState) and a full window audits at zero orphans, against 8-of-48 before. No span is created or dropped that was not before; only the parent id on the wire changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…e service sees
Filtering a span does not remove its Activity. ShouldTraceHttpRequest drops the HttpClient span for Dapr state-store, lock, secret and configuration calls in every profile below Verbose — but the activity is still created, its id is still written into the outgoing traceparent, and the receiver has no way to know that id names a span nobody will export. A Dapr sidecar with samplingRate "1" samples on its own terms and records a span whose parent document never arrives. Elastic APM resolves nesting strictly through parent.id and re-roots the orphan to the trace root; OpenObserve groups by trace id and hides it, which is why the same deployment looks healthy on one backend and littered on the other. Measured in a three-domain lab: 248 of 300 sidecar spans orphaned, and the orphan set matched the filter's method list exactly — every GetState, SaveState, TryLockAlpha1 and UnlockAlpha1, and none of the PublishEvent or Jobs calls, which are not on it.
The obvious fix does not work, and neither do the two next-most-obvious ones. "If this context is not recorded, propagate its nearest recorded ancestor" fails because the flag has not been cleared yet: logging every injection showed the context arriving as Recorded, with Activity.Current = System.Net.Http.HttpRequestOut. OpenTelemetry injects the headers first and applies FilterHttpRequestMessage afterwards, so at injection time nothing distinguishes a span that will be exported from one that will not. GrpcNetClient's SuppressDownstreamInstrumentation changes nothing in either position — the HTTP activity is created regardless. And a sidecar-side samplingRate of "0" is an off switch rather than a parent-based sampler: it silenced spans whose parents were exported and sampled.
What works is applying the filter's own predicate at injection time. The carrier is the HttpRequestMessage, so FilteredSpanRedirect can ask the same question the filter will ask and redirect the parent to the enclosing activity — for a Dapr state or lock call, the dapr.proto.runtime.v1.Dapr/GetState gRPC client span the framework already exports. IsDaprDiagnosticRequest moves into its own type for exactly this reason: if the two lists ever disagreed, a request would be filtered but still propagated, which is the defect itself.
Both injection layers are wrapped because both run and OpenTelemetry's runs last, overwriting what .NET wrote — wrapping only the DistributedContextPropagator left the orphans in place, verified rather than assumed.
Verified end to end against vnext through the local feed: sidecar spans now nest under the exported gRPC client span (Cache.Get -> dapr.proto…/GetState -> /dapr.proto…/GetState) and a full window audits at zero orphans, against 8-of-48 before. No span is created or dropped that was not before; only the parent id on the wire changes.