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

@@ -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 In
-from voluptuous import Length
 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']
+        scope_key = CONF.collect.scope_key
-        period = CONF.collect.period
+        method = self.conf[metric_name]['extra_args']['aggregation_method']
-
+        groupby = self.conf[metric_name].get('groupby', [])
-        if '$period' in query:
+        metadata = self.conf[metric_name].get('metadata', [])
-            try:
+        period = end - start
-                query = ck_utils.template_str_substitute(
+        time = end
-                    query, {'period': str(period) + 's'},
-                )
-            except (KeyError, ValueError):
-                raise PrometheusConfigError(
-                    'Invalid prometheus query: {}'.format(query))
 
+        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,

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': ''},
+                    'desc': {
-                    'groupby': {'project_id': ''},
+                        'bar': '', 'foo': '', 'project_id': '',
-                    'metadata': {},
+                        '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': ''},
+                    'desc': {
-                    'groupby': {'project_id': ''},
+                        'bar': '', 'foo': '', 'project_id': '',
-                    'metadata': {},
+                        'code': '200', 'instance': 'localhost:9090',
+                    },
+                    'groupby': {'bar': '', 'foo': '', 'project_id': ''},
+                    'metadata': {'code': '200', 'instance': 'localhost:9090'},
                     'vol': {
                         'qty': Decimal('42'),
                         'unit': 'instance'

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),

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.