Skip to content

Commit

Permalink
MQE: address post-merge feedback on #10533 (#10553)
Browse files Browse the repository at this point in the history
* Address PR feedback: use consistent indentation in string

* Address PR feedback: make test expressions explicit
  • Loading branch information
charleskorn authored Feb 3, 2025
1 parent cf0809e commit 4ad7c7d
Showing 1 changed file with 59 additions and 53 deletions.
112 changes: 59 additions & 53 deletions pkg/streamingpromql/functions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ package streamingpromql

import (
"context"
"fmt"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -65,8 +63,8 @@ func TestFunctionDeduplicateAndMerge(t *testing.T) {
load 30s
float_a{env="prod"} _ 0 1 _ _ _ _ _ _ _ _ _ _ _ _
float_b{env="prod"} _ _ _ _ _ _ _ _ _ _ _ _ _ 8 9
histogram_a{env="prod"} _ {{count:0}} {{count:1}} _ _ _ _ _ _ _ _ _ _ _ _
histogram_b{env="prod"} _ _ _ _ _ _ _ _ _ _ _ _ _ {{count:8}} {{count:9}}
histogram_a{env="prod"} _ {{count:0}} {{count:1}} _ _ _ _ _ _ _ _ _ _ _ _
histogram_b{env="prod"} _ _ _ _ _ _ _ _ _ _ _ _ _ {{count:8}} {{count:9}}
`

storage := promqltest.LoadedStorage(t, data)
Expand All @@ -79,49 +77,70 @@ func TestFunctionDeduplicateAndMerge(t *testing.T) {
end := timestamp.Time(0).Add(7 * time.Minute)
step := time.Minute

preSelectorFunctionArgs := map[string]string{
"histogram_quantile": "0.1",
"histogram_fraction": "0, 0.1",
}

postSelectorFunctionArgs := map[string]string{
"clamp": "-Inf, Inf",
"clamp_min": "-Inf",
"clamp_max": "Inf",
"label_replace": `"__name__", "$1", "env", "(.*)"`,
expressions := map[string]string{
// Please keep this list sorted alphabetically.
"abs": `abs({__name__=~"float.*"})`,
"acos": `acos({__name__=~"float.*"})`,
"acosh": `acosh({__name__=~"float.*"})`,
"asin": `asin({__name__=~"float.*"})`,
"asinh": `asinh({__name__=~"float.*"})`,
"atan": `atan({__name__=~"float.*"})`,
"atanh": `atanh({__name__=~"float.*"})`,
"avg_over_time": `avg_over_time({__name__=~"float.*"}[1m])`,
"ceil": `ceil({__name__=~"float.*"})`,
"changes": `changes({__name__=~"float.*"}[1m])`,
"clamp": `clamp({__name__=~"float.*"}, -Inf, Inf)`,
"clamp_max": `clamp_max({__name__=~"float.*"}, -Inf)`,
"clamp_min": `clamp_min({__name__=~"float.*"}, Inf)`,
"cos": `cos({__name__=~"float.*"})`,
"cosh": `cosh({__name__=~"float.*"})`,
"count_over_time": `count_over_time({__name__=~"float.*"}[1m])`,
"deg": `deg({__name__=~"float.*"})`,
"delta": `delta({__name__=~"float.*"}[1m])`,
"deriv": `deriv({__name__=~"float.*"}[1m])`,
"exp": `exp({__name__=~"float.*"})`,
"floor": `floor({__name__=~"float.*"})`,
"histogram_avg": `histogram_avg({__name__=~"histogram.*"})`,
"histogram_count": `histogram_count({__name__=~"histogram.*"})`,
"histogram_fraction": `histogram_fraction(0, 0.1, {__name__=~"histogram.*"})`,
"histogram_quantile": `histogram_quantile(0.1, {__name__=~"histogram.*"})`,
"histogram_stddev": `histogram_stddev({__name__=~"histogram.*"})`,
"histogram_stdvar": `histogram_stdvar({__name__=~"histogram.*"})`,
"histogram_sum": `histogram_sum({__name__=~"histogram.*"})`,
"idelta": `idelta({__name__=~"float.*"}[1m])`,
"increase": `increase({__name__=~"float.*"}[1m])`,
"irate": `irate({__name__=~"float.*"}[1m])`,
"label_replace": `label_replace({__name__=~"float.*"}, "__name__", "$1", "env", "(.*)")`,
"last_over_time": `<skip>`, // last_over_time() doesn't drop the metric name, so this test doesn't apply.
"ln": `ln({__name__=~"float.*"})`,
"log10": `log10({__name__=~"float.*"})`,
"log2": `log2({__name__=~"float.*"})`,
"max_over_time": `max_over_time({__name__=~"float.*"}[1m])`,
"min_over_time": `min_over_time({__name__=~"float.*"}[1m])`,
"present_over_time": `present_over_time({__name__=~"float.*"}[1m])`,
"rad": `rad({__name__=~"float.*"})`,
"rate": `rate({__name__=~"float.*"}[1m])`,
"resets": `resets({__name__=~"float.*"}[1m])`,
"round": `round({__name__=~"float.*"})`,
"sgn": `sgn({__name__=~"float.*"})`,
"sin": `sin({__name__=~"float.*"})`,
"sinh": `sinh({__name__=~"float.*"})`,
"sqrt": `sqrt({__name__=~"float.*"})`,
"sum_over_time": `sum_over_time({__name__=~"float.*"}[1m])`,
"tan": `tan({__name__=~"float.*"})`,
"tanh": `tanh({__name__=~"float.*"})`,
"vector": `<skip>`, // vector() takes a scalar, so this test doesn't apply.
}

for name := range instantVectorFunctionOperatorFactories {
if name == "vector" || name == "last_over_time" {
// This test doesn't apply to vector() because it takes a scalar parameter.
// This test doesn't apply to last_over_time() because it doesn't drop the metric name.
expr, haveExpression := expressions[name]

if expr == "<skip>" {
continue
}

t.Run(name, func(t *testing.T) {
metricType := "float"

if strings.HasPrefix(name, "histogram_") {
metricType = "histogram"
}

selector := fmt.Sprintf(`{__name__=~"%s.*"}`, metricType)

if isFunctionOverRangeVector(name) {
selector = selector + "[1m]"
}

preSelectorArgs := preSelectorFunctionArgs[name]
if preSelectorArgs != "" {
preSelectorArgs = preSelectorArgs + ", "
}

postSelectorArgs := postSelectorFunctionArgs[name]
if postSelectorArgs != "" {
postSelectorArgs = ", " + postSelectorArgs
}

expr := fmt.Sprintf("%s(%s%s%s)", name, preSelectorArgs, selector, postSelectorArgs)
require.Truef(t, haveExpression, "no expression defined for '%s' function", name)

q, err := engine.NewRangeQuery(ctx, storage, nil, expr, start, end, step)
require.NoError(t, err)
Expand All @@ -136,16 +155,3 @@ func TestFunctionDeduplicateAndMerge(t *testing.T) {
})
}
}

func isFunctionOverRangeVector(name string) bool {
if strings.HasSuffix(name, "_over_time") {
return true
}

switch name {
case "changes", "delta", "deriv", "idelta", "increase", "irate", "rate", "resets":
return true
default:
return false
}
}

0 comments on commit 4ad7c7d

Please sign in to comment.