openstack/designate
Code Issues Proposed changes

Simplified metrics implementation

Browse Source 
If monascastatsd is installed, the current implementation
will always open a socket when starting designate, even if
metrics is disabled in the configuration.

This patch changes this by changing the way the timer was
implemented, making sure that the metrics implemention
is only loaded when needed.

* Made the error message obvious when metrics is disabled,
  and when we are missing the monasca-statsd client.
* Removed support for the timed decorator.

Change-Id: I508269ae3baac9882a51fb61ce55d6169ffddad3
This commit is contained in:
Erik Olof Gunnar Andersson
2019-05-19 01:09:04 -07:00
parent b81755750a
commit 7acd220c44
7 changed files with 133 additions and 137 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
designate/mdns/notify.py
View File

@@ -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):

31
designate/mdns/xfr.py
View File

@@ -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):

102
designate/metrics.py
View File

@@ -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()

10
designate/metrics_client/noop.py
View File

@@ -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

3
designate/service.py
View File

@@ -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)

0
designate/tests/unit/test_metrics/__init__.py → designate/tests/unit/metrics/__init__.py
View File

110
designate/tests/unit/test_metrics/test_metrics.py → designate/tests/unit/metrics/test_metrics.py
View File

@@ -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