feat: Add step param to Patterns Query API (#12703)

pkg/loghttp/patterns.go

@@ -9,13 +9,27 @@ import (
 func ParsePatternsQuery(r *http.Request) (*logproto.QueryPatternsRequest, error) {
 	req := &logproto.QueryPatternsRequest{}
 
+	req.Query = query(r)
 	start, end, err := bounds(r)
 	if err != nil {
 		return nil, err
 	}
 	req.Start = start
 	req.End = end
 
-	req.Query = query(r)
+	calculatedStep, err := step(r, start, end)
+	if err != nil {
+		return nil, err
+	}
+	if calculatedStep <= 0 {
+		return nil, errZeroOrNegativeStep
+	}
+	// For safety, limit the number of returned points per timeseries.
+	// This is sufficient for 60s resolution for a week or 1h resolution for a year.
+	if (req.End.Sub(req.Start) / calculatedStep) > 11000 {
+		return nil, errStepTooSmall
+	}
+	req.Step = calculatedStep.Milliseconds()
+
 	return req, nil
 }

pkg/loghttp/patterns_test.go

@@ -0,0 +1,122 @@
+package loghttp
+
+import (
+	"net/http"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/grafana/loki/v3/pkg/logproto"
+)
+
+func TestParsePatternsQuery(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name    string
+		path    string
+		want    *logproto.QueryPatternsRequest
+		wantErr bool
+	}{
+		{
+			name: "should correctly parse valid params",
+			path: "/loki/api/v1/patterns?query={}&start=100000000000&end=3600000000000&step=5s",
+			want: &logproto.QueryPatternsRequest{
+				Query: "{}",
+				Start: time.Unix(100, 0),
+				End:   time.Unix(3600, 0),
+				Step:  (5 * time.Second).Milliseconds(),
+			},
+		},
+		{
+			name: "should default empty step param to sensible step for the range",
+			path: "/loki/api/v1/patterns?query={}&start=100000000000&end=3600000000000",
+			want: &logproto.QueryPatternsRequest{
+				Query: "{}",
+				Start: time.Unix(100, 0),
+				End:   time.Unix(3600, 0),
+				Step:  (14 * time.Second).Milliseconds(),
+			},
+		},
+		{
+			name: "should default start to zero for empty start param",
+			path: "/loki/api/v1/patterns?query={}&end=3600000000000",
+			want: &logproto.QueryPatternsRequest{
+				Query: "{}",
+				Start: time.Unix(0, 0),
+				End:   time.Unix(3600, 0),
+				Step:  (14 * time.Second).Milliseconds(),
+			},
+		},
+		{
+			name: "should accept step with no units as seconds",
+			path: "/loki/api/v1/patterns?query={}&start=100000000000&end=3600000000000&step=10",
+			want: &logproto.QueryPatternsRequest{
+				Query: "{}",
+				Start: time.Unix(100, 0),
+				End:   time.Unix(3600, 0),
+				Step:  (10 * time.Second).Milliseconds(),
+			},
+		},
+		{
+			name: "should accept step as string duration in seconds",
+			path: "/loki/api/v1/patterns?query={}&start=100000000000&end=3600000000000&step=15s",
+			want: &logproto.QueryPatternsRequest{
+				Query: "{}",
+				Start: time.Unix(100, 0),
+				End:   time.Unix(3600, 0),
+				Step:  (15 * time.Second).Milliseconds(),
+			},
+		},
+		{
+			name: "should correctly parse long duration for step",
+			path: "/loki/api/v1/patterns?query={}&start=100000000000&end=3600000000000&step=10h",
+			want: &logproto.QueryPatternsRequest{
+				Query: "{}",
+				Start: time.Unix(100, 0),
+				End:   time.Unix(3600, 0),
+				Step:  (10 * time.Hour).Milliseconds(),
+			},
+		},
+		{
+			name:    "should reject negative step value",
+			path:    "/loki/api/v1/patterns?query={}&start=100000000000&end=3600000000000&step=-5s",
+			want:    nil,
+			wantErr: true,
+		},
+		{
+			name:    "should reject very small step for big range",
+			path:    "/loki/api/v1/patterns?query={}&start=100000000000&end=3600000000000&step=50ms",
+			want:    nil,
+			wantErr: true,
+		},
+		{
+			name: "should accept very small step for small range",
+			path: "/loki/api/v1/patterns?query={}&start=100000000000&end=110000000000&step=50ms",
+			want: &logproto.QueryPatternsRequest{
+				Query: "{}",
+				Start: time.Unix(100, 0),
+				End:   time.Unix(110, 0),
+				Step:  (50 * time.Millisecond).Milliseconds(),
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			req, err := http.NewRequest(http.MethodGet, tt.path, nil)
+			require.NoError(t, err)
+			err = req.ParseForm()
+			require.NoError(t, err)
+
+			got, err := ParsePatternsQuery(req)
+			if tt.wantErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+			assert.Equalf(t, tt.want, got, "Incorrect response from input path: %s", tt.path)
+		})
+	}
+}

pkg/logproto/compat.go

@@ -335,6 +335,7 @@ func (m *VolumeRequest) LogToSpan(sp opentracing.Span) {
 		otlog.String("query", m.GetQuery()),
 		otlog.String("start", timestamp.Time(int64(m.From)).String()),
 		otlog.String("end", timestamp.Time(int64(m.Through)).String()),
+		otlog.String("step", time.Duration(m.Step).String()),
 	)
 }
 
@@ -448,8 +449,6 @@ func (m *ShardsRequest) LogToSpan(sp opentracing.Span) {
 
 func (m *QueryPatternsRequest) GetCachingOptions() (res definitions.CachingOptions) { return }
 
-func (m *QueryPatternsRequest) GetStep() int64 { return 0 }
-
 func (m *QueryPatternsRequest) WithStartEnd(start, end time.Time) definitions.Request {
 	clone := *m
 	clone.Start = start
@@ -469,9 +468,10 @@ func (m *QueryPatternsRequest) WithStartEndForCache(start, end time.Time) result
 
 func (m *QueryPatternsRequest) LogToSpan(sp opentracing.Span) {
 	fields := []otlog.Field{
+		otlog.String("query", m.GetQuery()),
 		otlog.String("start", m.Start.String()),
 		otlog.String("end", m.End.String()),
-		otlog.String("query", m.GetQuery()),
+		otlog.String("step", time.Duration(m.Step).String()),
 	}
 	sp.LogFields(fields...)
 }

pkg/logproto/pattern.proto

@@ -24,6 +24,7 @@ message QueryPatternsRequest {
     (gogoproto.stdtime) = true,
     (gogoproto.nullable) = false
   ];
+  int64 step = 4;
 }
 
 message QueryPatternsResponse {