From 75e962b1423f7d958b17957ebe971c0c2b262e92 Mon Sep 17 00:00:00 2001 From: erikayasuda <153395705+erikayasuda@users.noreply.github.com> Date: Mon, 16 Dec 2024 12:57:16 -0500 Subject: [PATCH 01/17] chore: use guess-next-dev instead of release-branch-semver [2.18] (#11723) This PR updates the `version_schema` in the `../pyproject.toml` file for the 2.18 release branch from `release-branch-semver` to `guess-next-dev`. This is to ensure that system tests work as intended with backports to this release branch. IMPORTANT: This PR needs to be merged before the first backport is created for 2.18.Otherwise, system tests will not work as expected. - [x] Reviewer check --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 9c8ff26d223..95bce61fcd3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -71,7 +71,7 @@ Homepage = "https://github.com/DataDog/dd-trace-py" "Source Code" = "https://github.com/DataDog/dd-trace-py/" [tool.setuptools_scm] -version_scheme = "release-branch-semver" # Must be "release-branch-semver" for now in main, see https://github.com/DataDog/dd-trace-py/issues/8801 +version_scheme = "guess-next-dev" # Must be "release-branch-semver" for now in main, see https://github.com/DataDog/dd-trace-py/issues/8801 write_to = "ddtrace/_version.py" [tool.cython-lint] From 6bb20368774d661bce8ea9d91dc5e2b5409153e0 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 18 Dec 2024 09:37:30 +0100 Subject: [PATCH 02/17] fix(iast): check context is enable in request and builtins patched funcions [backport 2.18] (#11756) Backport a4f0e380a459542795f2d881895f18765211893b from #11752 to 2.18. This fix resolves an issue where AppSec was using a patched request and builtins functions, creating telemetry errors. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - 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) Co-authored-by: Alberto Vara --- ddtrace/appsec/_common_module_patches.py | 16 ++++++++++++---- ...ast-propagation-error-2-ba4a998133269a7c.yaml | 5 +++++ 2 files changed, 17 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/iast-fix-iast-propagation-error-2-ba4a998133269a7c.yaml diff --git a/ddtrace/appsec/_common_module_patches.py b/ddtrace/appsec/_common_module_patches.py index a5ab2d1533d..e7ce12d13e9 100644 --- a/ddtrace/appsec/_common_module_patches.py +++ b/ddtrace/appsec/_common_module_patches.py @@ -60,8 +60,10 @@ def wrapped_read_F3E51D71B4EC16EF(original_read_callable, instance, args, kwargs """ wrapper for _io.BytesIO and _io.StringIO read function """ + from ddtrace.appsec._iast._iast_request_context import is_iast_request_enabled + result = original_read_callable(*args, **kwargs) - if asm_config._iast_enabled: + if asm_config._iast_enabled and is_iast_request_enabled(): from ddtrace.appsec._iast._taint_tracking import OriginType from ddtrace.appsec._iast._taint_tracking import Source from ddtrace.appsec._iast._taint_tracking import get_tainted_ranges @@ -87,7 +89,9 @@ def wrapped_open_CFDDB7ABBA9081B6(original_open_callable, instance, args, kwargs """ wrapper for open file function """ - if asm_config._iast_enabled: + from ddtrace.appsec._iast._iast_request_context import is_iast_request_enabled + + if asm_config._iast_enabled and is_iast_request_enabled(): try: from ddtrace.appsec._iast.taint_sinks.path_traversal import check_and_report_path_traversal @@ -176,7 +180,9 @@ def wrapped_request_D8CB81E472AF98A2(original_request_callable, instance, args, wrapper for third party requests.request function https://requests.readthedocs.io """ - if asm_config._iast_enabled: + from ddtrace.appsec._iast._iast_request_context import is_iast_request_enabled + + if asm_config._iast_enabled and is_iast_request_enabled(): from ddtrace.appsec._iast.taint_sinks.ssrf import _iast_report_ssrf _iast_report_ssrf(original_request_callable, *args, **kwargs) @@ -216,7 +222,9 @@ def wrapped_system_5542593D237084A7(original_command_callable, instance, args, k """ command = args[0] if args else kwargs.get("command", None) if command is not None: - if asm_config._iast_enabled: + from ddtrace.appsec._iast._iast_request_context import is_iast_request_enabled + + if asm_config._iast_enabled and is_iast_request_enabled(): from ddtrace.appsec._iast.taint_sinks.command_injection import _iast_report_cmdi _iast_report_cmdi(command) diff --git a/releasenotes/notes/iast-fix-iast-propagation-error-2-ba4a998133269a7c.yaml b/releasenotes/notes/iast-fix-iast-propagation-error-2-ba4a998133269a7c.yaml new file mode 100644 index 00000000000..4918edb17a7 --- /dev/null +++ b/releasenotes/notes/iast-fix-iast-propagation-error-2-ba4a998133269a7c.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + ASM: This fix resolves an issue where AppSec was using a patched request and builtins functions, + creating telemetry errors. \ No newline at end of file From 9924f37ad8536edd2466f3874bc691eb5e64719a Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 18 Dec 2024 20:20:28 +0000 Subject: [PATCH 03/17] chore(ci): upgrade python for build action [backport 2.18] (#11782) Co-authored-by: Christophe Papazian <114495376+christophe-papazian@users.noreply.github.com> --- .github/workflows/build_deploy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build_deploy.yml b/.github/workflows/build_deploy.yml index 77d52c757f5..df5184f83e5 100644 --- a/.github/workflows/build_deploy.yml +++ b/.github/workflows/build_deploy.yml @@ -40,7 +40,7 @@ jobs: - uses: actions/setup-python@v5 name: Install Python with: - python-version: '3.7' + python-version: '3.12' - name: Build sdist run: | pip install "setuptools_scm[toml]>=4" "cython" "cmake>=3.24.2,<3.28" "setuptools-rust" From e1b10df2aaddf1314c64aa73d1c810d698f7613f Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Thu, 19 Dec 2024 18:29:38 -0500 Subject: [PATCH 04/17] add extract behavior feat --- ddtrace/internal/constants.py | 4 + ddtrace/propagation/http.py | 61 +++-- ddtrace/settings/config.py | 8 + ...ion_behavior_extract-3d16765cfd07485b.yaml | 7 + tests/telemetry/test_writer.py | 4 +- tests/tracer/test_propagation.py | 224 +++++++++++++++++- 6 files changed, 282 insertions(+), 26 deletions(-) create mode 100644 releasenotes/notes/propagation_behavior_extract-3d16765cfd07485b.yaml diff --git a/ddtrace/internal/constants.py b/ddtrace/internal/constants.py index 4efdc754ef3..c4255035c41 100644 --- a/ddtrace/internal/constants.py +++ b/ddtrace/internal/constants.py @@ -19,6 +19,10 @@ _PROPAGATION_STYLE_NONE, _PROPAGATION_STYLE_BAGGAGE, ) +_PROPAGATION_BEHAVIOR_CONTINUE = "continue" +_PROPAGATION_BEHAVIOR_IGNORE = "ignore" +_PROPAGATION_BEHAVIOR_RESTART = "restart" +_PROPAGATION_BEHAVIOR_DEFAULT = _PROPAGATION_BEHAVIOR_CONTINUE W3C_TRACESTATE_KEY = "tracestate" W3C_TRACEPARENT_KEY = "traceparent" W3C_TRACESTATE_PARENT_ID_KEY = "p" diff --git a/ddtrace/propagation/http.py b/ddtrace/propagation/http.py index a1664664ace..6d7cd8325ab 100644 --- a/ddtrace/propagation/http.py +++ b/ddtrace/propagation/http.py @@ -40,6 +40,8 @@ from ..internal._tagset import decode_tagset_string from ..internal._tagset import encode_tagset_values from ..internal.compat import ensure_text +from ..internal.constants import _PROPAGATION_BEHAVIOR_IGNORE +from ..internal.constants import _PROPAGATION_BEHAVIOR_RESTART from ..internal.constants import _PROPAGATION_STYLE_BAGGAGE from ..internal.constants import _PROPAGATION_STYLE_NONE from ..internal.constants import _PROPAGATION_STYLE_W3C_TRACECONTEXT @@ -973,12 +975,12 @@ class HTTPPropagator(object): """ @staticmethod - def _extract_configured_contexts_avail(normalized_headers): + def _extract_configured_contexts_avail(normalized_headers: Dict[str, str]) -> Tuple[List[Context], List[str]]: contexts = [] styles_w_ctx = [] for prop_style in config._propagation_style_extract: propagator = _PROP_STYLES[prop_style] - context = propagator._extract(normalized_headers) + context = propagator._extract(normalized_headers) # type: ignore # baggage is handled separately if prop_style == _PROPAGATION_STYLE_BAGGAGE: continue @@ -987,6 +989,24 @@ def _extract_configured_contexts_avail(normalized_headers): styles_w_ctx.append(prop_style) return contexts, styles_w_ctx + @staticmethod + def _context_to_span_link(context: Context, style: str, reason: str) -> Optional[SpanLink]: + # encoding expects at least trace_id and span_id + if context.span_id and context.trace_id: + return SpanLink( + context.trace_id, + context.span_id, + flags=1 if context.sampling_priority and context.sampling_priority > 0 else 0, + tracestate=( + context._meta.get(W3C_TRACESTATE_KEY, "") if style == _PROPAGATION_STYLE_W3C_TRACECONTEXT else None + ), + attributes={ + "reason": reason, + "context_headers": style, + }, + ) + return None + @staticmethod def _resolve_contexts(contexts, styles_w_ctx, normalized_headers): primary_context = contexts[0] @@ -995,23 +1015,14 @@ def _resolve_contexts(contexts, styles_w_ctx, normalized_headers): for context in contexts[1:]: style_w_ctx = styles_w_ctx[contexts.index(context)] # encoding expects at least trace_id and span_id - if context.span_id and context.trace_id and context.trace_id != primary_context.trace_id: - links.append( - SpanLink( - context.trace_id, - context.span_id, - flags=1 if context.sampling_priority and context.sampling_priority > 0 else 0, - tracestate=( - context._meta.get(W3C_TRACESTATE_KEY, "") - if style_w_ctx == _PROPAGATION_STYLE_W3C_TRACECONTEXT - else None - ), - attributes={ - "reason": "terminated_context", - "context_headers": style_w_ctx, - }, - ) + if context.trace_id and context.trace_id != primary_context.trace_id: + link = HTTPPropagator._context_to_span_link( + context, + style_w_ctx, + "terminated_context", ) + if link: + links.append(link) # if trace_id matches and the propagation style is tracecontext # add the tracestate to the primary context elif style_w_ctx == _PROPAGATION_STYLE_W3C_TRACECONTEXT: @@ -1129,17 +1140,19 @@ def my_controller(url, headers): :param dict headers: HTTP headers to extract tracing attributes. :return: New `Context` with propagated attributes. """ - if not headers: - return Context() + context = Context() + if not headers or config._propagation_behavior_extract == _PROPAGATION_BEHAVIOR_IGNORE: + return context try: + style = "" normalized_headers = {name.lower(): v for name, v in headers.items()} - context = Context() # tracer configured to extract first only if config._propagation_extract_first: # loop through the extract propagation styles specified in order, return whatever context we get first for prop_style in config._propagation_style_extract: propagator = _PROP_STYLES[prop_style] context = propagator._extract(normalized_headers) + style = prop_style if config.propagation_http_baggage_enabled is True: _attach_baggage_to_context(normalized_headers, context) break @@ -1147,6 +1160,9 @@ def my_controller(url, headers): # loop through all extract propagation styles else: contexts, styles_w_ctx = HTTPPropagator._extract_configured_contexts_avail(normalized_headers) + # check that styles_w_ctx is not empty + if styles_w_ctx: + style = styles_w_ctx[0] if contexts: context = HTTPPropagator._resolve_contexts(contexts, styles_w_ctx, normalized_headers) @@ -1161,6 +1177,9 @@ def my_controller(url, headers): context._baggage = baggage_context._baggage else: context = baggage_context + if config._propagation_behavior_extract == _PROPAGATION_BEHAVIOR_RESTART: + link = HTTPPropagator._context_to_span_link(context, style, "propagation_behavior_extract") + context = Context(baggage=context.get_all_baggage_items(), span_links=[link] if link else []) return context diff --git a/ddtrace/settings/config.py b/ddtrace/settings/config.py index 65baf99ccb3..1f1f3730e3c 100644 --- a/ddtrace/settings/config.py +++ b/ddtrace/settings/config.py @@ -20,6 +20,7 @@ from ddtrace.vendor.debtcollector import deprecate from ..internal import gitmetadata +from ..internal.constants import _PROPAGATION_BEHAVIOR_DEFAULT from ..internal.constants import _PROPAGATION_STYLE_DEFAULT from ..internal.constants import DEFAULT_BUFFER_SIZE from ..internal.constants import DEFAULT_MAX_PAYLOAD_SIZE @@ -540,6 +541,10 @@ def __init__(self): self._propagation_extract_first = _get_config("DD_TRACE_PROPAGATION_EXTRACT_FIRST", False, asbool) + self._propagation_behavior_extract = _get_config( + ["DD_TRACE_PROPAGATION_BEHAVIOR_EXTRACT"], _PROPAGATION_BEHAVIOR_DEFAULT, self._lower + ) + # Datadog tracer tags propagation x_datadog_tags_max_length = _get_config("DD_TRACE_X_DATADOG_TAGS_MAX_LENGTH", 512, int) if x_datadog_tags_max_length < 0: @@ -978,3 +983,6 @@ def convert_rc_trace_sampling_rules(self, rc_rules: List[Dict[str, Any]]) -> Opt return json.dumps(rc_rules) else: return None + + def _lower(self, value): + return value.lower() diff --git a/releasenotes/notes/propagation_behavior_extract-3d16765cfd07485b.yaml b/releasenotes/notes/propagation_behavior_extract-3d16765cfd07485b.yaml new file mode 100644 index 00000000000..8e34092a6bb --- /dev/null +++ b/releasenotes/notes/propagation_behavior_extract-3d16765cfd07485b.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + propagation: This introduces the environment variable ``DD_TRACE_PROPAGATION_BEHAVIOR_EXTRACT`` + to control the behavior of the extraction of distributed tracing headers. The values, ``continue`` (default), + ``ignore``, and ``restart``, are supported. The default value is ``continue`` which should have no change from the current behavior of always propagating valid headers. + With ``ignore`` ignoring all incoming headers and with ``restart`` turning incoming headers into a span links and propagating baggage items. diff --git a/tests/telemetry/test_writer.py b/tests/telemetry/test_writer.py index 7718710ff60..e1a34f406a0 100644 --- a/tests/telemetry/test_writer.py +++ b/tests/telemetry/test_writer.py @@ -268,7 +268,8 @@ def test_app_started_event_configuration_override(test_agent_session, run_python env["DD_SPAN_SAMPLING_RULES_FILE"] = str(file) env["DD_TRACE_PARTIAL_FLUSH_ENABLED"] = "false" env["DD_TRACE_PARTIAL_FLUSH_MIN_SPANS"] = "3" - env["DD_SITE"] = "datadoghq.com" + env["DD_TRACE_PROPAGATION_BEHAVIOR_EXTRACT"] = "restart" + # By default telemetry collection is enabled after 10 seconds, so we either need to # to sleep for 10 seconds or manually call _app_started() to generate the app started event. # This delay allows us to collect start up errors and dynamic configurations @@ -444,6 +445,7 @@ def test_app_started_event_configuration_override(test_agent_session, run_python {"name": "DD_TRACE_OTEL_ENABLED", "origin": "env_var", "value": True}, {"name": "DD_TRACE_PARTIAL_FLUSH_ENABLED", "origin": "env_var", "value": False}, {"name": "DD_TRACE_PARTIAL_FLUSH_MIN_SPANS", "origin": "env_var", "value": 3}, + {"name": "DD_TRACE_PROPAGATION_BEHAVIOR_EXTRACT", "origin": "env_var", "value": "restart"}, {"name": "DD_TRACE_PROPAGATION_EXTRACT_FIRST", "origin": "default", "value": False}, {"name": "DD_TRACE_PROPAGATION_HTTP_BAGGAGE_ENABLED", "origin": "default", "value": False}, {"name": "DD_TRACE_PROPAGATION_STYLE_EXTRACT", "origin": "env_var", "value": "tracecontext"}, diff --git a/tests/tracer/test_propagation.py b/tests/tracer/test_propagation.py index 2e1a299c4d4..7c29ce9c473 100644 --- a/tests/tracer/test_propagation.py +++ b/tests/tracer/test_propagation.py @@ -15,6 +15,8 @@ from ddtrace.constants import AUTO_REJECT from ddtrace.constants import USER_KEEP from ddtrace.constants import USER_REJECT +from ddtrace.internal.constants import _PROPAGATION_BEHAVIOR_IGNORE +from ddtrace.internal.constants import _PROPAGATION_BEHAVIOR_RESTART from ddtrace.internal.constants import _PROPAGATION_STYLE_BAGGAGE from ddtrace.internal.constants import _PROPAGATION_STYLE_NONE from ddtrace.internal.constants import _PROPAGATION_STYLE_W3C_TRACECONTEXT @@ -1500,6 +1502,9 @@ def test_extract_tracecontext(headers, expected_context): HTTP_HEADER_PARENT_ID: "parent_id", HTTP_HEADER_SAMPLING_PRIORITY: "sample", } + +DATADOG_BAGGAGE_HEADERS_VALID = {**DATADOG_HEADERS_VALID, "baggage": "key1=val1,key2=val2"} + B3_HEADERS_VALID = { _HTTP_HEADER_B3_TRACE_ID: "80f198ee56343ba864fe8b2a57d3eff7", _HTTP_HEADER_B3_SPAN_ID: "a2fb4a1d1a96d312", @@ -1553,6 +1558,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_datadog_default", None, + None, DATADOG_HEADERS_VALID, { "trace_id": 13088165645273925489, @@ -1565,6 +1571,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_datadog_default_wsgi", None, + None, {get_wsgi_header(name): value for name, value in DATADOG_HEADERS_VALID.items()}, { "trace_id": 13088165645273925489, @@ -1577,6 +1584,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_datadog_no_priority", None, + None, DATADOG_HEADERS_VALID_NO_PRIORITY, { "trace_id": 13088165645273925489, @@ -1589,12 +1597,14 @@ def test_extract_tracecontext(headers, expected_context): ( "invalid_datadog", [PROPAGATION_STYLE_DATADOG], + None, DATADOG_HEADERS_INVALID, CONTEXT_EMPTY, ), ( "valid_datadog_explicit_style", [PROPAGATION_STYLE_DATADOG], + None, DATADOG_HEADERS_VALID, { "trace_id": 13088165645273925489, @@ -1607,6 +1617,7 @@ def test_extract_tracecontext(headers, expected_context): ( "invalid_datadog_negative_trace_id", [PROPAGATION_STYLE_DATADOG], + None, { HTTP_HEADER_TRACE_ID: "-1", HTTP_HEADER_PARENT_ID: "5678", @@ -1618,6 +1629,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_datadog_explicit_style_wsgi", [PROPAGATION_STYLE_DATADOG], + None, {get_wsgi_header(name): value for name, value in DATADOG_HEADERS_VALID.items()}, { "trace_id": 13088165645273925489, @@ -1630,6 +1642,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_datadog_all_styles", [PROPAGATION_STYLE_DATADOG, PROPAGATION_STYLE_B3_MULTI, PROPAGATION_STYLE_B3_SINGLE], + None, DATADOG_HEADERS_VALID, { "trace_id": 13088165645273925489, @@ -1642,13 +1655,29 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_datadog_no_datadog_style", [PROPAGATION_STYLE_B3_MULTI], + None, DATADOG_HEADERS_VALID, CONTEXT_EMPTY, ), + ( + "valid_datadog_and_baggage_default", + None, + None, + DATADOG_BAGGAGE_HEADERS_VALID, + { + "trace_id": 13088165645273925489, + "span_id": 5678, + "sampling_priority": 1, + "dd_origin": "synthetics", + "meta": {"_dd.p.dm": "-3"}, + "baggage": {"key1": "val1", "key2": "val2"}, + }, + ), # B3 headers ( "valid_b3_simple", [PROPAGATION_STYLE_B3_MULTI], + None, B3_HEADERS_VALID, { "trace_id": TRACE_ID, @@ -1660,6 +1689,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_b3_wsgi", [PROPAGATION_STYLE_B3_MULTI], + None, {get_wsgi_header(name): value for name, value in B3_HEADERS_VALID.items()}, { "trace_id": TRACE_ID, @@ -1671,6 +1701,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_b3_flags", [PROPAGATION_STYLE_B3_MULTI], + None, { _HTTP_HEADER_B3_TRACE_ID: B3_HEADERS_VALID[_HTTP_HEADER_B3_TRACE_ID], _HTTP_HEADER_B3_SPAN_ID: B3_HEADERS_VALID[_HTTP_HEADER_B3_SPAN_ID], @@ -1686,6 +1717,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_b3_with_parent_id", [PROPAGATION_STYLE_B3_MULTI], + None, { _HTTP_HEADER_B3_TRACE_ID: B3_HEADERS_VALID[_HTTP_HEADER_B3_TRACE_ID], _HTTP_HEADER_B3_SPAN_ID: B3_HEADERS_VALID[_HTTP_HEADER_B3_SPAN_ID], @@ -1702,6 +1734,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_b3_only_trace_and_span_id", [PROPAGATION_STYLE_B3_MULTI], + None, { _HTTP_HEADER_B3_TRACE_ID: B3_HEADERS_VALID[_HTTP_HEADER_B3_TRACE_ID], _HTTP_HEADER_B3_SPAN_ID: B3_HEADERS_VALID[_HTTP_HEADER_B3_SPAN_ID], @@ -1716,6 +1749,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_b3_only_trace_id", [PROPAGATION_STYLE_B3_MULTI], + None, { _HTTP_HEADER_B3_TRACE_ID: B3_HEADERS_VALID[_HTTP_HEADER_B3_TRACE_ID], }, @@ -1729,24 +1763,28 @@ def test_extract_tracecontext(headers, expected_context): ( "invalid_b3", [PROPAGATION_STYLE_B3_MULTI], + None, B3_HEADERS_INVALID, CONTEXT_EMPTY, ), ( "valid_b3_default_style", None, + None, B3_HEADERS_VALID, CONTEXT_EMPTY, ), ( "valid_b3_no_b3_style", [PROPAGATION_STYLE_B3_SINGLE], + None, B3_HEADERS_VALID, CONTEXT_EMPTY, ), ( "valid_b3_all_styles", [PROPAGATION_STYLE_DATADOG, PROPAGATION_STYLE_B3_MULTI, PROPAGATION_STYLE_B3_SINGLE], + None, B3_HEADERS_VALID, { "trace_id": TRACE_ID, @@ -1759,6 +1797,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_b3_single_header_simple", [PROPAGATION_STYLE_B3_SINGLE], + None, B3_SINGLE_HEADERS_VALID, { "trace_id": TRACE_ID, @@ -1770,6 +1809,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_b3_single_header_simple", [PROPAGATION_STYLE_B3_SINGLE], + None, { get_wsgi_header(_HTTP_HEADER_B3_SINGLE): B3_SINGLE_HEADERS_VALID[_HTTP_HEADER_B3_SINGLE], }, @@ -1783,6 +1823,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_b3_single_header_simple", [PROPAGATION_STYLE_B3_SINGLE], + None, { get_wsgi_header(_HTTP_HEADER_B3_SINGLE): B3_SINGLE_HEADERS_VALID[_HTTP_HEADER_B3_SINGLE], }, @@ -1796,6 +1837,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_b3_single_header_only_sampled", [PROPAGATION_STYLE_B3_SINGLE], + None, { _HTTP_HEADER_B3_SINGLE: "1", }, @@ -1809,6 +1851,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_b3_single_header_only_trace_and_span_id", [PROPAGATION_STYLE_B3_SINGLE], + None, { _HTTP_HEADER_B3_SINGLE: "80f198ee56343ba864fe8b2a57d3eff7-e457b5a2e4d86bd1", }, @@ -1822,12 +1865,14 @@ def test_extract_tracecontext(headers, expected_context): ( "invalid_b3_single_header", [PROPAGATION_STYLE_B3_SINGLE], + None, B3_SINGLE_HEADERS_INVALID, CONTEXT_EMPTY, ), ( "valid_b3_single_header_all_styles", [PROPAGATION_STYLE_DATADOG, PROPAGATION_STYLE_B3_MULTI, PROPAGATION_STYLE_B3_SINGLE], + None, B3_SINGLE_HEADERS_VALID, { "trace_id": TRACE_ID, @@ -1839,6 +1884,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_b3_single_header_extra_data", [PROPAGATION_STYLE_B3_SINGLE], + None, {_HTTP_HEADER_B3_SINGLE: B3_SINGLE_HEADERS_VALID[_HTTP_HEADER_B3_SINGLE] + "-05e3ac9a4f6e3b90-extra-data-here"}, { "trace_id": TRACE_ID, @@ -1850,12 +1896,14 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_b3_single_header_default_style", None, + None, B3_SINGLE_HEADERS_VALID, CONTEXT_EMPTY, ), ( "valid_b3_single_header_no_b3_single_header_style", [PROPAGATION_STYLE_B3_MULTI], + None, B3_SINGLE_HEADERS_VALID, CONTEXT_EMPTY, ), @@ -1863,6 +1911,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_all_headers_default_style", None, + None, ALL_HEADERS, { "trace_id": 13088165645273925489, @@ -1891,6 +1940,7 @@ def test_extract_tracecontext(headers, expected_context): PROPAGATION_STYLE_B3_SINGLE, _PROPAGATION_STYLE_W3C_TRACECONTEXT, ], + None, ALL_HEADERS, { "trace_id": 13088165645273925489, @@ -1931,6 +1981,7 @@ def test_extract_tracecontext(headers, expected_context): PROPAGATION_STYLE_B3_SINGLE, _PROPAGATION_STYLE_W3C_TRACECONTEXT, ], + None, {get_wsgi_header(name): value for name, value in ALL_HEADERS.items()}, { "trace_id": 13088165645273925489, @@ -1966,6 +2017,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_all_headers_datadog_style", [PROPAGATION_STYLE_DATADOG], + None, ALL_HEADERS, { "trace_id": 13088165645273925489, @@ -1978,6 +2030,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_all_headers_datadog_style_wsgi", [PROPAGATION_STYLE_DATADOG], + None, {get_wsgi_header(name): value for name, value in ALL_HEADERS.items()}, { "trace_id": 13088165645273925489, @@ -1990,6 +2043,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_all_headers_b3_style", [PROPAGATION_STYLE_B3_MULTI], + None, ALL_HEADERS, { "trace_id": TRACE_ID, @@ -2001,6 +2055,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_all_headers_b3_style_wsgi", [PROPAGATION_STYLE_B3_MULTI], + None, {get_wsgi_header(name): value for name, value in ALL_HEADERS.items()}, { "trace_id": TRACE_ID, @@ -2012,6 +2067,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_all_headers_both_b3_styles", [PROPAGATION_STYLE_B3_MULTI, PROPAGATION_STYLE_B3_SINGLE], + None, ALL_HEADERS, { "trace_id": TRACE_ID, @@ -2023,6 +2079,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_all_headers_b3_single_style", [PROPAGATION_STYLE_B3_SINGLE], + None, ALL_HEADERS, { "trace_id": TRACE_ID, @@ -2035,6 +2092,7 @@ def test_extract_tracecontext(headers, expected_context): # name, styles, headers, expected_context, "none_style", [_PROPAGATION_STYLE_NONE], + None, ALL_HEADERS, { "trace_id": None, @@ -2047,6 +2105,7 @@ def test_extract_tracecontext(headers, expected_context): # name, styles, headers, expected_context, "none_and_other_prop_style_still_extracts", [PROPAGATION_STYLE_DATADOG, _PROPAGATION_STYLE_NONE], + None, ALL_HEADERS, { "trace_id": 13088165645273925489, @@ -2060,6 +2119,7 @@ def test_extract_tracecontext(headers, expected_context): ( "order_matters_B3_SINGLE_HEADER_first", [PROPAGATION_STYLE_B3_SINGLE, PROPAGATION_STYLE_B3_MULTI, PROPAGATION_STYLE_DATADOG], + None, B3_SINGLE_HEADERS_VALID, { "trace_id": TRACE_ID, @@ -2076,6 +2136,7 @@ def test_extract_tracecontext(headers, expected_context): PROPAGATION_STYLE_DATADOG, _PROPAGATION_STYLE_W3C_TRACECONTEXT, ], + None, B3_HEADERS_VALID, { "trace_id": TRACE_ID, @@ -2087,6 +2148,7 @@ def test_extract_tracecontext(headers, expected_context): ( "order_matters_B3_second_no_Datadog_headers", [PROPAGATION_STYLE_DATADOG, PROPAGATION_STYLE_B3_MULTI], + None, B3_HEADERS_VALID, { "trace_id": TRACE_ID, @@ -2098,6 +2160,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_all_headers_b3_single_style_wsgi", [PROPAGATION_STYLE_B3_SINGLE], + None, {get_wsgi_header(name): value for name, value in ALL_HEADERS.items()}, { "trace_id": TRACE_ID, @@ -2116,6 +2179,7 @@ def test_extract_tracecontext(headers, expected_context): _PROPAGATION_STYLE_W3C_TRACECONTEXT, PROPAGATION_STYLE_B3_SINGLE, ], + None, DATADOG_TRACECONTEXT_MATCHING_TRACE_ID_HEADERS, { "trace_id": _get_64_lowest_order_bits_as_int(TRACE_ID), @@ -2133,6 +2197,7 @@ def test_extract_tracecontext(headers, expected_context): ( "no_additional_tracestate_support_when_present_but_trace_ids_do_not_match", [PROPAGATION_STYLE_DATADOG, _PROPAGATION_STYLE_W3C_TRACECONTEXT], + None, {**DATADOG_HEADERS_VALID, **TRACECONTEXT_HEADERS_VALID_RUM_NO_SAMPLING_DECISION}, { "trace_id": 13088165645273925489, @@ -2154,18 +2219,21 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_all_headers_no_style", [], + None, ALL_HEADERS, CONTEXT_EMPTY, ), ( "valid_all_headers_no_style_wsgi", [], + None, {get_wsgi_header(name): value for name, value in ALL_HEADERS.items()}, CONTEXT_EMPTY, ), ( "datadog_tracecontext_conflicting_span_ids", [PROPAGATION_STYLE_DATADOG, _PROPAGATION_STYLE_W3C_TRACECONTEXT], + None, { HTTP_HEADER_TRACE_ID: "9291375655657946024", HTTP_HEADER_PARENT_ID: "15", @@ -2178,6 +2246,142 @@ def test_extract_tracecontext(headers, expected_context): "meta": {"_dd.p.dm": "-3", LAST_DD_PARENT_ID_KEY: "000000000000000f"}, }, ), + ( + "valid_datadog_default_w_restart_behavior", + None, + _PROPAGATION_BEHAVIOR_RESTART, + DATADOG_HEADERS_VALID, + { + "trace_id": None, + "span_id": None, + "sampling_priority": None, + "dd_origin": None, + "span_links": [ + SpanLink( + trace_id=13088165645273925489, + span_id=5678, + tracestate=None, + flags=1, + attributes={"reason": "propagation_behavior_extract", "context_headers": "datadog"}, + ) + ], + }, + ), + ( + "valid_datadog_and_baggage_default_w_restart_behavior", + None, + _PROPAGATION_BEHAVIOR_RESTART, + DATADOG_BAGGAGE_HEADERS_VALID, + { + "trace_id": None, + "span_id": None, + "sampling_priority": None, + "dd_origin": None, + "baggage": {"key1": "val1", "key2": "val2"}, + "span_links": [ + SpanLink( + trace_id=13088165645273925489, + span_id=5678, + tracestate=None, + flags=1, + attributes={"reason": "propagation_behavior_extract", "context_headers": "datadog"}, + ) + ], + }, + ), + ( + "valid_datadog_default_w_ignore_behavior", + None, + _PROPAGATION_BEHAVIOR_IGNORE, + DATADOG_HEADERS_VALID, + CONTEXT_EMPTY, + ), + # All valid headers + ( + "valid_all_headers_default_style_w_restart_behavior", + None, + _PROPAGATION_BEHAVIOR_RESTART, + ALL_HEADERS, + { + "trace_id": None, + "span_id": None, + "sampling_priority": None, + "dd_origin": None, + "span_links": [ + SpanLink( + trace_id=13088165645273925489, + span_id=5678, + tracestate=None, + flags=1, + attributes={"reason": "propagation_behavior_extract", "context_headers": "datadog"}, + ) + ], + }, + ), + ( + "valid_all_headers_trace_context_datadog_style_w_restart_behavior", + [_PROPAGATION_STYLE_W3C_TRACECONTEXT, PROPAGATION_STYLE_DATADOG], + _PROPAGATION_BEHAVIOR_RESTART, + ALL_HEADERS, + { + "trace_id": None, + "span_id": None, + "sampling_priority": None, + "dd_origin": None, + "span_links": [ + SpanLink( + trace_id=171395628812617415352188477958425669623, + span_id=67667974448284343, + tracestate="dd=s:2;o:rum;t.dm:-4;t.usr.id:baz64,congo=t61rcWkgMzE", + flags=1, + attributes={"reason": "propagation_behavior_extract", "context_headers": "tracecontext"}, + ) + ], + }, + ), + ( + "valid_all_headers_all_styles_w_restart_behavior", + [PROPAGATION_STYLE_B3_MULTI, PROPAGATION_STYLE_B3_SINGLE, _PROPAGATION_STYLE_W3C_TRACECONTEXT], + _PROPAGATION_BEHAVIOR_RESTART, + ALL_HEADERS, + { + "trace_id": None, + "span_id": None, + "sampling_priority": None, + "dd_origin": None, + "span_links": [ + SpanLink( + trace_id=171395628812617415352188477958425669623, + span_id=67667974448284343, + tracestate=None, + flags=1, + attributes={"reason": "propagation_behavior_extract", "context_headers": "b3multi"}, + ) + ], + }, + ), + ( + "valid_all_headers_and_baggage_trace_context_datadog_style_w_restart_behavior", + None, + _PROPAGATION_BEHAVIOR_RESTART, + {**ALL_HEADERS, **DATADOG_BAGGAGE_HEADERS_VALID}, + { + "trace_id": None, + "span_id": None, + "sampling_priority": None, + "dd_origin": None, + "baggage": {"key1": "val1", "key2": "val2"}, + "span_links": [ + SpanLink( + trace_id=13088165645273925489, + span_id=5678, + tracestate=None, + flags=1, + attributes={"reason": "propagation_behavior_extract", "context_headers": "datadog"}, + ) + ], + }, + ), ] # Only add fixtures here if they can't pass both test_propagation_extract_env @@ -2188,6 +2392,7 @@ def test_extract_tracecontext(headers, expected_context): # can't be tested correctly via test_propagation_extract_w_config. It is tested separately "valid_tracecontext_simple", [_PROPAGATION_STYLE_W3C_TRACECONTEXT], + None, TRACECONTEXT_HEADERS_VALID_BASIC, { "trace_id": TRACE_ID, @@ -2204,6 +2409,7 @@ def test_extract_tracecontext(headers, expected_context): ( "valid_tracecontext_rum_no_sampling_decision", [_PROPAGATION_STYLE_W3C_TRACECONTEXT], + None, TRACECONTEXT_HEADERS_VALID_RUM_NO_SAMPLING_DECISION, { "trace_id": TRACE_ID, @@ -2218,8 +2424,12 @@ def test_extract_tracecontext(headers, expected_context): ] -@pytest.mark.parametrize("name,styles,headers,expected_context", EXTRACT_FIXTURES + EXTRACT_FIXTURES_ENV_ONLY) -def test_propagation_extract_env(name, styles, headers, expected_context, run_python_code_in_subprocess): +@pytest.mark.parametrize( + "name,styles,extract_behavior,headers,expected_context", EXTRACT_FIXTURES + EXTRACT_FIXTURES_ENV_ONLY +) +def test_propagation_extract_env( + name, styles, extract_behavior, headers, expected_context, run_python_code_in_subprocess +): # Execute the test code in isolation to ensure env variables work as expected code = """ import json @@ -2237,18 +2447,24 @@ def test_propagation_extract_env(name, styles, headers, expected_context, run_py env = os.environ.copy() if styles is not None: env["DD_TRACE_PROPAGATION_STYLE"] = ",".join(styles) + if extract_behavior is not None: + env["DD_TRACE_PROPAGATION_BEHAVIOR_EXTRACT"] = extract_behavior stdout, stderr, status, _ = run_python_code_in_subprocess(code=code, env=env) print(stderr, stdout) assert status == 0, (stdout, stderr) -@pytest.mark.parametrize("name,styles,headers,expected_context", EXTRACT_FIXTURES) -def test_propagation_extract_w_config(name, styles, headers, expected_context, run_python_code_in_subprocess): +@pytest.mark.parametrize("name,styles,extract_behavior,headers,expected_context", EXTRACT_FIXTURES) +def test_propagation_extract_w_config( + name, styles, extract_behavior, headers, expected_context, run_python_code_in_subprocess +): # Setting via ddtrace.config works as expected too # DEV: This also helps us get code coverage reporting overrides = {} if styles is not None: overrides["_propagation_style_extract"] = styles + if extract_behavior is not None: + overrides["_propagation_behavior_extract"] = extract_behavior with override_global_config(overrides): context = HTTPPropagator.extract(headers) if not expected_context.get("tracestate"): From 8ce6f80b3b30651041058d21c64c1babafb50eef Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Fri, 20 Dec 2024 15:21:20 -0500 Subject: [PATCH 05/17] allow override of config --- tests/utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/utils.py b/tests/utils.py index de0129f75a3..5b98fe42d1b 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -123,6 +123,7 @@ def override_global_config(values): "_health_metrics_enabled", "_propagation_style_extract", "_propagation_style_inject", + "_propagation_behavior_extract", "_x_datadog_tags_max_length", "_128_bit_trace_id_enabled", "_x_datadog_tags_enabled", From ffd8fe2e709db97c7e81f6f12595af702e4343cb Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Fri, 20 Dec 2024 15:43:28 -0500 Subject: [PATCH 06/17] add ref for system-tests --- .github/workflows/system-tests.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/system-tests.yml b/.github/workflows/system-tests.yml index ccf6c6501d9..c525c1e5978 100644 --- a/.github/workflows/system-tests.yml +++ b/.github/workflows/system-tests.yml @@ -20,6 +20,7 @@ jobs: with: persist-credentials: false repository: 'DataDog/system-tests' + ref: 'zach.montoya/poc/propagation-behavior' - name: Build agent run: ./build.sh -i agent @@ -65,6 +66,7 @@ jobs: with: persist-credentials: false repository: 'DataDog/system-tests' + ref: 'zach.montoya/poc/propagation-behavior' - name: Checkout dd-trace-py uses: actions/checkout@v4 @@ -117,6 +119,8 @@ jobs: with: persist-credentials: false repository: 'DataDog/system-tests' + ref: 'zach.montoya/poc/propagation-behavior' + - name: Build runner uses: ./.github/actions/install_runner @@ -290,6 +294,7 @@ jobs: with: persist-credentials: false repository: 'DataDog/system-tests' + ref: 'zach.montoya/poc/propagation-behavior' - name: Checkout dd-trace-py uses: actions/checkout@v4 with: From 290d9e66a34368da983c2068d935d76c8d9e358e Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Fri, 20 Dec 2024 17:24:28 -0500 Subject: [PATCH 07/17] add tracecontext headers to default case --- tests/tracer/test_propagation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/tracer/test_propagation.py b/tests/tracer/test_propagation.py index 150dbb1b704..4e330ccb886 100644 --- a/tests/tracer/test_propagation.py +++ b/tests/tracer/test_propagation.py @@ -2297,10 +2297,10 @@ def test_extract_tracecontext(headers, expected_context): }, ), ( - "valid_datadog_and_baggage_default_w_restart_behavior", + "valid_datadog_tracecontext_and_baggage_default_w_restart_behavior", None, _PROPAGATION_BEHAVIOR_RESTART, - DATADOG_BAGGAGE_HEADERS_VALID, + {**DATADOG_BAGGAGE_HEADERS_VALID, **TRACECONTEXT_HEADERS_VALID}, { "trace_id": None, "span_id": None, From 87532da8eddf1bd23ecab235bbc38f5afca4c22e Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Tue, 7 Jan 2025 14:52:42 -0500 Subject: [PATCH 08/17] don't drop extract contexts lacking trace_id --- ddtrace/contrib/trace_utils.py | 4 ++-- tests/tracer/test_propagation.py | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/ddtrace/contrib/trace_utils.py b/ddtrace/contrib/trace_utils.py index db8509d8c35..740ae9288b7 100644 --- a/ddtrace/contrib/trace_utils.py +++ b/ddtrace/contrib/trace_utils.py @@ -565,7 +565,7 @@ def activate_distributed_headers(tracer, int_config=None, request_headers=None, context = HTTPPropagator.extract(request_headers) # Only need to activate the new context if something was propagated - if not context.trace_id: + if not context.trace_id or context._baggage: return None # Do not reactivate a context with the same trace id @@ -577,7 +577,7 @@ def activate_distributed_headers(tracer, int_config=None, request_headers=None, # app = Flask(__name__) # Traced via Flask instrumentation # app = DDWSGIMiddleware(app) # Extra layer on top for WSGI current_context = tracer.current_trace_context() - if current_context and current_context.trace_id == context.trace_id: + if current_context and current_context.trace_id and current_context.trace_id == context.trace_id: log.debug( "will not activate extracted Context(trace_id=%r, span_id=%r), a context with that trace id is already active", # noqa: E501 context.trace_id, diff --git a/tests/tracer/test_propagation.py b/tests/tracer/test_propagation.py index 4e330ccb886..b2bd438032e 100644 --- a/tests/tracer/test_propagation.py +++ b/tests/tracer/test_propagation.py @@ -2494,14 +2494,14 @@ def test_propagation_extract_w_config( overrides["_propagation_style_extract"] = styles if extract_behavior is not None: overrides["_propagation_behavior_extract"] = extract_behavior - with override_global_config(overrides): - context = HTTPPropagator.extract(headers) - if not expected_context.get("tracestate"): - assert context == Context(**expected_context) - else: - copied_expectation = expected_context.copy() - tracestate = copied_expectation.pop("tracestate") - assert context == Context(**copied_expectation, meta={"tracestate": tracestate}) + with override_global_config(overrides): + context = HTTPPropagator.extract(headers) + if not expected_context.get("tracestate"): + assert context == Context(**expected_context) + else: + copied_expectation = expected_context.copy() + tracestate = copied_expectation.pop("tracestate") + assert context == Context(**copied_expectation, meta={"tracestate": tracestate}) EXTRACT_OVERRIDE_FIXTURES = [ From b32a467c76294494a5c6349075b0fada06765650 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Wed, 8 Jan 2025 17:25:43 -0500 Subject: [PATCH 09/17] fix extracting baggage and update so that baggage only context is used --- ddtrace/contrib/trace_utils.py | 4 +--- ddtrace/propagation/http.py | 3 ++- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/ddtrace/contrib/trace_utils.py b/ddtrace/contrib/trace_utils.py index 740ae9288b7..f168f7e2717 100644 --- a/ddtrace/contrib/trace_utils.py +++ b/ddtrace/contrib/trace_utils.py @@ -560,14 +560,12 @@ def activate_distributed_headers(tracer, int_config=None, request_headers=None, """ if override is False: return None - if override or (int_config and distributed_tracing_enabled(int_config)): context = HTTPPropagator.extract(request_headers) # Only need to activate the new context if something was propagated - if not context.trace_id or context._baggage: + if not context.trace_id and not context._baggage: return None - # Do not reactivate a context with the same trace id # DEV: An example could be nested web frameworks, when one layer already # parsed request headers and activated them. diff --git a/ddtrace/propagation/http.py b/ddtrace/propagation/http.py index 6d7cd8325ab..cae4d97a0b9 100644 --- a/ddtrace/propagation/http.py +++ b/ddtrace/propagation/http.py @@ -103,6 +103,7 @@ def _possible_header(header): _POSSIBLE_HTTP_HEADER_B3_FLAGS = _possible_header(_HTTP_HEADER_B3_FLAGS) _POSSIBLE_HTTP_HEADER_TRACEPARENT = _possible_header(_HTTP_HEADER_TRACEPARENT) _POSSIBLE_HTTP_HEADER_TRACESTATE = _possible_header(_HTTP_HEADER_TRACESTATE) +_POSSIBLE_HTTP_BAGGAGE_HEADER = _possible_header(_HTTP_HEADER_BAGGAGE) # https://www.w3.org/TR/trace-context/#traceparent-header-field-values @@ -939,7 +940,7 @@ def _inject(span_context: Context, headers: Dict[str, str]) -> None: @staticmethod def _extract(headers: Dict[str, str]) -> Context: - header_value = headers.get(_HTTP_HEADER_BAGGAGE) + header_value = _extract_header_value(_POSSIBLE_HTTP_BAGGAGE_HEADER, headers) if not header_value: return Context(baggage={}) From 837e2136b9e88e738e20b724de76cadbfcee17e5 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Thu, 9 Jan 2025 13:29:56 -0500 Subject: [PATCH 10/17] fix no span on execution context error --- ddtrace/contrib/trace_utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ddtrace/contrib/trace_utils.py b/ddtrace/contrib/trace_utils.py index f168f7e2717..5694ff63007 100644 --- a/ddtrace/contrib/trace_utils.py +++ b/ddtrace/contrib/trace_utils.py @@ -575,7 +575,10 @@ def activate_distributed_headers(tracer, int_config=None, request_headers=None, # app = Flask(__name__) # Traced via Flask instrumentation # app = DDWSGIMiddleware(app) # Extra layer on top for WSGI current_context = tracer.current_trace_context() - if current_context and current_context.trace_id and current_context.trace_id == context.trace_id: + + # We accept incoming contexts with only baggage, however if we + # already have a current_context then a baggage only context will be tossed out + if current_context and (not context.trace_id or current_context.trace_id == context.trace_id): log.debug( "will not activate extracted Context(trace_id=%r, span_id=%r), a context with that trace id is already active", # noqa: E501 context.trace_id, From 44368964881079a2211fe96de512c90268b05008 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Thu, 9 Jan 2025 14:02:35 -0500 Subject: [PATCH 11/17] cover case of only span_link --- ddtrace/contrib/trace_utils.py | 7 ++++--- tests/tracer/test_propagation.py | 1 + 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/ddtrace/contrib/trace_utils.py b/ddtrace/contrib/trace_utils.py index 5694ff63007..75a44d3db59 100644 --- a/ddtrace/contrib/trace_utils.py +++ b/ddtrace/contrib/trace_utils.py @@ -564,7 +564,8 @@ def activate_distributed_headers(tracer, int_config=None, request_headers=None, context = HTTPPropagator.extract(request_headers) # Only need to activate the new context if something was propagated - if not context.trace_id and not context._baggage: + # The new context must have one of these values in order for it to be activated + if not context.trace_id and not context._baggage and not context._span_links: return None # Do not reactivate a context with the same trace id # DEV: An example could be nested web frameworks, when one layer already @@ -576,8 +577,8 @@ def activate_distributed_headers(tracer, int_config=None, request_headers=None, # app = DDWSGIMiddleware(app) # Extra layer on top for WSGI current_context = tracer.current_trace_context() - # We accept incoming contexts with only baggage, however if we - # already have a current_context then a baggage only context will be tossed out + # We accept incoming contexts with only baggage or only span_links, however if we + # already have a current_context then a baggage only or span_link only context will be tossed out if current_context and (not context.trace_id or current_context.trace_id == context.trace_id): log.debug( "will not activate extracted Context(trace_id=%r, span_id=%r), a context with that trace id is already active", # noqa: E501 diff --git a/tests/tracer/test_propagation.py b/tests/tracer/test_propagation.py index 7caab0c8aff..b6368c71140 100644 --- a/tests/tracer/test_propagation.py +++ b/tests/tracer/test_propagation.py @@ -1939,6 +1939,7 @@ def test_extract_tracecontext(headers, expected_context): ( "baggage_case_insensitive", None, + None, {"BAgGage": "key1=val1,key2=val2"}, { "baggage": {"key1": "val1", "key2": "val2"}, From 57b32392422214d3423ea95c5dfac7bf6490ec95 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Thu, 9 Jan 2025 16:01:25 -0500 Subject: [PATCH 12/17] make sure span_link always added to root_span --- ddtrace/_trace/tracer.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ddtrace/_trace/tracer.py b/ddtrace/_trace/tracer.py index af9b09d3e02..4486e14cd7e 100644 --- a/ddtrace/_trace/tracer.py +++ b/ddtrace/_trace/tracer.py @@ -773,10 +773,8 @@ def _start_span( # Update the service name based on any mapping service = config.service_mapping.get(service, service) - links = context._span_links if not parent else [] - - if trace_id: + if trace_id or links: # child_of a non-empty context, so either a local child span or from a remote context span = Span( name=name, From a0b2c34532a6bd88604808d69c36ccf98e8b855e Mon Sep 17 00:00:00 2001 From: Zachary Groves <32471391+ZStriker19@users.noreply.github.com> Date: Fri, 10 Jan 2025 13:12:31 -0500 Subject: [PATCH 13/17] Update ddtrace/_trace/tracer.py --- ddtrace/_trace/tracer.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ddtrace/_trace/tracer.py b/ddtrace/_trace/tracer.py index 4486e14cd7e..aa891f289a1 100644 --- a/ddtrace/_trace/tracer.py +++ b/ddtrace/_trace/tracer.py @@ -773,6 +773,7 @@ def _start_span( # Update the service name based on any mapping service = config.service_mapping.get(service, service) + links = context._span_links if not parent else [] if trace_id or links: # child_of a non-empty context, so either a local child span or from a remote context From cf57ae8f4b8900eda126298532b90a47a74244df Mon Sep 17 00:00:00 2001 From: Zachary Groves <32471391+ZStriker19@users.noreply.github.com> Date: Fri, 10 Jan 2025 13:13:26 -0500 Subject: [PATCH 14/17] Update ddtrace/contrib/trace_utils.py --- ddtrace/contrib/trace_utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ddtrace/contrib/trace_utils.py b/ddtrace/contrib/trace_utils.py index 75a44d3db59..326074ea69d 100644 --- a/ddtrace/contrib/trace_utils.py +++ b/ddtrace/contrib/trace_utils.py @@ -560,6 +560,7 @@ def activate_distributed_headers(tracer, int_config=None, request_headers=None, """ if override is False: return None + if override or (int_config and distributed_tracing_enabled(int_config)): context = HTTPPropagator.extract(request_headers) From d5293e08e7a715fab68f8270e390e43c6a68b31a Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Fri, 10 Jan 2025 13:43:49 -0500 Subject: [PATCH 15/17] use public api to access baggage --- ddtrace/propagation/http.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddtrace/propagation/http.py b/ddtrace/propagation/http.py index cae4d97a0b9..681ce5436d8 100644 --- a/ddtrace/propagation/http.py +++ b/ddtrace/propagation/http.py @@ -1175,7 +1175,7 @@ def my_controller(url, headers): baggage_context = _BaggageHeader._extract(normalized_headers) if baggage_context._baggage != {}: if context: - context._baggage = baggage_context._baggage + context._baggage = baggage_context.get_all_baggage_items() else: context = baggage_context if config._propagation_behavior_extract == _PROPAGATION_BEHAVIOR_RESTART: From af2c94fa807670110a14a5fa7636456cb25f625d Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Fri, 10 Jan 2025 13:45:03 -0500 Subject: [PATCH 16/17] fix merge conflict --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 855c67e36f9..df5fbdcdbb2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -74,7 +74,7 @@ Homepage = "https://github.com/DataDog/dd-trace-py" "Source Code" = "https://github.com/DataDog/dd-trace-py/" [tool.setuptools_scm] -version_scheme = "guess-next-dev" # Must be "release-branch-semver" for now in main, see https://github.com/DataDog/dd-trace-py/issues/8801 +version_scheme = "release-branch-semver" # Must be "release-branch-semver" for now in main, see https://github.com/DataDog/dd-trace-py/issues/8801 write_to = "ddtrace/_version.py" [tool.cython-lint] From 37117d2c2fbd5eb44061a0b6250e46f63b6a28c7 Mon Sep 17 00:00:00 2001 From: ZStriker19 Date: Fri, 10 Jan 2025 14:06:54 -0500 Subject: [PATCH 17/17] docs and update rn --- docs/configuration.rst | 20 +++++++++++++++++++ ...ion_behavior_extract-3d16765cfd07485b.yaml | 5 +++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/docs/configuration.rst b/docs/configuration.rst index 35bb63fac20..455272f318d 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -368,6 +368,26 @@ Trace Context propagation version_added: v1.7.0: The ``b3multi`` propagation style was added and ``b3`` was deprecated in favor it. + DD_TRACE_PROPAGATION_BEHAVIOR_EXTRACT: + default: | + ``continue`` + + description: | + String for how to handle incoming request headers that are extracted for propagation of trace info. + + The supported values are ``continue``, ``restart``, and ``ignore``. + + After extracting the headers for propagation, this configuration determines what is done with them. + + The default value is ``continue`` which always propagates valid headers. + ``ignore`` ignores all incoming headers and ``restart`` turns the first extracted valid propagation header + into a span link and propagates baggage if present. + + Example: ``DD_TRACE_PROPAGATION_STYLE_EXTRACT="ignore"`` to ignore all incoming headers and to start a root span without a parent. + + version_added: + v2.20.0: + DD_TRACE_PROPAGATION_STYLE_INJECT: default: | ``tracecontext,datadog`` diff --git a/releasenotes/notes/propagation_behavior_extract-3d16765cfd07485b.yaml b/releasenotes/notes/propagation_behavior_extract-3d16765cfd07485b.yaml index 8e34092a6bb..9aa0f5ce208 100644 --- a/releasenotes/notes/propagation_behavior_extract-3d16765cfd07485b.yaml +++ b/releasenotes/notes/propagation_behavior_extract-3d16765cfd07485b.yaml @@ -3,5 +3,6 @@ features: - | propagation: This introduces the environment variable ``DD_TRACE_PROPAGATION_BEHAVIOR_EXTRACT`` to control the behavior of the extraction of distributed tracing headers. The values, ``continue`` (default), - ``ignore``, and ``restart``, are supported. The default value is ``continue`` which should have no change from the current behavior of always propagating valid headers. - With ``ignore`` ignoring all incoming headers and with ``restart`` turning incoming headers into a span links and propagating baggage items. + ``ignore``, and ``restart``, are supported. The default value is ``continue`` which has no change from the current behavior of always propagating valid headers. + ``ignore`` ignores all incoming headers, never propagating the incoming trace information + and ``restart`` turns the first extracted propagation style into a span link and propagates baggage if extracted.