feat(redis): Add instrumentation for redis pipeline (#1543)

Add automatic instrumentation of redis pipelining for both redis and rediscluster.
https://redis.io/docs/manual/pipelining/
Note: This does not add instrumentation for StrictRedisCluster.

Author: jjbayer

sentry_sdk/integrations/redis.py

@@ -7,13 +7,64 @@
 from sentry_sdk._types import MYPY
 
 if MYPY:
-    from typing import Any
+    from typing import Any, Sequence
 
 _SINGLE_KEY_COMMANDS = frozenset(
     ["decr", "decrby", "get", "incr", "incrby", "pttl", "set", "setex", "setnx", "ttl"]
 )
 _MULTI_KEY_COMMANDS = frozenset(["del", "touch", "unlink"])
 
+#: Trim argument lists to this many values
+_MAX_NUM_ARGS = 10
+
+
+def patch_redis_pipeline(pipeline_cls, is_cluster, get_command_args_fn):
+    # type: (Any, bool, Any) -> None
+    old_execute = pipeline_cls.execute
+
+    def sentry_patched_execute(self, *args, **kwargs):
+        # type: (Any, *Any, **Any) -> Any
+        hub = Hub.current
+
+        if hub.get_integration(RedisIntegration) is None:
+            return old_execute(self, *args, **kwargs)
+
+        with hub.start_span(op="redis", description="redis.pipeline.execute") as span:
+            with capture_internal_exceptions():
+                span.set_tag("redis.is_cluster", is_cluster)
+                transaction = self.transaction if not is_cluster else False
+                span.set_tag("redis.transaction", transaction)
+
+                commands = []
+                for i, arg in enumerate(self.command_stack):
+                    if i > _MAX_NUM_ARGS:
+                        break
+                    command_args = []
+                    for j, command_arg in enumerate(get_command_args_fn(arg)):
+                        if j > 0:
+                            command_arg = repr(command_arg)
+                        command_args.append(command_arg)
+                    commands.append(" ".join(command_args))
+
+                span.set_data(
+                    "redis.commands",
+                    {"count": len(self.command_stack), "first_ten": commands},
+                )
+
+            return old_execute(self, *args, **kwargs)
+
+    pipeline_cls.execute = sentry_patched_execute
+
+
+def _get_redis_command_args(command):
+    # type: (Any) -> Sequence[Any]
+    return command[0]
+
+
+def _parse_rediscluster_command(command):
+    # type: (Any) -> Sequence[Any]
+    return command.args
+
 
 def _patch_rediscluster():
     # type: () -> None
@@ -22,7 +73,7 @@ def _patch_rediscluster():
     except ImportError:
         return
 
-    patch_redis_client(rediscluster.RedisCluster)
+    patch_redis_client(rediscluster.RedisCluster, is_cluster=True)
 
     # up to v1.3.6, __version__ attribute is a tuple
     # from v2.0.0, __version__ is a string and VERSION a tuple
@@ -31,7 +82,12 @@ def _patch_rediscluster():
     # StrictRedisCluster was introduced in v0.2.0 and removed in v2.0.0
     # https://github.com/Grokzen/redis-py-cluster/blob/master/docs/release-notes.rst
     if (0, 2, 0) < version < (2, 0, 0):
-        patch_redis_client(rediscluster.StrictRedisCluster)
+        pipeline_cls = rediscluster.StrictClusterPipeline
+        patch_redis_client(rediscluster.StrictRedisCluster, is_cluster=True)
+    else:
+        pipeline_cls = rediscluster.ClusterPipeline
+
+    patch_redis_pipeline(pipeline_cls, True, _parse_rediscluster_command)
 
 
 class RedisIntegration(Integration):
@@ -45,25 +101,32 @@ def setup_once():
         except ImportError:
             raise DidNotEnable("Redis client not installed")
 
-        patch_redis_client(redis.StrictRedis)
+        patch_redis_client(redis.StrictRedis, is_cluster=False)
+        patch_redis_pipeline(redis.client.Pipeline, False, _get_redis_command_args)
+        try:
+            strict_pipeline = redis.client.StrictPipeline  # type: ignore
+        except AttributeError:
+            pass
+        else:
+            patch_redis_pipeline(strict_pipeline, False, _get_redis_command_args)
 
         try:
             import rb.clients  # type: ignore
         except ImportError:
             pass
         else:
-            patch_redis_client(rb.clients.FanoutClient)
-            patch_redis_client(rb.clients.MappingClient)
-            patch_redis_client(rb.clients.RoutingClient)
+            patch_redis_client(rb.clients.FanoutClient, is_cluster=False)
+            patch_redis_client(rb.clients.MappingClient, is_cluster=False)
+            patch_redis_client(rb.clients.RoutingClient, is_cluster=False)
 
         try:
             _patch_rediscluster()
         except Exception:
             logger.exception("Error occurred while patching `rediscluster` library")
 
 
-def patch_redis_client(cls):
-    # type: (Any) -> None
+def patch_redis_client(cls, is_cluster):
+    # type: (Any, bool) -> None
     """
     This function can be used to instrument custom redis client classes or
     subclasses.
@@ -83,14 +146,15 @@ def sentry_patched_execute_command(self, name, *args, **kwargs):
         with capture_internal_exceptions():
             description_parts = [name]
             for i, arg in enumerate(args):
-                if i > 10:
+                if i > _MAX_NUM_ARGS:
                     break
 
                 description_parts.append(repr(arg))
 
             description = " ".join(description_parts)
 
         with hub.start_span(op="redis", description=description) as span:
+            span.set_tag("redis.is_cluster", is_cluster)
             if name:
                 span.set_tag("redis.command", name)
 

tests/integrations/redis/test_redis.py

@@ -1,7 +1,8 @@
-from sentry_sdk import capture_message
+from sentry_sdk import capture_message, start_transaction
 from sentry_sdk.integrations.redis import RedisIntegration
 
 from fakeredis import FakeStrictRedis
+import pytest
 
 
 def test_basic(sentry_init, capture_events):
@@ -19,7 +20,41 @@ def test_basic(sentry_init, capture_events):
     assert crumb == {
         "category": "redis",
         "message": "GET 'foobar'",
-        "data": {"redis.key": "foobar", "redis.command": "GET"},
+        "data": {
+            "redis.key": "foobar",
+            "redis.command": "GET",
+            "redis.is_cluster": False,
+        },
         "timestamp": crumb["timestamp"],
         "type": "redis",
     }
+
+
+@pytest.mark.parametrize("is_transaction", [False, True])
+def test_redis_pipeline(sentry_init, capture_events, is_transaction):
+    sentry_init(integrations=[RedisIntegration()], traces_sample_rate=1.0)
+    events = capture_events()
+
+    connection = FakeStrictRedis()
+    with start_transaction():
+
+        pipeline = connection.pipeline(transaction=is_transaction)
+        pipeline.get("foo")
+        pipeline.set("bar", 1)
+        pipeline.set("baz", 2)
+        pipeline.execute()
+
+    (event,) = events
+    (span,) = event["spans"]
+    assert span["op"] == "redis"
+    assert span["description"] == "redis.pipeline.execute"
+    assert span["data"] == {
+        "redis.commands": {
+            "count": 3,
+            "first_ten": ["GET 'foo'", "SET 'bar' 1", "SET 'baz' 2"],
+        }
+    }
+    assert span["tags"] == {
+        "redis.transaction": is_transaction,
+        "redis.is_cluster": False,
+    }

tests/integrations/rediscluster/test_rediscluster.py

@@ -1,5 +1,6 @@
 import pytest
 from sentry_sdk import capture_message
+from sentry_sdk.api import start_transaction
 from sentry_sdk.integrations.redis import RedisIntegration
 
 import rediscluster
@@ -12,6 +13,15 @@
 
 @pytest.fixture(scope="module", autouse=True)
 def monkeypatch_rediscluster_classes():
+
+    try:
+        pipeline_cls = rediscluster.ClusterPipeline
+    except AttributeError:
+        pipeline_cls = rediscluster.StrictClusterPipeline
+    rediscluster.RedisCluster.pipeline = lambda *_, **__: pipeline_cls(
+        connection_pool=True
+    )
+    pipeline_cls.execute = lambda *_, **__: None
     for cls in rediscluster_classes:
         cls.execute_command = lambda *_, **__: None
 
@@ -31,7 +41,39 @@ def test_rediscluster_basic(rediscluster_cls, sentry_init, capture_events):
     assert crumb == {
         "category": "redis",
         "message": "GET 'foobar'",
-        "data": {"redis.key": "foobar", "redis.command": "GET"},
+        "data": {
+            "redis.key": "foobar",
+            "redis.command": "GET",
+            "redis.is_cluster": True,
+        },
         "timestamp": crumb["timestamp"],
         "type": "redis",
     }
+
+
+def test_rediscluster_pipeline(sentry_init, capture_events):
+    sentry_init(integrations=[RedisIntegration()], traces_sample_rate=1.0)
+    events = capture_events()
+
+    rc = rediscluster.RedisCluster(connection_pool=True)
+    with start_transaction():
+        pipeline = rc.pipeline()
+        pipeline.get("foo")
+        pipeline.set("bar", 1)
+        pipeline.set("baz", 2)
+        pipeline.execute()
+
+    (event,) = events
+    (span,) = event["spans"]
+    assert span["op"] == "redis"
+    assert span["description"] == "redis.pipeline.execute"
+    assert span["data"] == {
+        "redis.commands": {
+            "count": 3,
+            "first_ten": ["GET 'foo'", "SET 'bar' 1", "SET 'baz' 2"],
+        }
+    }
+    assert span["tags"] == {
+        "redis.transaction": False,  # For Cluster, this is always False
+        "redis.is_cluster": True,
+    }