From eae9bfe4bc0c6450c93200aa4b16f2bb3847c07d Mon Sep 17 00:00:00 2001 From: ismail simsek Date: Fri, 15 Mar 2024 13:37:29 +0100 Subject: [PATCH] Chore: Promlib allows extendOptions to be nil (#84463) * use logger from service * allow extendOptions to be nil * Update logger --- pkg/promlib/healthcheck.go | 7 ++- pkg/promlib/healthcheck_test.go | 8 +++- pkg/promlib/heuristics.go | 6 ++- pkg/promlib/heuristics_test.go | 8 +++- pkg/promlib/library.go | 8 ++-- pkg/promlib/library_test.go | 81 ++++++++++++++++++++++----------- 6 files changed, 79 insertions(+), 39 deletions(-) diff --git a/pkg/promlib/healthcheck.go b/pkg/promlib/healthcheck.go index c2e2de4224c..c3849398684 100644 --- a/pkg/promlib/healthcheck.go +++ b/pkg/promlib/healthcheck.go @@ -16,11 +16,8 @@ const ( refID = "__healthcheck__" ) -var logger = backend.NewLoggerWith("logger", "tsdb.prometheus") - func (s *Service) CheckHealth(ctx context.Context, req *backend.CheckHealthRequest) (*backend.CheckHealthResult, error) { - logger := logger.FromContext(ctx) ds, err := s.getInstance(ctx, req.PluginContext) // check that the datasource exists @@ -32,13 +29,15 @@ func (s *Service) CheckHealth(ctx context.Context, req *backend.CheckHealthReque return getHealthCheckMessage("", errors.New("invalid datasource info received")) } + logger := s.logger.FromContext(ctx) + hc, err := healthcheck(ctx, req, ds) if err != nil { logger.Warn("Error performing prometheus healthcheck", "err", err.Error()) return nil, err } - heuristics, err := getHeuristics(ctx, ds) + heuristics, err := getHeuristics(ctx, ds, logger) if err != nil { logger.Warn("Failed to get prometheus heuristics", "err", err.Error()) } else { diff --git a/pkg/promlib/healthcheck_test.go b/pkg/promlib/healthcheck_test.go index 20e06edd6b7..c60fc074726 100644 --- a/pkg/promlib/healthcheck_test.go +++ b/pkg/promlib/healthcheck_test.go @@ -82,8 +82,10 @@ func getMockProvider[T http.RoundTripper]() *sdkhttpclient.Provider { func Test_healthcheck(t *testing.T) { t.Run("should do a successful health check", func(t *testing.T) { httpProvider := getMockProvider[*healthCheckSuccessRoundTripper]() + logger := backend.NewLoggerWith("logger", "test") s := &Service{ - im: datasource.NewInstanceManager(newInstanceSettings(httpProvider, backend.NewLoggerWith("logger", "test"), mockExtendClientOpts)), + im: datasource.NewInstanceManager(newInstanceSettings(httpProvider, logger, mockExtendClientOpts)), + logger: logger, } req := &backend.CheckHealthRequest{ @@ -98,8 +100,10 @@ func Test_healthcheck(t *testing.T) { t.Run("should return an error for an unsuccessful health check", func(t *testing.T) { httpProvider := getMockProvider[*healthCheckFailRoundTripper]() + logger := backend.NewLoggerWith("logger", "test") s := &Service{ - im: datasource.NewInstanceManager(newInstanceSettings(httpProvider, backend.NewLoggerWith("logger", "test"), mockExtendClientOpts)), + im: datasource.NewInstanceManager(newInstanceSettings(httpProvider, logger, mockExtendClientOpts)), + logger: logger, } req := &backend.CheckHealthRequest{ diff --git a/pkg/promlib/heuristics.go b/pkg/promlib/heuristics.go index c0c3f1e318f..e97a1cd7e5d 100644 --- a/pkg/promlib/heuristics.go +++ b/pkg/promlib/heuristics.go @@ -8,6 +8,7 @@ import ( "net/http" "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana-plugin-sdk-go/backend/log" ) const ( @@ -85,10 +86,11 @@ func (s *Service) GetHeuristics(ctx context.Context, req HeuristicsRequest) (*He if err != nil { return nil, err } - return getHeuristics(ctx, ds) + logger := s.logger.FromContext(ctx) + return getHeuristics(ctx, ds, logger) } -func getHeuristics(ctx context.Context, i *instance) (*Heuristics, error) { +func getHeuristics(ctx context.Context, i *instance, logger log.Logger) (*Heuristics, error) { heuristics := Heuristics{ Application: "unknown", Features: Features{ diff --git a/pkg/promlib/heuristics_test.go b/pkg/promlib/heuristics_test.go index 3ed20a65553..48b6f380ad8 100644 --- a/pkg/promlib/heuristics_test.go +++ b/pkg/promlib/heuristics_test.go @@ -52,8 +52,10 @@ func Test_GetHeuristics(t *testing.T) { status: http.StatusOK, } httpProvider := newHeuristicsSDKProvider(rt) + logger := backend.NewLoggerWith("logger", "test") s := &Service{ - im: datasource.NewInstanceManager(newInstanceSettings(httpProvider, backend.NewLoggerWith("logger", "test"), mockExtendClientOpts)), + im: datasource.NewInstanceManager(newInstanceSettings(httpProvider, logger, mockExtendClientOpts)), + logger: logger, } req := HeuristicsRequest{ @@ -72,8 +74,10 @@ func Test_GetHeuristics(t *testing.T) { status: http.StatusOK, } httpProvider := newHeuristicsSDKProvider(rt) + logger := backend.NewLoggerWith("logger", "test") s := &Service{ - im: datasource.NewInstanceManager(newInstanceSettings(httpProvider, backend.NewLoggerWith("logger", "test"), mockExtendClientOpts)), + im: datasource.NewInstanceManager(newInstanceSettings(httpProvider, logger, mockExtendClientOpts)), + logger: logger, } req := HeuristicsRequest{ diff --git a/pkg/promlib/library.go b/pkg/promlib/library.go index c8316dbe750..1a441e9305e 100644 --- a/pkg/promlib/library.go +++ b/pkg/promlib/library.go @@ -52,9 +52,11 @@ func newInstanceSettings(httpClientProvider *sdkhttpclient.Provider, log log.Log return nil, fmt.Errorf("error creating transport options: %v", err) } - err = extendOptions(ctx, settings, opts) - if err != nil { - return nil, fmt.Errorf("error extending transport options: %v", err) + if extendOptions != nil { + err = extendOptions(ctx, settings, opts) + if err != nil { + return nil, fmt.Errorf("error extending transport options: %v", err) + } } httpClient, err := httpClientProvider.New(*opts) diff --git a/pkg/promlib/library_test.go b/pkg/promlib/library_test.go index cf789c43f60..04529d608d0 100644 --- a/pkg/promlib/library_test.go +++ b/pkg/promlib/library_test.go @@ -2,6 +2,7 @@ package promlib import ( "context" + "fmt" "io" "net/http" "testing" @@ -71,32 +72,7 @@ func TestService(t *testing.T) { httpProvider := getMockPromTestSDKProvider(f) service := NewService(httpProvider, backend.NewLoggerWith("logger", "test"), mockExtendTransportOptions) - req := &backend.CallResourceRequest{ - PluginContext: backend.PluginContext{ - OrgID: 0, - PluginID: "prometheus", - User: nil, - AppInstanceSettings: nil, - DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{ - ID: 0, - UID: "", - Type: "prometheus", - Name: "test-prom", - URL: "http://localhost:9090", - User: "", - Database: "", - BasicAuthEnabled: true, - BasicAuthUser: "admin", - Updated: time.Time{}, - JSONData: []byte("{}"), - }, - }, - Path: "/api/v1/series", - Method: http.MethodPost, - URL: "/api/v1/series", - Body: []byte("match%5B%5D: ALERTS\nstart: 1655271408\nend: 1655293008"), - } - + req := mockRequest() sender := &fakeSender{} err := service.CallResource(context.Background(), req, sender) require.NoError(t, err) @@ -115,4 +91,57 @@ func TestService(t *testing.T) { }) }) }) + + t.Run("no extendOptions function provided", func(t *testing.T) { + f := &fakeHTTPClientProvider{} + httpProvider := getMockPromTestSDKProvider(f) + service := NewService(httpProvider, backend.NewLoggerWith("logger", "test"), nil) + require.NotNil(t, service) + require.NotNil(t, service.im) + }) + + t.Run("extendOptions function provided", func(t *testing.T) { + f := &fakeHTTPClientProvider{} + httpProvider := getMockPromTestSDKProvider(f) + service := NewService(httpProvider, backend.NewLoggerWith("logger", "test"), func(ctx context.Context, settings backend.DataSourceInstanceSettings, clientOpts *sdkhttpclient.Options) error { + fmt.Println(ctx, settings, clientOpts) + require.NotNil(t, ctx) + require.NotNil(t, settings) + require.Equal(t, "test-prom", settings.Name) + return nil + }) + + req := mockRequest() + sender := &fakeSender{} + err := service.CallResource(context.Background(), req, sender) + require.NoError(t, err) + }) +} + +func mockRequest() *backend.CallResourceRequest { + return &backend.CallResourceRequest{ + PluginContext: backend.PluginContext{ + OrgID: 0, + PluginID: "prometheus", + User: nil, + AppInstanceSettings: nil, + DataSourceInstanceSettings: &backend.DataSourceInstanceSettings{ + ID: 0, + UID: "", + Type: "prometheus", + Name: "test-prom", + URL: "http://localhost:9090", + User: "", + Database: "", + BasicAuthEnabled: true, + BasicAuthUser: "admin", + Updated: time.Time{}, + JSONData: []byte("{}"), + }, + }, + Path: "/api/v1/series", + Method: http.MethodPost, + URL: "/api/v1/series", + Body: []byte("match%5B%5D: ALERTS\nstart: 1655271408\nend: 1655293008"), + } }