Skip to content

Commit

Permalink
chore(distributed_tracing): ensure last datadog parent id tag is alwa…
Browse files Browse the repository at this point in the history
…ys 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:
90a3e3f#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)
  • Loading branch information
mabdinur committed May 8, 2024
1 parent dac8aa2 commit 2230214
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 11 deletions.
1 change: 1 addition & 0 deletions ddtrace/internal/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
14 changes: 9 additions & 5 deletions ddtrace/propagation/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand All @@ -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 = {
Expand All @@ -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(
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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

Expand Down
17 changes: 11 additions & 6 deletions tests/tracer/test_propagation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}


Expand Down Expand Up @@ -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,
),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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=[
Expand Down Expand Up @@ -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=[
Expand Down

0 comments on commit 2230214

Please sign in to comment.