From b5112a00583e501ed4800d169c5819738c32d0b0 Mon Sep 17 00:00:00 2001
From: Vincent Fournier <v.fournier11@gmail.com>
Date: Fri, 3 Jul 2015 12:16:26 -0400
Subject: [PATCH] Refactor of metric handlers

Change-Id: I0657939b46fa9dd4c4cbcb3e4b81d9002c68f54b
---
 doc/source/webapi/v2/status.rst               |   2 +-
 .../controllers/v2/status/hosts/metrics.py    |  16 +--
 .../v2/status/hosts/services/metrics.py       |  16 +--
 .../metrics/{live_metric.py => metric.py}     |   2 +-
 .../status/metrics/live_metric_handler.py     | 130 ------------------
 .../handlers/status/metrics/metric_handler.py | 109 +++++++++++++++
 .../status/metrics/metric_name_handler.py     |  25 ++--
 .../v2/status/test_hosts_metric.py            |  41 +++++-
 .../v2/status/test_hosts_services_metrics.py  |   4 +-
 9 files changed, 176 insertions(+), 169 deletions(-)
 rename surveil/api/datamodel/status/metrics/{live_metric.py => metric.py} (98%)
 create mode 100644 surveil/api/handlers/status/metrics/metric_handler.py

diff --git a/doc/source/webapi/v2/status.rst b/doc/source/webapi/v2/status.rst
index db6ba79..1c9c6ee 100644
--- a/doc/source/webapi/v2/status.rst
+++ b/doc/source/webapi/v2/status.rst
@@ -62,7 +62,7 @@ types documentation
 .. autotype:: surveil.api.datamodel.status.live_query.LiveQuery
    :members:
 
-.. autotype:: surveil.api.datamodel.status.metrics.live_metric.LiveMetric
+.. autotype:: surveil.api.datamodel.status.metrics.metric.Metric
    :members:
 
 .. autotype:: surveil.api.datamodel.status.metrics.time_interval.TimeInterval
diff --git a/surveil/api/controllers/v2/status/hosts/metrics.py b/surveil/api/controllers/v2/status/hosts/metrics.py
index 517f807..dfdfdfb 100644
--- a/surveil/api/controllers/v2/status/hosts/metrics.py
+++ b/surveil/api/controllers/v2/status/hosts/metrics.py
@@ -17,8 +17,8 @@ from pecan import rest
 import wsmeext.pecan as wsme_pecan
 
 from surveil.api.datamodel.status import live_query
-from surveil.api.datamodel.status.metrics import live_metric
-from surveil.api.handlers.status.metrics import live_metric_handler
+from surveil.api.datamodel.status.metrics import metric as m
+from surveil.api.handlers.status.metrics import metric_handler
 from surveil.api.handlers.status.metrics import metric_name_handler
 from surveil.common import util
 
@@ -26,7 +26,7 @@ from surveil.common import util
 class MetricsController(rest.RestController):
 
     @util.policy_enforce(['authenticated'])
-    @wsme_pecan.wsexpose([live_metric.LiveMetric])
+    @wsme_pecan.wsexpose([m.Metric])
     def get(self):
         """Returns all metrics name for a host."""
         handler = metric_name_handler.MetricNameHandler(pecan.request)
@@ -45,10 +45,10 @@ class MetricController(rest.RestController):
         self.metric_name = metric_name
 
     @util.policy_enforce(['authenticated'])
-    @wsme_pecan.wsexpose(live_metric.LiveMetric)
+    @wsme_pecan.wsexpose(m.Metric)
     def get(self):
         """Return the last measure for the metric name on the host."""
