Merge "Ensure Nova metrics derived from a set of metrics"

nova/compute/monitors/base.py

@@ -20,20 +20,17 @@ from nova.objects import fields
 
 @six.add_metaclass(abc.ABCMeta)
 class MonitorBase(object):
-    """Base class for all resource monitor plugins."""
+    """Base class for all resource monitor plugins.
+
+    A monitor is responsible for adding a set of related metrics to
+    a `nova.objects.MonitorMetricList` object after the monitor has
+    performed some sampling or monitoring action.
+    """
 
     def __init__(self, compute_manager):
         self.compute_manager = compute_manager
         self.source = None
 
-    @abc.abstractmethod
-    def get_metric(self, name):
-        """Return a (value, timestamp) tuple for the supplied metric name.
-
-        :param name: The name/key for the metric to grab the value for.
-        """
-        raise NotImplementedError('get_metric')
-
     @abc.abstractmethod
     def get_metric_names(self):
         """Get available metric names.
@@ -46,6 +43,21 @@ class MonitorBase(object):
         """
         raise NotImplementedError('get_metric_names')
 
+    @abc.abstractmethod
+    def get_metrics(self):
+        """Returns a list of tuples containing information for all metrics
+        tracked by the monitor.
+
+        Note that if the monitor class is responsible for tracking a *related*
+        set of metrics -- e.g. a set of percentages of CPU time allocated to
+        user, kernel, and idle -- it is the responsibility of the monitor
+        implementation to do a single sampling call to the underlying monitor
+        to ensure that related metric values make logical sense.
+
+        :returns: list of (metric_name, value, timestamp) tuples
+        """
+        raise NotImplementedError('get_metrics')
+
     def add_metrics_to_list(self, metrics_list):
         """Adds metric objects to a supplied list object.
 
@@ -53,10 +65,9 @@ class MonitorBase(object):
                             plugin should append nova.objects.MonitorMetric
                             objects to.
         """
-        metric_names = self.get_metric_names()
+        metric_data = self.get_metrics()
         metrics = []
-        for name in metric_names:
-            value, timestamp = self.get_metric(name)
+        for (name, value, timestamp) in metric_data:
             metric = objects.MonitorMetric(name=name,
                                            value=value,
                                            timestamp=timestamp,

nova/compute/monitors/cpu/virt_driver.py

@@ -40,20 +40,16 @@ class Monitor(base.CPUMonitorBase):
         self._data = {}
         self._cpu_stats = {}
 
-    def get_metric(self, name):
+    def get_metrics(self):
+        metrics = []
         self._update_data()
-        return self._data[name], self._data["timestamp"]
+        for name in self.get_metric_names():
+            metrics.append((name, self._data[name], self._data["timestamp"]))
+        return metrics
 
     def _update_data(self):
-        # Don't allow to call this function so frequently (<= 1 sec)
-        now = timeutils.utcnow()
-        if self._data.get("timestamp") is not None:
-            delta = now - self._data.get("timestamp")
-            if delta.seconds <= 1:
-                return
-
         self._data = {}
-        self._data["timestamp"] = now
+        self._data["timestamp"] = timeutils.utcnow()
 
         # Extract node's CPU statistics.
         try:

nova/tests/unit/compute/monitors/cpu/test_virt_driver.py

@@ -15,6 +15,8 @@
 
 """Tests for Compute Driver CPU resource monitor."""
 
+import mock
+
 from nova.compute.monitors.cpu import virt_driver
 from nova import objects
 from nova import test
@@ -70,3 +72,15 @@ class VirtDriverCPUMonitorTestCase(test.NoDBTestCase):
         self.assertEqual(metrics["cpu.idle.percent"], 97)
         self.assertEqual(metrics["cpu.iowait.percent"], 0)
         self.assertEqual(metrics["cpu.percent"], 2)
+
+    def test_ensure_single_sampling(self):
+        # We want to ensure that the virt driver's get_host_cpu_stats()
+        # is only ever called once, otherwise values for monitor metrics
+        # might be illogical -- e.g. pct cpu times for user/system/idle
+        # may add up to more than 100.
+        metrics = objects.MonitorMetricList()
+        monitor = virt_driver.Monitor(FakeResourceTracker())
+
+        with mock.patch.object(FakeDriver, 'get_host_cpu_stats') as mocked:
+            monitor.add_metrics_to_list(metrics)
+            mocked.assert_called_once_with()

nova/tests/unit/compute/test_resource_tracker.py

@@ -1256,8 +1256,8 @@ class ComputeMonitorTestCase(BaseTestCase):
             def get_metric_names(self):
                 return set(["cpu.frequency"])
 
-            def get_metric(self, name):
-                return 100, self.NOW_TS
+            def get_metrics(self):
+                return [("cpu.frequency", 100, self.NOW_TS)]
 
         self.tracker.monitors = [FakeCPUMonitor(None)]
         mock_notifier = mock.Mock()