Registry a no-op metrics service by default (#3460)

* Registry a no-op metrics service by default

* Fix tests where registry did not have the backends

* registry.statsd is no-op by default

kinto/core/initialization.py

@@ -431,6 +431,9 @@ def on_new_response(event):
 def setup_metrics(config):
     settings = config.get_settings()
 
+    # Register a no-op metrics service by default.
+    config.registry.registerUtility(metrics.NoOpMetricsService(), metrics.IMetricsService)
+
     # This does not fully respect the Pyramid/ZCA patterns, but the rest of Kinto uses
     # `registry.storage`, `registry.cache`, etc. Consistency seems more important.
     config.registry.__class__.metrics = property(
@@ -449,9 +452,6 @@ def deprecated_registry(self):
     def on_app_created(event):
         config = event.app
         metrics_service = config.registry.metrics
-        if not metrics_service:
-            logger.warning("No metrics service registered.")
-            return
 
         metrics.watch_execution_time(metrics_service, config.registry.cache, prefix="backend")
         metrics.watch_execution_time(metrics_service, config.registry.storage, prefix="backend")
@@ -471,8 +471,6 @@ def on_app_created(event):
     def on_new_response(event):
         request = event.request
         metrics_service = config.registry.metrics
-        if not metrics_service:
-            return
 
         # Count unique users.
         user_id = request.prefixed_userid

kinto/core/metrics.py

@@ -1,6 +1,6 @@
 import types
 
-from zope.interface import Interface
+from zope.interface import Interface, implementer
 
 from kinto.core import utils
 
@@ -25,6 +25,30 @@ def count(key, count=1, unique=None):
         """
 
 
+class NoOpTimer:
+    def __call__(self, f):
+        @utils.safe_wraps(f)
+        def _wrapped(*args, **kwargs):
+            return f(*args, **kwargs)
+
+        return _wrapped
+
+    def __enter__(self):
+        pass
+
+    def __exit__(self, *args, **kwargs):
+        pass
+
+
+@implementer(IMetricsService)
+class NoOpMetricsService:
+    def timer(self, key):
+        return NoOpTimer()
+
+    def count(self, key, count=1, unique=None):
+        pass
+
+
 def watch_execution_time(metrics_service, obj, prefix="", classname=None):
     """
     Decorate all methods of an object in order to watch their execution time.
@@ -51,6 +75,8 @@ def listener_with_timer(config, key, func):
     def wrapped(*args, **kwargs):
         metrics_service = config.registry.metrics
         if not metrics_service:
+            # This only happens if `kinto.core.initialization.setup_metrics` is
+            # not listed in the `initialization_sequence` setting.
             return func(*args, **kwargs)
         # If metrics are enabled, monitor execution time of listeners.
         with metrics_service.timer(key):

kinto/core/utils.py

@@ -1,4 +1,5 @@
 import collections.abc as collections_abc
+import functools
 import hashlib
 import hmac
 import os
@@ -541,3 +542,10 @@ def apply_json_patch(obj, ops):
         raise ValueError(e)
 
     return result
+
+
+def safe_wraps(wrapper, *args, **kwargs):
+    """Safely wraps partial functions."""
+    while isinstance(wrapper, functools.partial):
+        wrapper = wrapper.func
+    return functools.wraps(wrapper, *args, **kwargs)

kinto/plugins/prometheus.py

@@ -1,4 +1,3 @@
-import functools
 import warnings
 from time import perf_counter as time_now
 
@@ -7,6 +6,7 @@
 from zope.interface import implementer
 
 from kinto.core import metrics
+from kinto.core.utils import safe_wraps
 
 
 try:
@@ -31,13 +31,6 @@ def _fix_metric_name(s):
     return s.replace("-", "_").replace(".", "_")
 
 
-def safe_wraps(wrapper, *args, **kwargs):
-    """Safely wraps partial functions."""
-    while isinstance(wrapper, functools.partial):
-        wrapper = wrapper.func
-    return functools.wraps(wrapper, *args, **kwargs)
-
-
 class Timer:
     def __init__(self, summary):
         self.summary = summary

tests/core/resource/test_views.py

@@ -613,19 +613,18 @@ class StorageErrorTest(BaseWebTest, unittest.TestCase):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.error = storage_exceptions.BackendError(ValueError())
-        self.storage_error_patcher = mock.patch(
-            "kinto.core.storage.memory.Storage.create", side_effect=self.error
-        )
 
     def test_backend_errors_are_served_as_503(self):
         body = {"data": MINIMALIST_OBJECT}
-        with self.storage_error_patcher:
+        with mock.patch.object(self.app.app.registry.storage, "create", side_effect=self.error):
             self.app.post_json(self.plural_url, body, headers=self.headers, status=503)
 
     def test_backend_errors_original_error_is_logged(self):
         body = {"data": MINIMALIST_OBJECT}
         with mock.patch("kinto.core.views.errors.logger.critical") as mocked:
-            with self.storage_error_patcher:
+            with mock.patch.object(
+                self.app.app.registry.storage, "create", side_effect=self.error
+            ):
                 self.app.post_json(self.plural_url, body, headers=self.headers, status=503)
                 self.assertTrue(mocked.called)
                 self.assertEqual(type(mocked.call_args[0][0]), ValueError)

tests/core/test_initialization.py

@@ -269,7 +269,14 @@ def test_sentry_enabled_if_sentry_dsn_is_set(self):
 
     @unittest.skipIf(initialization.sentry_sdk is None, "sentry is not installed.")
     def test_message_is_sent_on_startup(self):
-        config = Configurator(settings={**kinto.core.DEFAULT_SETTINGS})
+        config = Configurator(
+            settings={
+                **kinto.core.DEFAULT_SETTINGS,
+                "storage_backend": "kinto.core.storage.memory",
+                "cache_backend": "kinto.core.cache.memory",
+                "permission_backend": "kinto.core.permission.memory",
+            }
+        )
         config.add_settings(
             {
                 "sentry_dsn": "https://notempty",
@@ -416,13 +423,12 @@ def test_statsd_counts_unknown_urls(self):
         app.get("/v0/coucou", status=404)
         self.assertFalse(self.mocked.count.called)
 
-    def test_metrics_and_statsd_are_none_if_statsd_url_not_set(self):
+    def test_statsd_is_noop_service_if_statsd_url_not_set(self):
         self.config.add_settings({"statsd_url": None})
 
         initialization.setup_metrics(self.config)
 
-        self.assertIsNone(self.config.registry.statsd)
-        self.assertIsNone(self.config.registry.metrics)
+        self.assertEqual(self.config.registry.statsd.__class__.__name__, "NoOpMetricsService")
 
     def test_metrics_attr_is_set_if_statsd_url_is_set(self):
         self.config.add_settings({"statsd_url": "udp://host:8080"})
@@ -446,18 +452,19 @@ def test_statsd_attr_is_exposed_in_the_registry_if_url_is_set(self):
 
 
 class RequestsConfigurationTest(unittest.TestCase):
+    app_settings = {
+        "storage_backend": "kinto.core.storage.memory",
+        "cache_backend": "kinto.core.cache.memory",
+        "permission_backend": "kinto.core.permission.memory",
+    }
+
     def _get_app(self, settings={}):
-        app_settings = {
-            "storage_backend": "kinto.core.storage.memory",
-            "cache_backend": "kinto.core.cache.memory",
-        }
-        app_settings.update(**settings)
-        config = Configurator(settings=app_settings)
+        config = Configurator(settings={**self.app_settings, **settings})
         kinto.core.initialize(config, "0.0.1", "name")
         return webtest.TestApp(config.make_wsgi_app())
 
     def test_requests_have_a_bound_data_attribute(self):
-        config = Configurator()
+        config = Configurator(settings={**self.app_settings})
         kinto.core.initialize(config, "0.0.1", "name")
 
         def on_new_request(event):
@@ -470,7 +477,7 @@ def on_new_request(event):
         app.get("/v0/")
 
     def test_subrequests_share_parent_bound_data(self):
-        config = Configurator()
+        config = Configurator(settings={**self.app_settings})
         kinto.core.initialize(config, "0.0.1", "name")
 
         bound_datas = set()
@@ -534,6 +541,8 @@ def make_app(self, settings=None):
         config = Configurator(settings={**kinto.core.DEFAULT_SETTINGS})
         config.add_settings(
             {
+                "storage_backend": "kinto.core.storage.memory",
+                "cache_backend": "kinto.core.cache.memory",
                 "permission_backend": "kinto.core.permission.memory",
                 "includes": "tests.core.testplugin",
                 "multiauth.policies": "basicauth",