-        handler = live_metric_handler.MetricHandler(pecan.request)
+        handler = metric_handler.MetricHandler(pecan.request)
         metric = handler.get(
             pecan.request.context['host_name'],
             self.metric_name
@@ -56,14 +56,14 @@ class MetricController(rest.RestController):
         return metric
 
     @util.policy_enforce(['authenticated'])
-    @wsme_pecan.wsexpose([live_metric.LiveMetric], body=live_query.LiveQuery)
+    @wsme_pecan.wsexpose([m.Metric], body=live_query.LiveQuery)
     def post(self, query):
         """Given a live query, returns all matching metrics.
 
         :param live_query: a live query within the request body.
         """
-        handler = live_metric_handler.MetricHandler(pecan.request)
-        metrics = handler.get_all(live_query=query,
+        handler = metric_handler.MetricHandler(pecan.request)
+        metrics = handler.get_all(query=query,
                                   metric_name=self.metric_name,
                                   host_name=pecan.request.context['host_name']
                                   )
diff --git a/surveil/api/controllers/v2/status/hosts/services/metrics.py b/surveil/api/controllers/v2/status/hosts/services/metrics.py
index 5fcfe51..6bc32fa 100644
--- a/surveil/api/controllers/v2/status/hosts/services/metrics.py
+++ b/surveil/api/controllers/v2/status/hosts/services/metrics.py
@@ -17,8 +17,8 @@ from pecan import rest
 import wsmeext.pecan as wsme_pecan
 
 from surveil.api.datamodel.status import live_query
-from surveil.api.datamodel.status.metrics import live_metric
-from surveil.api.handlers.status.metrics import live_metric_handler
+from surveil.api.datamodel.status.metrics import metric as m
+from surveil.api.handlers.status.metrics import metric_handler
 from surveil.api.handlers.status.metrics import metric_name_handler
 from surveil.common import util
 
@@ -26,7 +26,7 @@ from surveil.common import util
 class MetricsController(rest.RestController):
 
     @util.policy_enforce(['authenticated'])
-    @wsme_pecan.wsexpose([live_metric.LiveMetric])
+    @wsme_pecan.wsexpose([m.Metric])
     def get(self):
         """Returns all metrics name for a host with a service."""
         handler = metric_name_handler.MetricNameHandler(pecan.request)
@@ -48,10 +48,10 @@ class MetricController(rest.RestController):
         self.metric_name = metric_name
 
     @util.policy_enforce(['authenticated'])
-    @wsme_pecan.wsexpose(live_metric.LiveMetric)
+    @wsme_pecan.wsexpose(m.Metric)
     def get(self):
         """Return the last measure of the metric of the service of the host."""
-        handler = live_metric_handler.MetricHandler(pecan.request)
+        handler = metric_handler.MetricHandler(pecan.request)
         metric = handler.get(
             pecan.request.context['host_name'],
             self.metric_name,
@@ -60,14 +60,14 @@ class MetricController(rest.RestController):
         return metric
 
     @util.policy_enforce(['authenticated'])
-    @wsme_pecan.wsexpose([live_metric.LiveMetric], body=live_query.LiveQuery)
+    @wsme_pecan.wsexpose([m.Metric], body=live_query.LiveQuery)
     def post(self, query):
         """Returns all matching metrics.
 
         :param live query: a live query
         """
-        handler = live_metric_handler.MetricHandler(pecan.request)
-        metrics = handler.get_all(live_query=query,
+        handler = metric_handler.MetricHandler(pecan.request)
+        metrics = handler.get_all(query=query,
                                   metric_name=self.metric_name,
                                   host_name=pecan.request.context['host_name'],
                                   service_description=pecan.request.
diff --git a/surveil/api/datamodel/status/metrics/live_metric.py b/surveil/api/datamodel/status/metrics/metric.py
similarity index 98%
rename from surveil/api/datamodel/status/metrics/live_metric.py
rename to surveil/api/datamodel/status/metrics/metric.py
index 82e68ec..cafc310 100644
--- a/surveil/api/datamodel/status/metrics/live_metric.py
+++ b/surveil/api/datamodel/status/metrics/metric.py
@@ -18,7 +18,7 @@ import wsme.types as wtypes
 from surveil.api.datamodel import types
 
 
-class LiveMetric(types.Base):
+class Metric(types.Base):
 
     metric_name = wsme.wsattr(wtypes.text, mandatory=True)
     """Name of the metric"""
diff --git a/surveil/api/handlers/status/metrics/live_metric_handler.py b/surveil/api/handlers/status/metrics/live_metric_handler.py
index bc3e02a..e69de29 100644
--- a/surveil/api/handlers/status/metrics/live_metric_handler.py
+++ b/surveil/api/handlers/status/metrics/live_metric_handler.py
@@ -1,130 +0,0 @@
-# Copyright 2014 - Savoir-Faire Linux inc.
-#
-# 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.
-
-from surveil.api.datamodel.status import live_query
-from surveil.api.datamodel.status.metrics import live_metric
-from surveil.api.handlers import handler
-from surveil.api.handlers.status import influxdb_query
-
-
-class MetricHandler(handler.Handler):
-    """Fulfills a request on the metrics."""
-
-    def get(self, host_name, metric_name, service_description=None):
-        """Return the last metric."""
-        metrics = []
-        cli = self.request.influxdb_client
-        if metric_name is None:
-            if service_description is None:
-                query = ("SHOW measurements WHERE host_name='%s' "
-                         "AND service_description=''"
-                         % host_name)
-            else:
-                query = ("SHOW measurements WHERE host_name='%s' "
-                         "AND service_description='%s'"
-                         % (host_name, service_description))
-        else:
-            if service_description is None:
-                query = ("SELECT * FROM metric_%s "
-                         "WHERE host_name= '%s' "
-                         "GROUP BY service_description "
-                         "ORDER BY time DESC "
-                         "LIMIT 1"
-                         % (metric_name, host_name))
-            else:
-                query = ("SELECT * FROM metric_%s "
-                         "WHERE host_name= '%s' "
-                         "AND service_description= '%s'"
-                         "ORDER BY time DESC "
-                         "LIMIT 1"
-                         % (metric_name, host_name, service_description))
-
-        response = cli.query(query)
-
-        if metric_name is None:
-            metric_dicts = []
-
-            for item in response[None]:
-                metric_dict = self._metric_dict_from_influx_item(item)
-                if metric_dict is not None:
-                    metric_dicts.append(metric_dict)
-
-            for metric_dict in metric_dicts:
-                metric = live_metric.LiveMetric(**metric_dict)
-                metrics.append(metric)
-
-        else:
-            metrics = live_metric.LiveMetric(**self.
-                                             _metric_dict_from_influx_item(
-                                                 next(response.items()[0][1]),
-                                                 metric_name))
-
-        return metrics
-
-    def get_all(self, metric_name,
-                host_name, service_description=None,
-                live_query=live_query.LiveQuery()):
-        """Return all metrics."""
-
-        filters = {
-            "is": {
-                "host_name": [host_name]
-            }
-        }
-
-        if service_description:
-            filters["is"]["service_description"] = [service_description]
-
-        influx_client = self.request.influxdb_client
-        query = influxdb_query.build_influxdb_query(live_query,
-                                                    'metric_' + metric_name,
-                                                    order_by=["time desc"],
-                                                    additional_filters=filters)
-        response = influx_client.query(query)
-
-        metric_dicts = []
-
-        for item in response[None]:
-            metric_dict = self._metric_dict_from_influx_item(item, metric_name)
-            metric_dicts.append(metric_dict)
-
-        metrics = []
-        for metric_dict in metric_dicts:
-            metric = live_metric.LiveMetric(**metric_dict)
-            metrics.append(metric)
-
-        return metrics
-
-    def _metric_dict_from_influx_item(self, item, metric_name=None):
-        metric_dict = {"metric_name": str(metric_name)}
-        mappings = [
-            ('min', str),
-            ('max', str),
-            ('critical', str),
-            ('warning', str),
-            ('value', str),
-            ('unit', str),
-        ]
-
-        for field in mappings:
-            value = item.get(field[0], None)
-            if value is not None:
-                if field[0] == 'name':
-                    if value.startswith('metric_'):
-                        metric_dict = {}
-                        metric_dict[field[1]] = field[2](value[7:])
-                else:
-                    metric_dict[field[0]] = field[1](value)
-
-        return metric_dict
diff --git a/surveil/api/handlers/status/metrics/metric_handler.py b/surveil/api/handlers/status/metrics/metric_handler.py
new file mode 100644
index 0000000..f4ca567
--- /dev/null
+++ b/surveil/api/handlers/status/metrics/metric_handler.py
@@ -0,0 +1,109 @@
+# Copyright 2014 - Savoir-Faire Linux inc.
+#
+# 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.
+
+from surveil.api.datamodel.status import live_query as q
+from surveil.api.datamodel.status.metrics import metric as m
+from surveil.api.handlers import handler
+from surveil.api.handlers.status import influxdb_query
+
+
+class MetricHandler(handler.Handler):
+    """Fulfills a request on the metrics."""
+
+    def get(self, host_name, metric_name, service_description=None):
+        """Return the last metric."""
+        query = self._build_metric_query(
+            host_name,
+            metric_name,
+            service_description=service_description,
+            limit=1)
+
+        influx_client = self.request.influxdb_client
+        response = influx_client.query(query)
+
+        metrics = []
+        for item in response[None]:
+            metric_dict = self._metric_dict_from_influx_item(item, metric_name)
+            metric = m.Metric(**metric_dict)
+            metrics.append(metric)
+
+        if metric_name:
+            metrics = metrics[0] or ''
+        return metrics
+
+    def get_all(self, metric_name,
+                host_name, service_description=None,
+                query=q.LiveQuery()):
+        """Return all metrics."""
+        query = self._build_metric_query(
+            host_name,
+            metric_name,
+            service_description=service_description,
+            query=query)
+        influx_client = self.request.influxdb_client
+        response = influx_client.query(query)
+
+        metrics = []
+        for item in response[None]:
+            metric_dict = self._metric_dict_from_influx_item(item, metric_name)
+            metric = m.Metric(**metric_dict)
+            metrics.append(metric)
+
+        return metrics
+
+    def _build_metric_query(self, host_name, metric_name,
+                            service_description=None,
+                            query=None, limit=None):
+        filters = {
+            "is": {
+                "host_name": [host_name]
+            }
+        }
+
+        group_by = []
+        if service_description:
+            filters["is"]["service_description"] = [service_description]
+        else:
+            group_by.append('service_description')
+
+        return influxdb_query.build_influxdb_query(query,
+                                                   'metric_' + metric_name,
+                                                   order_by=["time desc"],
+                                                   group_by=group_by,
+                                                   additional_filters=filters,
+                                                   limit=limit,
+                                                   )
+
+    def _metric_dict_from_influx_item(self, item, metric_name=None):
+        metric_dict = {"metric_name": str(metric_name)}
+        mappings = [
+            ('min', str),
+            ('max', str),
+            ('critical', str),
+            ('warning', str),
+            ('value', str),
+            ('unit', str),
+        ]
+
+        for field in mappings:
+            value = item.get(field[0], None)
+            if value is not None:
+                if field[0] == 'name':
+                    if value.startswith('metric_'):
+                        metric_dict = {}
+                        metric_dict[field[1]] = field[2](value[7:])
+                else:
+                    metric_dict[field[0]] = field[1](value)
+
+        return metric_dict
diff --git a/surveil/api/handlers/status/metrics/metric_name_handler.py b/surveil/api/handlers/status/metrics/metric_name_handler.py
index a57eef2..1d09610 100644
--- a/surveil/api/handlers/status/metrics/metric_name_handler.py
+++ b/surveil/api/handlers/status/metrics/metric_name_handler.py
@@ -12,7 +12,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-from surveil.api.datamodel.status.metrics import live_metric
+from surveil.api.datamodel.status.metrics import metric as m
 from surveil.api.handlers import handler
 
 
@@ -21,29 +21,22 @@ class MetricNameHandler(handler.Handler):
 
     def get(self, host_name, service_description=None):
         """Return all metrics name."""
+        service_description = service_description or ''
+        query = ("SHOW measurements WHERE host_name='%s' "
+                 "AND service_description='%s'"
+                 % (host_name, service_description))
+        influx_client = self.request.influxdb_client
+        response = influx_client.query(query)
+
         metrics = []
-        cli = self.request.influxdb_client
-
-        if service_description is None:
-            query = ("SHOW measurements WHERE host_name='%s' "
-                     "AND service_description=''"
-                     % host_name)
-        else:
-            query = ("SHOW measurements WHERE host_name='%s' "
-                     "AND service_description='%s'"
-                     % (host_name, service_description))
-
-        response = cli.query(query)
-
         for item in response[None]:
             metric_dict = self._metrics_name_from_influx_item(item)
             if metric_dict is not None:
-                metrics.append(live_metric.LiveMetric(**metric_dict))
+                metrics.append(m.Metric(**metric_dict))
 
         return metrics
 
     def _metrics_name_from_influx_item(self, item):
-
         metric_name = None
         mappings = [('metric_name', 'name', str), ]
         for field in mappings:
diff --git a/surveil/tests/api/controllers/v2/status/test_hosts_metric.py b/surveil/tests/api/controllers/v2/status/test_hosts_metric.py
index 92b28e7..9747a5c 100644
--- a/surveil/tests/api/controllers/v2/status/test_hosts_metric.py
+++ b/surveil/tests/api/controllers/v2/status/test_hosts_metric.py
@@ -85,10 +85,44 @@ class TestHostMetric(functionalTest.FunctionalTest):
         })
 
     def test_get_metric_hosts(self):
+        influxdb_response = json.dumps({
+            "results": [
+                {
+                    "series": [
+                        {
+                            "name": "metric_load1",
+                            "tags": {
+                                "host_name": "srv-monitoring-01",
+                                "service_description": "load"
+                            },
+                            "columns": [
+                                "time",
+                                "critical",
+                                "min",
+                                "unit",
+                                "value",
+                                "warning"
+                            ],
+                            "values": [
+                                [
+                                    "2015-07-03T15:18:46Z",
+                                    30,
+                                    0,
+                                    "",
+                                    0.78,
+                                    15
+                                ]
+                            ]
+                        }
+                    ]
+                }
+            ]
+        })
+
         with requests_mock.Mocker() as m:
             m.register_uri(requests_mock.GET,
                            "http://influxdb:8086/query",
-                           text=self.influxdb_response)
+                           text=influxdb_response)
 
             response = self.get(
                 "/v2/status/hosts/srv-monitoring-01/metrics/load1"
@@ -97,9 +131,10 @@ class TestHostMetric(functionalTest.FunctionalTest):
             expected = {
                 "metric_name": "load1",
                 "min": "0",
+                "unit": "",
                 "critical": "30",
                 "warning": "15",
-                "value": "0.6"
+                "value": "0.78"
             }
 
             self.assert_count_equal_backport(
@@ -108,7 +143,7 @@ class TestHostMetric(functionalTest.FunctionalTest):
             self.assertEqual(
                 m.last_request.qs['q'],
                 ["select * from metric_load1 "
-                 "where host_name= 'srv-monitoring-01' "
+                 "where host_name='srv-monitoring-01' "
                  "group by service_description "
                  "order by time desc limit 1"]
             )
diff --git a/surveil/tests/api/controllers/v2/status/test_hosts_services_metrics.py b/surveil/tests/api/controllers/v2/status/test_hosts_services_metrics.py
index f396bac..bd508f0 100644
--- a/surveil/tests/api/controllers/v2/status/test_hosts_services_metrics.py
+++ b/surveil/tests/api/controllers/v2/status/test_hosts_services_metrics.py
@@ -86,7 +86,7 @@ class TestHostMetric(functionalTest.FunctionalTest):
 
     def test_get(self):
         """Test get all metric names for a service."""
-        self.influxdb_response = json.dumps({
+        influxdb_response = json.dumps({
             "results": [
                 {
                     "series": [
@@ -105,7 +105,7 @@ class TestHostMetric(functionalTest.FunctionalTest):
         with requests_mock.Mocker() as m:
             m.register_uri(requests_mock.GET,
                            "http://influxdb:8086/query",
-                           text=self.influxdb_response)
+                           text=influxdb_response)
 
             response = self.get(
                 "/v2/status/hosts/localhost/services/load/metrics"