From c80244a3e42ff99566d4f41e7c7526fee6c6f270 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Tue, 22 Feb 2022 08:34:53 +0200 Subject: [PATCH] Parse duration expressions in accordance with promql (#5275) * Add support for Prometheus like duration units in logql * Remove redundant type conversion * deduplicate call to sb.String Add a comment to explain why both promql and time's ParseDuration methods are used * extract duration parsing to separate function. * Add tests to ensure promql duration units are supported * Sort duration rune comment and list of runes in ascending order of length. --- pkg/logql/ast_test.go | 4 ++-- pkg/logql/lex.go | 30 ++++++++++++++++++++++++++---- pkg/logql/lex_test.go | 29 +++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 6 deletions(-) diff --git a/pkg/logql/ast_test.go b/pkg/logql/ast_test.go index e49648fb0772..1f38522e4249 100644 --- a/pkg/logql/ast_test.go +++ b/pkg/logql/ast_test.go @@ -66,7 +66,7 @@ func Test_SampleExpr_String(t *testing.T) { for _, tc := range []string{ `rate( ( {job="mysql"} |="error" !="timeout" ) [10s] )`, `absent_over_time( ( {job="mysql"} |="error" !="timeout" ) [10s] )`, - `absent_over_time( ( {job="mysql"} |="error" !="timeout" ) [10s] offset 10m )`, + `absent_over_time( ( {job="mysql"} |="error" !="timeout" ) [10s] offset 10d )`, `sum without(a) ( rate ( ( {job="mysql"} |="error" !="timeout" ) [10s] ) )`, `sum by(a) (rate( ( {job="mysql"} |="error" !="timeout" ) [10s] ) )`, `sum(count_over_time({job="mysql"}[5m]))`, @@ -78,7 +78,7 @@ func Test_SampleExpr_String(t *testing.T) { `sum(count_over_time({job="mysql"} | pattern " bar " | json [5m]))`, `sum(count_over_time({job="mysql"} | unpack | json [5m]))`, `sum(count_over_time({job="mysql"} | regexp "(?Pfoo|bar)" [5m]))`, - `sum(count_over_time({job="mysql"} | regexp "(?Pfoo|bar)" [5m] offset 10m))`, + `sum(count_over_time({job="mysql"} | regexp "(?Pfoo|bar)" [5m] offset 10y))`, `topk(10,sum(rate({region="us-east1"}[5m])) by (name))`, `topk by (name)(10,sum(rate({region="us-east1"}[5m])))`, `avg( rate( ( {job="nginx"} |= "GET" ) [10s] ) ) by (region)`, diff --git a/pkg/logql/lex.go b/pkg/logql/lex.go index 9002ac478c5f..00d1a85d850f 100644 --- a/pkg/logql/lex.go +++ b/pkg/logql/lex.go @@ -240,21 +240,43 @@ func tryScanDuration(number string, l *scanner.Scanner) (time.Duration, bool) { return 0, false } // we've found more characters before a whitespace or the end - d, err := time.ParseDuration(sb.String()) + durationString := sb.String() + duration, err := parseDuration(durationString) if err != nil { return 0, false } + // we need to consume the scanner, now that we know this is a duration. for i := 0; i < consumed; i++ { _ = l.Next() } - return d, true + + return duration, true +} + +func parseDuration(d string) (time.Duration, error) { + var duration time.Duration + // Try to parse promql style durations first, to ensure that we support the same duration + // units as promql + prometheusDuration, err := model.ParseDuration(d) + if err != nil { + // Fall back to standard library's time.ParseDuration if a promql style + // duration couldn't be parsed. + duration, err = time.ParseDuration(d) + if err != nil { + return 0, err + } + } else { + duration = time.Duration(prometheusDuration) + } + + return duration, nil } func isDurationRune(r rune) bool { - // "ns", "us" (or "µs"), "ms", "s", "m", "h". + // "ns", "us" (or "µs"), "ms", "s", "m", "h", "d", "w", "y". switch r { - case 'n', 's', 'u', 'm', 'h', 'µ': + case 'n', 'u', 'µ', 'm', 's', 'h', 'd', 'w', 'y': return true default: return false diff --git a/pkg/logql/lex_test.go b/pkg/logql/lex_test.go index b57af7969f1b..a87b4e5c6422 100644 --- a/pkg/logql/lex_test.go +++ b/pkg/logql/lex_test.go @@ -4,6 +4,7 @@ import ( "strings" "testing" "text/scanner" + "time" "github.com/stretchr/testify/require" ) @@ -126,3 +127,31 @@ func Test_isFunction(t *testing.T) { }) } } + +func Test_parseDuration(t *testing.T) { + const MICROSECOND = 1000 * time.Nanosecond + const DAY = 24 * time.Hour + const WEEK = 7 * DAY + const YEAR = 365 * DAY + + for _, tc := range []struct { + input string + expected time.Duration + }{ + {"1ns", time.Nanosecond}, + {"1s", time.Second}, + {"1us", MICROSECOND}, + {"1m", time.Minute}, + {"1h", time.Hour}, + {"1µs", MICROSECOND}, + {"1y", YEAR}, + {"1w", WEEK}, + {"1d", DAY}, + {"1h15m30.918273645s", time.Hour + 15*time.Minute + 30*time.Second + 918273645*time.Nanosecond}, + } { + actual, err := parseDuration(tc.input) + + require.Equal(t, err, nil) + require.Equal(t, tc.expected, actual) + } +}