mirror of
https://github.com/sprockets/sprockets.mixins.metrics.git
synced 2024-11-21 19:28:34 +00:00
Addressing code review
This commit is contained in:
Dan g
parent
48629e6a38
commit
0fc46f82b4
3 changed files with 39 additions and 46 deletions
|
|
@ -104,7 +104,7 @@ class StatsDCollector(object):
|
|||
|
||||
:param str host: The StatsD host
|
||||
:param str port: The StatsD port
|
||||
:param bool tcp: Flag to set a TCP or UDP client
|
||||
:param str protocol: lowercase udp or tcp
|
||||
:param str namespace: The StatsD bucket to write metrics into.
|
||||
:param bool prepend_metric_type: Optional flag to prepend bucket path
|
||||
with the StatsD metric type
|
||||
|
|
@ -113,7 +113,7 @@ class StatsDCollector(object):
|
|||
METRIC_TYPES = {'c': 'counters',
|
||||
'ms': 'timers'}
|
||||
|
||||
def __init__(self, host, port, proto='udp', namespace='sprockets',
|
||||
def __init__(self, host, port, protocol='udp', namespace='sprockets',
|
||||
prepend_metric_type=True):
|
||||
self._host = host
|
||||
self._port = int(port)
|
||||
|
|
@ -121,12 +121,14 @@ class StatsDCollector(object):
|
|||
self._namespace = namespace
|
||||
self._prepend_metric_type = prepend_metric_type
|
||||
|
||||
if proto == 'udp':
|
||||
if protocol == 'tcp':
|
||||
self._tcp = True
|
||||
self._sock = self._tcp_socket()
|
||||
elif protocol == 'udp':
|
||||
self._tcp = False
|
||||
self._sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM, 0)
|
||||
else:
|
||||
self._tcp = True
|
||||
self._sock = self._tcp_socket()
|
||||
raise ValueError('Invalid protocol: {}'.format(protocol))
|
||||
|
||||
def _tcp_socket(self):
|
||||
"""Connect to statsd via TCP and return the IOStream handle.
|
||||
|
|
@ -161,18 +163,15 @@ class StatsDCollector(object):
|
|||
msg = '{0}:{1}|{2}'.format(
|
||||
self._build_path(path, metric_type), value, metric_type)
|
||||
|
||||
encoding = 'utf-8'
|
||||
#encoding = 'ascii'
|
||||
|
||||
try:
|
||||
LOGGER.debug('Sending %s to %s:%s', msg.encode(encoding),
|
||||
LOGGER.debug('Sending %s to %s:%s', msg.encode('ascii'),
|
||||
self._host, self._port)
|
||||
if self._tcp:
|
||||
return self._sock.write(msg.encode(encoding))
|
||||
else:
|
||||
self._sock.sendto(msg.encode(encoding), (self._host, self._port))
|
||||
except socket.error:
|
||||
LOGGER.exception('Error sending StatsD metrics')
|
||||
return self._sock.write(msg.encode('ascii'))
|
||||
|
||||
self._sock.sendto(msg.encode('ascii'), (self._host, self._port))
|
||||
except (OSError, socket.error) as error: # pragma: nocover
|
||||
LOGGER.exception('Error sending statsd metric: %s', error)
|
||||
|
||||
def _build_path(self, path, metric_type):
|
||||
"""Return a normalized path.
|
||||
|
|
@ -230,14 +229,8 @@ def install(application, **kwargs):
|
|||
if 'port' not in kwargs:
|
||||
kwargs['port'] = os.environ.get('STATSD_PORT', '8125')
|
||||
|
||||
if 'proto' not in kwargs:
|
||||
if "STATSD_PROTO" in os.environ:
|
||||
kwargs['proto'] = os.environ.get('STATSD_PROTO', 'udp')
|
||||
else:
|
||||
kwargs['proto'] = 'udp'
|
||||
|
||||
if kwargs['proto'] not in ['udp', 'tcp']:
|
||||
raise ValueError('Invalid value for STATSD_PROTO: {}'.format(kwargs['proto']))
|
||||
if 'protocol' not in kwargs:
|
||||
kwargs['protocol'] = os.environ.get('STATSD_PROTOCOL', 'udp')
|
||||
|
||||
setattr(application, 'statsd', StatsDCollector(**kwargs))
|
||||
return True
|
||||
|
|
|
|||
|
|
@ -39,32 +39,32 @@ class FakeTCPStatsdServer(tcpserver.TCPServer):
|
|||
return
|
||||
|
||||
def find_metrics(self, prefix, metric_type):
|
||||
"""
|
||||
Yields captured datagrams that start with `prefix`.
|
||||
"""
|
||||
Yields captured datagrams that start with `prefix`.
|
||||
|
||||
:param str prefix: the metric prefix to search for
|
||||
:param str metric_type: the statsd metric type (e.g., 'ms', 'c')
|
||||
:returns: yields (path, value, metric_type) tuples for each
|
||||
captured metric that matches
|
||||
:raises AssertionError: if no metrics match.
|
||||
:param str prefix: the metric prefix to search for
|
||||
:param str metric_type: the statsd metric type (e.g., 'ms', 'c')
|
||||
:returns: yields (path, value, metric_type) tuples for each
|
||||
captured metric that matches
|
||||
:raises AssertionError: if no metrics match.
|
||||
|
||||
"""
|
||||
pattern = re.compile(
|
||||
"""
|
||||
pattern = re.compile(
|
||||
'(?P<path>{}[^:]*):(?P<value>[^|]*)\\|(?P<type>{})'.format(
|
||||
re.escape(prefix), re.escape(metric_type)))
|
||||
matched = False
|
||||
matched = False
|
||||
|
||||
for datagram in self.datagrams:
|
||||
text_msg = datagram.decode('ascii')
|
||||
match = pattern.match(text_msg)
|
||||
if match:
|
||||
yield match.groups()
|
||||
matched = True
|
||||
for datagram in self.datagrams:
|
||||
text_msg = datagram.decode('ascii')
|
||||
match = pattern.match(text_msg)
|
||||
if match:
|
||||
yield match.groups()
|
||||
matched = True
|
||||
|
||||
if not matched:
|
||||
raise AssertionError(
|
||||
'Expected metric starting with "{}" in {!r}'.format(
|
||||
prefix, self.datagrams))
|
||||
if not matched:
|
||||
raise AssertionError(
|
||||
'Expected metric starting with "{}" in {!r}'.format(
|
||||
prefix, self.datagrams))
|
||||
|
||||
|
||||
class FakeUDPStatsdServer(object):
|
||||
|
|
|
|||
8
tests.py
8
tests.py
|
|
@ -59,7 +59,7 @@ class TCPStatsdMetricCollectionTests(testing.AsyncHTTPTestCase):
|
|||
statsd.install(self.application, **{'namespace': 'testing',
|
||||
'host': self.statsd.sockaddr[0],
|
||||
'port': self.statsd.sockaddr[1],
|
||||
'proto': 'tcp',
|
||||
'protocol': 'tcp',
|
||||
'prepend_metric_type': True})
|
||||
|
||||
def test_that_http_method_call_is_recorded(self):
|
||||
|
|
@ -125,7 +125,7 @@ class TCPStatsdConfigurationTests(testing.AsyncHTTPTestCase):
|
|||
statsd.install(self.application, **{'namespace': 'testing',
|
||||
'host': self.statsd.sockaddr[0],
|
||||
'port': self.statsd.sockaddr[1],
|
||||
'proto': 'tcp',
|
||||
'protocol': 'tcp',
|
||||
'prepend_metric_type': False})
|
||||
|
||||
def test_that_http_method_call_is_recorded(self):
|
||||
|
|
@ -163,7 +163,7 @@ class UDPStatsdMetricCollectionTests(testing.AsyncHTTPTestCase):
|
|||
statsd.install(self.application, **{'namespace': 'testing',
|
||||
'host': self.statsd.sockaddr[0],
|
||||
'port': self.statsd.sockaddr[1],
|
||||
'proto': 'udp',
|
||||
'protocol': 'udp',
|
||||
'prepend_metric_type': True})
|
||||
|
||||
def tearDown(self):
|
||||
|
|
@ -233,7 +233,7 @@ class UDPStatsdConfigurationTests(testing.AsyncHTTPTestCase):
|
|||
statsd.install(self.application, **{'namespace': 'testing',
|
||||
'host': self.statsd.sockaddr[0],
|
||||
'port': self.statsd.sockaddr[1],
|
||||
'proto': 'udp',
|
||||
'protocol': 'udp',
|
||||
'prepend_metric_type': False})
|
||||
|
||||
def tearDown(self):
|
||||
|
|
|
|||
Loading…
Reference in a new issue