diff --git a/cloudkitty/collector/__init__.py b/cloudkitty/collector/__init__.py index 5717e488..3549059f 100644 --- a/cloudkitty/collector/__init__.py +++ b/cloudkitty/collector/__init__.py @@ -193,11 +193,26 @@ class BaseCollector(object): dependency) @staticmethod - def check_configuration(self, conf): - """Check metrics configuration + def check_configuration(conf): + """Checks and validates metric configuration. + Collectors requiring extra parameters for metric collection + should implement this method, call the method of the parent class, + extend the ``extra_args`` key in ``METRIC_BASE_SCHEMA`` and validate + the metric configuration against the new schema. """ - return Schema(METRIC_BASE_SCHEMA)(conf) + conf = Schema(CONF_BASE_SCHEMA)(conf) + metric_schema = Schema(METRIC_BASE_SCHEMA) + + scope_key = CONF.collect.scope_key + + output = {} + for metric_name, metric in conf['metrics'].items(): + output[metric_name] = metric_schema(metric) + if scope_key not in output[metric_name]['groupby']: + output[metric_name]['groupby'].append(scope_key) + + return output @staticmethod def last_month(): @@ -231,6 +246,15 @@ class BaseCollector(object): @abc.abstractmethod def fetch_all(self, metric_name, start, end, project_id=None, q_filter=None): + """Fetches information about a specific metric for a given period. + + This method must respect the ``groupby`` and ``metadata`` arguments + provided in the metric conf at initialization. + (Available in ``self.conf['groupby']`` and ``self.conf['metadata']``). + + Returns a list of items formatted with + ``CloudKittyFormatTransformer.format_item``. + """ pass def retrieve(self, metric_name, start, end, diff --git a/cloudkitty/collector/gnocchi.py b/cloudkitty/collector/gnocchi.py index 0cca566f..95147a97 100644 --- a/cloudkitty/collector/gnocchi.py +++ b/cloudkitty/collector/gnocchi.py @@ -135,20 +135,17 @@ class GnocchiCollector(collector.BaseCollector): """Check metrics configuration """ - conf = Schema(collector.CONF_BASE_SCHEMA)(conf) + conf = collector.BaseCollector.check_configuration(conf) metric_schema = Schema(collector.METRIC_BASE_SCHEMA).extend( GNOCCHI_EXTRA_SCHEMA) - scope_key = CONF.collect.scope_key + output = {} + for metric_name, metric in conf.items(): + met = output[metric_name] = metric_schema(metric) + + if met['extra_args']['resource_key'] not in met['groupby']: + met['groupby'].append(met['extra_args']['resource_key']) - output = dict() - for metric_name, metric in conf['metrics'].items(): - output[metric_name] = metric_schema(metric) - output[metric_name]['groupby'].append( - output[metric_name]['extra_args']['resource_key'] - ) - if scope_key not in output[metric_name]['groupby']: - output[metric_name]['groupby'].append(scope_key) return output @classmethod diff --git a/cloudkitty/collector/monasca.py b/cloudkitty/collector/monasca.py index aab793d0..fce4fd95 100644 --- a/cloudkitty/collector/monasca.py +++ b/cloudkitty/collector/monasca.py @@ -83,25 +83,16 @@ class MonascaCollector(collector.BaseCollector): @staticmethod def check_configuration(conf): - """Check metrics configuration - - """ - conf = Schema(collector.CONF_BASE_SCHEMA)(conf) + conf = collector.BaseCollector.check_configuration(conf) metric_schema = Schema(collector.METRIC_BASE_SCHEMA).extend( MONASCA_EXTRA_SCHEMA) - scope_key = CONF.collect.scope_key + output = {} + for metric_name, metric in conf.items(): + met = output[metric_name] = metric_schema(metric) - 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']: - groupby.append(scope_key) - resource_key = conf[metric_name]['extra_args']['resource_key'] - if resource_key not in groupby: - groupby.append(resource_key) + if met['extra_args']['resource_key'] not in met['groupby']: + met['groupby'].append(met['extra_args']['resource_key']) return output diff --git a/cloudkitty/collector/prometheus.py b/cloudkitty/collector/prometheus.py index 399298a4..8f60c083 100644 --- a/cloudkitty/collector/prometheus.py +++ b/cloudkitty/collector/prometheus.py @@ -100,15 +100,15 @@ class PrometheusCollector(collector.BaseCollector): @staticmethod def check_configuration(conf): - """Check metrics configuration.""" - conf = Schema(collector.CONF_BASE_SCHEMA)(conf) + conf = collector.BaseCollector.check_configuration(conf) metric_schema = Schema(collector.METRIC_BASE_SCHEMA).extend( PROMETHEUS_EXTRA_SCHEMA, ) output = {} - for metric_name, metric in conf['metrics'].items(): + for metric_name, metric in conf.items(): output[metric_name] = metric_schema(metric) + return output def _format_data(self, metric_name, project_id, start, end, data): diff --git a/cloudkitty/tests/collectors/test_prometheus.py b/cloudkitty/tests/collectors/test_prometheus.py index 959700b5..9e178fd7 100644 --- a/cloudkitty/tests/collectors/test_prometheus.py +++ b/cloudkitty/tests/collectors/test_prometheus.py @@ -49,7 +49,7 @@ class PrometheusCollectorTest(tests.TestCase): self.collector = prometheus.PrometheusCollector(transformers, **args) def test_format_data_instant_query(self): - expected = ({}, {}, Decimal('7')) + expected = ({}, {'project_id': ''}, Decimal('7')) params = { 'metric_name': 'http_requests_total', @@ -62,7 +62,7 @@ class PrometheusCollectorTest(tests.TestCase): self.assertEqual(expected, actual) def test_format_data_instant_query_2(self): - expected = ({}, {}, Decimal('42')) + expected = ({}, {'project_id': ''}, Decimal('42')) params = { 'metric_name': 'http_requests_total', @@ -78,8 +78,8 @@ class PrometheusCollectorTest(tests.TestCase): expected = { 'http_requests_total': [ { - 'desc': {}, - 'groupby': {}, + 'desc': {'project_id': ''}, + 'groupby': {'project_id': ''}, 'metadata': {}, 'vol': { 'qty': Decimal('7'), @@ -87,15 +87,14 @@ class PrometheusCollectorTest(tests.TestCase): } }, { - 'desc': {}, - 'groupby': {}, + 'desc': {'project_id': ''}, + 'groupby': {'project_id': ''}, 'metadata': {}, 'vol': { 'qty': Decimal('42'), 'unit': 'instance' } } - ] } diff --git a/cloudkitty/tests/collectors/test_validation.py b/cloudkitty/tests/collectors/test_validation.py new file mode 100644 index 00000000..c2dedd11 --- /dev/null +++ b/cloudkitty/tests/collectors/test_validation.py @@ -0,0 +1,127 @@ +# -*- coding: utf-8 -*- +# Copyright 2014 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. +# +import copy + +from voluptuous import error as verror + +from cloudkitty import collector +from cloudkitty import tests + + +class MetricConfigValidationTest(tests.TestCase): + + base_data = { + 'metrics': { + 'metric_one': { + 'groupby': ['one'], + 'metadata': ['one'], + 'unit': 'u', + } + } + } + + base_output = { + 'metric_one': { + 'groupby': ['one'], + 'metadata': ['one'], + 'unit': 'u', + 'factor': 1, + 'offset': 0, + 'mutate': 'NONE', + } + } + + def test_base_minimal_config(self): + data = copy.deepcopy(self.base_data) + expected_output = copy.deepcopy(self.base_output) + expected_output['metric_one']['groupby'].append('project_id') + + self.assertEqual( + collector.BaseCollector.check_configuration(data), + expected_output, + ) + + def test_gnocchi_minimal_config_no_extra_args(self): + data = copy.deepcopy(self.base_data) + + self.assertRaises( + verror.MultipleInvalid, + collector.gnocchi.GnocchiCollector.check_configuration, + data, + ) + + def test_gnocchi_minimal_config_minimal_extra_args(self): + data = copy.deepcopy(self.base_data) + data['metrics']['metric_one']['extra_args'] = {'resource_type': 'res'} + expected_output = copy.deepcopy(self.base_output) + expected_output['metric_one']['groupby'] += ['project_id', 'id'] + expected_output['metric_one']['extra_args'] = { + 'aggregation_method': 'max', + 'resource_type': 'res', + 'resource_key': 'id', + } + + self.assertEqual( + collector.gnocchi.GnocchiCollector.check_configuration(data), + expected_output, + ) + + def test_monasca_minimal_config_no_extra_args(self): + data = copy.deepcopy(self.base_data) + + self.assertRaises( + verror.MultipleInvalid, + collector.monasca.MonascaCollector.check_configuration, + data, + ) + + def test_monasca_minimal_config_minimal_extra_args(self): + data = copy.deepcopy(self.base_data) + data['metrics']['metric_one']['extra_args'] = {} + expected_output = copy.deepcopy(self.base_output) + expected_output['metric_one']['groupby'].extend( + ['project_id', 'resource_id']) + expected_output['metric_one']['extra_args'] = { + 'aggregation_method': 'max', + 'resource_key': 'resource_id', + } + + self.assertEqual( + collector.monasca.MonascaCollector.check_configuration(data), + expected_output, + ) + + def test_prometheus_minimal_config_empty_extra_args(self): + data = copy.deepcopy(self.base_data) + data['extra_args'] = {} + + self.assertRaises( + verror.MultipleInvalid, + collector.prometheus.PrometheusCollector.check_configuration, + data, + ) + + def test_prometheus_minimal_config_minimal_extra_args(self): + data = copy.deepcopy(self.base_data) + data['metrics']['metric_one']['extra_args'] = {'query': 'query'} + expected_output = copy.deepcopy(self.base_output) + expected_output['metric_one']['groupby'].append('project_id') + expected_output['metric_one']['extra_args'] = {'query': 'query'} + + self.assertEqual( + collector.prometheus.PrometheusCollector.check_configuration(data), + expected_output, + )