From 7bfe768c8b83591e2bbd9d1ac99a739b0b84d562 Mon Sep 17 00:00:00 2001 From: Justin Ferrieu Date: Wed, 13 Mar 2019 10:30:03 +0100 Subject: [PATCH] Change configuration schema and query process for Prometheus collector Changes have been made on the query building process happening inside the PrometheusCollector which now has a new configuration option: Under collector_prometheus/extra_args section: * aggregation_method: Aggregation method to use on Prometheus metrics Aggregation method can be one of the following: * avg * count * max (default) * min * stddev * stdvar * sum Depends-On: https://review.openstack.org/#/c/636157/ Change-Id: I8aec9918df0a9b5fb66d1afa620e1ff0af128247 Story: 2004974 Task: 29430 --- cloudkitty/collector/prometheus.py | 50 +++++++------ .../tests/collectors/test_prometheus.py | 74 ++++++++++++++++--- .../tests/collectors/test_validation.py | 8 +- ...prometheus-collector-02bd6351d447e4fe.yaml | 6 ++ 4 files changed, 102 insertions(+), 36 deletions(-) create mode 100644 releasenotes/notes/rework-prometheus-collector-02bd6351d447e4fe.yaml diff --git a/cloudkitty/collector/prometheus.py b/cloudkitty/collector/prometheus.py index cf5250ba..401694f4 100644 --- a/cloudkitty/collector/prometheus.py +++ b/cloudkitty/collector/prometheus.py @@ -22,8 +22,7 @@ from decimal import ROUND_HALF_UP from oslo_config import cfg from oslo_log import log import requests -from voluptuous import All -from voluptuous import Length +from voluptuous import In from voluptuous import Required from voluptuous import Schema @@ -66,15 +65,16 @@ CONF = cfg.CONF PROMETHEUS_EXTRA_SCHEMA = { Required('extra_args'): { - Required('query'): All(str, Length(min=1)), + Required('aggregation_method', default='max'): + In([ + 'avg', 'count', 'max', + 'min', 'stddev', 'stdvar', + 'sum' + ]), } } -class PrometheusConfigError(collect_exceptions.CollectError): - pass - - class PrometheusResponseError(collect_exceptions.CollectError): pass @@ -164,7 +164,7 @@ class PrometheusCollector(collector.BaseCollector): return output - def _format_data(self, metric_name, project_id, start, end, data): + def _format_data(self, metric_name, scope_key, scope_id, start, end, data): """Formats Prometheus data format to Cloudkitty data format. Returns metadata, groupby, qty @@ -173,7 +173,7 @@ class PrometheusCollector(collector.BaseCollector): for meta in self.conf[metric_name]['metadata']: metadata[meta] = data['metric'][meta] - groupby = {} + groupby = {scope_key: scope_id} for meta in self.conf[metric_name]['groupby']: groupby[meta] = data['metric'].get(meta, '') @@ -189,23 +189,26 @@ class PrometheusCollector(collector.BaseCollector): return metadata, groupby, qty - def fetch_all(self, metric_name, start, end, project_id, q_filter=None): + def fetch_all(self, metric_name, start, end, scope_id, q_filter=None): """Returns metrics to be valorized.""" - query = self.conf[metric_name]['extra_args']['query'] - period = CONF.collect.period - - if '$period' in query: - try: - query = ck_utils.template_str_substitute( - query, {'period': str(period) + 's'}, - ) - except (KeyError, ValueError): - raise PrometheusConfigError( - 'Invalid prometheus query: {}'.format(query)) + scope_key = CONF.collect.scope_key + method = self.conf[metric_name]['extra_args']['aggregation_method'] + groupby = self.conf[metric_name].get('groupby', []) + metadata = self.conf[metric_name].get('metadata', []) + period = end - start + time = end + query = '{0}({0}_over_time({1}{{{2}="{3}"}}[{4}s])) by ({5})'.format( + method, + metric_name, + scope_key, + scope_id, + period, + ', '.join(groupby + metadata), + ) res = self._conn.get_instant( query, - end, + time, ) # If the query returns an empty dataset, @@ -218,7 +221,8 @@ class PrometheusCollector(collector.BaseCollector): for item in res['data']['result']: metadata, groupby, qty = self._format_data( metric_name, - project_id, + scope_key, + scope_id, start, end, item, diff --git a/cloudkitty/tests/collectors/test_prometheus.py b/cloudkitty/tests/collectors/test_prometheus.py index d896fcc9..c473b3e3 100644 --- a/cloudkitty/tests/collectors/test_prometheus.py +++ b/cloudkitty/tests/collectors/test_prometheus.py @@ -33,12 +33,21 @@ class PrometheusCollectorTest(tests.TestCase): self._tenant_id = samples.TENANT args = { 'period': 3600, + 'scope_key': 'namespace', 'conf': { 'metrics': { 'http_requests_total': { 'unit': 'instance', + 'groupby': [ + 'foo', + 'bar', + ], + 'metadata': [ + 'code', + 'instance', + ], 'extra_args': { - 'query': 'http_request_total[$period]', + 'aggregation_method': 'avg', }, }, } @@ -47,12 +56,41 @@ class PrometheusCollectorTest(tests.TestCase): transformers = transformer.get_transformers() self.collector = prometheus.PrometheusCollector(transformers, **args) + def test_fetch_all_build_query(self): + query = ( + 'avg(avg_over_time(http_requests_total' + '{project_id="f266f30b11f246b589fd266f85eeec39"}[3600s]' + ')) by (foo, bar, project_id, code, instance)' + ) + + with mock.patch.object( + prometheus.PrometheusClient, 'get_instant', + ) as mock_get: + self.collector.fetch_all( + 'http_requests_total', + samples.FIRST_PERIOD_BEGIN, + samples.FIRST_PERIOD_END, + self._tenant_id, + ) + mock_get.assert_called_once_with( + query, + samples.FIRST_PERIOD_END, + ) + def test_format_data_instant_query(self): - expected = ({}, {'project_id': ''}, Decimal('7')) + expected = ({ + 'code': '200', + 'instance': 'localhost:9090', + }, { + 'bar': '', + 'foo': '', + 'project_id': '' + }, Decimal('7')) params = { 'metric_name': 'http_requests_total', - 'project_id': self._tenant_id, + 'scope_key': 'project_id', + 'scope_id': self._tenant_id, 'start': samples.FIRST_PERIOD_BEGIN, 'end': samples.FIRST_PERIOD_END, 'data': samples.PROMETHEUS_RESP_INSTANT_QUERY['data']['result'][0], @@ -61,11 +99,19 @@ class PrometheusCollectorTest(tests.TestCase): self.assertEqual(expected, actual) def test_format_data_instant_query_2(self): - expected = ({}, {'project_id': ''}, Decimal('42')) + expected = ({ + 'code': '200', + 'instance': 'localhost:9090', + }, { + 'bar': '', + 'foo': '', + 'project_id': '' + }, Decimal('42')) params = { 'metric_name': 'http_requests_total', - 'project_id': self._tenant_id, + 'scope_key': 'project_id', + 'scope_id': self._tenant_id, 'start': samples.FIRST_PERIOD_BEGIN, 'end': samples.FIRST_PERIOD_END, 'data': samples.PROMETHEUS_RESP_INSTANT_QUERY['data']['result'][1], @@ -77,18 +123,24 @@ class PrometheusCollectorTest(tests.TestCase): expected = { 'http_requests_total': [ { - 'desc': {'project_id': ''}, - 'groupby': {'project_id': ''}, - 'metadata': {}, + 'desc': { + 'bar': '', 'foo': '', 'project_id': '', + 'code': '200', 'instance': 'localhost:9090', + }, + 'groupby': {'bar': '', 'foo': '', 'project_id': ''}, + 'metadata': {'code': '200', 'instance': 'localhost:9090'}, 'vol': { 'qty': Decimal('7'), 'unit': 'instance' } }, { - 'desc': {'project_id': ''}, - 'groupby': {'project_id': ''}, - 'metadata': {}, + 'desc': { + 'bar': '', 'foo': '', 'project_id': '', + 'code': '200', 'instance': 'localhost:9090', + }, + 'groupby': {'bar': '', 'foo': '', 'project_id': ''}, + 'metadata': {'code': '200', 'instance': 'localhost:9090'}, 'vol': { 'qty': Decimal('42'), 'unit': 'instance' diff --git a/cloudkitty/tests/collectors/test_validation.py b/cloudkitty/tests/collectors/test_validation.py index c2dedd11..031eef36 100644 --- a/cloudkitty/tests/collectors/test_validation.py +++ b/cloudkitty/tests/collectors/test_validation.py @@ -116,10 +116,14 @@ class MetricConfigValidationTest(tests.TestCase): def test_prometheus_minimal_config_minimal_extra_args(self): data = copy.deepcopy(self.base_data) - data['metrics']['metric_one']['extra_args'] = {'query': 'query'} + data['metrics']['metric_one']['extra_args'] = { + 'aggregation_method': 'max', + } expected_output = copy.deepcopy(self.base_output) expected_output['metric_one']['groupby'].append('project_id') - expected_output['metric_one']['extra_args'] = {'query': 'query'} + expected_output['metric_one']['extra_args'] = { + 'aggregation_method': 'max', + } self.assertEqual( collector.prometheus.PrometheusCollector.check_configuration(data), diff --git a/releasenotes/notes/rework-prometheus-collector-02bd6351d447e4fe.yaml b/releasenotes/notes/rework-prometheus-collector-02bd6351d447e4fe.yaml new file mode 100644 index 00000000..4eaf7756 --- /dev/null +++ b/releasenotes/notes/rework-prometheus-collector-02bd6351d447e4fe.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Prometheus collector now supports, under extra_args section, + an aggregation_method option to decide which aggregation + method is to be performed over collected metrics.