Prefix management -- take two.

Agree with @aremm here ... instead of requiring contortions to run
without a prefix, let's just require that it is explicitly set to `None`
if you want to skip the prefix.  If you `prefix` isn't in the settings
explicitly, then use `$STATSD_PREFIX` if it is set; otherwise, try to
construct a prefix from service & environment, if that fails, then fail
hard in the initializer.
This commit is contained in:
Dave Shawley 2021-03-24 08:10:07 -04:00
parent 65df480421
commit b84e52592d
No known key found for this signature in database
GPG key ID: 44A9C9992CCFAB82
2 changed files with 31 additions and 50 deletions

View file

@ -56,9 +56,9 @@ class Application(web.Application):
.. 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.
If you want to run without a prefix, then you are required to explicitly
set ``statsd.prefix`` to ``None``. This prevents accidentally 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
@ -79,18 +79,19 @@ class Application(web.Application):
statsd_settings.setdefault('protocol',
os.environ.get('STATSD_PROTOCOL', 'tcp'))
if os.environ.get('STATSD_PREFIX'):
statsd_settings.setdefault('prefix', os.environ['STATSD_PREFIX'])
else:
if 'prefix' not in statsd_settings:
statsd_settings['prefix'] = os.environ.get('STATSD_PREFIX')
if not statsd_settings['prefix']:
try:
prefix = '.'.join([
statsd_settings['prefix'] = '.'.join([
'applications',
settings['service'],
settings['environment'],
])
except KeyError:
prefix = None
statsd_settings.setdefault('prefix', prefix)
raise RuntimeError(
'statsd configuration error: prefix is not set. Set'
' $STATSD_PREFIX or configure settings.statsd.prefix')
super().__init__(*args, **settings)
@ -108,11 +109,6 @@ 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'],

View file

@ -62,7 +62,7 @@ class ApplicationTests(AsyncTestCaseWithTimeout):
self.unsetenv('STATSD_PREFIX')
self.unsetenv('STATSD_PROTOCOL')
app = sprockets_statsd.tornado.Application()
app = sprockets_statsd.tornado.Application(statsd={'prefix': None})
self.assertIn('statsd', app.settings)
self.assertIsNone(app.settings['statsd']['host'],
'default host value should be None')
@ -84,14 +84,12 @@ class ApplicationTests(AsyncTestCaseWithTimeout):
self.assertEqual('udp', app.settings['statsd']['protocol'])
def test_prefix_when_only_service_is_set(self):
app = sprockets_statsd.tornado.Application(service='blah')
self.assertIn('statsd', app.settings)
self.assertEqual(None, app.settings['statsd']['prefix'])
with self.assertRaises(RuntimeError):
sprockets_statsd.tornado.Application(service='blah')
def test_prefix_when_only_environment_is_set(self):
app = sprockets_statsd.tornado.Application(environment='whatever')
self.assertIn('statsd', app.settings)
self.assertEqual(None, app.settings['statsd']['prefix'])
with self.assertRaises(RuntimeError):
sprockets_statsd.tornado.Application(environment='whatever')
def test_prefix_default_when_service_and_environment_are_set(self):
app = sprockets_statsd.tornado.Application(environment='development',
@ -117,35 +115,20 @@ class ApplicationTests(AsyncTestCaseWithTimeout):
self.assertEqual('myapp', app.settings['statsd']['prefix'])
self.assertEqual('udp', app.settings['statsd']['protocol'])
def test_that_starting_without_configuration_fails(self):
def test_that_starting_without_host_fails(self):
self.unsetenv('STATSD_HOST')
app = sprockets_statsd.tornado.Application()
app = sprockets_statsd.tornado.Application(statsd={'prefix': 'app'})
with self.assertRaises(RuntimeError):
self.run_coroutine(app.start_statsd())
def test_that_starting_without_prefix_fails_by_default(self):
def test_creating_without_prefix_on_purpose(self):
self.unsetenv('STATSD_PREFIX')
app = sprockets_statsd.tornado.Application(statsd={
'host': 'statsd.example.com',
'protocol': 'udp',
'prefix': None,
})
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.tornado.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())
self.assertEqual(None, app.settings['statsd']['prefix'])
def test_starting_with_calculated_prefix(self):
self.unsetenv('STATSD_PREFIX')
@ -184,6 +167,7 @@ class ApplicationTests(AsyncTestCaseWithTimeout):
app = sprockets_statsd.tornado.Application(statsd={
'host': 'localhost',
'port': '8125',
'prefix': 'my-service',
})
self.run_coroutine(app.stop_statsd())
@ -244,8 +228,6 @@ class RequestHandlerTests(AsyncTestCaseWithTimeout, testing.AsyncHTTPTestCase):
self.app.settings['statsd'].update({
'host': self.statsd_server.host,
'port': self.statsd_server.port,
'prefix': 'applications.service',
'protocol': 'tcp',
})
self.run_coroutine(self.app.start_statsd())
@ -256,7 +238,10 @@ class RequestHandlerTests(AsyncTestCaseWithTimeout, testing.AsyncHTTPTestCase):
super().tearDown()
def get_app(self):
self.app = Application()
self.app = Application(statsd={
'prefix': 'applications.service',
'protocol': 'tcp',
})
return self.app
def wait_for_metrics(self, metric_count=3):