From 136ed77dbfcbdb81745bb716fbb0bedb1f0c69ee Mon Sep 17 00:00:00 2001 From: Rob Hudson Date: Thu, 30 Nov 2023 15:06:58 -0800 Subject: [PATCH] Add statsd metrics for locale redirects (#13314) --- lib/l10n_utils/__init__.py | 3 +++ lib/l10n_utils/tests/test_base.py | 28 +++++++++++++++++----------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/lib/l10n_utils/__init__.py b/lib/l10n_utils/__init__.py index 00a14517e26..d5410a74fbb 100644 --- a/lib/l10n_utils/__init__.py +++ b/lib/l10n_utils/__init__.py @@ -13,6 +13,7 @@ from product_details import product_details +from bedrock.base import metrics from bedrock.base.urlresolvers import _get_language_map, split_path from .fluent import fluent_l10n, ftl_file_is_active, get_active_locales as ftl_active_locales @@ -64,6 +65,8 @@ def redirect_to_best_locale(request, translations): def redirect_to_locale(request, locale, permanent=False): redirect_class = HttpResponsePermanentRedirect if permanent else HttpResponseRedirect response = redirect_class("/" + "/".join([locale, split_path(request.get_full_path())[1]])) + # Record count of redirects to this locale. + metrics.incr("locale.redirect", tags=[f"from_locale:{get_locale(request) or 'none'}", f"to_locale:{locale}"]) # Add the Vary header to avoid wrong redirects due to a cache response["Vary"] = "Accept-Language" return response diff --git a/lib/l10n_utils/tests/test_base.py b/lib/l10n_utils/tests/test_base.py index 09d42652b2d..dee57071bf1 100644 --- a/lib/l10n_utils/tests/test_base.py +++ b/lib/l10n_utils/tests/test_base.py @@ -11,6 +11,7 @@ import pytest from django_jinja.backend import Jinja2 +from markus.testing import MetricsMock from bedrock.base.urlresolvers import Prefixer from lib import l10n_utils @@ -41,18 +42,23 @@ def _test(self, path, template, locale, accept_lang, status, destination=None, a if add_active_locales: ctx["add_active_locales"] = add_active_locales - response = l10n_utils.render(request, template, ctx) - - if status == 302: - self.assertEqual(response.status_code, 302) - self.assertEqual(response["Location"], destination) - self.assertEqual(response["Vary"], "Accept-Language") - elif status == 404: - self.assertEqual(response.status_code, 404) - else: - self.assertEqual(response.status_code, 200) - if path == "/": + with MetricsMock() as mm: + response = l10n_utils.render(request, template, ctx) + + if status == 302: + self.assertEqual(response.status_code, 302) + self.assertEqual(response["Location"], destination) self.assertEqual(response["Vary"], "Accept-Language") + mm.assert_incr_once("locale.redirect", tags=[f"from_locale:{prefixer.locale or 'none'}", f"to_locale:{destination.split('/')[1]}"]) + + elif status == 404: + self.assertEqual(response.status_code, 404) + mm.assert_not_incr("locale.redirect") + else: + self.assertEqual(response.status_code, 200) + mm.assert_not_incr("locale.redirect") + if path == "/": + self.assertEqual(response["Vary"], "Accept-Language") def test_no_accept_language_header(self): template = "firefox/new.html"