From b82e2cabc53220f963e99f594e50ead5a6418981 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Wed, 18 Sep 2024 17:38:16 +0200 Subject: [PATCH] app/vmselect/promql: properly calculate `c1 and c2` and `c1 or c2` by upgrading github.com/VictoriaMetrics/metricsql to v0.79.0 The fix is in the https://github.com/VictoriaMetrics/metricsql/pull/34 Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6637 --- app/vmselect/promql/exec_test.go | 6 +++ docs/changelog/CHANGELOG.md | 2 +- go.mod | 2 +- go.sum | 4 +- .../VictoriaMetrics/metricsql/binary_op.go | 4 +- .../metricsql/binaryop/funcs.go | 16 +++++++ .../VictoriaMetrics/metricsql/parser.go | 42 +++++++++++++------ vendor/modules.txt | 2 +- 8 files changed, 59 insertions(+), 19 deletions(-) diff --git a/app/vmselect/promql/exec_test.go b/app/vmselect/promql/exec_test.go index 3a6faf6de..a3edd2c4e 100644 --- a/app/vmselect/promql/exec_test.go +++ b/app/vmselect/promql/exec_test.go @@ -3027,6 +3027,12 @@ func TestExecSuccess(t *testing.T) { resultExpected := []netstorage.Result{r} f(q, resultExpected) }) + t.Run(`1 and (0 > 1)`, func(t *testing.T) { + // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6637 + t.Parallel() + q := `1 and (0 > 1)` + f(q, nil) + }) t.Run(`time() and 2`, func(t *testing.T) { t.Parallel() q := `time() and 2` diff --git a/docs/changelog/CHANGELOG.md b/docs/changelog/CHANGELOG.md index 54b1769cc..49a2498b6 100644 --- a/docs/changelog/CHANGELOG.md +++ b/docs/changelog/CHANGELOG.md @@ -36,8 +36,8 @@ See also [LTS releases](https://docs.victoriametrics.com/lts-releases/). * BUGFIX: [Single-node VictoriaMetrics](https://docs.victoriametrics.com/) and `vmstorage` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/cluster-victoriametrics/): fix metric `vm_object_references{type="indexdb"}`. Previously, it was overcounted. * BUGFIX: [Single-node VictoriaMetrics](https://docs.victoriametrics.com/) and `vmstorage` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/cluster-victoriametrics/): properly ingest stale NaN samples. Previously it could be dropped if series didn't exist at storage node. See this issue [https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5069] for details. * BUGFIX: [Single-node VictoriaMetrics](https://docs.victoriametrics.com/) and `vmstorage` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/cluster-victoriametrics/): properly track `vm_missing_tsids_for_metric_id_total` metric. See this [issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6931) for details. - * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert): do not send notifications without labels to Alertmanager. Such notifications are rejected by Alertmanager anyway. Before, vmalert could send alert notifications even if no label-value pairs left after applying `alert_relabel_configs` from [notifier config](https://docs.victoriametrics.com/vmalert/#notifier-configuration-file). +* BUGFIX: [MetricsQL](https://docs.victoriametrics.com/metricsql/): properly handle `c1 AND c2` and `c1 OR c1` queries for constants `c1` and `c2`. Previously such queries could return unexpected results. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6637). ## [v1.103.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.103.0) diff --git a/go.mod b/go.mod index 781bdd217..081fd8f4a 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/VictoriaMetrics/easyproto v0.1.4 github.com/VictoriaMetrics/fastcache v1.12.2 github.com/VictoriaMetrics/metrics v1.35.1 - github.com/VictoriaMetrics/metricsql v0.77.0 + github.com/VictoriaMetrics/metricsql v0.79.0 github.com/aws/aws-sdk-go-v2 v1.30.5 github.com/aws/aws-sdk-go-v2/config v1.27.33 github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.17.18 diff --git a/go.sum b/go.sum index a2d1108f3..c422b0a22 100644 --- a/go.sum +++ b/go.sum @@ -74,8 +74,8 @@ github.com/VictoriaMetrics/fastcache v1.12.2/go.mod h1:AmC+Nzz1+3G2eCPapF6UcsnkT github.com/VictoriaMetrics/metrics v1.34.0/go.mod h1:r7hveu6xMdUACXvB8TYdAj8WEsKzWB0EkpJN+RDtOf8= github.com/VictoriaMetrics/metrics v1.35.1 h1:o84wtBKQbzLdDy14XeskkCZih6anG+veZ1SwJHFGwrU= github.com/VictoriaMetrics/metrics v1.35.1/go.mod h1:r7hveu6xMdUACXvB8TYdAj8WEsKzWB0EkpJN+RDtOf8= -github.com/VictoriaMetrics/metricsql v0.77.0 h1:eD+1RuIBQmbSPdl8ItbghxLifE+gexJxQBWKSJYwhBE= -github.com/VictoriaMetrics/metricsql v0.77.0/go.mod h1:1g4hdCwlbJZ851PU9VN65xy9Rdlzupo6fx3SNZ8Z64U= +github.com/VictoriaMetrics/metricsql v0.79.0 h1:6wU5oiHMAb0a59So5fV8nvssIfhXaB58wwrRhXf8sdg= +github.com/VictoriaMetrics/metricsql v0.79.0/go.mod h1:1g4hdCwlbJZ851PU9VN65xy9Rdlzupo6fx3SNZ8Z64U= 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/binary_op.go b/vendor/github.com/VictoriaMetrics/metricsql/binary_op.go index 5005089ec..ecb83f326 100644 --- a/vendor/github.com/VictoriaMetrics/metricsql/binary_op.go +++ b/vendor/github.com/VictoriaMetrics/metricsql/binary_op.go @@ -191,9 +191,9 @@ func binaryOpEvalNumber(op string, left, right float64, isBool bool) float64 { case "^": left = binaryop.Pow(left, right) case "and": - // Nothing to do + left = binaryop.And(left, right) case "or": - // Nothing to do + left = binaryop.Or(left, right) case "unless": left = nan case "default": diff --git a/vendor/github.com/VictoriaMetrics/metricsql/binaryop/funcs.go b/vendor/github.com/VictoriaMetrics/metricsql/binaryop/funcs.go index 1cce8cba4..3c50b3bdf 100644 --- a/vendor/github.com/VictoriaMetrics/metricsql/binaryop/funcs.go +++ b/vendor/github.com/VictoriaMetrics/metricsql/binaryop/funcs.go @@ -107,3 +107,19 @@ func Ifnot(left, right float64) float64 { } return nan } + +// And return left if left and right is not NaN. Otherwise, NaN is returned. +func And(left, right float64) float64 { + if math.IsNaN(left) || math.IsNaN(right) { + return nan + } + return left +} + +// Or return the first non-NaN item. If both left and right are NaN, it returns NaN. +func Or(left, right float64) float64 { + if !math.IsNaN(left) { + return left + } + return right +} diff --git a/vendor/github.com/VictoriaMetrics/metricsql/parser.go b/vendor/github.com/VictoriaMetrics/metricsql/parser.go index 4f23b157d..7363abe1f 100644 --- a/vendor/github.com/VictoriaMetrics/metricsql/parser.go +++ b/vendor/github.com/VictoriaMetrics/metricsql/parser.go @@ -1368,6 +1368,13 @@ func (p *parser) parseLabelFilterExpr() (*labelFilterExpr, error) { lfe.IsNegative = true lfe.IsRegexp = true case ",", "}", "or": + // Incomplete label filter 'lf' in the following forms: + // + // - {lf} + // - {lf,other="filter"} + // - {lf or other="filter"} + // + // It must be substituted by complete label filter during WITH template expand. return &lfe, nil default: return nil, fmt.Errorf(`labelFilterExpr: unexpected token %q; want "=", "!=", "=~", "!~", ",", "or", "}"`, p.lex.Token) @@ -1388,8 +1395,12 @@ func (p *parser) parseLabelFilterExpr() (*labelFilterExpr, error) { // // This type isn't exported. type labelFilterExpr struct { - Label string - Value *StringExpr + // Label contains either the label name or the WITH template reference. + Label string + + // Value can be nil if Label contains unexpanded WITH template reference. + Value *StringExpr + IsRegexp bool IsNegative bool } @@ -1658,7 +1669,7 @@ func (p *parser) parseMetricExpr() (*MetricExpr, error) { return nil, err } if p.lex.Token != "{" { - me.labelFilterss = append(me.labelFilterss[:0], []*labelFilterExpr{mf}) + me.labelFilterss = append(me.labelFilterss, []*labelFilterExpr{mf}) return &me, nil } } @@ -2009,7 +2020,7 @@ func (we *withExpr) AppendString(dst []byte) []byte { for i, wa := range we.Was { dst = wa.AppendString(dst) if i+1 < len(we.Was) { - dst = append(dst, ',') + dst = append(dst, ", "...) } } dst = append(dst, ") "...) @@ -2246,15 +2257,15 @@ func isOnlyMetricNameInLabelFilterss(lfss [][]*labelFilterExpr) bool { } func getMetricNameFromLabelFilterss(lfss [][]*labelFilterExpr) string { - if len(lfss) == 0 || len(lfss[0]) == 0 || lfss[0][0].Label != "__name__" || len(lfss[0][0].Value.tokens) != 1 { + if len(lfss) == 0 { + return "" + } + metricName := mustGetMetricName(lfss[0]) + if metricName == "" { return "" } - metricName := mustExtractMetricNameFromToken(lfss[0][0].Value.tokens[0]) for _, lfs := range lfss[1:] { - if len(lfs) == 0 || lfs[0].Label != "__name__" || len(lfs[0].Value.tokens) != 1 { - return "" - } - metricNameLocal := mustExtractMetricNameFromToken(lfs[0].Value.tokens[0]) + metricNameLocal := mustGetMetricName(lfs) if metricNameLocal != metricName { return "" } @@ -2262,8 +2273,15 @@ func getMetricNameFromLabelFilterss(lfss [][]*labelFilterExpr) string { return metricName } -func mustExtractMetricNameFromToken(token string) string { - metricName, err := extractStringValue(token) +func mustGetMetricName(lfss []*labelFilterExpr) string { + if len(lfss) == 0 { + return "" + } + lfs := lfss[0] + if lfs.Label != "__name__" || lfs.Value == nil || len(lfs.Value.tokens) != 1 { + return "" + } + metricName, err := extractStringValue(lfs.Value.tokens[0]) if err != nil { panic(fmt.Errorf("BUG: cannot obtain metric name: %w", err)) } diff --git a/vendor/modules.txt b/vendor/modules.txt index fe7b60419..c0b366f6f 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -118,7 +118,7 @@ github.com/VictoriaMetrics/fastcache # github.com/VictoriaMetrics/metrics v1.35.1 ## explicit; go 1.17 github.com/VictoriaMetrics/metrics -# github.com/VictoriaMetrics/metricsql v0.77.0 +# github.com/VictoriaMetrics/metricsql v0.79.0 ## explicit; go 1.13 github.com/VictoriaMetrics/metricsql github.com/VictoriaMetrics/metricsql/binaryop