Skip to content

Commit

Permalink
Add statsd metrics for locale redirects (#13314)
Browse files Browse the repository at this point in the history
  • Loading branch information
robhudson committed Dec 1, 2023
1 parent fa5a81a commit 136ed77
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 11 deletions.
3 changes: 3 additions & 0 deletions lib/l10n_utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
28 changes: 17 additions & 11 deletions lib/l10n_utils/tests/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit 136ed77

Please sign in to comment.