Skip to content

Commit 35e8f2a

Browse files
ZStriker19mabdinur
andauthored
fix(propagataion): handle whitespace in W3C tracecontext [backport #5351 to 1.9] (#5393)
Backport of #5351 to 1.9 Previously we were failing system-test `test_tracestate_ows_handling` https://github.com/DataDog/system-tests/blob/main/parametric/test_headers_tracecontext.py#L757-L835 This is because whitespace is allowed in tracecontext and we previously did not account for it. With this change we assure that we deal with whitespace by cutting it out and also ensure we do not send whitespace in our headers. ## 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/contributing.html#Release-Note-Guidelines) are followed. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] Author is aware of the performance implications of this PR as reported in the benchmarks PR comment. ## 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 is aware of, and discussed the performance implications of this PR as reported in the benchmarks PR comment. Co-authored-by: Munir Abdinur <[email protected]>
1 parent 00a6e2f commit 35e8f2a

File tree

4 files changed

+21
-7
lines changed

4 files changed

+21
-7
lines changed

ddtrace/propagation/http.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import re
22
from typing import Dict
33
from typing import FrozenSet
4+
from typing import List
45
from typing import Optional
56
from typing import Text
67
from typing import Tuple
@@ -611,12 +612,13 @@ def _get_traceparent_values(tp):
611612
return trace_id, span_id, sampling_priority
612613

613614
@staticmethod
614-
def _get_tracestate_values(ts):
615-
# type: (str) -> Tuple[Optional[int], Dict[str, str], Optional[str]]
615+
def _get_tracestate_values(ts_l):
616+
# type: (List[str]) -> Tuple[Optional[int], Dict[str, str], Optional[str]]
617+
618+
# tracestate list parsing example: ["dd=s:2;o:rum;t.dm:-4;t.usr.id:baz64","congo=t61rcWkgMzE"]
619+
# -> 2, {"_dd.p.dm":"-4","_dd.p.usr.id":"baz64"}, "rum"
616620

617-
# tracestate parsing, example: dd=s:2;o:rum;t.dm:-4;t.usr.id:baz64,congo=t61rcWkgMzE
618621
dd = None
619-
ts_l = ts.strip().split(",")
620622
for list_mem in ts_l:
621623
if list_mem.startswith("dd="):
622624
# cut out dd= before turning into dict
@@ -686,7 +688,12 @@ def _extract(headers):
686688
meta = {W3C_TRACEPARENT_KEY: tp} # type: _MetaDictType
687689

688690
ts = _extract_header_value(_POSSIBLE_HTTP_HEADER_TRACESTATE, headers)
691+
689692
if ts:
693+
# whitespace is allowed, but whitespace to start or end values should be trimmed
694+
# e.g. "foo=1 \t , \t bar=2, \t baz=3" -> "foo=1,bar=2,baz=3"
695+
ts_l = [member.strip() for member in ts.split(",")]
696+
ts = ",".join(ts_l)
690697
# the value MUST contain only ASCII characters in the
691698
# range of 0x20 to 0x7E
692699
if re.search(r"[^\x20-\x7E]+", ts):
@@ -695,7 +702,7 @@ def _extract(headers):
695702
# store tracestate so we keep other vendor data for injection, even if dd ends up being invalid
696703
meta[W3C_TRACESTATE_KEY] = ts
697704
try:
698-
tracestate_values = _TraceContext._get_tracestate_values(ts)
705+
tracestate_values = _TraceContext._get_tracestate_values(ts_l)
699706
except (TypeError, ValueError):
700707
log.debug("received invalid dd header value in tracestate: %r ", ts)
701708
tracestate_values = None

docs/spelling_wordlist.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ submodule
180180
submodules
181181
substring
182182
timestamp
183+
tracestate
183184
tweens
184185
uWSGI
185186
unbuffered
@@ -199,6 +200,7 @@ versioned
199200
vertica
200201
w3c
201202
whitelist
203+
whitespace
202204
workflow
203205
wsgi
204206
xfail
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 previously W3C tracestate propagation could not handle whitespace.
5+
With this fix whitespace is now removed for incoming and outgoing requests.

tests/tracer/test_propagation.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -705,10 +705,10 @@ def test_extract_tracestate(caplog, ts_string, expected_tuple, expected_logging,
705705
with caplog.at_level(logging.DEBUG):
706706
if expected_exception:
707707
with pytest.raises(expected_exception):
708-
tracestate_values = _TraceContext._get_tracestate_values(ts_string)
708+
tracestate_values = _TraceContext._get_tracestate_values(ts_string.split(","))
709709
assert tracestate_values == expected_tuple
710710
else:
711-
tracestate_values = _TraceContext._get_tracestate_values(ts_string)
711+
tracestate_values = _TraceContext._get_tracestate_values(ts_string.split(","))
712712
assert tracestate_values == expected_tuple
713713
if caplog.text or expected_logging:
714714
for expected_log in expected_logging:

0 commit comments

Comments
 (0)