From 3c076544bfd59867c85e3ce3561d6810ae7240d7 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Tue, 10 Dec 2019 23:41:11 +0200 Subject: [PATCH] app/vmselect/promql: allow negative offsets Updates https://github.com/prometheus/prometheus/issues/6282 --- app/vmselect/prometheus/prometheus.go | 2 +- app/vmselect/promql/eval.go | 6 +- app/vmselect/promql/exec_test.go | 35 ++++++ app/vmselect/promql/lexer.go | 31 +++++- app/vmselect/promql/lexer_test.go | 149 ++++++++++++++++++-------- app/vmselect/promql/parser.go | 27 ++++- app/vmselect/promql/parser_test.go | 7 +- docs/ExtendedPromQL.md | 3 +- lib/mergeset/part_search.go | 2 +- 9 files changed, 200 insertions(+), 62 deletions(-) diff --git a/app/vmselect/prometheus/prometheus.go b/app/vmselect/prometheus/prometheus.go index c2cc65a81..fe5e22f46 100644 --- a/app/vmselect/prometheus/prometheus.go +++ b/app/vmselect/prometheus/prometheus.go @@ -489,7 +489,7 @@ func QueryHandler(w http.ResponseWriter, r *http.Request) error { var window int64 if len(windowStr) > 0 { var err error - window, err = promql.DurationValue(windowStr, step) + window, err = promql.PositiveDurationValue(windowStr, step) if err != nil { return err } diff --git a/app/vmselect/promql/eval.go b/app/vmselect/promql/eval.go index 3f696b5f3..3ea1f126a 100644 --- a/app/vmselect/promql/eval.go +++ b/app/vmselect/promql/eval.go @@ -440,7 +440,7 @@ func evalRollupFuncWithSubquery(ec *EvalConfig, name string, rf rollupFunc, re * var step int64 if len(re.Step) > 0 { var err error - step, err = DurationValue(re.Step, ec.Step) + step, err = PositiveDurationValue(re.Step, ec.Step) if err != nil { return nil, err } @@ -450,7 +450,7 @@ func evalRollupFuncWithSubquery(ec *EvalConfig, name string, rf rollupFunc, re * var window int64 if len(re.Window) > 0 { var err error - window, err = DurationValue(re.Window, ec.Step) + window, err = PositiveDurationValue(re.Window, ec.Step) if err != nil { return nil, err } @@ -551,7 +551,7 @@ func evalRollupFuncWithMetricExpr(ec *EvalConfig, name string, rf rollupFunc, me var window int64 if len(windowStr) > 0 { var err error - window, err = DurationValue(windowStr, ec.Step) + window, err = PositiveDurationValue(windowStr, ec.Step) if err != nil { return nil, err } diff --git a/app/vmselect/promql/exec_test.go b/app/vmselect/promql/exec_test.go index 67aee800e..27e2d623f 100644 --- a/app/vmselect/promql/exec_test.go +++ b/app/vmselect/promql/exec_test.go @@ -198,6 +198,17 @@ func TestExecSuccess(t *testing.T) { resultExpected := []netstorage.Result{r} f(q, resultExpected) }) + t.Run("time() offset -100s", func(t *testing.T) { + t.Parallel() + q := `time() offset -100s` + r := netstorage.Result{ + MetricName: metricNameExpected, + Values: []float64{1000, 1200, 1400, 1600, 1800, 2000}, + Timestamps: timestampsExpected, + } + resultExpected := []netstorage.Result{r} + f(q, resultExpected) + }) t.Run("(a, b) offset 100s", func(t *testing.T) { t.Parallel() q := `sort((label_set(time(), "foo", "bar"), label_set(time()+10, "foo", "baz")) offset 100s)` @@ -270,6 +281,30 @@ func TestExecSuccess(t *testing.T) { resultExpected := []netstorage.Result{r1, r2} f(q, resultExpected) }) + t.Run("(a offset -100s, b offset -50s) offset -400s", func(t *testing.T) { + t.Parallel() + q := `sort((label_set(time() offset -100s, "foo", "bar"), label_set(time()+10, "foo", "baz") offset -50s) offset -400s)` + r1 := netstorage.Result{ + MetricName: metricNameExpected, + Values: []float64{1260, 1460, 1660, 1860, 2060, 2260}, + Timestamps: timestampsExpected, + } + r1.MetricName.Tags = []storage.Tag{{ + Key: []byte("foo"), + Value: []byte("baz"), + }} + r2 := netstorage.Result{ + MetricName: metricNameExpected, + Values: []float64{1300, 1500, 1700, 1900, 2100, 2300}, + Timestamps: timestampsExpected, + } + r2.MetricName.Tags = []storage.Tag{{ + Key: []byte("foo"), + Value: []byte("bar"), + }} + resultExpected := []netstorage.Result{r1, r2} + f(q, resultExpected) + }) t.Run("time()[:100s] offset 100s", func(t *testing.T) { t.Parallel() q := `time()[:100s] offset 100s` diff --git a/app/vmselect/promql/lexer.go b/app/vmselect/promql/lexer.go index 8dfe23208..34d0c0d2c 100644 --- a/app/vmselect/promql/lexer.go +++ b/app/vmselect/promql/lexer.go @@ -105,7 +105,7 @@ again: token = s[:n] goto tokenFoundLabel } - if n := scanDuration(s); n > 0 { + if n := scanDuration(s, false); n > 0 { token = s[:n] goto tokenFoundLabel } @@ -368,15 +368,30 @@ func isPositiveNumberPrefix(s string) bool { return isDecimalChar(s[1]) } -func isDuration(s string) bool { - n := scanDuration(s) +func isPositiveDuration(s string) bool { + n := scanDuration(s, false) return n == len(s) } +// PositiveDurationValue returns the duration in milliseconds for the given s +// and the given step. +func PositiveDurationValue(s string, step int64) (int64, error) { + d, err := DurationValue(s, step) + if err != nil { + return 0, err + } + if d < 0 { + return 0, fmt.Errorf("duration cannot be negative; got %q", s) + } + return d, nil +} + // DurationValue returns the duration in milliseconds for the given s // and the given step. +// +// The returned duration value can be negative. func DurationValue(s string, step int64) (int64, error) { - n := scanDuration(s) + n := scanDuration(s, true) if n != len(s) { return 0, fmt.Errorf("cannot parse duration %q", s) } @@ -408,8 +423,14 @@ func DurationValue(s string, step int64) (int64, error) { return int64(mp * f * 1e3), nil } -func scanDuration(s string) int { +func scanDuration(s string, canBeNegative bool) int { + if len(s) == 0 { + return -1 + } i := 0 + if s[0] == '-' && canBeNegative { + i++ + } for i < len(s) && isDecimalChar(s[i]) { i++ } diff --git a/app/vmselect/promql/lexer_test.go b/app/vmselect/promql/lexer_test.go index 0685f797e..6f8fed517 100644 --- a/app/vmselect/promql/lexer_test.go +++ b/app/vmselect/promql/lexer_test.go @@ -286,61 +286,116 @@ func testLexerError(t *testing.T, s string) { } } -func TestDurationSuccess(t *testing.T) { +func TestPositiveDurationSuccess(t *testing.T) { + f := func(s string, step, expectedD int64) { + t.Helper() + d, err := PositiveDurationValue(s, step) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + if d != expectedD { + t.Fatalf("unexpected duration; got %d; want %d", d, expectedD) + } + } + // Integer durations - testDurationSuccess(t, "123s", 42, 123*1000) - testDurationSuccess(t, "123m", 42, 123*60*1000) - testDurationSuccess(t, "1h", 42, 1*60*60*1000) - testDurationSuccess(t, "2d", 42, 2*24*60*60*1000) - testDurationSuccess(t, "3w", 42, 3*7*24*60*60*1000) - testDurationSuccess(t, "4y", 42, 4*365*24*60*60*1000) - testDurationSuccess(t, "1i", 42*1000, 42*1000) - testDurationSuccess(t, "3i", 42, 3*42) + f("123s", 42, 123*1000) + f("123m", 42, 123*60*1000) + f("1h", 42, 1*60*60*1000) + f("2d", 42, 2*24*60*60*1000) + f("3w", 42, 3*7*24*60*60*1000) + f("4y", 42, 4*365*24*60*60*1000) + f("1i", 42*1000, 42*1000) + f("3i", 42, 3*42) // Float durations - testDurationSuccess(t, "0.234s", 42, 234) - testDurationSuccess(t, "1.5s", 42, 1.5*1000) - testDurationSuccess(t, "1.5m", 42, 1.5*60*1000) - testDurationSuccess(t, "1.2h", 42, 1.2*60*60*1000) - testDurationSuccess(t, "1.1d", 42, 1.1*24*60*60*1000) - testDurationSuccess(t, "1.1w", 42, 1.1*7*24*60*60*1000) - testDurationSuccess(t, "1.3y", 42, 1.3*365*24*60*60*1000) - testDurationSuccess(t, "0.1i", 12340, 0.1*12340) + f("0.234s", 42, 234) + f("1.5s", 42, 1.5*1000) + f("1.5m", 42, 1.5*60*1000) + f("1.2h", 42, 1.2*60*60*1000) + f("1.1d", 42, 1.1*24*60*60*1000) + f("1.1w", 42, 1.1*7*24*60*60*1000) + f("1.3y", 42, 1.3*365*24*60*60*1000) + f("0.1i", 12340, 0.1*12340) } -func testDurationSuccess(t *testing.T, s string, step, expectedD int64) { - t.Helper() - d, err := DurationValue(s, step) - if err != nil { - t.Fatalf("unexpected error: %s", err) +func TestPositiveDurationError(t *testing.T) { + f := func(s string) { + t.Helper() + if isPositiveDuration(s) { + t.Fatalf("unexpected valid duration %q", s) + } + d, err := PositiveDurationValue(s, 42) + if err == nil { + t.Fatalf("expecting non-nil error for duration %q", s) + } + if d != 0 { + t.Fatalf("expecting zero duration; got %d", d) + } } - if d != expectedD { - t.Fatalf("unexpected duration; got %d; want %d", d, expectedD) + f("") + f("foo") + f("m") + f("12") + f("1.23") + f("1.23mm") + f("123q") + f("-123s") +} + +func TestDurationSuccess(t *testing.T) { + f := func(s string, step, expectedD int64) { + t.Helper() + d, err := DurationValue(s, step) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + if d != expectedD { + t.Fatalf("unexpected duration; got %d; want %d", d, expectedD) + } } + + // Integer durations + f("123s", 42, 123*1000) + f("-123s", 42, -123*1000) + f("123m", 42, 123*60*1000) + f("1h", 42, 1*60*60*1000) + f("2d", 42, 2*24*60*60*1000) + f("3w", 42, 3*7*24*60*60*1000) + f("4y", 42, 4*365*24*60*60*1000) + f("1i", 42*1000, 42*1000) + f("3i", 42, 3*42) + f("-3i", 42, -3*42) + + // Float durations + f("0.234s", 42, 234) + f("-0.234s", 42, -234) + f("1.5s", 42, 1.5*1000) + f("1.5m", 42, 1.5*60*1000) + f("1.2h", 42, 1.2*60*60*1000) + f("1.1d", 42, 1.1*24*60*60*1000) + f("1.1w", 42, 1.1*7*24*60*60*1000) + f("1.3y", 42, 1.3*365*24*60*60*1000) + f("-1.3y", 42, -1.3*365*24*60*60*1000) + f("0.1i", 12340, 0.1*12340) } func TestDurationError(t *testing.T) { - testDurationError(t, "") - testDurationError(t, "foo") - testDurationError(t, "m") - testDurationError(t, "12") - testDurationError(t, "1.23") - testDurationError(t, "1.23mm") - testDurationError(t, "123q") -} - -func testDurationError(t *testing.T, s string) { - t.Helper() - - if isDuration(s) { - t.Fatalf("unexpected valud duration %q", s) - } - - d, err := DurationValue(s, 42) - if err == nil { - t.Fatalf("expecting non-nil error for duration %q", s) - } - if d != 0 { - t.Fatalf("expecting zero duration; got %d", d) - } + f := func(s string) { + t.Helper() + d, err := DurationValue(s, 42) + if err == nil { + t.Fatalf("expecting non-nil error for duration %q", s) + } + if d != 0 { + t.Fatalf("expecting zero duration; got %d", d) + } + } + f("") + f("foo") + f("m") + f("12") + f("1.23") + f("1.23mm") + f("123q") } diff --git a/app/vmselect/promql/parser.go b/app/vmselect/promql/parser.go index 28a75ed91..528893b0d 100644 --- a/app/vmselect/promql/parser.go +++ b/app/vmselect/promql/parser.go @@ -1173,7 +1173,7 @@ func (p *parser) parseWindowAndStep() (string, string, bool, error) { } var window string if !strings.HasPrefix(p.lex.Token, ":") { - window, err = p.parseDuration() + window, err = p.parsePositiveDuration() if err != nil { return "", "", false, err } @@ -1192,7 +1192,7 @@ func (p *parser) parseWindowAndStep() (string, string, bool, error) { } } if p.lex.Token != "]" { - step, err = p.parseDuration() + step, err = p.parsePositiveDuration() if err != nil { return "", "", false, err } @@ -1222,13 +1222,34 @@ func (p *parser) parseOffset() (string, error) { } func (p *parser) parseDuration() (string, error) { - if !isDuration(p.lex.Token) { + isNegative := false + if p.lex.Token == "-" { + isNegative = true + if err := p.lex.Next(); err != nil { + return "", err + } + } + if !isPositiveDuration(p.lex.Token) { return "", fmt.Errorf(`duration: unexpected token %q; want "duration"`, p.lex.Token) } d := p.lex.Token if err := p.lex.Next(); err != nil { return "", err } + if isNegative { + d = "-" + d + } + return d, nil +} + +func (p *parser) parsePositiveDuration() (string, error) { + d, err := p.parseDuration() + if err != nil { + return "", err + } + if strings.HasPrefix(d, "-") { + return "", fmt.Errorf("positiveDuration: expecting positive duration; got %q", d) + } return d, nil } diff --git a/app/vmselect/promql/parser_test.go b/app/vmselect/promql/parser_test.go index 1fab9189e..b8e13a9c3 100644 --- a/app/vmselect/promql/parser_test.go +++ b/app/vmselect/promql/parser_test.go @@ -77,15 +77,18 @@ func TestParsePromQLSuccess(t *testing.T) { same(`{}[5m:3s]`) another(`{}[ 5m : 3s ]`, `{}[5m:3s]`) same(`{} offset 5m`) + same(`{} offset -5m`) same(`{}[5m] offset 10y`) same(`{}[5.3m:3.4s] offset 10y`) same(`{}[:3.4s] offset 10y`) + same(`{}[:3.4s] offset -10y`) same(`{Foo="bAR"}`) same(`{foo="bar"}`) same(`{foo="bar"}[5m]`) same(`{foo="bar"}[5m:]`) same(`{foo="bar"}[5m:3s]`) same(`{foo="bar"} offset 10y`) + same(`{foo="bar"} offset -10y`) same(`{foo="bar"}[5m] offset 10y`) same(`{foo="bar"}[5m:3s] offset 10y`) another(`{foo="bar"}[5m] oFFSEt 10y`, `{foo="bar"}[5m] offset 10y`) @@ -458,7 +461,6 @@ func TestParsePromQLError(t *testing.T) { // invalid metricExpr f(`{__name__="ff"} offset 55`) - f(`{__name__="ff"} offset -5m`) f(`foo[55]`) f(`m[-5m]`) f(`{`) @@ -491,6 +493,9 @@ func TestParsePromQLError(t *testing.T) { f(`m[5m:-`) f(`m[5m:-1`) f(`m[5m:-1]`) + f(`m[5m:-1s]`) + f(`m[-5m:1s]`) + f(`m[-5m:-1s]`) f(`m[:`) f(`m[:-`) f(`m[:1]`) diff --git a/docs/ExtendedPromQL.md b/docs/ExtendedPromQL.md index 0ae7bc244..bbd60eced 100644 --- a/docs/ExtendedPromQL.md +++ b/docs/ExtendedPromQL.md @@ -9,10 +9,11 @@ Try these extensions on [an editable Grafana dashboard](http://play-grafana.vict - Metric names and metric labels may contain escaped chars. For instance, `foo\-bar{baz\=aa="b"}` is valid expression. It returns time series with name `foo-bar` containing label `baz=aa` with value `b`. Additionally, `\xXX` escape sequence is supported, where `XX` is hexadecimal representation of escaped char. - `offset`, range duration and step value for range vector may refer to the current step aka `$__interval` value from Grafana. For instance, `rate(metric[10i] offset 5i)` would return per-second rate over a range covering 10 previous steps with the offset of 5 steps. +- `offset` may be put anywere in the query. For instance, `sum(foo) offset 24h`. +- `offset` may be negative. For example, `q offset -1h`. - `default` binary operator. `q1 default q2` substitutes `NaN` values from `q1` with the corresponding values from `q2`. - `if` binary operator. `q1 if q2` removes values from `q1` for `NaN` values from `q2`. - `ifnot` binary operator. `q1 ifnot q2` removes values from `q1` for non-`NaN` values from `q2`. -- `offset` may be put anywere in the query. For instance, `sum(foo) offset 24h`. - Trailing commas on all the lists are allowed - label filters, function args and with expressions. For instance, the following queries are valid: `m{foo="bar",}`, `f(a, b,)`, `WITH (x=y,) x`. This simplifies maintenance of multi-line queries. - String literals may be concatenated. This is useful with `WITH` templates: `WITH (commonPrefix="long_metric_prefix_") {__name__=commonPrefix+"suffix1"} / {__name__=commonPrefix+"suffix2"}`. - Range duration in functions such as [rate](https://prometheus.io/docs/prometheus/latest/querying/functions/#rate()) may be omitted. VictoriaMetrics automatically selects range duration depending on the current step used for building the graph. For instance, the following query is valid in VictoriaMetrics: `rate(node_network_receive_bytes_total)`. diff --git a/lib/mergeset/part_search.go b/lib/mergeset/part_search.go index ddf8d53c6..7922bf443 100644 --- a/lib/mergeset/part_search.go +++ b/lib/mergeset/part_search.go @@ -13,7 +13,7 @@ import ( type partSearch struct { // Item contains the last item found after the call to NextItem. // - // The Item content is valud intil the next call to NextItem. + // The Item content is valid until the next call to NextItem. Item []byte // p is a part to search.