From 9a8f23c6144ac034cb8cb42c9dd6252a6e20ffc4 Mon Sep 17 00:00:00 2001 From: Dave Shawley Date: Tue, 23 Mar 2021 22:01:27 -0400 Subject: [PATCH] Require prefix unless allow_no_prefix is set. --- sprockets_statsd/mixins.py | 23 ++++++++++++++----- tests/test_mixins.py | 45 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 6 deletions(-) diff --git a/sprockets_statsd/mixins.py b/sprockets_statsd/mixins.py index 2c0989a..a21635a 100644 --- a/sprockets_statsd/mixins.py +++ b/sprockets_statsd/mixins.py @@ -25,7 +25,7 @@ class Application(web.Application): +-------------------+---------------------------------------------+ | port | port number that statsd is listening on | +-------------------+---------------------------------------------+ - | prefix | segment to prefix to metrics. | + | prefix | segment to prefix to metrics | +-------------------+---------------------------------------------+ | protocol | "tcp" or "udp" | +-------------------+---------------------------------------------+ @@ -47,12 +47,18 @@ class Application(web.Application): **prefix** is prefixed to all metric paths. This provides a namespace for metrics so that each applications metrics are maintained - in separate buckets. + in separate buckets. The default is to use the :envvar:`STATSD_PREFIX` + environment variable. If it is unset and the *service* and + *environment* keys are set in ``settings``, then the default is + ``applications..``. This is a convenient way to + maintain consistent metric paths when you are managing a larger number + of services. - If the *service* and *environment* keys are set in ``settings``, - then the default prefix is ``applications..``. - This is a convenient way to maintain consistent metric paths when - you are managing a larger number of services. + .. rubric:: Warning + + If you want to run without a prefix, then you are required to also + set the ``allow_no_prefix`` key with a *truthy* value. This prevents + accidentilly polluting the metric namespace with unqualified paths. **protocol** defaults to the :envvar:`STATSD_PROTOCOL` environment variable with a back default of "tcp" if the environment variable @@ -102,6 +108,11 @@ class Application(web.Application): """ if self.statsd_connector is None: statsd_settings = self.settings['statsd'] + if (statsd_settings.get('prefix', None) is None + and not statsd_settings.get('allow_no_prefix', False)): + raise RuntimeError( + 'statsd configuration error: prefix is not set') + kwargs = { 'host': statsd_settings['host'], 'port': statsd_settings['port'], diff --git a/tests/test_mixins.py b/tests/test_mixins.py index 5155667..f0955ca 100644 --- a/tests/test_mixins.py +++ b/tests/test_mixins.py @@ -115,10 +115,51 @@ class ApplicationTests(AsyncTestCaseWithTimeout): with self.assertRaises(RuntimeError): self.run_coroutine(app.start_statsd()) + def test_that_starting_without_prefix_fails_by_default(self): + self.unsetenv('STATSD_PREFIX') + app = sprockets_statsd.mixins.Application(statsd={ + 'host': 'statsd.example.com', + 'protocol': 'udp', + }) + with self.assertRaises(RuntimeError) as cm: + self.run_coroutine(app.start_statsd()) + self.assertTrue('prefix is not set' in str(cm.exception), + 'Expected "prefix is not set" in exception message') + + def test_starting_without_prefix_on_purpose(self): + self.unsetenv('STATSD_PREFIX') + app = sprockets_statsd.mixins.Application( + statsd={ + 'allow_no_prefix': True, + 'host': 'statsd.example.com', + 'protocol': 'udp', + }) + try: + self.run_coroutine(app.start_statsd()) + finally: + self.run_coroutine(app.stop_statsd()) + + def test_starting_with_calculated_prefix(self): + self.unsetenv('STATSD_PREFIX') + app = sprockets_statsd.mixins.Application( + environment='development', + service='my-service', + statsd={ + 'host': 'statsd.example.com', + 'protocol': 'udp', + }) + try: + self.run_coroutine(app.start_statsd()) + self.assertEqual('applications.my-service.development', + app.settings['statsd']['prefix']) + finally: + self.run_coroutine(app.stop_statsd()) + def test_starting_twice(self): app = sprockets_statsd.mixins.Application(statsd={ 'host': 'localhost', 'port': '8125', + 'prefix': 'my-service', }) try: self.run_coroutine(app.start_statsd()) @@ -143,6 +184,7 @@ class ApplicationTests(AsyncTestCaseWithTimeout): statsd={ 'host': 'localhost', 'port': '8125', + 'prefix': 'my-service', 'reconnect_sleep': 0.5, 'wait_timeout': 0.25, }) @@ -156,6 +198,7 @@ class ApplicationTests(AsyncTestCaseWithTimeout): def test_starting_with_invalid_protocol(self): app = sprockets_statsd.mixins.Application(statsd={ 'host': 'localhost', + 'prefix': 'my-service', 'protocol': 'unknown' }) with self.assertRaises(RuntimeError): @@ -164,6 +207,7 @@ class ApplicationTests(AsyncTestCaseWithTimeout): def test_that_protocol_strings_are_translated(self): app = sprockets_statsd.mixins.Application(statsd={ 'host': 'localhost', + 'prefix': 'my-service', 'protocol': 'tcp', }) self.run_coroutine(app.start_statsd()) @@ -173,6 +217,7 @@ class ApplicationTests(AsyncTestCaseWithTimeout): app = sprockets_statsd.mixins.Application(statsd={ 'host': 'localhost', + 'prefix': 'my-service', 'protocol': 'udp', }) self.run_coroutine(app.start_statsd())