Don't raise NoDataCollected in case of collect error

This makes the different collectors raise collect exception in case of a
collect error.

Work items:

* The Prometheus collector raises a PrometheusConfigError instead of a
  NoDataCollected exception when the provided query URL is invalid.

* The collectors have been simplified: it is the BaseCollector which raises
  a NoDataCollected exception in case nothing was found. Collectors can
  return an empty iterable or None.

Change-Id: I65bc9ffb2618673e6fa42464dd841f960a7a0af1
Story: 2003828
Task: 26762

cloudkitty/collector/__init__.py

@@ -245,11 +245,10 @@ class BaseCollector(object):
         )
 
         name = self.conf[metric_name].get('alt_name', metric_name)
-        if data:
-            data = self.t_cloudkitty.format_service(name, data)
         if not data:
             raise NoDataCollected(self.collector_name, name)
-        return data
+
+        return self.t_cloudkitty.format_service(name, data)
 
 
 def validate_conf(conf):

cloudkitty/collector/exceptions.py

@@ -0,0 +1,19 @@
+# Copyright 2018 Objectif Libre
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+#
+
+
+class CollectError(Exception):
+    """Base exception for collect errors"""
+    pass

cloudkitty/collector/monasca.py

@@ -95,8 +95,14 @@ class MonascaCollector(collector.BaseCollector):
         output = dict()
         for metric_name, metric in conf['metrics'].items():
             output[metric_name] = metric_schema(metric)
+            groupby = output[metric_name]['groupby']
+
             if scope_key not in output[metric_name]['groupby']:
-                output[metric_name]['groupby'].append(scope_key)
+                groupby.append(scope_key)
+            resource_key = conf[metric_name]['extra_args']['resource_key']
+            if resource_key not in groupby:
+                groupby.append(resource_key)
+
         return output
 
     def __init__(self, transformers, **kwargs):
@@ -178,12 +184,6 @@ class MonascaCollector(collector.BaseCollector):
 
         dimensions = self._get_dimensions(metric_name, project_id, q_filter)
         group_by = self.conf[metric_name]['groupby']
-        resource_key = self.conf[metric_name]['extra_args']['resource_key']
-        if resource_key not in group_by:
-            LOG.error('Resource key "{}" is not in group_by keys: "{}". '
-                      'Please adapt your configuration.'.format(
-                          resource_key, group_by))
-            raise collector.NoDataCollected(self.collector_name, metric_name)
 
         # NOTE(lpeschke): One aggregated measure per collect period
         period = end - start

cloudkitty/collector/prometheus.py

@@ -28,6 +28,7 @@ from voluptuous import Required
 from voluptuous import Schema
 
 from cloudkitty import collector
+from cloudkitty.collector import exceptions as collect_exceptions
 from cloudkitty import utils as ck_utils
 
 
@@ -52,6 +53,10 @@ PROMETHEUS_EXTRA_SCHEMA = {
 }
 
 
+class PrometheusConfigError(collect_exceptions.CollectError):
+    pass
+
+
 class PrometheusClient(object):
     @classmethod
     def build_query(cls, source, query, start, end, period, metric_name):
@@ -65,10 +70,8 @@ class PrometheusClient(object):
                     query, {'period': str(period) + 's'},
                 )
             except (KeyError, ValueError):
-                raise collector.NoDataCollected(
-                    collector.collector_name,
-                    metric_name
-                )
+                raise PrometheusConfigError(
+                    'Invalid prometheus query: {}'.format(query))
 
         # Due to the design of Cloudkitty, only instant queries are supported.
         # In that case 'time' equals 'end' and
@@ -151,9 +154,9 @@ class PrometheusCollector(collector.BaseCollector):
         )
 
         # If the query returns an empty dataset,
-        # raise a NoDataCollected exception.
+        # return an empty list
         if not res['data']['result']:
-            raise collector.NoDataCollected(self.collector_name, metric_name)
+            return []
 
         formatted_resources = []
 

cloudkitty/tests/collectors/test_prometheus.py

@@ -168,3 +168,16 @@ class PrometheusClientTest(tests.TestCase):
         }
         actual = self.client.build_query(**params)
         self.assertEqual(expected, actual)
+
+    def test_build_query_raises_PrometheusConfigError(self):
+        class InvalidPeriod(object):
+            def __str__(self):
+                raise ValueError
+
+        period = InvalidPeriod()
+
+        self.assertRaises(
+            prometheus.PrometheusConfigError,
+            self.client.build_query,
+            None, '$period', 0, 0, period, 'broken_metric',
+        )