From 7b395150003bd410b211bc4f70130508bdccfb5d Mon Sep 17 00:00:00 2001 From: Dave Shawley Date: Mon, 2 Sep 2019 08:28:56 -0400 Subject: [PATCH 01/10] Add tornado 5 & 6 environments to tox.ini. --- tox.ini | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 14e942f..fb4c849 100644 --- a/tox.ini +++ b/tox.ini @@ -1,8 +1,18 @@ [tox] -envlist = py37 +envlist = py37,tornado5,tornado6 toxworkdir = build/tox skip_missing_interpreters = True [testenv] deps = -r requires/testing.txt commands = nosetests + +[testenv:tornado5] +deps = + tornado>=5,<6 + -r requires/testing.txt + +[testenv:tornado6] +deps = + tornado>=6,<7 + -r requires/testing.txt From 8d73f5bbf5a8e5a98b8b4f0842e4546d485b57d7 Mon Sep 17 00:00:00 2001 From: Dave Shawley Date: Mon, 2 Sep 2019 08:29:33 -0400 Subject: [PATCH 02/10] Show missing lines in coverage report. --- setup.cfg | 3 +++ 1 file changed, 3 insertions(+) diff --git a/setup.cfg b/setup.cfg index f8f9546..57da40c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -8,6 +8,9 @@ warning-is-error = 1 [check] strict = 1 +[coverage:report] +show_missing = 1 + [nosetests] cover-package = sprockets.mixins.metrics cover-branches = 1 From f24bc7c0650be0ce73e0bf6cec134c3ca3ce85a1 Mon Sep 17 00:00:00 2001 From: Dave Shawley Date: Mon, 2 Sep 2019 08:30:00 -0400 Subject: [PATCH 03/10] Use full path to unittest.mock in tests.py --- tests.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests.py b/tests.py index 2f90665..80c7a1f 100644 --- a/tests.py +++ b/tests.py @@ -1,8 +1,7 @@ import asyncio import itertools import socket -import unittest -from unittest import mock +import unittest.mock from tornado import iostream, testing, web @@ -48,7 +47,7 @@ class MisconfiguredStatsdMetricCollectionTests(testing.AsyncHTTPTestCase): def test_bad_protocol_raises_ValueError(self): with self.assertRaises(ValueError): statsd.StatsDCollector(host='127.0.0.1', - port=8125, + port='8125', protocol='bad_protocol') @@ -75,13 +74,13 @@ class TCPStatsdMetricCollectionTests(testing.AsyncHTTPTestCase): 'protocol': 'tcp', 'prepend_metric_type': True}) - @mock.patch.object(iostream.IOStream, 'write') + @unittest.mock.patch.object(iostream.IOStream, 'write') def test_write_not_executed_when_connection_is_closed(self, mock_write): self.application.statsd._sock.close() self.application.statsd.send('foo', 500, 'c') mock_write.assert_not_called() - @mock.patch.object(iostream.IOStream, 'write') + @unittest.mock.patch.object(iostream.IOStream, 'write') def test_expected_counters_data_written(self, mock_sock): path = ('foo', 'bar') value = 500 @@ -94,7 +93,7 @@ class TCPStatsdMetricCollectionTests(testing.AsyncHTTPTestCase): self.application.statsd.send(path, value, metric_type) mock_sock.assert_called_once_with(expected.encode()) - @mock.patch.object(iostream.IOStream, 'write') + @unittest.mock.patch.object(iostream.IOStream, 'write') def test_expected_timers_data_written(self, mock_sock): path = ('foo', 'bar') value = 500 @@ -223,7 +222,7 @@ class UDPStatsdMetricCollectionTests(testing.AsyncHTTPTestCase): self.statsd.close() super().tearDown() - @mock.patch.object(socket.socket, 'sendto') + @unittest.mock.patch.object(socket.socket, 'sendto') def test_expected_counters_data_written(self, mock_sock): path = ('foo', 'bar') value = 500 @@ -238,7 +237,7 @@ class UDPStatsdMetricCollectionTests(testing.AsyncHTTPTestCase): expected.encode(), (self.statsd.sockaddr[0], self.statsd.sockaddr[1])) - @mock.patch.object(socket.socket, 'sendto') + @unittest.mock.patch.object(socket.socket, 'sendto') def test_expected_timers_data_written(self, mock_sock): path = ('foo', 'bar') value = 500 From c4dd176ab3f871a4a18bf98530e719d42d7e26c1 Mon Sep 17 00:00:00 2001 From: Dave Shawley Date: Mon, 2 Sep 2019 08:30:08 -0400 Subject: [PATCH 04/10] Remove unnecessary "on connect" callback. This was removed in Tornado 6 when many of the other callbacks were removed. This was using the callback as the server hostname for SNI purposes. Luckily we aren't using SSL sockets here ;) --- sprockets/mixins/metrics/statsd.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/sprockets/mixins/metrics/statsd.py b/sprockets/mixins/metrics/statsd.py index de737de..d71fa2b 100644 --- a/sprockets/mixins/metrics/statsd.py +++ b/sprockets/mixins/metrics/statsd.py @@ -128,7 +128,7 @@ class StatsDCollector: """ sock = iostream.IOStream(socket.socket( socket.AF_INET, socket.SOCK_STREAM, socket.IPPROTO_TCP)) - sock.connect(self._address, self._tcp_on_connected) + sock.connect(self._address) sock.set_close_callback(self._tcp_on_closed) return sock @@ -139,10 +139,6 @@ class StatsDCollector: await asyncio.sleep(self._tcp_reconnect_sleep) self._sock = self._tcp_socket() - def _tcp_on_connected(self): - """Invoked when the IOStream is connected""" - LOGGER.debug('Connected to statsd at %s via TCP', self._address) - def send(self, path, value, metric_type): """Send a metric to Statsd. From 72859a6877e13ff9294ba03c419dc5e93cf642a3 Mon Sep 17 00:00:00 2001 From: Dave Shawley Date: Mon, 2 Sep 2019 08:43:03 -0400 Subject: [PATCH 05/10] Update development tools. --- requires/testing.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/requires/testing.txt b/requires/testing.txt index 266cc8b..3fe748a 100644 --- a/requires/testing.txt +++ b/requires/testing.txt @@ -1,3 +1,3 @@ -coverage==4.5.2 -flake8==3.6.0 +coverage==4.5.4 +flake8==3.7.8 nose==1.3.7 From 08b3cf206bea7f6fabd30db71719dd3eb1504c66 Mon Sep 17 00:00:00 2001 From: Dave Shawley Date: Mon, 2 Sep 2019 09:11:19 -0400 Subject: [PATCH 06/10] Fix documentation. --- docs/api.rst | 3 +++ sprockets/mixins/metrics/statsd.py | 13 ++++++------- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/docs/api.rst b/docs/api.rst index c68183d..48554ee 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -49,6 +49,9 @@ Statsd Implementation .. autoclass:: sprockets.mixins.metrics.statsd.StatsdMixin :members: +.. autoclass:: sprockets.mixins.metrics.statsd.StatsDCollector + :members: + Testing Helpers --------------- *So who actually tests that their metrics are emitted as they expect?* diff --git a/sprockets/mixins/metrics/statsd.py b/sprockets/mixins/metrics/statsd.py index d71fa2b..8556d41 100644 --- a/sprockets/mixins/metrics/statsd.py +++ b/sprockets/mixins/metrics/statsd.py @@ -86,10 +86,9 @@ class StatsdMixin: class StatsDCollector: """Collects and submits stats to StatsD. - This class should be constructed using the - :meth:`~sprockets.mixins.statsd.install` method. When installed, - it is attached to the :class:`~tornado.web.Application` instance - for your web application. + This class should be constructed using the :func:`.install` function. + When installed, it is attached to the :class:`~tornado.web.Application` + instance for your web application. :param str host: The StatsD host :param str port: The StatsD port @@ -201,16 +200,16 @@ def install(application, **kwargs): :param tornado.web.Application application: the application to install the collector into. :param kwargs: keyword parameters to pass to the - :class:`StatsDCollector` initializer. + :class:`.StatsDCollector` initializer. :returns: :data:`True` if the client was installed successfully, or :data:`False` otherwise. - **host** The StatsD host. If host is not specified, the ``STATSD_HOST`` environment variable, or default `127.0.0.1`, - will be pass into the :class:`StatsDCollector`. + will be pass into the :class:`.StatsDCollector`. - **port** The StatsD port. If port is not specified, the ``STATSD_PORT`` environment variable, or default `8125`, - will be pass into the :class:`StatsDCollector`. + will be pass into the :class:`.StatsDCollector`. - **namespace** The StatsD bucket to write metrics into. """ From 5ced9be0be78cc575dc41a948cb6b54eee680028 Mon Sep 17 00:00:00 2001 From: Dave Shawley Date: Mon, 2 Sep 2019 08:58:44 -0400 Subject: [PATCH 07/10] Add reconnect logic test. --- tests.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests.py b/tests.py index 80c7a1f..19b89be 100644 --- a/tests.py +++ b/tests.py @@ -155,6 +155,13 @@ class TCPStatsdMetricCollectionTests(testing.AsyncHTTPTestCase): self.assertEqual(expected, list(self.statsd.find_metrics(expected, 'ms'))[0][0]) + def test_reconnect_logic(self): + self.application.statsd._tcp_reconnect_sleep = 0.05 + self.application.statsd._sock.close() + asyncio.get_event_loop().run_until_complete(asyncio.sleep(0.075)) + response = self.fetch('/status_code') + self.assertEqual(response.code, 200) + class TCPStatsdConfigurationTests(testing.AsyncHTTPTestCase): From d6024301385ab47102fd5405d98cfc2739ed81c8 Mon Sep 17 00:00:00 2001 From: Dave Shawley Date: Mon, 2 Sep 2019 09:11:57 -0400 Subject: [PATCH 08/10] Stop mix-in from failing when not installed. --- docs/api.rst | 9 +++++++++ docs/history.rst | 1 + sprockets/mixins/metrics/statsd.py | 17 +++++++++++++++-- tests.py | 14 ++++++++++++++ 4 files changed, 39 insertions(+), 2 deletions(-) diff --git a/docs/api.rst b/docs/api.rst index 48554ee..e8a2326 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -52,6 +52,15 @@ Statsd Implementation .. autoclass:: sprockets.mixins.metrics.statsd.StatsDCollector :members: +Application Functions +--------------------- +Before you can use the mixin, you have to install the client by calling +the ``install`` function on your application instance. + +.. autofunction:: sprockets.mixins.metrics.statsd.install + +.. autofunction:: sprockets.mixins.metrics.statsd.get_client + Testing Helpers --------------- *So who actually tests that their metrics are emitted as they expect?* diff --git a/docs/history.rst b/docs/history.rst index 0e66ee7..515cc75 100644 --- a/docs/history.rst +++ b/docs/history.rst @@ -7,6 +7,7 @@ Release History --------------- - Add configuration documentation - Exclude Tornado >6 (as-yet-unreleased version) +- Add :func:`sprockets.mixins.metrics.statsd.get_client` function `4.0.0`_ (06-Feb-2019) ---------------------- diff --git a/sprockets/mixins/metrics/statsd.py b/sprockets/mixins/metrics/statsd.py index 8556d41..14952cc 100644 --- a/sprockets/mixins/metrics/statsd.py +++ b/sprockets/mixins/metrics/statsd.py @@ -29,7 +29,9 @@ class StatsdMixin: :param path: elements of the metric path to record """ - self.application.statsd.send(path, duration * 1000.0, 'ms') + client = get_client(self.application) + if client is not None: + client.send(path, duration * 1000.0, 'ms') def increase_counter(self, *path, **kwargs): """Increase a counter. @@ -45,7 +47,9 @@ class StatsdMixin: omitted, the counter is increased by one. """ - self.application.statsd.send(path, kwargs.get('amount', '1'), 'c') + client = get_client(self.application) + if client is not None: + client.send(path, kwargs.get('amount', '1'), 'c') @contextlib.contextmanager def execution_timer(self, *path): @@ -227,3 +231,12 @@ def install(application, **kwargs): setattr(application, 'statsd', StatsDCollector(**kwargs)) return True + + +def get_client(application): + """Fetch the statsd client if it is installed. + + :rtype: .StatsDCollector + + """ + return getattr(application, 'statsd', None) diff --git a/tests.py b/tests.py index 19b89be..c4b86d9 100644 --- a/tests.py +++ b/tests.py @@ -162,6 +162,13 @@ class TCPStatsdMetricCollectionTests(testing.AsyncHTTPTestCase): response = self.fetch('/status_code') self.assertEqual(response.code, 200) + def test_that_mixin_works_without_client(self): + self.application.statsd.close() + delattr(self.application, 'statsd') + + response = self.fetch('/', method='POST', body='') + self.assertEqual(response.code, 204) + class TCPStatsdConfigurationTests(testing.AsyncHTTPTestCase): @@ -308,6 +315,13 @@ class UDPStatsdMetricCollectionTests(testing.AsyncHTTPTestCase): self.assertEqual(expected, list(self.statsd.find_metrics(expected, 'ms'))[0][0]) + def test_that_mixin_works_without_client(self): + self.application.statsd.close() + delattr(self.application, 'statsd') + + response = self.fetch('/', method='POST', body='') + self.assertEqual(response.code, 204) + class UDPStatsdConfigurationTests(testing.AsyncHTTPTestCase): From bc02db0f48c088581b9f7c08e130b98eab39e402 Mon Sep 17 00:00:00 2001 From: Dave Shawley Date: Mon, 2 Sep 2019 09:14:02 -0400 Subject: [PATCH 09/10] Add StatsDCollector.close method. --- docs/history.rst | 1 + sprockets/mixins/metrics/statsd.py | 18 ++++++++++++++---- tests.py | 23 +++++++++++++++++++++++ 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/docs/history.rst b/docs/history.rst index 515cc75..12321fa 100644 --- a/docs/history.rst +++ b/docs/history.rst @@ -8,6 +8,7 @@ Release History - Add configuration documentation - Exclude Tornado >6 (as-yet-unreleased version) - Add :func:`sprockets.mixins.metrics.statsd.get_client` function +- Add :meth:`sprockets.mixins.metrics.statsd.StatsDCollector.close` method `4.0.0`_ (06-Feb-2019) ---------------------- diff --git a/sprockets/mixins/metrics/statsd.py b/sprockets/mixins/metrics/statsd.py index 14952cc..80ae5bf 100644 --- a/sprockets/mixins/metrics/statsd.py +++ b/sprockets/mixins/metrics/statsd.py @@ -113,6 +113,7 @@ class StatsDCollector: self._namespace = namespace self._prepend_metric_type = prepend_metric_type self._tcp_reconnect_sleep = 5 + self._closing = False if protocol == 'tcp': self._tcp = True @@ -137,10 +138,19 @@ class StatsDCollector: async def _tcp_on_closed(self): """Invoked when the socket is closed.""" - LOGGER.warning('Not connected to statsd, connecting in %s seconds', - self._tcp_reconnect_sleep) - await asyncio.sleep(self._tcp_reconnect_sleep) - self._sock = self._tcp_socket() + if self._closing: + LOGGER.info('Statsd socket closed') + else: + LOGGER.warning('Not connected to statsd, connecting in %s seconds', + self._tcp_reconnect_sleep) + await asyncio.sleep(self._tcp_reconnect_sleep) + self._sock = self._tcp_socket() + + def close(self): + """Gracefully close the socket.""" + if not self._closing: + self._closing = True + self._sock.close() def send(self, path, value, metric_type): """Send a metric to Statsd. diff --git a/tests.py b/tests.py index c4b86d9..a16d979 100644 --- a/tests.py +++ b/tests.py @@ -169,6 +169,29 @@ class TCPStatsdMetricCollectionTests(testing.AsyncHTTPTestCase): response = self.fetch('/', method='POST', body='') self.assertEqual(response.code, 204) + def test_that_client_closes_socket(self): + response = self.fetch('/status_code') + self.assertEqual(response.code, 200) + + self.application.statsd.close() + response = self.fetch('/status_code') + self.assertEqual(response.code, 200) + self.assertTrue(self.application.statsd._sock.closed()) + + def test_that_client_can_be_closed_multiple_times(self): + response = self.fetch('/status_code') + self.assertEqual(response.code, 200) + + self.application.statsd.close() + response = self.fetch('/status_code') + self.assertEqual(response.code, 200) + self.assertTrue(self.application.statsd._sock.closed()) + + self.application.statsd.close() + response = self.fetch('/status_code') + self.assertEqual(response.code, 200) + self.assertTrue(self.application.statsd._sock.closed()) + class TCPStatsdConfigurationTests(testing.AsyncHTTPTestCase): From f553cc2966ddbc6e752658203c171e3832e85bf6 Mon Sep 17 00:00:00 2001 From: Dave Shawley Date: Mon, 2 Sep 2019 17:04:16 -0400 Subject: [PATCH 10/10] Advertise support for python 3.8 --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index 3e01f99..50c219a 100755 --- a/setup.py +++ b/setup.py @@ -32,6 +32,7 @@ setuptools.setup( 'Natural Language :: English', 'Operating System :: OS Independent', 'Programming Language :: Python :: 3.7', + 'Programming Language :: Python :: 3.8', 'Programming Language :: Python :: Implementation :: CPython', 'Topic :: Internet :: WWW/HTTP', 'Topic :: Software Development :: Libraries',