diff --git a/app/vmselect/prometheus/prometheus.go b/app/vmselect/prometheus/prometheus.go index e072d8034..4cbed0d3b 100644 --- a/app/vmselect/prometheus/prometheus.go +++ b/app/vmselect/prometheus/prometheus.go @@ -694,7 +694,10 @@ func QueryHandler(qt *querytracer.Tracer, startTime time.Time, w http.ResponseWr return err } if childQuery, windowExpr, offsetExpr := promql.IsMetricSelectorWithRollup(query); childQuery != "" { - window := windowExpr.Duration(step) + window, err := windowExpr.NonNegativeDuration(step) + if err != nil { + return fmt.Errorf("cannot parse lookbehind window in square brackets at %s: %w", query, err) + } offset := offsetExpr.Duration(step) start -= offset end := start @@ -723,11 +726,17 @@ func QueryHandler(qt *querytracer.Tracer, startTime time.Time, w http.ResponseWr return nil } if childQuery, windowExpr, stepExpr, offsetExpr := promql.IsRollup(query); childQuery != "" { - newStep := stepExpr.Duration(step) + newStep, err := stepExpr.NonNegativeDuration(step) + if err != nil { + return fmt.Errorf("cannot parse step in square brackets at %s: %w", query, err) + } if newStep > 0 { step = newStep } - window := windowExpr.Duration(step) + window, err := windowExpr.NonNegativeDuration(step) + if err != nil { + return fmt.Errorf("cannot parse lookbehind window in square brackets at %s: %w", query, err) + } offset := offsetExpr.Duration(step) start -= offset end := start diff --git a/app/vmselect/promql/eval.go b/app/vmselect/promql/eval.go index f83296ed5..a9a85f624 100644 --- a/app/vmselect/promql/eval.go +++ b/app/vmselect/promql/eval.go @@ -896,11 +896,17 @@ func evalRollupFuncWithSubquery(qt *querytracer.Tracer, ec *EvalConfig, funcName // TODO: determine whether to use rollupResultCacheV here. qt = qt.NewChild("subquery") defer qt.Done() - step := re.Step.Duration(ec.Step) + step, err := re.Step.NonNegativeDuration(ec.Step) + if err != nil { + return nil, fmt.Errorf("cannot parse step in square brackets at %s: %w", expr.AppendString(nil), err) + } if step == 0 { step = ec.Step } - window := re.Window.Duration(ec.Step) + window, err := re.Window.NonNegativeDuration(ec.Step) + if err != nil { + return nil, fmt.Errorf("cannot parse lookbehind window in square brackets at %s: %w", expr.AppendString(nil), err) + } ecSQ := copyEvalConfig(ec) ecSQ.Start -= window + maxSilenceInterval + step @@ -1042,7 +1048,10 @@ var ( func evalRollupFuncWithMetricExpr(qt *querytracer.Tracer, ec *EvalConfig, funcName string, rf rollupFunc, expr metricsql.Expr, me *metricsql.MetricExpr, iafc *incrementalAggrFuncContext, windowExpr *metricsql.DurationExpr) ([]*timeseries, error) { var rollupMemorySize int64 - window := windowExpr.Duration(ec.Step) + window, err := windowExpr.NonNegativeDuration(ec.Step) + if err != nil { + return nil, fmt.Errorf("cannot parse lookbehind window in square brackets at %s: %w", expr.AppendString(nil), err) + } if qt.Enabled() { qt = qt.NewChild("rollup %s(): timeRange=%s, step=%d, window=%d", funcName, ec.timeRangeString(), ec.Step, window) defer func() { diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 646db5dfc..2282df56c 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -44,11 +44,12 @@ The following `tip` changes can be tested by building VictoriaMetrics components * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): properly apply `if` filters during [relabeling](https://docs.victoriametrics.com/vmagent.html#relabeling-enhancements). Previously the `if` filter could improperly work. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4806) and [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/4816). * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): use local scrape timestamps for the scraped metrics unless `honor_timestamps: true` option is explicitly set at [scrape_config](https://docs.victoriametrics.com/sd_configs.html#scrape_configs). This fixes gaps for metrics collected from [cadvisor](https://github.com/google/cadvisor) or similar exporters, which export metrics with invalid timestamps. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4697) and [this comment](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4697#issuecomment-1654614799) for details. The issue has been introduced in [v1.68.0](#v1680). +* BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): keep unmatched series at [stream aggregation](https://docs.victoriametrics.com/stream-aggregation.html) when `-remoteWrite.streamAggr.dropInput` is set to `false` to match intended behaviour introduced at [v1.92.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.92.0). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4804). * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): properly set `vmalert_config_last_reload_successful` value on configuration updates or rollbacks. The bug was introduced in [v1.92.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.92.0) in [this PR](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/4543). * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): fix `vmalert_remotewrite_send_duration_seconds_total` value, before it didn't count in the real time spending on remote write requests. See [this pr](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/4801) for details. * BUGFIX: [vmbackupmanager](https://docs.victoriametrics.com/vmbackupmanager.html): fix panic when creating a backup to a local filesystem on Windows. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4704). -* BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): keep unmatched series at [stream aggregation](https://docs.victoriametrics.com/stream-aggregation.html) when `-remoteWrite.streamAggr.dropInput` is set to `false` to match intended behaviour introduced at [v1.92.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.92.0). See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4804). * BUGFIX: [vmui](https://docs.victoriametrics.com/#vmui): properly handle client address with `X-Forwarded-For` part at the [Active queries](https://docs.victoriametrics.com/#active-queries) page. See [this comment](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/4676#issuecomment-1663203424). +* BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): prevent from panic when the lookbehind window in square brackets of [rollup function](https://docs.victoriametrics.com/MetricsQL.html#rollup-functions) is parsed into negative value. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4795). ## [v1.92.1](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.92.1) diff --git a/go.mod b/go.mod index 33596a774..1040b6255 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.62.0 + github.com/VictoriaMetrics/metricsql v0.63.0 github.com/aws/aws-sdk-go-v2 v1.20.1 github.com/aws/aws-sdk-go-v2/config v1.18.33 github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.11.77 diff --git a/go.sum b/go.sum index 30dabb5b7..1f25c9654 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.62.0 h1:g8Iv8HPuAtgZAJhIIHTHct3eby/ZTpW1zMlJBE3O01c= -github.com/VictoriaMetrics/metricsql v0.62.0/go.mod h1:k4UaP/+CjuZslIjd+kCigNG9TQmUqh5v0TP/nMEy90I= +github.com/VictoriaMetrics/metricsql v0.63.0 h1:RRu3lln7uhQwSRkzAknOUyB0uP9LwymFMHnzDqGbZ40= +github.com/VictoriaMetrics/metricsql v0.63.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/parser.go b/vendor/github.com/VictoriaMetrics/metricsql/parser.go index 8c520d163..bc3f83e7f 100644 --- a/vendor/github.com/VictoriaMetrics/metricsql/parser.go +++ b/vendor/github.com/VictoriaMetrics/metricsql/parser.go @@ -1569,6 +1569,17 @@ func (de *DurationExpr) AppendString(dst []byte) []byte { return append(dst, de.s...) } +// NonNegativeDuration returns non-negative duration for de in milliseconds. +// +// Error is returned if the duration is negative. +func (de *DurationExpr) NonNegativeDuration(step int64) (int64, error) { + d := de.Duration(step) + if d < 0 { + return 0, fmt.Errorf("unexpected negative duration %dms", d) + } + return d, nil +} + // Duration returns the duration from de in milliseconds. func (de *DurationExpr) Duration(step int64) int64 { if de == nil { diff --git a/vendor/modules.txt b/vendor/modules.txt index 67ae53be1..d39d351be 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.62.0 +# github.com/VictoriaMetrics/metricsql v0.63.0 ## explicit; go 1.13 github.com/VictoriaMetrics/metricsql github.com/VictoriaMetrics/metricsql/binaryop