diff --git a/sentry_sdk/integrations/redis/_async_common.py b/sentry_sdk/integrations/redis/_async_common.py index 196e85e74b..6a136fe29a 100644 --- a/sentry_sdk/integrations/redis/_async_common.py +++ b/sentry_sdk/integrations/redis/_async_common.py @@ -3,12 +3,14 @@ from sentry_sdk.integrations.redis.consts import SPAN_ORIGIN from sentry_sdk.integrations.redis.modules.caches import ( _compile_cache_span_properties, - _set_cache_data, + _get_cache_data, ) from sentry_sdk.integrations.redis.modules.queries import _compile_db_span_properties from sentry_sdk.integrations.redis.utils import ( - _set_client_data, - _set_pipeline_data, + _create_breadcrumb, + _get_client_data, + _get_pipeline_data, + _update_span, ) from sentry_sdk.tracing import Span from sentry_sdk.utils import capture_internal_exceptions @@ -23,9 +25,9 @@ def patch_redis_async_pipeline( - pipeline_cls, is_cluster, get_command_args_fn, set_db_data_fn + pipeline_cls, is_cluster, get_command_args_fn, get_db_data_fn ): - # type: (Union[type[Pipeline[Any]], type[ClusterPipeline[Any]]], bool, Any, Callable[[Span, Any], None]) -> None + # type: (Union[type[Pipeline[Any]], type[ClusterPipeline[Any]]], bool, Any, Callable[[Any], dict[str, Any]]) -> None old_execute = pipeline_cls.execute from sentry_sdk.integrations.redis import RedisIntegration @@ -41,22 +43,25 @@ async def _sentry_execute(self, *args, **kwargs): origin=SPAN_ORIGIN, ) as span: with capture_internal_exceptions(): - set_db_data_fn(span, self) - _set_pipeline_data( - span, - is_cluster, - get_command_args_fn, - False if is_cluster else self.is_transaction, - self._command_stack if is_cluster else self.command_stack, + span_data = get_db_data_fn(self) + pipeline_data = _get_pipeline_data( + is_cluster=is_cluster, + get_command_args_fn=get_command_args_fn, + is_transaction=False if is_cluster else self.is_transaction, + command_stack=( + self._command_stack if is_cluster else self.command_stack + ), ) + _update_span(span, span_data, pipeline_data) + _create_breadcrumb("redis.pipeline.execute", span_data, pipeline_data) return await old_execute(self, *args, **kwargs) pipeline_cls.execute = _sentry_execute # type: ignore -def patch_redis_async_client(cls, is_cluster, set_db_data_fn): - # type: (Union[type[StrictRedis[Any]], type[RedisCluster[Any]]], bool, Callable[[Span, Any], None]) -> None +def patch_redis_async_client(cls, is_cluster, get_db_data_fn): + # type: (Union[type[StrictRedis[Any]], type[RedisCluster[Any]]], bool, Callable[[Any], dict[str, Any]]) -> None old_execute_command = cls.execute_command from sentry_sdk.integrations.redis import RedisIntegration @@ -92,15 +97,20 @@ async def _sentry_execute_command(self, name, *args, **kwargs): ) db_span.__enter__() - set_db_data_fn(db_span, self) - _set_client_data(db_span, is_cluster, name, *args) + db_span_data = get_db_data_fn(self) + db_client_span_data = _get_client_data(is_cluster, name, *args) + _update_span(db_span, db_span_data, db_client_span_data) + _create_breadcrumb( + db_properties["description"], db_span_data, db_client_span_data + ) value = await old_execute_command(self, name, *args, **kwargs) db_span.__exit__(None, None, None) if cache_span: - _set_cache_data(cache_span, self, cache_properties, value) + cache_span_data = _get_cache_data(self, cache_properties, value) + _update_span(cache_span, cache_span_data) cache_span.__exit__(None, None, None) return value diff --git a/sentry_sdk/integrations/redis/_sync_common.py b/sentry_sdk/integrations/redis/_sync_common.py index ef10e9e4f0..f4cb6ee1b8 100644 --- a/sentry_sdk/integrations/redis/_sync_common.py +++ b/sentry_sdk/integrations/redis/_sync_common.py @@ -3,12 +3,14 @@ from sentry_sdk.integrations.redis.consts import SPAN_ORIGIN from sentry_sdk.integrations.redis.modules.caches import ( _compile_cache_span_properties, - _set_cache_data, + _get_cache_data, ) from sentry_sdk.integrations.redis.modules.queries import _compile_db_span_properties from sentry_sdk.integrations.redis.utils import ( - _set_client_data, - _set_pipeline_data, + _create_breadcrumb, + _get_client_data, + _get_pipeline_data, + _update_span, ) from sentry_sdk.tracing import Span from sentry_sdk.utils import capture_internal_exceptions @@ -24,9 +26,9 @@ def patch_redis_pipeline( pipeline_cls, is_cluster, get_command_args_fn, - set_db_data_fn, + get_db_data_fn, ): - # type: (Any, bool, Any, Callable[[Span, Any], None]) -> None + # type: (Any, bool, Any, Callable[[Any], dict[str, Any]]) -> None old_execute = pipeline_cls.execute from sentry_sdk.integrations.redis import RedisIntegration @@ -42,22 +44,23 @@ def sentry_patched_execute(self, *args, **kwargs): origin=SPAN_ORIGIN, ) as span: with capture_internal_exceptions(): - set_db_data_fn(span, self) - _set_pipeline_data( - span, - is_cluster, - get_command_args_fn, - False if is_cluster else self.transaction, - self.command_stack, + span_data = get_db_data_fn(self) + pipeline_data = _get_pipeline_data( + is_cluster=is_cluster, + get_command_args_fn=get_command_args_fn, + is_transaction=False if is_cluster else self.transaction, + command_stack=self.command_stack, ) + _update_span(span, span_data, pipeline_data) + _create_breadcrumb("redis.pipeline.execute", span_data, pipeline_data) return old_execute(self, *args, **kwargs) pipeline_cls.execute = sentry_patched_execute -def patch_redis_client(cls, is_cluster, set_db_data_fn): - # type: (Any, bool, Callable[[Span, Any], None]) -> None +def patch_redis_client(cls, is_cluster, get_db_data_fn): + # type: (Any, bool, Callable[[Any], dict[str, Any]]) -> None """ This function can be used to instrument custom redis client classes or subclasses. @@ -97,15 +100,20 @@ def sentry_patched_execute_command(self, name, *args, **kwargs): ) db_span.__enter__() - set_db_data_fn(db_span, self) - _set_client_data(db_span, is_cluster, name, *args) + db_span_data = get_db_data_fn(self) + db_client_span_data = _get_client_data(is_cluster, name, *args) + _update_span(db_span, db_span_data, db_client_span_data) + _create_breadcrumb( + db_properties["description"], db_span_data, db_client_span_data + ) value = old_execute_command(self, name, *args, **kwargs) db_span.__exit__(None, None, None) if cache_span: - _set_cache_data(cache_span, self, cache_properties, value) + cache_span_data = _get_cache_data(self, cache_properties, value) + _update_span(cache_span, cache_span_data) cache_span.__exit__(None, None, None) return value diff --git a/sentry_sdk/integrations/redis/modules/caches.py b/sentry_sdk/integrations/redis/modules/caches.py index c6fc19f5b2..d93e729f2b 100644 --- a/sentry_sdk/integrations/redis/modules/caches.py +++ b/sentry_sdk/integrations/redis/modules/caches.py @@ -75,22 +75,24 @@ def _get_cache_span_description(redis_command, args, kwargs, integration): return description -def _set_cache_data(span, redis_client, properties, return_value): - # type: (Span, Any, dict[str, Any], Optional[Any]) -> None +def _get_cache_data(redis_client, properties, return_value): + # type: (Any, dict[str, Any], Optional[Any]) -> dict[str, Any] + data = {} + with capture_internal_exceptions(): - span.set_data(SPANDATA.CACHE_KEY, properties["key"]) + data[SPANDATA.CACHE_KEY] = properties["key"] if properties["redis_command"] in GET_COMMANDS: if return_value is not None: - span.set_data(SPANDATA.CACHE_HIT, True) + data[SPANDATA.CACHE_HIT] = True size = ( len(str(return_value).encode("utf-8")) if not isinstance(return_value, bytes) else len(return_value) ) - span.set_data(SPANDATA.CACHE_ITEM_SIZE, size) + data[SPANDATA.CACHE_ITEM_SIZE] = size else: - span.set_data(SPANDATA.CACHE_HIT, False) + data[SPANDATA.CACHE_HIT] = False elif properties["redis_command"] in SET_COMMANDS: if properties["value"] is not None: @@ -99,7 +101,7 @@ def _set_cache_data(span, redis_client, properties, return_value): if not isinstance(properties["value"], bytes) else len(properties["value"]) ) - span.set_data(SPANDATA.CACHE_ITEM_SIZE, size) + data[SPANDATA.CACHE_ITEM_SIZE] = size try: connection_params = redis_client.connection_pool.connection_kwargs @@ -114,8 +116,10 @@ def _set_cache_data(span, redis_client, properties, return_value): host = connection_params.get("host") if host is not None: - span.set_data(SPANDATA.NETWORK_PEER_ADDRESS, host) + data[SPANDATA.NETWORK_PEER_ADDRESS] = host port = connection_params.get("port") if port is not None: - span.set_data(SPANDATA.NETWORK_PEER_PORT, port) + data[SPANDATA.NETWORK_PEER_PORT] = port + + return data diff --git a/sentry_sdk/integrations/redis/modules/queries.py b/sentry_sdk/integrations/redis/modules/queries.py index e0d85a4ef7..e2189b7f9c 100644 --- a/sentry_sdk/integrations/redis/modules/queries.py +++ b/sentry_sdk/integrations/redis/modules/queries.py @@ -43,26 +43,30 @@ def _get_db_span_description(integration, command_name, args): return description -def _set_db_data_on_span(span, connection_params): - # type: (Span, dict[str, Any]) -> None - span.set_data(SPANDATA.DB_SYSTEM, "redis") +def _get_connection_data(connection_params): + # type: (dict[str, Any]) -> dict[str, Any] + data = { + SPANDATA.DB_SYSTEM: "redis", + } db = connection_params.get("db") if db is not None: - span.set_data(SPANDATA.DB_NAME, str(db)) + data[SPANDATA.DB_NAME] = str(db) host = connection_params.get("host") if host is not None: - span.set_data(SPANDATA.SERVER_ADDRESS, host) + data[SPANDATA.SERVER_ADDRESS] = host port = connection_params.get("port") if port is not None: - span.set_data(SPANDATA.SERVER_PORT, port) + data[SPANDATA.SERVER_PORT] = port + + return data -def _set_db_data(span, redis_instance): - # type: (Span, Redis[Any]) -> None +def _get_db_data(redis_instance): + # type: (Redis[Any]) -> dict[str, Any] try: - _set_db_data_on_span(span, redis_instance.connection_pool.connection_kwargs) + return _get_connection_data(redis_instance.connection_pool.connection_kwargs) except AttributeError: - pass # connections_kwargs may be missing in some cases + return {} # connections_kwargs may be missing in some cases diff --git a/sentry_sdk/integrations/redis/rb.py b/sentry_sdk/integrations/redis/rb.py index 1b3e2e530c..68d3c3a9d6 100644 --- a/sentry_sdk/integrations/redis/rb.py +++ b/sentry_sdk/integrations/redis/rb.py @@ -5,7 +5,7 @@ """ from sentry_sdk.integrations.redis._sync_common import patch_redis_client -from sentry_sdk.integrations.redis.modules.queries import _set_db_data +from sentry_sdk.integrations.redis.modules.queries import _get_db_data def _patch_rb(): @@ -18,15 +18,15 @@ def _patch_rb(): patch_redis_client( rb.clients.FanoutClient, is_cluster=False, - set_db_data_fn=_set_db_data, + get_db_data_fn=_get_db_data, ) patch_redis_client( rb.clients.MappingClient, is_cluster=False, - set_db_data_fn=_set_db_data, + get_db_data_fn=_get_db_data, ) patch_redis_client( rb.clients.RoutingClient, is_cluster=False, - set_db_data_fn=_set_db_data, + get_db_data_fn=_get_db_data, ) diff --git a/sentry_sdk/integrations/redis/redis.py b/sentry_sdk/integrations/redis/redis.py index c92958a32d..935a828c3d 100644 --- a/sentry_sdk/integrations/redis/redis.py +++ b/sentry_sdk/integrations/redis/redis.py @@ -8,7 +8,7 @@ patch_redis_client, patch_redis_pipeline, ) -from sentry_sdk.integrations.redis.modules.queries import _set_db_data +from sentry_sdk.integrations.redis.modules.queries import _get_db_data from typing import TYPE_CHECKING @@ -26,13 +26,13 @@ def _patch_redis(StrictRedis, client): # noqa: N803 patch_redis_client( StrictRedis, is_cluster=False, - set_db_data_fn=_set_db_data, + get_db_data_fn=_get_db_data, ) patch_redis_pipeline( client.Pipeline, is_cluster=False, get_command_args_fn=_get_redis_command_args, - set_db_data_fn=_set_db_data, + get_db_data_fn=_get_db_data, ) try: strict_pipeline = client.StrictPipeline @@ -43,7 +43,7 @@ def _patch_redis(StrictRedis, client): # noqa: N803 strict_pipeline, is_cluster=False, get_command_args_fn=_get_redis_command_args, - set_db_data_fn=_set_db_data, + get_db_data_fn=_get_db_data, ) try: @@ -59,11 +59,11 @@ def _patch_redis(StrictRedis, client): # noqa: N803 patch_redis_async_client( redis.asyncio.client.StrictRedis, is_cluster=False, - set_db_data_fn=_set_db_data, + get_db_data_fn=_get_db_data, ) patch_redis_async_pipeline( redis.asyncio.client.Pipeline, False, _get_redis_command_args, - set_db_data_fn=_set_db_data, + get_db_data_fn=_get_db_data, ) diff --git a/sentry_sdk/integrations/redis/redis_cluster.py b/sentry_sdk/integrations/redis/redis_cluster.py index 80cdc7235a..dbcd20a65d 100644 --- a/sentry_sdk/integrations/redis/redis_cluster.py +++ b/sentry_sdk/integrations/redis/redis_cluster.py @@ -9,7 +9,7 @@ patch_redis_client, patch_redis_pipeline, ) -from sentry_sdk.integrations.redis.modules.queries import _set_db_data_on_span +from sentry_sdk.integrations.redis.modules.queries import _get_connection_data from sentry_sdk.integrations.redis.utils import _parse_rediscluster_command from sentry_sdk.utils import capture_internal_exceptions @@ -26,26 +26,27 @@ from sentry_sdk.tracing import Span -def _set_async_cluster_db_data(span, async_redis_cluster_instance): - # type: (Span, AsyncRedisCluster[Any]) -> None +def _get_async_cluster_db_data(async_redis_cluster_instance): + # type: (AsyncRedisCluster[Any]) -> dict[str, Any] default_node = async_redis_cluster_instance.get_default_node() if default_node is not None and default_node.connection_kwargs is not None: - _set_db_data_on_span(span, default_node.connection_kwargs) + return _get_connection_data(default_node.connection_kwargs) + else: + return {} -def _set_async_cluster_pipeline_db_data(span, async_redis_cluster_pipeline_instance): - # type: (Span, AsyncClusterPipeline[Any]) -> None +def _get_async_cluster_pipeline_db_data(async_redis_cluster_pipeline_instance): + # type: (AsyncClusterPipeline[Any]) -> dict[str, Any] with capture_internal_exceptions(): - _set_async_cluster_db_data( - span, + return _get_async_cluster_db_data( # the AsyncClusterPipeline has always had a `_client` attr but it is private so potentially problematic and mypy # does not recognize it - see https://github.com/redis/redis-py/blame/v5.0.0/redis/asyncio/cluster.py#L1386 async_redis_cluster_pipeline_instance._client, # type: ignore[attr-defined] ) -def _set_cluster_db_data(span, redis_cluster_instance): - # type: (Span, RedisCluster[Any]) -> None +def _get_cluster_db_data(redis_cluster_instance): + # type: (RedisCluster[Any]) -> dict[str, Any] default_node = redis_cluster_instance.get_default_node() if default_node is not None: @@ -53,7 +54,9 @@ def _set_cluster_db_data(span, redis_cluster_instance): "host": default_node.host, "port": default_node.port, } - _set_db_data_on_span(span, connection_params) + return _get_connection_data(connection_params) + else: + return {} def _patch_redis_cluster(): @@ -67,13 +70,13 @@ def _patch_redis_cluster(): patch_redis_client( RedisCluster, is_cluster=True, - set_db_data_fn=_set_cluster_db_data, + get_db_data_fn=_get_cluster_db_data, ) patch_redis_pipeline( cluster.ClusterPipeline, is_cluster=True, get_command_args_fn=_parse_rediscluster_command, - set_db_data_fn=_set_cluster_db_data, + get_db_data_fn=_get_cluster_db_data, ) try: @@ -89,11 +92,11 @@ def _patch_redis_cluster(): patch_redis_async_client( async_cluster.RedisCluster, is_cluster=True, - set_db_data_fn=_set_async_cluster_db_data, + get_db_data_fn=_get_async_cluster_db_data, ) patch_redis_async_pipeline( async_cluster.ClusterPipeline, is_cluster=True, get_command_args_fn=_parse_rediscluster_command, - set_db_data_fn=_set_async_cluster_pipeline_db_data, + get_db_data_fn=_get_async_cluster_pipeline_db_data, ) diff --git a/sentry_sdk/integrations/redis/redis_py_cluster_legacy.py b/sentry_sdk/integrations/redis/redis_py_cluster_legacy.py index ad1c23633f..53b545c21b 100644 --- a/sentry_sdk/integrations/redis/redis_py_cluster_legacy.py +++ b/sentry_sdk/integrations/redis/redis_py_cluster_legacy.py @@ -9,7 +9,7 @@ patch_redis_client, patch_redis_pipeline, ) -from sentry_sdk.integrations.redis.modules.queries import _set_db_data +from sentry_sdk.integrations.redis.modules.queries import _get_db_data from sentry_sdk.integrations.redis.utils import _parse_rediscluster_command @@ -23,7 +23,7 @@ def _patch_rediscluster(): patch_redis_client( rediscluster.RedisCluster, is_cluster=True, - set_db_data_fn=_set_db_data, + get_db_data_fn=_get_db_data, ) # up to v1.3.6, __version__ attribute is a tuple @@ -37,7 +37,7 @@ def _patch_rediscluster(): patch_redis_client( rediscluster.StrictRedisCluster, is_cluster=True, - set_db_data_fn=_set_db_data, + get_db_data_fn=_get_db_data, ) else: pipeline_cls = rediscluster.pipeline.ClusterPipeline @@ -46,5 +46,5 @@ def _patch_rediscluster(): pipeline_cls, is_cluster=True, get_command_args_fn=_parse_rediscluster_command, - set_db_data_fn=_set_db_data, + get_db_data_fn=_get_db_data, ) diff --git a/sentry_sdk/integrations/redis/utils.py b/sentry_sdk/integrations/redis/utils.py index 894c56305b..9eb16c5bc4 100644 --- a/sentry_sdk/integrations/redis/utils.py +++ b/sentry_sdk/integrations/redis/utils.py @@ -1,3 +1,4 @@ +import sentry_sdk from sentry_sdk.consts import SPANDATA from sentry_sdk.integrations.redis.consts import ( _COMMANDS_INCLUDING_SENSITIVE_DATA, @@ -16,6 +17,47 @@ from sentry_sdk.tracing import Span +TAG_KEYS = [ + "redis.command", + "redis.is_cluster", + "redis.key", + "redis.transaction", + SPANDATA.DB_OPERATION, +] + + +def _update_span(span, *data_bags): + # type: (Span, *dict[str, Any]) -> None + """ + Set tags and data on the given span to data from the given data bags. + """ + for data in data_bags: + for key, value in data.items(): + if key in TAG_KEYS: + span.set_tag(key, value) + else: + span.set_data(key, value) + + +def _create_breadcrumb(message, *data_bags): + # type: (str, *dict[str, Any]) -> None + """ + Create a breadcrumb containing the tags data from the given data bags. + """ + data = {} + for data in data_bags: + for key, value in data.items(): + if key in TAG_KEYS: + data[key] = value + + sentry_sdk.add_breadcrumb( + message=message, + type="redis", + category="redis", + data=data, + ) + + def _get_safe_command(name, args): # type: (str, Sequence[Any]) -> str command_parts = [name] @@ -105,12 +147,12 @@ def _parse_rediscluster_command(command): return command.args -def _set_pipeline_data( - span, is_cluster, get_command_args_fn, is_transaction, command_stack -): - # type: (Span, bool, Any, bool, Sequence[Any]) -> None - span.set_tag("redis.is_cluster", is_cluster) - span.set_tag("redis.transaction", is_transaction) +def _get_pipeline_data(is_cluster, get_command_args_fn, is_transaction, command_stack): + # type: (bool, Any, bool, Sequence[Any]) -> dict[str, Any] + data = { + "redis.is_cluster": is_cluster, + "redis.transaction": is_transaction, + } # type: dict[str, Any] commands = [] for i, arg in enumerate(command_stack): @@ -120,20 +162,27 @@ 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)) - span.set_data("redis.commands.first_ten", commands) + data["redis.commands.count"] = len(command_stack) + data["redis.commands.first_ten"] = commands + return data + + +def _get_client_data(is_cluster, name, *args): + # type: (bool, str, *Any) -> dict[str, Any] + data = { + "redis.is_cluster": is_cluster, + } # type: dict[str, Any] -def _set_client_data(span, is_cluster, name, *args): - # type: (Span, bool, str, *Any) -> None - span.set_tag("redis.is_cluster", is_cluster) if name: - span.set_tag("redis.command", name) - span.set_tag(SPANDATA.DB_OPERATION, name) + data["redis.command"] = name + data[SPANDATA.DB_OPERATION] = name if name and args: name_low = name.lower() if (name_low in _SINGLE_KEY_COMMANDS) or ( name_low in _MULTI_KEY_COMMANDS and len(args) == 1 ): - span.set_tag("redis.key", args[0]) + data["redis.key"] = args[0] + + return data diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index f063897cb9..2c6e7f2a7a 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -157,14 +157,7 @@ def record_sql_queries( def maybe_create_breadcrumbs_from_span(scope, span): # type: (sentry_sdk.Scope, sentry_sdk.tracing.Span) -> None - if span.op == OP.DB_REDIS: - scope.add_breadcrumb( - message=span.description, - type="redis", - category="redis", - data=span._tags, - ) - elif span.op == OP.HTTP_CLIENT: + if span.op == OP.HTTP_CLIENT: scope.add_breadcrumb( type="http", category="httplib", diff --git a/tests/integrations/redis/test_redis.py b/tests/integrations/redis/test_redis.py index aff5b3ec9b..f8225bed79 100644 --- a/tests/integrations/redis/test_redis.py +++ b/tests/integrations/redis/test_redis.py @@ -80,7 +80,7 @@ def test_redis_pipeline( } -def test_sensitive_data(sentry_init, capture_events): +def test_sensitive_data(sentry_init, capture_events, render_span_tree): # fakeredis does not support the AUTH command, so we need to mock it with mock.patch( "sentry_sdk.integrations.redis.utils._COMMANDS_INCLUDING_SENSITIVE_DATA", @@ -100,12 +100,16 @@ def test_sensitive_data(sentry_init, capture_events): ) # because fakeredis does not support AUTH we use GET instead (event,) = events - spans = event["spans"] - assert spans[0]["op"] == "db.redis" - assert spans[0]["description"] == "GET [Filtered]" + assert ( + render_span_tree(event) + == """\ +- op="": description=null + - op="db.redis": description="GET [Filtered]"\ +""" + ) -def test_pii_data_redacted(sentry_init, capture_events): +def test_pii_data_redacted(sentry_init, capture_events, render_span_tree): sentry_init( integrations=[RedisIntegration()], traces_sample_rate=1.0, @@ -120,15 +124,19 @@ def test_pii_data_redacted(sentry_init, capture_events): connection.delete("somekey1", "somekey2") (event,) = events - spans = event["spans"] - assert spans[0]["op"] == "db.redis" - assert spans[0]["description"] == "SET 'somekey1' [Filtered]" - assert spans[1]["description"] == "SET 'somekey2' [Filtered]" - assert spans[2]["description"] == "GET 'somekey2'" - assert spans[3]["description"] == "DEL 'somekey1' [Filtered]" + assert ( + render_span_tree(event) + == """\ +- op="": description=null + - op="db.redis": description="SET 'somekey1' [Filtered]" + - op="db.redis": description="SET 'somekey2' [Filtered]" + - op="db.redis": description="GET 'somekey2'" + - op="db.redis": description="DEL 'somekey1' [Filtered]"\ +""" + ) -def test_pii_data_sent(sentry_init, capture_events): +def test_pii_data_sent(sentry_init, capture_events, render_span_tree): sentry_init( integrations=[RedisIntegration()], traces_sample_rate=1.0, @@ -144,15 +152,19 @@ def test_pii_data_sent(sentry_init, capture_events): connection.delete("somekey1", "somekey2") (event,) = events - spans = event["spans"] - assert spans[0]["op"] == "db.redis" - assert spans[0]["description"] == "SET 'somekey1' 'my secret string1'" - assert spans[1]["description"] == "SET 'somekey2' 'my secret string2'" - assert spans[2]["description"] == "GET 'somekey2'" - assert spans[3]["description"] == "DEL 'somekey1' 'somekey2'" + assert ( + render_span_tree(event) + == """\ +- op="": description=null + - op="db.redis": description="SET 'somekey1' 'my secret string1'" + - op="db.redis": description="SET 'somekey2' 'my secret string2'" + - op="db.redis": description="GET 'somekey2'" + - op="db.redis": description="DEL 'somekey1' 'somekey2'"\ +""" + ) -def test_data_truncation(sentry_init, capture_events): +def test_data_truncation(sentry_init, capture_events, render_span_tree): sentry_init( integrations=[RedisIntegration()], traces_sample_rate=1.0, @@ -168,15 +180,17 @@ def test_data_truncation(sentry_init, capture_events): connection.set("somekey2", short_string) (event,) = events - spans = event["spans"] - assert spans[0]["op"] == "db.redis" - assert spans[0]["description"] == "SET 'somekey1' '%s..." % ( - long_string[: 1024 - len("...") - len("SET 'somekey1' '")], + assert ( + render_span_tree(event) + == f"""\ +- op="": description=null + - op="db.redis": description="SET 'somekey1' '{long_string[: 1024 - len("...") - len("SET 'somekey1' '")]}..." + - op="db.redis": description="SET 'somekey2' 'bbbbbbbbbb'"\ +""" ) - assert spans[1]["description"] == "SET 'somekey2' '%s'" % (short_string,) -def test_data_truncation_custom(sentry_init, capture_events): +def test_data_truncation_custom(sentry_init, capture_events, render_span_tree): sentry_init( integrations=[RedisIntegration(max_data_size=30)], traces_sample_rate=1.0, @@ -192,12 +206,14 @@ def test_data_truncation_custom(sentry_init, capture_events): connection.set("somekey2", short_string) (event,) = events - spans = event["spans"] - assert spans[0]["op"] == "db.redis" - assert spans[0]["description"] == "SET 'somekey1' '%s..." % ( - long_string[: 30 - len("...") - len("SET 'somekey1' '")], + assert ( + render_span_tree(event) + == f"""\ +- op="": description=null + - op="db.redis": description="SET 'somekey1' '{long_string[: 30 - len("...") - len("SET 'somekey1' '")]}..." + - op="db.redis": description="SET 'somekey2' '{short_string}'"\ +""" ) - assert spans[1]["description"] == "SET 'somekey2' '%s'" % (short_string,) def test_breadcrumbs(sentry_init, capture_events): diff --git a/tests/integrations/redis/test_redis_cache_module.py b/tests/integrations/redis/test_redis_cache_module.py index f118aa53f5..68f915c2e5 100644 --- a/tests/integrations/redis/test_redis_cache_module.py +++ b/tests/integrations/redis/test_redis_cache_module.py @@ -14,7 +14,7 @@ FAKEREDIS_VERSION = parse_version(fakeredis.__version__) -def test_no_cache_basic(sentry_init, capture_events): +def test_no_cache_basic(sentry_init, capture_events, render_span_tree): sentry_init( integrations=[ RedisIntegration(), @@ -28,12 +28,16 @@ def test_no_cache_basic(sentry_init, capture_events): connection.get("mycachekey") (event,) = events - spans = event["spans"] - assert len(spans) == 1 - assert spans[0]["op"] == "db.redis" + assert ( + render_span_tree(event) + == """\ +- op="": description=null + - op="db.redis": description="GET 'mycachekey'"\ +""" + ) -def test_cache_basic(sentry_init, capture_events): +def test_cache_basic(sentry_init, capture_events, render_span_tree): sentry_init( integrations=[ RedisIntegration( @@ -53,31 +57,25 @@ def test_cache_basic(sentry_init, capture_events): connection.mget("mycachekey1", "mycachekey2") (event,) = events - spans = event["spans"] - assert len(spans) == 9 - - # no cache support for hget command - assert spans[0]["op"] == "db.redis" - assert spans[0]["tags"]["redis.command"] == "HGET" - - assert spans[1]["op"] == "cache.get" - assert spans[2]["op"] == "db.redis" - assert spans[2]["tags"]["redis.command"] == "GET" - - assert spans[3]["op"] == "cache.put" - assert spans[4]["op"] == "db.redis" - assert spans[4]["tags"]["redis.command"] == "SET" - - assert spans[5]["op"] == "cache.put" - assert spans[6]["op"] == "db.redis" - assert spans[6]["tags"]["redis.command"] == "SETEX" - - assert spans[7]["op"] == "cache.get" - assert spans[8]["op"] == "db.redis" - assert spans[8]["tags"]["redis.command"] == "MGET" + # no cache support for HGET command + assert ( + render_span_tree(event) + == """\ +- op="": description=null + - op="db.redis": description="HGET 'mycachekey' [Filtered]" + - op="cache.get": description="mycachekey" + - op="db.redis": description="GET 'mycachekey'" + - op="cache.put": description="mycachekey1" + - op="db.redis": description="SET 'mycachekey1' [Filtered]" + - op="cache.put": description="mycachekey2" + - op="db.redis": description="SETEX 'mycachekey2' [Filtered] [Filtered]" + - op="cache.get": description="mycachekey1, mycachekey2" + - op="db.redis": description="MGET 'mycachekey1' [Filtered]"\ +""" + ) -def test_cache_keys(sentry_init, capture_events): +def test_cache_keys(sentry_init, capture_events, render_span_tree): sentry_init( integrations=[ RedisIntegration( @@ -96,23 +94,18 @@ def test_cache_keys(sentry_init, capture_events): connection.get("bl") (event,) = events - spans = event["spans"] - assert len(spans) == 6 - assert spans[0]["op"] == "db.redis" - assert spans[0]["description"] == "GET 'somethingelse'" - - assert spans[1]["op"] == "cache.get" - assert spans[1]["description"] == "blub" - assert spans[2]["op"] == "db.redis" - assert spans[2]["description"] == "GET 'blub'" - - assert spans[3]["op"] == "cache.get" - assert spans[3]["description"] == "blubkeything" - assert spans[4]["op"] == "db.redis" - assert spans[4]["description"] == "GET 'blubkeything'" - - assert spans[5]["op"] == "db.redis" - assert spans[5]["description"] == "GET 'bl'" + assert ( + render_span_tree(event) + == """\ +- op="": description=null + - op="db.redis": description="GET 'somethingelse'" + - op="cache.get": description="blub" + - op="db.redis": description="GET 'blub'" + - op="cache.get": description="blubkeything" + - op="db.redis": description="GET 'blubkeything'" + - op="db.redis": description="GET 'bl'"\ +""" + ) def test_cache_data(sentry_init, capture_events): @@ -133,7 +126,7 @@ def test_cache_data(sentry_init, capture_events): connection.get("mycachekey") (event,) = events - spans = event["spans"] + spans = sorted(event["spans"], key=lambda x: x["start_timestamp"]) assert len(spans) == 6 @@ -222,7 +215,7 @@ def test_cache_prefixes(sentry_init, capture_events): (event,) = events - spans = event["spans"] + spans = sorted(event["spans"], key=lambda x: x["start_timestamp"]) assert len(spans) == 13 # 8 db spans + 5 cache spans cache_spans = [span for span in spans if span["op"] == "cache.get"] diff --git a/tests/integrations/redis/test_redis_cache_module_async.py b/tests/integrations/redis/test_redis_cache_module_async.py index d607f92fbd..a6ea06a973 100644 --- a/tests/integrations/redis/test_redis_cache_module_async.py +++ b/tests/integrations/redis/test_redis_cache_module_async.py @@ -21,7 +21,7 @@ @pytest.mark.asyncio -async def test_no_cache_basic(sentry_init, capture_events): +async def test_no_cache_basic(sentry_init, capture_events, render_span_tree): sentry_init( integrations=[ RedisIntegration(), @@ -35,13 +35,17 @@ async def test_no_cache_basic(sentry_init, capture_events): await connection.get("myasynccachekey") (event,) = events - spans = event["spans"] - assert len(spans) == 1 - assert spans[0]["op"] == "db.redis" + assert ( + render_span_tree(event) + == """\ +- op="": description=null + - op="db.redis": description="GET 'myasynccachekey'"\ +""" + ) @pytest.mark.asyncio -async def test_cache_basic(sentry_init, capture_events): +async def test_cache_basic(sentry_init, capture_events, render_span_tree): sentry_init( integrations=[ RedisIntegration( @@ -57,15 +61,18 @@ async def test_cache_basic(sentry_init, capture_events): await connection.get("myasynccachekey") (event,) = events - spans = event["spans"] - assert len(spans) == 2 - - assert spans[0]["op"] == "cache.get" - assert spans[1]["op"] == "db.redis" + assert ( + render_span_tree(event) + == """\ +- op="": description=null + - op="cache.get": description="myasynccachekey" + - op="db.redis": description="GET 'myasynccachekey'"\ +""" + ) @pytest.mark.asyncio -async def test_cache_keys(sentry_init, capture_events): +async def test_cache_keys(sentry_init, capture_events, render_span_tree): sentry_init( integrations=[ RedisIntegration( @@ -84,23 +91,18 @@ async def test_cache_keys(sentry_init, capture_events): await connection.get("abl") (event,) = events - spans = event["spans"] - assert len(spans) == 6 - assert spans[0]["op"] == "db.redis" - assert spans[0]["description"] == "GET 'asomethingelse'" - - assert spans[1]["op"] == "cache.get" - assert spans[1]["description"] == "ablub" - assert spans[2]["op"] == "db.redis" - assert spans[2]["description"] == "GET 'ablub'" - - assert spans[3]["op"] == "cache.get" - assert spans[3]["description"] == "ablubkeything" - assert spans[4]["op"] == "db.redis" - assert spans[4]["description"] == "GET 'ablubkeything'" - - assert spans[5]["op"] == "db.redis" - assert spans[5]["description"] == "GET 'abl'" + assert ( + render_span_tree(event) + == """\ +- op="": description=null + - op="db.redis": description="GET 'asomethingelse'" + - op="cache.get": description="ablub" + - op="db.redis": description="GET 'ablub'" + - op="cache.get": description="ablubkeything" + - op="db.redis": description="GET 'ablubkeything'" + - op="db.redis": description="GET 'abl'"\ +""" + ) @pytest.mark.asyncio @@ -122,7 +124,7 @@ async def test_cache_data(sentry_init, capture_events): await connection.get("myasynccachekey") (event,) = events - spans = event["spans"] + spans = sorted(event["spans"], key=lambda x: x["start_timestamp"]) assert len(spans) == 6