From b99d89deb6f5e83a59969ecbed531afff79577ba Mon Sep 17 00:00:00 2001 From: Madison Swain-Bowden Date: Tue, 30 Jan 2024 15:32:37 -0800 Subject: [PATCH] Remove sentry exception capture for thumbnail errors (#3709) * Remove sentry exception capture for thumbnail errors * Remove unused code, reduce test cases --- api/api/utils/image_proxy/__init__.py | 27 ++-------------- api/conf/settings/thumbnails.py | 8 ----- api/test/unit/utils/test_image_proxy.py | 43 ++++--------------------- 3 files changed, 8 insertions(+), 70 deletions(-) diff --git a/api/api/utils/image_proxy/__init__.py b/api/api/utils/image_proxy/__init__.py index 03da60f1888..21b0b302521 100644 --- a/api/api/utils/image_proxy/__init__.py +++ b/api/api/utils/image_proxy/__init__.py @@ -1,4 +1,3 @@ -import itertools import logging from dataclasses import dataclass from typing import Literal @@ -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 @@ -26,8 +23,6 @@ parent_logger = logging.getLogger(__name__) -exception_iterator = itertools.count() - HEADERS = { "User-Agent": settings.OUTBOUND_USER_AGENT_TEMPLATE.format( purpose="ThumbnailGeneration" @@ -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( diff --git a/api/conf/settings/thumbnails.py b/api/conf/settings/thumbnails.py index 329bc222244..b5bf94a7b6b 100644 --- a/api/conf/settings/thumbnails.py +++ b/api/conf/settings/thumbnails.py @@ -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 -) diff --git a/api/test/unit/utils/test_image_proxy.py b/api/test/unit/utils/test_image_proxy.py index cc0c9871545..8b5401e5371 100644 --- a/api/test/unit/utils/test_image_proxy.py +++ b/api/test/unit/utils/test_image_proxy.py @@ -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 @@ -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( @@ -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, @@ -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() @@ -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, @@ -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()