fix(reduceSeries): error on out-of-range reduceNode, add tests

Builds on #933 (reduce on series.Name). Adds a bounds guard so an out-of-range reduceNode returns an error instead of panicking, and supports negative indices counted from the end.

Adds a reduce package test covering the aliased-name grouping (the #933 fix) and the out-of-range cases, and updates the existing reduceSeries expectations for the name tag now set by CopyNameWithVal.

Author: npazosmendez

expr/expr_test.go

@@ -258,8 +258,8 @@ func TestEvalExpression(t *testing.T) {
 				},
 			},
 			[]*types.MetricData{
-				types.MakeMetricData("devops.service.server1.filter.received.reduce.asPercent.count", []float64{25, 200, 200}, 1, now32).SetNameTag("devops.service.server1.filter.received.valid.count"),
-				types.MakeMetricData("devops.service.server2.filter.received.reduce.asPercent.count", []float64{25, 100, 400}, 1, now32).SetNameTag("devops.service.server2.filter.received.valid.count"),
+				types.MakeMetricData("devops.service.server1.filter.received.reduce.asPercent.count", []float64{25, 200, 200}, 1, now32),
+				types.MakeMetricData("devops.service.server2.filter.received.reduce.asPercent.count", []float64{25, 100, 400}, 1, now32),
 			},
 		},
 		{
@@ -272,7 +272,7 @@ func TestEvalExpression(t *testing.T) {
 				},
 			},
 			[]*types.MetricData{
-				types.MakeMetricData("devops.service.server2.filter.received.reduce.asPercent.count", []float64{25, 100, 400}, 1, now32).SetNameTag("devops.service.server2.filter.received.valid.count"),
+				types.MakeMetricData("devops.service.server2.filter.received.reduce.asPercent.count", []float64{25, 100, 400}, 1, now32),
 			},
 		},
 		{

expr/functions/reduce/function.go

@@ -3,6 +3,8 @@ package reduce
 import (
 	"context"
 
+	"github.com/ansel1/merry"
+
 	"github.com/go-graphite/carbonapi/expr/helper"
 	"github.com/go-graphite/carbonapi/expr/interfaces"
 	"github.com/go-graphite/carbonapi/expr/types"
@@ -70,8 +72,15 @@ func (f *reduce) Do(ctx context.Context, eval interfaces.Evaluator, e parser.Exp
 	for _, series := range seriesList {
 		metric := types.ExtractName(series.Name)
 		nodes := strings.Split(metric, ".")
-		reduceNodeKey := nodes[reduceNode]
-		nodes[reduceNode] = "reduce." + reduceFunction
+		node := reduceNode
+		if node < 0 {
+			node += len(nodes)
+		}
+		if node < 0 || node >= len(nodes) {
+			return nil, merry.WithMessagef(parser.ErrInvalidArg, "reduceNode %d out of range for metric %q", reduceNode, metric)
+		}
+		reduceNodeKey := nodes[node]
+		nodes[node] = "reduce." + reduceFunction
 		aliasName := strings.Join(nodes, ".")
 		_, exist := reduceGroups[aliasName]
 		if !exist {

expr/functions/reduce/function_test.go

@@ -0,0 +1,116 @@
+package reduce
+
+import (
+	"testing"
+	"time"
+
+	"github.com/go-graphite/carbonapi/expr/interfaces"
+	"github.com/go-graphite/carbonapi/expr/metadata"
+	"github.com/go-graphite/carbonapi/expr/types"
+	"github.com/go-graphite/carbonapi/pkg/parser"
+	th "github.com/go-graphite/carbonapi/tests"
+
+	"github.com/go-graphite/carbonapi/expr/functions/alias"
+	"github.com/go-graphite/carbonapi/expr/functions/aliasByNode"
+	"github.com/go-graphite/carbonapi/expr/functions/asPercent"
+)
+
+var (
+	md []interfaces.FunctionMetadata = New("")
+)
+
+func init() {
+	for _, m := range md {
+		metadata.RegisterFunction(m.Name, m.F)
+	}
+	for _, m := range alias.New("") {
+		metadata.RegisterFunction(m.Name, m.F)
+	}
+	for _, m := range aliasByNode.New("") {
+		metadata.RegisterFunction(m.Name, m.F)
+	}
+	for _, m := range asPercent.New("") {
+		metadata.RegisterFunction(m.Name, m.F)
+	}
+}
+
+func TestReduce(t *testing.T) {
+	now32 := int64(time.Now().Unix())
+
+	tests := []th.EvalTestItem{
+		{
+			// mimics graphite-web test_reduceSeries_asPercent
+			Target: `reduceSeries(group.server*.*, "asPercent", 2, "bytes_used", "total_bytes")`,
+			M: map[parser.MetricRequest][]*types.MetricData{
+				{Metric: "group.server*.*", From: 0, Until: 1}: {
+					types.MakeMetricData("group.server1.bytes_used", []float64{1}, 1, now32),
+					types.MakeMetricData("group.server1.total_bytes", []float64{2}, 1, now32),
+					types.MakeMetricData("group.server2.bytes_used", []float64{3}, 1, now32),
+					types.MakeMetricData("group.server2.total_bytes", []float64{4}, 1, now32),
+				},
+			},
+			Want: []*types.MetricData{
+				types.MakeMetricData("group.server1.reduce.asPercent", []float64{50}, 1, now32),
+				types.MakeMetricData("group.server2.reduce.asPercent", []float64{75}, 1, now32),
+			},
+		},
+		{
+			// reduceSeries must group by the aliased name, not the original "name" tag
+			Target: `reduceSeries(aliasByNode(servers.*.cpu.*, 1, 3), "asPercent", 1, "raw_used", "raw_total")`,
+			M: map[parser.MetricRequest][]*types.MetricData{
+				{Metric: "servers.*.cpu.*", From: 0, Until: 1}: {
+					types.MakeMetricData("servers.host1.cpu.raw_used", []float64{1}, 1, now32),
+					types.MakeMetricData("servers.host1.cpu.raw_total", []float64{2}, 1, now32),
+					types.MakeMetricData("servers.host2.cpu.raw_used", []float64{3}, 1, now32),
+					types.MakeMetricData("servers.host2.cpu.raw_total", []float64{4}, 1, now32),
+				},
+			},
+			Want: []*types.MetricData{
+				types.MakeMetricData("host1.reduce.asPercent", []float64{50}, 1, now32),
+				types.MakeMetricData("host2.reduce.asPercent", []float64{75}, 1, now32),
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.Target, func(t *testing.T) {
+			eval := th.EvaluatorFromFuncWithMetadata(metadata.FunctionMD.Functions)
+			th.TestEvalExpr(t, eval, &tt)
+		})
+	}
+}
+
+func TestReduceErrors(t *testing.T) {
+	now32 := int64(time.Now().Unix())
+
+	tests := []th.EvalTestItemWithError{
+		{
+			// reduceNode out of range for the series name must not panic
+			Target: `reduceSeries(group.*, "asPercent", 4, "bytes_used", "total_bytes")`,
+			M: map[parser.MetricRequest][]*types.MetricData{
+				{Metric: "group.*", From: 0, Until: 1}: {
+					types.MakeMetricData("group.bytes_used", []float64{1}, 1, now32),
+					types.MakeMetricData("group.total_bytes", []float64{2}, 1, now32),
+				},
+			},
+			Error: parser.ErrInvalidArg,
+		},
+		{
+			Target: `reduceSeries(group.*, "asPercent", -5, "bytes_used", "total_bytes")`,
+			M: map[parser.MetricRequest][]*types.MetricData{
+				{Metric: "group.*", From: 0, Until: 1}: {
+					types.MakeMetricData("group.bytes_used", []float64{1}, 1, now32),
+					types.MakeMetricData("group.total_bytes", []float64{2}, 1, now32),
+				},
+			},
+			Error: parser.ErrInvalidArg,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.Target, func(t *testing.T) {
+			eval := th.EvaluatorFromFuncWithMetadata(metadata.FunctionMD.Functions)
+			th.TestEvalExprWithError(t, eval, &tt)
+		})
+	}
+}