Skip to content

Commit

Permalink
Include media provider in thumbnail failure logging and tallying (#3324)
Browse files Browse the repository at this point in the history 
* Include media provider in thumbnail failure logging and tallying

* Make media provider required
  • Loading branch information
@AetherUnbound
AetherUnbound authored Nov 14, 2023
1 parent 5e55faa commit 822863f
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 16 deletions.
11 changes: 10 additions & 1 deletion api/api/utils/image_proxy/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ def get_request_params_for_extension(
def get(
image_url: str,
media_identifier: str,
media_provider: str,
accept_header: str = "image/*",
is_full_size: bool = False,
is_compressed: bool = True,
Expand Down Expand Up @@ -101,6 +102,10 @@ def get(
f"thumbnail_response_code_by_domain:{domain}:"
f"{month}:{upstream_response.status_code}"
)
tallies.incr(
f"thumbnail_response_code_by_provider:{media_provider}:"
f"{month}:{upstream_response.status_code}"
)
upstream_response.raise_for_status()
except Exception as exc:
exception_name = f"{exc.__class__.__module__}.{exc.__class__.__name__}"
Expand All @@ -123,8 +128,12 @@ def get(
)
sentry_sdk.capture_exception(exc)
if isinstance(exc, requests.exceptions.HTTPError):
code = exc.response.status_code
tallies.incr(
f"thumbnail_http_error:{domain}:{month}:{exc.response.status_code}:{exc.response.text}"
f"thumbnail_http_error:{domain}:{month}:{code}:{exc.response.text}"
)
logger.warning(
f"Failed to render thumbnail {upstream_url=} {code=} {media_provider=}"
)
raise UpstreamThumbnailException(f"Failed to render thumbnail. {exc}")

Expand Down
1 change: 1 addition & 0 deletions api/api/views/media_views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ def thumbnail(self, request, media_obj, image_url):
return image_proxy.get(
image_url,
media_obj.identifier,
media_obj.provider,
accept_header=request.headers.get("Accept", "image/*"),
**serializer.validated_data,
)
Expand Down
48 changes: 33 additions & 15 deletions api/test/unit/utils/test_image_proxy.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
PHOTON_URL_FOR_TEST_IMAGE = f"{settings.PHOTON_ENDPOINT}subdomain.example.com/path_part1/part2/image_dot_jpg.jpg"
TEST_IMAGE_URL = PHOTON_URL_FOR_TEST_IMAGE.replace(settings.PHOTON_ENDPOINT, "http://")
TEST_MEDIA_IDENTIFIER = "123"
TEST_MEDIA_PROVIDER = "foo"

UA_HEADER = HEADERS["User-Agent"]

Expand Down Expand Up @@ -59,7 +60,7 @@ def test_get_successful_no_auth_key_default_args(mock_image_data):
.mock
)

res = photon_get(TEST_IMAGE_URL, TEST_MEDIA_IDENTIFIER)
res = photon_get(TEST_IMAGE_URL, TEST_MEDIA_IDENTIFIER, TEST_MEDIA_PROVIDER)

assert res.content == MOCK_BODY.encode()
assert res.status_code == 200
Expand All @@ -77,7 +78,11 @@ def test_get_successful_original_svg_no_auth_key_default_args(mock_image_data):
.mock
)

res = photon_get(TEST_IMAGE_URL.replace(".jpg", ".svg"), TEST_MEDIA_IDENTIFIER)
res = photon_get(
TEST_IMAGE_URL.replace(".jpg", ".svg"),
TEST_MEDIA_IDENTIFIER,
TEST_MEDIA_PROVIDER,
)

assert res.content == SVG_BODY.encode()
assert res.status_code == 200
Expand All @@ -102,7 +107,7 @@ def test_get_successful_with_auth_key_default_args(mock_image_data, auth_key):
.mock
)

res = photon_get(TEST_IMAGE_URL, TEST_MEDIA_IDENTIFIER)
res = photon_get(TEST_IMAGE_URL, TEST_MEDIA_IDENTIFIER, TEST_MEDIA_PROVIDER)

assert res.content == MOCK_BODY.encode()
assert res.status_code == 200
Expand All @@ -125,7 +130,9 @@ def test_get_successful_no_auth_key_not_compressed(mock_image_data):
.mock
)

res = photon_get(TEST_IMAGE_URL, TEST_MEDIA_IDENTIFIER, is_compressed=False)
res = photon_get(
TEST_IMAGE_URL, TEST_MEDIA_IDENTIFIER, TEST_MEDIA_PROVIDER, is_compressed=False
)

assert res.content == MOCK_BODY.encode()
assert res.status_code == 200
Expand All @@ -148,7 +155,9 @@ def test_get_successful_no_auth_key_full_size(mock_image_data):
.mock
)

res = photon_get(TEST_IMAGE_URL, TEST_MEDIA_IDENTIFIER, is_full_size=True)
res = photon_get(
TEST_IMAGE_URL, TEST_MEDIA_IDENTIFIER, TEST_MEDIA_PROVIDER, is_full_size=True
)

