logcli: added --step support to query command (#1103)

* logcli: added --step support to query command

* Changed query range API default step value from 1s to a dynamic value based on start/end input params

cmd/logcli/main.go

@@ -157,6 +157,7 @@ func newQuery(instant bool, cmd *kingpin.CmdClause) *query.Query {
 		cmd.Flag("since", "Lookback window.").Default("1h").DurationVar(&since)
 		cmd.Flag("from", "Start looking for logs at this absolute time (inclusive)").StringVar(&from)
 		cmd.Flag("to", "Stop looking for logs at this absolute time (exclusive)").StringVar(&to)
+		cmd.Flag("step", "Query resolution step width").DurationVar(&query.Step)
 	}
 
 	cmd.Flag("forward", "Scan forwards through logs.").Default("false").BoolVar(&query.Forward)

docs/api.md

@@ -199,7 +199,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 1.
+- `step`: Query resolution step width in 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

pkg/logcli/client/client.go

@@ -8,6 +8,7 @@ import (
 	"log"
 	"net/http"
 	"net/url"
+	"strconv"
 	"strings"
 	"time"
 
@@ -21,7 +22,7 @@ import (
 
 const (
 	queryPath       = "/loki/api/v1/query?query=%s&limit=%d&time=%d&direction=%s"
-	queryRangePath  = "/loki/api/v1/query_range?query=%s&limit=%d&start=%d&end=%d&direction=%s"
+	queryRangePath  = "/loki/api/v1/query_range"
 	labelsPath      = "/loki/api/v1/label"
 	labelValuesPath = "/loki/api/v1/label/%s/values"
 	tailPath        = "/loki/api/v1/tail?query=%s&delay_for=%d&limit=%d&start=%d"
@@ -52,16 +53,21 @@ func (c *Client) Query(queryStr string, limit int, time time.Time, direction log
 // QueryRange uses the /api/v1/query_range endpoint to execute a range query
 // excluding interfacer b/c it suggests taking the interface promql.Node instead of logproto.Direction b/c it happens to have a String() method
 // nolint:interfacer
-func (c *Client) QueryRange(queryStr string, limit int, from, through time.Time, direction logproto.Direction, quiet bool) (*loghttp.QueryResponse, error) {
-	path := fmt.Sprintf(queryRangePath,
-		url.QueryEscape(queryStr), // query
-		limit,                     // limit
-		from.UnixNano(),           // start
-		through.UnixNano(),        // end
-		direction.String(),        // direction
-	)
+func (c *Client) QueryRange(queryStr string, limit int, from, through time.Time, direction logproto.Direction, step time.Duration, quiet bool) (*loghttp.QueryResponse, error) {
+	params := url.Values{}
+	params.Set("query", queryStr)
+	params.Set("limit", strconv.Itoa(limit))
+	params.Set("start", strconv.FormatInt(from.UnixNano(), 10))
+	params.Set("end", strconv.FormatInt(through.UnixNano(), 10))
+	params.Set("direction", direction.String())
 
-	return c.doQuery(path, quiet)
+	// The step is optional, so we do set it only if provided,
+	// otherwise we do leverage on the API defaults
+	if step != 0 {
+		params.Set("step", strconv.FormatInt(int64(step.Seconds()), 10))
+	}
+
+	return c.doQuery(queryRangePath+"?"+params.Encode(), quiet)
 }
 
 // ListLabelNames uses the /api/v1/label endpoint to list label names

pkg/logcli/query/query.go

@@ -30,6 +30,7 @@ type Query struct {
 	End             time.Time
 	Limit           int
 	Forward         bool
+	Step            time.Duration
 	Quiet           bool
 	NoLabels        bool
 	IgnoreLabelsKey []string
@@ -47,7 +48,7 @@ func (q *Query) DoQuery(c *client.Client, out output.LogOutput) {
 	if q.isInstant() {
 		resp, err = c.Query(q.QueryString, q.Limit, q.Start, d, q.Quiet)
 	} else {
-		resp, err = c.QueryRange(q.QueryString, q.Limit, q.Start, q.End, d, q.Quiet)
+		resp, err = c.QueryRange(q.QueryString, q.Limit, q.Start, q.End, d, q.Step, q.Quiet)
 	}
 
 	if err != nil {

pkg/querier/http.go

@@ -32,7 +32,6 @@ const (
 	defaultSince         = 1 * time.Hour
 	wsPingPeriod         = 1 * time.Second
 	maxDelayForInTailing = 5
-	defaultStep          = 1 // 1 seconds
 )
 
 // nolint
@@ -85,6 +84,12 @@ func directionParam(values url.Values, name string, def logproto.Direction) (log
 	return logproto.Direction(d), nil
 }
 
+// defaultQueryRangeStep returns the default step used in the query range API,
+// which is dinamically calculated based on the time range
+func defaultQueryRangeStep(start time.Time, end time.Time) int {
+	return int(math.Max(math.Floor(end.Sub(start).Seconds()/250), 1))
+}
+
 func httpRequestToInstantQueryRequest(httpRequest *http.Request) (*instantQueryRequest, error) {
 	params := httpRequest.URL.Query()
 	queryRequest := instantQueryRequest{
@@ -111,21 +116,24 @@ func httpRequestToInstantQueryRequest(httpRequest *http.Request) (*instantQueryR
 }
 
 func httpRequestToRangeQueryRequest(httpRequest *http.Request) (*rangeQueryRequest, error) {
+	var err error
+
 	params := httpRequest.URL.Query()
 	queryRequest := rangeQueryRequest{
 		query: params.Get("query"),
 	}
 
-	step, err := intParam(params, "step", defaultStep)
+	queryRequest.limit, queryRequest.start, queryRequest.end, err = httpRequestToLookback(httpRequest)
 	if err != nil {
-		return nil, httpgrpc.Errorf(http.StatusBadRequest, err.Error())
+		return nil, err
 	}
-	queryRequest.step = time.Duration(step) * time.Second
 
-	queryRequest.limit, queryRequest.start, queryRequest.end, err = httpRequestToLookback(httpRequest)
+	step, err := intParam(params, "step", defaultQueryRangeStep(queryRequest.start, queryRequest.end))
 	if err != nil {
-		return nil, err
+		return nil, httpgrpc.Errorf(http.StatusBadRequest, err.Error())
 	}
+	queryRequest.step = time.Duration(step) * time.Second
+
 	queryRequest.direction, err = directionParam(params, "direction", logproto.BACKWARD)
 	if err != nil {
 		return nil, httpgrpc.Errorf(http.StatusBadRequest, err.Error())

pkg/querier/http_test.go

@@ -0,0 +1,90 @@
+package querier
+
+import (
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"github.com/grafana/loki/pkg/logproto"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestHttp_defaultQueryRangeStep(t *testing.T) {
+	t.Parallel()
+
+	tests := map[string]struct {
+		start    time.Time
+		end      time.Time
+		expected int
+	}{
+		"should not be lower then 1s": {
+			start:    time.Unix(60, 0),
+			end:      time.Unix(60, 0),
+			expected: 1,
+		},
+		"should return 1s if input time range is 5m": {
+			start:    time.Unix(60, 0),
+			end:      time.Unix(360, 0),
+			expected: 1,
+		},
+		"should return 14s if input time range is 1h": {
+			start:    time.Unix(60, 0),
+			end:      time.Unix(3660, 0),
+			expected: 14,
+		},
+	}
+
+	for testName, testData := range tests {
+		testData := testData
+
+		t.Run(testName, func(t *testing.T) {
+			assert.Equal(t, testData.expected, defaultQueryRangeStep(testData.start, testData.end))
+		})
+	}
+}
+
+func TestHttp_httpRequestToRangeQueryRequest(t *testing.T) {
+	t.Parallel()
+
+	tests := map[string]struct {
+		reqPath  string
+		expected *rangeQueryRequest
+	}{
+		"should set the default step based on the input time range if the step parameter is not provided": {
+			reqPath: "/loki/api/v1/query_range?query={}&start=0&end=3600000000000",
+			expected: &rangeQueryRequest{
+				query:     "{}",
+				start:     time.Unix(0, 0),
+				end:       time.Unix(3600, 0),
+				step:      14 * time.Second,
+				limit:     100,
+				direction: logproto.BACKWARD,
+			},
+		},
+		"should use the input step parameter if provided": {
+			reqPath: "/loki/api/v1/query_range?query={}&start=0&end=3600000000000&step=5",
+			expected: &rangeQueryRequest{
+				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 {
+		testData := testData
+
+		t.Run(testName, func(t *testing.T) {
+			req := httptest.NewRequest("GET", testData.reqPath, nil)
+			actual, err := httpRequestToRangeQueryRequest(req)
+
+			require.NoError(t, err)
+			assert.Equal(t, testData.expected, actual)
+		})
+	}
+}