Skip to content

Commit

Permalink
Remove sentry exception capture for thumbnail errors (#3709)
Browse files Browse the repository at this point in the history
* Remove sentry exception capture for thumbnail errors

* Remove unused code, reduce test cases
  • Loading branch information
AetherUnbound authored Jan 30, 2024
1 parent a3f7930 commit b99d89d
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 70 deletions.
27 changes: 2 additions & 25 deletions api/api/utils/image_proxy/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import itertools
import logging
from dataclasses import dataclass
from typing import Literal
Expand All @@ -10,11 +9,9 @@

import aiohttp
import django_redis
import sentry_sdk
from aiohttp.client_exceptions import ClientResponseError
from asgiref.sync import sync_to_async
from redis.exceptions import ConnectionError
from sentry_sdk import push_scope, set_context

from api.utils.aiohttp import get_aiohttp_session
from api.utils.asyncio import do_not_wait_for
Expand All @@ -26,8 +23,6 @@

parent_logger = logging.getLogger(__name__)

exception_iterator = itertools.count()

HEADERS = {
"User-Agent": settings.OUTBOUND_USER_AGENT_TEMPLATE.format(
purpose="ThumbnailGeneration"
Expand Down Expand Up @@ -192,28 +187,10 @@ async def get(
exception_name = f"{exc.__class__.__module__}.{exc.__class__.__name__}"
key = f"thumbnail_error:{exception_name}:{domain}:{month}"
try:
count = await tallies_incr(key)
await tallies_incr(key)
except ConnectionError:
logger.warning("Redis connect failed, thumbnail errors not tallied.")
# We will use a counter to space out Sentry logs.
count = next(exception_iterator)

if count <= settings.THUMBNAIL_ERROR_INITIAL_ALERT_THRESHOLD or (
count % settings.THUMBNAIL_ERROR_REPEATED_ALERT_FREQUENCY == 0
):
with push_scope() as scope:
set_context(
"upstream_url",
{
"url": upstream_url,
"params": params,
"headers": headers,
},
)
scope.set_tag(
"occurrences", settings.THUMBNAIL_ERROR_REPEATED_ALERT_FREQUENCY
)
sentry_sdk.capture_exception(exc)

if isinstance(exc, ClientResponseError):
status = exc.status
do_not_wait_for(
Expand Down
8 changes: 0 additions & 8 deletions api/conf/settings/thumbnails.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,3 @@
# to cast them in assertions to match the parsed param types)
THUMBNAIL_WIDTH_PX = config("THUMBNAIL_WIDTH_PX", default="600")
THUMBNAIL_QUALITY = config("THUMBNAIL_JPG_QUALITY", default="80")

THUMBNAIL_ERROR_INITIAL_ALERT_THRESHOLD = config(
"THUMBNAIL_ERROR_INITIAL_ALERT_THRESHOLD", default=100, cast=int
)

THUMBNAIL_ERROR_REPEATED_ALERT_FREQUENCY = config(
"THUMBNAIL_ERROR_REPEATED_ALERT_FREQUENCY", default=1000, cast=int
)
43 changes: 6 additions & 37 deletions api/test/unit/utils/test_image_proxy.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import asyncio
import itertools
from dataclasses import replace
from unittest.mock import patch
from urllib.parse import urlencode

from django.conf import settings
Expand Down Expand Up @@ -331,18 +329,8 @@ def test_get_successful_records_response_code(


alert_count_params = pytest.mark.parametrize(
"count_start, should_alert",
[
(0, True),
(1, True),
(50, True),
(99, True),
(100, False),
(999, True),
(1000, False),
(1500, False),
(1999, True),
],
"count_start",
[0, 1, 999],
)

MOCK_CONNECTION_KEY = ConnectionKey(
Expand Down Expand Up @@ -385,7 +373,6 @@ def test_get_exception_handles_error(
exc,
exc_name,
count_start,
should_alert,
sentry_capture_exception,
setup_request_exception,
is_cache_reachable,
Expand All @@ -401,20 +388,10 @@ def test_get_exception_handles_error(
if is_cache_reachable:
cache.set(key, count_start)

with (
pytest.raises(UpstreamThumbnailException),
patch(
"api.utils.image_proxy.exception_iterator", itertools.count(count_start + 1)
),
):
with pytest.raises(UpstreamThumbnailException):
photon_get(TEST_MEDIA_INFO)

assert_func = (
sentry_capture_exception.assert_called_once
if should_alert
else sentry_capture_exception.assert_not_called
)
assert_func()
sentry_capture_exception.assert_not_called()

if is_cache_reachable:
assert cache.get(key) == str(count_start + 1).encode()
Expand All @@ -438,7 +415,6 @@ def test_get_http_exception_handles_error(
status_code,
text,
count_start,
should_alert,
sentry_capture_exception,
is_cache_reachable,
cache_name,
Expand All @@ -452,19 +428,12 @@ def test_get_http_exception_handles_error(
if is_cache_reachable:
cache.set(key, count_start)

with pytest.raises(UpstreamThumbnailException), patch(
"api.utils.image_proxy.exception_iterator", itertools.count(count_start + 1)
):
with pytest.raises(UpstreamThumbnailException):
with pook.use():
pook.get(PHOTON_URL_FOR_TEST_IMAGE).reply(status_code, text)
photon_get(TEST_MEDIA_INFO)

assert_func = (
sentry_capture_exception.assert_called_once
if should_alert
else sentry_capture_exception.assert_not_called
)
assert_func()
sentry_capture_exception.assert_not_called()

if is_cache_reachable:
assert cache.get(key) == str(count_start + 1).encode()
Expand Down

0 comments on commit b99d89d

Please sign in to comment.