diff --git a/snuba/state/cache/redis/backend.py b/snuba/state/cache/redis/backend.py index 4062e12102d..49ac5671f15 100644 --- a/snuba/state/cache/redis/backend.py +++ b/snuba/state/cache/redis/backend.py @@ -1,5 +1,5 @@ import logging -from typing import Callable, Optional +from typing import Callable, Optional, Type import sentry_sdk @@ -23,10 +23,29 @@ RESULT_WAIT = 2 SIMPLE_READTHROUGH = 3 -DONT_CAPTURE_ERRORS = { + +class FuzzyMatchException: + + def __init__(self, exception: Type[Exception], message: str | None = None): + self._exception = exception + self._message = message + + def __eq__(self, other: object) -> bool: + if isinstance(other, Exception): + return isinstance(other, self._exception) and ( + self._message is None or self._message == str(other) + ) + elif isinstance(other, self.__class__): + return other._exception == self._exception and other._message == self._message + else: + return False + + +DONT_CAPTURE_ERRORS = ( # if you need to track this error, see datadog metric snuba.read_through_cache.redis_cache_set_error - ResponseError("OOM command not allowed under OOM prevention."), -} + FuzzyMatchException(ResponseError, "OOM command not allowed under OOM prevention."), + FuzzyMatchException(RedisTimeoutError), +) class RedisCache(Cache[TValue]): @@ -70,6 +89,7 @@ def __get_value_with_simple_readthrough( ) -> TValue: record_cache_hit_type(SIMPLE_READTHROUGH) result_key = self.__build_key(key) + metric_tags = (timer.tags if timer is not None else {}) or {} cached_value = None try: @@ -79,10 +99,10 @@ def __get_value_with_simple_readthrough( raise e if e not in DONT_CAPTURE_ERRORS: sentry_sdk.capture_exception(e) + metrics.increment("redis_cache_get_error", tags={"error": str(e), **metric_tags}) if timer is not None: timer.mark("cache_get") - metric_tags = timer.tags if timer is not None else {} if cached_value is not None: record_cache_hit_type(RESULT_VALUE) @@ -96,8 +116,11 @@ def __get_value_with_simple_readthrough( self.__codec.encode(value), ex=get_config("cache_expiry_sec", 1), ) + except Exception as e: - metrics.increment("redis_cache_set_error", tags=metric_tags) + metrics.increment( + "redis_cache_set_error", tags={"error": str(e), **metric_tags} + ) if e not in DONT_CAPTURE_ERRORS: sentry_sdk.capture_exception(e) return value diff --git a/tests/state/test_cache.py b/tests/state/test_cache.py index e3dd5177064..f45cc9b9805 100644 --- a/tests/state/test_cache.py +++ b/tests/state/test_cache.py @@ -9,10 +9,12 @@ import pytest import rapidjson +import sentry_sdk from sentry_redis_tools.failover_redis import FailoverRedis -from redis import RedisError +from redis import RedisError, ResponseError from redis.exceptions import ReadOnlyError +from redis.exceptions import TimeoutError as RedisTimeoutError from snuba.redis import RedisClientKey, get_redis_client from snuba.state import set_config from snuba.state.cache.abstract import Cache @@ -255,3 +257,14 @@ def test_set_fails_open(backend: Cache[bytes]) -> None: assert isinstance(backend, RedisCache) with mock.patch.object(redis_client, "set", side_effect=RedisError()): backend.get_readthrough("key", lambda: b"value", noop) + + +@pytest.mark.redis_db +@pytest.mark.parametrize( + "error", [ResponseError("OOM command not allowed under OOM prevention."), RedisTimeoutError()] +) +def test_dont_record_expected_errors(backend: Cache[bytes], error: Exception) -> None: + with mock.patch.object(redis_client, "set", side_effect=error): + with mock.patch.object(sentry_sdk, "capture_exception") as capture_exception: + backend.get_readthrough("key", lambda: b"value", noop) + capture_exception.assert_not_called()