From 5700e307b7c875049a24f168b61bbe71121d6ed4 Mon Sep 17 00:00:00 2001 From: Daniel D'Abate Date: Tue, 3 Dec 2019 11:08:44 -0300 Subject: [PATCH 1/3] Support duration and float formats for step parameter in http api --- docs/api.md | 2 +- pkg/loghttp/params.go | 17 +++++++++++++---- pkg/loghttp/params_test.go | 24 +++++++++++++++++++++++- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/docs/api.md b/docs/api.md index ba5c824a6426..34c0f2e772c3 100644 --- a/docs/api.md +++ b/docs/api.md @@ -204,7 +204,7 @@ accepts the following query parameters in the URL: - `limit`: The max number of entries to return - `start`: The start time for the query as a nanosecond Unix epoch. Defaults to one hour ago. - `end`: The start time for the query as a nanosecond Unix epoch. Defaults to now. -- `step`: Query resolution step width in seconds. Defaults to a dynamic value based on `start` and `end`. +- `step`: Query resolution step width in duration format or seconds. Defaults to a dynamic value based on `start` and `end`. - `direction`: Determines the sort order of logs. Supported values are `forward` or `backward`. Defaults to `backward.` Requests against this endpoint require Loki to query the index store in order to diff --git a/pkg/loghttp/params.go b/pkg/loghttp/params.go index 419a73f79f0c..14b3f131a09c 100644 --- a/pkg/loghttp/params.go +++ b/pkg/loghttp/params.go @@ -50,11 +50,20 @@ func bounds(r *http.Request) (time.Time, time.Time, error) { } func step(r *http.Request, start, end time.Time) (time.Duration, error) { - s, err := parseInt(r.URL.Query().Get("step"), defaultQueryRangeStep(start, end)) - if err != nil { - return 0, err + value := r.URL.Query().Get("step") + if value == "" { + return time.Duration(defaultQueryRangeStep(start, end)) * time.Second, nil + } + + d, err := time.ParseDuration(value) + if err == nil { + return d, nil + } + f, err := strconv.ParseFloat(value, 64) + if err == nil { + return time.Duration(f) * time.Second, nil } - return time.Duration(s) * time.Second, nil + return 0, err } // defaultQueryRangeStep returns the default step used in the query range API, diff --git a/pkg/loghttp/params_test.go b/pkg/loghttp/params_test.go index b6559bd53ba9..ac0ae52f525d 100644 --- a/pkg/loghttp/params_test.go +++ b/pkg/loghttp/params_test.go @@ -63,7 +63,7 @@ func TestHttp_ParseRangeQuery_Step(t *testing.T) { Direction: logproto.BACKWARD, }, }, - "should use the input step parameter if provided": { + "should use the input step parameter if provided as an integer": { reqPath: "/loki/api/v1/query_range?query={}&start=0&end=3600000000000&step=5", expected: &RangeQuery{ Query: "{}", @@ -74,6 +74,28 @@ func TestHttp_ParseRangeQuery_Step(t *testing.T) { Direction: logproto.BACKWARD, }, }, + "should use the input step parameter if provided as a float": { + reqPath: "/loki/api/v1/query_range?query={}&start=0&end=3600000000000&step=5.000", + expected: &RangeQuery{ + Query: "{}", + Start: time.Unix(0, 0), + End: time.Unix(3600, 0), + Step: 5 * time.Second, + Limit: 100, + Direction: logproto.BACKWARD, + }, + }, + "should use the input step parameter if provided as a duration": { + reqPath: "/loki/api/v1/query_range?query={}&start=0&end=3600000000000&step=5s", + expected: &RangeQuery{ + Query: "{}", + Start: time.Unix(0, 0), + End: time.Unix(3600, 0), + Step: 5 * time.Second, + Limit: 100, + Direction: logproto.BACKWARD, + }, + }, } for testName, testData := range tests { From 6fb11da25e527edee6d7a1ceeebb796255f31f95 Mon Sep 17 00:00:00 2001 From: Daniel D'Abate Date: Tue, 3 Dec 2019 12:39:38 -0300 Subject: [PATCH 2/3] Support Prometheus duration format and add missing tests --- docs/api.md | 2 +- pkg/loghttp/params.go | 18 +++++++++++------- pkg/loghttp/params_test.go | 26 ++++++++++++++++++++++++-- 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/docs/api.md b/docs/api.md index 34c0f2e772c3..401746d8585a 100644 --- a/docs/api.md +++ b/docs/api.md @@ -204,7 +204,7 @@ accepts the following query parameters in the URL: - `limit`: The max number of entries to return - `start`: The start time for the query as a nanosecond Unix epoch. Defaults to one hour ago. - `end`: The start time for the query as a nanosecond Unix epoch. Defaults to now. -- `step`: Query resolution step width in duration format or seconds. Defaults to a dynamic value based on `start` and `end`. +- `step`: Query resolution step width in `duration` format or float number of seconds. `duration` refers to Prometheus duration strings of the form `[0-9]+[smhdwy]`. For example, 5m refers to a duration of 5 minutes. Defaults to a dynamic value based on `start` and `end`. - `direction`: Determines the sort order of logs. Supported values are `forward` or `backward`. Defaults to `backward.` Requests against this endpoint require Loki to query the index store in order to diff --git a/pkg/loghttp/params.go b/pkg/loghttp/params.go index 14b3f131a09c..c59d33d5a39e 100644 --- a/pkg/loghttp/params.go +++ b/pkg/loghttp/params.go @@ -2,6 +2,8 @@ package loghttp import ( "fmt" + "github.com/pkg/errors" + "github.com/prometheus/common/model" "math" "net/http" "strconv" @@ -55,15 +57,17 @@ func step(r *http.Request, start, end time.Time) (time.Duration, error) { return time.Duration(defaultQueryRangeStep(start, end)) * time.Second, nil } - d, err := time.ParseDuration(value) - if err == nil { - return d, nil + if d, err := strconv.ParseFloat(value, 64); err == nil { + ts := d * float64(time.Second) + if ts > float64(math.MaxInt64) || ts < float64(math.MinInt64) { + return 0, errors.Errorf("cannot parse %q to a valid duration. It overflows int64", value) + } + return time.Duration(ts), nil } - f, err := strconv.ParseFloat(value, 64) - if err == nil { - return time.Duration(f) * time.Second, nil + if d, err := model.ParseDuration(value); err == nil { + return time.Duration(d), nil } - return 0, err + return 0, errors.Errorf("cannot parse %q to a valid duration", value) } // defaultQueryRangeStep returns the default step used in the query range API, diff --git a/pkg/loghttp/params_test.go b/pkg/loghttp/params_test.go index ac0ae52f525d..7146a92e55dc 100644 --- a/pkg/loghttp/params_test.go +++ b/pkg/loghttp/params_test.go @@ -74,7 +74,7 @@ func TestHttp_ParseRangeQuery_Step(t *testing.T) { Direction: logproto.BACKWARD, }, }, - "should use the input step parameter if provided as a float": { + "should use the input step parameter if provided as a float without decimals": { reqPath: "/loki/api/v1/query_range?query={}&start=0&end=3600000000000&step=5.000", expected: &RangeQuery{ Query: "{}", @@ -85,7 +85,18 @@ func TestHttp_ParseRangeQuery_Step(t *testing.T) { Direction: logproto.BACKWARD, }, }, - "should use the input step parameter if provided as a duration": { + "should use the input step parameter if provided as a float with decimals": { + reqPath: "/loki/api/v1/query_range?query={}&start=0&end=3600000000000&step=5.500", + expected: &RangeQuery{ + Query: "{}", + Start: time.Unix(0, 0), + End: time.Unix(3600, 0), + Step: 5.5 * 1e9, + Limit: 100, + Direction: logproto.BACKWARD, + }, + }, + "should use the input step parameter if provided as a duration in seconds": { reqPath: "/loki/api/v1/query_range?query={}&start=0&end=3600000000000&step=5s", expected: &RangeQuery{ Query: "{}", @@ -96,6 +107,17 @@ func TestHttp_ParseRangeQuery_Step(t *testing.T) { Direction: logproto.BACKWARD, }, }, + "should use the input step parameter if provided as a duration in days": { + reqPath: "/loki/api/v1/query_range?query={}&start=0&end=3600000000000&step=5d", + expected: &RangeQuery{ + Query: "{}", + Start: time.Unix(0, 0), + End: time.Unix(3600, 0), + Step: 5 * 24 * 3600 * time.Second, + Limit: 100, + Direction: logproto.BACKWARD, + }, + }, } for testName, testData := range tests { From 9214a768e351fc1ebdc83d8262468535d8068b6c Mon Sep 17 00:00:00 2001 From: Cyril Tovena Date: Tue, 3 Dec 2019 10:49:08 -0500 Subject: [PATCH 3/3] Fixes linter Signed-off-by: Cyril Tovena --- pkg/loghttp/params.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/loghttp/params.go b/pkg/loghttp/params.go index c59d33d5a39e..948b0b7a65c2 100644 --- a/pkg/loghttp/params.go +++ b/pkg/loghttp/params.go @@ -2,14 +2,15 @@ package loghttp import ( "fmt" - "github.com/pkg/errors" - "github.com/prometheus/common/model" "math" "net/http" "strconv" "strings" "time" + "github.com/pkg/errors" + "github.com/prometheus/common/model" + "github.com/grafana/loki/pkg/logproto" )