fix(aioredis): correctly parent commands with aioredis 1.3 (#3147) (#3151)

When using aioredis 1.3, the Redis command spans are not correctly treated as children of a parent span. To fix this, we capture and set the parent span when the span is created.

fixes #3146

(cherry picked from commit a03be84)

Co-authored-by: Jameel Al-Aziz <[email protected]>

Author: mergify[bot]

ddtrace/contrib/aioredis/patch.py

@@ -110,8 +110,13 @@ def traced_13_execute_command(func, instance, args, kwargs):
     # Don't activate the span since this operation is performed as a future which concludes sometime later on in
     # execution so subsequent operations in the stack are not necessarily semantically related
     # (we don't want this span to be the parent of all other spans created before the future is resolved)
+    parent = pin.tracer.current_span()
     span = pin.tracer.start_span(
-        redisx.CMD, service=trace_utils.ext_service(pin, config.aioredis), span_type=SpanTypes.REDIS, activate=False
+        redisx.CMD,
+        service=trace_utils.ext_service(pin, config.aioredis),
+        span_type=SpanTypes.REDIS,
+        activate=False,
+        child_of=parent,
     )
 
     span.set_tag(SPAN_MEASURED_KEY)

releasenotes/notes/fix-aioredis-13-parenting-6b03cc692ac639ce.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fix parenting of Redis command spans when using aioredis 1.3. Redis spans
+    should now be correctly attributed as child of any active parent spans.

tests/contrib/aioredis/test_aioredis.py

@@ -64,8 +64,10 @@ def test_patching():
         assert not isinstance(aioredis.client.Pipeline.pipeline, ObjectProxy)
     else:
         assert isinstance(aioredis.Redis.execute, ObjectProxy)
+        assert isinstance(aioredis.commands.Redis.execute, ObjectProxy)
         unpatch()
         assert not isinstance(aioredis.Redis.execute, ObjectProxy)
+        assert not isinstance(aioredis.commands.Redis.execute, ObjectProxy)
 
 
 @pytest.mark.asyncio
@@ -240,3 +242,11 @@ async def test_two_traced_pipelines(redis_client):
         response_list1[1].decode() == "boo"
     )  # response from hset is 'Integer reply: The number of fields that were added.'
     assert response_list2[1].decode() == "bar"
+
+
+@pytest.mark.asyncio
+@pytest.mark.snapshot(variants={"": aioredis_version >= (2, 0), "13": aioredis_version < (2, 0)})
+async def test_parenting(redis_client):
+    with tracer.trace("web-request", service="test"):
+        await redis_client.set("blah", "boo")
+        await redis_client.get("blah")

tests/snapshots/tests.contrib.aioredis.test_aioredis.test_parenting.json

@@ -0,0 +1,65 @@
+[[
+  {
+    "name": "web-request",
+    "service": "test",
+    "resource": "web-request",
+    "trace_id": 0,
+    "span_id": 1,
+    "parent_id": 0,
+    "meta": {
+      "runtime-id": "c75fe8a124e046a88374ca4e690d96f7"
+    },
+    "metrics": {
+      "_dd.agent_psr": 1.0,
+      "_dd.top_level": 1,
+      "_dd.tracer_kr": 1.0,
+      "_sampling_priority_v1": 1,
+      "system.pid": 2807
+    },
+    "duration": 4925125,
+    "start": 1642687869451527501
+  },
+     {
+       "name": "redis.command",
+       "service": "redis",
+       "resource": "SET blah boo",
+       "trace_id": 0,
+       "span_id": 2,
+       "parent_id": 1,
+       "type": "redis",
+       "meta": {
+         "out.host": "127.0.0.1",
+         "redis.raw_command": "SET blah boo"
+       },
+       "metrics": {
+         "_dd.measured": 1,
+         "_dd.top_level": 1,
+         "out.port": 6379,
+         "out.redis_db": 0,
+         "redis.args_length": 3
+       },
+       "duration": 1989125,
+       "start": 1642687869452441335
+     },
+     {
+       "name": "redis.command",
+       "service": "redis",
+       "resource": "GET blah",
+       "trace_id": 0,
+       "span_id": 3,
+       "parent_id": 1,
+       "type": "redis",
+       "meta": {
+         "out.host": "127.0.0.1",
+         "redis.raw_command": "GET blah"
+       },
+       "metrics": {
+         "_dd.measured": 1,
+         "_dd.top_level": 1,
+         "out.port": 6379,
+         "out.redis_db": 0,
+         "redis.args_length": 2
+       },
+       "duration": 1576875,
+       "start": 1642687869454763335
+     }]]

tests/snapshots/tests.contrib.aioredis.test_aioredis.test_parenting_13.json

@@ -0,0 +1,65 @@
+[[
+  {
+    "name": "web-request",
+    "service": "test",
+    "resource": "web-request",
+    "trace_id": 0,
+    "span_id": 1,
+    "parent_id": 0,
+    "meta": {
+      "runtime-id": "f48fe46e55d54ff38b2bd90772d8001e"
+    },
+    "metrics": {
+      "_dd.agent_psr": 1.0,
+      "_dd.top_level": 1,
+      "_dd.tracer_kr": 1.0,
+      "_sampling_priority_v1": 1,
+      "system.pid": 2802
+    },
+    "duration": 2725083,
+    "start": 1642687829923888303
+  },
+     {
+       "name": "redis.command",
+       "service": "redis",
+       "resource": "SET blah boo",
+       "trace_id": 0,
+       "span_id": 2,
+       "parent_id": 1,
+       "type": "redis",
+       "meta": {
+         "out.host": "127.0.0.1",
+         "redis.raw_command": "SET blah boo"
+       },
+       "metrics": {
+         "_dd.measured": 1,
+         "_dd.top_level": 1,
+         "out.port": 6379,
+         "out.redis_db": 0,
+         "redis.args_length": 3
+       },
+       "duration": 1074709,
+       "start": 1642687829924429969
+     },
+     {
+       "name": "redis.command",
+       "service": "redis",
+       "resource": "GET blah",
+       "trace_id": 0,
+       "span_id": 3,
+       "parent_id": 1,
+       "type": "redis",
+       "meta": {
+         "out.host": "127.0.0.1",
+         "redis.raw_command": "GET blah"
+       },
+       "metrics": {
+         "_dd.measured": 1,
+         "_dd.top_level": 1,
+         "out.port": 6379,
+         "out.redis_db": 0,
+         "redis.args_length": 2
+       },
+       "duration": 809833,
+       "start": 1642687829925730428
+     }]]