From afbc5cfdadb5afeef73b176de8527040e59de6a1 Mon Sep 17 00:00:00 2001 From: Luka Peschke Date: Mon, 29 Oct 2018 12:03:02 +0100 Subject: [PATCH] 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 | 5 ++--- cloudkitty/collector/exceptions.py | 19 +++++++++++++++++++ cloudkitty/collector/monasca.py | 14 +++++++------- cloudkitty/collector/prometheus.py | 15 +++++++++------ .../tests/collectors/test_prometheus.py | 13 +++++++++++++ 5 files changed, 50 insertions(+), 16 deletions(-) create mode 100644 cloudkitty/collector/exceptions.py diff --git a/cloudkitty/collector/__init__.py b/cloudkitty/collector/__init__.py index 6b039b48..5717e488 100644 --- a/cloudkitty/collector/__init__.py +++ b/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): diff --git a/cloudkitty/collector/exceptions.py b/cloudkitty/collector/exceptions.py new file mode 100644 index 00000000..101e3837 --- /dev/null +++ b/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 diff --git a/cloudkitty/collector/monasca.py b/cloudkitty/collector/monasca.py index 6e9aa169..aab793d0 100644 --- a/cloudkitty/collector/monasca.py +++ b/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 diff --git a/cloudkitty/collector/prometheus.py b/cloudkitty/collector/prometheus.py index dd6b554c..399298a4 100644 --- a/cloudkitty/collector/prometheus.py +++ b/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 = [] diff --git a/cloudkitty/tests/collectors/test_prometheus.py b/cloudkitty/tests/collectors/test_prometheus.py index dc46d87d..959700b5 100644 --- a/cloudkitty/tests/collectors/test_prometheus.py +++ b/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', + )