From d84818464bde848daf9a88a02bcc6404e1b5751f Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 11 Apr 2024 09:39:14 +0200 Subject: [PATCH 01/54] some thoughts before starting --- sentry_sdk/consts.py | 1 + sentry_sdk/integrations/django/caching.py | 4 ++++ sentry_sdk/integrations/redis/__init__.py | 6 ++++++ sentry_sdk/integrations/redis/asyncio.py | 4 ++++ 4 files changed, 15 insertions(+) diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index 7db2220f68..e5c4ef4311 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -218,6 +218,7 @@ class SPANDATA: class OP: CACHE_GET_ITEM = "cache.get_item" + CACHE_SET_ITEM = "cache.set_item" DB = "db" DB_REDIS = "db.redis" EVENT_DJANGO = "event.django" diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index 1b2bb477b1..aa3d876b38 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -14,6 +14,9 @@ from typing import Callable +# TODO: In new Django there is also `aget` and `aget_many` methods +# TODO: for also creating spans for setting cache there is a `set`, `aset`, `add`, `aadd` methods +# see https://github.com/django/django/blob/main/django/core/cache/backends/base.py METHODS_TO_INSTRUMENT = [ "get", "get_many", @@ -70,6 +73,7 @@ def sentry_method(*args, **kwargs): def _patch_cache(cache): # type: (CacheHandler) -> None if not hasattr(cache, "_sentry_patched"): + # TODO: Here we can also patch the set methods (see TODO above) for method_name in METHODS_TO_INSTRUMENT: _patch_cache_method(cache, method_name) cache._sentry_patched = True diff --git a/sentry_sdk/integrations/redis/__init__.py b/sentry_sdk/integrations/redis/__init__.py index 45f8653e29..3a25e73881 100644 --- a/sentry_sdk/integrations/redis/__init__.py +++ b/sentry_sdk/integrations/redis/__init__.py @@ -218,6 +218,11 @@ def sentry_patched_execute_command(self, name, *args, **kwargs): if data_should_be_truncated: description = description[: integration.max_data_size - len("...")] + "..." + # TODO: Here we could also create the caching spans. + # TODO: also need to check if the `name` (if this is the cache key value) matches the prefix we want to configure in __init__ of the integration + # Questions: + # -) We should probablby have the OP.DB_REDIS span and a separate OP.CACHE_GET_ITEM (or set_item) span, right? + # -) We probably need to research what redis commands are used by caching libs. with sentry_sdk.start_span(op=OP.DB_REDIS, description=description) as span: set_db_data_fn(span, self) _set_client_data(span, is_cluster, name, *args) @@ -358,6 +363,7 @@ class RedisIntegration(Integration): def __init__(self, max_data_size=_DEFAULT_MAX_DATA_SIZE): # type: (int) -> None self.max_data_size = max_data_size + # TODO: add some prefix that users can set to specify a cache key @staticmethod def setup_once(): diff --git a/sentry_sdk/integrations/redis/asyncio.py b/sentry_sdk/integrations/redis/asyncio.py index 227e3fa85c..25dfddc9ff 100644 --- a/sentry_sdk/integrations/redis/asyncio.py +++ b/sentry_sdk/integrations/redis/asyncio.py @@ -56,6 +56,10 @@ async def _sentry_execute_command(self, name, *args, **kwargs): # type: (Any, str, *Any, **Any) -> Any description = _get_span_description(name, *args) + # TODO: Here we could also create the caching spans. + # Questions: + # -) We should probablby have the OP.DB_REDIS span and a separate OP.CACHE_GET_ITEM (or set_item) span, right? + # -) We probably need to research what redis commands are used by caching libs. with sentry_sdk.start_span(op=OP.DB_REDIS, description=description) as span: set_db_data_fn(span, self) _set_client_data(span, is_cluster, name, *args) From 8926c93f6f501c39cfb0409da1d5c17f74f662f8 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 16 Apr 2024 09:15:31 +0200 Subject: [PATCH 02/54] some work --- sentry_sdk/consts.py | 16 +++++++ sentry_sdk/integrations/django/caching.py | 55 +++++++++++++++++------ 2 files changed, 57 insertions(+), 14 deletions(-) diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index e5c4ef4311..871589b4f5 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -130,6 +130,22 @@ class SPANDATA: Example: 58 """ + CACHE_KEY = "cache.key" + """ + """ + + CACHE_TTL = "cache.ttl" + """ + """ + + NETWORK_PEER_ADDRESS = "network.peer.address" + """ + """ + + NETWORK_PEER_PORT = "network.peer.port" + """ + """ + HTTP_QUERY = "http.query" """ The Query string present in the URL. diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index aa3d876b38..369e9f93a6 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -12,26 +12,41 @@ if TYPE_CHECKING: from typing import Any from typing import Callable + from typing import Optional # TODO: In new Django there is also `aget` and `aget_many` methods # TODO: for also creating spans for setting cache there is a `set`, `aset`, `add`, `aadd` methods # see https://github.com/django/django/blob/main/django/core/cache/backends/base.py METHODS_TO_INSTRUMENT = [ + # "set", "get", "get_many", ] +def _get_timeout(args, kwargs): + # type: (Any, Any) -> Optional[int] + if args is not None and len(args) >= 3: + return args[2] + return None -def _get_span_description(method_name, args, kwargs): - # type: (str, Any, Any) -> str - description = "{} ".format(method_name) +def _get_key(args, kwargs): + # type: (Any, Any) -> str if args is not None and len(args) >= 1: - description += str(args[0]) + return str(args[0]) elif kwargs is not None and "key" in kwargs: - description += str(kwargs["key"]) + return str(kwargs["key"]) + + return "" + +def _get_span_description(method_name, args, kwargs): + # type: (str, Any, Any) -> str + description = "{} {}".format( + method_name, + _get_key(args, kwargs), + ) return description @@ -44,21 +59,33 @@ def _patch_cache_method(cache, method_name): @ensure_integration_enabled(DjangoIntegration, original_method) def _instrument_call(cache, method_name, original_method, args, kwargs): # type: (CacheHandler, str, Callable[..., Any], Any, Any) -> Any + import ipdb; ipdb.set_trace() + + is_set_operation = method_name == "set" + + op = OP.CACHE_SET_ITEM if is_set_operation else OP.CACHE_GET_ITEM description = _get_span_description(method_name, args, kwargs) - with sentry_sdk.start_span( - op=OP.CACHE_GET_ITEM, description=description - ) as span: + with sentry_sdk.start_span(op=op, description=description) as span: value = original_method(*args, **kwargs) - if value: - span.set_data(SPANDATA.CACHE_HIT, True) - - size = len(str(value)) - span.set_data(SPANDATA.CACHE_ITEM_SIZE, size) + key = _get_key(args, kwargs) + if key != "": + span.set_data(SPANDATA.CACHE_KEY, key) + if is_set_operation: + timeout = _get_timeout(args, kwargs) + if timeout is not None: + span.set_data(SPANDATA.CACHE_TTL, timeout) else: - span.set_data(SPANDATA.CACHE_HIT, False) + if value: + span.set_data(SPANDATA.CACHE_HIT, True) + + size = len(str(value)) + span.set_data(SPANDATA.CACHE_ITEM_SIZE, size) + + else: + span.set_data(SPANDATA.CACHE_HIT, False) return value From 618157b2f7eebe2521ae909ee0d16ca008ffc453 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 24 Apr 2024 13:27:27 +0200 Subject: [PATCH 03/54] Added address and port to cache span. --- sentry_sdk/consts.py | 4 +-- sentry_sdk/integrations/django/caching.py | 36 ++++++++++++++++------- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index 871589b4f5..dd8e68ae53 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -233,8 +233,8 @@ class SPANDATA: class OP: - CACHE_GET_ITEM = "cache.get_item" - CACHE_SET_ITEM = "cache.set_item" + CACHE_GET_ITEM = "cache.get" + CACHE_SET_ITEM = "cache.set" DB = "db" DB_REDIS = "db.redis" EVENT_DJANGO = "event.django" diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index 369e9f93a6..7fb6e34eed 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -50,17 +50,15 @@ def _get_span_description(method_name, args, kwargs): return description -def _patch_cache_method(cache, method_name): - # type: (CacheHandler, str) -> None +def _patch_cache_method(cache, method_name, address, port): + # type: (CacheHandler, str, Optional[str], Optional[str]) -> None from sentry_sdk.integrations.django import DjangoIntegration original_method = getattr(cache, method_name) @ensure_integration_enabled(DjangoIntegration, original_method) - def _instrument_call(cache, method_name, original_method, args, kwargs): - # type: (CacheHandler, str, Callable[..., Any], Any, Any) -> Any - import ipdb; ipdb.set_trace() - + def _instrument_call(cache, method_name, original_method, args, kwargs, address, port): + # type: (CacheHandler, str, Callable[..., Any], Any, Any, Optional[str], Optional[str]) -> Any is_set_operation = method_name == "set" op = OP.CACHE_SET_ITEM if is_set_operation else OP.CACHE_GET_ITEM @@ -69,6 +67,12 @@ def _instrument_call(cache, method_name, original_method, args, kwargs): with sentry_sdk.start_span(op=op, description=description) as span: value = original_method(*args, **kwargs) + if address is not None: + span.set_data(SPANDATA.NETWORK_PEER_ADDRESS, address) + + if port is not None: + span.set_data(SPANDATA.NETWORK_PEER_PORT, port) + key = _get_key(args, kwargs) if key != "": span.set_data(SPANDATA.CACHE_KEY, key) @@ -92,17 +96,17 @@ def _instrument_call(cache, method_name, original_method, args, kwargs): @functools.wraps(original_method) def sentry_method(*args, **kwargs): # type: (*Any, **Any) -> Any - return _instrument_call(cache, method_name, original_method, args, kwargs) + return _instrument_call(cache, method_name, original_method, args, kwargs, address, port) setattr(cache, method_name, sentry_method) -def _patch_cache(cache): - # type: (CacheHandler) -> None +def _patch_cache(cache, address, port): + # type: (CacheHandler, Optional[str], Optional[str]) -> None if not hasattr(cache, "_sentry_patched"): # TODO: Here we can also patch the set methods (see TODO above) for method_name in METHODS_TO_INSTRUMENT: - _patch_cache_method(cache, method_name) + _patch_cache_method(cache, method_name, address, port) cache._sentry_patched = True @@ -138,7 +142,17 @@ def sentry_create_connection(self, alias): integration = sentry_sdk.get_client().get_integration(DjangoIntegration) if integration is not None and integration.cache_spans: - _patch_cache(cache) + settings = self.settings[alias or "default"] + # TODO: location can also be an array of locations + # see: https://docs.djangoproject.com/en/5.0/topics/cache/#redis + location = settings.get("LOCATION") + if isinstance(location, str): + if ":" in location: + address, port = location.rsplit(":", 1) + else: + address, port = location, None + + _patch_cache(cache, address, port) return cache From 5ab4ce0cc6ad131b450c829fdd82d8f7e7e40fc7 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 24 Apr 2024 14:01:46 +0200 Subject: [PATCH 04/54] Formatting --- sentry_sdk/consts.py | 6 +++--- sentry_sdk/integrations/django/caching.py | 22 ++++++++++++++-------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index dd8e68ae53..5ede68d9bd 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -133,15 +133,15 @@ class SPANDATA: CACHE_KEY = "cache.key" """ """ - + CACHE_TTL = "cache.ttl" """ """ - + NETWORK_PEER_ADDRESS = "network.peer.address" """ """ - + NETWORK_PEER_PORT = "network.peer.port" """ """ diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index 7fb6e34eed..7f521bb9a9 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -19,11 +19,13 @@ # TODO: for also creating spans for setting cache there is a `set`, `aset`, `add`, `aadd` methods # see https://github.com/django/django/blob/main/django/core/cache/backends/base.py METHODS_TO_INSTRUMENT = [ - # "set", + "set", + "set_many", "get", "get_many", ] + def _get_timeout(args, kwargs): # type: (Any, Any) -> Optional[int] if args is not None and len(args) >= 3: @@ -44,8 +46,8 @@ def _get_key(args, kwargs): def _get_span_description(method_name, args, kwargs): # type: (str, Any, Any) -> str description = "{} {}".format( - method_name, - _get_key(args, kwargs), + method_name, + _get_key(args, kwargs), ) return description @@ -57,9 +59,11 @@ def _patch_cache_method(cache, method_name, address, port): original_method = getattr(cache, method_name) @ensure_integration_enabled(DjangoIntegration, original_method) - def _instrument_call(cache, method_name, original_method, args, kwargs, address, port): + def _instrument_call( + cache, method_name, original_method, args, kwargs, address, port + ): # type: (CacheHandler, str, Callable[..., Any], Any, Any, Optional[str], Optional[str]) -> Any - is_set_operation = method_name == "set" + is_set_operation = method_name.startswith("set") op = OP.CACHE_SET_ITEM if is_set_operation else OP.CACHE_GET_ITEM description = _get_span_description(method_name, args, kwargs) @@ -69,7 +73,7 @@ def _instrument_call(cache, method_name, original_method, args, kwargs, address, if address is not None: span.set_data(SPANDATA.NETWORK_PEER_ADDRESS, address) - + if port is not None: span.set_data(SPANDATA.NETWORK_PEER_PORT, port) @@ -96,12 +100,14 @@ def _instrument_call(cache, method_name, original_method, args, kwargs, address, @functools.wraps(original_method) def sentry_method(*args, **kwargs): # type: (*Any, **Any) -> Any - return _instrument_call(cache, method_name, original_method, args, kwargs, address, port) + return _instrument_call( + cache, method_name, original_method, args, kwargs, address, port + ) setattr(cache, method_name, sentry_method) -def _patch_cache(cache, address, port): +def _patch_cache(cache, address=None, port=None): # type: (CacheHandler, Optional[str], Optional[str]) -> None if not hasattr(cache, "_sentry_patched"): # TODO: Here we can also patch the set methods (see TODO above) From ae7a995cf39700771f5b5af4be9d8fa6ff33fba4 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 24 Apr 2024 14:34:14 +0200 Subject: [PATCH 05/54] Save correct itemsize --- sentry_sdk/consts.py | 4 ---- sentry_sdk/integrations/django/caching.py | 29 +++++++---------------- 2 files changed, 8 insertions(+), 25 deletions(-) diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index 5ede68d9bd..749f944c01 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -134,10 +134,6 @@ class SPANDATA: """ """ - CACHE_TTL = "cache.ttl" - """ - """ - NETWORK_PEER_ADDRESS = "network.peer.address" """ """ diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index 7f521bb9a9..e76340939b 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -15,24 +15,12 @@ from typing import Optional -# TODO: In new Django there is also `aget` and `aget_many` methods -# TODO: for also creating spans for setting cache there is a `set`, `aset`, `add`, `aadd` methods -# see https://github.com/django/django/blob/main/django/core/cache/backends/base.py METHODS_TO_INSTRUMENT = [ "set", - "set_many", "get", - "get_many", ] -def _get_timeout(args, kwargs): - # type: (Any, Any) -> Optional[int] - if args is not None and len(args) >= 3: - return args[2] - return None - - def _get_key(args, kwargs): # type: (Any, Any) -> str if args is not None and len(args) >= 1: @@ -64,6 +52,7 @@ def _instrument_call( ): # type: (CacheHandler, str, Callable[..., Any], Any, Any, Optional[str], Optional[str]) -> Any is_set_operation = method_name.startswith("set") + is_get_operation = not is_set_operation op = OP.CACHE_SET_ITEM if is_set_operation else OP.CACHE_GET_ITEM description = _get_span_description(method_name, args, kwargs) @@ -81,19 +70,17 @@ def _instrument_call( if key != "": span.set_data(SPANDATA.CACHE_KEY, key) - if is_set_operation: - timeout = _get_timeout(args, kwargs) - if timeout is not None: - span.set_data(SPANDATA.CACHE_TTL, timeout) - else: + item_size = None + if is_get_operation: if value: + item_size = len(str(value)) span.set_data(SPANDATA.CACHE_HIT, True) - - size = len(str(value)) - span.set_data(SPANDATA.CACHE_ITEM_SIZE, size) - else: span.set_data(SPANDATA.CACHE_HIT, False) + else: + item_size = len(str(args[1])) + + span.set_data(SPANDATA.CACHE_ITEM_SIZE, item_size) return value From 11f7e1553d698cff482a8bbd3246620d5704f712 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 24 Apr 2024 14:52:24 +0200 Subject: [PATCH 06/54] Made cache item_size optional --- sentry_sdk/integrations/django/__init__.py | 8 ++++++-- sentry_sdk/integrations/django/caching.py | 10 +++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index bf2648b6bd..6f414aadc5 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -119,8 +119,9 @@ def __init__( signals_spans=True, cache_spans=False, signals_denylist=None, + cache_spans_add_item_size=False, ): - # type: (str, bool, bool, bool, Optional[list[signals.Signal]]) -> None + # type: (str, bool, bool, bool, Optional[list[signals.Signal]], bool) -> None if transaction_style not in TRANSACTION_STYLE_VALUES: raise ValueError( "Invalid value for transaction_style: %s (must be in %s)" @@ -128,10 +129,13 @@ def __init__( ) self.transaction_style = transaction_style self.middleware_spans = middleware_spans + self.signals_spans = signals_spans - self.cache_spans = cache_spans self.signals_denylist = signals_denylist or [] + self.cache_spans = cache_spans + self.cache_spans_add_item_size = cache_spans_add_item_size + @staticmethod def setup_once(): # type: () -> None diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index e76340939b..2257910828 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -51,6 +51,7 @@ def _instrument_call( cache, method_name, original_method, args, kwargs, address, port ): # type: (CacheHandler, str, Callable[..., Any], Any, Any, Optional[str], Optional[str]) -> Any + integration = sentry_sdk.get_client().get_integration(DjangoIntegration) is_set_operation = method_name.startswith("set") is_get_operation = not is_set_operation @@ -73,14 +74,17 @@ def _instrument_call( item_size = None if is_get_operation: if value: - item_size = len(str(value)) + if integration.cache_spans_add_item_size: + item_size = len(str(value)) span.set_data(SPANDATA.CACHE_HIT, True) else: span.set_data(SPANDATA.CACHE_HIT, False) else: - item_size = len(str(args[1])) + if integration.cache_spans_add_item_size: + item_size = len(str(args[1])) - span.set_data(SPANDATA.CACHE_ITEM_SIZE, item_size) + if item_size is not None: + span.set_data(SPANDATA.CACHE_ITEM_SIZE, item_size) return value From 9ba4ccf63e08e255402659b6d73be42e58f78387 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 24 Apr 2024 15:29:28 +0200 Subject: [PATCH 07/54] Added some documentation --- sentry_sdk/integrations/django/__init__.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index 6f414aadc5..e83b93fefc 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -104,6 +104,17 @@ def is_authenticated(request_user): class DjangoIntegration(Integration): + """ + Auto instrument a Django applications. + + :param transaction_style: How to derive transaction names. Either `"function_name"` or `"url"`. Defaults to `"url"`. + :param middleware_spans: Whether to create spans for middleware. Defaults to `True`. + :param signals_spans: Whether to create spans for signals. Defaults to `True`. + :param signals_denylist: A list of signals to ignore when creating spans. + :param cache_spans: Whether to create spans for cache operations. Defaults to `False`. + :param cache_spans_add_item_size: Whether to add the size of the cache item to the span data. Only applicable when `cache_spans` is set to `True`. Defaults to `False`. + """ + identifier = "django" transaction_style = "" From 7a4e0cdc35663061a4052ff5d59e126a469f85d8 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 24 Apr 2024 15:36:33 +0200 Subject: [PATCH 08/54] explaining docs --- sentry_sdk/consts.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index 749f944c01..c6fe69e9b2 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -132,14 +132,20 @@ class SPANDATA: CACHE_KEY = "cache.key" """ + The key of the requested data. + Example: template.cache.some_item.867da7e2af8e6b2f3aa7213a4080edb3 """ NETWORK_PEER_ADDRESS = "network.peer.address" """ + Peer address of the network connection - IP address or Unix domain socket name. + Example: 10.1.2.80, /tmp/my.sock, localhost """ NETWORK_PEER_PORT = "network.peer.port" """ + Peer port number of the network connection. + Example: 6379 """ HTTP_QUERY = "http.query" From 8d51038d0b69cfd99d58d11465c6cbfbebedd9cd Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 29 Apr 2024 15:01:41 +0200 Subject: [PATCH 09/54] Testing cache module --- .../lambda_function.py | 18 +- sentry_sdk/integrations/django/caching.py | 1 - tests/integrations/django/test_basic.py | 271 +------------- .../integrations/django/test_cache_module.py | 348 ++++++++++++++++++ 4 files changed, 358 insertions(+), 280 deletions(-) create mode 100644 tests/integrations/django/test_cache_module.py diff --git a/scripts/aws_lambda_functions/sentryPythonDeleteTestFunctions/lambda_function.py b/scripts/aws_lambda_functions/sentryPythonDeleteTestFunctions/lambda_function.py index 1fc3994176..ce7afb6aa4 100644 --- a/scripts/aws_lambda_functions/sentryPythonDeleteTestFunctions/lambda_function.py +++ b/scripts/aws_lambda_functions/sentryPythonDeleteTestFunctions/lambda_function.py @@ -1,12 +1,12 @@ import boto3 -import sentry_sdk +import sentry_sdk monitor_slug = "python-sdk-aws-lambda-tests-cleanup" monitor_config = { "schedule": { "type": "crontab", - "value": "0 12 * * 0", # 12 o'clock on Sunday + "value": "0 12 * * 0", # 12 o'clock on Sunday }, "timezone": "UTC", "checkin_margin": 2, @@ -24,7 +24,7 @@ def delete_lambda_functions(prefix="test_"): """ client = boto3.client("lambda", region_name="us-east-1") functions_deleted = 0 - + functions_paginator = client.get_paginator("list_functions") for functions_page in functions_paginator.paginate(): for func in functions_page["Functions"]: @@ -39,17 +39,17 @@ def delete_lambda_functions(prefix="test_"): print(f"Got exception: {ex}") return functions_deleted - + def lambda_handler(event, context): functions_deleted = delete_lambda_functions() - + sentry_sdk.metrics.gauge( - key="num_aws_functions_deleted", + key="num_aws_functions_deleted", value=functions_deleted, ) - + return { - 'statusCode': 200, - 'body': f"{functions_deleted} AWS Lambda functions deleted successfully." + "statusCode": 200, + "body": f"{functions_deleted} AWS Lambda functions deleted successfully.", } diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index 2257910828..b08e821232 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -101,7 +101,6 @@ def sentry_method(*args, **kwargs): def _patch_cache(cache, address=None, port=None): # type: (CacheHandler, Optional[str], Optional[str]) -> None if not hasattr(cache, "_sentry_patched"): - # TODO: Here we can also patch the set methods (see TODO above) for method_name in METHODS_TO_INSTRUMENT: _patch_cache_method(cache, method_name, address, port) cache._sentry_patched = True diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py index 88cf413f47..5e1529c762 100644 --- a/tests/integrations/django/test_basic.py +++ b/tests/integrations/django/test_basic.py @@ -1,6 +1,5 @@ import json import os -import random import re import pytest from functools import partial @@ -22,11 +21,10 @@ from sentry_sdk.consts import SPANDATA from sentry_sdk.integrations.django import DjangoIntegration, _set_db_data from sentry_sdk.integrations.django.signals_handlers import _get_receiver_name -from sentry_sdk.integrations.django.caching import _get_span_description from sentry_sdk.integrations.executing import ExecutingIntegration from sentry_sdk.scope import Scope from sentry_sdk.tracing import Span -from tests.conftest import ApproxDict, unpack_werkzeug_response +from tests.conftest import unpack_werkzeug_response from tests.integrations.django.myapp.wsgi import application from tests.integrations.django.myapp.signals import myapp_custom_signal_silenced from tests.integrations.django.utils import pytest_mark_django_db_decorator @@ -39,36 +37,6 @@ def client(): return Client(application) -@pytest.fixture -def use_django_caching(settings): - settings.CACHES = { - "default": { - "BACKEND": "django.core.cache.backends.locmem.LocMemCache", - "LOCATION": "unique-snowflake-%s" % random.randint(1, 1000000), - } - } - - -@pytest.fixture -def use_django_caching_with_middlewares(settings): - settings.CACHES = { - "default": { - "BACKEND": "django.core.cache.backends.locmem.LocMemCache", - "LOCATION": "unique-snowflake-%s" % random.randint(1, 1000000), - } - } - if hasattr(settings, "MIDDLEWARE"): - middleware = settings.MIDDLEWARE - elif hasattr(settings, "MIDDLEWARE_CLASSES"): - middleware = settings.MIDDLEWARE_CLASSES - else: - middleware = None - - if middleware is not None: - middleware.insert(0, "django.middleware.cache.UpdateCacheMiddleware") - middleware.append("django.middleware.cache.FetchFromCacheMiddleware") - - def test_view_exceptions(sentry_init, client, capture_exceptions, capture_events): sentry_init(integrations=[DjangoIntegration()], send_default_pii=True) exceptions = capture_exceptions() @@ -1158,240 +1126,3 @@ def dummy(a, b): assert name == "functools.partial()" else: assert name == "partial()" - - -@pytest.mark.forked -@pytest_mark_django_db_decorator() -@pytest.mark.skipif(DJANGO_VERSION < (1, 9), reason="Requires Django >= 1.9") -def test_cache_spans_disabled_middleware( - sentry_init, client, capture_events, use_django_caching_with_middlewares -): - sentry_init( - integrations=[ - DjangoIntegration( - cache_spans=False, - middleware_spans=False, - signals_spans=False, - ) - ], - traces_sample_rate=1.0, - ) - events = capture_events() - - client.get(reverse("not_cached_view")) - client.get(reverse("not_cached_view")) - - (first_event, second_event) = events - assert len(first_event["spans"]) == 0 - assert len(second_event["spans"]) == 0 - - -@pytest.mark.forked -@pytest_mark_django_db_decorator() -@pytest.mark.skipif(DJANGO_VERSION < (1, 9), reason="Requires Django >= 1.9") -def test_cache_spans_disabled_decorator( - sentry_init, client, capture_events, use_django_caching -): - sentry_init( - integrations=[ - DjangoIntegration( - cache_spans=False, - middleware_spans=False, - signals_spans=False, - ) - ], - traces_sample_rate=1.0, - ) - events = capture_events() - - client.get(reverse("cached_view")) - client.get(reverse("cached_view")) - - (first_event, second_event) = events - assert len(first_event["spans"]) == 0 - assert len(second_event["spans"]) == 0 - - -@pytest.mark.forked -@pytest_mark_django_db_decorator() -@pytest.mark.skipif(DJANGO_VERSION < (1, 9), reason="Requires Django >= 1.9") -def test_cache_spans_disabled_templatetag( - sentry_init, client, capture_events, use_django_caching -): - sentry_init( - integrations=[ - DjangoIntegration( - cache_spans=False, - middleware_spans=False, - signals_spans=False, - ) - ], - traces_sample_rate=1.0, - ) - events = capture_events() - - client.get(reverse("view_with_cached_template_fragment")) - client.get(reverse("view_with_cached_template_fragment")) - - (first_event, second_event) = events - assert len(first_event["spans"]) == 0 - assert len(second_event["spans"]) == 0 - - -@pytest.mark.forked -@pytest_mark_django_db_decorator() -@pytest.mark.skipif(DJANGO_VERSION < (1, 9), reason="Requires Django >= 1.9") -def test_cache_spans_middleware( - sentry_init, client, capture_events, use_django_caching_with_middlewares -): - sentry_init( - integrations=[ - DjangoIntegration( - cache_spans=True, - middleware_spans=False, - signals_spans=False, - ) - ], - traces_sample_rate=1.0, - ) - - client.application.load_middleware() - events = capture_events() - - client.get(reverse("not_cached_view")) - client.get(reverse("not_cached_view")) - - (first_event, second_event) = events - assert len(first_event["spans"]) == 1 - assert first_event["spans"][0]["op"] == "cache.get_item" - assert first_event["spans"][0]["description"].startswith( - "get views.decorators.cache.cache_header." - ) - assert first_event["spans"][0]["data"] == ApproxDict({"cache.hit": False}) - - assert len(second_event["spans"]) == 2 - assert second_event["spans"][0]["op"] == "cache.get_item" - assert second_event["spans"][0]["description"].startswith( - "get views.decorators.cache.cache_header." - ) - assert second_event["spans"][0]["data"] == ApproxDict({"cache.hit": False}) - - assert second_event["spans"][1]["op"] == "cache.get_item" - assert second_event["spans"][1]["description"].startswith( - "get views.decorators.cache.cache_page." - ) - assert second_event["spans"][1]["data"]["cache.hit"] - assert "cache.item_size" in second_event["spans"][1]["data"] - - -@pytest.mark.forked -@pytest_mark_django_db_decorator() -@pytest.mark.skipif(DJANGO_VERSION < (1, 9), reason="Requires Django >= 1.9") -def test_cache_spans_decorator(sentry_init, client, capture_events, use_django_caching): - sentry_init( - integrations=[ - DjangoIntegration( - cache_spans=True, - middleware_spans=False, - signals_spans=False, - ) - ], - traces_sample_rate=1.0, - ) - events = capture_events() - - client.get(reverse("cached_view")) - client.get(reverse("cached_view")) - - (first_event, second_event) = events - assert len(first_event["spans"]) == 1 - assert first_event["spans"][0]["op"] == "cache.get_item" - assert first_event["spans"][0]["description"].startswith( - "get views.decorators.cache.cache_header." - ) - assert first_event["spans"][0]["data"] == ApproxDict({"cache.hit": False}) - - assert len(second_event["spans"]) == 2 - assert second_event["spans"][0]["op"] == "cache.get_item" - assert second_event["spans"][0]["description"].startswith( - "get views.decorators.cache.cache_header." - ) - assert second_event["spans"][0]["data"] == ApproxDict({"cache.hit": False}) - - assert second_event["spans"][1]["op"] == "cache.get_item" - assert second_event["spans"][1]["description"].startswith( - "get views.decorators.cache.cache_page." - ) - assert second_event["spans"][1]["data"]["cache.hit"] - assert "cache.item_size" in second_event["spans"][1]["data"] - - -@pytest.mark.forked -@pytest_mark_django_db_decorator() -@pytest.mark.skipif(DJANGO_VERSION < (1, 9), reason="Requires Django >= 1.9") -def test_cache_spans_templatetag( - sentry_init, client, capture_events, use_django_caching -): - sentry_init( - integrations=[ - DjangoIntegration( - cache_spans=True, - middleware_spans=False, - signals_spans=False, - ) - ], - traces_sample_rate=1.0, - ) - events = capture_events() - - client.get(reverse("view_with_cached_template_fragment")) - client.get(reverse("view_with_cached_template_fragment")) - - (first_event, second_event) = events - assert len(first_event["spans"]) == 1 - assert first_event["spans"][0]["op"] == "cache.get_item" - assert first_event["spans"][0]["description"].startswith( - "get template.cache.some_identifier." - ) - assert first_event["spans"][0]["data"] == ApproxDict({"cache.hit": False}) - - assert len(second_event["spans"]) == 1 - assert second_event["spans"][0]["op"] == "cache.get_item" - assert second_event["spans"][0]["description"].startswith( - "get template.cache.some_identifier." - ) - assert second_event["spans"][0]["data"]["cache.hit"] - assert "cache.item_size" in second_event["spans"][0]["data"] - - -@pytest.mark.parametrize( - "method_name, args, kwargs, expected_description", - [ - ("get", None, None, "get "), - ("get", [], {}, "get "), - ("get", ["bla", "blub", "foo"], {}, "get bla"), - ( - "get_many", - [["bla 1", "bla 2", "bla 3"], "blub", "foo"], - {}, - "get_many ['bla 1', 'bla 2', 'bla 3']", - ), - ( - "get_many", - [["bla 1", "bla 2", "bla 3"], "blub", "foo"], - {"key": "bar"}, - "get_many ['bla 1', 'bla 2', 'bla 3']", - ), - ("get", [], {"key": "bar"}, "get bar"), - ( - "get", - "something", - {}, - "get s", - ), # this should never happen, just making sure that we are not raising an exception in that case. - ], -) -def test_cache_spans_get_span_description( - method_name, args, kwargs, expected_description -): - assert _get_span_description(method_name, args, kwargs) == expected_description diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py new file mode 100644 index 0000000000..e3d8be86c1 --- /dev/null +++ b/tests/integrations/django/test_cache_module.py @@ -0,0 +1,348 @@ +import pytest +import random + +from django import VERSION as DJANGO_VERSION + +from werkzeug.test import Client + +try: + from django.urls import reverse +except ImportError: + from django.core.urlresolvers import reverse + +from sentry_sdk.integrations.django import DjangoIntegration +from sentry_sdk.integrations.django.caching import _get_span_description +from tests.integrations.django.myapp.wsgi import application +from tests.integrations.django.utils import pytest_mark_django_db_decorator + + +DJANGO_VERSION = DJANGO_VERSION[:2] + + +@pytest.fixture +def client(): + return Client(application) + + +@pytest.fixture +def use_django_caching(settings): + settings.CACHES = { + "default": { + "BACKEND": "django.core.cache.backends.locmem.LocMemCache", + "LOCATION": "unique-snowflake-%s" % random.randint(1, 1000000), + } + } + + +@pytest.fixture +def use_django_caching_with_middlewares(settings): + settings.CACHES = { + "default": { + "BACKEND": "django.core.cache.backends.locmem.LocMemCache", + "LOCATION": "unique-snowflake-%s" % random.randint(1, 1000000), + } + } + if hasattr(settings, "MIDDLEWARE"): + middleware = settings.MIDDLEWARE + elif hasattr(settings, "MIDDLEWARE_CLASSES"): + middleware = settings.MIDDLEWARE_CLASSES + else: + middleware = None + + if middleware is not None: + middleware.insert(0, "django.middleware.cache.UpdateCacheMiddleware") + middleware.append("django.middleware.cache.FetchFromCacheMiddleware") + + +@pytest.mark.forked +@pytest_mark_django_db_decorator() +@pytest.mark.skipif(DJANGO_VERSION < (1, 9), reason="Requires Django >= 1.9") +def test_cache_spans_disabled_middleware( + sentry_init, client, capture_events, use_django_caching_with_middlewares +): + sentry_init( + integrations=[ + DjangoIntegration( + cache_spans=False, + middleware_spans=False, + signals_spans=False, + ) + ], + traces_sample_rate=1.0, + ) + events = capture_events() + + client.get(reverse("not_cached_view")) + client.get(reverse("not_cached_view")) + + (first_event, second_event) = events + assert len(first_event["spans"]) == 0 + assert len(second_event["spans"]) == 0 + + +@pytest.mark.forked +@pytest_mark_django_db_decorator() +@pytest.mark.skipif(DJANGO_VERSION < (1, 9), reason="Requires Django >= 1.9") +def test_cache_spans_disabled_decorator( + sentry_init, client, capture_events, use_django_caching +): + sentry_init( + integrations=[ + DjangoIntegration( + cache_spans=False, + middleware_spans=False, + signals_spans=False, + ) + ], + traces_sample_rate=1.0, + ) + events = capture_events() + + client.get(reverse("cached_view")) + client.get(reverse("cached_view")) + + (first_event, second_event) = events + assert len(first_event["spans"]) == 0 + assert len(second_event["spans"]) == 0 + + +@pytest.mark.forked +@pytest_mark_django_db_decorator() +@pytest.mark.skipif(DJANGO_VERSION < (1, 9), reason="Requires Django >= 1.9") +def test_cache_spans_disabled_templatetag( + sentry_init, client, capture_events, use_django_caching +): + sentry_init( + integrations=[ + DjangoIntegration( + cache_spans=False, + middleware_spans=False, + signals_spans=False, + ) + ], + traces_sample_rate=1.0, + ) + events = capture_events() + + client.get(reverse("view_with_cached_template_fragment")) + client.get(reverse("view_with_cached_template_fragment")) + + (first_event, second_event) = events + assert len(first_event["spans"]) == 0 + assert len(second_event["spans"]) == 0 + + +# @pytest.mark.forked +@pytest_mark_django_db_decorator() +@pytest.mark.skipif(DJANGO_VERSION < (1, 9), reason="Requires Django >= 1.9") +def test_cache_spans_middleware( + sentry_init, client, capture_events, use_django_caching_with_middlewares +): + sentry_init( + integrations=[ + DjangoIntegration( + cache_spans=True, + middleware_spans=False, + signals_spans=False, + ) + ], + traces_sample_rate=1.0, + ) + + client.application.load_middleware() + events = capture_events() + + client.get(reverse("not_cached_view")) + client.get(reverse("not_cached_view")) + + (first_event, second_event) = events + # first_event - cache.get + assert first_event["spans"][0]["op"] == "cache.get" + assert first_event["spans"][0]["description"].startswith( + "get views.decorators.cache.cache_header." + ) + assert first_event["spans"][0]["data"]["network.peer.address"] is not None + assert first_event["spans"][0]["data"]["cache.key"].startswith( + "views.decorators.cache.cache_header." + ) + assert not first_event["spans"][0]["data"]["cache.hit"] + assert "cache.item_size" not in first_event["spans"][0]["data"] + # first_event - cache.set + assert first_event["spans"][1]["op"] == "cache.set" + assert first_event["spans"][1]["description"].startswith( + "set views.decorators.cache.cache_header." + ) + assert first_event["spans"][1]["data"]["network.peer.address"] is not None + assert first_event["spans"][1]["data"]["cache.key"].startswith( + "views.decorators.cache.cache_header." + ) + assert "cache.hit" not in first_event["spans"][1]["data"] + assert "cache.item_size" not in first_event["spans"][1]["data"] + # second_event - cache.get + assert second_event["spans"][0]["op"] == "cache.get" + assert second_event["spans"][0]["description"].startswith( + "get views.decorators.cache.cache_header." + ) + assert second_event["spans"][0]["data"]["network.peer.address"] is not None + assert second_event["spans"][0]["data"]["cache.key"].startswith( + "views.decorators.cache.cache_header." + ) + assert not second_event["spans"][0]["data"]["cache.hit"] + assert "cache.item_size" not in second_event["spans"][0]["data"] + # second_event - cache.get 2 + assert second_event["spans"][1]["op"] == "cache.get" + assert second_event["spans"][1]["description"].startswith( + "get views.decorators.cache.cache_page." + ) + assert second_event["spans"][1]["data"]["network.peer.address"] is not None + assert second_event["spans"][1]["data"]["cache.key"].startswith( + "views.decorators.cache.cache_page." + ) + assert second_event["spans"][1]["data"]["cache.hit"] + assert "cache.item_size" not in second_event["spans"][1]["data"] + + +@pytest.mark.forked +@pytest_mark_django_db_decorator() +@pytest.mark.skipif(DJANGO_VERSION < (1, 9), reason="Requires Django >= 1.9") +def test_cache_spans_decorator(sentry_init, client, capture_events, use_django_caching): + sentry_init( + integrations=[ + DjangoIntegration( + cache_spans=True, + middleware_spans=False, + signals_spans=False, + ) + ], + traces_sample_rate=1.0, + ) + events = capture_events() + + client.get(reverse("cached_view")) + client.get(reverse("cached_view")) + + (first_event, second_event) = events + # first_event - cache.get + assert first_event["spans"][0]["op"] == "cache.get" + assert first_event["spans"][0]["description"].startswith( + "get views.decorators.cache.cache_header." + ) + assert first_event["spans"][0]["data"]["network.peer.address"] is not None + assert first_event["spans"][0]["data"]["cache.key"].startswith( + "views.decorators.cache.cache_header." + ) + assert not first_event["spans"][0]["data"]["cache.hit"] + assert "cache.item_size" not in first_event["spans"][0]["data"] + # first_event - cache.set + assert first_event["spans"][1]["op"] == "cache.set" + assert first_event["spans"][1]["description"].startswith( + "set views.decorators.cache.cache_header." + ) + assert first_event["spans"][1]["data"]["network.peer.address"] is not None + assert first_event["spans"][1]["data"]["cache.key"].startswith( + "views.decorators.cache.cache_header." + ) + assert "cache.hit" not in first_event["spans"][1]["data"] + assert "cache.item_size" not in first_event["spans"][1]["data"] + # second_event - cache.get + assert second_event["spans"][1]["op"] == "cache.get" + assert second_event["spans"][1]["description"].startswith( + "get views.decorators.cache.cache_page." + ) + assert second_event["spans"][1]["data"]["network.peer.address"] is not None + assert second_event["spans"][1]["data"]["cache.key"].startswith( + "views.decorators.cache.cache_page." + ) + assert second_event["spans"][1]["data"]["cache.hit"] + assert "cache.item_size" not in second_event["spans"][0]["data"] + + +# @pytest.mark.forked +@pytest_mark_django_db_decorator() +@pytest.mark.skipif(DJANGO_VERSION < (1, 9), reason="Requires Django >= 1.9") +def test_cache_spans_templatetag( + sentry_init, client, capture_events, use_django_caching +): + sentry_init( + integrations=[ + DjangoIntegration( + cache_spans=True, + middleware_spans=False, + signals_spans=False, + ) + ], + traces_sample_rate=1.0, + ) + events = capture_events() + + client.get(reverse("view_with_cached_template_fragment")) + client.get(reverse("view_with_cached_template_fragment")) + + (first_event, second_event) = events + assert len(first_event["spans"]) == 2 + # first_event - cache.get + assert first_event["spans"][0]["op"] == "cache.get" + assert first_event["spans"][0]["description"].startswith( + "get template.cache.some_identifier." + ) + assert first_event["spans"][0]["data"]["network.peer.address"] is not None + assert first_event["spans"][0]["data"]["cache.key"].startswith( + "template.cache.some_identifier." + ) + assert not first_event["spans"][0]["data"]["cache.hit"] + assert "cache.item_size" not in first_event["spans"][0]["data"] + # first_event - cache.set + assert first_event["spans"][1]["op"] == "cache.set" + assert first_event["spans"][1]["description"].startswith( + "set template.cache.some_identifier." + ) + assert first_event["spans"][1]["data"]["network.peer.address"] is not None + assert first_event["spans"][1]["data"]["cache.key"].startswith( + "template.cache.some_identifier." + ) + assert "cache.hit" not in first_event["spans"][1]["data"] + assert "cache.item_size" not in first_event["spans"][1]["data"] + # second_event - cache.get + assert second_event["spans"][0]["op"] == "cache.get" + assert second_event["spans"][0]["description"].startswith( + "get template.cache.some_identifier." + ) + assert second_event["spans"][0]["data"]["network.peer.address"] is not None + assert second_event["spans"][0]["data"]["cache.key"].startswith( + "template.cache.some_identifier." + ) + assert second_event["spans"][0]["data"]["cache.hit"] + assert "cache.item_size" not in second_event["spans"][0]["data"] + + +@pytest.mark.parametrize( + "method_name, args, kwargs, expected_description", + [ + ("get", None, None, "get "), + ("get", [], {}, "get "), + ("get", ["bla", "blub", "foo"], {}, "get bla"), + ( + "get_many", + [["bla 1", "bla 2", "bla 3"], "blub", "foo"], + {}, + "get_many ['bla 1', 'bla 2', 'bla 3']", + ), + ( + "get_many", + [["bla 1", "bla 2", "bla 3"], "blub", "foo"], + {"key": "bar"}, + "get_many ['bla 1', 'bla 2', 'bla 3']", + ), + ("get", [], {"key": "bar"}, "get bar"), + ( + "get", + "something", + {}, + "get s", + ), # this should never happen, just making sure that we are not raising an exception in that case. + ], +) +def test_cache_spans_get_span_description( + method_name, args, kwargs, expected_description +): + assert _get_span_description(method_name, args, kwargs) == expected_description From 9791ec82e470d483f644c964614e49a4e7ccd009 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Mon, 29 Apr 2024 15:28:19 +0200 Subject: [PATCH 10/54] forking tests --- tests/integrations/django/test_cache_module.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index e3d8be86c1..b7ff1bf819 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -132,7 +132,7 @@ def test_cache_spans_disabled_templatetag( assert len(second_event["spans"]) == 0 -# @pytest.mark.forked +@pytest.mark.forked @pytest_mark_django_db_decorator() @pytest.mark.skipif(DJANGO_VERSION < (1, 9), reason="Requires Django >= 1.9") def test_cache_spans_middleware( @@ -257,7 +257,7 @@ def test_cache_spans_decorator(sentry_init, client, capture_events, use_django_c assert "cache.item_size" not in second_event["spans"][0]["data"] -# @pytest.mark.forked +@pytest.mark.forked @pytest_mark_django_db_decorator() @pytest.mark.skipif(DJANGO_VERSION < (1, 9), reason="Requires Django >= 1.9") def test_cache_spans_templatetag( From 3d5d08ae4c9155f94bbb87ebfce0bd40f9199af4 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 3 May 2024 08:57:23 +0200 Subject: [PATCH 11/54] changed op name --- sentry_sdk/consts.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index dc4212cedc..44abdcc799 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -235,8 +235,8 @@ class SPANDATA: class OP: - CACHE_GET_ITEM = "cache.get" - CACHE_SET_ITEM = "cache.set" + CACHE_GET_ITEM = "cache.get_item" + CACHE_SET_ITEM = "cache.set_item" DB = "db" DB_REDIS = "db.redis" EVENT_DJANGO = "event.django" From fa47da202aab26e18519d6d1767c32187c2ddd7d Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 3 May 2024 08:58:17 +0200 Subject: [PATCH 12/54] Updated tests --- .../integrations/django/test_cache_module.py | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index b7ff1bf819..5353ea34e6 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -157,7 +157,7 @@ def test_cache_spans_middleware( (first_event, second_event) = events # first_event - cache.get - assert first_event["spans"][0]["op"] == "cache.get" + assert first_event["spans"][0]["op"] == "cache.get_item" assert first_event["spans"][0]["description"].startswith( "get views.decorators.cache.cache_header." ) @@ -168,7 +168,7 @@ def test_cache_spans_middleware( assert not first_event["spans"][0]["data"]["cache.hit"] assert "cache.item_size" not in first_event["spans"][0]["data"] # first_event - cache.set - assert first_event["spans"][1]["op"] == "cache.set" + assert first_event["spans"][1]["op"] == "cache.set_item" assert first_event["spans"][1]["description"].startswith( "set views.decorators.cache.cache_header." ) @@ -179,7 +179,7 @@ def test_cache_spans_middleware( assert "cache.hit" not in first_event["spans"][1]["data"] assert "cache.item_size" not in first_event["spans"][1]["data"] # second_event - cache.get - assert second_event["spans"][0]["op"] == "cache.get" + assert second_event["spans"][0]["op"] == "cache.get_item" assert second_event["spans"][0]["description"].startswith( "get views.decorators.cache.cache_header." ) @@ -190,7 +190,7 @@ def test_cache_spans_middleware( assert not second_event["spans"][0]["data"]["cache.hit"] assert "cache.item_size" not in second_event["spans"][0]["data"] # second_event - cache.get 2 - assert second_event["spans"][1]["op"] == "cache.get" + assert second_event["spans"][1]["op"] == "cache.get_item" assert second_event["spans"][1]["description"].startswith( "get views.decorators.cache.cache_page." ) @@ -223,7 +223,7 @@ def test_cache_spans_decorator(sentry_init, client, capture_events, use_django_c (first_event, second_event) = events # first_event - cache.get - assert first_event["spans"][0]["op"] == "cache.get" + assert first_event["spans"][0]["op"] == "cache.get_item" assert first_event["spans"][0]["description"].startswith( "get views.decorators.cache.cache_header." ) @@ -234,7 +234,7 @@ def test_cache_spans_decorator(sentry_init, client, capture_events, use_django_c assert not first_event["spans"][0]["data"]["cache.hit"] assert "cache.item_size" not in first_event["spans"][0]["data"] # first_event - cache.set - assert first_event["spans"][1]["op"] == "cache.set" + assert first_event["spans"][1]["op"] == "cache.set_item" assert first_event["spans"][1]["description"].startswith( "set views.decorators.cache.cache_header." ) @@ -245,7 +245,7 @@ def test_cache_spans_decorator(sentry_init, client, capture_events, use_django_c assert "cache.hit" not in first_event["spans"][1]["data"] assert "cache.item_size" not in first_event["spans"][1]["data"] # second_event - cache.get - assert second_event["spans"][1]["op"] == "cache.get" + assert second_event["spans"][1]["op"] == "cache.get_item" assert second_event["spans"][1]["description"].startswith( "get views.decorators.cache.cache_page." ) @@ -281,7 +281,7 @@ def test_cache_spans_templatetag( (first_event, second_event) = events assert len(first_event["spans"]) == 2 # first_event - cache.get - assert first_event["spans"][0]["op"] == "cache.get" + assert first_event["spans"][0]["op"] == "cache.get_item" assert first_event["spans"][0]["description"].startswith( "get template.cache.some_identifier." ) @@ -292,7 +292,7 @@ def test_cache_spans_templatetag( assert not first_event["spans"][0]["data"]["cache.hit"] assert "cache.item_size" not in first_event["spans"][0]["data"] # first_event - cache.set - assert first_event["spans"][1]["op"] == "cache.set" + assert first_event["spans"][1]["op"] == "cache.set_item" assert first_event["spans"][1]["description"].startswith( "set template.cache.some_identifier." ) @@ -303,7 +303,7 @@ def test_cache_spans_templatetag( assert "cache.hit" not in first_event["spans"][1]["data"] assert "cache.item_size" not in first_event["spans"][1]["data"] # second_event - cache.get - assert second_event["spans"][0]["op"] == "cache.get" + assert second_event["spans"][0]["op"] == "cache.get_item" assert second_event["spans"][0]["description"].startswith( "get template.cache.some_identifier." ) From be1d4fdb312c55b73c2f78e4727623a79f8fb2c1 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 7 May 2024 08:25:52 +0200 Subject: [PATCH 13/54] Test for host/port/cluster --- sentry_sdk/integrations/django/caching.py | 57 ++++++++------ .../integrations/django/test_cache_module.py | 74 +++++++++++++++++++ 2 files changed, 107 insertions(+), 24 deletions(-) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index b08e821232..84c67e5044 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -1,12 +1,13 @@ import functools from typing import TYPE_CHECKING +from urllib3.util import parse_url as urlparse from django import VERSION as DJANGO_VERSION from django.core.cache import CacheHandler import sentry_sdk from sentry_sdk.consts import OP, SPANDATA -from sentry_sdk.utils import ensure_integration_enabled +from sentry_sdk.utils import capture_internal_exceptions, ensure_integration_enabled if TYPE_CHECKING: @@ -61,30 +62,37 @@ def _instrument_call( with sentry_sdk.start_span(op=op, description=description) as span: value = original_method(*args, **kwargs) - if address is not None: - span.set_data(SPANDATA.NETWORK_PEER_ADDRESS, address) - - if port is not None: - span.set_data(SPANDATA.NETWORK_PEER_PORT, port) - - key = _get_key(args, kwargs) - if key != "": - span.set_data(SPANDATA.CACHE_KEY, key) - - item_size = None - if is_get_operation: - if value: - if integration.cache_spans_add_item_size: - item_size = len(str(value)) - span.set_data(SPANDATA.CACHE_HIT, True) + with capture_internal_exceptions(): + if address is not None: + parsed_url = urlparse(address) + # Strip eventually contained username and password + address = "{}://{}{}".format( + parsed_url.scheme or "", parsed_url.netloc or "", parsed_url.path or "" + ) + + span.set_data(SPANDATA.NETWORK_PEER_ADDRESS, address) + + if port is not None: + span.set_data(SPANDATA.NETWORK_PEER_PORT, int(port)) + + key = _get_key(args, kwargs) + if key != "": + span.set_data(SPANDATA.CACHE_KEY, key) + + item_size = None + if is_get_operation: + if value: + if integration.cache_spans_add_item_size: + item_size = len(str(value)) + span.set_data(SPANDATA.CACHE_HIT, True) + else: + span.set_data(SPANDATA.CACHE_HIT, False) else: - span.set_data(SPANDATA.CACHE_HIT, False) - else: - if integration.cache_spans_add_item_size: - item_size = len(str(args[1])) + if integration.cache_spans_add_item_size: + item_size = len(str(args[1])) - if item_size is not None: - span.set_data(SPANDATA.CACHE_ITEM_SIZE, item_size) + if item_size is not None: + span.set_data(SPANDATA.CACHE_ITEM_SIZE, item_size) return value @@ -139,9 +147,10 @@ def sentry_create_connection(self, alias): integration = sentry_sdk.get_client().get_integration(DjangoIntegration) if integration is not None and integration.cache_spans: settings = self.settings[alias or "default"] + address, port = None, None + location = settings.get("LOCATION") # TODO: location can also be an array of locations # see: https://docs.djangoproject.com/en/5.0/topics/cache/#redis - location = settings.get("LOCATION") if isinstance(location, str): if ":" in location: address, port = location.rsplit(":", 1) diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index 5353ea34e6..71e7a4fefc 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -54,6 +54,30 @@ def use_django_caching_with_middlewares(settings): middleware.append("django.middleware.cache.FetchFromCacheMiddleware") +@pytest.fixture +def use_django_caching_with_port(settings): + settings.CACHES = { + "default": { + "BACKEND": "django.core.cache.backends.dummy.DummyCache", + "LOCATION": "redis://username:password@127.0.0.1:6379", + } + } + + +@pytest.fixture +def use_django_caching_with_cluster(settings): + settings.CACHES = { + "default": { + "BACKEND": "django.core.cache.backends.dummy.DummyCache", + "LOCATION": [ + "redis://127.0.0.1:6379", + "redis://127.0.0.2:6378", + "redis://127.0.0.3:6377", + ], + } + } + + @pytest.mark.forked @pytest_mark_django_db_decorator() @pytest.mark.skipif(DJANGO_VERSION < (1, 9), reason="Requires Django >= 1.9") @@ -346,3 +370,53 @@ def test_cache_spans_get_span_description( method_name, args, kwargs, expected_description ): assert _get_span_description(method_name, args, kwargs) == expected_description + + +@pytest.mark.forked +@pytest_mark_django_db_decorator() +def test_cache_spans_location_with_port(sentry_init, client, capture_events, use_django_caching_with_port): + sentry_init( + integrations=[ + DjangoIntegration( + cache_spans=True, + middleware_spans=False, + signals_spans=False, + ) + ], + traces_sample_rate=1.0, + ) + events = capture_events() + + client.get(reverse("cached_view")) + client.get(reverse("cached_view")) + + for event in events: + for span in event["spans"]: + assert span["data"]["network.peer.address"] == "redis://127.0.0.1" # Note: the username/password are not included in the address + assert span["data"]["network.peer.port"] == 6379 + + +@pytest.mark.forked +@pytest_mark_django_db_decorator() +def test_cache_spans_location_with_cluster(sentry_init, client, capture_events, use_django_caching_with_cluster): + sentry_init( + integrations=[ + DjangoIntegration( + cache_spans=True, + middleware_spans=False, + signals_spans=False, + ) + ], + traces_sample_rate=1.0, + ) + events = capture_events() + + client.get(reverse("cached_view")) + client.get(reverse("cached_view")) + + for event in events: + for span in event["spans"]: + # because it is a cluster we do not know what host is actually accessed, so we omit the data + assert "network.peer.address" not in span["data"].keys() + assert "network.peer.port" not in span["data"].keys() + From c8d86d4c576087d0e03d3bc29ed103f76f02b69c Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 7 May 2024 08:42:29 +0200 Subject: [PATCH 14/54] More tests --- sentry_sdk/integrations/django/caching.py | 13 +++-- .../integrations/django/test_cache_module.py | 53 ++++++++++++++++--- 2 files changed, 55 insertions(+), 11 deletions(-) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index 84c67e5044..3337301643 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -64,11 +64,14 @@ def _instrument_call( with capture_internal_exceptions(): if address is not None: - parsed_url = urlparse(address) - # Strip eventually contained username and password - address = "{}://{}{}".format( - parsed_url.scheme or "", parsed_url.netloc or "", parsed_url.path or "" - ) + # Strip eventually contained username and password in url style location + if "://" in address: + parsed_url = urlparse(address) + address = "{}://{}{}".format( + parsed_url.scheme or "", + parsed_url.netloc or "", + parsed_url.path or "", + ) span.set_data(SPANDATA.NETWORK_PEER_ADDRESS, address) diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index 71e7a4fefc..15eac91e83 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -70,9 +70,9 @@ def use_django_caching_with_cluster(settings): "default": { "BACKEND": "django.core.cache.backends.dummy.DummyCache", "LOCATION": [ - "redis://127.0.0.1:6379", - "redis://127.0.0.2:6378", - "redis://127.0.0.3:6377", + "redis://127.0.0.1:6379", + "redis://127.0.0.2:6378", + "redis://127.0.0.3:6377", ], } } @@ -374,7 +374,9 @@ def test_cache_spans_get_span_description( @pytest.mark.forked @pytest_mark_django_db_decorator() -def test_cache_spans_location_with_port(sentry_init, client, capture_events, use_django_caching_with_port): +def test_cache_spans_location_with_port( + sentry_init, client, capture_events, use_django_caching_with_port +): sentry_init( integrations=[ DjangoIntegration( @@ -392,13 +394,17 @@ def test_cache_spans_location_with_port(sentry_init, client, capture_events, use for event in events: for span in event["spans"]: - assert span["data"]["network.peer.address"] == "redis://127.0.0.1" # Note: the username/password are not included in the address + assert ( + span["data"]["network.peer.address"] == "redis://127.0.0.1" + ) # Note: the username/password are not included in the address assert span["data"]["network.peer.port"] == 6379 @pytest.mark.forked @pytest_mark_django_db_decorator() -def test_cache_spans_location_with_cluster(sentry_init, client, capture_events, use_django_caching_with_cluster): +def test_cache_spans_location_with_cluster( + sentry_init, client, capture_events, use_django_caching_with_cluster +): sentry_init( integrations=[ DjangoIntegration( @@ -420,3 +426,38 @@ def test_cache_spans_location_with_cluster(sentry_init, client, capture_events, assert "network.peer.address" not in span["data"].keys() assert "network.peer.port" not in span["data"].keys() + +@pytest.mark.forked +@pytest_mark_django_db_decorator() +def test_cache_spans_item_size(sentry_init, client, capture_events, use_django_caching): + sentry_init( + integrations=[ + DjangoIntegration( + cache_spans=True, + middleware_spans=False, + signals_spans=False, + cache_spans_add_item_size=True, + ) + ], + traces_sample_rate=1.0, + ) + events = capture_events() + + client.get(reverse("cached_view")) + + (event,) = events + assert len(event["spans"]) == 3 + assert event["spans"][0]["op"] == "cache.get_item" + assert not event["spans"][0]["data"]["cache.hit"] + assert "cache.item_size" not in event["spans"][0]["data"] + + assert event["spans"][1]["op"] == "cache.set_item" + assert "cache.hit" not in event["spans"][1]["data"] + assert event["spans"][1]["data"]["cache.item_size"] == 2 + + assert event["spans"][2]["op"] == "cache.set_item" + assert "cache.hit" not in event["spans"][2]["data"] + assert event["spans"][2]["data"]["cache.item_size"] == 58 + import pdb + + pdb.set_trace() From c9bff4b2319885944222afec30ca71d1593e9134 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 7 May 2024 08:46:27 +0200 Subject: [PATCH 15/54] Removed debug statement --- tests/integrations/django/test_cache_module.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index 15eac91e83..65e67d87f9 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -458,6 +458,3 @@ def test_cache_spans_item_size(sentry_init, client, capture_events, use_django_c assert event["spans"][2]["op"] == "cache.set_item" assert "cache.hit" not in event["spans"][2]["data"] assert event["spans"][2]["data"]["cache.item_size"] == 58 - import pdb - - pdb.set_trace() From 34743e0e31ae29a12dc5febd7653558db703205d Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 7 May 2024 09:02:15 +0200 Subject: [PATCH 16/54] Even more tests --- .../integrations/django/test_cache_module.py | 34 ++++++++++++------- tox.ini | 2 +- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index 65e67d87f9..422110f9b3 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -427,7 +427,7 @@ def test_cache_spans_location_with_cluster( assert "network.peer.port" not in span["data"].keys() -@pytest.mark.forked +# @pytest.mark.forked @pytest_mark_django_db_decorator() def test_cache_spans_item_size(sentry_init, client, capture_events, use_django_caching): sentry_init( @@ -443,18 +443,28 @@ def test_cache_spans_item_size(sentry_init, client, capture_events, use_django_c ) events = capture_events() + client.get(reverse("cached_view")) client.get(reverse("cached_view")) - (event,) = events - assert len(event["spans"]) == 3 - assert event["spans"][0]["op"] == "cache.get_item" - assert not event["spans"][0]["data"]["cache.hit"] - assert "cache.item_size" not in event["spans"][0]["data"] + (first_event, second_event) = events + assert len(first_event["spans"]) == 3 + assert first_event["spans"][0]["op"] == "cache.get_item" + assert not first_event["spans"][0]["data"]["cache.hit"] + assert "cache.item_size" not in first_event["spans"][0]["data"] - assert event["spans"][1]["op"] == "cache.set_item" - assert "cache.hit" not in event["spans"][1]["data"] - assert event["spans"][1]["data"]["cache.item_size"] == 2 + assert first_event["spans"][1]["op"] == "cache.set_item" + assert "cache.hit" not in first_event["spans"][1]["data"] + assert first_event["spans"][1]["data"]["cache.item_size"] == 2 - assert event["spans"][2]["op"] == "cache.set_item" - assert "cache.hit" not in event["spans"][2]["data"] - assert event["spans"][2]["data"]["cache.item_size"] == 58 + assert first_event["spans"][2]["op"] == "cache.set_item" + assert "cache.hit" not in first_event["spans"][2]["data"] + assert first_event["spans"][2]["data"]["cache.item_size"] == 58 + + assert len(second_event["spans"]) == 2 + assert second_event["spans"][0]["op"] == "cache.get_item" + assert not second_event["spans"][0]["data"]["cache.hit"] + assert "cache.item_size" not in second_event["spans"][0]["data"] + + assert second_event["spans"][1]["op"] == "cache.get_item" + assert second_event["spans"][1]["data"]["cache.hit"] + assert second_event["spans"][1]["data"]["cache.item_size"] == 58 diff --git a/tox.ini b/tox.ini index f1bc0e7a5e..3abaede04b 100644 --- a/tox.ini +++ b/tox.ini @@ -94,7 +94,7 @@ envlist = {py3.6,py3.9}-django-v{2.2} # - Django 3.x {py3.6,py3.9}-django-v{3.0} - {py3.6,py3.11}-django-v{3.2} + {py3.6,py3.9,py3.11}-django-v{3.2} # - Django 4.x {py3.8,py3.11,py3.12}-django-v{4.0,4.1,4.2} # - Django 5.x From 93b539c3ea1ca80caa621067056ca0bed9ee61d4 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 7 May 2024 09:03:18 +0200 Subject: [PATCH 17/54] fix --- tests/integrations/django/test_cache_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index 422110f9b3..fbdcdf53ec 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -427,7 +427,7 @@ def test_cache_spans_location_with_cluster( assert "network.peer.port" not in span["data"].keys() -# @pytest.mark.forked +@pytest.mark.forked @pytest_mark_django_db_decorator() def test_cache_spans_item_size(sentry_init, client, capture_events, use_django_caching): sentry_init( From 3a50d6684ecebf1b7911df7ce33cfd8cce7920c2 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 7 May 2024 09:28:19 +0200 Subject: [PATCH 18/54] Get cache settings also from old Django versions --- sentry_sdk/integrations/django/caching.py | 32 ++++++++++++++--------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index 3337301643..f17a4919a9 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -117,6 +117,20 @@ def _patch_cache(cache, address=None, port=None): cache._sentry_patched = True +def _get_address_port(settings): + address, port = None, None + location = settings.get("LOCATION") + # TODO: location can also be an array of locations + # see: https://docs.djangoproject.com/en/5.0/topics/cache/#redis + if isinstance(location, str): + if ":" in location: + address, port = location.rsplit(":", 1) + else: + address, port = location, None + + return address, port + + def patch_caching(): # type: () -> None from sentry_sdk.integrations.django import DjangoIntegration @@ -130,9 +144,13 @@ def sentry_get_item(self, alias): # type: (CacheHandler, str) -> Any cache = original_get_item(self, alias) + from django.conf import settings + cache_settings = settings.CACHES[alias] + integration = sentry_sdk.get_client().get_integration(DjangoIntegration) if integration is not None and integration.cache_spans: - _patch_cache(cache) + address, port = _get_address_port(cache_settings) + _patch_cache(cache, address, port) return cache @@ -149,17 +167,7 @@ def sentry_create_connection(self, alias): integration = sentry_sdk.get_client().get_integration(DjangoIntegration) if integration is not None and integration.cache_spans: - settings = self.settings[alias or "default"] - address, port = None, None - location = settings.get("LOCATION") - # TODO: location can also be an array of locations - # see: https://docs.djangoproject.com/en/5.0/topics/cache/#redis - if isinstance(location, str): - if ":" in location: - address, port = location.rsplit(":", 1) - else: - address, port = location, None - + address, port = _get_address_port(self.settings) _patch_cache(cache, address, port) return cache From 5a03f1255f6f664b227d77428d5c7453b9f06305 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 7 May 2024 09:31:47 +0200 Subject: [PATCH 19/54] make mypy happy --- sentry_sdk/integrations/django/caching.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index f17a4919a9..428a0da813 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -13,6 +13,7 @@ if TYPE_CHECKING: from typing import Any from typing import Callable + from typing import Tuple from typing import Optional @@ -118,6 +119,7 @@ def _patch_cache(cache, address=None, port=None): def _get_address_port(settings): + # type: (dict[str, Any]) -> Tuple[Optional[str], Optional[str]] address, port = None, None location = settings.get("LOCATION") # TODO: location can also be an array of locations @@ -145,6 +147,7 @@ def sentry_get_item(self, alias): cache = original_get_item(self, alias) from django.conf import settings + cache_settings = settings.CACHES[alias] integration = sentry_sdk.get_client().get_integration(DjangoIntegration) From 2e41ae92ff18878a10017804b570aaa6a840901a Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 7 May 2024 09:34:40 +0200 Subject: [PATCH 20/54] Fixed accessor to settings --- sentry_sdk/integrations/django/caching.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index 428a0da813..c472400f76 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -148,7 +148,7 @@ def sentry_get_item(self, alias): from django.conf import settings - cache_settings = settings.CACHES[alias] + cache_settings = settings.CACHES[alias or "default"] integration = sentry_sdk.get_client().get_integration(DjangoIntegration) if integration is not None and integration.cache_spans: @@ -170,7 +170,7 @@ def sentry_create_connection(self, alias): integration = sentry_sdk.get_client().get_integration(DjangoIntegration) if integration is not None and integration.cache_spans: - address, port = _get_address_port(self.settings) + address, port = _get_address_port(self.settings[alias or "default"]) _patch_cache(cache, address, port) return cache From 11a662ed5fea6b0e6e6a210798d2da26823e358e Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 7 May 2024 09:35:49 +0200 Subject: [PATCH 21/54] Cleanup --- sentry_sdk/integrations/django/caching.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index c472400f76..7084ad24f5 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -146,13 +146,14 @@ def sentry_get_item(self, alias): # type: (CacheHandler, str) -> Any cache = original_get_item(self, alias) - from django.conf import settings - - cache_settings = settings.CACHES[alias or "default"] - integration = sentry_sdk.get_client().get_integration(DjangoIntegration) if integration is not None and integration.cache_spans: - address, port = _get_address_port(cache_settings) + from django.conf import settings + + address, port = _get_address_port( + settings.CACHES[alias or "default"] + ) + _patch_cache(cache, address, port) return cache @@ -171,6 +172,7 @@ def sentry_create_connection(self, alias): integration = sentry_sdk.get_client().get_integration(DjangoIntegration) if integration is not None and integration.cache_spans: address, port = _get_address_port(self.settings[alias or "default"]) + _patch_cache(cache, address, port) return cache From 0fad52b924494bc82d1160db8b1ffd7eaadfef8e Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 8 May 2024 07:57:09 +0200 Subject: [PATCH 22/54] Apply suggestions from code review Co-authored-by: Daniel Szoke <7881302+szokeasaurusrex@users.noreply.github.com> --- sentry_sdk/integrations/django/__init__.py | 2 +- sentry_sdk/integrations/django/caching.py | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index e83b93fefc..4a3376a867 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -105,7 +105,7 @@ def is_authenticated(request_user): class DjangoIntegration(Integration): """ - Auto instrument a Django applications. + Auto instrument a Django application. :param transaction_style: How to derive transaction names. Either `"function_name"` or `"url"`. Defaults to `"url"`. :param middleware_spans: Whether to create spans for middleware. Defaults to `True`. diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index 7084ad24f5..c9278aab8a 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -24,7 +24,7 @@ def _get_key(args, kwargs): - # type: (Any, Any) -> str + # type: (list[Any], dict[str, Any]) -> str if args is not None and len(args) >= 1: return str(args[0]) elif kwargs is not None and "key" in kwargs: @@ -91,9 +91,8 @@ def _instrument_call( span.set_data(SPANDATA.CACHE_HIT, True) else: span.set_data(SPANDATA.CACHE_HIT, False) - else: - if integration.cache_spans_add_item_size: - item_size = len(str(args[1])) + elif integration.cache_spans_add_item_size: + item_size = len(str(args[1])) if item_size is not None: span.set_data(SPANDATA.CACHE_ITEM_SIZE, item_size) From 2dcdeb97699caefbc57afe45e93cd98c571957be Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 8 May 2024 08:16:44 +0200 Subject: [PATCH 23/54] Use tuple --- sentry_sdk/integrations/django/caching.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index c9278aab8a..5d60a104a6 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -13,7 +13,7 @@ if TYPE_CHECKING: from typing import Any from typing import Callable - from typing import Tuple + from typing import tuple from typing import Optional @@ -118,7 +118,7 @@ def _patch_cache(cache, address=None, port=None): def _get_address_port(settings): - # type: (dict[str, Any]) -> Tuple[Optional[str], Optional[str]] + # type: (dict[str, Any]) -> tuple[Optional[str], Optional[str]] address, port = None, None location = settings.get("LOCATION") # TODO: location can also be an array of locations From b398172cf6a8aeda9a8f29e8ac3a9568a03fcb78 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 8 May 2024 08:19:18 +0200 Subject: [PATCH 24/54] Updated comment --- sentry_sdk/integrations/django/caching.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index 5d60a104a6..a77e6d822a 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -65,7 +65,8 @@ def _instrument_call( with capture_internal_exceptions(): if address is not None: - # Strip eventually contained username and password in url style location + # If address is a URL (could also be a string identifier or path to a Unix socket) + # remove the username and password from URL to not leak sensitive data. if "://" in address: parsed_url = urlparse(address) address = "{}://{}{}".format( From bcf702c1189c685d6f8733920a84459cfd2720d3 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 8 May 2024 09:27:16 +0200 Subject: [PATCH 25/54] Instrument get_many and set_many --- sentry_sdk/integrations/django/caching.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index a77e6d822a..ffd7428fca 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -7,7 +7,7 @@ import sentry_sdk from sentry_sdk.consts import OP, SPANDATA -from sentry_sdk.utils import capture_internal_exceptions, ensure_integration_enabled +from sentry_sdk.utils import SENSITIVE_DATA_SUBSTITUTE, capture_internal_exceptions, ensure_integration_enabled if TYPE_CHECKING: @@ -19,14 +19,19 @@ METHODS_TO_INSTRUMENT = [ "set", + "set_many", "get", + "get_many", ] def _get_key(args, kwargs): # type: (list[Any], dict[str, Any]) -> str if args is not None and len(args) >= 1: - return str(args[0]) + key = args[0] + if isinstance(key, dict): + key = {x: SENSITIVE_DATA_SUBSTITUTE for x in key} + return str(key) elif kwargs is not None and "key" in kwargs: return str(kwargs["key"]) @@ -92,8 +97,14 @@ def _instrument_call( span.set_data(SPANDATA.CACHE_HIT, True) else: span.set_data(SPANDATA.CACHE_HIT, False) - elif integration.cache_spans_add_item_size: - item_size = len(str(args[1])) + else: + if integration.cache_spans_add_item_size: + try: + # 'set' command + item_size = len(str(args[1])) + except IndexError: + # 'set_many' command + item_size = len(str(args[0])) if item_size is not None: span.set_data(SPANDATA.CACHE_ITEM_SIZE, item_size) From cefe47fa2786a400d51619610114a491f26621a4 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 8 May 2024 09:28:36 +0200 Subject: [PATCH 26/54] Moved parsing url outside to not call it that often --- sentry_sdk/integrations/django/caching.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index ffd7428fca..4482810edf 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -70,16 +70,6 @@ def _instrument_call( with capture_internal_exceptions(): if address is not None: - # If address is a URL (could also be a string identifier or path to a Unix socket) - # remove the username and password from URL to not leak sensitive data. - if "://" in address: - parsed_url = urlparse(address) - address = "{}://{}{}".format( - parsed_url.scheme or "", - parsed_url.netloc or "", - parsed_url.path or "", - ) - span.set_data(SPANDATA.NETWORK_PEER_ADDRESS, address) if port is not None: @@ -141,6 +131,16 @@ def _get_address_port(settings): else: address, port = location, None + # If address is a URL (could also be a string identifier or path to a Unix socket) + # remove the username and password from URL to not leak sensitive data. + if "://" in address: + parsed_url = urlparse(address) + address = "{}://{}{}".format( + parsed_url.scheme or "", + parsed_url.netloc or "", + parsed_url.path or "", + ) + return address, port From c051197187cc1886c4e86c2e46969386b8d04816 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 8 May 2024 09:32:45 +0200 Subject: [PATCH 27/54] Comment --- sentry_sdk/integrations/django/caching.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index 4482810edf..d70c0c3dd4 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -30,6 +30,9 @@ def _get_key(args, kwargs): if args is not None and len(args) >= 1: key = args[0] if isinstance(key, dict): + # Do not leak sensitive data + # `set_many()` has a dict {"key1": "value1", "key2": "value2"} as first argument. + # Those values could include sensitive data so we replace them with a placeholder key = {x: SENSITIVE_DATA_SUBSTITUTE for x in key} return str(key) elif kwargs is not None and "key" in kwargs: From 11be8fa1dbf1023ae125c053c231d006b9b1f9a1 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 8 May 2024 09:38:39 +0200 Subject: [PATCH 28/54] Make port an int early on --- sentry_sdk/integrations/django/caching.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index d70c0c3dd4..9a2c5a94de 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -51,7 +51,7 @@ def _get_span_description(method_name, args, kwargs): def _patch_cache_method(cache, method_name, address, port): - # type: (CacheHandler, str, Optional[str], Optional[str]) -> None + # type: (CacheHandler, str, Optional[str], Optional[int]) -> None from sentry_sdk.integrations.django import DjangoIntegration original_method = getattr(cache, method_name) @@ -60,7 +60,7 @@ def _patch_cache_method(cache, method_name, address, port): def _instrument_call( cache, method_name, original_method, args, kwargs, address, port ): - # type: (CacheHandler, str, Callable[..., Any], Any, Any, Optional[str], Optional[str]) -> Any + # type: (CacheHandler, str, Callable[..., Any], Any, Any, Optional[str], Optional[int]) -> Any integration = sentry_sdk.get_client().get_integration(DjangoIntegration) is_set_operation = method_name.startswith("set") is_get_operation = not is_set_operation @@ -76,7 +76,7 @@ def _instrument_call( span.set_data(SPANDATA.NETWORK_PEER_ADDRESS, address) if port is not None: - span.set_data(SPANDATA.NETWORK_PEER_PORT, int(port)) + span.set_data(SPANDATA.NETWORK_PEER_PORT, port) key = _get_key(args, kwargs) if key != "": @@ -115,7 +115,7 @@ def sentry_method(*args, **kwargs): def _patch_cache(cache, address=None, port=None): - # type: (CacheHandler, Optional[str], Optional[str]) -> None + # type: (CacheHandler, Optional[str], Optional[int]) -> None if not hasattr(cache, "_sentry_patched"): for method_name in METHODS_TO_INSTRUMENT: _patch_cache_method(cache, method_name, address, port) @@ -123,7 +123,7 @@ def _patch_cache(cache, address=None, port=None): def _get_address_port(settings): - # type: (dict[str, Any]) -> tuple[Optional[str], Optional[str]] + # type: (dict[str, Any]) -> tuple[Optional[str], Optional[int]] address, port = None, None location = settings.get("LOCATION") # TODO: location can also be an array of locations @@ -144,7 +144,7 @@ def _get_address_port(settings): parsed_url.path or "", ) - return address, port + return address, int(port) if port is not None else None def patch_caching(): From 3718f41eaeb9b9aba9f8c208530737cf12fb1b39 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 8 May 2024 09:40:52 +0200 Subject: [PATCH 29/54] Linting --- sentry_sdk/integrations/django/caching.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index 9a2c5a94de..cfe0025aba 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -7,13 +7,16 @@ import sentry_sdk from sentry_sdk.consts import OP, SPANDATA -from sentry_sdk.utils import SENSITIVE_DATA_SUBSTITUTE, capture_internal_exceptions, ensure_integration_enabled +from sentry_sdk.utils import ( + SENSITIVE_DATA_SUBSTITUTE, + capture_internal_exceptions, + ensure_integration_enabled, +) if TYPE_CHECKING: from typing import Any from typing import Callable - from typing import tuple from typing import Optional @@ -136,7 +139,7 @@ def _get_address_port(settings): # If address is a URL (could also be a string identifier or path to a Unix socket) # remove the username and password from URL to not leak sensitive data. - if "://" in address: + if address is not None and "://" in address: parsed_url = urlparse(address) address = "{}://{}{}".format( parsed_url.scheme or "", From 16a9edf792a0f4afc84b1afdfbadfbd7bae841f7 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 8 May 2024 11:11:14 +0200 Subject: [PATCH 30/54] Added test --- .../integrations/django/test_cache_module.py | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index fbdcdf53ec..f67c9c5d2d 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -1,4 +1,5 @@ import pytest +import os import random from django import VERSION as DJANGO_VERSION @@ -10,6 +11,7 @@ except ImportError: from django.core.urlresolvers import reverse +import sentry_sdk from sentry_sdk.integrations.django import DjangoIntegration from sentry_sdk.integrations.django.caching import _get_span_description from tests.integrations.django.myapp.wsgi import application @@ -468,3 +470,68 @@ def test_cache_spans_item_size(sentry_init, client, capture_events, use_django_c assert second_event["spans"][1]["op"] == "cache.get_item" assert second_event["spans"][1]["data"]["cache.hit"] assert second_event["spans"][1]["data"]["cache.item_size"] == 58 + + +@pytest.mark.forked +@pytest_mark_django_db_decorator() +def test_cache_spans_get_many(sentry_init, capture_events, use_django_caching): + sentry_init( + integrations=[ + DjangoIntegration( + cache_spans=True, + middleware_spans=False, + signals_spans=False, + cache_spans_add_item_size=True, + ) + ], + traces_sample_rate=1.0, + ) + events = capture_events() + + id = os.getpid() + + from django.core.cache import cache + + with sentry_sdk.start_transaction(): + cache.get_many([f"S{id}", f"S{id+1}"]) + cache.set(f"S{id}", "Sensitive1") + cache.get_many([f"S{id}", f"S{id+1}"]) + + (transaction,) = events + assert len(transaction["spans"]) == 7 + + assert transaction["spans"][0]["op"] == "cache.get_item" + assert transaction["spans"][0]["description"] == f"get_many ['S{id}', 'S{id+1}']" + assert not transaction["spans"][0]["data"]["cache.hit"] + + assert transaction["spans"][1]["op"] == "cache.get_item" + assert transaction["spans"][1]["description"] == f"get S{id}" + assert transaction["spans"][1]["data"][ + "cache.hit" + ] # LocMemCache always returns True (but that is ok, because it is not production) + + assert transaction["spans"][2]["op"] == "cache.get_item" + assert transaction["spans"][2]["description"] == f"get S{id+1}" + assert transaction["spans"][2]["data"][ + "cache.hit" + ] # LocMemCache always returns True (but that is ok, because it is not production) + + assert transaction["spans"][3]["op"] == "cache.set_item" + assert transaction["spans"][3]["description"] == f"set S{id}" + assert "cache.hit" not in transaction["spans"][3]["data"] + + assert transaction["spans"][4]["op"] == "cache.get_item" + assert transaction["spans"][4]["description"] == f"get_many ['S{id}', 'S{id+1}']" + assert transaction["spans"][4]["data"]["cache.hit"] + + assert transaction["spans"][5]["op"] == "cache.get_item" + assert transaction["spans"][5]["description"] == f"get S{id}" + assert transaction["spans"][5]["data"][ + "cache.hit" + ] # LocMemCache always returns True (but that is ok, because it is not production) + + assert transaction["spans"][6]["op"] == "cache.get_item" + assert transaction["spans"][6]["description"] == f"get S{id+1}" + assert transaction["spans"][6]["data"][ + "cache.hit" + ] # LocMemCache always returns True (but that is ok, because it is not production) From 1ad1cb887898fcd483117c4fef5586bb8a47d737 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 8 May 2024 11:20:27 +0200 Subject: [PATCH 31/54] Another test --- .../integrations/django/test_cache_module.py | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index f67c9c5d2d..c13f348a10 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -535,3 +535,48 @@ def test_cache_spans_get_many(sentry_init, capture_events, use_django_caching): assert transaction["spans"][6]["data"][ "cache.hit" ] # LocMemCache always returns True (but that is ok, because it is not production) + + +@pytest.mark.forked +@pytest_mark_django_db_decorator() +def test_cache_spans_set_many(sentry_init, capture_events, use_django_caching): + sentry_init( + integrations=[ + DjangoIntegration( + cache_spans=True, + middleware_spans=False, + signals_spans=False, + cache_spans_add_item_size=True, + ) + ], + traces_sample_rate=1.0, + ) + events = capture_events() + + id = os.getpid() + + from django.core.cache import cache + + with sentry_sdk.start_transaction(): + cache.set_many({f"S{id}": "Sensitive1", f"S{id+1}": "Sensitive2"}) + cache.get(f"S{id}") + + import ipdb; ipdb.set_trace() + (transaction,) = events + assert len(transaction["spans"]) == 4 + + assert transaction["spans"][0]["op"] == "cache.set_item" + assert transaction["spans"][0]["description"] == f"set_many {{'S{id}': '[Filtered]', 'S{id+1}': '[Filtered]'}}" + assert "cache.hit" not in transaction["spans"][0]["data"] + + assert transaction["spans"][1]["op"] == "cache.set_item" + assert transaction["spans"][1]["description"] == f"set S{id}" + assert "cache.hit" not in transaction["spans"][1]["data"] + + assert transaction["spans"][2]["op"] == "cache.set_item" + assert transaction["spans"][2]["description"] == f"set S{id+1}" + assert "cache.hit" not in transaction["spans"][2]["data"] + + assert transaction["spans"][3]["op"] == "cache.get_item" + assert transaction["spans"][3]["description"] == f"get S{id}" + assert transaction["spans"][3]["data"]["cache.hit"] From 3b893944492042bc9ce241c0eb56aad276207428 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 8 May 2024 11:22:27 +0200 Subject: [PATCH 32/54] linting --- tests/integrations/django/test_cache_module.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index c13f348a10..ab4c76ec9a 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -560,13 +560,18 @@ def test_cache_spans_set_many(sentry_init, capture_events, use_django_caching): with sentry_sdk.start_transaction(): cache.set_many({f"S{id}": "Sensitive1", f"S{id+1}": "Sensitive2"}) cache.get(f"S{id}") - - import ipdb; ipdb.set_trace() + + import ipdb + + ipdb.set_trace() (transaction,) = events assert len(transaction["spans"]) == 4 assert transaction["spans"][0]["op"] == "cache.set_item" - assert transaction["spans"][0]["description"] == f"set_many {{'S{id}': '[Filtered]', 'S{id+1}': '[Filtered]'}}" + assert ( + transaction["spans"][0]["description"] + == f"set_many {{'S{id}': '[Filtered]', 'S{id+1}': '[Filtered]'}}" + ) assert "cache.hit" not in transaction["spans"][0]["data"] assert transaction["spans"][1]["op"] == "cache.set_item" From 01158430dd19ac22d9e05a4d085a6f10186b4c54 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 8 May 2024 11:25:50 +0200 Subject: [PATCH 33/54] oh, man... --- tests/integrations/django/test_cache_module.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index ab4c76ec9a..6f8a25ee46 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -561,9 +561,6 @@ def test_cache_spans_set_many(sentry_init, capture_events, use_django_caching): cache.set_many({f"S{id}": "Sensitive1", f"S{id+1}": "Sensitive2"}) cache.get(f"S{id}") - import ipdb - - ipdb.set_trace() (transaction,) = events assert len(transaction["spans"]) == 4 From 827858e5f10a1f74313289840e92361063b34c0b Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 8 May 2024 11:30:17 +0200 Subject: [PATCH 34/54] tests in CI actually do what is expected (compared to locally) --- tests/integrations/django/test_cache_module.py | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index 6f8a25ee46..fe960b825b 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -506,15 +506,11 @@ def test_cache_spans_get_many(sentry_init, capture_events, use_django_caching): assert transaction["spans"][1]["op"] == "cache.get_item" assert transaction["spans"][1]["description"] == f"get S{id}" - assert transaction["spans"][1]["data"][ - "cache.hit" - ] # LocMemCache always returns True (but that is ok, because it is not production) + assert not transaction["spans"][1]["data"]["cache.hit"] assert transaction["spans"][2]["op"] == "cache.get_item" assert transaction["spans"][2]["description"] == f"get S{id+1}" - assert transaction["spans"][2]["data"][ - "cache.hit" - ] # LocMemCache always returns True (but that is ok, because it is not production) + assert not transaction["spans"][2]["data"]["cache.hit"] assert transaction["spans"][3]["op"] == "cache.set_item" assert transaction["spans"][3]["description"] == f"set S{id}" @@ -526,15 +522,11 @@ def test_cache_spans_get_many(sentry_init, capture_events, use_django_caching): assert transaction["spans"][5]["op"] == "cache.get_item" assert transaction["spans"][5]["description"] == f"get S{id}" - assert transaction["spans"][5]["data"][ - "cache.hit" - ] # LocMemCache always returns True (but that is ok, because it is not production) + assert not transaction["spans"][5]["data"]["cache.hit"] assert transaction["spans"][6]["op"] == "cache.get_item" assert transaction["spans"][6]["description"] == f"get S{id+1}" - assert transaction["spans"][6]["data"][ - "cache.hit" - ] # LocMemCache always returns True (but that is ok, because it is not production) + assert not transaction["spans"][6]["data"]["cache.hit"] @pytest.mark.forked From cc0f1af0040da34cdd9c4a957923aef0195fdf50 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 8 May 2024 11:36:13 +0200 Subject: [PATCH 35/54] Removed cache.hit assert because it is unrealiable because of LocMemCache --- tests/integrations/django/test_cache_module.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index fe960b825b..320fa42660 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -502,31 +502,24 @@ def test_cache_spans_get_many(sentry_init, capture_events, use_django_caching): assert transaction["spans"][0]["op"] == "cache.get_item" assert transaction["spans"][0]["description"] == f"get_many ['S{id}', 'S{id+1}']" - assert not transaction["spans"][0]["data"]["cache.hit"] assert transaction["spans"][1]["op"] == "cache.get_item" assert transaction["spans"][1]["description"] == f"get S{id}" - assert not transaction["spans"][1]["data"]["cache.hit"] assert transaction["spans"][2]["op"] == "cache.get_item" assert transaction["spans"][2]["description"] == f"get S{id+1}" - assert not transaction["spans"][2]["data"]["cache.hit"] assert transaction["spans"][3]["op"] == "cache.set_item" assert transaction["spans"][3]["description"] == f"set S{id}" - assert "cache.hit" not in transaction["spans"][3]["data"] assert transaction["spans"][4]["op"] == "cache.get_item" assert transaction["spans"][4]["description"] == f"get_many ['S{id}', 'S{id+1}']" - assert transaction["spans"][4]["data"]["cache.hit"] assert transaction["spans"][5]["op"] == "cache.get_item" assert transaction["spans"][5]["description"] == f"get S{id}" - assert not transaction["spans"][5]["data"]["cache.hit"] assert transaction["spans"][6]["op"] == "cache.get_item" assert transaction["spans"][6]["description"] == f"get S{id+1}" - assert not transaction["spans"][6]["data"]["cache.hit"] @pytest.mark.forked @@ -561,16 +554,12 @@ def test_cache_spans_set_many(sentry_init, capture_events, use_django_caching): transaction["spans"][0]["description"] == f"set_many {{'S{id}': '[Filtered]', 'S{id+1}': '[Filtered]'}}" ) - assert "cache.hit" not in transaction["spans"][0]["data"] assert transaction["spans"][1]["op"] == "cache.set_item" assert transaction["spans"][1]["description"] == f"set S{id}" - assert "cache.hit" not in transaction["spans"][1]["data"] assert transaction["spans"][2]["op"] == "cache.set_item" assert transaction["spans"][2]["description"] == f"set S{id+1}" - assert "cache.hit" not in transaction["spans"][2]["data"] assert transaction["spans"][3]["op"] == "cache.get_item" assert transaction["spans"][3]["description"] == f"get S{id}" - assert transaction["spans"][3]["data"]["cache.hit"] From 2de1d03ad54b55baab612b93cec2dc1a7ca94b9d Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 10 May 2024 09:32:53 +0200 Subject: [PATCH 36/54] Always add item_size because load testing revealed no performance impact --- sentry_sdk/integrations/django/__init__.py | 3 --- sentry_sdk/integrations/django/caching.py | 16 +++++++--------- tests/integrations/django/test_cache_module.py | 3 --- 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index 4a3376a867..430feff1d4 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -112,7 +112,6 @@ class DjangoIntegration(Integration): :param signals_spans: Whether to create spans for signals. Defaults to `True`. :param signals_denylist: A list of signals to ignore when creating spans. :param cache_spans: Whether to create spans for cache operations. Defaults to `False`. - :param cache_spans_add_item_size: Whether to add the size of the cache item to the span data. Only applicable when `cache_spans` is set to `True`. Defaults to `False`. """ identifier = "django" @@ -130,7 +129,6 @@ def __init__( signals_spans=True, cache_spans=False, signals_denylist=None, - cache_spans_add_item_size=False, ): # type: (str, bool, bool, bool, Optional[list[signals.Signal]], bool) -> None if transaction_style not in TRANSACTION_STYLE_VALUES: @@ -145,7 +143,6 @@ def __init__( self.signals_denylist = signals_denylist or [] self.cache_spans = cache_spans - self.cache_spans_add_item_size = cache_spans_add_item_size @staticmethod def setup_once(): diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index cfe0025aba..066c874e20 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -88,19 +88,17 @@ def _instrument_call( item_size = None if is_get_operation: if value: - if integration.cache_spans_add_item_size: - item_size = len(str(value)) + item_size = len(str(value)) span.set_data(SPANDATA.CACHE_HIT, True) else: span.set_data(SPANDATA.CACHE_HIT, False) else: - if integration.cache_spans_add_item_size: - try: - # 'set' command - item_size = len(str(args[1])) - except IndexError: - # 'set_many' command - item_size = len(str(args[0])) + try: + # 'set' command + item_size = len(str(args[1])) + except IndexError: + # 'set_many' command + item_size = len(str(args[0])) if item_size is not None: span.set_data(SPANDATA.CACHE_ITEM_SIZE, item_size) diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index 320fa42660..546691ab46 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -438,7 +438,6 @@ def test_cache_spans_item_size(sentry_init, client, capture_events, use_django_c cache_spans=True, middleware_spans=False, signals_spans=False, - cache_spans_add_item_size=True, ) ], traces_sample_rate=1.0, @@ -481,7 +480,6 @@ def test_cache_spans_get_many(sentry_init, capture_events, use_django_caching): cache_spans=True, middleware_spans=False, signals_spans=False, - cache_spans_add_item_size=True, ) ], traces_sample_rate=1.0, @@ -531,7 +529,6 @@ def test_cache_spans_set_many(sentry_init, capture_events, use_django_caching): cache_spans=True, middleware_spans=False, signals_spans=False, - cache_spans_add_item_size=True, ) ], traces_sample_rate=1.0, From f2d8c77618c587610806845995c5f7cb1c4d6b6a Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 10 May 2024 10:23:44 +0200 Subject: [PATCH 37/54] Updated tests --- tests/integrations/django/test_cache_module.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index 546691ab46..dec2584608 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -203,7 +203,7 @@ def test_cache_spans_middleware( "views.decorators.cache.cache_header." ) assert "cache.hit" not in first_event["spans"][1]["data"] - assert "cache.item_size" not in first_event["spans"][1]["data"] + assert first_event["spans"][1]["data"]["cache.item_size"] == 2 # second_event - cache.get assert second_event["spans"][0]["op"] == "cache.get_item" assert second_event["spans"][0]["description"].startswith( @@ -225,7 +225,7 @@ def test_cache_spans_middleware( "views.decorators.cache.cache_page." ) assert second_event["spans"][1]["data"]["cache.hit"] - assert "cache.item_size" not in second_event["spans"][1]["data"] + assert second_event["spans"][1]["data"]["cache.item_size"] == 58 @pytest.mark.forked @@ -269,7 +269,7 @@ def test_cache_spans_decorator(sentry_init, client, capture_events, use_django_c "views.decorators.cache.cache_header." ) assert "cache.hit" not in first_event["spans"][1]["data"] - assert "cache.item_size" not in first_event["spans"][1]["data"] + assert first_event["spans"][1]["data"]["cache.item_size"] == 2 # second_event - cache.get assert second_event["spans"][1]["op"] == "cache.get_item" assert second_event["spans"][1]["description"].startswith( @@ -280,7 +280,7 @@ def test_cache_spans_decorator(sentry_init, client, capture_events, use_django_c "views.decorators.cache.cache_page." ) assert second_event["spans"][1]["data"]["cache.hit"] - assert "cache.item_size" not in second_event["spans"][0]["data"] + assert second_event["spans"][1]["data"]["cache.item_size"] == 58 @pytest.mark.forked @@ -327,7 +327,7 @@ def test_cache_spans_templatetag( "template.cache.some_identifier." ) assert "cache.hit" not in first_event["spans"][1]["data"] - assert "cache.item_size" not in first_event["spans"][1]["data"] + assert first_event["spans"][1]["data"]["cache.item_size"] == 51 # second_event - cache.get assert second_event["spans"][0]["op"] == "cache.get_item" assert second_event["spans"][0]["description"].startswith( @@ -338,7 +338,7 @@ def test_cache_spans_templatetag( "template.cache.some_identifier." ) assert second_event["spans"][0]["data"]["cache.hit"] - assert "cache.item_size" not in second_event["spans"][0]["data"] + assert second_event["spans"][0]["data"]["cache.item_size"] == 51 @pytest.mark.parametrize( From ca65d1750fcdfef5d7b239b4220be7481efe9595 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 10 May 2024 10:26:42 +0200 Subject: [PATCH 38/54] Removed unused var --- sentry_sdk/integrations/django/caching.py | 1 - tests/integrations/django/test_cache_module.py | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index 066c874e20..a2c89bb4d9 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -64,7 +64,6 @@ def _instrument_call( cache, method_name, original_method, args, kwargs, address, port ): # type: (CacheHandler, str, Callable[..., Any], Any, Any, Optional[str], Optional[int]) -> Any - integration = sentry_sdk.get_client().get_integration(DjangoIntegration) is_set_operation = method_name.startswith("set") is_get_operation = not is_set_operation diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index dec2584608..d897aca77f 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -203,7 +203,7 @@ def test_cache_spans_middleware( "views.decorators.cache.cache_header." ) assert "cache.hit" not in first_event["spans"][1]["data"] - assert first_event["spans"][1]["data"]["cache.item_size"] == 2 + assert first_event["spans"][1]["data"]["cache.item_size"] == 2 # second_event - cache.get assert second_event["spans"][0]["op"] == "cache.get_item" assert second_event["spans"][0]["description"].startswith( @@ -269,7 +269,7 @@ def test_cache_spans_decorator(sentry_init, client, capture_events, use_django_c "views.decorators.cache.cache_header." ) assert "cache.hit" not in first_event["spans"][1]["data"] - assert first_event["spans"][1]["data"]["cache.item_size"] == 2 + assert first_event["spans"][1]["data"]["cache.item_size"] == 2 # second_event - cache.get assert second_event["spans"][1]["op"] == "cache.get_item" assert second_event["spans"][1]["description"].startswith( From 7adb5a1652ee39c7fad60b763bea8db2eaacb539 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 10 May 2024 10:39:39 +0200 Subject: [PATCH 39/54] Make mypy happy --- sentry_sdk/integrations/django/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/django/__init__.py b/sentry_sdk/integrations/django/__init__.py index 430feff1d4..3a6a075c70 100644 --- a/sentry_sdk/integrations/django/__init__.py +++ b/sentry_sdk/integrations/django/__init__.py @@ -130,7 +130,7 @@ def __init__( cache_spans=False, signals_denylist=None, ): - # type: (str, bool, bool, bool, Optional[list[signals.Signal]], bool) -> None + # type: (str, bool, bool, bool, Optional[list[signals.Signal]]) -> None if transaction_style not in TRANSACTION_STYLE_VALUES: raise ValueError( "Invalid value for transaction_style: %s (must be in %s)" From 7232c444a92a3a34c192f763b9c8a949eff16556 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 10 May 2024 13:31:23 +0200 Subject: [PATCH 40/54] Linked GH issue in comment --- sentry_sdk/integrations/redis/__init__.py | 1 + sentry_sdk/integrations/redis/asyncio.py | 1 + 2 files changed, 2 insertions(+) diff --git a/sentry_sdk/integrations/redis/__init__.py b/sentry_sdk/integrations/redis/__init__.py index 3a25e73881..93c7e51d44 100644 --- a/sentry_sdk/integrations/redis/__init__.py +++ b/sentry_sdk/integrations/redis/__init__.py @@ -223,6 +223,7 @@ def sentry_patched_execute_command(self, name, *args, **kwargs): # Questions: # -) We should probablby have the OP.DB_REDIS span and a separate OP.CACHE_GET_ITEM (or set_item) span, right? # -) We probably need to research what redis commands are used by caching libs. + # GitHub issue: https://github.com/getsentry/sentry-python/issues/2965 with sentry_sdk.start_span(op=OP.DB_REDIS, description=description) as span: set_db_data_fn(span, self) _set_client_data(span, is_cluster, name, *args) diff --git a/sentry_sdk/integrations/redis/asyncio.py b/sentry_sdk/integrations/redis/asyncio.py index 93cfd63a65..5a4943c484 100644 --- a/sentry_sdk/integrations/redis/asyncio.py +++ b/sentry_sdk/integrations/redis/asyncio.py @@ -63,6 +63,7 @@ async def _sentry_execute_command(self, name, *args, **kwargs): # Questions: # -) We should probablby have the OP.DB_REDIS span and a separate OP.CACHE_GET_ITEM (or set_item) span, right? # -) We probably need to research what redis commands are used by caching libs. + # GitHub issue: https://github.com/getsentry/sentry-python/issues/2965 with sentry_sdk.start_span(op=OP.DB_REDIS, description=description) as span: set_db_data_fn(span, self) _set_client_data(span, is_cluster, name, *args) From a319a5123fb5ce0fe8c6e1048dd816326d50db6a Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 10 May 2024 13:35:02 +0200 Subject: [PATCH 41/54] Linked GH issue in comment --- sentry_sdk/integrations/redis/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sentry_sdk/integrations/redis/__init__.py b/sentry_sdk/integrations/redis/__init__.py index 93c7e51d44..eff27f8df1 100644 --- a/sentry_sdk/integrations/redis/__init__.py +++ b/sentry_sdk/integrations/redis/__init__.py @@ -365,6 +365,7 @@ def __init__(self, max_data_size=_DEFAULT_MAX_DATA_SIZE): # type: (int) -> None self.max_data_size = max_data_size # TODO: add some prefix that users can set to specify a cache key + # GitHub issue: https://github.com/getsentry/sentry-python/issues/2965 @staticmethod def setup_once(): From a691861eec018ba73127d4765cab84fe05e175dd Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 10 May 2024 13:35:49 +0200 Subject: [PATCH 42/54] Apply suggestions from code review Co-authored-by: Daniel Szoke <7881302+szokeasaurusrex@users.noreply.github.com> --- sentry_sdk/integrations/django/caching.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index a2c89bb4d9..9434a0705e 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -45,7 +45,7 @@ def _get_key(args, kwargs): def _get_span_description(method_name, args, kwargs): - # type: (str, Any, Any) -> str + # type: (str, list[Any], dict[str, Any]) -> str description = "{} {}".format( method_name, _get_key(args, kwargs), @@ -63,7 +63,7 @@ def _patch_cache_method(cache, method_name, address, port): def _instrument_call( cache, method_name, original_method, args, kwargs, address, port ): - # type: (CacheHandler, str, Callable[..., Any], Any, Any, Optional[str], Optional[int]) -> Any + # type: (CacheHandler, str, Callable[..., Any], list[Any], dict[str, Any], Optional[str], Optional[int]) -> Any is_set_operation = method_name.startswith("set") is_get_operation = not is_set_operation From 8b710ab2875eff324292472848d637baceed5cab Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 10 May 2024 13:41:13 +0200 Subject: [PATCH 43/54] Linked GH issue in comment --- sentry_sdk/integrations/django/caching.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index 9434a0705e..74be70cd09 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -128,6 +128,7 @@ def _get_address_port(settings): location = settings.get("LOCATION") # TODO: location can also be an array of locations # see: https://docs.djangoproject.com/en/5.0/topics/cache/#redis + # GitHub issue: https://github.com/getsentry/sentry-python/issues/3062 if isinstance(location, str): if ":" in location: address, port = location.rsplit(":", 1) From fe52b1454901f117d58a44c2663e5c3f5da0481a Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 10 May 2024 13:48:11 +0200 Subject: [PATCH 44/54] Improved cache location parsing --- sentry_sdk/integrations/django/caching.py | 28 +++++++++++------------ 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index 74be70cd09..58978cf9dd 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -127,23 +127,21 @@ def _get_address_port(settings): address, port = None, None location = settings.get("LOCATION") # TODO: location can also be an array of locations - # see: https://docs.djangoproject.com/en/5.0/topics/cache/#redis - # GitHub issue: https://github.com/getsentry/sentry-python/issues/3062 + # see: https://docs.djangoproject.com/en/5.0/topics/cache/#redis + # GitHub issue: https://github.com/getsentry/sentry-python/issues/3062 if isinstance(location, str): - if ":" in location: - address, port = location.rsplit(":", 1) + if "://" in location: + parsed_url = urlparse(address) + # remove the username and password from URL to not leak sensitive data. + address = "{}://{}{}".format( + parsed_url.scheme or "", + parsed_url.netloc or "", + parsed_url.path or "", + ) + port = parsed_url.port else: - address, port = location, None - - # If address is a URL (could also be a string identifier or path to a Unix socket) - # remove the username and password from URL to not leak sensitive data. - if address is not None and "://" in address: - parsed_url = urlparse(address) - address = "{}://{}{}".format( - parsed_url.scheme or "", - parsed_url.netloc or "", - parsed_url.path or "", - ) + address = location + port = None return address, int(port) if port is not None else None From 747e0c8f22fd6af6fd495d4a06dc1f30c75aa7d8 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 10 May 2024 13:52:31 +0200 Subject: [PATCH 45/54] added test and fixed typo --- sentry_sdk/integrations/django/caching.py | 2 +- .../integrations/django/test_cache_module.py | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index 58978cf9dd..d477205268 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -131,7 +131,7 @@ def _get_address_port(settings): # GitHub issue: https://github.com/getsentry/sentry-python/issues/3062 if isinstance(location, str): if "://" in location: - parsed_url = urlparse(address) + parsed_url = urlparse(location) # remove the username and password from URL to not leak sensitive data. address = "{}://{}{}".format( parsed_url.scheme or "", diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index d897aca77f..f2731ee73c 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -66,6 +66,16 @@ def use_django_caching_with_port(settings): } +@pytest.fixture +def use_django_caching_without_port(settings): + settings.CACHES = { + "default": { + "BACKEND": "django.core.cache.backends.dummy.DummyCache", + "LOCATION": "redis://example.com", + } + } + + @pytest.fixture def use_django_caching_with_cluster(settings): settings.CACHES = { @@ -402,6 +412,32 @@ def test_cache_spans_location_with_port( assert span["data"]["network.peer.port"] == 6379 +@pytest.mark.forked +@pytest_mark_django_db_decorator() +def test_cache_spans_location_without_port( + sentry_init, client, capture_events, use_django_caching_without_port +): + sentry_init( + integrations=[ + DjangoIntegration( + cache_spans=True, + middleware_spans=False, + signals_spans=False, + ) + ], + traces_sample_rate=1.0, + ) + events = capture_events() + + client.get(reverse("cached_view")) + client.get(reverse("cached_view")) + + for event in events: + for span in event["spans"]: + assert span["data"]["network.peer.address"] == "redis://example.com" + assert "network.peer.port" not in span["data"] + + @pytest.mark.forked @pytest_mark_django_db_decorator() def test_cache_spans_location_with_cluster( From 4fa92d0c701b42643765e44f0f40ee5057370881 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 10 May 2024 13:56:49 +0200 Subject: [PATCH 46/54] Improved stripping sensitive data from keys --- sentry_sdk/integrations/django/caching.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index d477205268..dd3f80234f 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -30,18 +30,20 @@ def _get_key(args, kwargs): # type: (list[Any], dict[str, Any]) -> str + key = "" + if args is not None and len(args) >= 1: key = args[0] - if isinstance(key, dict): - # Do not leak sensitive data - # `set_many()` has a dict {"key1": "value1", "key2": "value2"} as first argument. - # Those values could include sensitive data so we replace them with a placeholder - key = {x: SENSITIVE_DATA_SUBSTITUTE for x in key} - return str(key) elif kwargs is not None and "key" in kwargs: - return str(kwargs["key"]) - - return "" + key = kwargs["key"] + + if isinstance(key, dict): + # Do not leak sensitive data + # `set_many()` has a dict {"key1": "value1", "key2": "value2"} as first argument. + # Those values could include sensitive data so we replace them with a placeholder + key = {x: SENSITIVE_DATA_SUBSTITUTE for x in key} + + return str(key) def _get_span_description(method_name, args, kwargs): From e7986b8d4b2a1fdb01cca8901e1e5ad545fc5eb4 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 10 May 2024 14:00:33 +0200 Subject: [PATCH 47/54] whitespace --- sentry_sdk/integrations/django/caching.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index dd3f80234f..6560d294dc 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -42,7 +42,7 @@ def _get_key(args, kwargs): # `set_many()` has a dict {"key1": "value1", "key2": "value2"} as first argument. # Those values could include sensitive data so we replace them with a placeholder key = {x: SENSITIVE_DATA_SUBSTITUTE for x in key} - + return str(key) From 66eff133379f094178dbdb55f6961418a07d8275 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 10 May 2024 14:06:12 +0200 Subject: [PATCH 48/54] Improved url parsing --- sentry_sdk/integrations/django/caching.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index 6560d294dc..d5d990e951 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -137,7 +137,7 @@ def _get_address_port(settings): # remove the username and password from URL to not leak sensitive data. address = "{}://{}{}".format( parsed_url.scheme or "", - parsed_url.netloc or "", + parsed_url.hostname or "", parsed_url.path or "", ) port = parsed_url.port From 2648759a398d782258a6b82d385c57f2f5b97d4f Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Fri, 10 May 2024 14:09:31 +0200 Subject: [PATCH 49/54] return early --- sentry_sdk/integrations/django/caching.py | 30 ++++++++++++----------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index d5d990e951..986adb5c9a 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -126,24 +126,26 @@ def _patch_cache(cache, address=None, port=None): def _get_address_port(settings): # type: (dict[str, Any]) -> tuple[Optional[str], Optional[int]] - address, port = None, None location = settings.get("LOCATION") + # TODO: location can also be an array of locations # see: https://docs.djangoproject.com/en/5.0/topics/cache/#redis # GitHub issue: https://github.com/getsentry/sentry-python/issues/3062 - if isinstance(location, str): - if "://" in location: - parsed_url = urlparse(location) - # remove the username and password from URL to not leak sensitive data. - address = "{}://{}{}".format( - parsed_url.scheme or "", - parsed_url.hostname or "", - parsed_url.path or "", - ) - port = parsed_url.port - else: - address = location - port = None + if not isinstance(location, str): + return None, None + + if "://" in location: + parsed_url = urlparse(location) + # remove the username and password from URL to not leak sensitive data. + address = "{}://{}{}".format( + parsed_url.scheme or "", + parsed_url.hostname or "", + parsed_url.path or "", + ) + port = parsed_url.port + else: + address = location + port = None return address, int(port) if port is not None else None From 2b84023c733dc66753f8949289f3eb89e6412505 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 21 May 2024 17:15:56 +0200 Subject: [PATCH 50/54] use new names and only key in cache span description --- sentry_sdk/consts.py | 4 +- sentry_sdk/integrations/django/caching.py | 8 +-- sentry_sdk/integrations/redis/__init__.py | 2 +- sentry_sdk/integrations/redis/asyncio.py | 2 +- .../integrations/django/test_cache_module.py | 52 +++++++++---------- 5 files changed, 32 insertions(+), 36 deletions(-) diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index 9673cb7995..8cdccc8a53 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -367,8 +367,8 @@ class SPANDATA: class OP: ANTHROPIC_MESSAGES_CREATE = "ai.messages.create.anthropic" - CACHE_GET_ITEM = "cache.get_item" - CACHE_SET_ITEM = "cache.set_item" + CACHE_GET = "cache.get" + CACHE_SET = "cache.set" COHERE_CHAT_COMPLETIONS_CREATE = "ai.chat_completions.create.cohere" COHERE_EMBEDDINGS_CREATE = "ai.embeddings.create.cohere" DB = "db" diff --git a/sentry_sdk/integrations/django/caching.py b/sentry_sdk/integrations/django/caching.py index 986adb5c9a..1529aa8a7a 100644 --- a/sentry_sdk/integrations/django/caching.py +++ b/sentry_sdk/integrations/django/caching.py @@ -48,11 +48,7 @@ def _get_key(args, kwargs): def _get_span_description(method_name, args, kwargs): # type: (str, list[Any], dict[str, Any]) -> str - description = "{} {}".format( - method_name, - _get_key(args, kwargs), - ) - return description + return _get_key(args, kwargs) def _patch_cache_method(cache, method_name, address, port): @@ -69,7 +65,7 @@ def _instrument_call( is_set_operation = method_name.startswith("set") is_get_operation = not is_set_operation - op = OP.CACHE_SET_ITEM if is_set_operation else OP.CACHE_GET_ITEM + op = OP.CACHE_SET if is_set_operation else OP.CACHE_GET description = _get_span_description(method_name, args, kwargs) with sentry_sdk.start_span(op=op, description=description) as span: diff --git a/sentry_sdk/integrations/redis/__init__.py b/sentry_sdk/integrations/redis/__init__.py index eff27f8df1..705e1ade82 100644 --- a/sentry_sdk/integrations/redis/__init__.py +++ b/sentry_sdk/integrations/redis/__init__.py @@ -221,7 +221,7 @@ def sentry_patched_execute_command(self, name, *args, **kwargs): # TODO: Here we could also create the caching spans. # TODO: also need to check if the `name` (if this is the cache key value) matches the prefix we want to configure in __init__ of the integration # Questions: - # -) We should probablby have the OP.DB_REDIS span and a separate OP.CACHE_GET_ITEM (or set_item) span, right? + # -) We should probablby have the OP.DB_REDIS span and a separate OP.CACHE_GET (or set) span, right? # -) We probably need to research what redis commands are used by caching libs. # GitHub issue: https://github.com/getsentry/sentry-python/issues/2965 with sentry_sdk.start_span(op=OP.DB_REDIS, description=description) as span: diff --git a/sentry_sdk/integrations/redis/asyncio.py b/sentry_sdk/integrations/redis/asyncio.py index 5a4943c484..c40b441dfa 100644 --- a/sentry_sdk/integrations/redis/asyncio.py +++ b/sentry_sdk/integrations/redis/asyncio.py @@ -61,7 +61,7 @@ async def _sentry_execute_command(self, name, *args, **kwargs): # TODO: Here we could also create the caching spans. # Questions: - # -) We should probablby have the OP.DB_REDIS span and a separate OP.CACHE_GET_ITEM (or set_item) span, right? + # -) We should probablby have the OP.DB_REDIS span and a separate OP.CACHE_GET (or set) span, right? # -) We probably need to research what redis commands are used by caching libs. # GitHub issue: https://github.com/getsentry/sentry-python/issues/2965 with sentry_sdk.start_span(op=OP.DB_REDIS, description=description) as span: diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index f2731ee73c..201d51e38d 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -193,7 +193,7 @@ def test_cache_spans_middleware( (first_event, second_event) = events # first_event - cache.get - assert first_event["spans"][0]["op"] == "cache.get_item" + assert first_event["spans"][0]["op"] == "cache.get" assert first_event["spans"][0]["description"].startswith( "get views.decorators.cache.cache_header." ) @@ -204,7 +204,7 @@ def test_cache_spans_middleware( assert not first_event["spans"][0]["data"]["cache.hit"] assert "cache.item_size" not in first_event["spans"][0]["data"] # first_event - cache.set - assert first_event["spans"][1]["op"] == "cache.set_item" + assert first_event["spans"][1]["op"] == "cache.set" assert first_event["spans"][1]["description"].startswith( "set views.decorators.cache.cache_header." ) @@ -215,7 +215,7 @@ def test_cache_spans_middleware( assert "cache.hit" not in first_event["spans"][1]["data"] assert first_event["spans"][1]["data"]["cache.item_size"] == 2 # second_event - cache.get - assert second_event["spans"][0]["op"] == "cache.get_item" + assert second_event["spans"][0]["op"] == "cache.get" assert second_event["spans"][0]["description"].startswith( "get views.decorators.cache.cache_header." ) @@ -226,7 +226,7 @@ def test_cache_spans_middleware( assert not second_event["spans"][0]["data"]["cache.hit"] assert "cache.item_size" not in second_event["spans"][0]["data"] # second_event - cache.get 2 - assert second_event["spans"][1]["op"] == "cache.get_item" + assert second_event["spans"][1]["op"] == "cache.get" assert second_event["spans"][1]["description"].startswith( "get views.decorators.cache.cache_page." ) @@ -259,7 +259,7 @@ def test_cache_spans_decorator(sentry_init, client, capture_events, use_django_c (first_event, second_event) = events # first_event - cache.get - assert first_event["spans"][0]["op"] == "cache.get_item" + assert first_event["spans"][0]["op"] == "cache.get" assert first_event["spans"][0]["description"].startswith( "get views.decorators.cache.cache_header." ) @@ -270,7 +270,7 @@ def test_cache_spans_decorator(sentry_init, client, capture_events, use_django_c assert not first_event["spans"][0]["data"]["cache.hit"] assert "cache.item_size" not in first_event["spans"][0]["data"] # first_event - cache.set - assert first_event["spans"][1]["op"] == "cache.set_item" + assert first_event["spans"][1]["op"] == "cache.set" assert first_event["spans"][1]["description"].startswith( "set views.decorators.cache.cache_header." ) @@ -281,7 +281,7 @@ def test_cache_spans_decorator(sentry_init, client, capture_events, use_django_c assert "cache.hit" not in first_event["spans"][1]["data"] assert first_event["spans"][1]["data"]["cache.item_size"] == 2 # second_event - cache.get - assert second_event["spans"][1]["op"] == "cache.get_item" + assert second_event["spans"][1]["op"] == "cache.get" assert second_event["spans"][1]["description"].startswith( "get views.decorators.cache.cache_page." ) @@ -317,7 +317,7 @@ def test_cache_spans_templatetag( (first_event, second_event) = events assert len(first_event["spans"]) == 2 # first_event - cache.get - assert first_event["spans"][0]["op"] == "cache.get_item" + assert first_event["spans"][0]["op"] == "cache.get" assert first_event["spans"][0]["description"].startswith( "get template.cache.some_identifier." ) @@ -328,7 +328,7 @@ def test_cache_spans_templatetag( assert not first_event["spans"][0]["data"]["cache.hit"] assert "cache.item_size" not in first_event["spans"][0]["data"] # first_event - cache.set - assert first_event["spans"][1]["op"] == "cache.set_item" + assert first_event["spans"][1]["op"] == "cache.set" assert first_event["spans"][1]["description"].startswith( "set template.cache.some_identifier." ) @@ -339,7 +339,7 @@ def test_cache_spans_templatetag( assert "cache.hit" not in first_event["spans"][1]["data"] assert first_event["spans"][1]["data"]["cache.item_size"] == 51 # second_event - cache.get - assert second_event["spans"][0]["op"] == "cache.get_item" + assert second_event["spans"][0]["op"] == "cache.get" assert second_event["spans"][0]["description"].startswith( "get template.cache.some_identifier." ) @@ -485,24 +485,24 @@ def test_cache_spans_item_size(sentry_init, client, capture_events, use_django_c (first_event, second_event) = events assert len(first_event["spans"]) == 3 - assert first_event["spans"][0]["op"] == "cache.get_item" + assert first_event["spans"][0]["op"] == "cache.get" assert not first_event["spans"][0]["data"]["cache.hit"] assert "cache.item_size" not in first_event["spans"][0]["data"] - assert first_event["spans"][1]["op"] == "cache.set_item" + assert first_event["spans"][1]["op"] == "cache.set" assert "cache.hit" not in first_event["spans"][1]["data"] assert first_event["spans"][1]["data"]["cache.item_size"] == 2 - assert first_event["spans"][2]["op"] == "cache.set_item" + assert first_event["spans"][2]["op"] == "cache.set" assert "cache.hit" not in first_event["spans"][2]["data"] assert first_event["spans"][2]["data"]["cache.item_size"] == 58 assert len(second_event["spans"]) == 2 - assert second_event["spans"][0]["op"] == "cache.get_item" + assert second_event["spans"][0]["op"] == "cache.get" assert not second_event["spans"][0]["data"]["cache.hit"] assert "cache.item_size" not in second_event["spans"][0]["data"] - assert second_event["spans"][1]["op"] == "cache.get_item" + assert second_event["spans"][1]["op"] == "cache.get" assert second_event["spans"][1]["data"]["cache.hit"] assert second_event["spans"][1]["data"]["cache.item_size"] == 58 @@ -534,25 +534,25 @@ def test_cache_spans_get_many(sentry_init, capture_events, use_django_caching): (transaction,) = events assert len(transaction["spans"]) == 7 - assert transaction["spans"][0]["op"] == "cache.get_item" + assert transaction["spans"][0]["op"] == "cache.get" assert transaction["spans"][0]["description"] == f"get_many ['S{id}', 'S{id+1}']" - assert transaction["spans"][1]["op"] == "cache.get_item" + assert transaction["spans"][1]["op"] == "cache.get" assert transaction["spans"][1]["description"] == f"get S{id}" - assert transaction["spans"][2]["op"] == "cache.get_item" + assert transaction["spans"][2]["op"] == "cache.get" assert transaction["spans"][2]["description"] == f"get S{id+1}" - assert transaction["spans"][3]["op"] == "cache.set_item" + assert transaction["spans"][3]["op"] == "cache.set" assert transaction["spans"][3]["description"] == f"set S{id}" - assert transaction["spans"][4]["op"] == "cache.get_item" + assert transaction["spans"][4]["op"] == "cache.get" assert transaction["spans"][4]["description"] == f"get_many ['S{id}', 'S{id+1}']" - assert transaction["spans"][5]["op"] == "cache.get_item" + assert transaction["spans"][5]["op"] == "cache.get" assert transaction["spans"][5]["description"] == f"get S{id}" - assert transaction["spans"][6]["op"] == "cache.get_item" + assert transaction["spans"][6]["op"] == "cache.get" assert transaction["spans"][6]["description"] == f"get S{id+1}" @@ -582,17 +582,17 @@ def test_cache_spans_set_many(sentry_init, capture_events, use_django_caching): (transaction,) = events assert len(transaction["spans"]) == 4 - assert transaction["spans"][0]["op"] == "cache.set_item" + assert transaction["spans"][0]["op"] == "cache.set" assert ( transaction["spans"][0]["description"] == f"set_many {{'S{id}': '[Filtered]', 'S{id+1}': '[Filtered]'}}" ) - assert transaction["spans"][1]["op"] == "cache.set_item" + assert transaction["spans"][1]["op"] == "cache.set" assert transaction["spans"][1]["description"] == f"set S{id}" - assert transaction["spans"][2]["op"] == "cache.set_item" + assert transaction["spans"][2]["op"] == "cache.set" assert transaction["spans"][2]["description"] == f"set S{id+1}" - assert transaction["spans"][3]["op"] == "cache.get_item" + assert transaction["spans"][3]["op"] == "cache.get" assert transaction["spans"][3]["description"] == f"get S{id}" From a001e15ed3fb22624420ace074ae7f07009a99fa Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 21 May 2024 17:18:44 +0200 Subject: [PATCH 51/54] cleanup --- sentry_sdk/integrations/redis/__init__.py | 6 ------ sentry_sdk/integrations/redis/asyncio.py | 5 ----- 2 files changed, 11 deletions(-) diff --git a/sentry_sdk/integrations/redis/__init__.py b/sentry_sdk/integrations/redis/__init__.py index 705e1ade82..725290407b 100644 --- a/sentry_sdk/integrations/redis/__init__.py +++ b/sentry_sdk/integrations/redis/__init__.py @@ -218,12 +218,6 @@ def sentry_patched_execute_command(self, name, *args, **kwargs): if data_should_be_truncated: description = description[: integration.max_data_size - len("...")] + "..." - # TODO: Here we could also create the caching spans. - # TODO: also need to check if the `name` (if this is the cache key value) matches the prefix we want to configure in __init__ of the integration - # Questions: - # -) We should probablby have the OP.DB_REDIS span and a separate OP.CACHE_GET (or set) span, right? - # -) We probably need to research what redis commands are used by caching libs. - # GitHub issue: https://github.com/getsentry/sentry-python/issues/2965 with sentry_sdk.start_span(op=OP.DB_REDIS, description=description) as span: set_db_data_fn(span, self) _set_client_data(span, is_cluster, name, *args) diff --git a/sentry_sdk/integrations/redis/asyncio.py b/sentry_sdk/integrations/redis/asyncio.py index c40b441dfa..6cb12b0d51 100644 --- a/sentry_sdk/integrations/redis/asyncio.py +++ b/sentry_sdk/integrations/redis/asyncio.py @@ -59,11 +59,6 @@ async def _sentry_execute_command(self, name, *args, **kwargs): description = _get_span_description(name, *args) - # TODO: Here we could also create the caching spans. - # Questions: - # -) We should probablby have the OP.DB_REDIS span and a separate OP.CACHE_GET (or set) span, right? - # -) We probably need to research what redis commands are used by caching libs. - # GitHub issue: https://github.com/getsentry/sentry-python/issues/2965 with sentry_sdk.start_span(op=OP.DB_REDIS, description=description) as span: set_db_data_fn(span, self) _set_client_data(span, is_cluster, name, *args) From 7ac3c62e7c4abf2cce810face7534c6a3b7f25dd Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 21 May 2024 17:20:28 +0200 Subject: [PATCH 52/54] cleanup --- tests/integrations/aiohttp/test_aiohttp.py | 2 +- tests/integrations/aws_lambda/test_aws.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integrations/aiohttp/test_aiohttp.py b/tests/integrations/aiohttp/test_aiohttp.py index 954cf853b2..2123f1c303 100644 --- a/tests/integrations/aiohttp/test_aiohttp.py +++ b/tests/integrations/aiohttp/test_aiohttp.py @@ -287,7 +287,7 @@ async def hello(request): async def test_traces_sampler_gets_request_object_in_sampling_context( sentry_init, aiohttp_client, - DictionaryContaining, # noqa:N803 + DictionaryContaining, # noqa: N803 ObjectDescribedBy, # noqa: N803 ): traces_sampler = mock.Mock() diff --git a/tests/integrations/aws_lambda/test_aws.py b/tests/integrations/aws_lambda/test_aws.py index 98196d1fcb..d18511397b 100644 --- a/tests/integrations/aws_lambda/test_aws.py +++ b/tests/integrations/aws_lambda/test_aws.py @@ -554,7 +554,7 @@ def test_handler(event, context): def test_traces_sampler_gets_correct_values_in_sampling_context( run_lambda_function, - DictionaryContaining, # noqa:N803 + DictionaryContaining, # noqa: N803 ObjectDescribedBy, # noqa: N803 StringContaining, # noqa: N803 ): From dbdf1e9f712a6a3cc0066b199baa70fa11b70b58 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 21 May 2024 17:47:13 +0200 Subject: [PATCH 53/54] Fixed tests --- .../integrations/django/test_cache_module.py | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index 201d51e38d..4e4724c752 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -319,7 +319,7 @@ def test_cache_spans_templatetag( # first_event - cache.get assert first_event["spans"][0]["op"] == "cache.get" assert first_event["spans"][0]["description"].startswith( - "get template.cache.some_identifier." + "template.cache.some_identifier." ) assert first_event["spans"][0]["data"]["network.peer.address"] is not None assert first_event["spans"][0]["data"]["cache.key"].startswith( @@ -330,7 +330,7 @@ def test_cache_spans_templatetag( # first_event - cache.set assert first_event["spans"][1]["op"] == "cache.set" assert first_event["spans"][1]["description"].startswith( - "set template.cache.some_identifier." + "template.cache.some_identifier." ) assert first_event["spans"][1]["data"]["network.peer.address"] is not None assert first_event["spans"][1]["data"]["cache.key"].startswith( @@ -341,7 +341,7 @@ def test_cache_spans_templatetag( # second_event - cache.get assert second_event["spans"][0]["op"] == "cache.get" assert second_event["spans"][0]["description"].startswith( - "get template.cache.some_identifier." + "template.cache.some_identifier." ) assert second_event["spans"][0]["data"]["network.peer.address"] is not None assert second_event["spans"][0]["data"]["cache.key"].startswith( @@ -354,27 +354,27 @@ def test_cache_spans_templatetag( @pytest.mark.parametrize( "method_name, args, kwargs, expected_description", [ - ("get", None, None, "get "), - ("get", [], {}, "get "), - ("get", ["bla", "blub", "foo"], {}, "get bla"), + ("get", None, None, ""), + ("get", [], {}, ""), + ("get", ["bla", "blub", "foo"], {}, "bla"), ( "get_many", [["bla 1", "bla 2", "bla 3"], "blub", "foo"], {}, - "get_many ['bla 1', 'bla 2', 'bla 3']", + "['bla 1', 'bla 2', 'bla 3']", ), ( "get_many", [["bla 1", "bla 2", "bla 3"], "blub", "foo"], {"key": "bar"}, - "get_many ['bla 1', 'bla 2', 'bla 3']", + "['bla 1', 'bla 2', 'bla 3']", ), - ("get", [], {"key": "bar"}, "get bar"), + ("get", [], {"key": "bar"}, "bar"), ( "get", "something", {}, - "get s", + "s", ), # this should never happen, just making sure that we are not raising an exception in that case. ], ) From 891b453ec31f061770dd9b86a9687883ba6477b0 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 21 May 2024 17:54:54 +0200 Subject: [PATCH 54/54] Fixed tests --- .../integrations/django/test_cache_module.py | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index 4e4724c752..3815d4249a 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -195,7 +195,7 @@ def test_cache_spans_middleware( # first_event - cache.get assert first_event["spans"][0]["op"] == "cache.get" assert first_event["spans"][0]["description"].startswith( - "get views.decorators.cache.cache_header." + "views.decorators.cache.cache_header." ) assert first_event["spans"][0]["data"]["network.peer.address"] is not None assert first_event["spans"][0]["data"]["cache.key"].startswith( @@ -206,7 +206,7 @@ def test_cache_spans_middleware( # first_event - cache.set assert first_event["spans"][1]["op"] == "cache.set" assert first_event["spans"][1]["description"].startswith( - "set views.decorators.cache.cache_header." + "views.decorators.cache.cache_header." ) assert first_event["spans"][1]["data"]["network.peer.address"] is not None assert first_event["spans"][1]["data"]["cache.key"].startswith( @@ -217,7 +217,7 @@ def test_cache_spans_middleware( # second_event - cache.get assert second_event["spans"][0]["op"] == "cache.get" assert second_event["spans"][0]["description"].startswith( - "get views.decorators.cache.cache_header." + "views.decorators.cache.cache_header." ) assert second_event["spans"][0]["data"]["network.peer.address"] is not None assert second_event["spans"][0]["data"]["cache.key"].startswith( @@ -228,7 +228,7 @@ def test_cache_spans_middleware( # second_event - cache.get 2 assert second_event["spans"][1]["op"] == "cache.get" assert second_event["spans"][1]["description"].startswith( - "get views.decorators.cache.cache_page." + "views.decorators.cache.cache_page." ) assert second_event["spans"][1]["data"]["network.peer.address"] is not None assert second_event["spans"][1]["data"]["cache.key"].startswith( @@ -261,7 +261,7 @@ def test_cache_spans_decorator(sentry_init, client, capture_events, use_django_c # first_event - cache.get assert first_event["spans"][0]["op"] == "cache.get" assert first_event["spans"][0]["description"].startswith( - "get views.decorators.cache.cache_header." + "views.decorators.cache.cache_header." ) assert first_event["spans"][0]["data"]["network.peer.address"] is not None assert first_event["spans"][0]["data"]["cache.key"].startswith( @@ -272,7 +272,7 @@ def test_cache_spans_decorator(sentry_init, client, capture_events, use_django_c # first_event - cache.set assert first_event["spans"][1]["op"] == "cache.set" assert first_event["spans"][1]["description"].startswith( - "set views.decorators.cache.cache_header." + "views.decorators.cache.cache_header." ) assert first_event["spans"][1]["data"]["network.peer.address"] is not None assert first_event["spans"][1]["data"]["cache.key"].startswith( @@ -283,7 +283,7 @@ def test_cache_spans_decorator(sentry_init, client, capture_events, use_django_c # second_event - cache.get assert second_event["spans"][1]["op"] == "cache.get" assert second_event["spans"][1]["description"].startswith( - "get views.decorators.cache.cache_page." + "views.decorators.cache.cache_page." ) assert second_event["spans"][1]["data"]["network.peer.address"] is not None assert second_event["spans"][1]["data"]["cache.key"].startswith( @@ -535,25 +535,25 @@ def test_cache_spans_get_many(sentry_init, capture_events, use_django_caching): assert len(transaction["spans"]) == 7 assert transaction["spans"][0]["op"] == "cache.get" - assert transaction["spans"][0]["description"] == f"get_many ['S{id}', 'S{id+1}']" + assert transaction["spans"][0]["description"] == f"['S{id}', 'S{id+1}']" assert transaction["spans"][1]["op"] == "cache.get" - assert transaction["spans"][1]["description"] == f"get S{id}" + assert transaction["spans"][1]["description"] == f"S{id}" assert transaction["spans"][2]["op"] == "cache.get" - assert transaction["spans"][2]["description"] == f"get S{id+1}" + assert transaction["spans"][2]["description"] == f"S{id+1}" assert transaction["spans"][3]["op"] == "cache.set" - assert transaction["spans"][3]["description"] == f"set S{id}" + assert transaction["spans"][3]["description"] == f"S{id}" assert transaction["spans"][4]["op"] == "cache.get" - assert transaction["spans"][4]["description"] == f"get_many ['S{id}', 'S{id+1}']" + assert transaction["spans"][4]["description"] == f"['S{id}', 'S{id+1}']" assert transaction["spans"][5]["op"] == "cache.get" - assert transaction["spans"][5]["description"] == f"get S{id}" + assert transaction["spans"][5]["description"] == f"S{id}" assert transaction["spans"][6]["op"] == "cache.get" - assert transaction["spans"][6]["description"] == f"get S{id+1}" + assert transaction["spans"][6]["description"] == f"S{id+1}" @pytest.mark.forked @@ -585,14 +585,14 @@ def test_cache_spans_set_many(sentry_init, capture_events, use_django_caching): assert transaction["spans"][0]["op"] == "cache.set" assert ( transaction["spans"][0]["description"] - == f"set_many {{'S{id}': '[Filtered]', 'S{id+1}': '[Filtered]'}}" + == f"{{'S{id}': '[Filtered]', 'S{id+1}': '[Filtered]'}}" ) assert transaction["spans"][1]["op"] == "cache.set" - assert transaction["spans"][1]["description"] == f"set S{id}" + assert transaction["spans"][1]["description"] == f"S{id}" assert transaction["spans"][2]["op"] == "cache.set" - assert transaction["spans"][2]["description"] == f"set S{id+1}" + assert transaction["spans"][2]["description"] == f"S{id+1}" assert transaction["spans"][3]["op"] == "cache.get" - assert transaction["spans"][3]["description"] == f"get S{id}" + assert transaction["spans"][3]["description"] == f"S{id}"