From c4c05a5b7136dfadf05ddd6a60c01d7cc5ff5b08 Mon Sep 17 00:00:00 2001 From: Travis Patterson Date: Tue, 4 Jan 2022 07:22:33 -0700 Subject: [PATCH] refactor promethueus package into sub packages (#43634) --- pkg/tsdb/prometheus/client/provider.go | 53 +++++++++++++++++++ pkg/tsdb/prometheus/client/provider_test.go | 47 ++++++++++++++++ .../custom_query_params.go} | 4 +- .../custom_query_params_test.go} | 14 ++--- .../force_http_get.go} | 4 +- .../force_http_get_test.go} | 46 ++-------------- pkg/tsdb/prometheus/prometheus.go | 51 ++---------------- 7 files changed, 117 insertions(+), 102 deletions(-) create mode 100644 pkg/tsdb/prometheus/client/provider.go create mode 100644 pkg/tsdb/prometheus/client/provider_test.go rename pkg/tsdb/prometheus/{custom_query_params_middleware.go => middleware/custom_query_params.go} (93%) rename pkg/tsdb/prometheus/{custom_query_params_middleware_test.go => middleware/custom_query_params_test.go} (93%) rename pkg/tsdb/prometheus/{force_http_get_middleware.go => middleware/force_http_get.go} (91%) rename pkg/tsdb/prometheus/{force_http_get_middleware_test.go => middleware/force_http_get_test.go} (51%) diff --git a/pkg/tsdb/prometheus/client/provider.go b/pkg/tsdb/prometheus/client/provider.go new file mode 100644 index 00000000000..6039250cbb3 --- /dev/null +++ b/pkg/tsdb/prometheus/client/provider.go @@ -0,0 +1,53 @@ +package client + +import ( + "strings" + + "github.com/grafana/grafana/pkg/tsdb/prometheus/middleware" + + sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" + "github.com/grafana/grafana/pkg/infra/httpclient" + "github.com/grafana/grafana/pkg/infra/log" + "github.com/prometheus/client_golang/api" + apiv1 "github.com/prometheus/client_golang/api/prometheus/v1" +) + +func Create(url string, httpOpts sdkhttpclient.Options, clientProvider httpclient.Provider, jsonData map[string]interface{}, plog log.Logger) (apiv1.API, error) { + customParamsMiddleware := middleware.CustomQueryParameters(plog) + middlewares := []sdkhttpclient.Middleware{customParamsMiddleware} + if shouldForceGet(jsonData) { + middlewares = append(middlewares, middleware.ForceHttpGet(plog)) + } + httpOpts.Middlewares = middlewares + + roundTripper, err := clientProvider.GetTransport(httpOpts) + if err != nil { + return nil, err + } + + cfg := api.Config{ + Address: url, + RoundTripper: roundTripper, + } + + client, err := api.NewClient(cfg) + if err != nil { + return nil, err + } + + return apiv1.NewAPI(client), nil +} + +func shouldForceGet(settingsJson map[string]interface{}) bool { + methodInterface, exists := settingsJson["httpMethod"] + if !exists { + return false + } + + method, ok := methodInterface.(string) + if !ok { + return false + } + + return strings.ToLower(method) == "get" +} diff --git a/pkg/tsdb/prometheus/client/provider_test.go b/pkg/tsdb/prometheus/client/provider_test.go new file mode 100644 index 00000000000..a5571458021 --- /dev/null +++ b/pkg/tsdb/prometheus/client/provider_test.go @@ -0,0 +1,47 @@ +package client + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestForceGet(t *testing.T) { + t.Run("With nil jsonOpts, should not force get-method", func(t *testing.T) { + var jsonOpts map[string]interface{} + require.False(t, shouldForceGet(jsonOpts)) + }) + + t.Run("With empty jsonOpts, should not force get-method", func(t *testing.T) { + jsonOpts := make(map[string]interface{}) + require.False(t, shouldForceGet(jsonOpts)) + }) + + t.Run("With httpMethod=nil, should not not force get-method", func(t *testing.T) { + jsonOpts := map[string]interface{}{ + "httpMethod": nil, + } + require.False(t, shouldForceGet(jsonOpts)) + }) + + t.Run("With httpMethod=post, should not force get-method", func(t *testing.T) { + jsonOpts := map[string]interface{}{ + "httpMethod": "POST", + } + require.False(t, shouldForceGet(jsonOpts)) + }) + + t.Run("With httpMethod=get, should force get-method", func(t *testing.T) { + jsonOpts := map[string]interface{}{ + "httpMethod": "get", + } + require.True(t, shouldForceGet(jsonOpts)) + }) + + t.Run("With httpMethod=GET, should force get-method", func(t *testing.T) { + jsonOpts := map[string]interface{}{ + "httpMethod": "GET", + } + require.True(t, shouldForceGet(jsonOpts)) + }) +} diff --git a/pkg/tsdb/prometheus/custom_query_params_middleware.go b/pkg/tsdb/prometheus/middleware/custom_query_params.go similarity index 93% rename from pkg/tsdb/prometheus/custom_query_params_middleware.go rename to pkg/tsdb/prometheus/middleware/custom_query_params.go index 3bdab4cfe9b..b352c2f6a0d 100644 --- a/pkg/tsdb/prometheus/custom_query_params_middleware.go +++ b/pkg/tsdb/prometheus/middleware/custom_query_params.go @@ -1,4 +1,4 @@ -package prometheus +package middleware import ( "net/http" @@ -14,7 +14,7 @@ const ( grafanaDataKey = "grafanaData" ) -func customQueryParametersMiddleware(logger log.Logger) sdkhttpclient.Middleware { +func CustomQueryParameters(logger log.Logger) sdkhttpclient.Middleware { return sdkhttpclient.NamedMiddlewareFunc(customQueryParametersMiddlewareName, func(opts sdkhttpclient.Options, next http.RoundTripper) http.RoundTripper { grafanaData, exists := opts.CustomOptions[grafanaDataKey] if !exists { diff --git a/pkg/tsdb/prometheus/custom_query_params_middleware_test.go b/pkg/tsdb/prometheus/middleware/custom_query_params_test.go similarity index 93% rename from pkg/tsdb/prometheus/custom_query_params_middleware_test.go rename to pkg/tsdb/prometheus/middleware/custom_query_params_test.go index c4e41ee49e2..fd1e0e22e0a 100644 --- a/pkg/tsdb/prometheus/custom_query_params_middleware_test.go +++ b/pkg/tsdb/prometheus/middleware/custom_query_params_test.go @@ -1,4 +1,4 @@ -package prometheus +package middleware import ( "net/http" @@ -19,7 +19,7 @@ func TestCustomQueryParametersMiddleware(t *testing.T) { }) t.Run("Without custom query parameters set should not apply middleware", func(t *testing.T) { - mw := customQueryParametersMiddleware(log.New("test")) + mw := CustomQueryParameters(log.New("test")) rt := mw.CreateMiddleware(httpclient.Options{}, finalRoundTripper) require.NotNil(t, rt) middlewareName, ok := mw.(httpclient.MiddlewareName) @@ -39,7 +39,7 @@ func TestCustomQueryParametersMiddleware(t *testing.T) { }) t.Run("Without custom query parameters set as string should not apply middleware", func(t *testing.T) { - mw := customQueryParametersMiddleware(log.New("test")) + mw := CustomQueryParameters(log.New("test")) rt := mw.CreateMiddleware(httpclient.Options{ CustomOptions: map[string]interface{}{ customQueryParametersKey: 64, @@ -63,7 +63,7 @@ func TestCustomQueryParametersMiddleware(t *testing.T) { }) t.Run("With custom query parameters set as empty string should not apply middleware", func(t *testing.T) { - mw := customQueryParametersMiddleware(log.New("test")) + mw := CustomQueryParameters(log.New("test")) rt := mw.CreateMiddleware(httpclient.Options{ CustomOptions: map[string]interface{}{ customQueryParametersKey: "", @@ -87,7 +87,7 @@ func TestCustomQueryParametersMiddleware(t *testing.T) { }) t.Run("With custom query parameters set as invalid query string should not apply middleware", func(t *testing.T) { - mw := customQueryParametersMiddleware(log.New("test")) + mw := CustomQueryParameters(log.New("test")) rt := mw.CreateMiddleware(httpclient.Options{ CustomOptions: map[string]interface{}{ customQueryParametersKey: "custom=%%abc&test=abc", @@ -111,7 +111,7 @@ func TestCustomQueryParametersMiddleware(t *testing.T) { }) t.Run("With custom query parameters set should apply middleware for request URL containing query parameters ", func(t *testing.T) { - mw := customQueryParametersMiddleware(log.New("test")) + mw := CustomQueryParameters(log.New("test")) rt := mw.CreateMiddleware(httpclient.Options{ CustomOptions: map[string]interface{}{ grafanaDataKey: map[string]interface{}{ @@ -143,7 +143,7 @@ func TestCustomQueryParametersMiddleware(t *testing.T) { }) t.Run("With custom query parameters set should apply middleware for request URL not containing query parameters", func(t *testing.T) { - mw := customQueryParametersMiddleware(log.New("test")) + mw := CustomQueryParameters(log.New("test")) rt := mw.CreateMiddleware(httpclient.Options{ CustomOptions: map[string]interface{}{ grafanaDataKey: map[string]interface{}{ diff --git a/pkg/tsdb/prometheus/force_http_get_middleware.go b/pkg/tsdb/prometheus/middleware/force_http_get.go similarity index 91% rename from pkg/tsdb/prometheus/force_http_get_middleware.go rename to pkg/tsdb/prometheus/middleware/force_http_get.go index b3211515506..89061609478 100644 --- a/pkg/tsdb/prometheus/force_http_get_middleware.go +++ b/pkg/tsdb/prometheus/middleware/force_http_get.go @@ -1,4 +1,4 @@ -package prometheus +package middleware import ( "net/http" @@ -7,7 +7,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" ) -func forceHttpGetMiddleware(logger log.Logger) sdkhttpclient.Middleware { +func ForceHttpGet(logger log.Logger) sdkhttpclient.Middleware { return sdkhttpclient.NamedMiddlewareFunc("force-http-get", func(opts sdkhttpclient.Options, next http.RoundTripper) http.RoundTripper { // the prometheus library we use does not allow us to set the http method. // it's behavior is to first try POST, and if it fails in certain ways diff --git a/pkg/tsdb/prometheus/force_http_get_middleware_test.go b/pkg/tsdb/prometheus/middleware/force_http_get_test.go similarity index 51% rename from pkg/tsdb/prometheus/force_http_get_middleware_test.go rename to pkg/tsdb/prometheus/middleware/force_http_get_test.go index c500059b9bd..21cadae7290 100644 --- a/pkg/tsdb/prometheus/force_http_get_middleware_test.go +++ b/pkg/tsdb/prometheus/middleware/force_http_get_test.go @@ -1,4 +1,4 @@ -package prometheus +package middleware import ( "net/http" @@ -9,52 +9,12 @@ import ( "github.com/stretchr/testify/require" ) -func TestForceGet(t *testing.T) { - t.Run("With nil jsonOpts, should not force get-method", func(t *testing.T) { - var jsonOpts map[string]interface{} - require.False(t, forceHttpGet(jsonOpts)) - }) - - t.Run("With empty jsonOpts, should not force get-method", func(t *testing.T) { - jsonOpts := make(map[string]interface{}) - require.False(t, forceHttpGet(jsonOpts)) - }) - - t.Run("With httpMethod=nil, should not not force get-method", func(t *testing.T) { - jsonOpts := map[string]interface{}{ - "httpMethod": nil, - } - require.False(t, forceHttpGet(jsonOpts)) - }) - - t.Run("With httpMethod=post, should not force get-method", func(t *testing.T) { - jsonOpts := map[string]interface{}{ - "httpMethod": "POST", - } - require.False(t, forceHttpGet(jsonOpts)) - }) - - t.Run("With httpMethod=get, should force get-method", func(t *testing.T) { - jsonOpts := map[string]interface{}{ - "httpMethod": "get", - } - require.True(t, forceHttpGet(jsonOpts)) - }) - - t.Run("With httpMethod=GET, should force get-method", func(t *testing.T) { - jsonOpts := map[string]interface{}{ - "httpMethod": "GET", - } - require.True(t, forceHttpGet(jsonOpts)) - }) -} - func TestEnsureHttpMethodMiddleware(t *testing.T) { t.Run("Name should be correct", func(t *testing.T) { finalRoundTripper := httpclient.RoundTripperFunc(func(req *http.Request) (*http.Response, error) { return &http.Response{StatusCode: http.StatusOK}, nil }) - mw := forceHttpGetMiddleware(log.New("test")) + mw := ForceHttpGet(log.New("test")) rt := mw.CreateMiddleware(httpclient.Options{}, finalRoundTripper) require.NotNil(t, rt) middlewareName, ok := mw.(httpclient.MiddlewareName) @@ -67,7 +27,7 @@ func TestEnsureHttpMethodMiddleware(t *testing.T) { return &http.Response{StatusCode: http.StatusOK}, nil }) - mw := forceHttpGetMiddleware(log.New("test")) + mw := ForceHttpGet(log.New("test")) rt := mw.CreateMiddleware(httpclient.Options{}, finalRoundTripper) require.NotNil(t, rt) diff --git a/pkg/tsdb/prometheus/prometheus.go b/pkg/tsdb/prometheus/prometheus.go index 40f12e56929..10807cfa46a 100644 --- a/pkg/tsdb/prometheus/prometheus.go +++ b/pkg/tsdb/prometheus/prometheus.go @@ -6,11 +6,11 @@ import ( "errors" "fmt" "regexp" - "strings" + + "github.com/grafana/grafana/pkg/tsdb/prometheus/client" "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/backend/datasource" - sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" "github.com/grafana/grafana-plugin-sdk-go/backend/instancemgmt" "github.com/grafana/grafana/pkg/infra/httpclient" "github.com/grafana/grafana/pkg/infra/log" @@ -18,7 +18,6 @@ import ( "github.com/grafana/grafana/pkg/plugins/backendplugin/coreplugin" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/tsdb/intervalv2" - "github.com/prometheus/client_golang/api" apiv1 "github.com/prometheus/client_golang/api/prometheus/v1" ) @@ -56,24 +55,6 @@ func ProvideService(cfg *setting.Cfg, httpClientProvider httpclient.Provider, pl return s, nil } -func forceHttpGet(settingsJson map[string]interface{}) bool { - methodInterface, exists := settingsJson["httpMethod"] - if !exists { - return false - } - - method, ok := methodInterface.(string) - if !ok { - return false - } - - if strings.ToLower(method) != "get" { - return false - } - - return true -} - func newInstanceSettings(httpClientProvider httpclient.Provider) datasource.InstanceFactoryFunc { return func(settings backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) { jsonData := map[string]interface{}{} @@ -105,7 +86,7 @@ func newInstanceSettings(httpClientProvider httpclient.Provider) datasource.Inst } } - client, err := createClient(settings.URL, httpCliOpts, httpClientProvider, forceHttpGet(jsonData)) + client, err := client.Create(settings.URL, httpCliOpts, httpClientProvider, jsonData, plog) if err != nil { return nil, err } @@ -143,32 +124,6 @@ func (s *Service) QueryData(ctx context.Context, req *backend.QueryDataRequest) return result, err } -func createClient(url string, httpOpts sdkhttpclient.Options, clientProvider httpclient.Provider, forceHttpGet bool) (apiv1.API, error) { - customParamsMiddleware := customQueryParametersMiddleware(plog) - middlewares := []sdkhttpclient.Middleware{customParamsMiddleware} - if forceHttpGet { - middlewares = append(middlewares, forceHttpGetMiddleware(plog)) - } - httpOpts.Middlewares = middlewares - - roundTripper, err := clientProvider.GetTransport(httpOpts) - if err != nil { - return nil, err - } - - cfg := api.Config{ - Address: url, - RoundTripper: roundTripper, - } - - client, err := api.NewClient(cfg) - if err != nil { - return nil, err - } - - return apiv1.NewAPI(client), nil -} - func (s *Service) getDSInfo(pluginCtx backend.PluginContext) (*DatasourceInfo, error) { i, err := s.im.Get(pluginCtx) if err != nil {