From 110ee5009bbd39c6fd2e7d78c956337b7551cad4 Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Fri, 7 Jun 2019 22:47:36 -0700 Subject: [PATCH 1/2] Pass warnings through on non-error responses Signed-off-by: Thomas Jackson --- api/client.go | 4 ++++ api/prometheus/v1/api.go | 18 ++++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/api/client.go b/api/client.go index 2e6a5518e..fb76d1f27 100644 --- a/api/client.go +++ b/api/client.go @@ -26,6 +26,10 @@ import ( ) func NewErrorAPI(err error, warnings []string) Error { + if apiErr, ok := err.(Error); ok { + return apiErr + } + if err == nil && warnings == nil { return nil } diff --git a/api/prometheus/v1/api.go b/api/prometheus/v1/api.go index 28cdaef69..4f243868d 100644 --- a/api/prometheus/v1/api.go +++ b/api/prometheus/v1/api.go @@ -211,8 +211,11 @@ func (e *Error) Error() string { return "Warnings: " + strings.Join(e.warnings, " , ") } -func (w *Error) Err() error { - return w +func (e *Error) Err() error { + if e.Type != "" || e.Msg != "" { + return errors.New(fmt.Sprintf("%s: %s", e.Type, e.Msg)) + } + return nil } func (w *Error) Warnings() []string { @@ -847,8 +850,6 @@ func (c apiClient) Do(ctx context.Context, req *http.Request) (*http.Response, [ code := resp.StatusCode - var err api.Error - if code/100 != 2 && !apiError(code) { errorType, errorMsg := errorTypeAndMsgFor(resp) return resp, body, &Error{ @@ -869,6 +870,8 @@ func (c apiClient) Do(ctx context.Context, req *http.Request) (*http.Response, [ } } + var err *Error + if apiError(code) != (result.Status == "error") { err = &Error{ Type: ErrBadResponse, @@ -885,6 +888,13 @@ func (c apiClient) Do(ctx context.Context, req *http.Request) (*http.Response, [ } } + if len(result.Warnings) > 0 { + if err == nil { + err = &Error{} + } + err.warnings = result.Warnings + } + return resp, []byte(result.Data), err } From c126de30eeee9e08058a26d65b52832ffbe73818 Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Thu, 13 Jun 2019 09:33:00 -0700 Subject: [PATCH 2/2] Change api client to pass through warnings Signed-off-by: Thomas Jackson --- api/client.go | 60 ++------ api/client_test.go | 4 +- api/prometheus/v1/api.go | 252 +++++++++++++++------------------- api/prometheus/v1/api_test.go | 181 ++++++++++++++++-------- 4 files changed, 241 insertions(+), 256 deletions(-) diff --git a/api/client.go b/api/client.go index fb76d1f27..53b87ae20 100644 --- a/api/client.go +++ b/api/client.go @@ -25,45 +25,7 @@ import ( "time" ) -func NewErrorAPI(err error, warnings []string) Error { - if apiErr, ok := err.(Error); ok { - return apiErr - } - - if err == nil && warnings == nil { - return nil - } - return &ErrorAPI{err, warnings} -} - -type ErrorAPI struct { - err error - warnings []string -} - -func (w *ErrorAPI) Err() error { - return w.err -} - -func (w *ErrorAPI) Error() string { - if w.err != nil { - return w.err.Error() - } - return "Warnings: " + strings.Join(w.warnings, " , ") -} - -func (w *ErrorAPI) Warnings() []string { - return w.warnings -} - -// Error encapsulates an error + warning -type Error interface { - error - // Err returns the underlying error. - Err() error - // Warnings returns a list of warnings. - Warnings() []string -} +type Warnings []string // DefaultRoundTripper is used if no RoundTripper is set in Config. var DefaultRoundTripper http.RoundTripper = &http.Transport{ @@ -95,30 +57,30 @@ func (cfg *Config) roundTripper() http.RoundTripper { // Client is the interface for an API client. type Client interface { URL(ep string, args map[string]string) *url.URL - Do(context.Context, *http.Request) (*http.Response, []byte, Error) + Do(context.Context, *http.Request) (*http.Response, []byte, Warnings, error) } // DoGetFallback will attempt to do the request as-is, and on a 405 it will fallback to a GET request. -func DoGetFallback(c Client, ctx context.Context, u *url.URL, args url.Values) (*http.Response, []byte, Error) { +func DoGetFallback(c Client, ctx context.Context, u *url.URL, args url.Values) (*http.Response, []byte, Warnings, error) { req, err := http.NewRequest(http.MethodPost, u.String(), strings.NewReader(args.Encode())) if err != nil { - return nil, nil, NewErrorAPI(err, nil) + return nil, nil, nil, err } req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - resp, body, err := c.Do(ctx, req) + resp, body, warnings, err := c.Do(ctx, req) if resp != nil && resp.StatusCode == http.StatusMethodNotAllowed { u.RawQuery = args.Encode() req, err = http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return nil, nil, NewErrorAPI(err, nil) + return nil, nil, warnings, err } } else { if err != nil { - return resp, body, NewErrorAPI(err, nil) + return resp, body, warnings, err } - return resp, body, nil + return resp, body, warnings, nil } return c.Do(ctx, req) } @@ -158,7 +120,7 @@ func (c *httpClient) URL(ep string, args map[string]string) *url.URL { return &u } -func (c *httpClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, Error) { +func (c *httpClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, Warnings, error) { if ctx != nil { req = req.WithContext(ctx) } @@ -170,7 +132,7 @@ func (c *httpClient) Do(ctx context.Context, req *http.Request) (*http.Response, }() if err != nil { - return nil, nil, NewErrorAPI(err, nil) + return nil, nil, nil, err } var body []byte @@ -190,5 +152,5 @@ func (c *httpClient) Do(ctx context.Context, req *http.Request) (*http.Response, case <-done: } - return resp, body, NewErrorAPI(err, nil) + return resp, body, nil, err } diff --git a/api/client_test.go b/api/client_test.go index b0ea306d6..b3c95eee6 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -152,7 +152,7 @@ func TestDoGetFallback(t *testing.T) { client := &httpClient{client: *(server.Client())} // Do a post, and ensure that the post succeeds. - _, b, err := DoGetFallback(client, context.TODO(), u, v) + _, b, _, err := DoGetFallback(client, context.TODO(), u, v) if err != nil { t.Fatalf("Error doing local request: %v", err) } @@ -169,7 +169,7 @@ func TestDoGetFallback(t *testing.T) { // Do a fallbcak to a get. u.Path = "/blockPost" - _, b, err = DoGetFallback(client, context.TODO(), u, v) + _, b, _, err = DoGetFallback(client, context.TODO(), u, v) if err != nil { t.Fatalf("Error doing local request: %v", err) } diff --git a/api/prometheus/v1/api.go b/api/prometheus/v1/api.go index 4f243868d..694c7cd02 100644 --- a/api/prometheus/v1/api.go +++ b/api/prometheus/v1/api.go @@ -22,7 +22,6 @@ import ( "math" "net/http" "strconv" - "strings" "time" "unsafe" @@ -197,29 +196,13 @@ const ( // Error is an error returned by the API. type Error struct { - Type ErrorType - Msg string - Detail string - warnings []string + Type ErrorType + Msg string + Detail string } func (e *Error) Error() string { - if e.Type != "" || e.Msg != "" { - return fmt.Sprintf("%s: %s", e.Type, e.Msg) - } - - return "Warnings: " + strings.Join(e.warnings, " , ") -} - -func (e *Error) Err() error { - if e.Type != "" || e.Msg != "" { - return errors.New(fmt.Sprintf("%s: %s", e.Type, e.Msg)) - } - return nil -} - -func (w *Error) Warnings() []string { - return w.warnings + return fmt.Sprintf("%s: %s", e.Type, e.Msg) } // Range represents a sliced time range. @@ -233,34 +216,34 @@ type Range struct { // API provides bindings for Prometheus's v1 API. type API interface { // Alerts returns a list of all active alerts. - Alerts(ctx context.Context) (AlertsResult, api.Error) + Alerts(ctx context.Context) (AlertsResult, error) // AlertManagers returns an overview of the current state of the Prometheus alert manager discovery. - AlertManagers(ctx context.Context) (AlertManagersResult, api.Error) + AlertManagers(ctx context.Context) (AlertManagersResult, error) // CleanTombstones removes the deleted data from disk and cleans up the existing tombstones. - CleanTombstones(ctx context.Context) api.Error + CleanTombstones(ctx context.Context) error // Config returns the current Prometheus configuration. - Config(ctx context.Context) (ConfigResult, api.Error) + Config(ctx context.Context) (ConfigResult, error) // DeleteSeries deletes data for a selection of series in a time range. - DeleteSeries(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) api.Error + DeleteSeries(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) error // Flags returns the flag values that Prometheus was launched with. - Flags(ctx context.Context) (FlagsResult, api.Error) + Flags(ctx context.Context) (FlagsResult, error) // LabelValues performs a query for the values of the given label. - LabelValues(ctx context.Context, label string) (model.LabelValues, api.Error) + LabelValues(ctx context.Context, label string) (model.LabelValues, error) // Query performs a query for the given time. - Query(ctx context.Context, query string, ts time.Time) (model.Value, api.Error) + Query(ctx context.Context, query string, ts time.Time) (model.Value, api.Warnings, error) // QueryRange performs a query for the given range. - QueryRange(ctx context.Context, query string, r Range) (model.Value, api.Error) + QueryRange(ctx context.Context, query string, r Range) (model.Value, api.Warnings, error) // Series finds series by label matchers. - Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, api.Error) + Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, error) // Snapshot creates a snapshot of all current data into snapshots/- // under the TSDB's data directory and returns the directory as response. - Snapshot(ctx context.Context, skipHead bool) (SnapshotResult, api.Error) + Snapshot(ctx context.Context, skipHead bool) (SnapshotResult, error) // Rules returns a list of alerting and recording rules that are currently loaded. - Rules(ctx context.Context) (RulesResult, api.Error) + Rules(ctx context.Context) (RulesResult, error) // Targets returns an overview of the current state of the Prometheus target discovery. - Targets(ctx context.Context) (TargetsResult, api.Error) + Targets(ctx context.Context) (TargetsResult, error) // TargetsMetadata returns metadata about metrics currently scraped by the target. - TargetsMetadata(ctx context.Context, matchTarget string, metric string, limit string) ([]MetricMetadata, api.Error) + TargetsMetadata(ctx context.Context, matchTarget string, metric string, limit string) ([]MetricMetadata, error) } // AlertsResult contains the result from querying the alerts endpoint. @@ -537,73 +520,70 @@ type httpAPI struct { client api.Client } -func (h *httpAPI) Alerts(ctx context.Context) (AlertsResult, api.Error) { +func (h *httpAPI) Alerts(ctx context.Context) (AlertsResult, error) { u := h.client.URL(epAlerts, nil) req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return AlertsResult{}, api.NewErrorAPI(err, nil) + return AlertsResult{}, err } - _, body, apiErr := h.client.Do(ctx, req) - if apiErr != nil { - return AlertsResult{}, apiErr + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return AlertsResult{}, err } var res AlertsResult - err = json.Unmarshal(body, &res) - return res, api.NewErrorAPI(err, nil) + return res, json.Unmarshal(body, &res) } -func (h *httpAPI) AlertManagers(ctx context.Context) (AlertManagersResult, api.Error) { +func (h *httpAPI) AlertManagers(ctx context.Context) (AlertManagersResult, error) { u := h.client.URL(epAlertManagers, nil) req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return AlertManagersResult{}, api.NewErrorAPI(err, nil) + return AlertManagersResult{}, err } - _, body, apiErr := h.client.Do(ctx, req) - if apiErr != nil { - return AlertManagersResult{}, apiErr + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return AlertManagersResult{}, err } var res AlertManagersResult - err = json.Unmarshal(body, &res) - return res, api.NewErrorAPI(err, nil) + return res, json.Unmarshal(body, &res) } -func (h *httpAPI) CleanTombstones(ctx context.Context) api.Error { +func (h *httpAPI) CleanTombstones(ctx context.Context) error { u := h.client.URL(epCleanTombstones, nil) req, err := http.NewRequest(http.MethodPost, u.String(), nil) if err != nil { - return api.NewErrorAPI(err, nil) + return err } - _, _, apiErr := h.client.Do(ctx, req) - return apiErr + _, _, _, err = h.client.Do(ctx, req) + return err } -func (h *httpAPI) Config(ctx context.Context) (ConfigResult, api.Error) { +func (h *httpAPI) Config(ctx context.Context) (ConfigResult, error) { u := h.client.URL(epConfig, nil) req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return ConfigResult{}, api.NewErrorAPI(err, nil) + return ConfigResult{}, err } - _, body, apiErr := h.client.Do(ctx, req) - if apiErr != nil { - return ConfigResult{}, apiErr + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return ConfigResult{}, err } var res ConfigResult - err = json.Unmarshal(body, &res) - return res, api.NewErrorAPI(err, nil) + return res, json.Unmarshal(body, &res) } -func (h *httpAPI) DeleteSeries(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) api.Error { +func (h *httpAPI) DeleteSeries(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) error { u := h.client.URL(epDeleteSeries, nil) q := u.Query() @@ -618,47 +598,45 @@ func (h *httpAPI) DeleteSeries(ctx context.Context, matches []string, startTime req, err := http.NewRequest(http.MethodPost, u.String(), nil) if err != nil { - return api.NewErrorAPI(err, nil) + return err } - _, _, apiErr := h.client.Do(ctx, req) - return apiErr + _, _, _, err = h.client.Do(ctx, req) + return err } -func (h *httpAPI) Flags(ctx context.Context) (FlagsResult, api.Error) { +func (h *httpAPI) Flags(ctx context.Context) (FlagsResult, error) { u := h.client.URL(epFlags, nil) req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return FlagsResult{}, api.NewErrorAPI(err, nil) + return FlagsResult{}, err } - _, body, apiErr := h.client.Do(ctx, req) - if apiErr != nil { - return FlagsResult{}, apiErr + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return FlagsResult{}, err } var res FlagsResult - err = json.Unmarshal(body, &res) - return res, api.NewErrorAPI(err, nil) + return res, json.Unmarshal(body, &res) } -func (h *httpAPI) LabelValues(ctx context.Context, label string) (model.LabelValues, api.Error) { +func (h *httpAPI) LabelValues(ctx context.Context, label string) (model.LabelValues, error) { u := h.client.URL(epLabelValues, map[string]string{"name": label}) req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return nil, api.NewErrorAPI(err, nil) + return nil, err } - _, body, apiErr := h.client.Do(ctx, req) - if apiErr != nil { - return nil, apiErr + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return nil, err } var labelValues model.LabelValues - err = json.Unmarshal(body, &labelValues) - return labelValues, api.NewErrorAPI(err, nil) + return labelValues, json.Unmarshal(body, &labelValues) } -func (h *httpAPI) Query(ctx context.Context, query string, ts time.Time) (model.Value, api.Error) { +func (h *httpAPI) Query(ctx context.Context, query string, ts time.Time) (model.Value, api.Warnings, error) { u := h.client.URL(epQuery, nil) q := u.Query() @@ -667,16 +645,16 @@ func (h *httpAPI) Query(ctx context.Context, query string, ts time.Time) (model. q.Set("time", ts.Format(time.RFC3339Nano)) } - _, body, apiErr := api.DoGetFallback(h.client, ctx, u, q) - if apiErr != nil { - return nil, apiErr + _, body, warnings, err := api.DoGetFallback(h.client, ctx, u, q) + if err != nil { + return nil, warnings, err } var qres queryResult - return model.Value(qres.v), api.NewErrorAPI(json.Unmarshal(body, &qres), nil) + return model.Value(qres.v), warnings, json.Unmarshal(body, &qres) } -func (h *httpAPI) QueryRange(ctx context.Context, query string, r Range) (model.Value, api.Error) { +func (h *httpAPI) QueryRange(ctx context.Context, query string, r Range) (model.Value, api.Warnings, error) { u := h.client.URL(epQueryRange, nil) q := u.Query() @@ -691,17 +669,17 @@ func (h *httpAPI) QueryRange(ctx context.Context, query string, r Range) (model. q.Set("end", end) q.Set("step", step) - _, body, apiErr := api.DoGetFallback(h.client, ctx, u, q) - if apiErr != nil { - return nil, apiErr + _, body, warnings, err := api.DoGetFallback(h.client, ctx, u, q) + if err != nil { + return nil, warnings, err } var qres queryResult - return model.Value(qres.v), api.NewErrorAPI(json.Unmarshal(body, &qres), nil) + return model.Value(qres.v), warnings, json.Unmarshal(body, &qres) } -func (h *httpAPI) Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, api.Error) { +func (h *httpAPI) Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, error) { u := h.client.URL(epSeries, nil) q := u.Query() @@ -716,20 +694,19 @@ func (h *httpAPI) Series(ctx context.Context, matches []string, startTime time.T req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return nil, api.NewErrorAPI(err, nil) + return nil, err } - _, body, apiErr := h.client.Do(ctx, req) - if apiErr != nil { - return nil, apiErr + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return nil, err } var mset []model.LabelSet - err = json.Unmarshal(body, &mset) - return mset, api.NewErrorAPI(err, nil) + return mset, json.Unmarshal(body, &mset) } -func (h *httpAPI) Snapshot(ctx context.Context, skipHead bool) (SnapshotResult, api.Error) { +func (h *httpAPI) Snapshot(ctx context.Context, skipHead bool) (SnapshotResult, error) { u := h.client.URL(epSnapshot, nil) q := u.Query() @@ -739,56 +716,53 @@ func (h *httpAPI) Snapshot(ctx context.Context, skipHead bool) (SnapshotResult, req, err := http.NewRequest(http.MethodPost, u.String(), nil) if err != nil { - return SnapshotResult{}, api.NewErrorAPI(err, nil) + return SnapshotResult{}, err } - _, body, apiErr := h.client.Do(ctx, req) - if apiErr != nil { - return SnapshotResult{}, apiErr + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return SnapshotResult{}, err } var res SnapshotResult - err = json.Unmarshal(body, &res) - return res, api.NewErrorAPI(err, nil) + return res, json.Unmarshal(body, &res) } -func (h *httpAPI) Rules(ctx context.Context) (RulesResult, api.Error) { +func (h *httpAPI) Rules(ctx context.Context) (RulesResult, error) { u := h.client.URL(epRules, nil) req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return RulesResult{}, api.NewErrorAPI(err, nil) + return RulesResult{}, err } - _, body, apiErr := h.client.Do(ctx, req) - if apiErr != nil { - return RulesResult{}, apiErr + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return RulesResult{}, err } var res RulesResult - err = json.Unmarshal(body, &res) - return res, api.NewErrorAPI(err, nil) + return res, json.Unmarshal(body, &res) } -func (h *httpAPI) Targets(ctx context.Context) (TargetsResult, api.Error) { +func (h *httpAPI) Targets(ctx context.Context) (TargetsResult, error) { u := h.client.URL(epTargets, nil) req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return TargetsResult{}, api.NewErrorAPI(err, nil) + return TargetsResult{}, err } - _, body, apiErr := h.client.Do(ctx, req) - if apiErr != nil { - return TargetsResult{}, apiErr + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return TargetsResult{}, err } var res TargetsResult - err = json.Unmarshal(body, &res) - return res, api.NewErrorAPI(err, nil) + return res, json.Unmarshal(body, &res) } -func (h *httpAPI) TargetsMetadata(ctx context.Context, matchTarget string, metric string, limit string) ([]MetricMetadata, api.Error) { +func (h *httpAPI) TargetsMetadata(ctx context.Context, matchTarget string, metric string, limit string) ([]MetricMetadata, error) { u := h.client.URL(epTargetsMetadata, nil) q := u.Query() @@ -800,17 +774,16 @@ func (h *httpAPI) TargetsMetadata(ctx context.Context, matchTarget string, metri req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { - return nil, api.NewErrorAPI(err, nil) + return nil, err } - _, body, apiErr := h.client.Do(ctx, req) - if apiErr != nil { - return nil, apiErr + _, body, _, err := h.client.Do(ctx, req) + if err != nil { + return nil, err } var res []MetricMetadata - err = json.Unmarshal(body, &res) - return res, api.NewErrorAPI(err, nil) + return res, json.Unmarshal(body, &res) } // apiClient wraps a regular client and processes successful API responses. @@ -842,17 +815,17 @@ func errorTypeAndMsgFor(resp *http.Response) (ErrorType, string) { return ErrBadResponse, fmt.Sprintf("bad response code %d", resp.StatusCode) } -func (c apiClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, api.Error) { - resp, body, apiErr := c.Client.Do(ctx, req) - if apiErr != nil { - return resp, body, apiErr +func (c apiClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, api.Warnings, error) { + resp, body, warnings, err := c.Client.Do(ctx, req) + if err != nil { + return resp, body, warnings, err } code := resp.StatusCode if code/100 != 2 && !apiError(code) { errorType, errorMsg := errorTypeAndMsgFor(resp) - return resp, body, &Error{ + return resp, body, warnings, &Error{ Type: errorType, Msg: errorMsg, Detail: string(body), @@ -863,38 +836,27 @@ func (c apiClient) Do(ctx context.Context, req *http.Request) (*http.Response, [ if http.StatusNoContent != code { if jsonErr := json.Unmarshal(body, &result); jsonErr != nil { - return resp, body, &Error{ + return resp, body, warnings, &Error{ Type: ErrBadResponse, Msg: jsonErr.Error(), } } } - var err *Error - if apiError(code) != (result.Status == "error") { err = &Error{ - Type: ErrBadResponse, - Msg: "inconsistent body for response code", - warnings: result.Warnings, + Type: ErrBadResponse, + Msg: "inconsistent body for response code", } } if apiError(code) && result.Status == "error" { err = &Error{ - Type: result.ErrorType, - Msg: result.Error, - warnings: result.Warnings, - } - } - - if len(result.Warnings) > 0 { - if err == nil { - err = &Error{} + Type: result.ErrorType, + Msg: result.Error, } - err.warnings = result.Warnings } - return resp, []byte(result.Data), err + return resp, []byte(result.Data), warnings, err } diff --git a/api/prometheus/v1/api_test.go b/api/prometheus/v1/api_test.go index 0b8fe2b11..49c90d697 100644 --- a/api/prometheus/v1/api_test.go +++ b/api/prometheus/v1/api_test.go @@ -33,7 +33,7 @@ import ( ) type apiTest struct { - do func() (interface{}, api.Error) + do func() (interface{}, api.Warnings, error) inWarnings []string inErr error inStatusCode int @@ -43,6 +43,7 @@ type apiTest struct { reqParam url.Values reqMethod string res interface{} + warnings api.Warnings err error } @@ -63,7 +64,7 @@ func (c *apiTestClient) URL(ep string, args map[string]string) *url.URL { return u } -func (c *apiTestClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, api.Error) { +func (c *apiTestClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, api.Warnings, error) { test := c.curTest @@ -88,7 +89,7 @@ func (c *apiTestClient) Do(ctx context.Context, req *http.Request) (*http.Respon resp.StatusCode = http.StatusOK } - return resp, b, api.NewErrorAPI(test.inErr, test.inWarnings) + return resp, b, test.inWarnings, test.inErr } func TestAPIs(t *testing.T) { @@ -101,81 +102,90 @@ func TestAPIs(t *testing.T) { client: client, } - doAlertManagers := func() func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return promAPI.AlertManagers(context.Background()) + doAlertManagers := func() func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + v, err := promAPI.AlertManagers(context.Background()) + return v, nil, err } } - doCleanTombstones := func() func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return nil, promAPI.CleanTombstones(context.Background()) + doCleanTombstones := func() func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + return nil, nil, promAPI.CleanTombstones(context.Background()) } } - doConfig := func() func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return promAPI.Config(context.Background()) + doConfig := func() func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + v, err := promAPI.Config(context.Background()) + return v, nil, err } } - doDeleteSeries := func(matcher string, startTime time.Time, endTime time.Time) func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return nil, promAPI.DeleteSeries(context.Background(), []string{matcher}, startTime, endTime) + doDeleteSeries := func(matcher string, startTime time.Time, endTime time.Time) func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + return nil, nil, promAPI.DeleteSeries(context.Background(), []string{matcher}, startTime, endTime) } } - doFlags := func() func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return promAPI.Flags(context.Background()) + doFlags := func() func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + v, err := promAPI.Flags(context.Background()) + return v, nil, err } } - doLabelValues := func(label string) func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return promAPI.LabelValues(context.Background(), label) + doLabelValues := func(label string) func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + v, err := promAPI.LabelValues(context.Background(), label) + return v, nil, err } } - doQuery := func(q string, ts time.Time) func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { + doQuery := func(q string, ts time.Time) func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { return promAPI.Query(context.Background(), q, ts) } } - doQueryRange := func(q string, rng Range) func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { + doQueryRange := func(q string, rng Range) func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { return promAPI.QueryRange(context.Background(), q, rng) } } - doSeries := func(matcher string, startTime time.Time, endTime time.Time) func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return promAPI.Series(context.Background(), []string{matcher}, startTime, endTime) + doSeries := func(matcher string, startTime time.Time, endTime time.Time) func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + v, err := promAPI.Series(context.Background(), []string{matcher}, startTime, endTime) + return v, nil, err } } - doSnapshot := func(skipHead bool) func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return promAPI.Snapshot(context.Background(), skipHead) + doSnapshot := func(skipHead bool) func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + v, err := promAPI.Snapshot(context.Background(), skipHead) + return v, nil, err } } - doRules := func() func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return promAPI.Rules(context.Background()) + doRules := func() func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + v, err := promAPI.Rules(context.Background()) + return v, nil, err } } - doTargets := func() func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return promAPI.Targets(context.Background()) + doTargets := func() func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + v, err := promAPI.Targets(context.Background()) + return v, nil, err } } - doTargetsMetadata := func(matchTarget string, metric string, limit string) func() (interface{}, api.Error) { - return func() (interface{}, api.Error) { - return promAPI.TargetsMetadata(context.Background(), matchTarget, metric, limit) + doTargetsMetadata := func(matchTarget string, metric string, limit string) func() (interface{}, api.Warnings, error) { + return func() (interface{}, api.Warnings, error) { + v, err := promAPI.TargetsMetadata(context.Background(), matchTarget, metric, limit) + return v, nil, err } } @@ -249,6 +259,51 @@ func TestAPIs(t *testing.T) { }, err: errors.New("client_error: client error: 404"), }, + // Warning only. + { + do: doQuery("2", testTime), + inWarnings: []string{"warning"}, + inRes: &queryResult{ + Type: model.ValScalar, + Result: &model.Scalar{ + Value: 2, + Timestamp: model.TimeFromUnix(testTime.Unix()), + }, + }, + + reqMethod: "POST", + reqPath: "/api/v1/query", + reqParam: url.Values{ + "query": []string{"2"}, + "time": []string{testTime.Format(time.RFC3339Nano)}, + }, + res: &model.Scalar{ + Value: 2, + Timestamp: model.TimeFromUnix(testTime.Unix()), + }, + warnings: []string{"warning"}, + }, + // Warning + error. + { + do: doQuery("2", testTime), + inWarnings: []string{"warning"}, + inRes: "some body", + inStatusCode: 404, + inErr: &Error{ + Type: ErrClient, + Msg: "client error: 404", + Detail: "some body", + }, + + reqMethod: "POST", + reqPath: "/api/v1/query", + reqParam: url.Values{ + "query": []string{"2"}, + "time": []string{testTime.Format(time.RFC3339Nano)}, + }, + err: errors.New("client_error: client error: 404"), + warnings: []string{"warning"}, + }, { do: doQueryRange("2", Range{ @@ -705,7 +760,11 @@ func TestAPIs(t *testing.T) { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { client.curTest = test - res, err := test.do() + res, warnings, err := test.do() + + if (test.inWarnings == nil) != (warnings == nil) && !reflect.DeepEqual(test.inWarnings, warnings) { + t.Fatalf("mismatch in warnings expected=%v actual=%v", test.inWarnings, warnings) + } if test.err != nil { if err == nil { @@ -740,17 +799,18 @@ type testClient struct { } type apiClientTest struct { - code int - response interface{} - expectedBody string - expectedErr *Error + code int + response interface{} + expectedBody string + expectedErr *Error + expectedWarnings api.Warnings } func (c *testClient) URL(ep string, args map[string]string) *url.URL { return nil } -func (c *testClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, api.Error) { +func (c *testClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, api.Warnings, error) { if ctx == nil { c.Fatalf("context was not passed down") } @@ -777,7 +837,7 @@ func (c *testClient) Do(ctx context.Context, req *http.Request) (*http.Response, StatusCode: test.code, } - return resp, b, nil + return resp, b, test.expectedWarnings, nil } func TestAPIClientDo(t *testing.T) { @@ -896,10 +956,10 @@ func TestAPIClientDo(t *testing.T) { Warnings: []string{"a"}, }, expectedErr: &Error{ - Type: ErrBadResponse, - Msg: "inconsistent body for response code", - warnings: []string{"a"}, + Type: ErrBadResponse, + Msg: "inconsistent body for response code", }, + expectedWarnings: []string{"a"}, }, } @@ -915,7 +975,17 @@ func TestAPIClientDo(t *testing.T) { tc.ch <- test - _, body, err := client.Do(context.Background(), tc.req) + _, body, warnings, err := client.Do(context.Background(), tc.req) + + if test.expectedWarnings != nil { + if !reflect.DeepEqual(test.expectedWarnings, warnings) { + t.Fatalf("mismatch in warnings expected=%v actual=%v", test.expectedWarnings, warnings) + } + } else { + if warnings != nil { + t.Fatalf("unexpexted warnings: %v", warnings) + } + } if test.expectedErr != nil { if err == nil { @@ -933,15 +1003,6 @@ func TestAPIClientDo(t *testing.T) { } } - if len(test.expectedErr.Warnings()) != len(err.Warnings()) { - t.Fatalf("expected warnings length :%v, but got:%v", len(test.expectedErr.Warnings()), len(err.Warnings())) - } - - for x, warning := range test.expectedErr.Warnings() { - if warning != err.Warnings()[x] { - t.Fatalf("expected warning :%v, but got:%v", warning, err.Warnings()[x]) - } - } return }