diff --git a/docs/configuration/settings.rst b/docs/configuration/settings.rst index 786b3929b..f23196ff7 100644 --- a/docs/configuration/settings.rst +++ b/docs/configuration/settings.rst @@ -304,18 +304,6 @@ hello view `. Logging and Monitoring ====================== -+------------------------+----------------------------------------+--------------------------------------------------------------------------+ -| Setting name | Default | What does it do? | -+========================+========================================+==========================================================================+ -| kinto.statsd_backend | ``kinto.core.statsd`` | The Python **dotted** location of the StatsD module that should be used | -| | | for monitoring. Useful to plug custom implementations like Datadog™. | -+------------------------+----------------------------------------+--------------------------------------------------------------------------+ -| kinto.statsd_prefix | ``kinto`` | The prefix to use when sending data to statsd. | -+------------------------+----------------------------------------+--------------------------------------------------------------------------+ -| kinto.statsd_url | ``None`` | The fully qualified URL to use to connect to the statsd host. e.g. | -| | | ``udp://localhost:8125`` | -+------------------------+----------------------------------------+--------------------------------------------------------------------------+ - Standard Logging :::::::::::::::: @@ -425,19 +413,47 @@ Or the equivalent environment variables: The application sends an event on startup (mainly for setup check). +.. _monitoring-with-statsd: + Monitoring with StatsD :::::::::::::::::::::: Requires the ``statsd`` package. -StatsD metrics can be enabled (disabled by default): ++------------------------+----------------------------------------+--------------------------------------------------------------------------+ +| Setting name | Default | What does it do? | ++========================+========================================+==========================================================================+ +| kinto.statsd_backend | ``kinto.core.statsd`` | The Python **dotted** location of the StatsD module that should be used | +| | | for monitoring. Useful to plug custom implementations like Datadog™. | ++------------------------+----------------------------------------+--------------------------------------------------------------------------+ +| kinto.statsd_prefix | ``kinto`` | The prefix to use when sending data to statsd. | ++------------------------+----------------------------------------+--------------------------------------------------------------------------+ +| kinto.statsd_url | ``None`` | The fully qualified URL to use to connect to the statsd host. e.g. | +| | | ``udp://host:8125`` | ++------------------------+----------------------------------------+--------------------------------------------------------------------------+ + + +StatsD metrics can be enabled with (disabled by default): .. code-block:: ini - kinto.statsd_url = udp://localhost:8125 + kinto.statsd_url = udp://host:8125 # kinto.statsd_prefix = kinto-prod +StatsD can also be enabled at the *uWSGI* level: + +.. code-block:: ini + + [uwsgi] + + # ... + + enable-metrics = true + plugin = dogstatsd + stats-push = dogstatsd:host:8125,kinto.{{ $deployment }} + + Monitoring with New Relic ::::::::::::::::::::::::: @@ -501,6 +517,9 @@ list of Python modules: | ``kinto.plugins.quotas`` | It allows to limit storage per collection size, number of records, etc. | | | (:ref:`more details `). | +---------------------------------------+--------------------------------------------------------------------------+ +| ``kinto.plugins.statsd`` | Send metrics about backend duration, authentication, endpoints hits, .. | +| | (:ref:`more details `). | ++---------------------------------------+--------------------------------------------------------------------------+ There are `many available packages`_ in Pyramid ecosystem, and it is straightforward to build one, diff --git a/kinto/core/__init__.py b/kinto/core/__init__.py index 464abe1f2..0665b58a3 100644 --- a/kinto/core/__init__.py +++ b/kinto/core/__init__.py @@ -66,8 +66,8 @@ "kinto.core.initialization.setup_authentication", "kinto.core.initialization.setup_backoff", "kinto.core.initialization.setup_sentry", - "kinto.core.initialization.setup_statsd", "kinto.core.initialization.setup_listeners", + "kinto.core.initialization.setup_metrics", "kinto.core.events.setup_transaction_hook", ), "event_listeners": "", diff --git a/kinto/core/initialization.py b/kinto/core/initialization.py index 561ebfc1d..023e939a3 100644 --- a/kinto/core/initialization.py +++ b/kinto/core/initialization.py @@ -21,7 +21,7 @@ from pyramid.settings import asbool, aslist from pyramid_multiauth import MultiAuthenticationPolicy, MultiAuthPolicySelected -from kinto.core import cache, errors, permission, storage, utils +from kinto.core import cache, errors, metrics, permission, storage, utils from kinto.core.events import ACTIONS, ResourceChanged, ResourceRead @@ -334,51 +334,13 @@ def on_app_created(event): def setup_statsd(config): - settings = config.get_settings() - config.registry.statsd = None - - if settings["statsd_url"]: - statsd_mod = settings["statsd_backend"] - statsd_mod = config.maybe_dotted(statsd_mod) - client = statsd_mod.load_from_config(config) - - config.registry.statsd = client - - client.watch_execution_time(config.registry.cache, prefix="backend") - client.watch_execution_time(config.registry.storage, prefix="backend") - client.watch_execution_time(config.registry.permission, prefix="backend") - - # Commit so that configured policy can be queried. - config.commit() - policy = config.registry.queryUtility(IAuthenticationPolicy) - if isinstance(policy, MultiAuthenticationPolicy): - for name, subpolicy in policy.get_policies(): - client.watch_execution_time(subpolicy, prefix="authentication", classname=name) - else: - client.watch_execution_time(policy, prefix="authentication") - - def on_new_response(event): - request = event.request - - # Count unique users. - user_id = request.prefixed_userid - if user_id: - # Get rid of colons in metric packet (see #1282). - user_id = user_id.replace(":", ".") - client.count("users", unique=user_id) - - # Count authentication verifications. - if hasattr(request, "authn_type"): - client.count(f"authn_type.{request.authn_type}") - - # Count view calls. - service = request.current_service - if service: - client.count(f"view.{service.name}.{request.method}") - - config.add_subscriber(on_new_response, NewResponse) - - return client + # It would be pretty rare to find users that have a custom ``kinto.initialization_sequence`` setting. + # But just in case, warn that it will be removed in next major. + warnings.warn( + "``setup_statsd()`` is now deprecated. Use ``kinto.core.initialization.setup_metrics()`` instead.", + DeprecationWarning, + ) + setup_metrics(config) def install_middlewares(app, settings): @@ -466,6 +428,75 @@ def on_new_response(event): config.add_subscriber(on_new_response, NewResponse) +def setup_metrics(config): + settings = config.get_settings() + + # 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( + lambda reg: reg.queryUtility(metrics.IMetricsService) + ) + + def deprecated_registry(self): + warnings.warn( + "``config.registry.statsd`` is now deprecated. Use ``config.registry.metrics`` instead.", + DeprecationWarning, + ) + return self.metrics + + config.registry.__class__.statsd = property(deprecated_registry) + + 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") + metrics.watch_execution_time(metrics_service, config.registry.permission, prefix="backend") + + policy = config.registry.queryUtility(IAuthenticationPolicy) + if isinstance(policy, MultiAuthenticationPolicy): + for name, subpolicy in policy.get_policies(): + metrics.watch_execution_time( + metrics_service, subpolicy, prefix="authentication", classname=name + ) + else: + metrics.watch_execution_time(metrics_service, policy, prefix="authentication") + + config.add_subscriber(on_app_created, ApplicationCreated) + + 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 + if user_id: + # Get rid of colons in metric packet (see #1282). + user_id = user_id.replace(":", ".") + metrics_service.count("users", unique=user_id) + + # Count authentication verifications. + if hasattr(request, "authn_type"): + metrics_service.count(f"authn_type.{request.authn_type}") + + # Count view calls. + service = request.current_service + if service: + metrics_service.count(f"view.{service.name}.{request.method}") + + config.add_subscriber(on_new_response, NewResponse) + + # While statsd is deprecated, we include its plugin by default for retro-compability. + if settings["statsd_url"]: + config.include("kinto.plugins.statsd") + + class EventActionFilter: def __init__(self, actions, config): actions = ACTIONS.from_string_list(actions) @@ -518,11 +549,9 @@ def setup_listeners(config): listener_mod = config.maybe_dotted(module_value) listener = listener_mod.load_from_config(config, prefix) - # If StatsD is enabled, monitor execution time of listeners. - if getattr(config.registry, "statsd", None): - statsd_client = config.registry.statsd - key = f"listeners.{name}" - listener = statsd_client.timer(key)(listener.__call__) + wrapped_listener = metrics.listener_with_timer( + config, f"listeners.{name}", listener.__call__ + ) # Optional filter by event action. actions_setting = prefix + "actions" @@ -548,11 +577,11 @@ def setup_listeners(config): options = dict(for_actions=actions, for_resources=resource_names) if ACTIONS.READ in actions: - config.add_subscriber(listener, ResourceRead, **options) + config.add_subscriber(wrapped_listener, ResourceRead, **options) actions = [a for a in actions if a != ACTIONS.READ] if len(actions) > 0: - config.add_subscriber(listener, ResourceChanged, **options) + config.add_subscriber(wrapped_listener, ResourceChanged, **options) def load_default_settings(config, default_settings): diff --git a/kinto/core/metrics.py b/kinto/core/metrics.py new file mode 100644 index 000000000..dbb02128a --- /dev/null +++ b/kinto/core/metrics.py @@ -0,0 +1,57 @@ +import types + +from zope.interface import Interface + +from kinto.core import utils + + +class IMetricsService(Interface): + """ + An interface that defines the metrics service contract. + Any class implementing this must provide all its methods. + """ + + def timer(key): + """ + Watch execution time. + """ + + def count(key, count=1, unique=None): + """ + Count occurrences. If `unique` is set, overwrites the counter value + on each call. + """ + + +def watch_execution_time(metrics_service, obj, prefix="", classname=None): + """ + Decorate all methods of an object in order to watch their execution time. + Metrics will be named `{prefix}.{classname}.{method}`. + """ + classname = classname or utils.classname(obj) + members = dir(obj) + for name in members: + value = getattr(obj, name) + is_method = isinstance(value, types.MethodType) + if not name.startswith("_") and is_method: + statsd_key = f"{prefix}.{classname}.{name}" + decorated_method = metrics_service.timer(statsd_key)(value) + setattr(obj, name, decorated_method) + + +def listener_with_timer(config, key, func): + """ + Add a timer with the specified `key` on the specified `func`. + This is used to avoid evaluating `config.registry.metrics` during setup time + to avoid having to deal with initialization order and configuration committing. + """ + + def wrapped(*args, **kwargs): + metrics_service = config.registry.metrics + if not metrics_service: + return func(*args, **kwargs) + # If metrics are enabled, monitor execution time of listeners. + with metrics_service.timer(key): + return func(*args, **kwargs) + + return wrapped diff --git a/kinto/core/statsd.py b/kinto/core/statsd.py index aea62ecd8..2c6b1ecad 100644 --- a/kinto/core/statsd.py +++ b/kinto/core/statsd.py @@ -1,63 +1 @@ -import types -from urllib.parse import urlparse - -from pyramid.exceptions import ConfigurationError - -from kinto.core import utils - - -try: - import statsd as statsd_module -except ImportError: # pragma: no cover - statsd_module = None - - -class Client: - def __init__(self, host, port, prefix): - self._client = statsd_module.StatsClient(host, port, prefix=prefix) - - def watch_execution_time(self, obj, prefix="", classname=None): - classname = classname or utils.classname(obj) - members = dir(obj) - for name in members: - value = getattr(obj, name) - is_method = isinstance(value, types.MethodType) - if not name.startswith("_") and is_method: - statsd_key = f"{prefix}.{classname}.{name}" - decorated_method = self.timer(statsd_key)(value) - setattr(obj, name, decorated_method) - - def timer(self, key): - return self._client.timer(key) - - def count(self, key, count=1, unique=None): - if unique is None: - return self._client.incr(key, count=count) - else: - return self._client.set(key, unique) - - -def statsd_count(request, count_key): - statsd = request.registry.statsd - if statsd: - statsd.count(count_key) - - -def load_from_config(config): - # If this is called, it means that a ``statsd_url`` was specified in settings. - # (see ``kinto.core.initialization``) - # Raise a proper error if the ``statsd`` module is not installed. - if statsd_module is None: - error_msg = "Please install Kinto with monitoring dependencies (e.g. statsd package)" - raise ConfigurationError(error_msg) - - settings = config.get_settings() - uri = settings["statsd_url"] - uri = urlparse(uri) - - if settings["project_name"] != "": - prefix = settings["project_name"] - else: - prefix = settings["statsd_prefix"] - - return Client(uri.hostname, uri.port, prefix) +from kinto.plugins.statsd import load_from_config # noqa: F401 diff --git a/kinto/core/testing.py b/kinto/core/testing.py index 01225ef36..9a779fb08 100644 --- a/kinto/core/testing.py +++ b/kinto/core/testing.py @@ -8,9 +8,10 @@ from cornice import errors as cornice_errors from pyramid.url import parse_url_overrides -from kinto.core import DEFAULT_SETTINGS, statsd +from kinto.core import DEFAULT_SETTINGS from kinto.core.storage import generators from kinto.core.utils import encode64, follow_subrequest, memcache, sqlalchemy +from kinto.plugins import statsd skip_if_ci = unittest.skipIf("CI" in os.environ, "ci") diff --git a/kinto/plugins/history/__init__.py b/kinto/plugins/history/__init__.py index 7551ea127..dbb6fc7e7 100644 --- a/kinto/plugins/history/__init__.py +++ b/kinto/plugins/history/__init__.py @@ -1,4 +1,5 @@ from kinto.authorization import PERMISSIONS_INHERITANCE_TREE +from kinto.core import metrics from kinto.core.events import ResourceChanged from .listener import on_resource_changed @@ -14,15 +15,13 @@ def includeme(config): # Activate end-points. config.scan("kinto.plugins.history.views") - # If StatsD is enabled, monitor execution time of listener. - listener = on_resource_changed - if config.registry.statsd: - key = "plugins.history" - listener = config.registry.statsd.timer(key)(on_resource_changed) + wrapped_listener = metrics.listener_with_timer(config, "plugins.history", on_resource_changed) # Listen to every resources (except history) config.add_subscriber( - listener, ResourceChanged, for_resources=("bucket", "group", "collection", "record") + wrapped_listener, + ResourceChanged, + for_resources=("bucket", "group", "collection", "record"), ) # Register the permission inheritance for history entries. diff --git a/kinto/plugins/quotas/__init__.py b/kinto/plugins/quotas/__init__.py index 144fe151b..119e9394a 100644 --- a/kinto/plugins/quotas/__init__.py +++ b/kinto/plugins/quotas/__init__.py @@ -1,3 +1,4 @@ +from kinto.core import metrics from kinto.core.events import ResourceChanged from .listener import on_resource_changed @@ -10,13 +11,11 @@ def includeme(config): url="https://kinto.readthedocs.io", ) - # If StatsD is enabled, monitor execution time of listener. - listener = on_resource_changed - if config.registry.statsd: - key = "plugins.quotas" - listener = config.registry.statsd.timer(key)(on_resource_changed) + wrapped_listener = metrics.listener_with_timer(config, "plugins.quotas", on_resource_changed) # Listen to every resources (except history) config.add_subscriber( - listener, ResourceChanged, for_resources=("bucket", "group", "collection", "record") + wrapped_listener, + ResourceChanged, + for_resources=("bucket", "group", "collection", "record"), ) diff --git a/kinto/plugins/statsd.py b/kinto/plugins/statsd.py new file mode 100644 index 000000000..4cc77165f --- /dev/null +++ b/kinto/plugins/statsd.py @@ -0,0 +1,58 @@ +from urllib.parse import urlparse + +from pyramid.exceptions import ConfigurationError +from zope.interface import implementer + +from kinto.core import metrics + + +try: + import statsd as statsd_module +except ImportError: # pragma: no cover + statsd_module = None + + +@implementer(metrics.IMetricsService) +class StatsDService: + def __init__(self, host, port, prefix): + self._client = statsd_module.StatsClient(host, port, prefix=prefix) + + def timer(self, key): + return self._client.timer(key) + + def count(self, key, count=1, unique=None): + if unique is None: + return self._client.incr(key, count=count) + else: + return self._client.set(key, unique) + + +def load_from_config(config): + # If this is called, it means that a ``statsd_url`` was specified in settings. + # (see ``kinto.core.initialization``) + # Raise a proper error if the ``statsd`` module is not installed. + if statsd_module is None: + error_msg = "Please install Kinto with monitoring dependencies (e.g. statsd package)" + raise ConfigurationError(error_msg) + + settings = config.get_settings() + uri = settings["statsd_url"] + uri = urlparse(uri) + + if settings["project_name"] != "": + prefix = settings["project_name"] + else: + prefix = settings["statsd_prefix"] + + return StatsDService(uri.hostname, uri.port, prefix) + + +def includeme(config): + settings = config.get_settings() + + # TODO: this backend abstraction may not be required anymore. + statsd_mod = settings["statsd_backend"] + statsd_mod = config.maybe_dotted(statsd_mod) + client = statsd_mod.load_from_config(config) + + config.registry.registerUtility(client, metrics.IMetricsService) diff --git a/tests/core/resource/test_events.py b/tests/core/resource/test_events.py index cbb9c8b66..6de66c201 100644 --- a/tests/core/resource/test_events.py +++ b/tests/core/resource/test_events.py @@ -4,7 +4,6 @@ from pyramid.config import Configurator -from kinto.core import statsd from kinto.core.events import ( ACTIONS, AfterResourceChanged, @@ -14,7 +13,8 @@ notify_resource_event, ) from kinto.core.storage.exceptions import BackendError -from kinto.core.testing import unittest +from kinto.core.testing import skip_if_no_statsd, unittest +from kinto.plugins import statsd from ..support import BaseWebTest @@ -491,7 +491,7 @@ def __call__(self, event): return ClassListener() -@unittest.skipIf(not statsd.statsd_module, "statsd is not installed.") +@skip_if_no_statsd class StatsDTest(BaseWebTest, unittest.TestCase): @classmethod def get_app_settings(cls, *args, **kwargs): @@ -506,8 +506,9 @@ def get_app_settings(cls, *args, **kwargs): return settings def test_statds_tracks_listeners_execution_duration(self): - statsd_client = self.app.app.registry.statsd._client - with mock.patch.object(statsd_client, "timing") as mocked: + # This test may break when introducing a generic interface for Prometheus. + metrics_client = self.app.app.registry.metrics._client + with mock.patch.object(metrics_client, "timing") as mocked: self.app.post_json(self.plural_url, {"data": {"name": "pouet"}}, headers=self.headers) timers = set(c[0][0] for c in mocked.call_args_list) self.assertIn("listeners.test", timers) diff --git a/tests/core/test_initialization.py b/tests/core/test_initialization.py index caa072507..e999d3628 100644 --- a/tests/core/test_initialization.py +++ b/tests/core/test_initialization.py @@ -2,13 +2,14 @@ from unittest import mock import webtest +from pyramid.authentication import SessionAuthenticationPolicy from pyramid.config import Configurator from pyramid.events import NewRequest from pyramid.exceptions import ConfigurationError import kinto.core from kinto.core import initialization -from kinto.core.testing import unittest +from kinto.core.testing import get_user_headers, skip_if_no_statsd, unittest class InitializationTest(unittest.TestCase): @@ -292,56 +293,108 @@ def unexpected_exceptions_are_reported(self): assert len(mocked.call_args_list) > 0 -class StatsDConfigurationTest(unittest.TestCase): +@skip_if_no_statsd +class MetricsConfigurationTest(unittest.TestCase): + settings = { + **kinto.core.DEFAULT_SETTINGS, + "statsd_url": "udp://host:8080", + "multiauth.policies": "basicauth", + } + def setUp(self): - settings = { - **kinto.core.DEFAULT_SETTINGS, - "statsd_url": "udp://host:8080", - "multiauth.policies": "basicauth", - } - self.config = Configurator(settings=settings) + patch = mock.patch("kinto.plugins.statsd.StatsDService") + self.mocked = patch.start() + self.addCleanup(patch.stop) + + patch_watch = mock.patch("kinto.core.metrics.watch_execution_time") + self.mocked_watch = patch_watch.start() + self.addCleanup(patch_watch.stop) + + self.config = Configurator(settings=self.settings) self.config.registry.storage = {} self.config.registry.cache = {} self.config.registry.permission = {} - patch = mock.patch("kinto.core.statsd.load_from_config") - self.mocked = patch.start() - self.addCleanup(patch.stop) + def test_setup_statsd_step_is_still_supported(self): + with mock.patch("warnings.warn") as mocked_warnings: + initialization.setup_statsd(self.config) + mocked_warnings.assert_called_with( + "``setup_statsd()`` is now deprecated. Use ``kinto.core.initialization.setup_metrics()`` instead.", + DeprecationWarning, + ) + self.mocked.assert_called_with("host", 8080, "kinto.core") - def test_statsd_isnt_called_if_statsd_url_is_not_set(self): - self.config.add_settings({"statsd_url": None}) - initialization.setup_statsd(self.config) - self.mocked.assert_not_called() + def test_statsd_is_included_by_default(self): + kinto.core.initialize(self.config, "0.0.1", "name") + + self.mocked.assert_called_with("host", 8080, "kinto.core") - def test_statsd_is_set_to_none_if_statsd_url_not_set(self): + def test_statsd_isnt_included_if_statsd_url_is_not_set(self): self.config.add_settings({"statsd_url": None}) - initialization.setup_statsd(self.config) - self.assertEqual(self.config.registry.statsd, None) + kinto.core.initialize(self.config, "0.0.1", "name") - def test_statsd_is_called_if_statsd_url_is_set(self): - initialization.setup_statsd(self.config) - self.mocked.assert_called_with(self.config) - # See `tests/core/test_statsd.py` for instantiation tests. + self.mocked.assert_not_called() - def test_statsd_is_expose_in_the_registry_if_url_is_set(self): - initialization.setup_statsd(self.config) - self.assertEqual(self.config.registry.statsd, self.mocked.return_value) + # + # Backends. + # def test_statsd_is_set_on_cache(self): - c = initialization.setup_statsd(self.config) - c.watch_execution_time.assert_any_call({}, prefix="backend") + kinto.core.initialize(self.config, "0.0.1", "settings_prefix") + _app = webtest.TestApp(self.config.make_wsgi_app()) + + self.mocked_watch.assert_any_call(self.mocked(), {}, prefix="backend") def test_statsd_is_set_on_storage(self): - c = initialization.setup_statsd(self.config) - c.watch_execution_time.assert_any_call({}, prefix="backend") + kinto.core.initialize(self.config, "0.0.1", "settings_prefix") + _app = webtest.TestApp(self.config.make_wsgi_app()) + + self.mocked_watch.assert_any_call(self.mocked(), {}, prefix="backend") def test_statsd_is_set_on_permission(self): - c = initialization.setup_statsd(self.config) - c.watch_execution_time.assert_any_call({}, prefix="backend") + kinto.core.initialize(self.config, "0.0.1", "settings_prefix") + _app = webtest.TestApp(self.config.make_wsgi_app()) + + self.mocked_watch.assert_any_call(self.mocked(), {}, prefix="backend") - def test_statsd_is_set_on_authentication(self): - c = initialization.setup_statsd(self.config) - c.watch_execution_time.assert_any_call(None, prefix="authentication") + # + # Authentication. + # + + def test_statsd_is_set_on_authentication_multiauth(self): + kinto.core.initialize(self.config, "0.0.1", "settings_prefix") + _app = webtest.TestApp(self.config.make_wsgi_app()) + + self.mocked_watch.assert_any_call( + self.mocked(), mock.ANY, prefix="authentication", classname="basicauth" + ) + + def test_statsd_is_set_on_authentication_raw_auth(self): + authn_policy = SessionAuthenticationPolicy() + self.config.set_authentication_policy(authn_policy) + + kinto.core.initialize(self.config, "0.0.1", "settings_prefix") + _app = webtest.TestApp(self.config.make_wsgi_app()) + + self.mocked_watch.assert_any_call(self.mocked(), mock.ANY, prefix="authentication") + + @mock.patch("kinto.core.utils.hmac_digest") + def test_statsd_counts_unique_users(self, digest_mocked): + digest_mocked.return_value = "mat" + 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") + + def test_statsd_counts_authentication_types(self): + 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("authn_type.basicauth") + + # + # Endpoints. + # def test_statsd_counts_nothing_on_anonymous_requests(self): kinto.core.initialize(self.config, "0.0.1", "settings_prefix") @@ -361,21 +414,33 @@ def test_statsd_counts_unknown_urls(self): app.get("/v0/coucou", status=404) self.assertFalse(self.mocked.count.called) - @mock.patch("kinto.core.utils.hmac_digest") - def test_statsd_counts_unique_users(self, digest_mocked): - digest_mocked.return_value = "mat" - kinto.core.initialize(self.config, "0.0.1", "settings_prefix") - app = webtest.TestApp(self.config.make_wsgi_app()) - headers = {"Authorization": "Basic bWF0Og=="} - app.get("/v0/", headers=headers) - self.mocked().count.assert_any_call("users", unique="basicauth.mat") + def test_metrics_and_statsd_are_none_if_statsd_url_not_set(self): + self.config.add_settings({"statsd_url": None}) - def test_statsd_counts_authentication_types(self): - kinto.core.initialize(self.config, "0.0.1", "settings_prefix") - app = webtest.TestApp(self.config.make_wsgi_app()) - headers = {"Authorization": "Basic bWF0Og=="} - app.get("/v0/", headers=headers) - self.mocked().count.assert_any_call("authn_type.basicauth") + initialization.setup_metrics(self.config) + + self.assertIsNone(self.config.registry.statsd) + self.assertIsNone(self.config.registry.metrics) + + def test_metrics_attr_is_set_if_statsd_url_is_set(self): + self.config.add_settings({"statsd_url": "udp://host:8080"}) + + initialization.setup_metrics(self.config) + + self.assertIsNotNone(self.config.registry.statsd) + self.assertIsNotNone(self.config.registry.metrics) + + def test_statsd_attr_is_exposed_in_the_registry_if_url_is_set(self): + self.config.add_settings({"statsd_url": "udp://host:8080"}) + + initialization.setup_metrics(self.config) + + with mock.patch("warnings.warn") as mocked_warnings: + self.config.registry.statsd.count("key") + mocked_warnings.assert_called_with( + "``config.registry.statsd`` is now deprecated. Use ``config.registry.metrics`` instead.", + DeprecationWarning, + ) class RequestsConfigurationTest(unittest.TestCase): diff --git a/tests/core/test_metrics.py b/tests/core/test_metrics.py new file mode 100644 index 000000000..c6ff06a22 --- /dev/null +++ b/tests/core/test_metrics.py @@ -0,0 +1,49 @@ +import unittest +from unittest import mock + +from pyramid.config import Configurator + +from kinto.core import metrics + + +class TestedClass: + attribute = 3.14 + + def test_method(self): + pass + + def _private_method(self): + pass + + +class WatchExecutionTimeTest(unittest.TestCase): + def setUp(self): + self.test_object = TestedClass() + self.mocked = mock.MagicMock() + metrics.watch_execution_time(self.mocked, self.test_object, prefix="test") + + def test_public_methods_generates_statsd_calls(self): + self.test_object.test_method() + self.mocked.timer.assert_called_with("test.testedclass.test_method") + + def test_private_methods_does_not_generates_statsd_calls(self): + self.test_object._private_method() + self.assertFalse(self.mocked().timer.called) + + +class ListenerWithTimerTest(unittest.TestCase): + def setUp(self): + self.config = Configurator() + self.func = lambda x: x # noqa: E731 + + def test_without_metrics_service(self): + wrapped = metrics.listener_with_timer(self.config, "key", self.func) + + self.assertEqual(wrapped(42), 42) # does not raise + + def test_with_metrics_service(self): + self.config.registry.registerUtility(mock.MagicMock(), metrics.IMetricsService) + wrapped = metrics.listener_with_timer(self.config, "key", self.func) + + self.assertEqual(wrapped(42), 42) + self.config.registry.metrics.timer.assert_called_with("key") diff --git a/tests/core/test_statsd.py b/tests/core/test_statsd.py deleted file mode 100644 index 748b9ce06..000000000 --- a/tests/core/test_statsd.py +++ /dev/null @@ -1,120 +0,0 @@ -from unittest import mock - -from pyramid import testing -from pyramid.exceptions import ConfigurationError - -from kinto.core import statsd -from kinto.core.testing import skip_if_no_statsd, unittest - -from .support import BaseWebTest - - -class TestedClass: - attribute = 3.14 - - def test_method(self): - pass - - def _private_method(self): - pass - - -class StatsDMissing(unittest.TestCase): - def setUp(self): - self.previous = statsd.statsd_module - statsd.statsd_module = None - - def tearDown(self): - statsd.statsd_module = self.previous - - def test_client_instantiation_raises_properly(self): - with self.assertRaises(ConfigurationError): - statsd.load_from_config(mock.MagicMock()) - - -@skip_if_no_statsd -class StatsdClientTest(unittest.TestCase): - settings = {"statsd_url": "udp://foo:1234", "statsd_prefix": "prefix", "project_name": ""} - - def setUp(self): - self.client = statsd.Client("localhost", 1234, "prefix") - self.test_object = TestedClass() - - with mock.patch.object(self.client, "_client") as mocked_client: - self.client.watch_execution_time(self.test_object, prefix="test") - self.mocked_client = mocked_client - - def test_public_methods_generates_statsd_calls(self): - self.test_object.test_method() - - self.mocked_client.timer.assert_called_with("test.testedclass.test_method") - - def test_private_methods_does_not_generates_statsd_calls(self): - self.mocked_client.reset_mock() - self.test_object._private_method() - self.assertFalse(self.mocked_client.timer.called) - - def test_count_increments_the_counter_for_key(self): - with mock.patch.object(self.client, "_client") as mocked_client: - self.client.count("click") - mocked_client.incr.assert_called_with("click", count=1) - - def test_count_can_increment_by_more_than_one(self): - with mock.patch.object(self.client, "_client") as mocked_client: - self.client.count("click", count=10) - mocked_client.incr.assert_called_with("click", count=10) - - def test_count_with_unique_uses_sets_for_key(self): - with mock.patch.object(self.client, "_client") as mocked_client: - self.client.count("click", unique="menu") - mocked_client.set.assert_called_with("click", "menu") - - @mock.patch("kinto.core.statsd.statsd_module") - def test_load_from_config(self, module_mock): - config = testing.setUp() - config.registry.settings = self.settings - statsd.load_from_config(config) - module_mock.StatsClient.assert_called_with("foo", 1234, prefix="prefix") - - @mock.patch("kinto.core.statsd.statsd_module") - def test_load_from_config_uses_project_name_if_defined(self, module_mock): - config = testing.setUp() - config.registry.settings = {**self.settings, "project_name": "projectname"} - statsd.load_from_config(config) - module_mock.StatsClient.assert_called_with("foo", 1234, prefix="projectname") - - def test_statsd_count_handle_unconfigured_statsd_client(self): - request = mock.MagicMock() - request.registry.statsd = None - statsd.statsd_count(request, "toto") # Doesn't raise - - def test_statsd_count_call_the_client_if_configured(self): - request = mock.MagicMock() - request.registry.statsd = self.mocked_client - statsd.statsd_count(request, "toto") - self.mocked_client.count.assert_called_with("toto") - - -@skip_if_no_statsd -class TimingTest(BaseWebTest, unittest.TestCase): - @classmethod - def get_app_settings(cls, *args, **kwargs): - settings = super().get_app_settings(*args, **kwargs) - if not statsd.statsd_module: - return settings - - settings["statsd_url"] = "udp://localhost:8125" - return settings - - def test_statds_tracks_listeners_execution_duration(self): - statsd_client = self.app.app.registry.statsd._client - with mock.patch.object(statsd_client, "timing") as mocked: - self.app.get("/", headers=self.headers) - self.assertTrue(mocked.called) - - def test_statds_tracks_authentication_policies(self): - statsd_client = self.app.app.registry.statsd._client - with mock.patch.object(statsd_client, "timing") as mocked: - self.app.get("/", headers=self.headers) - timers = set(c[0][0] for c in mocked.call_args_list) - self.assertIn("authentication.basicauth.unauthenticated_userid", timers) diff --git a/tests/plugins/test_history.py b/tests/plugins/test_history.py index 720136151..36088fc25 100644 --- a/tests/plugins/test_history.py +++ b/tests/plugins/test_history.py @@ -3,9 +3,6 @@ import unittest from unittest import mock -from pyramid import testing - -from kinto import main as kinto_main from kinto.core.testing import get_user_headers, skip_if_no_statsd from .. import support @@ -14,19 +11,6 @@ DATETIME_REGEX = r"^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{6}\+\d{2}:\d{2}$" -class PluginSetup(unittest.TestCase): - @skip_if_no_statsd - def test_a_statsd_timer_is_used_for_history_if_configured(self): - settings = { - "statsd_url": "udp://127.0.0.1:8125", - "includes": "kinto.plugins.history", - } - config = testing.setUp(settings=settings) - with mock.patch("kinto.core.statsd.Client.timer") as mocked: - kinto_main(None, config=config) - mocked.assert_called_with("plugins.history") - - class HistoryWebTest(support.BaseWebTest, unittest.TestCase): @classmethod def get_app_settings(cls, extras=None): @@ -42,6 +26,24 @@ def test_history_capability_if_enabled(self): self.assertIn("history", capabilities) +@skip_if_no_statsd +class MetricsTest(HistoryWebTest): + @classmethod + def get_app_settings(cls, extras=None): + settings = super().get_app_settings(extras) + settings.update( + **{ + "statsd_url": "udp://127.0.0.1:8125", + } + ) + return settings + + def test_a_statsd_timer_is_used_for_history_if_configured(self): + with mock.patch("kinto.plugins.statsd.StatsDService.timer") as mocked: + self.app.put("/buckets/test", headers=self.headers) + mocked.assert_any_call("plugins.history") + + class HistoryViewTest(HistoryWebTest): def setUp(self): self.bucket_uri = "/buckets/test" diff --git a/tests/plugins/test_quotas.py b/tests/plugins/test_quotas.py index c31408eb7..27ec5c736 100644 --- a/tests/plugins/test_quotas.py +++ b/tests/plugins/test_quotas.py @@ -3,9 +3,7 @@ import pytest import transaction -from pyramid import testing -from kinto import main as kinto_main from kinto.core.errors import ERRORS from kinto.core.storage import Sort from kinto.core.storage.exceptions import ObjectNotFoundError @@ -21,19 +19,6 @@ from .. import support -class PluginSetup(unittest.TestCase): - @skip_if_no_statsd - def test_a_statsd_timer_is_used_for_quotas_if_configured(self): - settings = { - "statsd_url": "udp://127.0.0.1:8125", - "includes": "kinto.plugins.quotas", - } - config = testing.setUp(settings=settings) - with mock.patch("kinto.core.statsd.Client.timer") as mocked: - kinto_main(None, config=config) - mocked.assert_called_with("plugins.quotas") - - class QuotaWebTest(support.BaseWebTest, unittest.TestCase): bucket_uri = "/buckets/test" collection_uri = "/buckets/test/collections/col" @@ -91,6 +76,24 @@ def test_quota_capability_if_enabled(self): self.assertIn("quotas", capabilities) +@skip_if_no_statsd +class MetricsTest(QuotaWebTest): + @classmethod + def get_app_settings(cls, extras=None): + settings = super().get_app_settings(extras) + settings.update( + **{ + "statsd_url": "udp://127.0.0.1:8125", + } + ) + return settings + + def test_a_statsd_timer_is_used_for_quotas_if_configured(self): + with mock.patch("kinto.plugins.statsd.StatsDService.timer") as mocked: + self.app.put("/buckets/test", headers=self.headers) + mocked.assert_any_call("plugins.quotas") + + class QuotaListenerTest(QuotaWebTest): # # Bucket diff --git a/tests/plugins/test_statsd.py b/tests/plugins/test_statsd.py new file mode 100644 index 000000000..975941f2a --- /dev/null +++ b/tests/plugins/test_statsd.py @@ -0,0 +1,61 @@ +from unittest import mock + +from pyramid import testing +from pyramid.exceptions import ConfigurationError + +from kinto.core.testing import skip_if_no_statsd, unittest +from kinto.plugins import statsd + + +class StatsDMissing(unittest.TestCase): + def setUp(self): + self.previous = statsd.statsd_module + statsd.statsd_module = None + + def tearDown(self): + statsd.statsd_module = self.previous + + def test_client_instantiation_raises_properly(self): + with self.assertRaises(ConfigurationError): + statsd.load_from_config(mock.MagicMock()) + + +@skip_if_no_statsd +class StatsdClientTest(unittest.TestCase): + settings = {"statsd_url": "udp://foo:1234", "statsd_prefix": "prefix", "project_name": ""} + + def setUp(self): + self.client = statsd.StatsDService("localhost", 1234, "prefix") + + patch = mock.patch.object(self.client, "_client") + self.mocked_client = patch.start() + self.addCleanup(patch.stop) + + def test_count_increments_the_counter_for_key(self): + with mock.patch.object(self.client, "_client") as mocked_client: + self.client.count("click") + mocked_client.incr.assert_called_with("click", count=1) + + def test_count_can_increment_by_more_than_one(self): + with mock.patch.object(self.client, "_client") as mocked_client: + self.client.count("click", count=10) + mocked_client.incr.assert_called_with("click", count=10) + + def test_count_with_unique_uses_sets_for_key(self): + with mock.patch.object(self.client, "_client") as mocked_client: + self.client.count("click", unique="menu") + mocked_client.set.assert_called_with("click", "menu") + + @mock.patch("kinto.plugins.statsd.statsd_module") + def test_load_from_config(self, module_mock): + config = testing.setUp() + config.registry.settings = self.settings + statsd.load_from_config(config) + module_mock.StatsClient.assert_called_with("foo", 1234, prefix="prefix") + + @mock.patch("kinto.plugins.statsd.statsd_module") + def test_load_from_config_uses_project_name_if_defined(self, module_mock): + config = testing.setUp() + config.registry.settings = {**self.settings, "project_name": "projectname"} + statsd.load_from_config(config) + module_mock.StatsClient.assert_called_with("foo", 1234, prefix="projectname")