From 62ec9ab3f4e92129ac14384e4e661c7cebbe9bd0 Mon Sep 17 00:00:00 2001 From: Thomas Oettli Date: Sat, 30 Nov 2024 01:55:57 +0100 Subject: [PATCH 1/6] fix SummarizeValues to take absent values into account Add parameter "absent" to func SummarizeValues and ignore absent values when calculating the average, min, last or count values. --- pkg/expr/helper/helper.go | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/pkg/expr/helper/helper.go b/pkg/expr/helper/helper.go index bd6a687a..1bf12a44 100644 --- a/pkg/expr/helper/helper.go +++ b/pkg/expr/helper/helper.go @@ -188,7 +188,7 @@ func AggregateSeriesWithWildcards(name string, args []*types.MetricData, fields } // SummarizeValues summarizes values -func SummarizeValues(f string, values []float64) (float64, bool, error) { +func SummarizeValues(f string, values []float64, absent []bool) (float64, bool, error) { rv := 0.0 if len(values) == 0 { @@ -201,8 +201,10 @@ func SummarizeValues(f string, values []float64) (float64, bool, error) { rv += av } case "avg", "average": - for _, av := range values { - rv += av + for i, av := range values { + if !absent[i] { + rv += av + } } rv /= float64(len(values)) case "max": @@ -214,20 +216,31 @@ func SummarizeValues(f string, values []float64) (float64, bool, error) { } case "min": rv = math.Inf(1) - for _, av := range values { - if av < rv { - rv = av + for i, av := range values { + if !absent[i] { + if av < rv { + rv = av + } } } case "last": - if len(values) > 0 { - rv = values[len(values)-1] + for i := len(values) - 1; i >= 0; i-- { + if !absent[i] { + rv = values[i] + break + } } case "count": - rv = float64(len(values)) + total := 0 + for i, _ := range values { + if !absent[i] { + total++ + } + } + rv = float64(total) case "median": - val, absent := Percentile(values, 50, true) - return val, absent, nil + val, abs := Percentile(values, 50, true) + return val, abs, nil default: looks_like_percentile, err := regexp.MatchString(`^p\d\d?$`, f) if err != nil { @@ -239,8 +252,8 @@ func SummarizeValues(f string, values []float64) (float64, bool, error) { if err != nil { return 0, true, parser.ParseError(err.Error()) } - val, absent := Percentile(values, percent, true) - return val, absent, nil + val, abs := Percentile(values, percent, true) + return val, abs, nil } else { return 0, true, parser.ParseError(fmt.Sprintf("unsupported aggregation function: %s", f)) } From 00788b3ed277b38ecd2ec0e679c2b90aca1b843a Mon Sep 17 00:00:00 2001 From: Thomas Oettli Date: Sat, 30 Nov 2024 02:01:31 +0100 Subject: [PATCH 2/6] add parameter absent to every call to SummarizeValues --- pkg/expr/functions/legendValue/function.go | 2 +- pkg/expr/functions/sortBy/function.go | 6 +++--- pkg/expr/functions/summarize/function.go | 7 +++++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/pkg/expr/functions/legendValue/function.go b/pkg/expr/functions/legendValue/function.go index 87678614..2b72cd17 100644 --- a/pkg/expr/functions/legendValue/function.go +++ b/pkg/expr/functions/legendValue/function.go @@ -57,7 +57,7 @@ func (f *legendValue) Do(ctx context.Context, e parser.Expr, from, until int32, for _, a := range arg { var values []string for _, method := range methods { - summaryVal, _, err := helper.SummarizeValues(method, a.Values) + summaryVal, _, err := helper.SummarizeValues(method, a.Values, a.IsAbsent) if err != nil { return []*types.MetricData{}, err } diff --git a/pkg/expr/functions/sortBy/function.go b/pkg/expr/functions/sortBy/function.go index 75314537..6c1fe70c 100644 --- a/pkg/expr/functions/sortBy/function.go +++ b/pkg/expr/functions/sortBy/function.go @@ -42,11 +42,11 @@ func (f *sortBy) Do(ctx context.Context, e parser.Expr, from, until int32, value for i, a := range arg { switch e.Target() { case "sortByTotal": - vals[i], _, _ = helper.SummarizeValues("sum", a.Values) + vals[i], _, _ = helper.SummarizeValues("sum", a.Values, a.IsAbsent) case "sortByMaxima": - vals[i], _, _ = helper.SummarizeValues("max", a.Values) + vals[i], _, _ = helper.SummarizeValues("max", a.Values, a.IsAbsent) case "sortByMinima": - min, _, _ := helper.SummarizeValues("min", a.Values) + min, _, _ := helper.SummarizeValues("min", a.Values, a.IsAbsent) vals[i] = 1 / min } } diff --git a/pkg/expr/functions/summarize/function.go b/pkg/expr/functions/summarize/function.go index 46b0004d..f96c2c38 100644 --- a/pkg/expr/functions/summarize/function.go +++ b/pkg/expr/functions/summarize/function.go @@ -117,12 +117,14 @@ func (f *summarize) Do(ctx context.Context, e parser.Expr, from, until int32, va t := arg.StartTime // unadjusted bucketEnd := start + bucketSize values := make([]float64, 0, bucketSize/arg.StepTime) + absent := make([]bool, 0, bucketSize/arg.StepTime) ridx := 0 bucketItems := 0 for i, v := range arg.Values { bucketItems++ if !arg.IsAbsent[i] { values = append(values, v) + absent = append(absent, false) } t += arg.StepTime @@ -132,7 +134,7 @@ func (f *summarize) Do(ctx context.Context, e parser.Expr, from, until int32, va } if t >= bucketEnd { - r.Values[ridx], r.IsAbsent[ridx], err = helper.SummarizeValues(summarizeFunction, values) + r.Values[ridx], r.IsAbsent[ridx], err = helper.SummarizeValues(summarizeFunction, values, absent) if err != nil { return []*types.MetricData{}, err } @@ -140,12 +142,13 @@ func (f *summarize) Do(ctx context.Context, e parser.Expr, from, until int32, va bucketEnd += bucketSize bucketItems = 0 values = values[:0] + absent = absent[:0] } } // last partial bucket if bucketItems > 0 { - r.Values[ridx], r.IsAbsent[ridx], err = helper.SummarizeValues(summarizeFunction, values) + r.Values[ridx], r.IsAbsent[ridx], err = helper.SummarizeValues(summarizeFunction, values, absent) if err != nil { return []*types.MetricData{}, err } From 22bbd2cf6718037523e46c9156465dfd45698aef Mon Sep 17 00:00:00 2001 From: Thomas Oettli Date: Sat, 30 Nov 2024 02:25:52 +0100 Subject: [PATCH 3/6] fix SummarizeValues average value --- pkg/expr/helper/helper.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/expr/helper/helper.go b/pkg/expr/helper/helper.go index 1bf12a44..d90c1ac1 100644 --- a/pkg/expr/helper/helper.go +++ b/pkg/expr/helper/helper.go @@ -201,12 +201,16 @@ func SummarizeValues(f string, values []float64, absent []bool) (float64, bool, rv += av } case "avg", "average": + total := 0 for i, av := range values { if !absent[i] { rv += av + total++ } } - rv /= float64(len(values)) + if total > 0 { + rv /= float64(total) + } case "max": rv = math.Inf(-1) for _, av := range values { From 51fab5fbf3379e98c3b2752654667f47970bc8a9 Mon Sep 17 00:00:00 2001 From: Thomas Oettli Date: Sat, 30 Nov 2024 02:29:58 +0100 Subject: [PATCH 4/6] reset average value to 0.0 if all values in serie are absent --- pkg/expr/helper/helper.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/expr/helper/helper.go b/pkg/expr/helper/helper.go index d90c1ac1..3a2cf275 100644 --- a/pkg/expr/helper/helper.go +++ b/pkg/expr/helper/helper.go @@ -210,6 +210,8 @@ func SummarizeValues(f string, values []float64, absent []bool) (float64, bool, } if total > 0 { rv /= float64(total) + } else { + rv = 0.0 } case "max": rv = math.Inf(-1) From bf89303dceda0c0b25b9553a2ec61af42da19788 Mon Sep 17 00:00:00 2001 From: Thomas Oettli Date: Sat, 30 Nov 2024 02:35:49 +0100 Subject: [PATCH 5/6] simplify for loop --- pkg/expr/helper/helper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/expr/helper/helper.go b/pkg/expr/helper/helper.go index 3a2cf275..34b4a888 100644 --- a/pkg/expr/helper/helper.go +++ b/pkg/expr/helper/helper.go @@ -238,7 +238,7 @@ func SummarizeValues(f string, values []float64, absent []bool) (float64, bool, } case "count": total := 0 - for i, _ := range values { + for i := range values { if !absent[i] { total++ } From 57762a2749679eac5ddea6abcf44a4b23060d821 Mon Sep 17 00:00:00 2001 From: Thomas Oettli Date: Sat, 30 Nov 2024 02:40:54 +0100 Subject: [PATCH 6/6] revert needless resetting of average value --- pkg/expr/helper/helper.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/expr/helper/helper.go b/pkg/expr/helper/helper.go index 34b4a888..b0c6c111 100644 --- a/pkg/expr/helper/helper.go +++ b/pkg/expr/helper/helper.go @@ -210,8 +210,6 @@ func SummarizeValues(f string, values []float64, absent []bool) (float64, bool, } if total > 0 { rv /= float64(total) - } else { - rv = 0.0 } case "max": rv = math.Inf(-1)