Improve metrics API to support multiple key/value labels

kinto/core/initialization.py

@@ -478,8 +478,8 @@ def on_new_response(event):
         user_id = request.prefixed_userid
         if user_id:
             # Get rid of colons in metric packet (see #1282).
-            user_id = user_id.replace(":", ".")
-            metrics_service.count("users", unique=user_id)
+            auth, user_id = user_id.split(":")
+            metrics_service.count("users", unique=[("auth", auth), ("userid", user_id)])
 
         # Count authentication verifications.
         if hasattr(request, "authn_type"):

kinto/core/metrics.py

@@ -20,6 +20,8 @@ def count(key, count=1, unique=None):
         """
         Count occurrences. If `unique` is set, overwrites the counter value
         on each call.
+
+        `unique` should be of type ``list[tuple[str,str]]``.
         """
 
 

kinto/plugins/prometheus.py

@@ -1,4 +1,5 @@
 import functools
+import warnings
 from time import perf_counter as time_now
 
 from pyramid.exceptions import ConfigurationError
@@ -91,22 +92,31 @@ def timer(self, key):
     def count(self, key, count=1, unique=None):
         global _METRICS
 
-        # Turn `unique` into a group and a value:
-        # eg. `method.basicauth.mat` -> `method_basicauth="mat"`
-        label_value = None
+        labels = []
+
         if unique:
-            if "." not in unique:
-                unique = f"group.{unique}"
-            label_name, label_value = unique.rsplit(".", 1)
-            label_names = (_fix_metric_name(label_name),)
-        else:
-            label_names = tuple()
+            if isinstance(unique, str):
+                warnings.warn(
+                    "`unique` parameter should be of type ``list[tuple[str, str]]``",
+                    DeprecationWarning,
+                )
+                # Turn `unique` into a group and a value:
+                # "bob" -> "group.bob"
+                # "method.basicauth.mat" -> [("method_basicauth", "mat")]`
+                if "." not in unique:
+                    unique = f"group.{unique}"
+                label_name, label_value = unique.rsplit(".", 1)
+                unique = [(label_name, label_value)]
+
+            labels = [
+                (_fix_metric_name(label_name), label_value) for label_name, label_value in unique
+            ]
 
         if key not in _METRICS:
             _METRICS[key] = prometheus_module.Counter(
                 _fix_metric_name(key),
                 f"Counter of {key}",
-                labelnames=label_names,
+                labelnames=[label_name for label_name, _ in labels],
                 registry=get_registry(),
             )
 
@@ -116,8 +126,8 @@ def count(self, key, count=1, unique=None):
             )
 
         m = _METRICS[key]
-        if label_value is not None:
-            m = m.labels(label_value)
+        if labels:
+            m = m.labels(*(label_value for _, label_value in labels))
 
         m.inc(count)
 

kinto/plugins/statsd.py

@@ -1,3 +1,4 @@
+import warnings
 from urllib.parse import urlparse
 
 from pyramid.exceptions import ConfigurationError
@@ -23,8 +24,16 @@ def timer(self, key):
     def count(self, key, count=1, unique=None):
         if unique is None:
             return self._client.incr(key, count=count)
+        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)
         else:
-            return self._client.set(key, unique)
+            warnings.warn(
+                "`unique` parameter should be of type ``list[tuple[str, str]]``",
+                DeprecationWarning,
+            )
+        return self._client.set(key, unique)
 
 
 def load_from_config(config):

tests/core/test_initialization.py

@@ -384,7 +384,9 @@ def test_statsd_counts_unique_users(self, digest_mocked):
         kinto.core.initialize(self.config, "0.0.1", "settings_prefix")
         app = webtest.TestApp(self.config.make_wsgi_app())
         app.get("/v0/", headers=get_user_headers("bob"))
-        self.mocked().count.assert_any_call("users", unique="basicauth.mat")
+        self.mocked().count.assert_any_call(
+            "users", unique=[("auth", "basicauth"), ("userid", "mat")]
+        )
 
     def test_statsd_counts_authentication_types(self):
         kinto.core.initialize(self.config, "0.0.1", "settings_prefix")

tests/plugins/test_prometheus.py

@@ -91,19 +91,13 @@ def test_count_by_key_value(self):
         self.assertIn("bigstep_total 2.0", resp.text)
 
     def test_count_by_key_grouped(self):
-        self.app.app.registry.metrics.count("http", unique="status.500")
-        self.app.app.registry.metrics.count("http", unique="status.200")
+        self.app.app.registry.metrics.count("http", unique=[("status", "500")])
+        self.app.app.registry.metrics.count("http", unique=[("status", "200")])
 
         resp = self.app.get("/__metrics__")
         self.assertIn('http_total{status="500"} 1.0', resp.text)
         self.assertIn('http_total{status="200"} 1.0', resp.text)
 
-    def test_count_with_generic_group(self):
-        self.app.app.registry.metrics.count("mushrooms", unique="boletus")
-
-        resp = self.app.get("/__metrics__")
-        self.assertIn('mushrooms_total{group="boletus"} 1.0', resp.text)
-
     def test_metrics_cant_be_mixed(self):
         self.app.app.registry.metrics.count("counter")
         with self.assertRaises(RuntimeError):
@@ -114,7 +108,19 @@ def test_metrics_cant_be_mixed(self):
             self.app.app.registry.metrics.count("timer")
 
     def test_metrics_names_and_labels_are_transformed(self):
-        self.app.app.registry.metrics.count("http.home.status", unique="code.get.200")
+        self.app.app.registry.metrics.count("http.home.status", unique=[("code.get", "200")])
 
         resp = self.app.get("/__metrics__")
         self.assertIn('http_home_status_total{code_get="200"} 1.0', resp.text)
+
+    def test_count_with_legacy_string_generic_group(self):
+        self.app.app.registry.metrics.count("champignons", unique="boletus")
+
+        resp = self.app.get("/__metrics__")
+        self.assertIn('champignons_total{group="boletus"} 1.0', resp.text)
+
+    def test_count_with_legacy_string_basic_group(self):
+        self.app.app.registry.metrics.count("mushrooms", unique="species.boletus")
+
+        resp = self.app.get("/__metrics__")
+        self.assertIn('mushrooms_total{species="boletus"} 1.0', resp.text)

tests/plugins/test_statsd.py

@@ -46,6 +46,16 @@ def test_count_with_unique_uses_sets_for_key(self):
             self.client.count("click", unique="menu")
             mocked_client.set.assert_called_with("click", "menu")
 
+    def test_count_turns_tuples_into_set_key(self):
+        with mock.patch.object(self.client, "_client") as mocked_client:
+            self.client.count("click", unique=[("component", "menu")])
+            mocked_client.set.assert_called_with("click", "component.menu")
+
+    def test_count_turns_multiple_tuples_into_one_set_key(self):
+        with mock.patch.object(self.client, "_client") as mocked_client:
+            self.client.count("click", unique=[("component", "menu"), ("sound", "off")])
+            mocked_client.set.assert_called_with("click", "component.menu.sound.off")
+
     @mock.patch("kinto.plugins.statsd.statsd_module")
     def test_load_from_config(self, module_mock):
         config = testing.setUp()