fix(tracer): fix sampling mechanism tag propagation bug (#6579)

This change fixes a bug causing `_dd.p.*` span tags not to be inherited by child spans when there is a process boundary between the parent and child spans. This makes cross-process parenting match the behavior of parenting within a single process with respect to tag inheritance. This change updates all snapshot files and test expectations affected by this change to include expectation of all relevant `_dd.p.*` tags. There are also two style changes that were required to get the CI pre_check step to pass (I'm not sure why that isn't appearing on 1.x).
DataDog · Aug 8, 2023 · 5183394 · 5183394
1 parent b730396
commit 5183394
Show file tree

Hide file tree

Showing 249 changed files with 4,552 additions and 3,410 deletions.
diff --git a/ddtrace/internal/utils/__init__.py b/ddtrace/internal/utils/__init__.py
@@ -4,6 +4,8 @@
 from typing import Optional
 from typing import Tuple
 
+import six
+
 
 class ArgumentError(Exception):
     """
@@ -67,3 +69,12 @@ def set_argument_value(
         raise ArgumentError("%s (at position %d) is invalid" % (kw, pos))
 
     return args, kwargs
+
+
+def _get_metas_to_propagate(context):
+    # type: (Any) -> List[Tuple[str, str]]
+    metas_to_propagate = []
+    for k, v in context._meta.items():
+        if isinstance(k, six.string_types) and k.startswith("_dd.p."):
+            metas_to_propagate.append((k, v))
+    return metas_to_propagate
diff --git a/ddtrace/internal/utils/http.py b/ddtrace/internal/utils/http.py
@@ -12,8 +12,6 @@
 from typing import Tuple
 from typing import Union
 
-import six
-
 from ddtrace.constants import USER_ID_KEY
 from ddtrace.internal import compat
 from ddtrace.internal.compat import parse
@@ -25,6 +23,7 @@
 from ddtrace.internal.http import HTTPConnection
 from ddtrace.internal.http import HTTPSConnection
 from ddtrace.internal.uds import UDSHTTPConnection
+from ddtrace.internal.utils import _get_metas_to_propagate
 from ddtrace.internal.utils.cache import cached
 
 
@@ -173,13 +172,8 @@ def w3c_get_dd_list_member(context):
         tags.append("t.usr.id:{}".format(w3c_encode_tag((_W3C_TRACESTATE_INVALID_CHARS_REGEX_VALUE, "_", usr_id))))
 
     current_tags_len = sum(len(i) for i in tags)
-    for k, v in context._meta.items():
-        if (
-            isinstance(k, six.string_types)
-            and k.startswith("_dd.p.")
-            # we've already added sampling decision and user id
-            and k not in [SAMPLING_DECISION_TRACE_TAG_KEY, USER_ID_KEY]
-        ):
+    for k, v in _get_metas_to_propagate(context):
+        if k not in [SAMPLING_DECISION_TRACE_TAG_KEY, USER_ID_KEY]:
             # for key replace ",", "=", and characters outside the ASCII range 0x20 to 0x7E
             # for value replace ",", ";", "~" and characters outside the ASCII range 0x20 to 0x7E
             k = k.replace("_dd.p.", "t.")

diff --git a/ddtrace/tracer.py b/ddtrace/tracer.py
@@ -15,6 +15,7 @@
 from ddtrace.internal.processor.endpoint_call_counter import EndpointCallCounterProcessor
 from ddtrace.internal.sampling import SpanSamplingRule
 from ddtrace.internal.sampling import get_span_sampling_rules
+from ddtrace.internal.utils import _get_metas_to_propagate
 from ddtrace.settings.peer_service import PeerServiceConfig
 from ddtrace.vendor import debtcollector
 
@@ -718,6 +719,8 @@ def _start_span(
 
             if span._local_root is None:
                 span._local_root = span
+            for k, v in _get_metas_to_propagate(context):
+                span._meta[k] = v
         else:
             # this is the root span of a new trace
             span = Span(

diff --git a/releasenotes/notes/tag-inheritance-interprocess-bc5a2bd188fc08db.yaml b/releasenotes/notes/tag-inheritance-interprocess-bc5a2bd188fc08db.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    This fix resolves an issue causing span tags used by the Datadog backend not to be inherited by spans
+    that exist in a different process from their parents.
diff --git a/tests/contrib/botocore/test.py b/tests/contrib/botocore/test.py
@@ -2853,7 +2853,7 @@ def test_kinesis_put_records_json_trace_injection(self):
     @mock_kinesis
     @TracerTestCase.run_in_subprocess(env_overrides=dict(DD_DATA_STREAMS_ENABLED="True"))
     @unittest.skipIf(BOTOCORE_VERSION < (1, 26, 31), "Kinesis didn't support streamARN till 1.26.31")
-    @mock.patch('time.time', mock.MagicMock(return_value=1642544540))
+    @mock.patch("time.time", mock.MagicMock(return_value=1642544540))
     def test_kinesis_data_streams_enabled_put_records(self):
         # (dict -> json string)[]
         data = json.dumps({"json": "string"})

diff --git a/tests/contrib/django/test_django.py b/tests/contrib/django/test_django.py
@@ -1218,6 +1218,7 @@ def test_cache_get_many(test_spans):
         "component": "django",
         "django.cache.backend": "django.core.cache.backends.locmem.LocMemCache",
         "django.cache.key": "missing_key another_key",
+        "_dd.p.dm": "-0",
     }
 
     assert_dict_issuperset(span_get_many.get_tags(), expected_meta)
@@ -1419,12 +1420,14 @@ def test_cached_view(client, test_spans):
         "django.cache.key": (
             "views.decorators.cache.cache_page..GET.03cdc1cc4aab71b038a6764e5fcabb82.d41d8cd98f00b204e9800998ecf8..."
         ),
+        "_dd.p.dm": "-0",
     }
 
     expected_meta_header = {
         "component": "django",
         "django.cache.backend": "django.core.cache.backends.locmem.LocMemCache",
         "django.cache.key": "views.decorators.cache.cache_header..03cdc1cc4aab71b038a6764e5fcabb82.en-us",
+        "_dd.p.dm": "-0",
     }
 
     assert span_view.get_tags() == expected_meta_view
@@ -1463,6 +1466,7 @@ def test_cached_template(client, test_spans):
         "component": "django",
         "django.cache.backend": "django.core.cache.backends.locmem.LocMemCache",
         "django.cache.key": "template.cache.users_list.d41d8cd98f00b204e9800998ecf8427e",
+        "_dd.p.dm": "-0",
     }
 
     assert span_template_cache.get_tags() == expected_meta

diff --git a/tests/contrib/flask/test_blueprint.py b/tests/contrib/flask/test_blueprint.py
@@ -103,7 +103,7 @@ def test():
         self.assertEqual(span.service, "flask")
         self.assertEqual(span.name, "bp.test")
         self.assertEqual(span.resource, "/")
-        self.assertEqual(span.get_tags(), {"component": "flask"})
+        self.assertEqual(span.get_tags(), {"component": "flask", "_dd.p.dm": "-0"})
 
     def test_blueprint_request_pin_override(self):
         """
@@ -130,7 +130,7 @@ def test():
         self.assertEqual(span.service, "flask-bp")
         self.assertEqual(span.name, "bp.test")
         self.assertEqual(span.resource, "/")
-        self.assertEqual(span.get_tags(), {"component": "flask"})
+        self.assertEqual(span.get_tags(), {"component": "flask", "_dd.p.dm": "-0"})
 
     def test_blueprint_request_pin_disabled(self):
         """

diff --git a/tests/contrib/flask/test_errorhandler.py b/tests/contrib/flask/test_errorhandler.py
@@ -6,7 +6,7 @@
 from . import BaseFlaskTestCase
 
 
-EXPECTED_METADATA = {"component": "flask"}
+EXPECTED_METADATA = {"component": "flask", "_dd.p.dm": "-0"}
 
 
 class FlaskErrorhandlerTestCase(BaseFlaskTestCase):

diff --git a/tests/contrib/flask/test_views.py b/tests/contrib/flask/test_views.py
@@ -13,7 +13,7 @@
 if PY2:
     base_exception_name = "exceptions.Exception"
 
-EXPECTED_METADATA = {"component": "flask"}
+EXPECTED_METADATA = {"component": "flask", "_dd.p.dm": "-0"}
 
 
 class FlaskViewTestCase(BaseFlaskTestCase):

diff --git a/tests/contrib/patch.py b/tests/contrib/patch.py
@@ -7,8 +7,8 @@
 import unittest
 
 from ddtrace.vendor import wrapt
-from tests.subprocesstest import run_in_subprocess
 from tests.subprocesstest import SubprocessTestCase
+from tests.subprocesstest import run_in_subprocess
 from tests.utils import call_program
 
 

diff --git a/tests/snapshots/test_sampling_with_default_sample_rate_1.json b/tests/snapshots/test_sampling_with_default_sample_rate_1.json
@@ -11,17 +11,17 @@
     "meta": {
       "_dd.p.dm": "-3",
       "language": "python",
-      "runtime-id": "920bc049d045440d84bcac9f8e5d75ac"
+      "runtime-id": "d5f3e666fe79454c90ec7063f44787b5"
     },
     "metrics": {
       "_dd.rule_psr": 1.0,
       "_dd.top_level": 1,
       "_dd.tracer_kr": 1.0,
       "_sampling_priority_v1": 2,
-      "process_id": 17067
+      "process_id": 19794
     },
-    "duration": 77321,
-    "start": 1690576754754026969
+    "duration": 126664,
+    "start": 1691163616727396529
   },
      {
        "name": "child",
@@ -32,6 +32,9 @@
        "parent_id": 1,
        "type": "",
        "error": 0,
-       "duration": 7840,
-       "start": 1690576754754081258
+       "meta": {
+         "_dd.p.dm": "-3"
+       },
+       "duration": 10976,
+       "start": 1691163616727499421
      }]]
diff --git a/tests/snapshots/test_sampling_with_default_sample_rate_1_and_manual_drop.json b/tests/snapshots/test_sampling_with_default_sample_rate_1_and_manual_drop.json
@@ -10,17 +10,17 @@
     "error": 0,
     "meta": {
       "language": "python",
-      "runtime-id": "920bc049d045440d84bcac9f8e5d75ac"
+      "runtime-id": "d5f3e666fe79454c90ec7063f44787b5"
     },
     "metrics": {
       "_dd.rule_psr": 1,
       "_dd.top_level": 1,
       "_dd.tracer_kr": 1.0,
       "_sampling_priority_v1": -1,
-      "process_id": 17067
+      "process_id": 19794
     },
-    "duration": 81861,
-    "start": 1690576754809505288
+    "duration": 79749,
+    "start": 1691163616781341134
   },
      {
        "name": "child",
@@ -31,6 +31,9 @@
        "parent_id": 1,
        "type": "",
        "error": 0,
-       "duration": 15163,
-       "start": 1690576754809561072
+       "meta": {
+         "_dd.p.dm": "-3"
+       },
+       "duration": 16195,
+       "start": 1691163616781394547
      }]]
diff --git a/tests/snapshots/test_sampling_with_default_sample_rate_1_and_manual_keep.json b/tests/snapshots/test_sampling_with_default_sample_rate_1_and_manual_keep.json
@@ -11,17 +11,17 @@
     "meta": {
       "_dd.p.dm": "-4",
       "language": "python",
-      "runtime-id": "920bc049d045440d84bcac9f8e5d75ac"
+      "runtime-id": "d5f3e666fe79454c90ec7063f44787b5"
     },
     "metrics": {
       "_dd.rule_psr": 1,
       "_dd.top_level": 1,
       "_dd.tracer_kr": 1.0,
       "_sampling_priority_v1": 2,
-      "process_id": 17067
+      "process_id": 19794
     },
-    "duration": 85568,
-    "start": 1690576754822418944
+    "duration": 76557,
+    "start": 1691163616795561784
   },
      {
        "name": "child",
@@ -32,6 +32,9 @@
        "parent_id": 1,
        "type": "",
        "error": 0,
-       "duration": 13959,
-       "start": 1690576754822480449
+       "meta": {
+         "_dd.p.dm": "-3"
+       },
+       "duration": 15852,
+       "start": 1691163616795612344
      }]]
diff --git a/tests/snapshots/test_sampling_with_default_sample_rate_1_and_rule_0.json b/tests/snapshots/test_sampling_with_default_sample_rate_1_and_rule_0.json
@@ -10,17 +10,17 @@
     "error": 0,
     "meta": {
       "language": "python",
-      "runtime-id": "920bc049d045440d84bcac9f8e5d75ac"
+      "runtime-id": "d5f3e666fe79454c90ec7063f44787b5"
     },
     "metrics": {
       "_dd.rule_psr": 0,
       "_dd.top_level": 1,
       "_dd.tracer_kr": 1.0,
       "_sampling_priority_v1": -1,
-      "process_id": 17067
+      "process_id": 19794
     },
-    "duration": 84947,
-    "start": 1690576754794142563
+    "duration": 98844,
+    "start": 1691163616766926118
   },
      {
        "name": "child",
@@ -31,6 +31,6 @@
        "parent_id": 1,
        "type": "",
        "error": 0,
-       "duration": 15550,
-       "start": 1690576754794200656
+       "duration": 11972,
+       "start": 1691163616766999066
      }]]
diff --git a/tests/snapshots/test_sampling_with_default_sample_rate_1_and_rule_1.json b/tests/snapshots/test_sampling_with_default_sample_rate_1_and_rule_1.json
@@ -11,17 +11,17 @@
     "meta": {
       "_dd.p.dm": "-3",
       "language": "python",
-      "runtime-id": "920bc049d045440d84bcac9f8e5d75ac"
+      "runtime-id": "d5f3e666fe79454c90ec7063f44787b5"
     },
     "metrics": {
       "_dd.rule_psr": 1.0,
       "_dd.top_level": 1,
       "_dd.tracer_kr": 1.0,
       "_sampling_priority_v1": 2,
-      "process_id": 17067
+      "process_id": 19794
     },
-    "duration": 66501,
-    "start": 1690576754779959707
+    "duration": 69943,
+    "start": 1691163616754727901
   },
      {
        "name": "child",
@@ -32,6 +32,9 @@
        "parent_id": 1,
        "type": "",
        "error": 0,
-       "duration": 8155,
-       "start": 1690576754780008088
+       "meta": {
+         "_dd.p.dm": "-3"
+       },
+       "duration": 9551,
+       "start": 1691163616754778565
      }]]
diff --git a/tests/snapshots/test_sampling_with_default_sample_rate_tiny.json b/tests/snapshots/test_sampling_with_default_sample_rate_tiny.json
@@ -10,17 +10,17 @@
     "error": 0,
     "meta": {
       "language": "python",
-      "runtime-id": "920bc049d045440d84bcac9f8e5d75ac"
+      "runtime-id": "d5f3e666fe79454c90ec7063f44787b5"
     },
     "metrics": {
       "_dd.rule_psr": 1e-06,
       "_dd.top_level": 1,
       "_dd.tracer_kr": 1.0,
       "_sampling_priority_v1": -1,
-      "process_id": 17067
+      "process_id": 19794
     },
-    "duration": 70470,
-    "start": 1690576754767664582
+    "duration": 69145,
+    "start": 1691163616740768458
   },
      {
        "name": "child",
@@ -31,6 +31,6 @@
        "parent_id": 1,
        "type": "",
        "error": 0,
-       "duration": 7822,
-       "start": 1690576754767717272
+       "duration": 9098,
+       "start": 1691163616740817917
      }]]
diff --git a/tests/snapshots/test_sampling_with_defaults.json b/tests/snapshots/test_sampling_with_defaults.json
@@ -11,17 +11,17 @@
     "meta": {
       "_dd.p.dm": "-0",
       "language": "python",
-      "runtime-id": "920bc049d045440d84bcac9f8e5d75ac"
+      "runtime-id": "d5f3e666fe79454c90ec7063f44787b5"
     },
     "metrics": {
       "_dd.agent_psr": 1.0,
       "_dd.top_level": 1,
       "_dd.tracer_kr": 1.0,
       "_sampling_priority_v1": 1,
-      "process_id": 17067
+      "process_id": 19794
     },
-    "duration": 76297,
-    "start": 1690576754721987440
+    "duration": 86588,
+    "start": 1691163616713154842
   },
      {
        "name": "child",
@@ -32,6 +32,9 @@
        "parent_id": 1,
        "type": "",
        "error": 0,
-       "duration": 8418,
-       "start": 1690576754722037863
+       "meta": {
+         "_dd.p.dm": "-0"
+       },
+       "duration": 10853,
+       "start": 1691163616713219858
      }]]