diff --git a/swift/common/statsd_client.py b/swift/common/statsd_client.py index d8e05f0dc1..492d264bbf 100644 --- a/swift/common/statsd_client.py +++ b/swift/common/statsd_client.py @@ -35,9 +35,28 @@ class StatsdClient(object): self._sample_rate_factor = sample_rate_factor self.random = random self.logger = logger + self._sock_family = self._target = None + if self._host: + self._set_sock_family_and_target(host, port) + + def _set_sock_family_and_target(self, host, port): # Determine if host is IPv4 or IPv6 - addr_info, self._sock_family = self._determine_sock_family(host, port) + addr_info = None + try: + addr_info = socket.getaddrinfo(host, port, socket.AF_INET) + self._sock_family = socket.AF_INET + except socket.gaierror: + try: + addr_info = socket.getaddrinfo(host, port, socket.AF_INET6) + self._sock_family = socket.AF_INET6 + except socket.gaierror: + # Don't keep the server from starting from what could be a + # transient DNS failure. Any hostname will get re-resolved as + # necessary in the .sendto() calls. + # However, we don't know if we're IPv4 or IPv6 in this case, so + # we assume legacy IPv4. + self._sock_family = socket.AF_INET # NOTE: we use the original host value, not the DNS-resolved one # because if host is a hostname, we don't want to cache the DNS @@ -57,24 +76,6 @@ class StatsdClient(object): else: self._target = (host, port) - def _determine_sock_family(self, host, port): - addr_info = sock_family = None - try: - addr_info = socket.getaddrinfo(host, port, socket.AF_INET) - sock_family = socket.AF_INET - except socket.gaierror: - try: - addr_info = socket.getaddrinfo(host, port, socket.AF_INET6) - sock_family = socket.AF_INET6 - except socket.gaierror: - # Don't keep the server from starting from what could be a - # transient DNS failure. Any hostname will get re-resolved as - # necessary in the .sendto() calls. - # However, we don't know if we're IPv4 or IPv6 in this case, so - # we assume legacy IPv4. - sock_family = socket.AF_INET - return addr_info, sock_family - def _set_prefix(self, tail_prefix): """ Modifies the prefix that is added to metric names. The resulting prefix @@ -115,6 +116,10 @@ class StatsdClient(object): self._set_prefix(tail_prefix) def _send(self, m_name, m_value, m_type, sample_rate): + if not self._host: + # StatsD not configured + return + if sample_rate is None: sample_rate = self._default_sample_rate sample_rate = sample_rate * self._sample_rate_factor diff --git a/swift/common/utils/logs.py b/swift/common/utils/logs.py index 3e918a34e4..35c161fb6f 100644 --- a/swift/common/utils/logs.py +++ b/swift/common/utils/logs.py @@ -735,20 +735,17 @@ def get_logger(conf, name=None, log_to_console=False, log_route=None, # Setup logger with a StatsD client if so configured statsd_host = conf.get('log_statsd_host') - if statsd_host: - statsd_port = int(conf.get('log_statsd_port', 8125)) - base_prefix = conf.get('log_statsd_metric_prefix', '') - default_sample_rate = float(conf.get( - 'log_statsd_default_sample_rate', 1)) - sample_rate_factor = float(conf.get( - 'log_statsd_sample_rate_factor', 1)) - if statsd_tail_prefix is None: - statsd_tail_prefix = name - logger.statsd_client = statsd_client.StatsdClient( - statsd_host, statsd_port, base_prefix, statsd_tail_prefix, - default_sample_rate, sample_rate_factor, logger=logger) - else: - logger.statsd_client = None + statsd_port = int(conf.get('log_statsd_port', 8125)) + base_prefix = conf.get('log_statsd_metric_prefix', '') + default_sample_rate = float(conf.get( + 'log_statsd_default_sample_rate', 1)) + sample_rate_factor = float(conf.get( + 'log_statsd_sample_rate_factor', 1)) + if statsd_tail_prefix is None: + statsd_tail_prefix = name + logger.statsd_client = statsd_client.StatsdClient( + statsd_host, statsd_port, base_prefix, statsd_tail_prefix, + default_sample_rate, sample_rate_factor, logger=logger) adapted_logger = LogAdapter(logger, name) other_handlers = conf.get('log_custom_handlers', None) diff --git a/test/debug_logger.py b/test/debug_logger.py index 52e39777ca..b59dfe9e70 100644 --- a/test/debug_logger.py +++ b/test/debug_logger.py @@ -149,7 +149,7 @@ class FakeLogger(logging.Logger, CaptureLog): self.level = logging.NOTSET if 'facility' in kwargs: self.facility = kwargs['facility'] - self.statsd_client = FakeStatsdClient("host", 8125) + self.statsd_client = FakeStatsdClient(None, 8125) self.thread_locals = None self.parent = None # ensure the NOTICE level has been named, in case it has not already diff --git a/test/unit/common/middleware/s3api/test_s3api.py b/test/unit/common/middleware/s3api/test_s3api.py index b5f715d990..39fd39a3a2 100644 --- a/test/unit/common/middleware/s3api/test_s3api.py +++ b/test/unit/common/middleware/s3api/test_s3api.py @@ -223,7 +223,8 @@ class TestS3ApiMiddleware(S3ApiTestCase): self.assertEqual('s3api', s3api.logger.logger.name) self.assertIsNot(s3api.logger.logger, proxy_logger) self.assertEqual('swift', s3api.logger.server) - self.assertIsNone(s3api.logger.logger.statsd_client) + # there's a stats client, but with no host, it can't send anything + self.assertIsNone(s3api.logger.logger.statsd_client._host) with mock.patch('swift.common.statsd_client.StatsdClient', FakeStatsdClient): diff --git a/test/unit/common/middleware/test_tempauth.py b/test/unit/common/middleware/test_tempauth.py index 3cdb1b5ffd..5ed83953f9 100644 --- a/test/unit/common/middleware/test_tempauth.py +++ b/test/unit/common/middleware/test_tempauth.py @@ -100,7 +100,7 @@ class TestAuth(unittest.TestCase): def test_statsd_prefix(self): app = FakeApp() - ath = auth.filter_factory({'log_statsd_host': 'example.com'})(app) + ath = auth.filter_factory({})(app) self.assertIsNotNone(ath.logger.logger.statsd_client) self.assertIsInstance(ath.logger.logger.statsd_client, StatsdClient) @@ -108,8 +108,7 @@ class TestAuth(unittest.TestCase): ath.logger.logger.statsd_client._prefix) ath = auth.filter_factory({'log_statsd_metric_prefix': 'foo', - 'log_name': 'bar', - 'log_statsd_host': 'example.com'})(app) + 'log_name': 'bar'})(app) self.assertIsNotNone(ath.logger.logger.statsd_client) self.assertIsInstance(ath.logger.logger.statsd_client, StatsdClient) @@ -118,7 +117,6 @@ class TestAuth(unittest.TestCase): ath = auth.filter_factory({'log_statsd_metric_prefix': 'foo', 'log_name': 'bar', - 'log_statsd_host': 'example.com', 'reseller_prefix': 'TEST'})(app) self.assertIsNotNone(ath.logger.logger.statsd_client) self.assertIsInstance(ath.logger.logger.statsd_client, diff --git a/test/unit/common/test_statsd_client.py b/test/unit/common/test_statsd_client.py index 16b8ee8a86..12329f2118 100644 --- a/test/unit/common/test_statsd_client.py +++ b/test/unit/common/test_statsd_client.py @@ -30,16 +30,34 @@ from mock import patch from swift.common import utils from swift.common import statsd_client +from swift.common.statsd_client import StatsdClient from test.debug_logger import debug_logger -from test.unit.common.test_utils import MockUdpSocket from swift.common.swob import Response -class TestStatsdLogging(unittest.TestCase): +class MockUdpSocket(object): + def __init__(self, sendto_errno=None): + self.sent = [] + self.sendto_errno = sendto_errno + + def sendto(self, data, target): + if self.sendto_errno: + raise socket.error(self.sendto_errno, + 'test errno %s' % self.sendto_errno) + self.sent.append((data, target)) + return len(data) + + def close(self): + pass + + +class BaseTestStasdClient(unittest.TestCase): def setUp(self): + self.getaddrinfo_calls = [] def fake_getaddrinfo(host, port, *args): + self.getaddrinfo_calls.append((host, port)) # this is what a real getaddrinfo('localhost', port, # socket.AF_INET) returned once return [(socket.AF_INET, # address family @@ -59,10 +77,43 @@ class TestStatsdLogging(unittest.TestCase): self.mock_getaddrinfo = self.getaddrinfo_patcher.start() self.addCleanup(self.getaddrinfo_patcher.stop) + +class TestStatsdClient(BaseTestStasdClient): + def test_init_host(self): + client = StatsdClient('myhost', 1234) + self.assertEqual([('myhost', 1234)], self.getaddrinfo_calls) + with mock.patch.object(client, '_open_socket') as mock_open: + self.assertIs(client.increment('tunafish'), + mock_open.return_value.sendto.return_value) + self.assertEqual(mock_open.mock_calls, [ + mock.call(), + mock.call().sendto(b'tunafish:1|c', ('myhost', 1234)), + mock.call().close(), + ]) + + def test_init_host_is_none(self): + client = StatsdClient(None, None) + self.assertIsNone(client._host) + self.assertFalse(self.getaddrinfo_calls) + with mock.patch.object(client, '_open_socket') as mock_open: + self.assertIsNone(client.increment('tunafish')) + self.assertFalse(mock_open.mock_calls) + self.assertFalse(self.getaddrinfo_calls) + + +class TestStatsdLogging(BaseTestStasdClient): def test_get_logger_statsd_client_not_specified(self): logger = utils.get_logger({}, 'some-name', log_route='some-route') - # white-box construction validation - self.assertIsNone(logger.logger.statsd_client) + # white-box construction validation: + # there may be a client instance, but it won't send anything; + # won't even open a socket + self.assertIsInstance(logger.logger.statsd_client, + statsd_client.StatsdClient) + self.assertIsNone(logger.logger.statsd_client._host) + with mock.patch.object(logger.logger.statsd_client, + '_open_socket') as mock_open: + logger.increment('tunafish') + self.assertFalse(mock_open.mock_calls) def test_get_logger_statsd_client_defaults(self): logger = utils.get_logger({'log_statsd_host': 'some.host.com'}, @@ -325,11 +376,12 @@ class TestStatsdLogging(unittest.TestCase): client._open_socket = lambda *_: mock_socket client.random = lambda: 0.50001 - logger.increment('tribbles', sample_rate=0.5) + self.assertIsNone(logger.increment('tribbles', sample_rate=0.5)) self.assertEqual(len(mock_socket.sent), 0) client.random = lambda: 0.49999 - logger.increment('tribbles', sample_rate=0.5) + rv = logger.increment('tribbles', sample_rate=0.5) + self.assertIsInstance(rv, int) self.assertEqual(len(mock_socket.sent), 1) payload = mock_socket.sent[0][0] diff --git a/test/unit/common/test_utils.py b/test/unit/common/test_utils.py index df0e3caaca..829ce7867a 100644 --- a/test/unit/common/test_utils.py +++ b/test/unit/common/test_utils.py @@ -136,21 +136,6 @@ class MockOs(object): return getattr(os, name) -class MockUdpSocket(object): - def __init__(self, sendto_errno=None): - self.sent = [] - self.sendto_errno = sendto_errno - - def sendto(self, data, target): - if self.sendto_errno: - raise socket.error(self.sendto_errno, - 'test errno %s' % self.sendto_errno) - self.sent.append((data, target)) - - def close(self): - pass - - class MockSys(object): def __init__(self):