assert res.content == MOCK_BODY.encode()
assert res.status_code == 200
Expand All @@ -167,7 +176,11 @@ def test_get_successful_no_auth_key_full_size_not_compressed(mock_image_data):
)

res = photon_get(
TEST_IMAGE_URL, TEST_MEDIA_IDENTIFIER, is_full_size=True, is_compressed=False
TEST_IMAGE_URL,
TEST_MEDIA_IDENTIFIER,
TEST_MEDIA_PROVIDER,
is_full_size=True,
is_compressed=False,
)

assert res.content == MOCK_BODY.encode()
Expand All @@ -192,7 +205,12 @@ def test_get_successful_no_auth_key_png_only(mock_image_data):
.mock
)

res = photon_get(TEST_IMAGE_URL, TEST_MEDIA_IDENTIFIER, accept_header="image/png")
res = photon_get(
TEST_IMAGE_URL,
TEST_MEDIA_IDENTIFIER,
TEST_MEDIA_PROVIDER,
accept_header="image/png",
)

assert res.content == MOCK_BODY.encode()
assert res.status_code == 200
Expand Down Expand Up @@ -220,7 +238,7 @@ def test_get_successful_forward_query_params(mock_image_data):

url_with_params = f"{TEST_IMAGE_URL}?{params}"

res = photon_get(url_with_params, TEST_MEDIA_IDENTIFIER)
res = photon_get(url_with_params, TEST_MEDIA_IDENTIFIER, TEST_MEDIA_PROVIDER)

assert res.content == MOCK_BODY.encode()
assert res.status_code == 200
Expand Down Expand Up @@ -255,7 +273,7 @@ def test_get_successful_records_response_code(mock_image_data, redis):
.mock
)

photon_get(TEST_IMAGE_URL, TEST_MEDIA_IDENTIFIER)
photon_get(TEST_IMAGE_URL, TEST_MEDIA_IDENTIFIER, TEST_MEDIA_PROVIDER)
month = get_monthly_timestamp()
assert redis.get(f"thumbnail_response_code:{month}:200") == b"1"
assert (
Expand Down Expand Up @@ -310,7 +328,7 @@ def test_get_exception_handles_error(
redis.set(key, count_start)

with pytest.raises(UpstreamThumbnailException):
photon_get(TEST_IMAGE_URL, TEST_MEDIA_IDENTIFIER)
photon_get(TEST_IMAGE_URL, TEST_MEDIA_IDENTIFIER, TEST_MEDIA_PROVIDER)

assert_func = (
capture_exception.assert_called_once
Expand Down Expand Up @@ -351,7 +369,7 @@ def test_get_http_exception_handles_error(
redis.set(key, count_start)

with pytest.raises(UpstreamThumbnailException):
photon_get(TEST_IMAGE_URL, TEST_MEDIA_IDENTIFIER)
photon_get(TEST_IMAGE_URL, TEST_MEDIA_IDENTIFIER, TEST_MEDIA_PROVIDER)

assert_func = (
capture_exception.assert_called_once
Expand Down Expand Up @@ -389,7 +407,7 @@ def test_get_successful_https_image_url_sends_ssl_parameter(mock_image_data):
.mock
)

res = photon_get(https_url, TEST_MEDIA_IDENTIFIER)
res = photon_get(https_url, TEST_MEDIA_IDENTIFIER, TEST_MEDIA_PROVIDER)

assert res.content == MOCK_BODY.encode()
assert res.status_code == 200
Expand All @@ -403,7 +421,7 @@ def test_get_unsuccessful_request_raises_custom_exception():
with pytest.raises(
UpstreamThumbnailException, match=r"Failed to render thumbnail."
):
photon_get(TEST_IMAGE_URL, TEST_MEDIA_IDENTIFIER)
photon_get(TEST_IMAGE_URL, TEST_MEDIA_IDENTIFIER, TEST_MEDIA_PROVIDER)

assert mock_get.matched

Expand Down Expand Up @@ -434,7 +452,7 @@ def test_photon_get_raises_by_not_allowed_types(image_type):
image = ImageFactory.create(url=image_url)

with pytest.raises(UnsupportedMediaType):
photon_get(image_url, image.identifier)
photon_get(image_url, image.identifier, image.provider)


@pytest.mark.django_db
Expand All @@ -451,7 +469,7 @@ def test_photon_get_saves_image_type_to_cache(redis, headers, expected_cache_val
with pook.use():
pook.head(image_url, reply=200, response_headers=headers)
with pytest.raises(UnsupportedMediaType):
photon_get(image_url, image.identifier)
photon_get(image_url, image.identifier, image.provider)

key = f"media:{image.identifier}:thumb_type"
assert redis.get(key) == expected_cache_val

0 comments on commit 822863f

Please sign in to comment.