From d6fdb6ec64919b6dac8fddedec5955ccfa1727dd Mon Sep 17 00:00:00 2001
From: Xi Chen <xichen@uber.com>
Date: Thu, 5 Jul 2018 21:44:40 -0400
Subject: [PATCH] perSecond returns empty datapoint for negative rate

---
 transformation/binary.go      | 11 ++++++++++-
 transformation/binary_test.go | 10 ++++++++--
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/transformation/binary.go b/transformation/binary.go
index 8580481..20b9810 100644
--- a/transformation/binary.go
+++ b/transformation/binary.go
@@ -29,10 +29,19 @@ const (
 	nanosPerSecond = time.Second / time.Nanosecond
 )
 
+// perSecond computes the derivative between consecutive datapoints, taking into
+// account the time interval between the values.
+// * It skips NaN values.
+// * It assumes the timestamps are monotonically increasing, and values are non-decreasing.
+//   If either of the two conditions is not met, an empty datapoint is returned.
 func perSecond(prev, curr Datapoint) Datapoint {
 	if prev.TimeNanos >= curr.TimeNanos || math.IsNaN(prev.Value) || math.IsNaN(curr.Value) {
 		return emptyDatapoint
 	}
-	rate := (curr.Value - prev.Value) * float64(nanosPerSecond) / float64(curr.TimeNanos-prev.TimeNanos)
+	diff := curr.Value - prev.Value
+	if diff < 0 {
+		return emptyDatapoint
+	}
+	rate := diff * float64(nanosPerSecond) / float64(curr.TimeNanos-prev.TimeNanos)
 	return Datapoint{TimeNanos: curr.TimeNanos, Value: rate}
 }
diff --git a/transformation/binary_test.go b/transformation/binary_test.go
index 1be97b3..5a573bb 100644
--- a/transformation/binary_test.go
+++ b/transformation/binary_test.go
@@ -42,8 +42,8 @@ func TestPerSecond(t *testing.T) {
 		},
 		{
 			prev:     Datapoint{TimeNanos: time.Unix(1230, 0).UnixNano(), Value: 25},
-			curr:     Datapoint{TimeNanos: time.Unix(1240, 0).UnixNano(), Value: 20},
-			expected: Datapoint{TimeNanos: time.Unix(1240, 0).UnixNano(), Value: -0.5},
+			curr:     Datapoint{TimeNanos: time.Unix(1240, 0).UnixNano(), Value: 30},
+			expected: Datapoint{TimeNanos: time.Unix(1240, 0).UnixNano(), Value: 0.5},
 		},
 		{
 			prev:        Datapoint{TimeNanos: time.Unix(1230, 0).UnixNano(), Value: 25},
@@ -63,6 +63,12 @@ func TestPerSecond(t *testing.T) {
 			expectedNaN: true,
 			expected:    emptyDatapoint,
 		},
+		{
+			prev:        Datapoint{TimeNanos: time.Unix(1230, 0).UnixNano(), Value: 30},
+			curr:        Datapoint{TimeNanos: time.Unix(1240, 0).UnixNano(), Value: 20},
+			expectedNaN: true,
+			expected:    emptyDatapoint,
+		},
 	}
 
 	for _, input := range inputs {