From 5ac12ac3517be1f1150d68e05df030edec4e85d4 Mon Sep 17 00:00:00 2001 From: Alan Protasio Date: Thu, 20 Oct 2022 02:17:00 -0700 Subject: [PATCH] api: Wrapped promQL based API errors with `returnAPIError` function (#11356) * wrap api error on get series/labels on `returnAPIError` function Signed-off-by: Alan Protasio * lint Signed-off-by: Alan Protasio * query exemplars Signed-off-by: Alan Protasio Signed-off-by: Alan Protasio --- web/api/v1/api.go | 12 +- web/api/v1/api_test.go | 283 ++++++++++++++++++++++++++++++++++++-- web/api/v1/errors_test.go | 5 + 3 files changed, 285 insertions(+), 15 deletions(-) diff --git a/web/api/v1/api.go b/web/api/v1/api.go index bd040912ef..1927ebb82b 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -550,12 +550,12 @@ func (api *API) queryExemplars(r *http.Request) apiFuncResult { ctx := r.Context() eq, err := api.ExemplarQueryable.ExemplarQuerier(ctx) if err != nil { - return apiFuncResult{nil, &apiError{errorExec, err}, nil, nil} + return apiFuncResult{nil, returnAPIError(err), nil, nil} } res, err := eq.Select(timestamp.FromTime(start), timestamp.FromTime(end), selectors...) if err != nil { - return apiFuncResult{nil, &apiError{errorExec, err}, nil, nil} + return apiFuncResult{nil, returnAPIError(err), nil, nil} } return apiFuncResult{res, nil, nil, nil} @@ -600,7 +600,7 @@ func (api *API) labelNames(r *http.Request) apiFuncResult { q, err := api.Queryable.Querier(r.Context(), timestamp.FromTime(start), timestamp.FromTime(end)) if err != nil { - return apiFuncResult{nil, &apiError{errorExec, err}, nil, nil} + return apiFuncResult{nil, returnAPIError(err), nil, nil} } defer q.Close() @@ -614,7 +614,7 @@ func (api *API) labelNames(r *http.Request) apiFuncResult { for _, matchers := range matcherSets { vals, callWarnings, err := q.LabelNames(matchers...) if err != nil { - return apiFuncResult{nil, &apiError{errorExec, err}, warnings, nil} + return apiFuncResult{nil, returnAPIError(err), warnings, nil} } warnings = append(warnings, callWarnings...) @@ -750,7 +750,7 @@ func (api *API) series(r *http.Request) (result apiFuncResult) { q, err := api.Queryable.Querier(r.Context(), timestamp.FromTime(start), timestamp.FromTime(end)) if err != nil { - return apiFuncResult{nil, &apiError{errorExec, err}, nil, nil} + return apiFuncResult{nil, returnAPIError(err), nil, nil} } // From now on, we must only return with a finalizer in the result (to // be called by the caller) or call q.Close ourselves (which is required @@ -791,7 +791,7 @@ func (api *API) series(r *http.Request) (result apiFuncResult) { warnings := set.Warnings() if set.Err() != nil { - return apiFuncResult{nil, &apiError{errorExec, set.Err()}, warnings, closer} + return apiFuncResult{nil, returnAPIError(set.Err()), warnings, closer} } return apiFuncResult{metrics, nil, warnings, closer} diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index d672807d3f..0fc0b39879 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -30,6 +30,7 @@ import ( "testing" "time" + "github.com/prometheus/prometheus/prompb" "github.com/prometheus/prometheus/util/stats" "github.com/go-kit/log" @@ -46,7 +47,6 @@ import ( "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/model/textparse" "github.com/prometheus/prometheus/model/timestamp" - "github.com/prometheus/prometheus/prompb" "github.com/prometheus/prometheus/promql" "github.com/prometheus/prometheus/promql/parser" "github.com/prometheus/prometheus/rules" @@ -452,7 +452,13 @@ func TestEndpoints(t *testing.T) { }) } -func TestLabelNames(t *testing.T) { +type byLabels []labels.Labels + +func (b byLabels) Len() int { return len(b) } +func (b byLabels) Swap(i, j int) { b[i], b[j] = b[j], b[i] } +func (b byLabels) Less(i, j int) bool { return labels.Compare(b[i], b[j]) < 0 } + +func TestGetSeries(t *testing.T) { // TestEndpoints doesn't have enough label names to test api.labelNames // endpoint properly. Hence we test it separately. suite, err := promql.NewTest(t, ` @@ -466,7 +472,6 @@ func TestLabelNames(t *testing.T) { require.NoError(t, err) defer suite.Close() require.NoError(t, suite.Run()) - api := &API{ Queryable: suite.Storage(), } @@ -487,28 +492,286 @@ func TestLabelNames(t *testing.T) { } for _, tc := range []struct { - name string - matchers []string - expected []string + name string + api *API + matchers []string + expected []labels.Labels + expectedErrorType errorType + }{ + { + name: "no matchers", + expectedErrorType: errorBadData, + api: api, + }, + { + name: "non empty label matcher", + matchers: []string{`{foo=~".+"}`}, + expected: []labels.Labels{ + {labels.Label{Name: "__name__", Value: "test_metric2"}, labels.Label{Name: "abc", Value: "qwerty"}, labels.Label{Name: "foo", Value: "baz"}}, + {labels.Label{Name: "__name__", Value: "test_metric2"}, labels.Label{Name: "foo", Value: "boo"}}, + {labels.Label{Name: "__name__", Value: "test_metric2"}, labels.Label{Name: "foo", Value: "boo"}, labels.Label{Name: "xyz", Value: "qwerty"}}, + }, + api: api, + }, + { + name: "exact label matcher", + matchers: []string{`{foo="boo"}`}, + expected: []labels.Labels{ + {labels.Label{Name: "__name__", Value: "test_metric2"}, labels.Label{Name: "foo", Value: "boo"}}, + {labels.Label{Name: "__name__", Value: "test_metric2"}, labels.Label{Name: "foo", Value: "boo"}, labels.Label{Name: "xyz", Value: "qwerty"}}, + }, + api: api, + }, + { + name: "two matchers", + matchers: []string{`{foo="boo"}`, `{foo="baz"}`}, + expected: []labels.Labels{ + {labels.Label{Name: "__name__", Value: "test_metric2"}, labels.Label{Name: "abc", Value: "qwerty"}, labels.Label{Name: "foo", Value: "baz"}}, + {labels.Label{Name: "__name__", Value: "test_metric2"}, labels.Label{Name: "foo", Value: "boo"}}, + {labels.Label{Name: "__name__", Value: "test_metric2"}, labels.Label{Name: "foo", Value: "boo"}, labels.Label{Name: "xyz", Value: "qwerty"}}, + }, + api: api, + }, + { + name: "exec error type", + matchers: []string{`{foo="boo"}`, `{foo="baz"}`}, + expectedErrorType: errorExec, + api: &API{ + Queryable: errorTestQueryable{err: fmt.Errorf("generic")}, + }, + }, + { + name: "storage error type", + matchers: []string{`{foo="boo"}`, `{foo="baz"}`}, + expectedErrorType: errorInternal, + api: &API{ + Queryable: errorTestQueryable{err: promql.ErrStorage{Err: fmt.Errorf("generic")}}, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + req, err := request(http.MethodGet, tc.matchers...) + require.NoError(t, err) + res := tc.api.series(req.WithContext(ctx)) + assertAPIError(t, res.err, tc.expectedErrorType) + if tc.expectedErrorType == errorNone { + r := res.data.([]labels.Labels) + for _, l := range tc.expected { + sort.Sort(l) + } + for _, l := range r { + sort.Sort(l) + } + sort.Sort(byLabels(tc.expected)) + sort.Sort(byLabels(r)) + require.Equal(t, tc.expected, r) + } + }) + } +} + +func TestQueryExemplars(t *testing.T) { + start := time.Unix(0, 0) + suite, err := promql.NewTest(t, ` + load 1m + test_metric1{foo="bar"} 0+100x100 + test_metric1{foo="boo"} 1+0x100 + test_metric2{foo="boo"} 1+0x100 + test_metric3{foo="bar", dup="1"} 1+0x100 + test_metric3{foo="boo", dup="1"} 1+0x100 + test_metric4{foo="bar", dup="1"} 1+0x100 + test_metric4{foo="boo", dup="1"} 1+0x100 + test_metric4{foo="boo"} 1+0x100 + `) + + require.NoError(t, err) + defer suite.Close() + require.NoError(t, suite.Run()) + + api := &API{ + Queryable: suite.Storage(), + QueryEngine: suite.QueryEngine(), + ExemplarQueryable: suite.ExemplarQueryable(), + } + + request := func(method string, qs url.Values) (*http.Request, error) { + u, err := url.Parse("http://example.com") + require.NoError(t, err) + u.RawQuery = qs.Encode() + r, err := http.NewRequest(method, u.String(), nil) + if method == http.MethodPost { + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") + } + return r, err + } + + for _, tc := range []struct { + name string + query url.Values + exemplars []exemplar.QueryResult + api *API + expectedErrorType errorType + }{ + { + name: "no error", + api: api, + query: url.Values{ + "query": []string{`test_metric3{foo="boo"} - test_metric4{foo="bar"}`}, + "start": []string{"0"}, + "end": []string{"4"}, + }, + exemplars: []exemplar.QueryResult{ + { + SeriesLabels: labels.FromStrings("__name__", "test_metric3", "foo", "boo", "dup", "1"), + Exemplars: []exemplar.Exemplar{ + { + Labels: labels.FromStrings("id", "abc"), + Value: 10, + Ts: timestamp.FromTime(start.Add(0 * time.Second)), + }, + }, + }, + { + SeriesLabels: labels.FromStrings("__name__", "test_metric4", "foo", "bar", "dup", "1"), + Exemplars: []exemplar.Exemplar{ + { + Labels: labels.FromStrings("id", "lul"), + Value: 10, + Ts: timestamp.FromTime(start.Add(3 * time.Second)), + }, + }, + }, + }, + }, + { + name: "should return errorExec upon genetic error", + expectedErrorType: errorExec, + api: &API{ + ExemplarQueryable: errorTestQueryable{err: fmt.Errorf("generic")}, + }, + query: url.Values{ + "query": []string{`test_metric3{foo="boo"} - test_metric4{foo="bar"}`}, + "start": []string{"0"}, + "end": []string{"4"}, + }, + }, + { + name: "should return errorInternal err type is ErrStorage", + expectedErrorType: errorInternal, + api: &API{ + ExemplarQueryable: errorTestQueryable{err: promql.ErrStorage{Err: fmt.Errorf("generic")}}, + }, + query: url.Values{ + "query": []string{`test_metric3{foo="boo"} - test_metric4{foo="bar"}`}, + "start": []string{"0"}, + "end": []string{"4"}, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + es := suite.ExemplarStorage() + ctx := context.Background() + + for _, te := range tc.exemplars { + for _, e := range te.Exemplars { + _, err := es.AppendExemplar(0, te.SeriesLabels, e) + if err != nil { + t.Fatal(err) + } + } + } + + req, err := request(http.MethodGet, tc.query) + require.NoError(t, err) + res := tc.api.queryExemplars(req.WithContext(ctx)) + assertAPIError(t, res.err, tc.expectedErrorType) + + if tc.expectedErrorType == errorNone { + assertAPIResponse(t, res.data, tc.exemplars) + } + }) + } +} + +func TestLabelNames(t *testing.T) { + // TestEndpoints doesn't have enough label names to test api.labelNames + // endpoint properly. Hence we test it separately. + suite, err := promql.NewTest(t, ` + load 1m + test_metric1{foo1="bar", baz="abc"} 0+100x100 + test_metric1{foo2="boo"} 1+0x100 + test_metric2{foo="boo"} 1+0x100 + test_metric2{foo="boo", xyz="qwerty"} 1+0x100 + test_metric2{foo="baz", abc="qwerty"} 1+0x100 + `) + require.NoError(t, err) + defer suite.Close() + require.NoError(t, suite.Run()) + api := &API{ + Queryable: suite.Storage(), + } + request := func(method 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) + } + u.RawQuery = q.Encode() + + r, err := http.NewRequest(method, u.String(), nil) + if method == http.MethodPost { + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") + } + return r, err + } + + for _, tc := range []struct { + name string + api *API + matchers []string + expected []string + expectedErrorType errorType }{ { name: "no matchers", expected: []string{"__name__", "abc", "baz", "foo", "foo1", "foo2", "xyz"}, + api: api, }, { name: "non empty label matcher", matchers: []string{`{foo=~".+"}`}, expected: []string{"__name__", "abc", "foo", "xyz"}, + api: api, }, { name: "exact label matcher", matchers: []string{`{foo="boo"}`}, expected: []string{"__name__", "foo", "xyz"}, + api: api, }, { name: "two matchers", matchers: []string{`{foo="boo"}`, `{foo="baz"}`}, expected: []string{"__name__", "abc", "foo", "xyz"}, + api: api, + }, + { + name: "exec error type", + matchers: []string{`{foo="boo"}`, `{foo="baz"}`}, + expectedErrorType: errorExec, + api: &API{ + Queryable: errorTestQueryable{err: fmt.Errorf("generic")}, + }, + }, + { + name: "storage error type", + matchers: []string{`{foo="boo"}`, `{foo="baz"}`}, + expectedErrorType: errorInternal, + api: &API{ + Queryable: errorTestQueryable{err: promql.ErrStorage{Err: fmt.Errorf("generic")}}, + }, }, } { t.Run(tc.name, func(t *testing.T) { @@ -516,9 +779,11 @@ func TestLabelNames(t *testing.T) { ctx := context.Background() req, err := request(method, tc.matchers...) require.NoError(t, err) - res := api.labelNames(req.WithContext(ctx)) - assertAPIError(t, res.err, "") - assertAPIResponse(t, res.data, tc.expected) + res := tc.api.labelNames(req.WithContext(ctx)) + assertAPIError(t, res.err, tc.expectedErrorType) + if tc.expectedErrorType == errorNone { + assertAPIResponse(t, res.data, tc.expected) + } } }) } diff --git a/web/api/v1/errors_test.go b/web/api/v1/errors_test.go index 3c40c803af..90d5f18de3 100644 --- a/web/api/v1/errors_test.go +++ b/web/api/v1/errors_test.go @@ -141,10 +141,15 @@ func createPrometheusAPI(q storage.SampleAndChunkQueryable) *route.Router { } type errorTestQueryable struct { + storage.ExemplarQueryable q storage.Querier err error } +func (t errorTestQueryable) ExemplarQuerier(ctx context.Context) (storage.ExemplarQuerier, error) { + return nil, t.err +} + func (t errorTestQueryable) ChunkQuerier(ctx context.Context, mint, maxt int64) (storage.ChunkQuerier, error) { return nil, t.err }