Skip to content

Commit

Permalink
Make limit setting more explicit
Browse files Browse the repository at this point in the history 
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
  • Loading branch information
@dimitarvdimitrov
dimitarvdimitrov committed Apr 3, 2024
1 parent 81b3329 commit 663121c
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 12 deletions.
15 changes: 12 additions & 3 deletions pkg/frontend/querymiddleware/codec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,9 +313,12 @@ func (prometheusCodec) DecodeLabelsQueryRequest(_ context.Context, r *http.Reque

labelMatcherSets := reqValues["match[]"]

limit, err := strconv.ParseUint(reqValues.Get("limit"), 10, 64)
if reqValues.Has("limit") && err != nil {
return nil, apierror.New(apierror.TypeBadData, "limit parameter: "+err.Error())
limit := uint64(0) // 0 means unlimited
if limitStr := reqValues.Get("limit"); limitStr != "" {
limit, err = strconv.ParseUint(limitStr, 10, 64)
if err != nil || limit == 0 {
return nil, apierror.New(apierror.TypeBadData, fmt.Sprintf("limit parameter must be a positive number: %s", limitStr))
}
}

if IsLabelNamesQuery(r.URL.Path) {
Expand Down Expand Up @@ -535,6 +538,9 @@ func (c prometheusCodec) EncodeLabelsQueryRequest(ctx context.Context, req Label
if len(req.GetLabelMatcherSets()) > 0 {
urlValues["match[]"] = req.GetLabelMatcherSets()
}
if req.GetLimit() > 0 {
urlValues["limit"] = []string{strconv.FormatUint(req.GetLimit(), 10)}
}
u = &url.URL{
Path: req.Path,
RawQuery: urlValues.Encode(),
Expand All @@ -552,6 +558,9 @@ func (c prometheusCodec) EncodeLabelsQueryRequest(ctx context.Context, req Label
if len(req.GetLabelMatcherSets()) > 0 {
urlValues["match[]"] = req.GetLabelMatcherSets()
}
if req.GetLimit() > 0 {
urlValues["limit"] = []string{strconv.FormatUint(req.GetLimit(), 10)}
}
u = &url.URL{
Path: req.Path, // path still contains label name
RawQuery: urlValues.Encode(),
Expand Down
59 changes: 54 additions & 5 deletions pkg/frontend/querymiddleware/codec_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ func TestLabelsQueryRequest(t *testing.T) {
expectedGetLabelName string
expectedGetStartOrDefault int64
expectedGetEndOrDefault int64
expectedErr error
expectedErr string
expectedLimit uint64
}{
{
name: "label names with start and end timestamps, no matcher sets",
Expand Down Expand Up @@ -205,7 +206,7 @@ func TestLabelsQueryRequest(t *testing.T) {
expectedGetEndOrDefault: 1708588800 * 1e3,
},
{
name: "label names with start timestamp, no end timestamp, multiple matcher sets",
name: "label names with start and end timestamp, multiple matcher sets",
url: "/api/v1/labels?end=1708588800&match%5B%5D=go_goroutines%7Bcontainer%3D~%22quer.%2A%22%7D&match%5B%5D=go_goroutines%7Bcontainer%21%3D%22query-scheduler%22%7D&start=1708502400",
expectedStruct: &PrometheusLabelNamesQueryRequest{
Path: "/api/v1/labels",
Expand All @@ -221,7 +222,7 @@ func TestLabelsQueryRequest(t *testing.T) {
expectedGetEndOrDefault: 1708588800 * 1e3,
},
{
name: "label values with start timestamp, no end timestamp, multiple matcher sets",
name: "label values with start and end timestamp, multiple matcher sets",
url: "/api/v1/label/job/values?end=1708588800&match%5B%5D=go_goroutines%7Bcontainer%3D~%22quer.%2A%22%7D&match%5B%5D=go_goroutines%7Bcontainer%21%3D%22query-scheduler%22%7D&start=1708502400",
expectedStruct: &PrometheusLabelValuesQueryRequest{
Path: "/api/v1/label/job/values",
Expand All @@ -237,6 +238,53 @@ func TestLabelsQueryRequest(t *testing.T) {
expectedGetStartOrDefault: 1708502400 * 1e3,
expectedGetEndOrDefault: 1708588800 * 1e3,
},
{
name: "label names with start and end timestamp, multiple matcher sets, limit",
url: "/api/v1/labels?end=1708588800&limit=10&match%5B%5D=go_goroutines%7Bcontainer%3D~%22quer.%2A%22%7D&match%5B%5D=go_goroutines%7Bcontainer%21%3D%22query-scheduler%22%7D&start=1708502400",
expectedStruct: &PrometheusLabelNamesQueryRequest{
Path: "/api/v1/labels",
Start: 1708502400 * 1e3,
End: 1708588800 * 1e3,
Limit: 10,
LabelMatcherSets: []string{
"go_goroutines{container=~\"quer.*\"}",
"go_goroutines{container!=\"query-scheduler\"}",
},
},
expectedGetLabelName: "",
expectedLimit: 10,
expectedGetStartOrDefault: 1708502400 * 1e3,
expectedGetEndOrDefault: 1708588800 * 1e3,
},
{
name: "label values with start and end timestamp, multiple matcher sets, limit",
url: "/api/v1/label/job/values?end=1708588800&limit=10&match%5B%5D=go_goroutines%7Bcontainer%3D~%22quer.%2A%22%7D&match%5B%5D=go_goroutines%7Bcontainer%21%3D%22query-scheduler%22%7D&start=1708502400",
expectedStruct: &PrometheusLabelValuesQueryRequest{
Path: "/api/v1/label/job/values",
LabelName: "job",
Start: 1708502400 * 1e3,
End: 1708588800 * 1e3,
Limit: 10,
LabelMatcherSets: []string{
"go_goroutines{container=~\"quer.*\"}",
"go_goroutines{container!=\"query-scheduler\"}",
},
},
expectedGetLabelName: "job",
expectedLimit: 10,
expectedGetStartOrDefault: 1708502400 * 1e3,
expectedGetEndOrDefault: 1708588800 * 1e3,
},
{
name: "zero limit is not allowed",
url: "/api/v1/label/job/values?limit=0",
expectedErr: "limit parameter must be a positive number: 0",
},
{
name: "negative limit is not allowed",
url: "/api/v1/label/job/values?limit=-1",
expectedErr: "limit parameter must be a positive number: -1",
},
} {
t.Run(testCase.name, func(t *testing.T) {
for _, reqMethod := range []string{http.MethodGet, http.MethodPost} {
Expand All @@ -261,13 +309,14 @@ func TestLabelsQueryRequest(t *testing.T) {
r = r.WithContext(ctx)

reqDecoded, err := codec.DecodeLabelsQueryRequest(ctx, r)
if err != nil || testCase.expectedErr != nil {
require.EqualValues(t, testCase.expectedErr, err)
if err != nil || testCase.expectedErr != "" {
require.EqualError(t, err, testCase.expectedErr)
return
}
require.EqualValues(t, testCase.expectedStruct, reqDecoded)
require.EqualValues(t, testCase.expectedGetStartOrDefault, reqDecoded.GetStartOrDefault())
require.EqualValues(t, testCase.expectedGetEndOrDefault, reqDecoded.GetEndOrDefault())
require.EqualValues(t, testCase.expectedLimit, reqDecoded.GetLimit())

reqEncoded, err := codec.EncodeLabelsQueryRequest(context.Background(), reqDecoded)
require.NoError(t, err)
Expand Down
4 changes: 2 additions & 2 deletions pkg/frontend/querymiddleware/model.pb.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions pkg/frontend/querymiddleware/model.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ message PrometheusLabelNamesQueryRequest {
// ID of the request used to correlate downstream requests and responses.
int64 id = 5;

// Limit the number of label names returned.
// Limit the number of label names returned. A value of 0 means no limit.
uint64 limit = 6;
}

Expand All @@ -102,7 +102,7 @@ message PrometheusLabelValuesQueryRequest {
// ID of the request used to correlate downstream requests and responses.
int64 id = 6;

// Limit the number of label values returned.
// Limit the number of label values returned. A value of 0 means no limit.
uint64 limit = 7;
}

Expand Down

0 comments on commit 663121c

Please sign in to comment.