From a5f46d9b90d3055093ed2cf8c7ac79681849fc62 Mon Sep 17 00:00:00 2001 From: hagen1778 Date: Mon, 4 Sep 2023 10:11:34 +0200 Subject: [PATCH] follow-up after https://github.com/VictoriaMetrics/grafana-datasource/commit/23e35c8e40897647b07293536380dd2c511fd9e6 --- pkg/plugin/datasource.go | 23 ++++++------ pkg/plugin/query.go | 47 ++++++++++++------------ pkg/plugin/query_test.go | 77 +++++++++++++++++++++++----------------- 3 files changed, 79 insertions(+), 68 deletions(-) diff --git a/pkg/plugin/datasource.go b/pkg/plugin/datasource.go index 2043bce6..711fb833 100644 --- a/pkg/plugin/datasource.go +++ b/pkg/plugin/datasource.go @@ -81,30 +81,31 @@ func (d *Datasource) query(ctx context.Context, query backend.DataQuery) backend q.TimeRange = TimeRange(query.TimeRange) q.MaxDataPoints = query.MaxDataPoints - minInterval, err := q.calculateMinInterval() if err != nil { err = fmt.Errorf("failed to calculate minimal interval: %w", err) return newResponseError(err, backend.StatusBadRequest) } - httpMethod := http.MethodPost - var settingsData struct { - HTTPMethod string `json:"httpMethod"` - CustomQueryParameters string `json:"customQueryParameters"` + var settings struct { + HTTPMethod string `json:"httpMethod"` + QueryParams string `json:"customQueryParameters"` + } + if err := json.Unmarshal(d.settings.JSONData, &settings); err != nil { + err = fmt.Errorf("failed to parse datasource settings: %w", err) + return newResponseError(err, backend.StatusBadRequest) } - if err := json.Unmarshal(d.settings.JSONData, &settingsData); err == nil && settingsData.HTTPMethod != "" { - httpMethod = settingsData.HTTPMethod + if settings.HTTPMethod == "" { + settings.HTTPMethod = http.MethodPost } - customQueryParameters := settingsData.CustomQueryParameters - reqURL, err := q.getQueryURL(minInterval, d.settings.URL, customQueryParameters) + reqURL, err := q.getQueryURL(minInterval, d.settings.URL, settings.QueryParams) if err != nil { - err = fmt.Errorf("failed to create request url: %w", err) + err = fmt.Errorf("failed to create request URL: %w", err) return newResponseError(err, backend.StatusBadRequest) } - req, err := http.NewRequestWithContext(ctx, httpMethod, reqURL, nil) + req, err := http.NewRequestWithContext(ctx, settings.HTTPMethod, reqURL, nil) if err != nil { err = fmt.Errorf("failed to create new request with context: %w", err) return newResponseError(err, backend.StatusBadRequest) diff --git a/pkg/plugin/query.go b/pkg/plugin/query.go index a5c8502b..e1b3d5a1 100644 --- a/pkg/plugin/query.go +++ b/pkg/plugin/query.go @@ -7,8 +7,6 @@ import ( "strconv" "strings" "time" - - "github.com/grafana/grafana-plugin-sdk-go/backend/log" ) const ( @@ -39,7 +37,7 @@ type TimeRange struct { // GetQueryURL calculates step and clear expression from template variables, // and after builds query url depends on query type -func (q *Query) getQueryURL(minInterval time.Duration, rawURL string, customQueryParams string) (string, error) { +func (q *Query) getQueryURL(minInterval time.Duration, rawURL string, queryParams string) (string, error) { if rawURL == "" { return "", fmt.Errorf("url can't be blank") } @@ -47,6 +45,10 @@ func (q *Query) getQueryURL(minInterval time.Duration, rawURL string, customQuer if err != nil { return "", fmt.Errorf("failed to parse datasource url: %s", err) } + params, err := url.ParseQuery(queryParams) + if err != nil { + return "", fmt.Errorf("failed to parse query params: %s", err.Error()) + } q.url = u from := q.TimeRange.From @@ -61,9 +63,9 @@ func (q *Query) getQueryURL(minInterval time.Duration, rawURL string, customQuer } if q.Instant { - return q.queryInstantURL(expr, step, customQueryParams), nil + return q.queryInstantURL(expr, step, params), nil } - return q.queryRangeURL(expr, step, customQueryParams), nil + return q.queryRangeURL(expr, step, params), nil } // withIntervalVariable checks does query has interval variable @@ -81,29 +83,37 @@ func (q *Query) calculateMinInterval() (time.Duration, error) { } // queryInstantURL prepare query url for instant query -func (q *Query) queryInstantURL(expr string, step time.Duration, customQueryParams string) string { +func (q *Query) queryInstantURL(expr string, step time.Duration, queryParams url.Values) string { q.url.Path = path.Join(q.url.Path, instantQueryPath) values := q.url.Query() - values.Add("query", expr) - values.Add("time", strconv.FormatInt(q.TimeRange.To.Unix(), 10)) - values.Add("step", step.String()) - addCustomParams(values, customQueryParams) + for k, vl := range queryParams { + for _, v := range vl { + values.Add(k, v) + } + } + values.Set("query", expr) + values.Set("time", strconv.FormatInt(q.TimeRange.To.Unix(), 10)) + values.Set("step", step.String()) q.url.RawQuery = values.Encode() return q.url.String() } // queryRangeURL prepare query url for range query -func (q *Query) queryRangeURL(expr string, step time.Duration, customQueryParams string) string { +func (q *Query) queryRangeURL(expr string, step time.Duration, queryParams url.Values) string { q.url.Path = path.Join(q.url.Path, rangeQueryPath) values := q.url.Query() + for k, vl := range queryParams { + for _, v := range vl { + values.Add(k, v) + } + } values.Add("query", expr) values.Add("start", strconv.FormatInt(q.TimeRange.From.Unix(), 10)) values.Add("end", strconv.FormatInt(q.TimeRange.To.Unix(), 10)) values.Add("step", step.String()) - addCustomParams(values, customQueryParams) q.url.RawQuery = values.Encode() return q.url.String() @@ -119,16 +129,3 @@ func (q *Query) parseLegend() string { } return legend } - -func addCustomParams(values url.Values, customQueryParams string) url.Values { - params, err := url.ParseQuery(customQueryParams) - if err != nil { - log.DefaultLogger.Error("failed to parse custom query params", "err", err.Error()) - } - for key, valueList := range params { - for _, value := range valueList { - values.Add(key, value) - } - } - return values -} diff --git a/pkg/plugin/query_test.go b/pkg/plugin/query_test.go index 1b8ae3ec..0ed8ef04 100644 --- a/pkg/plugin/query_test.go +++ b/pkg/plugin/query_test.go @@ -27,22 +27,21 @@ func TestQuery_getQueryURL(t *testing.T) { name string fields fields args args + params string want string wantErr bool }{ { name: "empty values", fields: fields{ - RefID: "1", - Instant: false, - Range: false, - Interval: "", - IntervalMs: 0, - TimeInterval: "", - Expr: "", - MaxDataPoints: 0, - getTimeRange: getTimeRage, - url: nil, + RefID: "1", + Instant: false, + Range: false, + Interval: "", + IntervalMs: 0, + TimeInterval: "", + Expr: "", + getTimeRange: getTimeRage, }, args: args{ minInterval: 0, @@ -54,16 +53,13 @@ func TestQuery_getQueryURL(t *testing.T) { { name: "empty instant expression", fields: fields{ - RefID: "1", - Instant: true, - Range: false, - Interval: "10s", - IntervalMs: 0, - TimeInterval: "", - Expr: "", - MaxDataPoints: 0, - getTimeRange: getTimeRage, - url: nil, + RefID: "1", + Instant: true, + Range: false, + Interval: "10s", + TimeInterval: "", + Expr: "", + getTimeRange: getTimeRage, }, args: args{ minInterval: 0, @@ -75,16 +71,14 @@ func TestQuery_getQueryURL(t *testing.T) { { name: "empty instant query with interval", fields: fields{ - RefID: "1", - Instant: true, - Range: false, - Interval: "10s", - IntervalMs: 5_000_000, - TimeInterval: "", - Expr: "rate(ingress_nginx_request_qps{}[$__interval])", - MaxDataPoints: 0, - getTimeRange: getTimeRage, - url: nil, + RefID: "1", + Instant: true, + Range: false, + Interval: "10s", + IntervalMs: 5_000_000, + TimeInterval: "", + Expr: "rate(ingress_nginx_request_qps{}[$__interval])", + getTimeRange: getTimeRage, }, args: args{ minInterval: 0, @@ -126,7 +120,6 @@ func TestQuery_getQueryURL(t *testing.T) { Expr: "rate(ingress_nginx_request_qps{}[$__rate_interval])", MaxDataPoints: 3000, getTimeRange: getTimeRage, - url: nil, }, args: args{ minInterval: time.Second * 10, @@ -135,6 +128,26 @@ func TestQuery_getQueryURL(t *testing.T) { wantErr: false, want: "http://127.0.0.1:8428/api/v1/query_range?end=1670226793&query=rate%28ingress_nginx_request_qps%7B%7D%5B10s%5D%29&start=1670226733&step=10s", }, + { + name: "custom query params", + fields: fields{ + RefID: "1", + Instant: true, + Range: false, + Interval: "10s", + IntervalMs: 5_000_000, + TimeInterval: "", + Expr: "rate(ingress_nginx_request_qps{}[$__interval])", + getTimeRange: getTimeRage, + }, + args: args{ + minInterval: 0, + rawURL: "http://127.0.0.1:8428", + }, + params: "extra_filters[]={job=\"vmalert\"}", + wantErr: false, + want: "http://127.0.0.1:8428/api/v1/query?extra_filters%5B%5D=%7Bjob%3D%22vmalert%22%7D&query=rate%28ingress_nginx_request_qps%7B%7D%5B1ms%5D%29&step=50ms&time=1670226793", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -150,7 +163,7 @@ func TestQuery_getQueryURL(t *testing.T) { TimeRange: tt.fields.getTimeRange(), url: tt.fields.url, } - got, err := q.getQueryURL(tt.args.minInterval, tt.args.rawURL, "") + got, err := q.getQueryURL(tt.args.minInterval, tt.args.rawURL, tt.params) if (err != nil) != tt.wantErr { t.Errorf("getQueryURL() error = %v, wantErr %v", err, tt.wantErr) return