Reuse aggregateWithWildcards in functions

Also added test for multiplySeriesWithWildcards
bookingcom · May 21, 2024 · 630cc37 · 630cc37
1 parent 39dccde
commit 630cc37
Show file tree

Hide file tree

Showing 6 changed files with 149 additions and 178 deletions.
diff --git a/pkg/app/carbonapi/http_handlers.go b/pkg/app/carbonapi/http_handlers.go
@@ -389,7 +389,7 @@ func evalExprRender(ctx context.Context, exp parser.Expr, res *([]*types.MetricD
 	form *renderForm, printErrorStackTrace bool, getTargetData interfaces.GetTargetData) (retErr error) {
 	defer func() {
 		if r := recover(); r != nil {
-			retErr = fmt.Errorf("panic during expr eval: %s", r)
+			retErr = fmt.Errorf("panic during expr eval: %s\n%s", r, string(debug.Stack()))
 			if printErrorStackTrace {
 				debug.PrintStack()
 			}

diff --git a/pkg/expr/functions/averageSeriesWithWildcards/function.go b/pkg/expr/functions/averageSeriesWithWildcards/function.go
@@ -2,8 +2,6 @@ package averageSeriesWithWildcards
 
 import (
 	"context"
-	"fmt"
-	"strings"
 
 	"github.com/bookingcom/carbonapi/pkg/expr/helper"
 	"github.com/bookingcom/carbonapi/pkg/expr/interfaces"
@@ -42,64 +40,13 @@ func (f *averageSeriesWithWildcards) Do(ctx context.Context, e parser.Expr, from
 		return nil, err
 	}
 
-	var results []*types.MetricData
-
-	nodeList := []string{}
-	groups := make(map[string][]*types.MetricData)
-
-	for _, a := range args {
-		metric := helper.ExtractMetric(a.Name)
-		nodes := strings.Split(metric, ".")
-		var s []string
-		// Yes, this is O(n^2), but len(nodes) < 10 and len(fields) < 3
-		// Iterating an int slice is faster than a map for n ~ 30
-		// http://www.antoine.im/posts/someone_is_wrong_on_the_internet
-		for i, n := range nodes {
-			if !helper.Contains(fields, i) {
-				s = append(s, n)
-			}
-		}
-
-		node := strings.Join(s, ".")
-
-		if len(groups[node]) == 0 {
-			nodeList = append(nodeList, node)
-		}
-
-		groups[node] = append(groups[node], a)
-	}
-
-	for _, series := range nodeList {
-		args := groups[series]
-		r := *args[0]
-		r.Name = fmt.Sprintf("averageSeriesWithWildcards(%s)", series)
-		r.Values = make([]float64, len(args[0].Values))
-		r.IsAbsent = make([]bool, len(args[0].Values))
-
-		length := make([]float64, len(args[0].Values))
-		atLeastOne := make([]bool, len(args[0].Values))
-		for _, arg := range args {
-			for i, v := range arg.Values {
-				if arg.IsAbsent[i] {
-					continue
-				}
-				atLeastOne[i] = true
-				length[i]++
-				r.Values[i] += v
-			}
+	return helper.AggregateSeriesWithWildcards("averageSeriesWithWildcards", args, fields, func(values []float64) (float64, bool) {
+		sum := 0.0
+		for _, value := range values {
+			sum += value
 		}
-
-		for i, v := range atLeastOne {
-			if v {
-				r.Values[i] = r.Values[i] / length[i]
-			} else {
-				r.IsAbsent[i] = true
-			}
-		}
-
-		results = append(results, &r)
-	}
-	return results, nil
+		return sum / float64(len(values)), false
+	})
 }
 
 // Description is auto-generated description, based on output of https://github.com/graphite-project/graphite-web

diff --git a/pkg/expr/functions/multiplySeriesWithWildcards/function.go b/pkg/expr/functions/multiplySeriesWithWildcards/function.go
@@ -2,8 +2,6 @@ package multiplySeriesWithWildcards
 
 import (
 	"context"
-	"fmt"
-	"strings"
 
 	"github.com/bookingcom/carbonapi/pkg/expr/helper"
 	"github.com/bookingcom/carbonapi/pkg/expr/interfaces"
@@ -43,68 +41,14 @@ func (f *multiplySeriesWithWildcards) Do(ctx context.Context, e parser.Expr, fro
 		return nil, err
 	}
 
-	var results []*types.MetricData
-
-	nodeList := []string{}
-	groups := make(map[string][]*types.MetricData)
-
-	for _, a := range args {
-		metric := helper.ExtractMetric(a.Name)
-		nodes := strings.Split(metric, ".")
-		var s []string
-		// Yes, this is O(n^2), but len(nodes) < 10 and len(fields) < 3
-		// Iterating an int slice is faster than a map for n ~ 30
-		// http://www.antoine.im/posts/someone_is_wrong_on_the_internet
-		for i, n := range nodes {
-			if !helper.Contains(fields, i) {
-				s = append(s, n)
-			}
-		}
-
-		node := strings.Join(s, ".")
-
-		if len(groups[node]) == 0 {
-			nodeList = append(nodeList, node)
+	return helper.AggregateSeriesWithWildcards("multiplySeriesWithWildcards", args, fields, func(values []float64) (float64, bool) {
+		ret := values[0]
+		for _, value := range values[1:] {
+			ret *= value
 		}
 
-		groups[node] = append(groups[node], a)
-	}
-
-	for _, series := range nodeList {
-		args := groups[series]
-		r := *args[0]
-		r.Name = fmt.Sprintf("multiplySeriesWithWildcards(%s)", series)
-		r.Values = make([]float64, len(args[0].Values))
-		r.IsAbsent = make([]bool, len(args[0].Values))
-
-		atLeastOne := make([]bool, len(args[0].Values))
-		hasVal := make([]bool, len(args[0].Values))
-
-		for _, arg := range args {
-			for i, v := range arg.Values {
-				if arg.IsAbsent[i] {
-					continue
-				}
-
-				atLeastOne[i] = true
-				if !hasVal[i] {
-					r.Values[i] = v
-					hasVal[i] = true
-				} else {
-					r.Values[i] *= v
-				}
-			}
-		}
-
-		for i, v := range atLeastOne {
-			if !v {
-				r.IsAbsent[i] = true
-			}
-		}
-
-		results = append(results, &r)
-	}
-	return results, nil
+		return ret, false
+	})
 }
 
 // Description is auto-generated description, based on output of https://github.com/graphite-project/graphite-web

diff --git a/pkg/expr/functions/multiplySeriesWithWildcards/function_test.go b/pkg/expr/functions/multiplySeriesWithWildcards/function_test.go
@@ -0,0 +1,93 @@
+package multiplySeriesWithWildcards
+
+import (
+	"testing"
+	"time"
+
+	"go.uber.org/zap"
+
+	"github.com/bookingcom/carbonapi/pkg/expr/helper"
+	"github.com/bookingcom/carbonapi/pkg/expr/metadata"
+	"github.com/bookingcom/carbonapi/pkg/expr/types"
+	"github.com/bookingcom/carbonapi/pkg/parser"
+	th "github.com/bookingcom/carbonapi/tests"
+)
+
+func init() {
+	md := New("")
+	evaluator := th.EvaluatorFromFunc(md[0].F)
+	metadata.SetEvaluator(evaluator)
+	helper.SetEvaluator(evaluator)
+	for _, m := range md {
+		metadata.RegisterFunction(m.Name, m.F, zap.NewNop())
+	}
+}
+
+func TestMultiplySeriesWithWildcards(t *testing.T) {
+	now32 := int32(time.Now().Unix())
+
+	tests := []th.EvalTestItem{
+		{
+			"multiplySeriesWithWildcards(empty,0)",
+			map[parser.MetricRequest][]*types.MetricData{
+				{"empty", 0, 1}: {
+					types.MakeMetricData("metric0", []float64{}, 1, now32),
+				},
+			},
+			[]*types.MetricData{types.MakeMetricData("multiplySeriesWithWildcards()",
+				[]float64{}, 1, now32)},
+		},
+		{
+			"multiplySeriesWithWildcards(*,0)",
+			map[parser.MetricRequest][]*types.MetricData{
+				{"*", 0, 1}: {
+					types.MakeMetricData("m1", []float64{1, 2, 3, 4, 5}, 1, now32),
+					types.MakeMetricData("m2", []float64{5, 4, 3, 2, 1}, 1, now32),
+				},
+			},
+			[]*types.MetricData{types.MakeMetricData("multiplySeriesWithWildcards()",
+				[]float64{5, 8, 9, 8, 5}, 1, now32)},
+		},
+		{
+			"multiplySeriesWithWildcards(m1.*.*,2)",
+			map[parser.MetricRequest][]*types.MetricData{
+				{"m1.*.*", 0, 1}: {
+					types.MakeMetricData("m1.n1.o1", []float64{1, 2, 3, 4, 5}, 1, now32),
+					types.MakeMetricData("m1.n2.o2", []float64{1, 2, 3, 4, 5}, 1, now32),
+					types.MakeMetricData("m1.n1.o2", []float64{1, 2, 3, 4, 5}, 1, now32),
+					types.MakeMetricData("m1.n2.o1", []float64{1, 2, 3, 4, 5}, 1, now32),
+					types.MakeMetricData("m1.n2.o3", []float64{1, 2, 3, 4, 5}, 1, now32),
+					types.MakeMetricData("m1.n2.o4", []float64{1, 2, 3, 4, 5}, 1, now32),
+				},
+			},
+			[]*types.MetricData{
+				types.MakeMetricData("multiplySeriesWithWildcards(m1.n1)", []float64{1, 4, 9, 16, 25}, 1, now32),
+				types.MakeMetricData("multiplySeriesWithWildcards(m1.n2)", []float64{1, 16, 81, 256, 625}, 1, now32),
+			},
+		},
+		{
+			"multiplySeriesWithWildcards(m1.*.*,1,2)",
+			map[parser.MetricRequest][]*types.MetricData{
+				{"m1.*.*", 0, 1}: {
+					types.MakeMetricData("m1.n1.o1", []float64{1, 2, 3, 4, 5}, 1, now32),
+					types.MakeMetricData("m1.n2.o2", []float64{1, 2, 3, 4, 5}, 1, now32),
+					types.MakeMetricData("m1.n1.o2", []float64{1, 2, 3, 4, 5}, 1, now32),
+					types.MakeMetricData("m1.n2.o1", []float64{1, 2, 3, 4, 5}, 1, now32),
+					types.MakeMetricData("m1.n2.o3", []float64{1, 2, 3, 4, 5}, 1, now32),
+					types.MakeMetricData("m1.n2.o4", []float64{1, 2, 3, 4, 5}, 1, now32),
+				},
+			},
+			[]*types.MetricData{
+				types.MakeMetricData("multiplySeriesWithWildcards(m1)", []float64{1, 64, 729, 4096, 15625}, 1, now32),
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		copy := tt
+		testName := tt.Target
+		t.Run(testName, func(t *testing.T) {
+			th.TestEvalExpr(t, &copy)
+		})
+	}
+}
diff --git a/pkg/expr/functions/sumSeriesWithWildcards/function.go b/pkg/expr/functions/sumSeriesWithWildcards/function.go
@@ -2,9 +2,8 @@ package sumSeriesWithWildcards
 
 import (
 	"context"
-	"fmt"
-	"strings"
 
+	"github.com/bookingcom/carbonapi/pkg/expr/functions/sum"
 	"github.com/bookingcom/carbonapi/pkg/expr/helper"
 	"github.com/bookingcom/carbonapi/pkg/expr/interfaces"
 	"github.com/bookingcom/carbonapi/pkg/expr/types"
@@ -42,60 +41,7 @@ func (f *sumSeriesWithWildcards) Do(ctx context.Context, e parser.Expr, from, un
 		return nil, err
 	}
 
-	var results []*types.MetricData
-
-	nodeList := []string{}
-	groups := make(map[string][]*types.MetricData)
-
-	for _, a := range args {
-		metric := helper.ExtractMetric(a.Name)
-		nodes := strings.Split(metric, ".")
-		var s []string
-		// Yes, this is O(n^2), but len(nodes) < 10 and len(fields) < 3
-		// Iterating an int slice is faster than a map for n ~ 30
-		// http://www.antoine.im/posts/someone_is_wrong_on_the_internet
-		for i, n := range nodes {
-			if !helper.Contains(fields, i) {
-				s = append(s, n)
-			}
-		}
-
-		node := strings.Join(s, ".")
-
-		if len(groups[node]) == 0 {
-			nodeList = append(nodeList, node)
-		}
-
-		groups[node] = append(groups[node], a)
-	}
-
-	for _, series := range nodeList {
-		args := groups[series]
-		r := *args[0]
-		r.Name = fmt.Sprintf("sumSeriesWithWildcards(%s)", series)
-		r.Values = make([]float64, len(args[0].Values))
-		r.IsAbsent = make([]bool, len(args[0].Values))
-
-		atLeastOne := make([]bool, len(args[0].Values))
-		for _, arg := range args {
-			for i, v := range arg.Values {
-				if arg.IsAbsent[i] {
-					continue
-				}
-				atLeastOne[i] = true
-				r.Values[i] += v
-			}
-		}
-
-		for i, v := range atLeastOne {
-			if !v {
-				r.IsAbsent[i] = true
-			}
-		}
-
-		results = append(results, &r)
-	}
-	return results, nil
+	return helper.AggregateSeriesWithWildcards("sumSeriesWithWildcards", args, fields, sum.SumAggregation)
 }
 
 // Description is auto-generated description, based on output of https://github.com/graphite-project/graphite-web

diff --git a/pkg/expr/helper/helper.go b/pkg/expr/helper/helper.go
@@ -146,6 +146,47 @@ func AggregateSeries(name string, args []*types.MetricData, absent_if_first_seri
 	return []*types.MetricData{ret}, nil
 }
 
+// AggregateSeries aggregates series with wildcarcs
+func AggregateSeriesWithWildcards(name string, args []*types.MetricData, fields []int, function AggregateFunc) ([]*types.MetricData, error) {
+	var results []*types.MetricData
+
+	nodeList := []string{}
+	groups := make(map[string][]*types.MetricData)
+
+	for _, a := range args {
+		metric := ExtractMetric(a.Name)
+		nodes := strings.Split(metric, ".")
+		var s []string
+		// Yes, this is O(n^2), but len(nodes) < 10 and len(fields) < 3
+		// Iterating an int slice is faster than a map for n ~ 30
+		// http://www.antoine.im/posts/someone_is_wrong_on_the_internet
+		for i, n := range nodes {
+			if !Contains(fields, i) {
+				s = append(s, n)
+			}
+		}
+
+		node := strings.Join(s, ".")
+
+		if len(groups[node]) == 0 {
+			nodeList = append(nodeList, node)
+		}
+
+		groups[node] = append(groups[node], a)
+	}
+
+	for _, series := range nodeList {
+		groupArgs := groups[series]
+		groupName := fmt.Sprintf("%s(%s)", name, series)
+		res, err := AggregateSeries(groupName, groupArgs, false, false, function)
+		if err != nil {
+			return nil, err
+		}
+		results = append(results, res[0])
+	}
+	return results, nil
+}
+
 // SummarizeValues summarizes values
 func SummarizeValues(f string, values []float64) (float64, bool, error) {
 	rv := 0.0