Adding Span Link support for compound header tag extractions with conflicting trace IDs

[mhlidd]

What does this PR do?

Adds support for span links when trace propagators extract headers with conflicting trace IDs.

• Under the above condition, the (*chainedPropagator).Extract function now appends spanLinks to the returned SpanContext
• All integrations that invoke tracer.Extract now check for spanLinks on the returned SpanContext; if spanLinks is not nil, pass tracer.WithSpanLinks as a StartSpanOption.

Motivation

Make distrubuted tracing with span links consistent across Datadog libraries according the following RFC.

APMAPI-762

Reviewer's Checklist

• Changed code has unit tests for its functionality at or near 100% coverage.
• System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
• There is a benchmark for any new code, or changes to existing code.
• If this interacts with the agent in a new way, a system test has been added.
• Add an appropriate team label so this PR gets put in the right place for the release notes.
• Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.

Unsure? Have a question? Request a review!

[pr-commenter]

Benchmarks

Benchmark execution time: 2024-12-09 12:43:26

Comparing candidate commit 08f578a in PR branch mhlidd/tracecontext_span_links with baseline commit 79a1f60 in branch main.

Found 0 performance improvements and 2 performance regressions! Performance is the same for 57 metrics, 0 unstable metrics.

scenario:BenchmarkSetTagMetric-24

• 🟥 execution_time [+4.934ns; +7.986ns] or [+4.147%; +6.713%]

scenario:BenchmarkSetTagStringer-24

• 🟥 execution_time [+3.249ns; +9.551ns] or [+2.315%; +6.805%]

[mtoffl01]

Requesting various changes, lmk if anything unclear!

[mtoffl01]

Requesting various changes, lmk if anything unclear!

[mtoffl01]

Mostly nits...
Just be sure all the existing extract tests pass, like in textmap_test.go etc.

[mhlidd]

@mtoffl01 FYI the context_headers field is supposed to hold whichever propagator type was used to extract the violating context. For instance, if the propagator type was Datadog and we have violating trace IDs, we want context_headers to have the value of datadog

[mtoffl01]

@mhlidd , make sure you update the expected "context_headers" in TestSpanLinks::Links_on_divergent_trace_IDs test!

[mhlidd]

A few small comments!

[mhlidd]

What is the reason for casting to spanContext struct? We had conversation in the past to use SpanContextW3C instead of spanContext so I'm wondering what the motivation to change it back is.

PS: Should update the comment below if the decision is to cast to spanContext

[mtoffl01]

To my memory, I think the cast to SpanContextW3C was never even used except to further cast to spanContext.
(Like for example, pW3C.propagateTracestate requires spanContext type)

I do remember that casting directly to spanContext made this code a lot easier to read.

[mhlidd]

I can't approve my own PR but LGTM! The flow is a lot clearer, thanks for helping out!

[datadog-datadog-prod-us1]

Datadog Report

Branch report: mhlidd/tracecontext_span_links
Commit report: fad2f96
Test service: dd-trace-go

✅ 0 Failed, 5100 Passed, 67 Skipped, 2m 54.86s Total Time

[darccio]

LGTM