From 4c8173acac15579feb3b180d972bac84745467e9 Mon Sep 17 00:00:00 2001
From: Brian Brazil <brian.brazil@robustperception.io>
Date: Mon, 7 Aug 2017 17:15:38 +0100
Subject: [PATCH] Use timestamp of a sample in deriv() to avoid FP issues
 (#2958)

With the squaring of the timestamp, we run into the
limitations of the 53bit mantissa for a 64bit float.

By subtracting away a timestamp of one of the samples (which is how the
intercept is used) we avoid this issue in practice as it's unlikely
that it is used over a very long time range.

Fixes #2674
---
 promql/functions.go      |  5 ++++-
 promql/functions_test.go | 38 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/promql/functions.go b/promql/functions.go
index e1168807b..f5b8ea1de 100644
--- a/promql/functions.go
+++ b/promql/functions.go
@@ -678,7 +678,10 @@ func funcDeriv(ev *evaluator, args Expressions) model.Value {
 		if len(samples.Values) < 2 {
 			continue
 		}
-		slope, _ := linearRegression(samples.Values, 0)
+		// We pass in an arbitrary timestamp that is near the values in use
+		// to avoid floating point accuracy issues, see
+		// https://github.com/prometheus/prometheus/issues/2674
+		slope, _ := linearRegression(samples.Values, samples.Values[0].Timestamp)
 		resultSample := &sample{
 			Metric:    samples.Metric,
 			Value:     slope,
diff --git a/promql/functions_test.go b/promql/functions_test.go
index 41a5d3216..ee341c0af 100644
--- a/promql/functions_test.go
+++ b/promql/functions_test.go
@@ -13,7 +13,13 @@
 
 package promql
 
-import "testing"
+import (
+	"context"
+	"testing"
+
+	"github.com/prometheus/common/model"
+	"github.com/prometheus/prometheus/storage/local"
+)
 
 func BenchmarkHoltWinters4Week5Min(b *testing.B) {
 	input := `
@@ -71,3 +77,33 @@ eval instant at 1d changes(http_requests[1d])
 	bench := NewBenchmark(b, input)
 	bench.Run()
 }
+
+func TestDeriv(t *testing.T) {
+	// https://github.com/prometheus/prometheus/issues/2674#issuecomment-315439393
+	// This requires more precision than the usual test system offers,
+	// so we test it by hand.
+	storage, closer := local.NewTestStorage(t, 2)
+	defer closer.Close()
+	engine := NewEngine(storage, nil)
+
+	metric := model.Metric{model.MetricNameLabel: model.LabelValue("foo")}
+	storage.Append(&model.Sample{Metric: metric, Timestamp: 1493712816939, Value: 1.0})
+	storage.Append(&model.Sample{Metric: metric, Timestamp: 1493712846939, Value: 1.0})
+	storage.WaitForIndexing()
+
+	query, err := engine.NewInstantQuery("deriv(foo[30m])", 1493712846939)
+	if err != nil {
+		t.Fatalf("Error parsing query: %s", err)
+	}
+	result := query.Exec(context.Background())
+	if result.Err != nil {
+		t.Fatalf("Error running query: %s", result.Err)
+	}
+	vec, _ := result.Vector()
+	if vec.Len() != 1 {
+		t.Fatalf("Expected 1 result, got %d", vec.Len())
+	}
+	if vec[0].Value != 0.0 {
+		t.Fatalf("Expected 0.0 as value, got %f", vec[0].Value)
+	}
+}