Skip to content

Commit ebc88ae

Browse files
fix(propagation): check for span_id and trace_id before creating span link [backport 2.3] (#7724)
Backport 7954661 from #7708 to 2.3. Fixes: #7701 ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. - [x] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) - [x] 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`. - [x] This PR doesn't touch any of that. Co-authored-by: Zachary Groves <[email protected]>
1 parent 39e71ee commit ebc88ae

File tree

3 files changed

+59
-7
lines changed

3 files changed

+59
-7
lines changed

ddtrace/propagation/http.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -839,7 +839,8 @@ def _resolve_contexts(contexts, styles_w_ctx, normalized_headers):
839839
links = []
840840
for context in contexts[1:]:
841841
style_w_ctx = styles_w_ctx[contexts.index(context)]
842-
if context.trace_id != primary_context.trace_id:
842+
# encoding expects at least trace_id and span_id
843+
if context.span_id and context.trace_id and context.trace_id != primary_context.trace_id:
843844
links.append(
844845
SpanLink(
845846
context.trace_id,
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
fixes:
3+
- |
4+
propagation: This fix resolves an issue where a ``Context`` generated from extracted headers could
5+
lack a span_id or trace_id, leading ``SpanLink`` encoding errors.

tests/tracer/test_propagation.py

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,6 +1046,11 @@ def test_extract_tracecontext(headers, expected_context):
10461046
_HTTP_HEADER_B3_SPAN_ID: "NON_HEX",
10471047
_HTTP_HEADER_B3_SAMPLED: "3", # unexpected sampling value
10481048
}
1049+
B3_HEADERS_VALID_W_OUT_SPAN_ID = {
1050+
_HTTP_HEADER_B3_TRACE_ID: "80f198ee56343ba864fe8b2a57d3eff7",
1051+
_HTTP_HEADER_B3_SAMPLED: "1",
1052+
}
1053+
10491054
B3_SINGLE_HEADERS_VALID = {
10501055
_HTTP_HEADER_B3_SINGLE: "80f198ee56343ba864fe8b2a57d3eff7-e457b5a2e4d86bd1-1",
10511056
}
@@ -1066,11 +1071,18 @@ def test_extract_tracecontext(headers, expected_context):
10661071
DATADOG_TRACECONTEXT_MATCHING_TRACE_ID_HEADERS.update(TRACECONTEXT_HEADERS_VALID_64_bit)
10671072

10681073
# edge case testing
1069-
ALL_HEADERS_CHAOTIC = {}
1070-
ALL_HEADERS_CHAOTIC.update(DATADOG_HEADERS_VALID_MATCHING_TRACE_CONTEXT_VALID_TRACE_ID)
1071-
ALL_HEADERS_CHAOTIC.update(TRACECONTEXT_HEADERS_VALID_64_bit)
1072-
ALL_HEADERS_CHAOTIC.update(B3_SINGLE_HEADERS_VALID)
1073-
ALL_HEADERS_CHAOTIC.update(B3_HEADERS_INVALID)
1074+
ALL_HEADERS_CHAOTIC_1 = {}
1075+
ALL_HEADERS_CHAOTIC_1.update(DATADOG_HEADERS_VALID_MATCHING_TRACE_CONTEXT_VALID_TRACE_ID)
1076+
ALL_HEADERS_CHAOTIC_1.update(TRACECONTEXT_HEADERS_VALID_64_bit)
1077+
ALL_HEADERS_CHAOTIC_1.update(B3_SINGLE_HEADERS_VALID)
1078+
ALL_HEADERS_CHAOTIC_1.update(B3_HEADERS_INVALID)
1079+
1080+
# edge case testing
1081+
ALL_HEADERS_CHAOTIC_2 = {}
1082+
ALL_HEADERS_CHAOTIC_2.update(DATADOG_HEADERS_VALID)
1083+
ALL_HEADERS_CHAOTIC_2.update(TRACECONTEXT_HEADERS_VALID_64_bit)
1084+
ALL_HEADERS_CHAOTIC_2.update(B3_HEADERS_VALID_W_OUT_SPAN_ID)
1085+
ALL_HEADERS_CHAOTIC_2.update(B3_SINGLE_HEADERS_INVALID)
10741086

10751087
EXTRACT_FIXTURES = [
10761088
# Datadog headers
@@ -1786,6 +1798,40 @@ def test_DD_TRACE_PROPAGATION_STYLE_EXTRACT_overrides_DD_TRACE_PROPAGATION_STYLE
17861798
span_links=[],
17871799
),
17881800
),
1801+
# The trace_id from Datadog context will not align with the tracecontext primary context
1802+
# therefore we get a span link. B3 single headers are invalid so we won't see a trace of them.
1803+
# The b3 multi headers are missing a span_id, so we will skip creating a span link for it.
1804+
(
1805+
"all_headers_all_styles_do_not_create_span_link_for_context_w_out_span_id",
1806+
[
1807+
_PROPAGATION_STYLE_W3C_TRACECONTEXT,
1808+
PROPAGATION_STYLE_DATADOG,
1809+
PROPAGATION_STYLE_B3_SINGLE,
1810+
PROPAGATION_STYLE_B3_MULTI,
1811+
],
1812+
ALL_HEADERS_CHAOTIC_2,
1813+
Context(
1814+
trace_id=7277407061855694839,
1815+
span_id=67667974448284343,
1816+
meta={
1817+
"traceparent": "00-000000000000000064fe8b2a57d3eff7-00f067aa0ba902b7-01",
1818+
"tracestate": "dd=s:2;o:rum;t.dm:-4;t.usr.id:baz64,congo=t61rcWkgMzE",
1819+
"_dd.p.dm": "-4",
1820+
"_dd.p.usr.id": "baz64",
1821+
"_dd.origin": "rum",
1822+
},
1823+
metrics={"_sampling_priority_v1": 2},
1824+
span_links=[
1825+
SpanLink(
1826+
trace_id=13088165645273925489,
1827+
span_id=5678,
1828+
tracestate=None,
1829+
flags=1,
1830+
attributes={"reason": "terminated_context", "context_headers": "datadog"},
1831+
)
1832+
],
1833+
),
1834+
),
17891835
# tracecontext, b3, and b3multi all have the same trace_id
17901836
# therefore only datadog SpanLink is added to context
17911837
(
@@ -1872,7 +1918,7 @@ def test_DD_TRACE_PROPAGATION_STYLE_EXTRACT_overrides_DD_TRACE_PROPAGATION_STYLE
18721918
_PROPAGATION_STYLE_W3C_TRACECONTEXT,
18731919
PROPAGATION_STYLE_B3_SINGLE,
18741920
],
1875-
ALL_HEADERS_CHAOTIC,
1921+
ALL_HEADERS_CHAOTIC_1,
18761922
Context(
18771923
trace_id=7277407061855694839,
18781924
span_id=5678,

0 commit comments

Comments
 (0)