app/vmselect: implement cmd-line flags -search.disableImplicitConversions and -search.logImplicitConversions (#6180)

address https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4338
support disable or log [implicit
conversions](https://docs.victoriametrics.com/metricsql/#implicit-query-conversions)
for subquery with cmd-line flags `-search.disableImplicitConversion` and
`-search.logImplicitConversion`

Signed-off-by: hagen1778 <roman@victoriametrics.com>
Co-authored-by: hagen1778 <roman@victoriametrics.com>
This commit is contained in:
Hui Wang 2024-04-25 18:54:42 +08:00 committed by GitHub
parent 57b7d16259
commit dd0d2c77c8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 179 additions and 4 deletions

View file

@ -13,6 +13,7 @@ import (
"github.com/VictoriaMetrics/VictoriaMetrics/app/vmselect/netstorage"
"github.com/VictoriaMetrics/VictoriaMetrics/app/vmselect/querystats"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/decimal"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/querytracer"
"github.com/VictoriaMetrics/VictoriaMetrics/lib/storage"
"github.com/VictoriaMetrics/metrics"
@ -26,6 +27,12 @@ var (
`For example, foo{bar=~"a.b.c"} will be automatically converted to foo{bar=~"a\\.b\\.c"}, i.e. all the dots in regexp filters will be automatically escaped `+
`in order to match only dot char instead of matching any char. Dots in ".+", ".*" and ".{n}" regexps aren't escaped. `+
`This option is DEPRECATED in favor of {__graphite__="a.*.c"} syntax for selecting metrics matching the given Graphite metrics filter`)
disableImplicitConversion = flag.Bool("search.disableImplicitConversion", false, "Whether to return an error for queries that rely on implicit subquery conversions, "+
"see https://docs.victoriametrics.com/metricsql/#subqueries for details. "+
"See also -search.logImplicitConversion.")
logImplicitConversion = flag.Bool("search.logImplicitConversion", false, "Whether to log queries with implicit subquery conversions, "+
"see https://docs.victoriametrics.com/metricsql/#subqueries for details. "+
"Such conversion can be disabled using -search.disableImplicitConversion.")
)
// UserReadableError is a type of error which supposed to be returned to the user without additional context.
@ -63,6 +70,16 @@ func Exec(qt *querytracer.Tracer, ec *EvalConfig, q string, isFirstPointOnly boo
return nil, err
}
if *disableImplicitConversion || *logImplicitConversion {
complete := isSubQueryComplete(e, false)
if !complete && *disableImplicitConversion {
return nil, fmt.Errorf("query contains subquery that requires implicit conversion and is rejected according to `-search.disableImplicitConversion=true` setting. See https://docs.victoriametrics.com/metricsql/#subqueries for details")
}
if !complete && *logImplicitConversion {
logger.Warnf("query=%q contains subquery that requires implicit conversion, see https://docs.victoriametrics.com/metricsql/#subqueries for details", e.AppendString(nil))
}
}
qid := activeQueriesV.Add(ec, q)
rv, err := evalExpr(qt, ec, e)
activeQueriesV.Remove(qid)
@ -404,3 +421,55 @@ func (pc *parseCache) Put(q string, pcv *parseCacheValue) {
pc.m[q] = pcv
pc.mu.Unlock()
}
// isSubQueryComplete checks if expr contains incomplete subquery
func isSubQueryComplete(e metricsql.Expr, isSubExpr bool) bool {
switch exp := e.(type) {
case *metricsql.FuncExpr:
if isSubExpr {
return false
}
fe := e.(*metricsql.FuncExpr)
for _, arg := range exp.Args {
if getRollupFunc(fe.Name) != nil {
isSubExpr = true
}
if !isSubQueryComplete(arg, isSubExpr) {
return false
}
}
case *metricsql.RollupExpr:
if _, ok := exp.Expr.(*metricsql.MetricExpr); ok {
return true
}
// exp.Step is optional in subqueries
if exp.Window == nil {
return false
}
return isSubQueryComplete(exp.Expr, false)
case *metricsql.AggrFuncExpr:
if isSubExpr {
return false
}
for _, arg := range exp.Args {
if !isSubQueryComplete(arg, false) {
return false
}
}
case *metricsql.BinaryOpExpr:
if isSubExpr {
return false
}
if !isSubQueryComplete(exp.Left, false) {
return false
}
if !isSubQueryComplete(exp.Right, false) {
return false
}
case *metricsql.MetricExpr:
return true
default:
return true
}
return true
}

View file

@ -9432,3 +9432,104 @@ func testAddLabels(t *testing.T, mn *storage.MetricName, labels ...string) {
})
}
}
func TestIsSubQueryCompleteTrue(t *testing.T) {
f := func(q string) {
t.Helper()
e, err := metricsql.Parse(q)
if err != nil {
t.Fatal(err)
}
if !isSubQueryComplete(e, false) {
t.Fatalf("query should be complete: %s", e.AppendString(nil))
}
}
f("rate(http_total)")
f("sum(http_total)")
f("absent(http_total)")
f("rate(http_total[1m])")
f("avg_over_time(up[1m])")
f("sum(http_total[1m])")
f("sum(rate(http_total))")
f("sum(sum(http_total))")
f(`sum(sum_over_time(http_total[1m] )) by (instance)`)
f("sum(up{cluster='a'}[1m] or up{cluster='b'}[1m])")
f("(avg_over_time(alarm_test1[1m]) - avg_over_time(alarm_test1[1m] offset 5m)) > 0.1")
f("http_total[1m] offset 1m")
// subquery
f("rate(http_total)[5m:1m]")
f("rate(sum(http_total)[5m:1m])")
f("rate(rate(http_total)[5m:1m])")
f("sum(rate(http_total[1m]))")
f("sum(rate(sum(http_total)[5m:1m]))")
f("rate(sum(rate(http_total))[5m:1m])")
f("rate(sum(sum(http_total))[5m:1m])")
f("rate(sum(rate(http_total))[5m:1m])")
f("rate(sum(sum(http_total))[5m:1m])")
f("avg_over_time(rate(http_total[5m])[5m:1m])")
f("delta(avg_over_time(up[1m])[5m:1m]) > 0.1")
f("avg_over_time(avg by (site) (metric)[2m:1m])")
f("sum(http_total)[5m:1m] offset 1m")
f("round(sum(sum_over_time(http_total[1m])) by (instance)) [5m:1m] offset 1m")
f("rate(sum(http_total)[5m:1m]) - rate(sum(http_total)[5m:1m])")
f("avg_over_time((rate(http_total)-rate(http_total))[5m:1m])")
f("sum_over_time((up{cluster='a'} or up{cluster='b'})[5m:1m])")
f("sum_over_time((up{cluster='a'} or up{cluster='b'})[5m:1m])")
f("sum(sum_over_time((up{cluster='a'} or up{cluster='b'})[5m:1m])) by (instance)")
// step (or resolution) is optional in subqueries
f("max_over_time(rate(my_counter_total[5m])[1h:])")
f("max_over_time(rate(my_counter_total[5m])[1h:1m])[5m:1m]")
f("max_over_time(rate(my_counter_total[5m])[1h:])[5m:]")
f(`
WITH (
cpuSeconds = node_cpu_seconds_total{instance=~"$node:$port",job=~"$job"},
cpuIdle = rate(cpuSeconds{mode='idle'}[5m])
)
max_over_time(cpuIdle[1h:])`)
}
func TestIsSubQueryCompleteFalse(t *testing.T) {
f := func(q string) {
t.Helper()
e, err := metricsql.Parse(q)
if err != nil {
t.Fatal(err)
}
if isSubQueryComplete(e, false) {
t.Fatalf("expect to detect incomplete subquery: %s", e.AppendString(nil))
}
}
f("rate(sum(http_total))")
f("rate(rate(http_total))")
f("sum(rate(sum(http_total)))")
f("rate(sum(rate(http_total)))")
f("rate(sum(sum(http_total)))")
f("avg_over_time(rate(http_total[5m]))")
// https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3974
f("sum(http_total) offset 1m")
f(`round(sum(sum_over_time(http_total[1m])) by (instance)) offset 1m`)
f("rate(sum(http_total)) - rate(sum(http_total))")
f("avg_over_time(rate(http_total)-rate(http_total))")
// https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3996
f("sum_over_time(up{cluster='a'} or up{cluster='b'})")
f("sum_over_time(up{cluster='a'}[1m] or up{cluster='b'}[1m])")
f("sum(sum_over_time(up{cluster='a'}[1m] or up{cluster='b'}[1m])) by (instance)")
f(`
WITH (
cpuSeconds = node_cpu_seconds_total{instance=~"$node:$port",job=~"$job"},
cpuIdle = rate(cpuSeconds{mode='idle'}[5m])
)
max_over_time(cpuIdle)`)
}

