From e76c20d1c57b571ca19f9863057ff91e09aa4301 Mon Sep 17 00:00:00 2001 From: Dantali0n Date: Thu, 9 May 2019 17:00:06 +0200 Subject: [PATCH] Improve exceptions and logging in ds manager MetricNotAvailable and NoDatasourceAvailable allow to differentiate between having no datasources configured and a required metric being unavailable from the datasource. Both exceptions have comments so that the use case is clear. The input validation of the get_backend method in the datasource manager is improved. Additional logging information allows to identify which metric caused the available datasource to be discarded. Tests are updated to validate the correct functionality of the new exceptions. Change-Id: I512976cce2401dbcd249d42686b78843e111a0e7 --- watcher/common/exception.py | 18 ++++-- watcher/datasources/manager.py | 20 ++++++- watcher/tests/datasources/test_manager.py | 67 +++++++++++++++-------- 3 files changed, 76 insertions(+), 29 deletions(-) diff --git a/watcher/common/exception.py b/watcher/common/exception.py index 90d87dab8..f962b3bbe 100644 --- a/watcher/common/exception.py +++ b/watcher/common/exception.py @@ -354,10 +354,6 @@ class IllegalArgumentException(WatcherException): msg_fmt = _('Illegal argument') -class NoSuchMetric(WatcherException): - msg_fmt = _('No such metric') - - class AuthorizationFailure(WatcherException): msg_fmt = _('%(client)s connection failed. Reason: %(reason)s') @@ -397,6 +393,20 @@ class DataSourceNotAvailable(WatcherException): msg_fmt = _("Datasource %(datasource)s is not available.") +class MetricNotAvailable(WatcherException): + """Indicate that a metric is not configured or does not exists""" + msg_fmt = _('Metric: %(metric)s not available') + + +class NoDatasourceAvailable(WatcherException): + """No datasources have been configured""" + msg_fmt = _('No datasources available') + + +class NoSuchMetricForHost(WatcherException): + msg_fmt = _("No %(metric)s metric for %(host)s found.") + + class ServiceAlreadyExists(Conflict): msg_fmt = _("A service with name %(name)s is already working on %(host)s.") diff --git a/watcher/datasources/manager.py b/watcher/datasources/manager.py index 77a1feb81..682a5d488 100644 --- a/watcher/datasources/manager.py +++ b/watcher/datasources/manager.py @@ -87,12 +87,30 @@ class DataSourceManager(object): self._gnocchi = gnocchi def get_backend(self, metrics): + """Determine the datasource to use from the configuration + + Iterates over the configured datasources in order to find the first + which can support all specified metrics. Upon a missing metric the next + datasource is attempted. + """ + + if not self.datasources or len(self.datasources) is 0: + raise exception.NoDatasourceAvailable + + if not metrics or len(metrics) is 0: + LOG.critical("Can not retrieve datasource without specifying" + "list of required metrics.") + raise exception.InvalidParameter(parameter='metrics', + parameter_type='none empty list') + for datasource in self.datasources: no_metric = False for metric in metrics: if (metric not in self.metric_map[datasource] or self.metric_map[datasource].get(metric) is None): no_metric = True + LOG.warning("Datasource: {0} could not be used due to" + "metric: {1}".format(datasource, metric)) break if not no_metric: # Try to use a specific datasource but attempt additional @@ -103,7 +121,7 @@ class DataSourceManager(object): return ds except Exception: pass - raise exception.NoSuchMetric() + raise exception.MetricNotAvailable(metric=metric) def load_metric_map(self, file_path): """Load metrics from the metric_map_path""" diff --git a/watcher/tests/datasources/test_manager.py b/watcher/tests/datasources/test_manager.py index 7494bd3d5..a295f9944 100644 --- a/watcher/tests/datasources/test_manager.py +++ b/watcher/tests/datasources/test_manager.py @@ -15,6 +15,7 @@ # limitations under the License. import mock +import six from mock import MagicMock @@ -38,30 +39,6 @@ class TestDataSourceManager(base.BaseTestCase): opts.update(kwargs) return ds_manager.DataSourceManager(**opts) - def test_get_backend(self): - manager = self._dsm() - backend = manager.get_backend(['host_cpu_usage', 'instance_cpu_usage']) - self.assertEqual(backend, manager.gnocchi) - - def test_get_backend_order(self): - dss = ['monasca', 'ceilometer', 'gnocchi'] - dsmcfg = self._dsm_config(datasources=dss) - manager = self._dsm(config=dsmcfg) - backend = manager.get_backend(['host_cpu_usage', 'instance_cpu_usage']) - self.assertEqual(backend, manager.monasca) - - def test_get_backend_wrong_metric(self): - manager = self._dsm() - self.assertRaises(exception.NoSuchMetric, manager.get_backend, - ['host_cpu', 'instance_cpu_usage']) - - @mock.patch.object(gnocchi, 'GnocchiHelper') - def test_get_backend_error_datasource(self, m_gnocchi): - m_gnocchi.side_effect = exception.DataSourceNotAvailable - manager = self._dsm() - backend = manager.get_backend(['host_cpu_usage', 'instance_cpu_usage']) - self.assertEqual(backend, manager.ceilometer) - def test_metric_file_path_not_exists(self): manager = self._dsm() expected = ds_manager.DataSourceManager.metric_map @@ -85,3 +62,45 @@ class TestDataSourceManager(base.BaseTestCase): mo.return_value = {"newds": {"metric_one": "i_am_metric_one"}} mgr = self._dsm() self.assertNotIn('newds', mgr.metric_map.keys()) + + def test_get_backend(self): + manager = self._dsm() + backend = manager.get_backend(['host_cpu_usage', 'instance_cpu_usage']) + self.assertEqual(backend, manager.gnocchi) + + def test_get_backend_order(self): + dss = ['monasca', 'ceilometer', 'gnocchi'] + dsmcfg = self._dsm_config(datasources=dss) + manager = self._dsm(config=dsmcfg) + backend = manager.get_backend(['host_cpu_usage', 'instance_cpu_usage']) + self.assertEqual(backend, manager.monasca) + + def test_get_backend_wrong_metric(self): + manager = self._dsm() + ex = self.assertRaises( + exception.MetricNotAvailable, manager.get_backend, + ['host_cpu', 'instance_cpu_usage']) + self.assertIn('Metric: host_cpu not available', six.text_type(ex)) + + @mock.patch.object(gnocchi, 'GnocchiHelper') + def test_get_backend_error_datasource(self, m_gnocchi): + m_gnocchi.side_effect = exception.DataSourceNotAvailable + manager = self._dsm() + backend = manager.get_backend(['host_cpu_usage', 'instance_cpu_usage']) + self.assertEqual(backend, manager.ceilometer) + + def test_get_backend_no_datasources(self): + dsmcfg = self._dsm_config(datasources=[]) + manager = self._dsm(config=dsmcfg) + self.assertRaises(exception.NoDatasourceAvailable, manager.get_backend, + ['host_cpu_usage', 'instance_cpu_usage']) + dsmcfg = self._dsm_config(datasources=None) + manager = self._dsm(config=dsmcfg) + self.assertRaises(exception.NoDatasourceAvailable, manager.get_backend, + ['host_cpu_usage', 'instance_cpu_usage']) + + def test_get_backend_no_metrics(self): + manager = self._dsm() + self.assertRaises(exception.InvalidParameter, manager.get_backend, []) + self.assertRaises(exception.InvalidParameter, manager.get_backend, + None)