From 7f4279992ed68ae6a4a00cf4f28ea75b22ec5566 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 10 Dec 2019 15:16:44 -0500 Subject: [PATCH 1/4] First pass unravel Signed-off-by: Joe Elliott --- api/client.go | 33 +++---------------- api/prometheus/v1/api.go | 69 ++++++++++++++++++++++++++-------------- 2 files changed, 49 insertions(+), 53 deletions(-) diff --git a/api/client.go b/api/client.go index 53b87ae20..6a2fe2c25 100644 --- a/api/client.go +++ b/api/client.go @@ -57,32 +57,7 @@ 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, 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, Warnings, error) { - req, err := http.NewRequest(http.MethodPost, u.String(), strings.NewReader(args.Encode())) - if err != nil { - return nil, nil, nil, err - } - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - - 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, warnings, err - } - - } else { - if err != nil { - return resp, body, warnings, err - } - return resp, body, warnings, nil - } - return c.Do(ctx, req) + Do(context.Context, *http.Request) (*http.Response, []byte, error) } // NewClient returns a new Client. @@ -120,7 +95,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, Warnings, error) { +func (c *httpClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, error) { if ctx != nil { req = req.WithContext(ctx) } @@ -132,7 +107,7 @@ func (c *httpClient) Do(ctx context.Context, req *http.Request) (*http.Response, }() if err != nil { - return nil, nil, nil, err + return nil, nil, err } var body []byte @@ -152,5 +127,5 @@ func (c *httpClient) Do(ctx context.Context, req *http.Request) (*http.Response, case <-done: } - return resp, body, nil, err + return resp, body, err } diff --git a/api/prometheus/v1/api.go b/api/prometheus/v1/api.go index 00cd76ea2..154e4a81f 100644 --- a/api/prometheus/v1/api.go +++ b/api/prometheus/v1/api.go @@ -21,7 +21,9 @@ import ( "fmt" "math" "net/http" + "net/url" "strconv" + "strings" "time" "unsafe" @@ -515,7 +517,7 @@ func (qr *queryResult) UnmarshalJSON(b []byte) error { // // It is safe to use the returned API from multiple goroutines. func NewAPI(c api.Client) API { - return &httpAPI{client: apiClient{c}} + return &httpAPI{client: c} } type httpAPI struct { @@ -530,7 +532,7 @@ func (h *httpAPI) Alerts(ctx context.Context) (AlertsResult, error) { return AlertsResult{}, err } - _, body, _, err := h.client.Do(ctx, req) + _, body, _, err := h.Do(ctx, req) if err != nil { return AlertsResult{}, err } @@ -547,7 +549,7 @@ func (h *httpAPI) AlertManagers(ctx context.Context) (AlertManagersResult, error return AlertManagersResult{}, err } - _, body, _, err := h.client.Do(ctx, req) + _, body, _, err := h.Do(ctx, req) if err != nil { return AlertManagersResult{}, err } @@ -564,7 +566,7 @@ func (h *httpAPI) CleanTombstones(ctx context.Context) error { return err } - _, _, _, err = h.client.Do(ctx, req) + _, _, _, err = h.Do(ctx, req) return err } @@ -576,7 +578,7 @@ func (h *httpAPI) Config(ctx context.Context) (ConfigResult, error) { return ConfigResult{}, err } - _, body, _, err := h.client.Do(ctx, req) + _, body, _, err := h.Do(ctx, req) if err != nil { return ConfigResult{}, err } @@ -603,7 +605,7 @@ func (h *httpAPI) DeleteSeries(ctx context.Context, matches []string, startTime return err } - _, _, _, err = h.client.Do(ctx, req) + _, _, _, err = h.Do(ctx, req) return err } @@ -615,7 +617,7 @@ func (h *httpAPI) Flags(ctx context.Context) (FlagsResult, error) { return FlagsResult{}, err } - _, body, _, err := h.client.Do(ctx, req) + _, body, _, err := h.Do(ctx, req) if err != nil { return FlagsResult{}, err } @@ -630,7 +632,7 @@ func (h *httpAPI) LabelNames(ctx context.Context) ([]string, api.Warnings, error if err != nil { return nil, nil, err } - _, body, w, err := h.client.Do(ctx, req) + _, body, w, err := h.Do(ctx, req) if err != nil { return nil, w, err } @@ -644,7 +646,7 @@ func (h *httpAPI) LabelValues(ctx context.Context, label string) (model.LabelVal if err != nil { return nil, nil, err } - _, body, w, err := h.client.Do(ctx, req) + _, body, w, err := h.Do(ctx, req) if err != nil { return nil, w, err } @@ -661,7 +663,7 @@ func (h *httpAPI) Query(ctx context.Context, query string, ts time.Time) (model. q.Set("time", formatTime(ts)) } - _, body, warnings, err := api.DoGetFallback(h.client, ctx, u, q) + _, body, warnings, err := h.DoGetFallback(ctx, u, q) if err != nil { return nil, warnings, err } @@ -679,7 +681,7 @@ func (h *httpAPI) QueryRange(ctx context.Context, query string, r Range) (model. q.Set("end", formatTime(r.End)) q.Set("step", strconv.FormatFloat(r.Step.Seconds(), 'f', -1, 64)) - _, body, warnings, err := api.DoGetFallback(h.client, ctx, u, q) + _, body, warnings, err := h.DoGetFallback(ctx, u, q) if err != nil { return nil, warnings, err } @@ -707,7 +709,7 @@ func (h *httpAPI) Series(ctx context.Context, matches []string, startTime time.T return nil, nil, err } - _, body, warnings, err := h.client.Do(ctx, req) + _, body, warnings, err := h.Do(ctx, req) if err != nil { return nil, warnings, err } @@ -729,7 +731,7 @@ func (h *httpAPI) Snapshot(ctx context.Context, skipHead bool) (SnapshotResult, return SnapshotResult{}, err } - _, body, _, err := h.client.Do(ctx, req) + _, body, _, err := h.Do(ctx, req) if err != nil { return SnapshotResult{}, err } @@ -746,7 +748,7 @@ func (h *httpAPI) Rules(ctx context.Context) (RulesResult, error) { return RulesResult{}, err } - _, body, _, err := h.client.Do(ctx, req) + _, body, _, err := h.Do(ctx, req) if err != nil { return RulesResult{}, err } @@ -763,7 +765,7 @@ func (h *httpAPI) Targets(ctx context.Context) (TargetsResult, error) { return TargetsResult{}, err } - _, body, _, err := h.client.Do(ctx, req) + _, body, _, err := h.Do(ctx, req) if err != nil { return TargetsResult{}, err } @@ -787,7 +789,7 @@ func (h *httpAPI) TargetsMetadata(ctx context.Context, matchTarget string, metri return nil, err } - _, body, _, err := h.client.Do(ctx, req) + _, body, _, err := h.Do(ctx, req) if err != nil { return nil, err } @@ -796,12 +798,6 @@ func (h *httpAPI) TargetsMetadata(ctx context.Context, matchTarget string, metri return res, json.Unmarshal(body, &res) } -// apiClient wraps a regular client and processes successful API responses. -// Successful also includes responses that errored at the API level. -type apiClient struct { - api.Client -} - type apiResponse struct { Status string `json:"status"` Data json.RawMessage `json:"data"` @@ -825,8 +821,8 @@ 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.Warnings, error) { - resp, body, warnings, err := c.Client.Do(ctx, req) +func (h *httpAPI) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, api.Warnings, error) { + resp, body, warnings, err := h.Do(ctx, req) if err != nil { return resp, body, warnings, err } @@ -871,6 +867,31 @@ func (c apiClient) Do(ctx context.Context, req *http.Request) (*http.Response, [ } +// DoGetFallback will attempt to do the request as-is, and on a 405 it will fallback to a GET request. +func (h *httpAPI) DoGetFallback(ctx context.Context, u *url.URL, args url.Values) (*http.Response, []byte, api.Warnings, error) { + req, err := http.NewRequest(http.MethodPost, u.String(), strings.NewReader(args.Encode())) + if err != nil { + return nil, nil, nil, err + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + resp, body, warnings, err := h.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, warnings, err + } + + } else { + if err != nil { + return resp, body, warnings, err + } + return resp, body, warnings, nil + } + return h.Do(ctx, req) +} + func formatTime(t time.Time) string { return strconv.FormatFloat(float64(t.Unix())+float64(t.Nanosecond())/1e9, 'f', -1, 64) } From 393adc926170ffaf9eded50fd90a61b0b2b169f6 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Tue, 10 Dec 2019 16:53:38 -0500 Subject: [PATCH 2/4] Refactor ~worked. All tests passing except one Signed-off-by: Joe Elliott --- api/client_test.go | 72 ------------------ api/prometheus/v1/api.go | 68 +++++++++++------ api/prometheus/v1/api_test.go | 133 ++++++++++++++++++++++++++++++++-- 3 files changed, 170 insertions(+), 103 deletions(-) diff --git a/api/client_test.go b/api/client_test.go index b3c95eee6..47094fccd 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -14,10 +14,7 @@ package api import ( - "context" - "encoding/json" "net/http" - "net/http/httptest" "net/url" "testing" ) @@ -114,72 +111,3 @@ func TestClientURL(t *testing.T) { } } } - -func TestDoGetFallback(t *testing.T) { - v := url.Values{"a": []string{"1", "2"}} - - type testResponse struct { - Values string - Method string - } - - // Start a local HTTP server. - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - req.ParseForm() - r := &testResponse{ - Values: req.Form.Encode(), - Method: req.Method, - } - - body, _ := json.Marshal(r) - - if req.Method == http.MethodPost { - if req.URL.Path == "/blockPost" { - http.Error(w, string(body), http.StatusMethodNotAllowed) - return - } - } - - w.Write(body) - })) - // Close the server when test finishes. - defer server.Close() - - u, err := url.Parse(server.URL) - if err != nil { - t.Fatal(err) - } - client := &httpClient{client: *(server.Client())} - - // Do a post, and ensure that the post succeeds. - _, b, _, err := DoGetFallback(client, context.TODO(), u, v) - if err != nil { - t.Fatalf("Error doing local request: %v", err) - } - resp := &testResponse{} - if err := json.Unmarshal(b, resp); err != nil { - t.Fatal(err) - } - if resp.Method != http.MethodPost { - t.Fatalf("Mismatch method") - } - if resp.Values != v.Encode() { - t.Fatalf("Mismatch in values") - } - - // Do a fallbcak to a get. - u.Path = "/blockPost" - _, b, _, err = DoGetFallback(client, context.TODO(), u, v) - if err != nil { - t.Fatalf("Error doing local request: %v", err) - } - if err := json.Unmarshal(b, resp); err != nil { - t.Fatal(err) - } - if resp.Method != http.MethodGet { - t.Fatalf("Mismatch method") - } - if resp.Values != v.Encode() { - t.Fatalf("Mismatch in values") - } -} diff --git a/api/prometheus/v1/api.go b/api/prometheus/v1/api.go index 154e4a81f..9d1345d57 100644 --- a/api/prometheus/v1/api.go +++ b/api/prometheus/v1/api.go @@ -517,11 +517,15 @@ func (qr *queryResult) UnmarshalJSON(b []byte) error { // // It is safe to use the returned API from multiple goroutines. func NewAPI(c api.Client) API { - return &httpAPI{client: c} + return &httpAPI{ + client: &apiClientImpl{ + client: c, + }, + } } type httpAPI struct { - client api.Client + client apiClient } func (h *httpAPI) Alerts(ctx context.Context) (AlertsResult, error) { @@ -532,7 +536,7 @@ func (h *httpAPI) Alerts(ctx context.Context) (AlertsResult, error) { return AlertsResult{}, err } - _, body, _, err := h.Do(ctx, req) + _, body, _, err := h.client.Do(ctx, req) if err != nil { return AlertsResult{}, err } @@ -549,7 +553,7 @@ func (h *httpAPI) AlertManagers(ctx context.Context) (AlertManagersResult, error return AlertManagersResult{}, err } - _, body, _, err := h.Do(ctx, req) + _, body, _, err := h.client.Do(ctx, req) if err != nil { return AlertManagersResult{}, err } @@ -566,7 +570,7 @@ func (h *httpAPI) CleanTombstones(ctx context.Context) error { return err } - _, _, _, err = h.Do(ctx, req) + _, _, _, err = h.client.Do(ctx, req) return err } @@ -578,7 +582,7 @@ func (h *httpAPI) Config(ctx context.Context) (ConfigResult, error) { return ConfigResult{}, err } - _, body, _, err := h.Do(ctx, req) + _, body, _, err := h.client.Do(ctx, req) if err != nil { return ConfigResult{}, err } @@ -605,7 +609,7 @@ func (h *httpAPI) DeleteSeries(ctx context.Context, matches []string, startTime return err } - _, _, _, err = h.Do(ctx, req) + _, _, _, err = h.client.Do(ctx, req) return err } @@ -617,7 +621,7 @@ func (h *httpAPI) Flags(ctx context.Context) (FlagsResult, error) { return FlagsResult{}, err } - _, body, _, err := h.Do(ctx, req) + _, body, _, err := h.client.Do(ctx, req) if err != nil { return FlagsResult{}, err } @@ -632,7 +636,7 @@ func (h *httpAPI) LabelNames(ctx context.Context) ([]string, api.Warnings, error if err != nil { return nil, nil, err } - _, body, w, err := h.Do(ctx, req) + _, body, w, err := h.client.Do(ctx, req) if err != nil { return nil, w, err } @@ -646,7 +650,7 @@ func (h *httpAPI) LabelValues(ctx context.Context, label string) (model.LabelVal if err != nil { return nil, nil, err } - _, body, w, err := h.Do(ctx, req) + _, body, w, err := h.client.Do(ctx, req) if err != nil { return nil, w, err } @@ -663,7 +667,7 @@ func (h *httpAPI) Query(ctx context.Context, query string, ts time.Time) (model. q.Set("time", formatTime(ts)) } - _, body, warnings, err := h.DoGetFallback(ctx, u, q) + _, body, warnings, err := h.client.DoGetFallback(ctx, u, q) if err != nil { return nil, warnings, err } @@ -681,7 +685,7 @@ func (h *httpAPI) QueryRange(ctx context.Context, query string, r Range) (model. q.Set("end", formatTime(r.End)) q.Set("step", strconv.FormatFloat(r.Step.Seconds(), 'f', -1, 64)) - _, body, warnings, err := h.DoGetFallback(ctx, u, q) + _, body, warnings, err := h.client.DoGetFallback(ctx, u, q) if err != nil { return nil, warnings, err } @@ -709,7 +713,7 @@ func (h *httpAPI) Series(ctx context.Context, matches []string, startTime time.T return nil, nil, err } - _, body, warnings, err := h.Do(ctx, req) + _, body, warnings, err := h.client.Do(ctx, req) if err != nil { return nil, warnings, err } @@ -731,7 +735,7 @@ func (h *httpAPI) Snapshot(ctx context.Context, skipHead bool) (SnapshotResult, return SnapshotResult{}, err } - _, body, _, err := h.Do(ctx, req) + _, body, _, err := h.client.Do(ctx, req) if err != nil { return SnapshotResult{}, err } @@ -748,7 +752,7 @@ func (h *httpAPI) Rules(ctx context.Context) (RulesResult, error) { return RulesResult{}, err } - _, body, _, err := h.Do(ctx, req) + _, body, _, err := h.client.Do(ctx, req) if err != nil { return RulesResult{}, err } @@ -765,7 +769,7 @@ func (h *httpAPI) Targets(ctx context.Context) (TargetsResult, error) { return TargetsResult{}, err } - _, body, _, err := h.Do(ctx, req) + _, body, _, err := h.client.Do(ctx, req) if err != nil { return TargetsResult{}, err } @@ -789,7 +793,7 @@ func (h *httpAPI) TargetsMetadata(ctx context.Context, matchTarget string, metri return nil, err } - _, body, _, err := h.Do(ctx, req) + _, body, _, err := h.client.Do(ctx, req) if err != nil { return nil, err } @@ -798,6 +802,18 @@ func (h *httpAPI) TargetsMetadata(ctx context.Context, matchTarget string, metri return res, json.Unmarshal(body, &res) } +// apiClient wraps a regular client and processes successful API responses. +// Successful also includes responses that errored at the API level. +type apiClient interface { + URL(ep string, args map[string]string) *url.URL + Do(context.Context, *http.Request) (*http.Response, []byte, api.Warnings, error) + DoGetFallback(ctx context.Context, u *url.URL, args url.Values) (*http.Response, []byte, api.Warnings, error) +} + +type apiClientImpl struct { + client api.Client +} + type apiResponse struct { Status string `json:"status"` Data json.RawMessage `json:"data"` @@ -821,17 +837,21 @@ func errorTypeAndMsgFor(resp *http.Response) (ErrorType, string) { return ErrBadResponse, fmt.Sprintf("bad response code %d", resp.StatusCode) } -func (h *httpAPI) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, api.Warnings, error) { - resp, body, warnings, err := h.Do(ctx, req) +func (h *apiClientImpl) URL(ep string, args map[string]string) *url.URL { + return h.client.URL(ep, args) +} + +func (h *apiClientImpl) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, api.Warnings, error) { + resp, body, err := h.client.Do(ctx, req) if err != nil { - return resp, body, warnings, err + return resp, body, nil, err } code := resp.StatusCode if code/100 != 2 && !apiError(code) { errorType, errorMsg := errorTypeAndMsgFor(resp) - return resp, body, warnings, &Error{ + return resp, body, nil, &Error{ Type: errorType, Msg: errorMsg, Detail: string(body), @@ -842,7 +862,7 @@ func (h *httpAPI) Do(ctx context.Context, req *http.Request) (*http.Response, [] if http.StatusNoContent != code { if jsonErr := json.Unmarshal(body, &result); jsonErr != nil { - return resp, body, warnings, &Error{ + return resp, body, nil, &Error{ Type: ErrBadResponse, Msg: jsonErr.Error(), } @@ -863,12 +883,12 @@ func (h *httpAPI) Do(ctx context.Context, req *http.Request) (*http.Response, [] } } - return resp, []byte(result.Data), warnings, err + return resp, []byte(result.Data), result.Warnings, err } // DoGetFallback will attempt to do the request as-is, and on a 405 it will fallback to a GET request. -func (h *httpAPI) DoGetFallback(ctx context.Context, u *url.URL, args url.Values) (*http.Response, []byte, api.Warnings, error) { +func (h *apiClientImpl) DoGetFallback(ctx context.Context, u *url.URL, args url.Values) (*http.Response, []byte, api.Warnings, error) { req, err := http.NewRequest(http.MethodPost, u.String(), strings.NewReader(args.Encode())) if err != nil { return nil, nil, nil, err diff --git a/api/prometheus/v1/api_test.go b/api/prometheus/v1/api_test.go index 0572f8650..374d6cd32 100644 --- a/api/prometheus/v1/api_test.go +++ b/api/prometheus/v1/api_test.go @@ -17,8 +17,10 @@ import ( "context" "errors" "fmt" + "io/ioutil" "math" "net/http" + "net/http/httptest" "net/url" "reflect" "strings" @@ -92,14 +94,23 @@ func (c *apiTestClient) Do(ctx context.Context, req *http.Request) (*http.Respon return resp, b, test.inWarnings, test.inErr } +func (c *apiTestClient) DoGetFallback(ctx context.Context, u *url.URL, args url.Values) (*http.Response, []byte, api.Warnings, error) { + req, err := http.NewRequest(http.MethodPost, u.String(), strings.NewReader(args.Encode())) + if err != nil { + return nil, nil, nil, err + } + return c.Do(ctx, req) +} + func TestAPIs(t *testing.T) { testTime := time.Now() - client := &apiTestClient{T: t} - + tc := &apiTestClient{ + T: t, + } promAPI := &httpAPI{ - client: client, + client: tc, } doAlertManagers := func() func() (interface{}, api.Warnings, error) { @@ -855,7 +866,7 @@ func TestAPIs(t *testing.T) { for i, test := range tests { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { - client.curTest = test + tc.curTest = test res, warnings, err := test.do() @@ -907,7 +918,7 @@ 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.Warnings, error) { +func (c *testClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, error) { if ctx == nil { c.Fatalf("context was not passed down") } @@ -934,7 +945,7 @@ func (c *testClient) Do(ctx context.Context, req *http.Request) (*http.Response, StatusCode: test.code, } - return resp, b, test.expectedWarnings, nil + return resp, b, nil } func TestAPIClientDo(t *testing.T) { @@ -1065,7 +1076,9 @@ func TestAPIClientDo(t *testing.T) { ch: make(chan apiClientTest, 1), req: &http.Request{}, } - client := &apiClient{tc} + client := &apiClientImpl{ + client: tc, + } for i, test := range tests { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { @@ -1209,3 +1222,109 @@ func TestSamplesJsonSerialization(t *testing.T) { }) } } + +type httpTestClient struct { + client http.Client +} + +func (c *httpTestClient) URL(ep string, args map[string]string) *url.URL { + return nil +} + +func (c *httpTestClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, error) { + resp, err := c.client.Do(req) + if err != nil { + return nil, nil, err + } + + var body []byte + done := make(chan struct{}) + go func() { + body, err = ioutil.ReadAll(resp.Body) + close(done) + }() + + select { + case <-ctx.Done(): + <-done + err = resp.Body.Close() + if err == nil { + err = ctx.Err() + } + case <-done: + } + + return resp, body, err +} + +func TestDoGetFallback(t *testing.T) { + v := url.Values{"a": []string{"1", "2"}} + + type testResponse struct { + Values string + Method string + } + + // Start a local HTTP server. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + req.ParseForm() + r := &testResponse{ + Values: req.Form.Encode(), + Method: req.Method, + } + + body, _ := json.Marshal(r) + + if req.Method == http.MethodPost { + if req.URL.Path == "/blockPost" { + http.Error(w, string(body), http.StatusMethodNotAllowed) + return + } + } + + w.Write(body) + })) + // Close the server when test finishes. + defer server.Close() + + u, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + client := &httpTestClient{client: *(server.Client())} + api := &apiClientImpl{ + client: client, + } + + // Do a post, and ensure that the post succeeds. + _, b, _, err := api.DoGetFallback(context.TODO(), u, v) + if err != nil { + t.Fatalf("Error doing local request: %v", err) + } + resp := &testResponse{} + if err := json.Unmarshal(b, resp); err != nil { + t.Fatal(err) + } + if resp.Method != http.MethodPost { + t.Fatalf("Mismatch method") + } + if resp.Values != v.Encode() { + t.Fatalf("Mismatch in values") + } + + // Do a fallbcak to a get. + u.Path = "/blockPost" + _, b, _, err = api.DoGetFallback(context.TODO(), u, v) + if err != nil { + t.Fatalf("Error doing local request: %v", err) + } + if err := json.Unmarshal(b, resp); err != nil { + t.Fatal(err) + } + if resp.Method != http.MethodGet { + t.Fatalf("Mismatch method") + } + if resp.Values != v.Encode() { + t.Fatalf("Mismatch in values") + } +} From bd858421cd8dea5fbbe0a119ea5f6ef5866bf2dc Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Wed, 11 Dec 2019 09:36:49 -0500 Subject: [PATCH 3/4] Fixed TestDoGetFallback test Signed-off-by: Joe Elliott --- api/prometheus/v1/api_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/api/prometheus/v1/api_test.go b/api/prometheus/v1/api_test.go index 374d6cd32..d87283474 100644 --- a/api/prometheus/v1/api_test.go +++ b/api/prometheus/v1/api_test.go @@ -1268,12 +1268,16 @@ func TestDoGetFallback(t *testing.T) { // Start a local HTTP server. server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { req.ParseForm() - r := &testResponse{ + testResp, _ := json.Marshal(&testResponse{ Values: req.Form.Encode(), Method: req.Method, + }) + + apiResp := &apiResponse{ + Data: testResp, } - body, _ := json.Marshal(r) + body, _ := json.Marshal(apiResp) if req.Method == http.MethodPost { if req.URL.Path == "/blockPost" { From 88792b11691f7ef2a663b3ee3488c8870720a753 Mon Sep 17 00:00:00 2001 From: Joe Elliott Date: Wed, 11 Dec 2019 10:04:00 -0500 Subject: [PATCH 4/4] Moved warnings into apiclient Signed-off-by: Joe Elliott --- api/client.go | 2 -- api/prometheus/v1/api.go | 31 ++++++++-------- api/prometheus/v1/api_test.go | 68 +++++++++++++++++------------------ 3 files changed, 50 insertions(+), 51 deletions(-) diff --git a/api/client.go b/api/client.go index 6a2fe2c25..f7ca60b67 100644 --- a/api/client.go +++ b/api/client.go @@ -25,8 +25,6 @@ import ( "time" ) -type Warnings []string - // DefaultRoundTripper is used if no RoundTripper is set in Config. var DefaultRoundTripper http.RoundTripper = &http.Transport{ Proxy: http.ProxyFromEnvironment, diff --git a/api/prometheus/v1/api.go b/api/prometheus/v1/api.go index 9d1345d57..06319a89f 100644 --- a/api/prometheus/v1/api.go +++ b/api/prometheus/v1/api.go @@ -230,15 +230,15 @@ type API interface { // Flags returns the flag values that Prometheus was launched with. Flags(ctx context.Context) (FlagsResult, error) // LabelNames returns all the unique label names present in the block in sorted order. - LabelNames(ctx context.Context) ([]string, api.Warnings, error) + LabelNames(ctx context.Context) ([]string, Warnings, error) // LabelValues performs a query for the values of the given label. - LabelValues(ctx context.Context, label string) (model.LabelValues, api.Warnings, error) + LabelValues(ctx context.Context, label string) (model.LabelValues, Warnings, error) // Query performs a query for the given time. - Query(ctx context.Context, query string, ts time.Time) (model.Value, api.Warnings, error) + Query(ctx context.Context, query string, ts time.Time) (model.Value, Warnings, error) // QueryRange performs a query for the given range. - QueryRange(ctx context.Context, query string, r Range) (model.Value, api.Warnings, error) + QueryRange(ctx context.Context, query string, r Range) (model.Value, Warnings, error) // Series finds series by label matchers. - Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, api.Warnings, error) + Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, Warnings, 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, error) @@ -630,7 +630,7 @@ func (h *httpAPI) Flags(ctx context.Context) (FlagsResult, error) { return res, json.Unmarshal(body, &res) } -func (h *httpAPI) LabelNames(ctx context.Context) ([]string, api.Warnings, error) { +func (h *httpAPI) LabelNames(ctx context.Context) ([]string, Warnings, error) { u := h.client.URL(epLabels, nil) req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { @@ -644,7 +644,7 @@ func (h *httpAPI) LabelNames(ctx context.Context) ([]string, api.Warnings, error return labelNames, w, json.Unmarshal(body, &labelNames) } -func (h *httpAPI) LabelValues(ctx context.Context, label string) (model.LabelValues, api.Warnings, error) { +func (h *httpAPI) LabelValues(ctx context.Context, label string) (model.LabelValues, Warnings, error) { u := h.client.URL(epLabelValues, map[string]string{"name": label}) req, err := http.NewRequest(http.MethodGet, u.String(), nil) if err != nil { @@ -658,7 +658,7 @@ func (h *httpAPI) LabelValues(ctx context.Context, label string) (model.LabelVal return labelValues, w, json.Unmarshal(body, &labelValues) } -func (h *httpAPI) Query(ctx context.Context, query string, ts time.Time) (model.Value, api.Warnings, error) { +func (h *httpAPI) Query(ctx context.Context, query string, ts time.Time) (model.Value, Warnings, error) { u := h.client.URL(epQuery, nil) q := u.Query() @@ -676,7 +676,7 @@ func (h *httpAPI) Query(ctx context.Context, query string, ts time.Time) (model. return model.Value(qres.v), warnings, json.Unmarshal(body, &qres) } -func (h *httpAPI) QueryRange(ctx context.Context, query string, r Range) (model.Value, api.Warnings, error) { +func (h *httpAPI) QueryRange(ctx context.Context, query string, r Range) (model.Value, Warnings, error) { u := h.client.URL(epQueryRange, nil) q := u.Query() @@ -695,7 +695,7 @@ func (h *httpAPI) QueryRange(ctx context.Context, query string, r Range) (model. 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.Warnings, error) { +func (h *httpAPI) Series(ctx context.Context, matches []string, startTime time.Time, endTime time.Time) ([]model.LabelSet, Warnings, error) { u := h.client.URL(epSeries, nil) q := u.Query() @@ -802,12 +802,15 @@ func (h *httpAPI) TargetsMetadata(ctx context.Context, matchTarget string, metri return res, json.Unmarshal(body, &res) } +// Warnings is an array of non critical errors +type Warnings []string + // apiClient wraps a regular client and processes successful API responses. // Successful also includes responses that errored at the API level. type apiClient interface { URL(ep string, args map[string]string) *url.URL - Do(context.Context, *http.Request) (*http.Response, []byte, api.Warnings, error) - DoGetFallback(ctx context.Context, u *url.URL, args url.Values) (*http.Response, []byte, api.Warnings, error) + Do(context.Context, *http.Request) (*http.Response, []byte, Warnings, error) + DoGetFallback(ctx context.Context, u *url.URL, args url.Values) (*http.Response, []byte, Warnings, error) } type apiClientImpl struct { @@ -841,7 +844,7 @@ func (h *apiClientImpl) URL(ep string, args map[string]string) *url.URL { return h.client.URL(ep, args) } -func (h *apiClientImpl) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, api.Warnings, error) { +func (h *apiClientImpl) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, Warnings, error) { resp, body, err := h.client.Do(ctx, req) if err != nil { return resp, body, nil, err @@ -888,7 +891,7 @@ func (h *apiClientImpl) Do(ctx context.Context, req *http.Request) (*http.Respon } // DoGetFallback will attempt to do the request as-is, and on a 405 it will fallback to a GET request. -func (h *apiClientImpl) DoGetFallback(ctx context.Context, u *url.URL, args url.Values) (*http.Response, []byte, api.Warnings, error) { +func (h *apiClientImpl) DoGetFallback(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, nil, err diff --git a/api/prometheus/v1/api_test.go b/api/prometheus/v1/api_test.go index d87283474..b43318e65 100644 --- a/api/prometheus/v1/api_test.go +++ b/api/prometheus/v1/api_test.go @@ -30,12 +30,10 @@ import ( json "github.com/json-iterator/go" "github.com/prometheus/common/model" - - "github.com/prometheus/client_golang/api" ) type apiTest struct { - do func() (interface{}, api.Warnings, error) + do func() (interface{}, Warnings, error) inWarnings []string inErr error inStatusCode int @@ -45,7 +43,7 @@ type apiTest struct { reqParam url.Values reqMethod string res interface{} - warnings api.Warnings + warnings Warnings err error } @@ -66,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.Warnings, error) { +func (c *apiTestClient) Do(ctx context.Context, req *http.Request) (*http.Response, []byte, Warnings, error) { test := c.curTest @@ -94,7 +92,7 @@ func (c *apiTestClient) Do(ctx context.Context, req *http.Request) (*http.Respon return resp, b, test.inWarnings, test.inErr } -func (c *apiTestClient) DoGetFallback(ctx context.Context, u *url.URL, args url.Values) (*http.Response, []byte, api.Warnings, error) { +func (c *apiTestClient) DoGetFallback(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, nil, err @@ -113,92 +111,92 @@ func TestAPIs(t *testing.T) { client: tc, } - doAlertManagers := func() func() (interface{}, api.Warnings, error) { - return func() (interface{}, api.Warnings, error) { + doAlertManagers := func() func() (interface{}, Warnings, error) { + return func() (interface{}, Warnings, error) { v, err := promAPI.AlertManagers(context.Background()) return v, nil, err } } - doCleanTombstones := func() func() (interface{}, api.Warnings, error) { - return func() (interface{}, api.Warnings, error) { + doCleanTombstones := func() func() (interface{}, Warnings, error) { + return func() (interface{}, Warnings, error) { return nil, nil, promAPI.CleanTombstones(context.Background()) } } - doConfig := func() func() (interface{}, api.Warnings, error) { - return func() (interface{}, api.Warnings, error) { + doConfig := func() func() (interface{}, Warnings, error) { + return func() (interface{}, 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.Warnings, error) { - return func() (interface{}, api.Warnings, error) { + doDeleteSeries := func(matcher string, startTime time.Time, endTime time.Time) func() (interface{}, Warnings, error) { + return func() (interface{}, Warnings, error) { return nil, nil, promAPI.DeleteSeries(context.Background(), []string{matcher}, startTime, endTime) } } - doFlags := func() func() (interface{}, api.Warnings, error) { - return func() (interface{}, api.Warnings, error) { + doFlags := func() func() (interface{}, Warnings, error) { + return func() (interface{}, Warnings, error) { v, err := promAPI.Flags(context.Background()) return v, nil, err } } - doLabelNames := func(label string) func() (interface{}, api.Warnings, error) { - return func() (interface{}, api.Warnings, error) { + doLabelNames := func(label string) func() (interface{}, Warnings, error) { + return func() (interface{}, Warnings, error) { return promAPI.LabelNames(context.Background()) } } - doLabelValues := func(label string) func() (interface{}, api.Warnings, error) { - return func() (interface{}, api.Warnings, error) { + doLabelValues := func(label string) func() (interface{}, Warnings, error) { + return func() (interface{}, Warnings, error) { return promAPI.LabelValues(context.Background(), label) } } - doQuery := func(q string, ts time.Time) func() (interface{}, api.Warnings, error) { - return func() (interface{}, api.Warnings, error) { + doQuery := func(q string, ts time.Time) func() (interface{}, Warnings, error) { + return func() (interface{}, Warnings, error) { return promAPI.Query(context.Background(), q, ts) } } - doQueryRange := func(q string, rng Range) func() (interface{}, api.Warnings, error) { - return func() (interface{}, api.Warnings, error) { + doQueryRange := func(q string, rng Range) func() (interface{}, Warnings, error) { + return func() (interface{}, Warnings, error) { return promAPI.QueryRange(context.Background(), q, rng) } } - doSeries := func(matcher string, startTime time.Time, endTime time.Time) func() (interface{}, api.Warnings, error) { - return func() (interface{}, api.Warnings, error) { + doSeries := func(matcher string, startTime time.Time, endTime time.Time) func() (interface{}, Warnings, error) { + return func() (interface{}, Warnings, error) { return promAPI.Series(context.Background(), []string{matcher}, startTime, endTime) } } - doSnapshot := func(skipHead bool) func() (interface{}, api.Warnings, error) { - return func() (interface{}, api.Warnings, error) { + doSnapshot := func(skipHead bool) func() (interface{}, Warnings, error) { + return func() (interface{}, Warnings, error) { v, err := promAPI.Snapshot(context.Background(), skipHead) return v, nil, err } } - doRules := func() func() (interface{}, api.Warnings, error) { - return func() (interface{}, api.Warnings, error) { + doRules := func() func() (interface{}, Warnings, error) { + return func() (interface{}, Warnings, error) { v, err := promAPI.Rules(context.Background()) return v, nil, err } } - doTargets := func() func() (interface{}, api.Warnings, error) { - return func() (interface{}, api.Warnings, error) { + doTargets := func() func() (interface{}, Warnings, error) { + return func() (interface{}, Warnings, error) { v, err := promAPI.Targets(context.Background()) return v, nil, err } } - doTargetsMetadata := func(matchTarget string, metric string, limit string) func() (interface{}, api.Warnings, error) { - return func() (interface{}, api.Warnings, error) { + doTargetsMetadata := func(matchTarget string, metric string, limit string) func() (interface{}, Warnings, error) { + return func() (interface{}, Warnings, error) { v, err := promAPI.TargetsMetadata(context.Background(), matchTarget, metric, limit) return v, nil, err } @@ -911,7 +909,7 @@ type apiClientTest struct { response interface{} expectedBody string expectedErr *Error - expectedWarnings api.Warnings + expectedWarnings Warnings } func (c *testClient) URL(ep string, args map[string]string) *url.URL {