chore: Ensure consistent DT header insertion/replacement logic across all wrappers#3512
Merged
tippmar-nr merged 4 commits intomainfrom Apr 2, 2026
Merged
Conversation
nr-ahemsath
reviewed
Apr 2, 2026
nr-ahemsath
reviewed
Apr 2, 2026
Member
nr-ahemsath
left a comment
There was a problem hiding this comment.
Overall this looks great, I just have a minor concern about magic string values being hardcoded in both test code and exerciser code; this may be a fundamental limitation of how our tests are structured, but it least should be documented very clearly that those magic values have to match exactly in order for the tests to actually be verifying what we want them to.
Move existing-headers produce+consume before cancellation-token consumers to prevent race where cancellation consumer could grab the larger existing-headers message. Widen byte count assertion range to accommodate non-deterministic span ordering from FirstOrDefault.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3512 +/- ##
==========================================
+ Coverage 81.79% 81.81% +0.01%
==========================================
Files 508 508
Lines 34220 34228 +8
Branches 4040 4040
==========================================
+ Hits 27990 28003 +13
+ Misses 5265 5257 -8
- Partials 965 968 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
nrcventura
approved these changes
Apr 2, 2026
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
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.
Description
Validates and updates all distributed trace header insertion/replacement logic to ensure consistent behavior everywhere. Only SQS, RestSharp and the (very old) Wcf3 ChannelFactory wrapper were implemented inconsistently.
Adds unit and integration tests that ensure the replacement logic works correctly everywhere.
See the related Jira ticket and the Google doc referenced in it for a report on the entire inventory of DT header insertion across the .NET agent.
One change that's important to note is in the RestSharp
AppendHeaderswrapper - header replacement was moved to the onComplete delegate to ensure anything added by the realAppendHeaderscall would be correctly replaced if there was a collision. This was potentially a bug in the original implementation, but would have been very unlikely to occur.Some wrapper implementations could not be tested easily or did not need a test because of the way the underlying library handles headers:
by the NServiceBus pipeline, so pre-populating DT headers before the agent's hook would require a custom pipeline
behavior.
design. Pre-populating would require a custom ISendObserver or IFilter.
the WCF client programming model, so there's no natural way for pre-existing DT headers to appear. The fix (FindHeader + RemoveAt + Add) was validated by code review.
Author Checklist
Reviewer Checklist