-
Notifications
You must be signed in to change notification settings - Fork 373
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
w3c: Adds p datadog member to tracestate #3488
Conversation
3cf37f6
to
faa268d
Compare
6d0c425
to
9249d34
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3488 +/- ##
==========================================
- Coverage 98.23% 98.23% -0.01%
==========================================
Files 1275 1275
Lines 75167 75175 +8
Branches 3548 3549 +1
==========================================
+ Hits 73842 73848 +6
- Misses 1325 1327 +2 ☔ View full report in Codecov by Sentry. |
It looks like the span_id value in the traceparent:
will always be equal to the value of the new tag:
Doesn't this mean the new tagged introduced in this PR is actually redundant, given the same In other words, when can the traceparent span_id and the new tag span_id diverge? |
I think I found my answer. It looks like when both values diverge, when extracting remote distributed headers, we should use the value of the tag instead of the value from the traceparent: We should add a unit test to cover this specific behaviour in |
Ideally both values should never diverge. When we inject distributed tracing headers, traceparent and We are adding this new dd member to detect the following scenario:
In the scenario above Datadog Trace Intake services receive spans from Host A and Host C (host B uses Dynatrace). Although Host A and Host C generate spans in the same trace the relationship between both trace chunks are lost. By updating the tracestate to include the span_id we can ensure third party vendors do not "break" trace propagation for Datadog spans. The span_id in the tracestate is extracted as a distributed tracing tag and sent to the backend/agent. This will allow Datadog UI to better visualize the trace. This big trade off here is that we are adding 20 new characters to every tracestate header. tldr: trace parent will always contain |
ce07e4f
to
490f02e
Compare
@mabdinur makes sense, thank you! I think my comment is still correct and should be implemented:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, I just have a question on when exactly this would be adding the _dd.parent_id
tag, I couldn't tell for sure if it was always being added even if datadog
headers would have been used.
b308587
to
c59e423
Compare
W3C Phase 2 support
The following operations will allow us to reconstruct traces that contain non datadog spans and use w3c tracecontext headers:
_dd.parent_id
distributed tracing tagTests: DataDog/system-tests#2181
2.0 Upgrade Guide notes
What does this PR do?
Motivation:
Additional Notes:
How to test the change?
For Datadog employees:
credentials of any kind, I've requested a review from
@DataDog/security-design-and-guidance
.Unsure? Have a question? Request a review!