Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 29 additions & 6 deletions snuba/state/cache/redis/backend.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import logging
from typing import Callable, Optional
from typing import Callable, Optional, Type

import sentry_sdk

Expand All @@ -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]):
Expand Down Expand Up @@ -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:
Expand All @@ -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)
Expand All @@ -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
Expand Down
15 changes: 14 additions & 1 deletion tests/state/test_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Loading