From d5f68872943062d4535ef8cffc53b4613a541d11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=8C=B2=20Harry=20=F0=9F=8C=8A=20John=20=F0=9F=8F=94?= Date: Wed, 15 May 2024 11:39:54 -0700 Subject: [PATCH] Pass limit param as hint to storage.Querier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: 🌲 Harry 🌊 John 🏔 --- docs/querying/api.md | 6 +++--- promql/engine_test.go | 4 ++-- storage/fanout_test.go | 4 ++-- storage/interface.go | 18 ++++++++++++++---- storage/merge.go | 16 ++++++++-------- storage/merge_test.go | 10 +++++----- storage/noop.go | 8 ++++---- storage/remote/read.go | 4 ++-- storage/secondary.go | 8 ++++---- tsdb/db_test.go | 6 +++--- tsdb/querier.go | 4 ++-- tsdb/querier_test.go | 2 +- web/api/v1/api.go | 39 +++++++++++++++++++++++++++++---------- web/api/v1/api_test.go | 24 ++++++++++++++++++++++-- web/api/v1/errors_test.go | 4 ++-- 15 files changed, 103 insertions(+), 54 deletions(-) diff --git a/docs/querying/api.md b/docs/querying/api.md index 71e01b3b9..a1c3cb8fe 100644 --- a/docs/querying/api.md +++ b/docs/querying/api.md @@ -256,7 +256,7 @@ URL query parameters: series to return. At least one `match[]` argument must be provided. - `start=`: Start timestamp. - `end=`: End timestamp. -- `limit=`: Maximum number of returned series. Optional. +- `limit=`: Maximum number of returned series. Optional. 0 means disabled. You can URL-encode these parameters directly in the request body by using the `POST` method and `Content-Type: application/x-www-form-urlencoded` header. This is useful when specifying a large @@ -307,7 +307,7 @@ URL query parameters: - `end=`: End timestamp. Optional. - `match[]=`: Repeated series selector argument that selects the series from which to read the label names. Optional. -- `limit=`: Maximum number of returned series. Optional. +- `limit=`: Maximum number of returned series. Optional. 0 means disabled. The `data` section of the JSON response is a list of string label names. @@ -358,7 +358,7 @@ URL query parameters: - `end=`: End timestamp. Optional. - `match[]=`: Repeated series selector argument that selects the series from which to read the label values. Optional. -- `limit=`: Maximum number of returned series. Optional. +- `limit=`: Maximum number of returned series. Optional. 0 means disabled. The `data` section of the JSON response is a list of string label values. diff --git a/promql/engine_test.go b/promql/engine_test.go index 4e321a6c3..503db4187 100644 --- a/promql/engine_test.go +++ b/promql/engine_test.go @@ -236,11 +236,11 @@ func (q *errQuerier) Select(context.Context, bool, *storage.SelectHints, ...*lab return errSeriesSet{err: q.err} } -func (*errQuerier) LabelValues(context.Context, string, ...*labels.Matcher) ([]string, annotations.Annotations, error) { +func (*errQuerier) LabelValues(context.Context, string, *storage.LabelHints, ...*labels.Matcher) ([]string, annotations.Annotations, error) { return nil, nil, nil } -func (*errQuerier) LabelNames(context.Context, ...*labels.Matcher) ([]string, annotations.Annotations, error) { +func (*errQuerier) LabelNames(context.Context, *storage.LabelHints, ...*labels.Matcher) ([]string, annotations.Annotations, error) { return nil, nil, nil } func (*errQuerier) Close() error { return nil } diff --git a/storage/fanout_test.go b/storage/fanout_test.go index 913e2fe24..19bce6172 100644 --- a/storage/fanout_test.go +++ b/storage/fanout_test.go @@ -236,11 +236,11 @@ func (errQuerier) Select(context.Context, bool, *storage.SelectHints, ...*labels return storage.ErrSeriesSet(errSelect) } -func (errQuerier) LabelValues(context.Context, string, ...*labels.Matcher) ([]string, annotations.Annotations, error) { +func (errQuerier) LabelValues(context.Context, string, *storage.LabelHints, ...*labels.Matcher) ([]string, annotations.Annotations, error) { return nil, nil, errors.New("label values error") } -func (errQuerier) LabelNames(context.Context, ...*labels.Matcher) ([]string, annotations.Annotations, error) { +func (errQuerier) LabelNames(context.Context, *storage.LabelHints, ...*labels.Matcher) ([]string, annotations.Annotations, error) { return nil, nil, errors.New("label names error") } diff --git a/storage/interface.go b/storage/interface.go index 493c2d689..f85f985e9 100644 --- a/storage/interface.go +++ b/storage/interface.go @@ -122,11 +122,11 @@ type MockQuerier struct { SelectMockFunction func(sortSeries bool, hints *SelectHints, matchers ...*labels.Matcher) SeriesSet } -func (q *MockQuerier) LabelValues(context.Context, string, ...*labels.Matcher) ([]string, annotations.Annotations, error) { +func (q *MockQuerier) LabelValues(context.Context, string, *LabelHints, ...*labels.Matcher) ([]string, annotations.Annotations, error) { return nil, nil, nil } -func (q *MockQuerier) LabelNames(context.Context, ...*labels.Matcher) ([]string, annotations.Annotations, error) { +func (q *MockQuerier) LabelNames(context.Context, *LabelHints, ...*labels.Matcher) ([]string, annotations.Annotations, error) { return nil, nil, nil } @@ -161,12 +161,12 @@ type LabelQuerier interface { // It is not safe to use the strings beyond the lifetime of the querier. // If matchers are specified the returned result set is reduced // to label values of metrics matching the matchers. - LabelValues(ctx context.Context, name string, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) + LabelValues(ctx context.Context, name string, hints *LabelHints, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) // LabelNames returns all the unique label names present in the block in sorted order. // If matchers are specified the returned result set is reduced // to label names of metrics matching the matchers. - LabelNames(ctx context.Context, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) + LabelNames(ctx context.Context, hints *LabelHints, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) // Close releases the resources of the Querier. Close() error @@ -190,6 +190,9 @@ type SelectHints struct { Start int64 // Start time in milliseconds for this select. End int64 // End time in milliseconds for this select. + // Maximum number of results returned. Use a value of 0 to disable. + Limit int + Step int64 // Query step size in milliseconds. Func string // String representation of surrounding function or aggregation. @@ -217,6 +220,13 @@ type SelectHints struct { DisableTrimming bool } +// LabelHints specifies hints passed for label reads. +// This is used only as an option for implementation to use. +type LabelHints struct { + // Maximum number of results returned. Use a value of 0 to disable. + Limit int +} + // TODO(bwplotka): Move to promql/engine_test.go? // QueryableFunc is an adapter to allow the use of ordinary functions as // Queryables. It follows the idea of http.HandlerFunc. diff --git a/storage/merge.go b/storage/merge.go index 885560022..35c0c44ba 100644 --- a/storage/merge.go +++ b/storage/merge.go @@ -161,8 +161,8 @@ func (l labelGenericQueriers) SplitByHalf() (labelGenericQueriers, labelGenericQ // LabelValues returns all potential values for a label name. // If matchers are specified the returned result set is reduced // to label values of metrics matching the matchers. -func (q *mergeGenericQuerier) LabelValues(ctx context.Context, name string, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) { - res, ws, err := q.lvals(ctx, q.queriers, name, matchers...) +func (q *mergeGenericQuerier) LabelValues(ctx context.Context, name string, hints *LabelHints, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) { + res, ws, err := q.lvals(ctx, q.queriers, name, hints, matchers...) if err != nil { return nil, nil, fmt.Errorf("LabelValues() from merge generic querier for label %s: %w", name, err) } @@ -170,22 +170,22 @@ func (q *mergeGenericQuerier) LabelValues(ctx context.Context, name string, matc } // lvals performs merge sort for LabelValues from multiple queriers. -func (q *mergeGenericQuerier) lvals(ctx context.Context, lq labelGenericQueriers, n string, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) { +func (q *mergeGenericQuerier) lvals(ctx context.Context, lq labelGenericQueriers, n string, hints *LabelHints, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) { if lq.Len() == 0 { return nil, nil, nil } if lq.Len() == 1 { - return lq.Get(0).LabelValues(ctx, n, matchers...) + return lq.Get(0).LabelValues(ctx, n, hints, matchers...) } a, b := lq.SplitByHalf() var ws annotations.Annotations - s1, w, err := q.lvals(ctx, a, n, matchers...) + s1, w, err := q.lvals(ctx, a, n, hints, matchers...) ws.Merge(w) if err != nil { return nil, ws, err } - s2, ws, err := q.lvals(ctx, b, n, matchers...) + s2, ws, err := q.lvals(ctx, b, n, hints, matchers...) ws.Merge(w) if err != nil { return nil, ws, err @@ -221,13 +221,13 @@ func mergeStrings(a, b []string) []string { } // LabelNames returns all the unique label names present in all queriers in sorted order. -func (q *mergeGenericQuerier) LabelNames(ctx context.Context, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) { +func (q *mergeGenericQuerier) LabelNames(ctx context.Context, hints *LabelHints, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) { var ( labelNamesMap = make(map[string]struct{}) warnings annotations.Annotations ) for _, querier := range q.queriers { - names, wrn, err := querier.LabelNames(ctx, matchers...) + names, wrn, err := querier.LabelNames(ctx, hints, matchers...) if wrn != nil { // TODO(bwplotka): We could potentially wrap warnings. warnings.Merge(wrn) diff --git a/storage/merge_test.go b/storage/merge_test.go index 0e63affbb..e108e7146 100644 --- a/storage/merge_test.go +++ b/storage/merge_test.go @@ -1361,7 +1361,7 @@ func (m *mockGenericQuerier) Select(_ context.Context, b bool, _ *SelectHints, _ return &mockGenericSeriesSet{resp: m.resp, warnings: m.warnings, err: m.err} } -func (m *mockGenericQuerier) LabelValues(_ context.Context, name string, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) { +func (m *mockGenericQuerier) LabelValues(_ context.Context, name string, hints *LabelHints, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) { m.mtx.Lock() m.labelNamesRequested = append(m.labelNamesRequested, labelNameRequest{ name: name, @@ -1371,7 +1371,7 @@ func (m *mockGenericQuerier) LabelValues(_ context.Context, name string, matcher return m.resp, m.warnings, m.err } -func (m *mockGenericQuerier) LabelNames(context.Context, ...*labels.Matcher) ([]string, annotations.Annotations, error) { +func (m *mockGenericQuerier) LabelNames(context.Context, *LabelHints, ...*labels.Matcher) ([]string, annotations.Annotations, error) { m.mtx.Lock() m.labelNamesCalls++ m.mtx.Unlock() @@ -1560,7 +1560,7 @@ func TestMergeGenericQuerierWithSecondaries_ErrorHandling(t *testing.T) { } }) t.Run("LabelNames", func(t *testing.T) { - res, w, err := q.LabelNames(ctx) + res, w, err := q.LabelNames(ctx, nil) require.Subset(t, tcase.expectedWarnings, w) require.ErrorIs(t, err, tcase.expectedErrs[1], "expected error doesn't match") require.Equal(t, tcase.expectedLabels, res) @@ -1575,7 +1575,7 @@ func TestMergeGenericQuerierWithSecondaries_ErrorHandling(t *testing.T) { } }) t.Run("LabelValues", func(t *testing.T) { - res, w, err := q.LabelValues(ctx, "test") + res, w, err := q.LabelValues(ctx, "test", nil) require.Subset(t, tcase.expectedWarnings, w) require.ErrorIs(t, err, tcase.expectedErrs[2], "expected error doesn't match") require.Equal(t, tcase.expectedLabels, res) @@ -1591,7 +1591,7 @@ func TestMergeGenericQuerierWithSecondaries_ErrorHandling(t *testing.T) { }) t.Run("LabelValuesWithMatchers", func(t *testing.T) { matcher := labels.MustNewMatcher(labels.MatchEqual, "otherLabel", "someValue") - res, w, err := q.LabelValues(ctx, "test2", matcher) + res, w, err := q.LabelValues(ctx, "test2", nil, matcher) require.Subset(t, tcase.expectedWarnings, w) require.ErrorIs(t, err, tcase.expectedErrs[3], "expected error doesn't match") require.Equal(t, tcase.expectedLabels, res) diff --git a/storage/noop.go b/storage/noop.go index be5741ddd..f5092da7c 100644 --- a/storage/noop.go +++ b/storage/noop.go @@ -31,11 +31,11 @@ func (noopQuerier) Select(context.Context, bool, *SelectHints, ...*labels.Matche return NoopSeriesSet() } -func (noopQuerier) LabelValues(context.Context, string, ...*labels.Matcher) ([]string, annotations.Annotations, error) { +func (noopQuerier) LabelValues(context.Context, string, *LabelHints, ...*labels.Matcher) ([]string, annotations.Annotations, error) { return nil, nil, nil } -func (noopQuerier) LabelNames(context.Context, ...*labels.Matcher) ([]string, annotations.Annotations, error) { +func (noopQuerier) LabelNames(context.Context, *LabelHints, ...*labels.Matcher) ([]string, annotations.Annotations, error) { return nil, nil, nil } @@ -54,11 +54,11 @@ func (noopChunkQuerier) Select(context.Context, bool, *SelectHints, ...*labels.M return NoopChunkedSeriesSet() } -func (noopChunkQuerier) LabelValues(context.Context, string, ...*labels.Matcher) ([]string, annotations.Annotations, error) { +func (noopChunkQuerier) LabelValues(context.Context, string, *LabelHints, ...*labels.Matcher) ([]string, annotations.Annotations, error) { return nil, nil, nil } -func (noopChunkQuerier) LabelNames(context.Context, ...*labels.Matcher) ([]string, annotations.Annotations, error) { +func (noopChunkQuerier) LabelNames(context.Context, *LabelHints, ...*labels.Matcher) ([]string, annotations.Annotations, error) { return nil, nil, nil } diff --git a/storage/remote/read.go b/storage/remote/read.go index 723030091..e54b14f1e 100644 --- a/storage/remote/read.go +++ b/storage/remote/read.go @@ -210,13 +210,13 @@ func (q querier) addExternalLabels(ms []*labels.Matcher) ([]*labels.Matcher, []s } // LabelValues implements storage.Querier and is a noop. -func (q *querier) LabelValues(context.Context, string, ...*labels.Matcher) ([]string, annotations.Annotations, error) { +func (q *querier) LabelValues(context.Context, string, *storage.LabelHints, ...*labels.Matcher) ([]string, annotations.Annotations, error) { // TODO: Implement: https://github.com/prometheus/prometheus/issues/3351 return nil, nil, errors.New("not implemented") } // LabelNames implements storage.Querier and is a noop. -func (q *querier) LabelNames(context.Context, ...*labels.Matcher) ([]string, annotations.Annotations, error) { +func (q *querier) LabelNames(context.Context, *storage.LabelHints, ...*labels.Matcher) ([]string, annotations.Annotations, error) { // TODO: Implement: https://github.com/prometheus/prometheus/issues/3351 return nil, nil, errors.New("not implemented") } diff --git a/storage/secondary.go b/storage/secondary.go index 44d978183..1cf8024b6 100644 --- a/storage/secondary.go +++ b/storage/secondary.go @@ -49,16 +49,16 @@ func newSecondaryQuerierFromChunk(cq ChunkQuerier) genericQuerier { return &secondaryQuerier{genericQuerier: newGenericQuerierFromChunk(cq)} } -func (s *secondaryQuerier) LabelValues(ctx context.Context, name string, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) { - vals, w, err := s.genericQuerier.LabelValues(ctx, name, matchers...) +func (s *secondaryQuerier) LabelValues(ctx context.Context, name string, hints *LabelHints, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) { + vals, w, err := s.genericQuerier.LabelValues(ctx, name, hints, matchers...) if err != nil { return nil, w.Add(err), nil } return vals, w, nil } -func (s *secondaryQuerier) LabelNames(ctx context.Context, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) { - names, w, err := s.genericQuerier.LabelNames(ctx, matchers...) +func (s *secondaryQuerier) LabelNames(ctx context.Context, hints *LabelHints, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) { + names, w, err := s.genericQuerier.LabelNames(ctx, hints, matchers...) if err != nil { return nil, w.Add(err), nil } diff --git a/tsdb/db_test.go b/tsdb/db_test.go index 3d2fb2d99..a96a2f9a6 100644 --- a/tsdb/db_test.go +++ b/tsdb/db_test.go @@ -1001,7 +1001,7 @@ func TestWALFlushedOnDBClose(t *testing.T) { q, err := db.Querier(0, 1) require.NoError(t, err) - values, ws, err := q.LabelValues(ctx, "labelname") + values, ws, err := q.LabelValues(ctx, "labelname", nil) require.NoError(t, err) require.Empty(t, ws) require.Equal(t, []string{"labelvalue"}, values) @@ -1976,7 +1976,7 @@ func TestQuerierWithBoundaryChunks(t *testing.T) { defer q.Close() // The requested interval covers 2 blocks, so the querier's label values for blockID should give us 2 values, one from each block. - b, ws, err := q.LabelValues(ctx, "blockID") + b, ws, err := q.LabelValues(ctx, "blockID", nil) require.NoError(t, err) var nilAnnotations annotations.Annotations require.Equal(t, nilAnnotations, ws) @@ -2288,7 +2288,7 @@ func TestDB_LabelNames(t *testing.T) { q, err := db.Querier(math.MinInt64, math.MaxInt64) require.NoError(t, err) var ws annotations.Annotations - labelNames, ws, err = q.LabelNames(ctx) + labelNames, ws, err = q.LabelNames(ctx, nil) require.NoError(t, err) require.Empty(t, ws) require.NoError(t, q.Close()) diff --git a/tsdb/querier.go b/tsdb/querier.go index fb4a87cc8..910c2d7fc 100644 --- a/tsdb/querier.go +++ b/tsdb/querier.go @@ -77,12 +77,12 @@ func newBlockBaseQuerier(b BlockReader, mint, maxt int64) (*blockBaseQuerier, er }, nil } -func (q *blockBaseQuerier) LabelValues(ctx context.Context, name string, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) { +func (q *blockBaseQuerier) LabelValues(ctx context.Context, name string, hints *storage.LabelHints, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) { res, err := q.index.SortedLabelValues(ctx, name, matchers...) return res, nil, err } -func (q *blockBaseQuerier) LabelNames(ctx context.Context, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) { +func (q *blockBaseQuerier) LabelNames(ctx context.Context, hints *storage.LabelHints, matchers ...*labels.Matcher) ([]string, annotations.Annotations, error) { res, err := q.index.LabelNames(ctx, matchers...) return res, nil, err } diff --git a/tsdb/querier_test.go b/tsdb/querier_test.go index a1af49465..ffdf8dc02 100644 --- a/tsdb/querier_test.go +++ b/tsdb/querier_test.go @@ -3022,7 +3022,7 @@ func TestQuerierIndexQueriesRace(t *testing.T) { q, err := db.Querier(math.MinInt64, math.MaxInt64) require.NoError(t, err) - values, _, err := q.LabelValues(ctx, "seq", c.matchers...) + values, _, err := q.LabelValues(ctx, "seq", nil, c.matchers...) require.NoError(t, err) require.Emptyf(t, values, `label values for label "seq" should be empty`) diff --git a/web/api/v1/api.go b/web/api/v1/api.go index b95ff25cf..4468a3263 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -658,6 +658,10 @@ func (api *API) labelNames(r *http.Request) apiFuncResult { return apiFuncResult{nil, &apiError{errorBadData, err}, nil, nil} } + hints := &storage.LabelHints{ + Limit: toHintLimit(limit), + } + q, err := api.Queryable.Querier(timestamp.FromTime(start), timestamp.FromTime(end)) if err != nil { return apiFuncResult{nil, returnAPIError(err), nil, nil} @@ -672,7 +676,7 @@ func (api *API) labelNames(r *http.Request) apiFuncResult { labelNamesSet := make(map[string]struct{}) for _, matchers := range matcherSets { - vals, callWarnings, err := q.LabelNames(r.Context(), matchers...) + vals, callWarnings, err := q.LabelNames(r.Context(), hints, matchers...) if err != nil { return apiFuncResult{nil, returnAPIError(err), warnings, nil} } @@ -694,7 +698,7 @@ func (api *API) labelNames(r *http.Request) apiFuncResult { if len(matcherSets) == 1 { matchers = matcherSets[0] } - names, warnings, err = q.LabelNames(r.Context(), matchers...) + names, warnings, err = q.LabelNames(r.Context(), hints, matchers...) if err != nil { return apiFuncResult{nil, &apiError{errorExec, err}, warnings, nil} } @@ -704,7 +708,7 @@ func (api *API) labelNames(r *http.Request) apiFuncResult { names = []string{} } - if len(names) > limit { + if limit > 0 && len(names) > limit { names = names[:limit] warnings = warnings.Add(errors.New("results truncated due to limit")) } @@ -738,6 +742,10 @@ func (api *API) labelValues(r *http.Request) (result apiFuncResult) { return apiFuncResult{nil, &apiError{errorBadData, err}, nil, nil} } + hints := &storage.LabelHints{ + Limit: toHintLimit(limit), + } + q, err := api.Queryable.Querier(timestamp.FromTime(start), timestamp.FromTime(end)) if err != nil { return apiFuncResult{nil, &apiError{errorExec, err}, nil, nil} @@ -762,7 +770,7 @@ func (api *API) labelValues(r *http.Request) (result apiFuncResult) { var callWarnings annotations.Annotations labelValuesSet := make(map[string]struct{}) for _, matchers := range matcherSets { - vals, callWarnings, err = q.LabelValues(ctx, name, matchers...) + vals, callWarnings, err = q.LabelValues(ctx, name, hints, matchers...) if err != nil { return apiFuncResult{nil, &apiError{errorExec, err}, warnings, closer} } @@ -781,7 +789,7 @@ func (api *API) labelValues(r *http.Request) (result apiFuncResult) { if len(matcherSets) == 1 { matchers = matcherSets[0] } - vals, warnings, err = q.LabelValues(ctx, name, matchers...) + vals, warnings, err = q.LabelValues(ctx, name, hints, matchers...) if err != nil { return apiFuncResult{nil, &apiError{errorExec, err}, warnings, closer} } @@ -793,7 +801,7 @@ func (api *API) labelValues(r *http.Request) (result apiFuncResult) { slices.Sort(vals) - if len(vals) > limit { + if limit > 0 && len(vals) > limit { vals = vals[:limit] warnings = warnings.Add(errors.New("results truncated due to limit")) } @@ -863,6 +871,7 @@ func (api *API) series(r *http.Request) (result apiFuncResult) { Start: timestamp.FromTime(start), End: timestamp.FromTime(end), Func: "series", // There is no series function, this token is used for lookups that don't need samples. + Limit: toHintLimit(limit), } var set storage.SeriesSet @@ -889,7 +898,7 @@ func (api *API) series(r *http.Request) (result apiFuncResult) { } metrics = append(metrics, set.At().Labels()) - if len(metrics) > limit { + if limit > 0 && len(metrics) > limit { metrics = metrics[:limit] warnings.Add(errors.New("results truncated due to limit")) return apiFuncResult{metrics, nil, warnings, closer} @@ -1898,8 +1907,8 @@ OUTER: return matcherSets, nil } +// parseLimitParam returning 0 means no limit is to be applied. func parseLimitParam(limitStr string) (limit int, err error) { - limit = math.MaxInt if limitStr == "" { return limit, nil } @@ -1908,9 +1917,19 @@ func parseLimitParam(limitStr string) (limit int, err error) { if err != nil { return limit, err } - if limit <= 0 { - return limit, errors.New("limit must be positive") + if limit < 0 { + return limit, errors.New("limit must be non-negative") } return limit, nil } + +// toHintLimit increases the API limit, as returned by parseLimitParam, by 1. +// This allows for emitting warnings when the results are truncated. +func toHintLimit(limit int) int { + // 0 means no limit and avoid int overflow + if limit > 0 && limit < math.MaxInt { + return limit + 1 + } + return limit +} diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index b30890893..f0aed49d6 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -739,13 +739,16 @@ func TestLabelNames(t *testing.T) { api := &API{ Queryable: storage, } - request := func(method string, matchers ...string) (*http.Request, error) { + request := func(method, limit string, matchers ...string) (*http.Request, error) { u, err := url.Parse("http://example.com") require.NoError(t, err) q := u.Query() for _, matcher := range matchers { q.Add("match[]", matcher) } + if limit != "" { + q.Add("limit", limit) + } u.RawQuery = q.Encode() r, err := http.NewRequest(method, u.String(), nil) @@ -759,6 +762,7 @@ func TestLabelNames(t *testing.T) { name string api *API matchers []string + limit string expected []string expectedErrorType errorType }{ @@ -773,6 +777,13 @@ func TestLabelNames(t *testing.T) { expected: []string{"__name__", "abc", "foo", "xyz"}, api: api, }, + { + name: "non empty label matcher with limit", + matchers: []string{`{foo=~".+"}`}, + expected: []string{"__name__", "abc"}, + limit: "2", + api: api, + }, { name: "exact label matcher", matchers: []string{`{foo="boo"}`}, @@ -805,7 +816,7 @@ func TestLabelNames(t *testing.T) { t.Run(tc.name, func(t *testing.T) { for _, method := range []string{http.MethodGet, http.MethodPost} { ctx := context.Background() - req, err := request(method, tc.matchers...) + req, err := request(method, tc.limit, tc.matchers...) require.NoError(t, err) res := tc.api.labelNames(req.WithContext(ctx)) assertAPIError(t, res.err, tc.expectedErrorType) @@ -1430,6 +1441,15 @@ func testEndpoints(t *testing.T, api *API, tr *testTargetRetriever, es storage.E responseLen: 2, // API does not specify which particular value will come back. warningsCount: 0, // No warnings if limit isn't exceeded. }, + { + endpoint: api.series, + query: url.Values{ + "match[]": []string{"test_metric1"}, + "limit": []string{"0"}, + }, + responseLen: 2, // API does not specify which particular value will come back. + warningsCount: 0, // No warnings if limit isn't exceeded. + }, // Missing match[] query params in series requests. { endpoint: api.series, diff --git a/web/api/v1/errors_test.go b/web/api/v1/errors_test.go index e76a1a3d3..2a2b847cf 100644 --- a/web/api/v1/errors_test.go +++ b/web/api/v1/errors_test.go @@ -170,11 +170,11 @@ type errorTestQuerier struct { err error } -func (t errorTestQuerier) LabelValues(context.Context, string, ...*labels.Matcher) ([]string, annotations.Annotations, error) { +func (t errorTestQuerier) LabelValues(context.Context, string, *storage.LabelHints, ...*labels.Matcher) ([]string, annotations.Annotations, error) { return nil, nil, t.err } -func (t errorTestQuerier) LabelNames(context.Context, ...*labels.Matcher) ([]string, annotations.Annotations, error) { +func (t errorTestQuerier) LabelNames(context.Context, *storage.LabelHints, ...*labels.Matcher) ([]string, annotations.Annotations, error) { return nil, nil, t.err }