w3c: Adds p datadog member to tracestate

[mabdinur]

W3C Phase 2 support

The following operations will allow us to reconstruct traces that contain non datadog spans and use w3c tracecontext headers:

• Injects last seen datadog span id in tracestate headers
• Extracts last seen datadog span id and uses this value to set the _dd.parent_id distributed tracing tag

Tests: 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:

• If this PR touches code that signs or publishes builds or packages, or handles
  credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
• This PR doesn't touch any of that.

Unsure? Have a question? Request a review!

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.23%. Comparing base (81e6060) to head (c9530f6).
Report is 51 commits behind head on master.

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.
📢 Have feedback on the report? Share it here.

[marcotc]

It looks like the span_id value in the traceparent:

dd-trace-rb/lib/datadog/tracing/distributed/trace_context.rb

Line 108 in c9530f6

digest.span_id,

will always be equal to the value of the new tag:

dd-trace-rb/lib/datadog/tracing/distributed/trace_context.rb

Line 150 in c9530f6

tracestate << "p:#{format('%016x', digest.span_id)};"

Doesn't this mean the new tagged introduced in this PR is actually redundant, given the same span_id is already captured in the main traceparent?

In other words, when can the traceparent span_id and the new tag span_id diverge?

[marcotc]

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:
https://github.com/DataDog/system-tests/blob/f2c546afe09dce0ad7319c0b6108908c378638da/tests/parametric/test_headers_tracecontext.py#L780-L781

We should add a unit test to cover this specific behaviour in spec/datadog/tracing/distributed/trace_context_spec.rb.

[mabdinur]

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: https://github.com/DataDog/system-tests/blob/f2c546afe09dce0ad7319c0b6108908c378638da/tests/parametric/test_headers_tracecontext.py#L780-L781

We should add a unit test to cover this specific behaviour in spec/datadog/tracing/distributed/trace_context_spec.rb.

Ideally both values should never diverge. When we inject distributed tracing headers, traceparent and x-datadog headers SHOULD always be consistent (if they are not then this is a bug we need to fix).

We are adding this new dd member to detect the following scenario:

• Host A: Injects tracecontext headers via ddtrace -> Host B: Injects/Extracts tracecontext headers via another vendor (ex: Dynatrace) -> Host C: Extracts tracecontext headers via ddtrace

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 00-current_trace_id-last_span_id-sampling. We will use the tracestate to track the last datadog span id ( last_span_id does not always equal to last_datadog_span_id)

[marcotc]

@mabdinur makes sense, thank you!

I think my comment is still correct and should be implemented:

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: DataDog/system-tests@f2c546a/tests/parametric/test_headers_tracecontext.py#L780-L781

[bouwkast]

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.