diff --git a/kinto/core/initialization.py b/kinto/core/initialization.py index 397dc7ce9..0b8fa7adc 100644 --- a/kinto/core/initialization.py +++ b/kinto/core/initialization.py @@ -1,6 +1,7 @@ import logging import random import re +import urllib.parse import warnings from datetime import datetime from secrets import token_hex @@ -474,6 +475,7 @@ def on_new_response(event): try: endpoint = utils.strip_uri_prefix(request.path) + endpoint = urllib.parse.quote_plus(endpoint, safe="/?&=-_") except UnicodeDecodeError as e: # This `on_new_response` callback is also called when a HTTP 400 # is returned because of an invalid UTF-8 path. We still want metrics. @@ -507,7 +509,7 @@ def on_new_response(event): unique=[ ("method", request.method.lower()), ("endpoint", endpoint), - ("status", str(request.response.status_code)), + ("status", str(event.response.status_code)), ] + metrics_matchdict_labels, ) @@ -527,7 +529,7 @@ def on_new_response(event): # Observe response size. metrics_service.observe( "request_size", - len(request.response.body or b""), + len(event.response.body or b""), labels=[("endpoint", endpoint)] + metrics_matchdict_labels, ) diff --git a/kinto/plugins/statsd.py b/kinto/plugins/statsd.py index b6a8830fb..5ae894011 100644 --- a/kinto/plugins/statsd.py +++ b/kinto/plugins/statsd.py @@ -13,6 +13,14 @@ statsd_module = None +def sanitize(value): + """ + Telegraf does not support ':' in values. + See https://github.com/influxdata/telegraf/issues/4495 + """ + return value.replace(":", "") if isinstance(value, str) else value + + @implementer(metrics.IMetricsService) class StatsDService: def __init__(self, host, port, prefix): @@ -22,7 +30,7 @@ def timer(self, key): return self._client.timer(key) def observe(self, key, value, labels=[]): - return self._client.gauge(key, value) + return self._client.gauge(key, sanitize(value)) def count(self, key, count=1, unique=None): if unique is None: @@ -30,7 +38,7 @@ def count(self, key, count=1, unique=None): if isinstance(unique, list): # [("method", "get")] -> "method.get" # [("endpoint", "/"), ("method", "get")] -> "endpoint./.method.get" - unique = ".".join(f"{label[0]}.{label[1]}" for label in unique) + unique = ".".join(f"{label[0]}.{sanitize(label[1])}" for label in unique) else: warnings.warn( "`unique` parameter should be of type ``list[tuple[str, str]]``", diff --git a/tests/core/test_initialization.py b/tests/core/test_initialization.py index bac6dd423..fd57620b8 100644 --- a/tests/core/test_initialization.py +++ b/tests/core/test_initialization.py @@ -420,6 +420,25 @@ def test_statsd_counts_endpoints(self): unique=[("method", "get"), ("endpoint", "/__heartbeat__"), ("status", "200")], ) + def test_statsd_sanitizes_url_in_metrics(self): + kinto.core.initialize(self.config, "0.0.1", "settings_prefix") + app = webtest.TestApp(self.config.make_wsgi_app()) + app.get( + "/v0/changeset'%7C%22%3F%3E%3C!DOCTYPE%22http://xh3E'),'/l')%20from%20dual)%7C'", + status=404, + ) + self.mocked().count.assert_any_call( + "request_summary", + unique=[ + ("method", "get"), + ( + "endpoint", + "/changeset%27%257C%2522%253F%253E%253C%21DOCTYPE%2522http%3A//xh3E%27%29%2C%27/l%27%29%2520from%2520dual%29%257C%27", + ), + ("status", "404"), + ], + ) + def test_statsd_observe_request_size(self): kinto.core.initialize(self.config, "0.0.1", "settings_prefix") app = webtest.TestApp(self.config.make_wsgi_app()) diff --git a/tests/plugins/test_statsd.py b/tests/plugins/test_statsd.py index e40af66b3..d801eea86 100644 --- a/tests/plugins/test_statsd.py +++ b/tests/plugins/test_statsd.py @@ -61,6 +61,11 @@ def test_count_turns_multiple_tuples_into_one_set_key(self): self.client.count("click", unique=[("component", "menu"), ("sound", "off")]) mocked_client.set.assert_called_with("click", "component.menu.sound.off") + def test_values_are_sanitized(self): + with mock.patch.object(self.client, "_client") as mocked_client: + self.client.count("click", unique=[("user", "account:boss")]) + mocked_client.set.assert_called_with("click", "user.accountboss") + @mock.patch("kinto.plugins.statsd.statsd_module") def test_load_from_config(self, module_mock): config = testing.setUp()