Django caching instrumentation update

sentry_sdk/consts.py

@@ -209,6 +209,24 @@ class SPANDATA:
     Example: 58
     """
 
+    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"
     """
     The Query string present in the URL.
@@ -298,6 +316,7 @@ class SPANDATA:
 class OP:
     ANTHROPIC_MESSAGES_CREATE = "ai.messages.create.anthropic"
     CACHE_GET_ITEM = "cache.get_item"
+    CACHE_SET_ITEM = "cache.set_item"
     DB = "db"
     DB_REDIS = "db.redis"
     EVENT_DJANGO = "event.django"

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 = ""
@@ -119,19 +130,23 @@ 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)"
                 % (transaction_style, TRANSACTION_STYLE_VALUES)
             )
         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

sentry_sdk/integrations/django/caching.py

@@ -1,80 +1,138 @@
 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:
     from typing import Any
     from typing import Callable
+    from typing import Tuple
+    from typing import Optional
 
 
 METHODS_TO_INSTRUMENT = [
+    "set",
     "get",
-    "get_many",
 ]
 
 
-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
 
 
-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
+    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
+
+        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)
-
-            else:
-                span.set_data(SPANDATA.CACHE_HIT, False)
+            with capture_internal_exceptions():
+                if address is not None:
+                    # 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)
+
+                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:
+                    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)
 
             return value
 
     @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=None, port=None):
+    # type: (CacheHandler, Optional[str], Optional[str]) -> None
     if not hasattr(cache, "_sentry_patched"):
         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
 
 
+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
+    # 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
@@ -90,7 +148,13 @@ def sentry_get_item(self, alias):
 
                 integration = sentry_sdk.get_client().get_integration(DjangoIntegration)
                 if integration is not None and integration.cache_spans:
-                    _patch_cache(cache)
+                    from django.conf import settings
+
+                    address, port = _get_address_port(
+                        settings.CACHES[alias or "default"]
+                    )
+
+                    _patch_cache(cache, address, port)
 
                 return cache
 
@@ -107,7 +171,9 @@ 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)
+                    address, port = _get_address_port(self.settings[alias or "default"])
+
+                    _patch_cache(cache, address, port)
 
                 return cache
 

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():

sentry_sdk/integrations/redis/asyncio.py

@@ -59,6 +59,10 @@ 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_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)