diff --git a/designate/mdns/notify.py b/designate/mdns/notify.py index 328f8819c..4aa017d1e 100644 --- a/designate/mdns/notify.py +++ b/designate/mdns/notify.py @@ -41,7 +41,6 @@ class NotifyEndpoint(base.BaseEndpoint): RPC_API_VERSION = '2.0' RPC_API_NAMESPACE = 'notify' - @metrics.timed('mdns.notify_zone_changed') def notify_zone_changed(self, context, zone, host, port, timeout, retry_interval, max_retries, delay): """ @@ -61,10 +60,15 @@ class NotifyEndpoint(base.BaseEndpoint): current_retry is the current retry number. The return value is just used for testing and not by pool manager. """ - time.sleep(delay) - return self._make_and_send_dns_message( - zone, host, port, timeout, retry_interval, max_retries, - notify=True) + start_time = time.time() + try: + time.sleep(delay) + return self._make_and_send_dns_message( + zone, host, port, timeout, retry_interval, max_retries, + notify=True) + finally: + metrics.timing('mdns.notify_zone_changed', + time.time() - start_time) def poll_for_serial_number(self, context, zone, nameserver, timeout, retry_interval, max_retries, delay): diff --git a/designate/mdns/xfr.py b/designate/mdns/xfr.py index e6c8bf831..11751bb06 100644 --- a/designate/mdns/xfr.py +++ b/designate/mdns/xfr.py @@ -13,6 +13,8 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +import time + from oslo_config import cfg from oslo_utils import timeutils from oslo_log import log as logging @@ -30,24 +32,27 @@ class XFRMixin(object): """ Utility mixin that holds common methods for XFR functionality. """ - @metrics.timed('mdns.xfr.zone_sync') def zone_sync(self, context, zone, servers=None): - servers = servers or zone.masters - servers = servers.to_list() - - timeout = cfg.CONF["service:mdns"].xfr_timeout + start_time = time.time() try: - dnspython_zone = dnsutils.do_axfr(zone.name, servers, - timeout=timeout) - except exceptions.XFRFailure as e: - LOG.warning(e) - return + servers = servers or zone.masters + servers = servers.to_list() - zone.update(dnsutils.from_dnspython_zone(dnspython_zone)) + timeout = cfg.CONF["service:mdns"].xfr_timeout + try: + dnspython_zone = dnsutils.do_axfr(zone.name, servers, + timeout=timeout) + except exceptions.XFRFailure as e: + LOG.warning(e) + return - zone.transferred_at = timeutils.utcnow() + zone.update(dnsutils.from_dnspython_zone(dnspython_zone)) - self.central_api.update_zone(context, zone, increment_serial=False) + zone.transferred_at = timeutils.utcnow() + + self.central_api.update_zone(context, zone, increment_serial=False) + finally: + metrics.timing('mdns.xfr.zone_sync', time.time() - start_time) class XfrEndpoint(base.BaseEndpoint, XFRMixin): diff --git a/designate/metrics.py b/designate/metrics.py index 656f77683..d7418980d 100644 --- a/designate/metrics.py +++ b/designate/metrics.py @@ -11,96 +11,70 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. - -""" -Monasca-Statsd based metrics -~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -Based on metrics-and-stats blueprint - -Usage examples: - -.. code-block:: python - - from designate.metrics import metrics - - @metrics.timed('dot.separated.name') - def your_function(): - pass - - with metrics.time('dot.separated.name'): - pass - - # Increment and decrement a counter. - metrics.counter(name='foo.bar').increment() - metrics.counter(name='foo.bar') -= 10 - -""" - +from oslo_config import cfg from oslo_log import log as logging from oslo_utils import importutils -import designate.conf.metrics +import designate.conf +from designate.metrics_client import noop -stats_client = importutils.import_any('monascastatsd', - 'designate.metrics_client.noop') +monascastatsd = importutils.try_import('monascastatsd') CFG_GROUP_NAME = 'monasca:statsd' CONF = designate.conf.CONF LOG = logging.getLogger(__name__) - # Global metrics client to be imported by other modules metrics = None class Metrics(object): - def __init__(self): - """Initialize Monasca-Statsd client with its default configuration. - Do not start sending metrics yet. - """ - self._client = stats_client.Client(dimensions={ - 'service_name': 'dns' - }) - # cfg.CONF is not available at this time - # Buffer all metrics until init() is called - # https://bugs.launchpad.net/monasca/+bug/1616060 - self._client.connection.open_buffer() - self._client.connection.max_buffer_size = 50000 + self._client = None def init(self): - """Setup client connection or disable metrics based on configuration. - This is called once the cfg.CONF is ready. - """ - conf = CONF[CFG_GROUP_NAME] - if conf.enabled: - LOG.info("Statsd reports to %(host)s %(port)d", - { - 'host': conf.hostname, - 'port': conf.port - }) - self._client.connection._flush_buffer() - self._client.connection.close_buffer() - self._client.connection.connect(conf.hostname, conf.port) + conf = cfg.CONF[CFG_GROUP_NAME] + if conf.enabled and monascastatsd: + LOG.info( + 'Statsd reports to %(host)s:%(port)d', + { + 'host': conf.hostname, + 'port': conf.port + } + ) + self._client = monascastatsd.Client( + host=conf.hostname, port=conf.port, + dimensions={ + 'service_name': 'dns' + }) + return + + if conf.enabled and not monascastatsd: + LOG.error('monasca-statsd client not installed. ' + 'Metrics will be ignored.') else: - LOG.info("Statsd disabled") - # The client cannot be disabled: mock out report() - self._client.connection.report = lambda *a, **kw: None - # There's no clean way to drain the outgoing buffer + LOG.info('Statsd disabled') + + self._client = noop.Client() def counter(self, *a, **kw): - return self._client.get_counter(*a, **kw) + return self.client.get_counter(*a, **kw) def gauge(self, *a, **kw): - return self._client.get_gauge(*a, **kw) + return self.client.get_gauge(*a, **kw) @property - def timed(self): - return self._client.get_timer().timed + def timing(self): + return self.client.get_timer().timing def timer(self): - return self._client.get_timer() + return self.client.get_timer() + + @property + def client(self): + if not self._client: + self.init() + return self._client metrics = Metrics() diff --git a/designate/metrics_client/noop.py b/designate/metrics_client/noop.py index d45ef8565..004193d63 100644 --- a/designate/metrics_client/noop.py +++ b/designate/metrics_client/noop.py @@ -29,10 +29,9 @@ class NoopConnection(object): pass def connect(self, *a, **kw): - LOG.error('Using noop metrics client. Metrics will be ignored.') pass - def open_buffer(self): + def open_buffer(self, *a, **kw): pass @@ -65,10 +64,8 @@ class NoopTimer(object): def __init__(self): pass - def timed(self, *a, **kw): - def wrapper(func): - return func - return wrapper + def timing(self, *a, **kw): + pass class Client(object): @@ -77,7 +74,6 @@ class Client(object): self._gauge = NoopGauge() self._timer = NoopTimer() self.connection = NoopConnection() - pass def get_counter(self, *a, **kw): return self._counter diff --git a/designate/service.py b/designate/service.py index 83477eb85..54f5d1914 100644 --- a/designate/service.py +++ b/designate/service.py @@ -64,7 +64,6 @@ class Service(service.Service): self._service_config = CONF['service:%s' % self.service_name] policy.init() - metrics.init() # NOTE(kiall): All services need RPC initialized, as this is used # for clients AND servers. Hence, this is common to @@ -253,6 +252,8 @@ class DNSService(object): def __init__(self, *args, **kwargs): super(DNSService, self).__init__(*args, **kwargs) + metrics.init() + # Eventet will complain loudly about our use of multiple greentheads # reading/writing to the UDP socket at once. Disable this warning. eventlet.debug.hub_prevent_multiple_readers(False) diff --git a/designate/tests/unit/test_metrics/__init__.py b/designate/tests/unit/metrics/__init__.py similarity index 100% rename from designate/tests/unit/test_metrics/__init__.py rename to designate/tests/unit/metrics/__init__.py diff --git a/designate/tests/unit/test_metrics/test_metrics.py b/designate/tests/unit/metrics/test_metrics.py similarity index 52% rename from designate/tests/unit/test_metrics/test_metrics.py rename to designate/tests/unit/metrics/test_metrics.py index 44ab16f9f..d5c5d1839 100644 --- a/designate/tests/unit/test_metrics/test_metrics.py +++ b/designate/tests/unit/metrics/test_metrics.py @@ -13,97 +13,113 @@ # License for the specific language governing permissions and limitations # under the License. # +import time import mock import monascastatsd - -from designate.metrics import Metrics -from designate.metrics_client import noop -from designate.tests import fixtures -from designate.tests import TestCase - from oslo_config import cfg from oslo_config import fixture as cfg_fixture +from designate import metrics +from designate.metrics_client import noop +from designate.tests import TestCase +from designate.tests import fixtures + class TestNoopMetrics(TestCase): - def setUp(self): super(TestCase, self).setUp() self.stdlog = fixtures.StandardLogging() self.useFixture(self.stdlog) self.CONF = self.useFixture(cfg_fixture.Config(cfg.CONF)).conf - self.metrics = Metrics() - self.metrics._client = noop.Client() + self.CONF.set_override('enabled', False, 'monasca:statsd') - def test_noop_metrics_enabled(self): - self.CONF.set_override('enabled', True, 'monasca:statsd') - self.metrics.init() - self.assertIn("Using noop metrics client. Metrics will be ignored.", - self.stdlog.logger.output) - - def test_noop_metrics_disabled(self): - with mock.patch('designate.metrics_client.noop.LOG') as log_mock: - self.metrics.init() - log_mock.error.assert_not_called() + def test_monasca_metrics_disabled(self): + self.metrics = metrics.Metrics() + self.assertIsInstance(self.metrics.client, noop.Client) + self.assertIn('Statsd disabled', self.stdlog.logger.output) def test_noop_metrics_client_getters(self): - self.CONF.set_override('enabled', True, 'monasca:statsd') - self.metrics.init() + self.metrics = metrics.Metrics() self.assertIsInstance(self.metrics.counter('name'), noop.NoopCounter) self.assertIsInstance(self.metrics.gauge(), noop.NoopGauge) self.assertIsInstance(self.metrics.timer(), noop.NoopTimer) - self.assertIsNotNone(self.metrics.timed.__self__) + self.assertIsNotNone(self.metrics.timer.__self__) def test_noop_metrics_client_timed(self): - timer = self.metrics._client.get_timer() + self.metrics = metrics.Metrics() + timer = self.metrics.client.get_timer() - @timer.timed('timed.test') def func(a): - return a + start_time = time.time() + try: + return a + finally: + timer.timing('mdns.xfr.zone_sync', time.time() - start_time) + result = func(1) self.assertEqual(result, 1) class TestMonascaMetrics(TestCase): - def setUp(self): super(TestCase, self).setUp() self.stdlog = fixtures.StandardLogging() self.useFixture(self.stdlog) self.CONF = self.useFixture(cfg_fixture.Config(cfg.CONF)).conf - self.metrics = Metrics() - - def test_monasca_metrics_enabled(self): self.CONF.set_override('enabled', True, 'monasca:statsd') - self.metrics.init() - self.assertIn("Statsd reports to 127.0.0.1 8125", - self.stdlog.logger.output) - - def test_monasca_metrics_disabled(self): - self.metrics.init() - self.assertIn( - "Statsd disabled", - self.stdlog.logger.output) @mock.patch('socket.socket.connect') - @mock.patch('socket.socket.send') - def test_monasca_metrics_client_getters(self, conn_mock, send_mock): - self.CONF.set_override('enabled', True, 'monasca:statsd') - self.metrics.init() + def test_monasca_metrics_enabled(self, conn_mock): + self.metrics = metrics.Metrics() + + self.assertIsInstance(self.metrics.client, monascastatsd.client.Client) + self.assertIn('Statsd reports to 127.0.0.1:8125', + self.stdlog.logger.output) + self.assertTrue(conn_mock.called) + + @mock.patch('socket.socket.connect') + def test_monasca_metrics_client_getters(self, conn_mock): + self.metrics = metrics.Metrics() + self.assertIsInstance(self.metrics.counter('name'), monascastatsd.counter.Counter) self.assertIsInstance(self.metrics.gauge(), monascastatsd.gauge.Gauge) self.assertIsInstance(self.metrics.timer(), monascastatsd.timer.Timer) - self.assertIsNotNone(self.metrics.timed.__self__) + self.assertIsNotNone(self.metrics.timer.__self__) - def test_monasca_metrics_client_timed(self): - timer = self.metrics._client.get_timer() + self.assertTrue(conn_mock.called) + + @mock.patch('socket.socket.send') + @mock.patch('socket.socket.connect') + def test_monasca_metrics_client_timed(self, conn_mock, send_mock): + self.metrics = metrics.Metrics() + timer = self.metrics.client.get_timer() - @timer.timed('timed.test') def func(a): - return a + start_time = time.time() + try: + return a + finally: + timer.timing('mdns.xfr.zone_sync', time.time() - start_time) + result = func(1) self.assertEqual(result, 1) + self.assertTrue(conn_mock.called) + self.assertTrue(send_mock.called) + + def test_monasca_enabled_but_client_not_installed(self): + restore = metrics.monascastatsd + try: + metrics.monascastatsd = None + self.metrics = metrics.Metrics() + self.assertIsInstance(self.metrics.client, noop.Client) + self.assertIn( + 'monasca-statsd client not installed. ' + 'Metrics will be ignored.', + self.stdlog.logger.output + ) + finally: + metrics.monascastatsd = restore