diff --git a/kinto/core/initialization.py b/kinto/core/initialization.py index 643551681..0351b88fc 100644 --- a/kinto/core/initialization.py +++ b/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 diff --git a/kinto/core/metrics.py b/kinto/core/metrics.py index f856cea38..6eb477734 100644 --- a/kinto/core/metrics.py +++ b/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): diff --git a/kinto/core/utils.py b/kinto/core/utils.py index 00396e4ba..b2e3c0680 100644 --- a/kinto/core/utils.py +++ b/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) diff --git a/kinto/plugins/prometheus.py b/kinto/plugins/prometheus.py index bd3c01354..404588235 100644 --- a/kinto/plugins/prometheus.py +++ b/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 diff --git a/tests/core/resource/test_views.py b/tests/core/resource/test_views.py index b6016b53a..ea934128c 100644 --- a/tests/core/resource/test_views.py +++ b/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) diff --git a/tests/core/test_initialization.py b/tests/core/test_initialization.py index 3801ae919..a2ebc0021 100644 --- a/tests/core/test_initialization.py +++ b/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",