Fix statistics period computing with start/end time

This fixes bug #1132732

Change-Id: I97545340ad9f49ec575e27c1a4fb4a09a3aefbf4
Signed-off-by: Julien Danjou <julien@danjou.info>

ceilometer/storage/base.py

@@ -19,10 +19,30 @@
 """
 
 import abc
+import datetime
+import math
 
-from ceilometer.openstack.common import log
+from ceilometer.openstack.common import timeutils
 
-LOG = log.getLogger(__name__)
+
+def iter_period(start, end, period):
+    """Split a time from start to end in periods of a number of seconds. This
+    function yield the (start, end) time for each period composing the time
+    passed as argument.
+
+    :param start: When the period set start.
+    :param end: When the period end starts.
+    :param period: The duration of the period.
+
+    """
+    period_start = start
+    increment = datetime.timedelta(seconds=period)
+    for i in xrange(int(math.ceil(
+            timeutils.delta_seconds(start, end)
+            / float(period)))):
+        next_start = period_start + increment
+        yield (period_start, next_start)
+        period_start = next_start
 
 
 class StorageEngine(object):

ceilometer/storage/impl_sqlalchemy.py

@@ -20,8 +20,6 @@ from __future__ import absolute_import
 
 import copy
 import datetime
-import math
-
 from sqlalchemy import func
 
 from ceilometer.openstack.common import log
@@ -398,7 +396,7 @@ class Connection(base.Connection):
 
     @staticmethod
     def _stats_result_to_dict(result, period, period_start, period_end):
-        return {'count': result.count,
+        return {'count': int(result.count),
                 'min': result.min,
                 'max': result.max,
                 'avg': result.avg,
@@ -431,7 +429,8 @@ class Connection(base.Connection):
           }
         """
 
-        res = self._make_stats_query(event_filter).all()[0]
+        if not period or not event_filter.start or not event_filter.end:
+            res = self._make_stats_query(event_filter).all()[0]
 
         if not period:
             return [self._stats_result_to_dict(res, 0, res.tsmin, res.tsmax)]
@@ -443,21 +442,23 @@ class Connection(base.Connection):
         # stats by period. We would like to use GROUP BY, but there's no
         # portable way to manipulate timestamp in SQL, so we can't.
         results = []
-        for i in range(int(math.ceil(
+        for period_start, period_end in base.iter_period(
-                timeutils.delta_seconds(event_filter.start or res.tsmin,
+                event_filter.start or res.tsmin,
-                                        event_filter.end or res.tsmax)
+                event_filter.end or res.tsmax,
-                / float(period)))):
+                period):
-            period_start = (event_filter.start
-                            + datetime.timedelta(seconds=i * period))
-            period_end = period_start + datetime.timedelta(seconds=period)
             q = query.filter(Meter.timestamp >= period_start)
             q = q.filter(Meter.timestamp < period_end)
-            results.append(self._stats_result_to_dict(
+            r = q.all()[0]
-                result=q.all()[0],
+            # Don't add results that didn't have any event
-                period=int(timeutils.delta_seconds(period_start, period_end)),
+            if r.count:
-                period_start=period_start,
+                results.append(self._stats_result_to_dict(
-                period_end=period_end,
+                    result=r,
-            ))
+                    period=int(timeutils.delta_seconds(period_start,
+                                                       period_end)),
+                    period_start=period_start,
+                    period_end=period_end,
+                ))
+
         return results
 
 

tests/storage/base.py

@@ -822,6 +822,33 @@ class StatisticsTest(DBTestBase):
         self.assertEqual(r['duration_end'],
                          datetime.datetime(2012, 9, 25, 11, 31))
 
+    def test_by_user_period_start_end(self):
+        f = storage.EventFilter(
+            user='user-5',
+            meter='volume.size',
+            start='2012-09-25T10:28:00',
+            end='2012-09-25T11:28:00',
+        )
+        results = self.conn.get_meter_statistics(f, period=1800)
+        self.assertEqual(len(results), 1)
+        r = results[0]
+        self.assertEqual(r['period_start'],
+                         datetime.datetime(2012, 9, 25, 10, 28))
+        self.assertEqual(r['count'], 1)
+        self.assertEqual(r['avg'], 8)
+        self.assertEqual(r['min'], 8)
+        self.assertEqual(r['max'], 8)
+        self.assertEqual(r['sum'], 8)
+        self.assertEqual(r['period'], 1800)
+        self.assertEqual(r['period_end'],
+                         r['period_start']
+                         + datetime.timedelta(seconds=1800))
+        self.assertEqual(r['duration'], 0)
+        self.assertEqual(r['duration_start'],
+                         datetime.datetime(2012, 9, 25, 10, 30))
+        self.assertEqual(r['duration_end'],
+                         datetime.datetime(2012, 9, 25, 10, 30))
+
     def test_by_project(self):
         f = storage.EventFilter(
             meter='volume.size',

tests/storage/test_base.py

@@ -0,0 +1,51 @@
+# -*- encoding: utf-8 -*-
+#
+# Copyright © 2013 eNovance
+#
+# Author: Julien Danjou <julien@danjou.info>
+#
+# 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 datetime
+import math
+import unittest
+
+from ceilometer.storage import base
+
+
+class BaseTest(unittest.TestCase):
+
+    def test_iter_period(self):
+        times = list(base.iter_period(
+            datetime.datetime(2013, 01, 01, 12, 00),
+            datetime.datetime(2013, 01, 01, 13, 00),
+            60))
+        self.assertEqual(len(times), 60)
+        self.assertEqual(times[10],
+                         (datetime.datetime(2013, 01, 01, 12, 10),
+                          datetime.datetime(2013, 01, 01, 12, 11)))
+        self.assertEqual(times[21],
+                         (datetime.datetime(2013, 01, 01, 12, 21),
+                          datetime.datetime(2013, 01, 01, 12, 22)))
+
+    def test_iter_period_bis(self):
+        times = list(base.iter_period(
+            datetime.datetime(2013, 01, 02, 13, 00),
+            datetime.datetime(2013, 01, 02, 14, 00),
+            55))
+        self.assertEqual(len(times), math.ceil(3600 / 55.0))
+        self.assertEqual(times[10],
+                         (datetime.datetime(2013, 01, 02, 13, 9, 10),
+                          datetime.datetime(2013, 01, 02, 13, 10, 5)))
+        self.assertEqual(times[21],
+                         (datetime.datetime(2013, 01, 02, 13, 19, 15),
+                          datetime.datetime(2013, 01, 02, 13, 20, 10)))