From 5363e075cf01279f1dcc8236e638529f8b18485c Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 8 Oct 2024 14:52:57 +0200 Subject: [PATCH] Fix redis pipeline commands, because otel attributes do not allow dicts --- MIGRATION_GUIDE.md | 1 + sentry_sdk/integrations/redis/utils.py | 9 ++------- .../redis/asyncio/test_redis_asyncio.py | 6 ++---- .../redis/cluster/test_redis_cluster.py | 6 ++---- .../cluster_asyncio/test_redis_cluster_asyncio.py | 6 ++---- tests/integrations/redis/test_redis.py | 6 ++---- .../test_redis_py_cluster_legacy.py | 13 +++++-------- 7 files changed, 16 insertions(+), 31 deletions(-) diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 2a20350121..e90f63cbaf 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -17,6 +17,7 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh - The `Profile()` constructor does not accept a `hub` parameter anymore. - A `Profile` object does not have a `.hub` property anymore. - `sentry_sdk.continue_trace` no longer returns a `Transaction` and is now a context manager. +- Redis integration: In Redis pipeline spans there is no `span["data"]["redis.commands"]` that contains a dict `{"count": 3, "first_ten": ["cmd1", "cmd2", ...]}` but instead `span["data"]["redis.commands.count"]` (containing `3`) and `span["data"]["redis.commands.first_ten"]` (containing `["cmd1", "cmd2", ...]`). ### Removed diff --git a/sentry_sdk/integrations/redis/utils.py b/sentry_sdk/integrations/redis/utils.py index 27fae1e8ca..894c56305b 100644 --- a/sentry_sdk/integrations/redis/utils.py +++ b/sentry_sdk/integrations/redis/utils.py @@ -120,13 +120,8 @@ def _set_pipeline_data( command = get_command_args_fn(arg) commands.append(_get_safe_command(command[0], command[1:])) - span.set_data( - "redis.commands", - { - "count": len(command_stack), - "first_ten": commands, - }, - ) + span.set_data("redis.commands.count", len(command_stack)) + span.set_data("redis.commands.first_ten", commands) def _set_client_data(span, is_cluster, name, *args): diff --git a/tests/integrations/redis/asyncio/test_redis_asyncio.py b/tests/integrations/redis/asyncio/test_redis_asyncio.py index 17130b337b..2a0b96b021 100644 --- a/tests/integrations/redis/asyncio/test_redis_asyncio.py +++ b/tests/integrations/redis/asyncio/test_redis_asyncio.py @@ -65,12 +65,10 @@ async def test_async_redis_pipeline( (span,) = event["spans"] assert span["op"] == "db.redis" assert span["description"] == "redis.pipeline.execute" + assert span["data"]["redis.commands.count"] == 3 + assert span["data"]["redis.commands.first_ten"] == expected_first_ten assert span["data"] == ApproxDict( { - "redis.commands": { - "count": 3, - "first_ten": expected_first_ten, - }, SPANDATA.DB_SYSTEM: "redis", SPANDATA.DB_NAME: "0", SPANDATA.SERVER_ADDRESS: connection.connection_pool.connection_kwargs.get( diff --git a/tests/integrations/redis/cluster/test_redis_cluster.py b/tests/integrations/redis/cluster/test_redis_cluster.py index 83d1b45cc9..26feee1dae 100644 --- a/tests/integrations/redis/cluster/test_redis_cluster.py +++ b/tests/integrations/redis/cluster/test_redis_cluster.py @@ -128,12 +128,10 @@ def test_rediscluster_pipeline( (span,) = event["spans"] assert span["op"] == "db.redis" assert span["description"] == "redis.pipeline.execute" + assert span["data"]["redis.commands.count"] == 3 + assert span["data"]["redis.commands.first_ten"] == expected_first_ten assert span["data"] == ApproxDict( { - "redis.commands": { - "count": 3, - "first_ten": expected_first_ten, - }, SPANDATA.DB_SYSTEM: "redis", # ClusterNode converts localhost to 127.0.0.1 SPANDATA.SERVER_ADDRESS: "127.0.0.1", diff --git a/tests/integrations/redis/cluster_asyncio/test_redis_cluster_asyncio.py b/tests/integrations/redis/cluster_asyncio/test_redis_cluster_asyncio.py index 993a2962ca..b11808fb50 100644 --- a/tests/integrations/redis/cluster_asyncio/test_redis_cluster_asyncio.py +++ b/tests/integrations/redis/cluster_asyncio/test_redis_cluster_asyncio.py @@ -131,12 +131,10 @@ async def test_async_redis_pipeline( (span,) = event["spans"] assert span["op"] == "db.redis" assert span["description"] == "redis.pipeline.execute" + assert span["data"]["redis.commands.count"] == 3 + assert span["data"]["redis.commands.first_ten"] == expected_first_ten assert span["data"] == ApproxDict( { - "redis.commands": { - "count": 3, - "first_ten": expected_first_ten, - }, SPANDATA.DB_SYSTEM: "redis", # ClusterNode converts localhost to 127.0.0.1 SPANDATA.SERVER_ADDRESS: "127.0.0.1", diff --git a/tests/integrations/redis/test_redis.py b/tests/integrations/redis/test_redis.py index 5173885f33..aff5b3ec9b 100644 --- a/tests/integrations/redis/test_redis.py +++ b/tests/integrations/redis/test_redis.py @@ -72,10 +72,8 @@ def test_redis_pipeline( assert span["op"] == "db.redis" assert span["description"] == "redis.pipeline.execute" assert span["data"][SPANDATA.DB_SYSTEM] == "redis" - assert span["data"]["redis.commands"] == { - "count": 3, - "first_ten": expected_first_ten, - } + assert span["data"]["redis.commands.count"] == 3 + assert span["data"]["redis.commands.first_ten"] == expected_first_ten assert span["tags"] == { "redis.transaction": is_transaction, "redis.is_cluster": False, diff --git a/tests/integrations/redis_py_cluster_legacy/test_redis_py_cluster_legacy.py b/tests/integrations/redis_py_cluster_legacy/test_redis_py_cluster_legacy.py index 36a27d569d..5e0b724436 100644 --- a/tests/integrations/redis_py_cluster_legacy/test_redis_py_cluster_legacy.py +++ b/tests/integrations/redis_py_cluster_legacy/test_redis_py_cluster_legacy.py @@ -95,12 +95,10 @@ def test_rediscluster_pipeline( (span,) = event["spans"] assert span["op"] == "db.redis" assert span["description"] == "redis.pipeline.execute" + assert span["data"]["redis.commands.count"] == 3 + assert span["data"]["redis.commands.first_ten"] == expected_first_ten assert span["data"] == ApproxDict( { - "redis.commands": { - "count": 3, - "first_ten": expected_first_ten, - }, SPANDATA.DB_SYSTEM: "redis", SPANDATA.DB_NAME: "1", SPANDATA.SERVER_ADDRESS: "localhost", @@ -158,12 +156,11 @@ def test_db_connection_attributes_pipeline( (span,) = event["spans"] assert span["op"] == "db.redis" assert span["description"] == "redis.pipeline.execute" + assert span["data"]["redis.commands.count"] == 1 + assert span["data"]["redis.commands.first_ten"] == ["GET 'foo'"] + assert span["data"] == ApproxDict( { - "redis.commands": { - "count": 1, - "first_ten": ["GET 'foo'"], - }, SPANDATA.DB_SYSTEM: "redis", SPANDATA.DB_NAME: "1", SPANDATA.SERVER_ADDRESS: "localhost",