Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Registry a no-op metrics service by default #3460

Merged
merged 4 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions kinto/core/initialization.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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")
Expand All @@ -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
Expand Down
28 changes: 27 additions & 1 deletion kinto/core/metrics.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import types

from zope.interface import Interface
from zope.interface import Interface, implementer

from kinto.core import utils

Expand All @@ -23,6 +23,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.
Expand All @@ -49,6 +73,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):
Expand Down
8 changes: 8 additions & 0 deletions kinto/core/utils.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import collections.abc as collections_abc
import functools
import hashlib
import hmac
import os
Expand Down Expand Up @@ -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)
9 changes: 1 addition & 8 deletions kinto/plugins/prometheus.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import functools
from time import perf_counter as time_now

from pyramid.exceptions import ConfigurationError
from pyramid.response import Response
from zope.interface import implementer

from kinto.core import metrics
from kinto.core.utils import safe_wraps


try:
Expand All @@ -30,13 +30,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
Expand Down
9 changes: 4 additions & 5 deletions tests/core/resource/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
33 changes: 21 additions & 12 deletions tests/core/test_initialization.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -414,13 +421,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"})
Expand All @@ -444,18 +450,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):
Expand All @@ -468,7 +475,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()
Expand Down Expand Up @@ -532,6 +539,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",
Expand Down
Loading