Merge pull request #832 from mchrome/fix/sortBy-null-series

sortBy: substitute NaN values for negative infinity
go-graphite · May 14, 2024 · 0b081ee · 0b081ee
2 parents af84f05 + 7311204
commit 0b081ee
Show file tree

Hide file tree

Showing 3 changed files with 100 additions and 24 deletions.
diff --git a/cmd/mockbackend/testcases/sortBy/sortBy.yaml b/cmd/mockbackend/testcases/sortBy/sortBy.yaml
@@ -0,0 +1,56 @@
+version: "v1"
+test:
+    apps:
+        - name: "carbonapi"
+          binary: "./carbonapi"
+          args:
+              - "-config"
+              - "./cmd/mockbackend/carbonapi_singlebackend.yaml"
+    queries:
+            - endpoint: "http://127.0.0.1:8081"
+              delay: 1
+              type: "GET"
+              URL: "/render?format=json&target=limit(sortByMaxima(metric*),2)"
+              expectedResponse:
+                  httpCode: 200
+                  contentType: "application/json"
+                  expectedResults:
+                          - metrics:
+                                  - target: "metricZ2"
+                                    datapoints: [[8.0, 1],[1.0, 2],[1.0, 3],[7.0, 4],[4.0, 5]]
+                                  - target: "metricZ1"
+                                    datapoints: [[4.0, 1],[6.0, 2],[2.0, 3],[2.0, 4],[3.0, 5]]
+            - endpoint: "http://127.0.0.1:8081"
+              delay: 1
+              type: "GET"
+              URL: "/render?format=json&target=limit(sortByMaxima(metric*),3)"
+              expectedResponse:
+                  httpCode: 200
+                  contentType: "application/json"
+                  expectedResults:
+                          - metrics:
+                                  - target: "metricZ2"
+                                    datapoints: [[8.0, 1],[1.0, 2],[1.0, 3],[7.0, 4],[4.0, 5]]
+                                  - target: "metricZ1"
+                                    datapoints: [[4.0, 1],[6.0, 2],[2.0, 3],[2.0, 4],[3.0, 5]]
+                                  - target: "metricNaN"
+                                    datapoints: [["null", 1],["null", 2],["null", 3],["null", 4],["null", 5]]    
+
+listeners:
+        - address: ":9070"
+          expressions:
+            "metric*":
+                pathExpression: "metric*"
+                data:
+                    - metricName: "metricNaN"
+                      values: [.NaN, .NaN, .NaN, .NaN, .NaN]
+                      step: 1
+                      startTime: 1
+                    - metricName: "metricZ1"
+                      values: [4.0, 6.0, 2.0, 2.0, 3.0]
+                      step: 1
+                      startTime: 1
+                    - metricName: "metricZ2"
+                      values: [8.0, 1.0, 1.0, 7.0, 4.0]
+                      step: 1
+                      startTime: 1
diff --git a/expr/functions/sortBy/function.go b/expr/functions/sortBy/function.go
@@ -3,6 +3,7 @@ package sortBy
 import (
 	"context"
 	"fmt"
+	"math"
 	"sort"
 
 	"github.com/go-graphite/carbonapi/expr/consolidations"
@@ -80,6 +81,9 @@ func doSort(aggFuncName string, ascending bool, original []*types.MetricData) []
 
 	for i, a := range arg {
 		vals[i] = consolidations.SummarizeValues(aggFuncName, a.Values, a.XFilesFactor)
+		if math.IsNaN(vals[i]) {
+			vals[i] = math.Inf(-1)
+		}
 	}
 
 	if ascending {

diff --git a/expr/functions/sortBy/function_test.go b/expr/functions/sortBy/function_test.go
@@ -1,6 +1,7 @@
 package sortBy
 
 import (
+	"math"
 	"testing"
 	"time"
 
@@ -14,7 +15,8 @@ import (
 )
 
 var (
-	md []interfaces.FunctionMetadata = New("")
+	md  []interfaces.FunctionMetadata = New("")
+	nan                               = math.NaN()
 )
 
 func init() {
@@ -28,96 +30,110 @@ func TestFunction(t *testing.T) {
 
 	tests := []th.EvalTestItem{
 		{
-			"sortByTotal(metric1)",
-			map[parser.MetricRequest][]*types.MetricData{
+			Target: "sortByTotal(metric1)",
+			M: map[parser.MetricRequest][]*types.MetricData{
 				{Metric: "metric1", From: 0, Until: 1}: {
 					types.MakeMetricData("metricA", []float64{0, 0, 0, 0, 0, 0}, 1, now32),
 					types.MakeMetricData("metricB", []float64{5, 5, 5, 5, 5, 5}, 1, now32),
 					types.MakeMetricData("metricC", []float64{4, 4, 5, 5, 4, 4}, 1, now32),
 				},
 			},
-			[]*types.MetricData{
+			Want: []*types.MetricData{
 				types.MakeMetricData("metricB", []float64{5, 5, 5, 5, 5, 5}, 1, now32),
 				types.MakeMetricData("metricC", []float64{4, 4, 5, 5, 4, 4}, 1, now32),
 				types.MakeMetricData("metricA", []float64{0, 0, 0, 0, 0, 0}, 1, now32),
 			},
 		},
 		{
-			"sortByMaxima(metric*)",
-			map[parser.MetricRequest][]*types.MetricData{
+			Target: "sortByMaxima(metric*)",
+			M: map[parser.MetricRequest][]*types.MetricData{
 				{Metric: "metric*", From: 0, Until: 1}: {
 					types.MakeMetricData("metricA", []float64{0, 0, 0, 0, 0, 0}, 1, now32),
 					types.MakeMetricData("metricB", []float64{5, 5, 5, 5, 5, 5}, 1, now32),
 					types.MakeMetricData("metricC", []float64{2, 2, 10, 5, 2, 2}, 1, now32),
 				},
 			},
-			[]*types.MetricData{
+			Want: []*types.MetricData{
 				types.MakeMetricData("metricC", []float64{2, 2, 10, 5, 2, 2}, 1, now32),
 				types.MakeMetricData("metricB", []float64{5, 5, 5, 5, 5, 5}, 1, now32),
 				types.MakeMetricData("metricA", []float64{0, 0, 0, 0, 0, 0}, 1, now32),
 			},
 		},
 		{
-			"sortByMinima(metric*)",
-			map[parser.MetricRequest][]*types.MetricData{
+			Target: "sortByMinima(metric*)",
+			M: map[parser.MetricRequest][]*types.MetricData{
 				{Metric: "metric*", From: 0, Until: 1}: {
 					types.MakeMetricData("metricA", []float64{0, 0, 0, 0, 0, 0}, 1, now32),
 					types.MakeMetricData("metricB", []float64{3, 4, 5, 6, 7, 8}, 1, now32),
 					types.MakeMetricData("metricC", []float64{4, 4, 5, 5, 6, 6}, 1, now32),
 				},
 			},
-			[]*types.MetricData{
+			Want: []*types.MetricData{
 				types.MakeMetricData("metricA", []float64{0, 0, 0, 0, 0, 0}, 1, now32),
 				types.MakeMetricData("metricB", []float64{3, 4, 5, 6, 7, 8}, 1, now32),
 				types.MakeMetricData("metricC", []float64{4, 4, 5, 5, 6, 6}, 1, now32),
 			},
 		},
 		{
-			"sortBy(metric*)",
-			map[parser.MetricRequest][]*types.MetricData{
+			Target: "sortBy(metric*)",
+			M: map[parser.MetricRequest][]*types.MetricData{
 				{Metric: "metric*", From: 0, Until: 1}: {
 					types.MakeMetricData("metricA", []float64{0, 0, 0, 0, 0, 0}, 1, now32),
 					types.MakeMetricData("metricB", []float64{3, 4, 5, 6, 7, 8}, 1, now32),
 					types.MakeMetricData("metricC", []float64{1, 2, 3, 4, 5, 6}, 1, now32),
 				},
 			},
-			[]*types.MetricData{
+			Want: []*types.MetricData{
 				types.MakeMetricData("metricA", []float64{0, 0, 0, 0, 0, 0}, 1, now32),
 				types.MakeMetricData("metricC", []float64{1, 2, 3, 4, 5, 6}, 1, now32),
 				types.MakeMetricData("metricB", []float64{3, 4, 5, 6, 7, 8}, 1, now32),
 			},
 		},
 		{
-			"sortBy(metric*, 'median')",
-			map[parser.MetricRequest][]*types.MetricData{
+			Target: "sortBy(metric*, 'median')",
+			M: map[parser.MetricRequest][]*types.MetricData{
 				{Metric: "metric*", From: 0, Until: 1}: {
 					types.MakeMetricData("metricA", []float64{0, 0, 0, 0, 0, 0}, 1, now32),
 					types.MakeMetricData("metricB", []float64{4, 4, 5, 5, 6, 6}, 1, now32),
 					types.MakeMetricData("metricC", []float64{3, 4, 5, 6, 7, 8}, 1, now32),
 				},
 			},
-			[]*types.MetricData{
+			Want: []*types.MetricData{
 				types.MakeMetricData("metricA", []float64{0, 0, 0, 0, 0, 0}, 1, now32),
 				types.MakeMetricData("metricB", []float64{4, 4, 5, 5, 6, 6}, 1, now32),
 				types.MakeMetricData("metricC", []float64{3, 4, 5, 6, 7, 8}, 1, now32),
 			},
 		},
-
 		{
-			"sortBy(metric*, 'max', true)",
-			map[parser.MetricRequest][]*types.MetricData{
+			Target: "sortBy(metric*, 'max', true)",
+			M: map[parser.MetricRequest][]*types.MetricData{
 				{Metric: "metric*", From: 0, Until: 1}: {
 					types.MakeMetricData("metricA", []float64{0, 0, 0, 0, 0, 0}, 1, now32),
 					types.MakeMetricData("metricB", []float64{3, 4, 5, 6, 7, 8}, 1, now32),
 					types.MakeMetricData("metricC", []float64{4, 4, 5, 5, 6, 6}, 1, now32),
 				},
 			},
-			[]*types.MetricData{
+			Want: []*types.MetricData{
 				types.MakeMetricData("metricB", []float64{3, 4, 5, 6, 7, 8}, 1, now32),
 				types.MakeMetricData("metricC", []float64{4, 4, 5, 5, 6, 6}, 1, now32),
 				types.MakeMetricData("metricA", []float64{0, 0, 0, 0, 0, 0}, 1, now32),
 			},
 		},
+		{
+			Target: "sortBy(metric*, 'max', true)",
+			M: map[parser.MetricRequest][]*types.MetricData{
+				{Metric: "metric*", From: 0, Until: 1}: {
+					types.MakeMetricData("metricA", []float64{nan, nan, nan, nan, nan, nan}, 1, now32),
+					types.MakeMetricData("metricB", []float64{3, 4, 5, 6, 7, 8}, 1, now32),
+					types.MakeMetricData("metricC", []float64{4, 4, 5, 5, 6, 6}, 1, now32),
+				},
+			},
+			Want: []*types.MetricData{
+				types.MakeMetricData("metricB", []float64{3, 4, 5, 6, 7, 8}, 1, now32),
+				types.MakeMetricData("metricC", []float64{4, 4, 5, 5, 6, 6}, 1, now32),
+				types.MakeMetricData("metricA", []float64{nan, nan, nan, nan, nan, nan}, 1, now32),
+			},
+		},
 	}
 
 	for _, tt := range tests {
@@ -135,16 +151,16 @@ func TestErrorInvalidConsolidationFunction(t *testing.T) {
 
 	tests := []th.EvalTestItemWithError{
 		{
-			"sortBy(metric*, 'test')",
-			map[parser.MetricRequest][]*types.MetricData{
+			Target: "sortBy(metric*, 'test')",
+			M: map[parser.MetricRequest][]*types.MetricData{
 				{Metric: "metric*", From: 0, Until: 1}: {
 					types.MakeMetricData("metricA", []float64{0, 0, 0, 0, 0, 0}, 1, now32),
 					types.MakeMetricData("metricB", []float64{4, 4, 5, 5, 6, 6}, 1, now32),
 					types.MakeMetricData("metricC", []float64{3, 4, 5, 6, 7, 8}, 1, now32),
 				},
 			},
-			nil,
-			consolidations.ErrInvalidConsolidationFunc,
+			Want:  nil,
+			Error: consolidations.ErrInvalidConsolidationFunc,
 		},
 	}