Repository: VictoriaMetrics/VictoriaMetrics
Pull request #4991 (2023-09-11 11:55:40)

vmagent: add validation of MetricsQL functions
This PR adds the ability to check MetricsQL functions when vmalert executes rules.

Related issue: https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4933

Signed-off-by: dmitryk-dk [d.kozlov@victoriametrics.com](mailto:d.kozlov@victoriametrics.com)

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/4991?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) Report
Patch coverage has no change and project coverage change: **`-0.05%`** :warning:
> Comparison is base [(`a740950`)](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/commit/a7409500fc86b313207bfffab7f4544e3e230b38?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) 60.42% compared to head [(`4a5fc42`)](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/4991?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics) 60.37%.
> Report is 3 commits behind head on master.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #4991      +/-   ##
==========================================
- Coverage   60.42%   60.37%   -0.05%     
==========================================
  Files         388      388              
  Lines       72732    72732              
==========================================
- Hits        43945    43912      -33     
- Misses      26391    26419      +28     
- Partials     2396     2401       +5     
```


[see 11 files with indirect coverage changes](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/4991/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics)


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/4991?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=VictoriaMetrics).


Patch:
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index 115b61ac2..30c817879 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -42,6 +42,7 @@ The following `tip` changes can be tested by building VictoriaMetrics components
 ssue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4825) and [these docs](https://docs.victoriametrics.com/vmauth.html#auth-config) for details.
 * FEATURE: [vmauth](https://docs.victoriametrics.com/vmauth.html): add ability to retry requests to the [remaining backends](https://docs.victoriametrics.com/vmauth.html#load-balancing) if they return response status codes specified in the `retry_status_codes` list. See [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4893).
 * FEATURE: [vmalert](https://docs.victoriametrics.com/vmalert.html): add `eval_offset` attribute for [Groups](https://docs.victoriametrics.com/vmalert.html#groups). If specified, Group will be evaluated at the exact time offset on the range of [0...evaluationInterval]. The setting might be useful for cron-like rules which must be evaluated at specific moments of time. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3409) for details.
+* FEATURE: [vmalert](https://docs.victoriametrics.com/vmalert.html): validate [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html) function names in alerting and recording rules when `vmalert` runs with `-dryRun` command-line flag. Previously it was allowed to use unknown (aka invalid) MetricsQL function names there. For example, `foo()` was counted as a valid query. See [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4933).
 * FEATURE: limit the length of string params in log messages to 500 chars. Longer string params are replaced with the `first_250_chars..last_250_chars`. This prevents from too long log lines, which can be emitted by VictoriaMetrics components.
 
 * BUGFIX: [Official Grafana dashboards for VictoriaMetrics](https://grafana.com/orgs/victoriametrics): fix display of ingested rows rate for `Samples ingested/s` and `Samples rate` panels for vmagent's dasbhoard. Previously, not all ingested protocols were accounted in these panels. An extra panel `Rows rate` was added to `Ingestion` section to display the split for rows ingested rate by protocol.
diff --git a/go.mod b/go.mod
index 603c4a567..a6ee13e10 100644
--- a/go.mod
+++ b/go.mod
@@ -12,7 +12,7 @@ require (
 	// like https://github.com/valyala/fasthttp/commit/996610f021ff45fdc98c2ce7884d5fa4e7f9199b
 	github.com/VictoriaMetrics/fasthttp v1.2.0
 	github.com/VictoriaMetrics/metrics v1.24.0
-	github.com/VictoriaMetrics/metricsql v0.64.0
+	github.com/VictoriaMetrics/metricsql v0.65.0
 	github.com/aws/aws-sdk-go-v2 v1.21.0
 	github.com/aws/aws-sdk-go-v2/config v1.18.39
 	github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.11.83
diff --git a/go.sum b/go.sum
index 0b5db09a0..9bfe01c62 100644
--- a/go.sum
+++ b/go.sum
@@ -70,8 +70,8 @@ github.com/VictoriaMetrics/fasthttp v1.2.0 h1:nd9Wng4DlNtaI27WlYh5mGXCJOmee/2c2b
 github.com/VictoriaMetrics/fasthttp v1.2.0/go.mod h1:zv5YSmasAoSyv8sBVexfArzFDIGGTN4TfCKAtAw7IfE=
 github.com/VictoriaMetrics/metrics v1.24.0 h1:ILavebReOjYctAGY5QU2F9X0MYvkcrG3aEn2RKa1Zkw=
 github.com/VictoriaMetrics/metrics v1.24.0/go.mod h1:eFT25kvsTidQFHb6U0oa0rTrDRdz4xTYjpL8+UPohys=
-github.com/VictoriaMetrics/metricsql v0.64.0 h1:uty6AXQFY3OpQ+eopo1jDjCcTctuqkqYLnRbQVhukW8=
-github.com/VictoriaMetrics/metricsql v0.64.0/go.mod h1:k4UaP/+CjuZslIjd+kCigNG9TQmUqh5v0TP/nMEy90I=
+github.com/VictoriaMetrics/metricsql v0.65.0 h1:+/Oit3QycM8z/NbMHy4KENSUDS5q9QRx8h2x6cvoQOk=
+github.com/VictoriaMetrics/metricsql v0.65.0/go.mod h1:k4UaP/+CjuZslIjd+kCigNG9TQmUqh5v0TP/nMEy90I=
 github.com/VividCortex/ewma v1.2.0 h1:f58SaIzcDXrSy3kWaHNvuJgJ3Nmz59Zji6XoJR/q1ow=
 github.com/VividCortex/ewma v1.2.0/go.mod h1:nz4BbCtbLyFDeC9SUHbtcT5644juEuWfUAUnGx7j5l4=
 github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
diff --git a/vendor/github.com/VictoriaMetrics/metricsql/aggr.go b/vendor/github.com/VictoriaMetrics/metricsql/aggr.go
index b8c77c349..51bc36a0f 100644
--- a/vendor/github.com/VictoriaMetrics/metricsql/aggr.go
+++ b/vendor/github.com/VictoriaMetrics/metricsql/aggr.go
@@ -43,7 +43,8 @@ var aggrFuncs = map[string]bool{
 	"zscore":         true,
 }
 
-func isAggrFunc(s string) bool {
+// IsAggrFunc returns whether funcName is a known aggregate function.
+func IsAggrFunc(s string) bool {
 	s = strings.ToLower(s)
 	return aggrFuncs[s]
 }
diff --git a/vendor/github.com/VictoriaMetrics/metricsql/optimizer.go b/vendor/github.com/VictoriaMetrics/metricsql/optimizer.go
index 8072667cd..046a8f1b8 100644
--- a/vendor/github.com/VictoriaMetrics/metricsql/optimizer.go
+++ b/vendor/github.com/VictoriaMetrics/metricsql/optimizer.go
@@ -362,7 +362,7 @@ func getFuncArgIdxForOptimization(funcName string, args []Expr) int {
 	if IsTransformFunc(funcName) {
 		return getTransformArgIdxForOptimization(funcName, args)
 	}
-	if isAggrFunc(funcName) {
+	if IsAggrFunc(funcName) {
 		return getAggrArgIdxForOptimization(funcName, args)
 	}
 	return -1
diff --git a/vendor/github.com/VictoriaMetrics/metricsql/parser.go b/vendor/github.com/VictoriaMetrics/metricsql/parser.go
index bc3f83e7f..c40876857 100644
--- a/vendor/github.com/VictoriaMetrics/metricsql/parser.go
+++ b/vendor/github.com/VictoriaMetrics/metricsql/parser.go
@@ -31,6 +31,9 @@ func Parse(s string) (Expr, error) {
 	}
 	e = removeParensExpr(e)
 	e = simplifyConstants(e)
+	if err := checkSupportedFunctions(e); err != nil {
+		return nil, err
+	}
 	return e, nil
 }
 
@@ -596,7 +599,7 @@ func (p *parser) parseParensExpr() (*parensExpr, error) {
 }
 
 func (p *parser) parseAggrFuncExpr() (*AggrFuncExpr, error) {
-	if !isAggrFunc(p.lex.Token) {
+	if !IsAggrFunc(p.lex.Token) {
 		return nil, fmt.Errorf(`AggrFuncExpr: unexpected token %q; want aggregate func`, p.lex.Token)
 	}
 
@@ -1608,7 +1611,7 @@ func (p *parser) parseIdentExpr() (Expr, error) {
 	}
 	if isIdentPrefix(p.lex.Token) {
 		p.lex.Prev()
-		if isAggrFunc(p.lex.Token) {
+		if IsAggrFunc(p.lex.Token) {
 			return p.parseAggrFuncExpr()
 		}
 		return p.parseMetricExpr()
@@ -1620,7 +1623,7 @@ func (p *parser) parseIdentExpr() (Expr, error) {
 	switch p.lex.Token {
 	case "(":
 		p.lex.Prev()
-		if isAggrFunc(p.lex.Token) {
+		if IsAggrFunc(p.lex.Token) {
 			return p.parseAggrFuncExpr()
 		}
 		return p.parseFuncExpr()
diff --git a/vendor/github.com/VictoriaMetrics/metricsql/utils.go b/vendor/github.com/VictoriaMetrics/metricsql/utils.go
index 11828f39f..2405b1fef 100644
--- a/vendor/github.com/VictoriaMetrics/metricsql/utils.go
+++ b/vendor/github.com/VictoriaMetrics/metricsql/utils.go
@@ -1,5 +1,10 @@
 package metricsql
 
+import (
+	"fmt"
+	"strings"
+)
+
 // ExpandWithExprs expands WITH expressions inside q and returns the resulting
 // PromQL without WITH expressions.
 func ExpandWithExprs(q string) (string, error) {
@@ -14,7 +19,7 @@ func ExpandWithExprs(q string) (string, error) {
 // VisitAll recursively calls f for all the Expr children in e.
 //
 // It visits leaf children at first and then visits parent nodes.
-// It is safe modifying expr in f.
+// It is safe modifying e in f.
 func VisitAll(e Expr, f func(expr Expr)) {
 	switch expr := e.(type) {
 	case *BinaryOpExpr:
@@ -36,3 +41,38 @@ func VisitAll(e Expr, f func(expr Expr)) {
 	}
 	f(e)
 }
+
+// IsSupportedFunction returns true if funcName contains supported MetricsQL function
+func IsSupportedFunction(funcName string) bool {
+	funcName = strings.ToLower(funcName)
+	if IsRollupFunc(funcName) {
+		return true
+	}
+	if IsTransformFunc(funcName) {
+		return true
+	}
+	if IsAggrFunc(funcName) {
+		return true
+	}
+	return false
+}
+
+func checkSupportedFunctions(e Expr) error {
+	var err error
+	VisitAll(e, func(expr Expr) {
+		if err != nil {
+			return
+		}
+		switch t := expr.(type) {
+		case *FuncExpr:
+			if !IsRollupFunc(t.Name) && !IsTransformFunc(t.Name) {
+				err = fmt.Errorf("unsupported function %q", t.Name)
+			}
+		case *AggrFuncExpr:
+			if !IsAggrFunc(t.Name) {
+				err = fmt.Errorf("unsupported aggregate function %q", t.Name)
+			}
+		}
+	})
+	return err
+}
diff --git a/vendor/modules.txt b/vendor/modules.txt
index 1dbceba9a..7c406cb47 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -99,7 +99,7 @@ github.com/VictoriaMetrics/fasthttp/stackless
 # github.com/VictoriaMetrics/metrics v1.24.0
 ## explicit; go 1.20
 github.com/VictoriaMetrics/metrics
-# github.com/VictoriaMetrics/metricsql v0.64.0
+# github.com/VictoriaMetrics/metricsql v0.65.0
 ## explicit; go 1.13
 github.com/VictoriaMetrics/metricsql
 github.com/VictoriaMetrics/metricsql/binaryop


Test patch:
diff --git a/app/vmselect/promql/exec_test.go b/app/vmselect/promql/exec_test.go
index 90d31e349..3290612a7 100644
--- a/app/vmselect/promql/exec_test.go
+++ b/app/vmselect/promql/exec_test.go
@@ -45,7 +45,7 @@ func TestEscapeDotsInRegexpLabelFilters(t *testing.T) {
 	f("2", "2")
 	f(`foo.bar + 123`, `foo.bar + 123`)
 	f(`foo{bar=~"baz.xx.yyy"}`, `foo{bar=~"baz\\.xx\\.yyy"}`)
-	f(`foo(a.b{c="d.e",x=~"a.b.+[.a]",y!~"aaa.bb|cc.dd"}) + x.y(1,sum({x=~"aa.bb"}))`, `foo(a.b{c="d.e",x=~"a\\.b.+[\\.a]",y!~"aaa\\.bb|cc\\.dd"}) + x.y(1, sum({x=~"aa\\.bb"}))`)
+	f(`sum(a.b{c="d.e",x=~"a.b.+[.a]",y!~"aaa.bb|cc.dd"}) + avg_over_time(1,sum({x=~"aa.bb"}))`, `sum(a.b{c="d.e",x=~"a\\.b.+[\\.a]",y!~"aaa\\.bb|cc\\.dd"}) + avg_over_time(1, sum({x=~"aa\\.bb"}))`)
 }
 
 func TestExecSuccess(t *testing.T) {