From d29f4a8f762ac6e9e1f425899c4787fd4025c45d Mon Sep 17 00:00:00 2001 From: Will Browne Date: Thu, 10 Aug 2023 10:32:12 +0200 Subject: [PATCH] Plugins: Add context to StaticRouteResolver and ErrorResolver interfaces (#73121) * add ctx * fix tests --- pkg/api/fakes.go | 2 +- pkg/api/frontend_logging.go | 2 +- .../grafana_javascript_agent_sourcemaps.go | 8 +++++--- pkg/api/frontendlogging/source_maps.go | 13 +++++++------ pkg/api/plugins.go | 4 ++-- pkg/plugins/ifaces.go | 4 ++-- pkg/plugins/manager/loader/loader.go | 8 +------- pkg/plugins/manager/store/store.go | 4 ++-- pkg/plugins/manager/store/store_test.go | 2 +- .../pluginsintegration/pluginerrs/errors.go | 4 ++-- .../pluginsintegration/plugins_integration_test.go | 4 ++-- 11 files changed, 26 insertions(+), 29 deletions(-) diff --git a/pkg/api/fakes.go b/pkg/api/fakes.go index e39a4214028..629bf56eb6c 100644 --- a/pkg/api/fakes.go +++ b/pkg/api/fakes.go @@ -48,6 +48,6 @@ type fakePluginStaticRouteResolver struct { routes []*plugins.StaticRoute } -func (psrr *fakePluginStaticRouteResolver) Routes() []*plugins.StaticRoute { +func (psrr *fakePluginStaticRouteResolver) Routes(_ context.Context) []*plugins.StaticRoute { return psrr.routes } diff --git a/pkg/api/frontend_logging.go b/pkg/api/frontend_logging.go index 470a77cb4e0..7ad646fc815 100644 --- a/pkg/api/frontend_logging.go +++ b/pkg/api/frontend_logging.go @@ -80,7 +80,7 @@ func GrafanaJavascriptAgentLogMessageHandler(store *frontendlogging.SourceMapSto var ctx = frontendlogging.CtxVector{} ctx = event.AddMetaToContext(ctx) exception := exception - transformedException := frontendlogging.TransformException(&exception, store) + transformedException := frontendlogging.TransformException(c.Req.Context(), &exception, store) ctx = append(ctx, "kind", "exception", "type", transformedException.Type, "value", transformedException.Value, "stacktrace", transformedException.String()) ctx = append(ctx, "original_timestamp", exception.Timestamp) frontendLogger.Error(exception.Message(), ctx...) diff --git a/pkg/api/frontendlogging/grafana_javascript_agent_sourcemaps.go b/pkg/api/frontendlogging/grafana_javascript_agent_sourcemaps.go index 5eb7d375899..365493e276d 100644 --- a/pkg/api/frontendlogging/grafana_javascript_agent_sourcemaps.go +++ b/pkg/api/frontendlogging/grafana_javascript_agent_sourcemaps.go @@ -1,7 +1,9 @@ package frontendlogging -// TransformException will attempt to resolved all monified source locations in the stacktrace with original source locations -func TransformException(ex *Exception, store *SourceMapStore) *Exception { +import "context" + +// TransformException will attempt to resolve all modified source locations in the stacktrace with original source locations +func TransformException(ctx context.Context, ex *Exception, store *SourceMapStore) *Exception { if ex.Stacktrace == nil { return ex } @@ -9,7 +11,7 @@ func TransformException(ex *Exception, store *SourceMapStore) *Exception { for _, frame := range ex.Stacktrace.Frames { frame := frame - mappedFrame, err := store.resolveSourceLocation(frame) + mappedFrame, err := store.resolveSourceLocation(ctx, frame) if err != nil { frames = append(frames, frame) } else if mappedFrame != nil { diff --git a/pkg/api/frontendlogging/source_maps.go b/pkg/api/frontendlogging/source_maps.go index d811b107364..0ed72d8a627 100644 --- a/pkg/api/frontendlogging/source_maps.go +++ b/pkg/api/frontendlogging/source_maps.go @@ -1,6 +1,7 @@ package frontendlogging import ( + "context" "io" "net/http" "net/url" @@ -65,7 +66,7 @@ func NewSourceMapStore(cfg *setting.Cfg, routeResolver plugins.StaticRouteResolv * just assumes that a [source filename].map file might exist in the same dir as the source file * and only considers sources coming from grafana core or plugins` */ -func (store *SourceMapStore) guessSourceMapLocation(sourceURL string) (*sourceMapLocation, error) { +func (store *SourceMapStore) guessSourceMapLocation(ctx context.Context, sourceURL string) (*sourceMapLocation, error) { u, err := url.Parse(sourceURL) if err != nil { return nil, err @@ -84,7 +85,7 @@ func (store *SourceMapStore) guessSourceMapLocation(sourceURL string) (*sourceMa } // if source comes from a plugin, look in plugin dir } else if strings.HasPrefix(u.Path, "/public/plugins/") { - for _, route := range store.routeResolver.Routes() { + for _, route := range store.routeResolver.Routes(ctx) { pluginPrefix := filepath.Join("/public/plugins/", route.PluginID) if strings.HasPrefix(u.Path, pluginPrefix) { return &sourceMapLocation{ @@ -98,14 +99,14 @@ func (store *SourceMapStore) guessSourceMapLocation(sourceURL string) (*sourceMa return nil, nil } -func (store *SourceMapStore) getSourceMap(sourceURL string) (*sourceMap, error) { +func (store *SourceMapStore) getSourceMap(ctx context.Context, sourceURL string) (*sourceMap, error) { store.Lock() defer store.Unlock() if smap, ok := store.cache[sourceURL]; ok { return smap, nil } - sourceMapLocation, err := store.guessSourceMapLocation(sourceURL) + sourceMapLocation, err := store.guessSourceMapLocation(ctx, sourceURL) if err != nil { return nil, err } @@ -137,8 +138,8 @@ func (store *SourceMapStore) getSourceMap(sourceURL string) (*sourceMap, error) return smap, nil } -func (store *SourceMapStore) resolveSourceLocation(frame Frame) (*Frame, error) { - smap, err := store.getSourceMap(frame.Filename) +func (store *SourceMapStore) resolveSourceLocation(ctx context.Context, frame Frame) (*Frame, error) { + smap, err := store.getSourceMap(ctx, frame.Filename) if err != nil { return nil, err } diff --git a/pkg/api/plugins.go b/pkg/api/plugins.go index 55c7395620a..0d60aaddf2b 100644 --- a/pkg/api/plugins.go +++ b/pkg/api/plugins.go @@ -431,8 +431,8 @@ func (hs *HTTPServer) CheckHealth(c *contextmodel.ReqContext) response.Response return response.JSON(http.StatusOK, payload) } -func (hs *HTTPServer) GetPluginErrorsList(_ *contextmodel.ReqContext) response.Response { - return response.JSON(http.StatusOK, hs.pluginErrorResolver.PluginErrors()) +func (hs *HTTPServer) GetPluginErrorsList(c *contextmodel.ReqContext) response.Response { + return response.JSON(http.StatusOK, hs.pluginErrorResolver.PluginErrors(c.Req.Context())) } func (hs *HTTPServer) InstallPlugin(c *contextmodel.ReqContext) response.Response { diff --git a/pkg/plugins/ifaces.go b/pkg/plugins/ifaces.go index f9e5e506b45..1ed75c2d2ec 100644 --- a/pkg/plugins/ifaces.go +++ b/pkg/plugins/ifaces.go @@ -118,11 +118,11 @@ type SecretsPluginManager interface { } type StaticRouteResolver interface { - Routes() []*StaticRoute + Routes(ctx context.Context) []*StaticRoute } type ErrorResolver interface { - PluginErrors() []*Error + PluginErrors(ctx context.Context) []*Error } type PluginLoaderAuthorizer interface { diff --git a/pkg/plugins/manager/loader/loader.go b/pkg/plugins/manager/loader/loader.go index 83992709e2e..723ad0dc703 100644 --- a/pkg/plugins/manager/loader/loader.go +++ b/pkg/plugins/manager/loader/loader.go @@ -21,13 +21,7 @@ type Loader struct { log log.Logger } -func ProvideService(discovery discovery.Discoverer, bootstrap bootstrap.Bootstrapper, validation validation.Validator, - initializer initialization.Initializer, termination termination.Terminator) *Loader { - return New(discovery, bootstrap, validation, initializer, termination) -} - -func New( - discovery discovery.Discoverer, bootstrap bootstrap.Bootstrapper, validation validation.Validator, +func New(discovery discovery.Discoverer, bootstrap bootstrap.Bootstrapper, validation validation.Validator, initializer initialization.Initializer, termination termination.Terminator) *Loader { return &Loader{ discovery: discovery, diff --git a/pkg/plugins/manager/store/store.go b/pkg/plugins/manager/store/store.go index e7731def82d..b57d7c88bf8 100644 --- a/pkg/plugins/manager/store/store.go +++ b/pkg/plugins/manager/store/store.go @@ -110,10 +110,10 @@ func (s *Service) availablePlugins(ctx context.Context) []*plugins.Plugin { return res } -func (s *Service) Routes() []*plugins.StaticRoute { +func (s *Service) Routes(ctx context.Context) []*plugins.StaticRoute { staticRoutes := make([]*plugins.StaticRoute, 0) - for _, p := range s.availablePlugins(context.TODO()) { + for _, p := range s.availablePlugins(ctx) { if p.StaticRoute() != nil { staticRoutes = append(staticRoutes, p.StaticRoute()) } diff --git a/pkg/plugins/manager/store/store_test.go b/pkg/plugins/manager/store/store_test.go index 8772ca9824f..9564c270c11 100644 --- a/pkg/plugins/manager/store/store_test.go +++ b/pkg/plugins/manager/store/store_test.go @@ -132,7 +132,7 @@ func TestStore_Routes(t *testing.T) { return &plugins.StaticRoute{PluginID: p.ID, Directory: p.FS.Base()} } - rs := ps.Routes() + rs := ps.Routes(context.Background()) require.Equal(t, []*plugins.StaticRoute{sr(p1), sr(p2), sr(p4), sr(p5)}, rs) }) } diff --git a/pkg/services/pluginsintegration/pluginerrs/errors.go b/pkg/services/pluginsintegration/pluginerrs/errors.go index 47007d93938..d2a425f3f6a 100644 --- a/pkg/services/pluginsintegration/pluginerrs/errors.go +++ b/pkg/services/pluginsintegration/pluginerrs/errors.go @@ -19,8 +19,8 @@ func ProvideStore(signatureErrs SignatureErrorTracker) *Store { } } -func (s *Store) PluginErrors() []*plugins.Error { - sigErrs := s.signatureErrs.SignatureErrors(context.Background()) +func (s *Store) PluginErrors(ctx context.Context) []*plugins.Error { + sigErrs := s.signatureErrs.SignatureErrors(ctx) errs := make([]*plugins.Error, 0, len(sigErrs)) for _, err := range sigErrs { errs = append(errs, &plugins.Error{ diff --git a/pkg/services/pluginsintegration/plugins_integration_test.go b/pkg/services/pluginsintegration/plugins_integration_test.go index 53ff40329cf..8caa1cc2caa 100644 --- a/pkg/services/pluginsintegration/plugins_integration_test.go +++ b/pkg/services/pluginsintegration/plugins_integration_test.go @@ -247,7 +247,7 @@ func verifyBundledPlugins(t *testing.T, ctx context.Context, ps *store.Service) require.NotNil(t, dsPlugins["input"]) pluginRoutes := make(map[string]*plugins.StaticRoute) - for _, r := range ps.Routes() { + for _, r := range ps.Routes(ctx) { pluginRoutes[r.PluginID] = r } @@ -259,7 +259,7 @@ func verifyBundledPlugins(t *testing.T, ctx context.Context, ps *store.Service) func verifyPluginStaticRoutes(t *testing.T, ctx context.Context, rr plugins.StaticRouteResolver, reg registry.Service) { routes := make(map[string]*plugins.StaticRoute) - for _, route := range rr.Routes() { + for _, route := range rr.Routes(ctx) { routes[route.PluginID] = route }