View file

@ -2211,7 +2211,8 @@ MetricsQL supports and extends PromQL subqueries. See [this article](https://val
Any [rollup function](#rollup-functions) for something other than [series selector](https://docs.victoriametrics.com/keyconcepts/#filtering) form a subquery.
Nested rollup functions can be implicit thanks to the [implicit query conversions](#implicit-query-conversions).
For example, `delta(sum(m))` is implicitly converted to `delta(sum(default_rollup(m))[1i:1i])`, so it becomes a subquery,
since it contains [default_rollup](#default_rollup) nested into [delta](#delta).
since it contains [default_rollup](#default_rollup) nested into [delta](#delta).
This behavior can be disabled or logged via cmd-line flags `-search.disableImplicitConversion` and `-search.logImplicitConversion` since v1.101.0.
VictoriaMetrics performs subqueries in the following way:
@ -2241,7 +2242,8 @@ VictoriaMetrics performs the following implicit conversions for incoming queries
* `abs(temperature)` is transformed to `abs(default_rollup(temperature))`, because [abs](#abs) isn't a [rollup function](#rollup-functions) -
it is [transform function](#transform-functions)
* If `step` in square brackets is missing inside [subquery](#subqueries), then `1i` step is automatically added there.
For example, `avg_over_time(rate(http_requests_total[5m])[1h])` is automatically converted to `avg_over_time(rate(http_requests_total[5m])[1h:1i])`.
For example, `avg_over_time(rate(http_requests_total[5m])[1h])` is automatically converted to `avg_over_time(rate(http_requests_total[5m])[1h:1i])`.
* If something other than [series selector](https://docs.victoriametrics.com/keyconcepts/#filtering)
is passed to [rollup function](#rollup-functions), then a [subquery](#subqueries) with `1i` lookbehind window and `1i` step is automatically formed.
For example, `rate(sum(up))` is automatically converted to `rate((sum(default_rollup(up)))[1i:1i])`.
For example, `rate(sum(up))` is automatically converted to `rate((sum(default_rollup(up)))[1i:1i])`.
This behavior can be disabled or logged via cmd-line flags `-search.disableImplicitConversion` and `-search.logImplicitConversion` since v1.101.0.

View file

@ -33,6 +33,7 @@ See also [LTS releases](https://docs.victoriametrics.com/lts-releases/).
* FEATURE: [MetricsQL](https://docs.victoriametrics.com/metricsql/): support filtering by multiple numeric constants via `q == (C1, ..., CN)` and `q != (C1, ..., CN)` syntax. For example, `status_code == (200, 201, 300)` returns `status_code` metrics with any of `200`, `201` or `300` values, while `status_code != (400, 404, 500)` returns `status_code` metrics with all the values except of `400`, `404` and `500`.
* FEATURE: [VictoriaMetrics cluster](https://docs.victoriametrics.com/cluster-victoriametrics/): add support for fault domain awareness to `vmselect`. It can be configured to return full responses if up to `-globalReplicationFactor - 1` fault domains (aka `vmstorage` groups) are unavailable. See [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6054) and [these docs](https://docs.victoriametrics.com/cluster-victoriametrics/#vmstorage-groups-at-vmselect).
* FEATURE: all VictoriaMetrics [enterprise](https://docs.victoriametrics.com/enterprise/) components: add support for automatic issuing of TLS certificates for HTTPS server at `-httpListenAddr` via [Let's Encrypt service](https://letsencrypt.org/). See [these docs](https://docs.victoriametrics.com/#automatic-issuing-of-tls-certificates) and [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5949).
* FEATURE: [vmsingle](https://docs.victoriametrics.com/single-server-victoriametrics/): support disable or log [implicit conversions](https://docs.victoriametrics.com/metricsql/#implicit-query-conversions) for subquery with cmd-line flags `-search.disableImplicitConversion` and `-search.logImplicitConversion`. See [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4338).
* FEATURE: [vmagent](https://docs.victoriametrics.com/vmagent/): support data replication additionally to sharding among remote storage systems if `-remoteWrite.shardByURLReplicas=N` command-line flag is set additionally to `-remoteWrite.shardByURL` command-line flag, where `N` is desired replication factor. This allows setting up data replication among failure domains when the replication factor is smaller than the number of failure domains. See [these docs](https://docs.victoriametrics.com/vmagent/#sharding-among-remote-storages) and [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6054).
* FEATURE: [vmagent](https://docs.victoriametrics.com/vmagent/): reduce CPU usage when [sharding among remote storage systems](https://docs.victoriametrics.com/vmagent/#sharding-among-remote-storages) is enabled.
* FEATURE: [vmagent](https://docs.victoriametrics.com/vmagent/): support [DNS SRV](https://en.wikipedia.org/wiki/SRV_record) addresses in `-remoteWrite.url` command-line option and in scrape target urls. For example, `-remoteWrite.url=http://srv+victoria-metrics/api/v1/write` automatically resolves the `victoria-metrics` DNS SRV to a list of hostnames with TCP ports and then sends the collected metrics to these TCP addresses. See [these docs](https://docs.victoriametrics.com/vmagent/#srv-urls) and [this feature request](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6053).

View file

@ -2229,6 +2229,7 @@ Any [rollup function](#rollup-functions) for something other than [series select
Nested rollup functions can be implicit thanks to the [implicit query conversions](#implicit-query-conversions).
For example, `delta(sum(m))` is implicitly converted to `delta(sum(default_rollup(m))[1i:1i])`, so it becomes a subquery,
since it contains [default_rollup](#default_rollup) nested into [delta](#delta).
This behavior can be disabled or logged via cmd-line flags `-search.disableImplicitConversion` and `-search.logImplicitConversion` since v1.101.0.
VictoriaMetrics performs subqueries in the following way:
@ -2261,4 +2262,5 @@ VictoriaMetrics performs the following implicit conversions for incoming queries
For example, `avg_over_time(rate(http_requests_total[5m])[1h])` is automatically converted to `avg_over_time(rate(http_requests_total[5m])[1h:1i])`.
* If something other than [series selector](https://docs.victoriametrics.com/keyconcepts/#filtering)
is passed to [rollup function](#rollup-functions), then a [subquery](#subqueries) with `1i` lookbehind window and `1i` step is automatically formed.
For example, `rate(sum(up))` is automatically converted to `rate((sum(default_rollup(up)))[1i:1i])`.
For example, `rate(sum(up))` is automatically converted to `rate((sum(default_rollup(up)))[1i:1i])`.
This behavior can be disabled or logged via cmd-line flags `-search.disableImplicitConversion` and `-search.logImplicitConversion` since v1.101.0.