clean up perSecond and nonNegativeDerivative maxValue and None handling

webapp/graphite/render/functions.py

@@ -1167,9 +1167,12 @@ def scaleToSeconds(requestContext, seriesList, seconds):
     series.tags['scaleToSeconds'] = seconds
     series.name = "scaleToSeconds(%s,%d)" % (series.name,seconds)
     series.pathExpression = series.name
+    factor = seconds / series.step
+
     for i,value in enumerate(series):
-      factor = seconds * 1.0 / series.step
-      series[i] = safeMul(value,factor)
+      if value is not None:
+        series[i] = round(value * factor, 6)
+
   return seriesList
 
 def pow(requestContext, seriesList, factor):
@@ -1595,30 +1598,20 @@ def perSecond(requestContext, seriesList, maxValue=None):
     newValues = []
     prev = None
     step = series.step
+
     for val in series:
-      if prev is None:
-        newValues.append(None)
-        prev = val
-        continue
-      if val is None:
-        newValues.append(None)
-        step += series.step
-        continue
+      delta, prev = _nonNegativeDelta(val, prev, maxValue)
 
-      diff = val - prev
-      if diff >= 0:
-        newValues.append(round(diff / step, 6))
-      elif maxValue is not None and maxValue >= val:
-        newValues.append(round((maxValue + diff) / step, 6))
+      if delta is not None:
+        newValues.append(round(delta / step, 6))
       else:
         newValues.append(None)
 
-      step = series.step
-      prev = val
     series.tags['perSecond'] = 1
     newName = "perSecond(%s)" % series.name
     newSeries = series.copy(name=newName, values=newValues)
     results.append(newSeries)
+
   return results
 
 def delay(requestContext, seriesList, steps):
@@ -1750,20 +1743,9 @@ def nonNegativeDerivative(requestContext, seriesList, maxValue=None):
     prev = None
 
     for val in series:
-      if None in (prev, val):
-        newValues.append(None)
-        prev = val
-        continue
-
-      diff = val - prev
-      if diff >= 0:
-        newValues.append(diff)
-      elif maxValue is not None and maxValue >= val:
-        newValues.append( (maxValue - prev) + val  + 1 )
-      else:
-        newValues.append(None)
+      delta, prev = _nonNegativeDelta(val, prev, maxValue)
 
-      prev = val
+      newValues.append(delta)
 
     series.tags['nonNegativeDerivative'] = 1
     newName = "nonNegativeDerivative(%s)" % series.name
@@ -1772,6 +1754,28 @@ def nonNegativeDerivative(requestContext, seriesList, maxValue=None):
 
   return results
 
+def _nonNegativeDelta(val, prev, maxValue):
+  # ignore values larger than maxValue
+  if maxValue is not None and val > maxValue:
+    return None, None
+
+  # first reading
+  if None in (prev, val):
+    return None, val
+
+  # counter increased, use the difference
+  if val >= prev:
+    return val - prev, val
+
+  # counter wrapped and we have maxValue
+  # calculate delta based on maxValue + 1 + val - prev
+  if maxValue is not None:
+    return maxValue + 1 + val - prev, val
+
+  # counter wrapped or reset and we don't have maxValue
+  # just use None
+  return None, val
+
 def stacked(requestContext,seriesLists,stackName='__DEFAULT__'):
   """
   Takes one metric or a wildcard seriesList and change them so they are

webapp/tests/test_functions.py

@@ -1587,27 +1587,43 @@ def test_perSecond(self):
         self.assertEqual(list(expected[0]), list(result[0]))
         self.assertEqual(expected, result)
 
+        # verify against scaleToSeconds(nonNegativeDerivative(<series>, <maxValue>), 1)
+        result = functions.scaleToSeconds({}, functions.nonNegativeDerivative({}, seriesList), 1)
+        self.assertEqual(list(expected[0]), list(result[0]))
+
     def test_perSecond_float(self):
-        seriesList = self._gen_series_list_with_data(key='test',start=0,end=600,step=60,data=[0, 90, 186, 291, 411, 561, 747, 939, 137, 337])
+        seriesList = self._gen_series_list_with_data(key='test',start=0,end=600,step=60,data=[0, 90, 186, 291, 411, 561, 747, 939, 136, 336])
         expected = [TimeSeries('perSecond(test)', 0, 600, 60, [None, 1.5, 1.6, 1.75, 2, 2.5, 3.1, 3.2, 3.3, 3.333333])]
         result = functions.perSecond({}, seriesList, maxValue=1000)
         self.assertEqual(list(expected[0]), list(result[0]))
         self.assertEqual(expected, result)
 
+        # verify against scaleToSeconds(nonNegativeDerivative(<series>, <maxValue>), 1)
+        result = functions.scaleToSeconds({}, functions.nonNegativeDerivative({}, seriesList, maxValue=1000), 1)
+        self.assertEqual(list(expected[0]), list(result[0]))
+
     def test_perSecond_nones(self):
-        seriesList = self._gen_series_list_with_data(key='test',start=0,end=600,step=60,data=[0, 60, None, 180, None, None, None, 420, None, 540])
-        expected = [TimeSeries('perSecond(test)', 0, 600, 60, [None, 1, None, 1, None, None, None, 1, None, 1])]
+        seriesList = self._gen_series_list_with_data(key='test',start=0,end=600,step=60,data=[0, 60, None, 180, None, None, 360, 420, None, 540])
+        expected = [TimeSeries('perSecond(test)', 0, 600, 60, [None, 1, None, None, None, None, None, 1, None, None])]
         result = functions.perSecond({}, seriesList)
         self.assertEqual(list(expected[0]), list(result[0]))
         self.assertEqual(expected, result)
 
+        # verify against scaleToSeconds(nonNegativeDerivative(<series>, <maxValue>), 1)
+        result = functions.scaleToSeconds({}, functions.nonNegativeDerivative({}, seriesList), 1)
+        self.assertEqual(list(expected[0]), list(result[0]))
+
     def test_perSecond_max(self):
-        seriesList = self._gen_series_list_with_data(key='test',start=0,end=600,step=60,data=[0, 120, 240, 480, 960, 900, 120, 240, 120, 0])
-        expected = [TimeSeries('perSecond(test)', 0, 600, 60, [None, 2, 2, 4, 8, None, -5, 2, 6, 6])]
+        seriesList = self._gen_series_list_with_data(key='test',start=0,end=600,step=60,data=[0, 120, 240, 480, 960, 900, 120, 240, 119, 479])
+        expected = [TimeSeries('perSecond(test)', 0, 600, 60, [None, 2, 2, 4, None, None, None, 2, 6, 6])]
         result = functions.perSecond({}, seriesList, 480)
         self.assertEqual(list(expected[0]), list(result[0]))
         self.assertEqual(expected, result)
 
+        # verify against scaleToSeconds(nonNegativeDerivative(<series>, <maxValue>), 1)
+        result = functions.scaleToSeconds({}, functions.nonNegativeDerivative({}, seriesList, 480), 1)
+        self.assertEqual(list(expected[0]), list(result[0]))
+
     def test_integral(self):
         seriesList = self._gen_series_list_with_data(key='test',start=0,end=600,step=60,data=[None, 1, 2, 3, 4, 5, None, 6, 7, 8])
         expected = [TimeSeries('integral(test)', 0, 600, 60, [None, 1, 3, 6, 10, 15, None, 21, 28, 36])]