From 055c3b7332ea0342ac53d4cfbde2cf53f7c03498 Mon Sep 17 00:00:00 2001 From: Fionera Date: Fri, 16 Dec 2022 11:15:19 +0100 Subject: [PATCH] Prometheus: Add POST support to client (#60243) * Prometheus: Add POST support to client * Prometheus: Revert client test change from 1c503908 --- pkg/tests/api/prometheus/prometheus_test.go | 5 +- pkg/tsdb/prometheus/client/client.go | 67 +++++++++++++-------- pkg/tsdb/prometheus/client/client_test.go | 59 ++++++++++++++++++ 3 files changed, 104 insertions(+), 27 deletions(-) diff --git a/pkg/tests/api/prometheus/prometheus_test.go b/pkg/tests/api/prometheus/prometheus_test.go index 68259c35639..cab8f2b7e8f 100644 --- a/pkg/tests/api/prometheus/prometheus_test.go +++ b/pkg/tests/api/prometheus/prometheus_test.go @@ -96,8 +96,7 @@ func TestIntegrationPrometheus(t *testing.T) { require.NoError(t, err) require.NotNil(t, outgoingRequest) - require.Equal(t, "/api/v1/query_range?end=1668081660&q1=1&q2=2&query=up&start=1668078060&step=30", - outgoingRequest.URL.String()) + require.Equal(t, "/api/v1/query_range?q1=1&q2=2", outgoingRequest.URL.String()) require.Equal(t, "custom-header-value", outgoingRequest.Header.Get("X-CUSTOM-HEADER")) username, pwd, ok := outgoingRequest.BasicAuth() require.True(t, ok) @@ -134,7 +133,7 @@ func TestIntegrationPrometheus(t *testing.T) { require.NotNil(t, outgoingRequest) require.Equal(t, "/api/v1/query_range", outgoingRequest.URL.Path) - require.Contains(t, outgoingRequest.URL.String(), "&q1=1&q2=2") + require.Contains(t, outgoingRequest.URL.String(), "?q1=1&q2=2") require.Equal(t, "custom-header-value", outgoingRequest.Header.Get("X-CUSTOM-HEADER")) username, pwd, ok := outgoingRequest.BasicAuth() require.True(t, ok) diff --git a/pkg/tsdb/prometheus/client/client.go b/pkg/tsdb/prometheus/client/client.go index 21e847afe0f..c3f0c4d0719 100644 --- a/pkg/tsdb/prometheus/client/client.go +++ b/pkg/tsdb/prometheus/client/client.go @@ -3,6 +3,7 @@ package client import ( "bytes" "context" + "io" "net/http" "net/url" "path" @@ -33,16 +34,14 @@ func NewClient(d doer, method, baseUrl string) *Client { func (c *Client) QueryRange(ctx context.Context, q *models.Query, headers http.Header) (*http.Response, error) { tr := q.TimeRange() - u, err := c.createUrl("api/v1/query_range", map[string]string{ + qv := map[string]string{ "query": q.Expr, "start": formatTime(tr.Start), "end": formatTime(tr.End), "step": strconv.FormatFloat(tr.Step.Seconds(), 'f', -1, 64), - }) - if err != nil { - return nil, err } - req, err := createRequest(ctx, c.method, u, nil, headers) + + req, err := c.createQueryRequest(ctx, "api/v1/query_range", qv, headers) if err != nil { return nil, err } @@ -51,17 +50,13 @@ func (c *Client) QueryRange(ctx context.Context, q *models.Query, headers http.H } func (c *Client) QueryInstant(ctx context.Context, q *models.Query, headers http.Header) (*http.Response, error) { - qs := map[string]string{"query": q.Expr} + qv := map[string]string{"query": q.Expr} tr := q.TimeRange() if !tr.End.IsZero() { - qs["time"] = formatTime(tr.End) + qv["time"] = formatTime(tr.End) } - u, err := c.createUrl("api/v1/query", qs) - if err != nil { - return nil, err - } - req, err := createRequest(ctx, c.method, u, nil, headers) + req, err := c.createQueryRequest(ctx, "api/v1/query", qv, headers) if err != nil { return nil, err } @@ -71,16 +66,13 @@ func (c *Client) QueryInstant(ctx context.Context, q *models.Query, headers http func (c *Client) QueryExemplars(ctx context.Context, q *models.Query, headers http.Header) (*http.Response, error) { tr := q.TimeRange() - u, err := c.createUrl("api/v1/query_exemplars", map[string]string{ + qv := map[string]string{ "query": q.Expr, "start": formatTime(tr.Start), "end": formatTime(tr.End), - }) - if err != nil { - return nil, err } - req, err := createRequest(ctx, c.method, u, nil, headers) + req, err := c.createQueryRequest(ctx, "api/v1/query_exemplars", qv, headers) if err != nil { return nil, err } @@ -103,7 +95,7 @@ func (c *Client) QueryResource(ctx context.Context, req *backend.CallResourceReq // We use method from the request, as for resources front end may do a fallback to GET if POST does not work // nad we want to respect that. - httpRequest, err := createRequest(ctx, req.Method, u, req.Body, req.Headers) + httpRequest, err := createRequest(ctx, req.Method, u, bytes.NewReader(req.Body), req.Headers) if err != nil { return nil, err } @@ -111,6 +103,29 @@ func (c *Client) QueryResource(ctx context.Context, req *backend.CallResourceReq return c.doer.Do(httpRequest) } +func (c *Client) createQueryRequest(ctx context.Context, endpoint string, qv map[string]string, headers http.Header) (*http.Request, error) { + if strings.ToUpper(c.method) == http.MethodPost { + u, err := c.createUrl(endpoint, nil) + if err != nil { + return nil, err + } + + v := make(url.Values) + for key, val := range qv { + v.Set(key, val) + } + + return createRequest(ctx, c.method, u, strings.NewReader(v.Encode()), headers) + } + + u, err := c.createUrl(endpoint, qv) + if err != nil { + return nil, err + } + + return createRequest(ctx, c.method, u, http.NoBody, headers) +} + func (c *Client) createUrl(endpoint string, qs map[string]string) (*url.URL, error) { finalUrl, err := url.ParseRequestURI(c.baseUrl) if err != nil { @@ -118,18 +133,22 @@ func (c *Client) createUrl(endpoint string, qs map[string]string) (*url.URL, err } finalUrl.Path = path.Join(finalUrl.Path, endpoint) - urlQuery := finalUrl.Query() - for key, val := range qs { - urlQuery.Set(key, val) + // don't re-encode the Query if not needed + if len(qs) != 0 { + urlQuery := finalUrl.Query() + + for key, val := range qs { + urlQuery.Set(key, val) + } + + finalUrl.RawQuery = urlQuery.Encode() } - finalUrl.RawQuery = urlQuery.Encode() return finalUrl, nil } -func createRequest(ctx context.Context, method string, u *url.URL, body []byte, header http.Header) (*http.Request, error) { - bodyReader := bytes.NewReader(body) +func createRequest(ctx context.Context, method string, u *url.URL, bodyReader io.Reader, header http.Header) (*http.Request, error) { request, err := http.NewRequestWithContext(ctx, method, u.String(), bodyReader) if err != nil { return nil, err diff --git a/pkg/tsdb/prometheus/client/client_test.go b/pkg/tsdb/prometheus/client/client_test.go index 17b287aff4d..9e18008b422 100644 --- a/pkg/tsdb/prometheus/client/client_test.go +++ b/pkg/tsdb/prometheus/client/client_test.go @@ -5,9 +5,11 @@ import ( "io" "net/http" "testing" + "time" "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana/pkg/cmd/grafana-cli/logger" + "github.com/grafana/grafana/pkg/tsdb/prometheus/models" "github.com/stretchr/testify/require" ) @@ -80,4 +82,61 @@ func TestClient(t *testing.T) { require.Equal(t, "http://localhost:9090/api/v1/series?match%5B%5D=ALERTS&start=1655272558&end=1655294158", doer.Req.URL.String()) }) }) + + t.Run("QueryRange", func(t *testing.T) { + doer := &MockDoer{} + + t.Run("sends correct POST query", func(t *testing.T) { + client := NewClient(doer, http.MethodPost, "http://localhost:9090") + req := &models.Query{ + Expr: "rate(ALERTS{job=\"test\" [$__rate_interval]})", + Start: time.Unix(0, 0), + End: time.Unix(1234, 0), + RangeQuery: true, + Step: 1 * time.Second, + } + res, err := client.QueryRange(context.Background(), req, nil) + defer func() { + if res != nil && res.Body != nil { + if err := res.Body.Close(); err != nil { + logger.Warn("Error", "err", err) + } + } + }() + require.NoError(t, err) + require.NotNil(t, doer.Req) + require.Equal(t, http.MethodPost, doer.Req.Method) + require.Equal(t, "application/x-www-form-urlencoded", doer.Req.Header.Get("Content-Type")) + body, err := io.ReadAll(doer.Req.Body) + require.NoError(t, err) + require.Equal(t, []byte("end=1234&query=rate%28ALERTS%7Bjob%3D%22test%22+%5B%24__rate_interval%5D%7D%29&start=0&step=1"), body) + require.Equal(t, "http://localhost:9090/api/v1/query_range", doer.Req.URL.String()) + }) + + t.Run("sends correct GET query", func(t *testing.T) { + client := NewClient(doer, http.MethodGet, "http://localhost:9090") + req := &models.Query{ + Expr: "rate(ALERTS{job=\"test\" [$__rate_interval]})", + Start: time.Unix(0, 0), + End: time.Unix(1234, 0), + RangeQuery: true, + Step: 1 * time.Second, + } + res, err := client.QueryRange(context.Background(), req, nil) + defer func() { + if res != nil && res.Body != nil { + if err := res.Body.Close(); err != nil { + logger.Warn("Error", "err", err) + } + } + }() + require.NoError(t, err) + require.NotNil(t, doer.Req) + require.Equal(t, http.MethodGet, doer.Req.Method) + body, err := io.ReadAll(doer.Req.Body) + require.NoError(t, err) + require.Equal(t, []byte{}, body) + require.Equal(t, "http://localhost:9090/api/v1/query_range?end=1234&query=rate%28ALERTS%7Bjob%3D%22test%22+%5B%24__rate_interval%5D%7D%29&start=0&step=1", doer.Req.URL.String()) + }) + }) }