From 22302147a2ac3baa03727c73085fd6575fba60e9 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Tue, 7 May 2024 21:58:41 -0400 Subject: [PATCH] chore(distributed_tracing): ensure last datadog parent id tag is always set (#9174) Implements W3C Phase 3. The last datadog parent id is always propagated in a distributed trace (if ddtrace propagation style includes tracecontext). This change will reduce instances where flamegraphs contain missing spans. Note - This change does not require a release note. It is a follow up to a previous PR. Follow up to: https://github.com/DataDog/dd-trace-py/commit/90a3e3fed47763abf64cb4c04c6a32f5c64076cb#diff-18af2a7682cf3014ab4ae236fc54d2454bbf5293e81e098b1d22d39bdb61012d ## 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] Risks are described (performance impact, potential for breakage, maintainability) - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed or label `changelog/no-changelog` is set - [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)) - [x] If this PR changes the public interface, I've notified `@DataDog/apm-tees`. ## Reviewer Checklist - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Description motivates each change - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] Release note makes sense to a user of the library - [x] Author has 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) --- ddtrace/internal/constants.py | 1 + ddtrace/propagation/http.py | 14 +++++++++----- tests/tracer/test_propagation.py | 17 +++++++++++------ 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/ddtrace/internal/constants.py b/ddtrace/internal/constants.py index 50b8e1280e4..7988687a74c 100644 --- a/ddtrace/internal/constants.py +++ b/ddtrace/internal/constants.py @@ -25,6 +25,7 @@ DEFAULT_SAMPLING_RATE_LIMIT = 100 SAMPLING_DECISION_TRACE_TAG_KEY = "_dd.p.dm" LAST_DD_PARENT_ID_KEY = "_dd.parent_id" +DEFAULT_LAST_PARENT_ID = "0000000000000000" DEFAULT_SERVICE_NAME = "unnamed-python-service" # Used to set the name of an integration on a span COMPONENT = "component" diff --git a/ddtrace/propagation/http.py b/ddtrace/propagation/http.py index c0768194d8b..6ad0c86d41d 100644 --- a/ddtrace/propagation/http.py +++ b/ddtrace/propagation/http.py @@ -38,6 +38,7 @@ from ..internal.compat import ensure_text from ..internal.constants import _PROPAGATION_STYLE_NONE from ..internal.constants import _PROPAGATION_STYLE_W3C_TRACECONTEXT +from ..internal.constants import DEFAULT_LAST_PARENT_ID from ..internal.constants import HIGHER_ORDER_TRACE_ID_BITS as _HIGHER_ORDER_TRACE_ID_BITS from ..internal.constants import LAST_DD_PARENT_ID_KEY from ..internal.constants import MAX_UINT_64BITS as _MAX_UINT_64BITS @@ -700,7 +701,7 @@ def _get_traceparent_values(tp): @staticmethod def _get_tracestate_values(ts_l): - # type: (List[str]) -> Tuple[Optional[int], Dict[str, str], Optional[str], Optional[str]] + # type: (List[str]) -> Tuple[Optional[int], Dict[str, str], Optional[str], str] # tracestate list parsing example: ["dd=s:2;o:rum;t.dm:-4;t.usr.id:baz64","congo=t61rcWkgMzE"] # -> 2, {"_dd.p.dm":"-4","_dd.p.usr.id":"baz64"}, "rum" @@ -727,7 +728,7 @@ def _get_tracestate_values(ts_l): origin = _TraceContext.decode_tag_val(origin) # Get last datadog parent id, this field is used to reconnect traces with missing spans - lpid = dd.get("p", "0000000000000000") + lpid = dd.get("p", DEFAULT_LAST_PARENT_ID) # need to convert from t. to _dd.p. other_propagated_tags = { @@ -736,7 +737,7 @@ def _get_tracestate_values(ts_l): return sampling_priority_ts_int, other_propagated_tags, origin, lpid else: - return None, {}, None, None + return None, {}, None, DEFAULT_LAST_PARENT_ID @staticmethod def _get_sampling_priority( @@ -820,8 +821,7 @@ def _get_context(trace_id, span_id, trace_flag, ts, meta=None): if tracestate_values: sampling_priority_ts, other_propagated_tags, origin, lpid = tracestate_values meta.update(other_propagated_tags.items()) - if lpid: - meta[LAST_DD_PARENT_ID_KEY] = lpid + meta[LAST_DD_PARENT_ID_KEY] = lpid sampling_priority = _TraceContext._get_sampling_priority(trace_flag, sampling_priority_ts, origin) else: @@ -921,6 +921,10 @@ def _resolve_contexts(contexts, styles_w_ctx, normalized_headers): ts = _extract_header_value(_POSSIBLE_HTTP_HEADER_TRACESTATE, normalized_headers) if ts: primary_context._meta[W3C_TRACESTATE_KEY] = ts + # Ensure the last datadog parent id is always set on the primary context + primary_context._meta[LAST_DD_PARENT_ID_KEY] = context._meta.get( + LAST_DD_PARENT_ID_KEY, DEFAULT_LAST_PARENT_ID + ) primary_context._span_links = links return primary_context diff --git a/tests/tracer/test_propagation.py b/tests/tracer/test_propagation.py index e6c263e5d83..4c3071e61b0 100644 --- a/tests/tracer/test_propagation.py +++ b/tests/tracer/test_propagation.py @@ -692,7 +692,7 @@ def test_get_wsgi_header(tracer): # noqa: F811 TRACECONTEXT_HEADERS_VALID_64_bit = { _HTTP_HEADER_TRACEPARENT: "00-000000000000000064fe8b2a57d3eff7-00f067aa0ba902b7-01", - _HTTP_HEADER_TRACESTATE: "dd=s:2;o:rum;t.dm:-4;t.usr.id:baz64,congo=t61rcWkgMzE", + _HTTP_HEADER_TRACESTATE: "dd=s:2;o:rum;p:00f067aa0ba902b7;t.dm:-4;t.usr.id:baz64,congo=t61rcWkgMzE", } @@ -937,7 +937,7 @@ def test_extract_traceparent(caplog, headers, expected_tuple, expected_logging, ( "congo=t61rcWkgMzE,mako=s:2;o:rum;", # sampling_priority_ts, other_propagated_tags, origin, parent id - (None, {}, None, None), + (None, {}, None, "0000000000000000"), None, None, ), @@ -1754,7 +1754,11 @@ def test_extract_tracecontext(headers, expected_context): "span_id": 5678, "sampling_priority": 1, "dd_origin": "synthetics", - "meta": {"tracestate": TRACECONTEXT_HEADERS_VALID[_HTTP_HEADER_TRACESTATE], "_dd.p.dm": "-3"}, + "meta": { + "tracestate": DATADOG_TRACECONTEXT_MATCHING_TRACE_ID_HEADERS[_HTTP_HEADER_TRACESTATE], + "_dd.p.dm": "-3", + LAST_DD_PARENT_ID_KEY: "00f067aa0ba902b7", + }, }, ), # testing that tracestate is not added when tracecontext style comes later and does not match first style's trace-id @@ -2002,11 +2006,11 @@ def test_DD_TRACE_PROPAGATION_STYLE_EXTRACT_overrides_DD_TRACE_PROPAGATION_STYLE span_id=67667974448284343, meta={ "traceparent": "00-000000000000000064fe8b2a57d3eff7-00f067aa0ba902b7-01", - "tracestate": TRACECONTEXT_HEADERS_VALID[_HTTP_HEADER_TRACESTATE], + "tracestate": ALL_HEADERS_CHAOTIC_2[_HTTP_HEADER_TRACESTATE], "_dd.p.dm": "-4", "_dd.p.usr.id": "baz64", "_dd.origin": "rum", - LAST_DD_PARENT_ID_KEY: "0000000000000000", + LAST_DD_PARENT_ID_KEY: "00f067aa0ba902b7", }, metrics={"_sampling_priority_v1": 2}, span_links=[ @@ -2117,7 +2121,8 @@ def test_DD_TRACE_PROPAGATION_STYLE_EXTRACT_overrides_DD_TRACE_PROPAGATION_STYLE meta={ "_dd.p.dm": "-3", "_dd.origin": "synthetics", - "tracestate": "dd=s:2;o:rum;t.dm:-4;t.usr.id:baz64,congo=t61rcWkgMzE", + "tracestate": ALL_HEADERS_CHAOTIC_1[_HTTP_HEADER_TRACESTATE], + LAST_DD_PARENT_ID_KEY: "00f067aa0ba902b7", }, metrics={"_sampling_priority_v1": 1}, span_links=[