Chore: Remove unused dependenices in plugin client middlewares (#84624)

* Chore: Remove unused dependenices in plugin client middlewares

* refactor logger middleware to remove cfg dependency

* hack to make tracing work in api group builders
This commit is contained in:
Marcus Efraimsson 2024-03-22 12:54:35 +01:00 committed by GitHub
parent 41bf6a92fd
commit 488bbaacab
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 54 additions and 34 deletions

View File

@ -9,6 +9,7 @@ import (
"github.com/grafana/grafana/pkg/cmd/grafana-server/commands"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/server"
"github.com/grafana/grafana/pkg/services/apiserver/standalone"
)
@ -47,8 +48,14 @@ func newCommandStartExampleAPIServer(o *APIServerOptions, stopCh <-chan struct{}
return err
}
// Currently TracingOptions.ApplyTo, which will configure/initialize tracing,
// happens after loadAPIGroupBuilders. Hack to workaround this for now to allow
// the tracer to be initialized at a later stage, when tracer is available.
// TODO: Fix so that TracingOptions.ApplyTo happens before or during loadAPIGroupBuilders.
tracer := newLateInitializedTracingService()
// Load each group from the args
if err := o.loadAPIGroupBuilders(apis); err != nil {
if err := o.loadAPIGroupBuilders(tracer, apis); err != nil {
return err
}
@ -62,6 +69,10 @@ func newCommandStartExampleAPIServer(o *APIServerOptions, stopCh <-chan struct{}
return err
}
if o.Options.TracingOptions.TracingService != nil {
tracer.InitTracer(o.Options.TracingOptions.TracingService)
}
if err := o.RunAPIServer(config, stopCh); err != nil {
return err
}
@ -87,3 +98,19 @@ func RunCLI(opts commands.ServerOptions) int {
return cli.Run(cmd)
}
type lateInitializedTracingService struct {
tracing.Tracer
}
func newLateInitializedTracingService() *lateInitializedTracingService {
return &lateInitializedTracingService{
Tracer: tracing.InitializeTracerForTest(),
}
}
func (s *lateInitializedTracingService) InitTracer(tracer tracing.Tracer) {
s.Tracer = tracer
}
var _ tracing.Tracer = &lateInitializedTracingService{}

View File

@ -14,6 +14,7 @@ import (
"github.com/grafana/grafana/pkg/apiserver/builder"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/tracing"
grafanaAPIServer "github.com/grafana/grafana/pkg/services/apiserver"
"github.com/grafana/grafana/pkg/services/apiserver/standalone"
standaloneoptions "github.com/grafana/grafana/pkg/services/apiserver/standalone/options"
@ -50,10 +51,10 @@ func newAPIServerOptions(out, errOut io.Writer) *APIServerOptions {
}
}
func (o *APIServerOptions) loadAPIGroupBuilders(apis []schema.GroupVersion) error {
func (o *APIServerOptions) loadAPIGroupBuilders(tracer tracing.Tracer, apis []schema.GroupVersion) error {
o.builders = []builder.APIGroupBuilder{}
for _, gv := range apis {
api, err := o.factory.MakeAPIServer(gv)
api, err := o.factory.MakeAPIServer(tracer, gv)
if err != nil {
return err
}

View File

@ -34,7 +34,7 @@ type APIServerFactory interface {
GetEnabled(runtime []RuntimeConfig) ([]schema.GroupVersion, error)
// Make an API server for a given group+version
MakeAPIServer(gv schema.GroupVersion) (builder.APIGroupBuilder, error)
MakeAPIServer(tracer tracing.Tracer, gv schema.GroupVersion) (builder.APIGroupBuilder, error)
}
// Zero dependency provider for testing
@ -62,7 +62,7 @@ func (p *DummyAPIFactory) GetEnabled(runtime []RuntimeConfig) ([]schema.GroupVer
return gv, nil
}
func (p *DummyAPIFactory) MakeAPIServer(gv schema.GroupVersion) (builder.APIGroupBuilder, error) {
func (p *DummyAPIFactory) MakeAPIServer(tracer tracing.Tracer, gv schema.GroupVersion) (builder.APIGroupBuilder, error) {
if gv.Version != "v0alpha1" {
return nil, fmt.Errorf("only alpha supported now")
}
@ -79,9 +79,9 @@ func (p *DummyAPIFactory) MakeAPIServer(gv schema.GroupVersion) (builder.APIGrou
Client: client.NewTestDataClient(),
},
client.NewTestDataRegistry(),
nil, // legacy lookup
prometheus.NewRegistry(), // ???
tracing.InitializeTracerForTest(), // ???
nil, // legacy lookup
prometheus.NewRegistry(), // ???
tracer,
)
case "featuretoggle.grafana.app":

View File

@ -10,30 +10,22 @@ import (
"github.com/grafana/grafana/pkg/plugins"
plog "github.com/grafana/grafana/pkg/plugins/log"
"github.com/grafana/grafana/pkg/plugins/pluginrequestmeta"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/setting"
)
// NewLoggerMiddleware creates a new plugins.ClientMiddleware that will
// log requests.
func NewLoggerMiddleware(cfg *setting.Cfg, logger plog.Logger, features featuremgmt.FeatureToggles) plugins.ClientMiddleware {
func NewLoggerMiddleware(logger plog.Logger) plugins.ClientMiddleware {
return plugins.ClientMiddlewareFunc(func(next plugins.Client) plugins.Client {
if !cfg.PluginLogBackendRequests {
return next
}
return &LoggerMiddleware{
next: next,
logger: logger,
features: features,
next: next,
logger: logger,
}
})
}
type LoggerMiddleware struct {
next plugins.Client
logger plog.Logger
features featuremgmt.FeatureToggles
next plugins.Client
logger plog.Logger
}
func (m *LoggerMiddleware) logRequest(ctx context.Context, fn func(ctx context.Context) (requestStatus, error)) error {

View File

@ -11,7 +11,6 @@ import (
"github.com/grafana/grafana/pkg/plugins"
"github.com/grafana/grafana/pkg/plugins/manager/registry"
"github.com/grafana/grafana/pkg/plugins/pluginrequestmeta"
"github.com/grafana/grafana/pkg/services/featuremgmt"
)
// pluginMetrics contains the prometheus metrics used by the MetricsMiddleware.
@ -27,11 +26,10 @@ type pluginMetrics struct {
type MetricsMiddleware struct {
pluginMetrics
pluginRegistry registry.Service
features featuremgmt.FeatureToggles
next plugins.Client
}
func newMetricsMiddleware(promRegisterer prometheus.Registerer, pluginRegistry registry.Service, features featuremgmt.FeatureToggles) *MetricsMiddleware {
func newMetricsMiddleware(promRegisterer prometheus.Registerer, pluginRegistry registry.Service) *MetricsMiddleware {
additionalLabels := []string{"status_source"}
pluginRequestCounter := prometheus.NewCounterVec(prometheus.CounterOpts{
Namespace: "grafana",
@ -72,13 +70,12 @@ func newMetricsMiddleware(promRegisterer prometheus.Registerer, pluginRegistry r
pluginRequestDurationSeconds: pluginRequestDurationSeconds,
},
pluginRegistry: pluginRegistry,
features: features,
}
}
// NewMetricsMiddleware returns a new MetricsMiddleware.
func NewMetricsMiddleware(promRegisterer prometheus.Registerer, pluginRegistry registry.Service, features featuremgmt.FeatureToggles) plugins.ClientMiddleware {
imw := newMetricsMiddleware(promRegisterer, pluginRegistry, features)
func NewMetricsMiddleware(promRegisterer prometheus.Registerer, pluginRegistry registry.Service) plugins.ClientMiddleware {
imw := newMetricsMiddleware(promRegisterer, pluginRegistry)
return plugins.ClientMiddlewareFunc(func(next plugins.Client) plugins.Client {
imw.next = next
return imw

View File

@ -17,7 +17,6 @@ import (
"github.com/grafana/grafana/pkg/plugins/manager/client/clienttest"
"github.com/grafana/grafana/pkg/plugins/manager/fakes"
"github.com/grafana/grafana/pkg/plugins/pluginrequestmeta"
"github.com/grafana/grafana/pkg/services/featuremgmt"
)
const (
@ -76,7 +75,7 @@ func TestInstrumentationMiddleware(t *testing.T) {
JSONData: plugins.JSONData{ID: pluginID, Backend: true},
}))
mw := newMetricsMiddleware(promRegistry, pluginsRegistry, featuremgmt.WithFeatures())
mw := newMetricsMiddleware(promRegistry, pluginsRegistry)
cdt := clienttest.NewClientDecoratorTest(t, clienttest.WithMiddlewares(
plugins.ClientMiddlewareFunc(func(next plugins.Client) plugins.Client {
mw.next = next
@ -153,7 +152,7 @@ func TestInstrumentationMiddlewareStatusSource(t *testing.T) {
require.NoError(t, pluginsRegistry.Add(context.Background(), &plugins.Plugin{
JSONData: plugins.JSONData{ID: pluginID, Backend: true},
}))
metricsMw := newMetricsMiddleware(promRegistry, pluginsRegistry, featuremgmt.WithFeatures())
metricsMw := newMetricsMiddleware(promRegistry, pluginsRegistry)
cdt := clienttest.NewClientDecoratorTest(t, clienttest.WithMiddlewares(
NewPluginRequestMetaMiddleware(),
plugins.ClientMiddlewareFunc(func(next plugins.Client) plugins.Client {

View File

@ -160,14 +160,18 @@ func NewClientDecorator(
func CreateMiddlewares(cfg *setting.Cfg, oAuthTokenService oauthtoken.OAuthTokenService, tracer tracing.Tracer, cachingService caching.CachingService, features *featuremgmt.FeatureManager, promRegisterer prometheus.Registerer, registry registry.Service) []plugins.ClientMiddleware {
middlewares := []plugins.ClientMiddleware{
clientmiddleware.NewPluginRequestMetaMiddleware(),
clientmiddleware.NewTracingMiddleware(tracer),
clientmiddleware.NewMetricsMiddleware(promRegisterer, registry),
clientmiddleware.NewContextualLoggerMiddleware(),
}
if cfg.PluginLogBackendRequests {
middlewares = append(middlewares, clientmiddleware.NewLoggerMiddleware(log.New("plugin.instrumentation")))
}
skipCookiesNames := []string{cfg.LoginCookieName}
middlewares = append(middlewares,
clientmiddleware.NewTracingMiddleware(tracer),
clientmiddleware.NewMetricsMiddleware(promRegisterer, registry, features),
clientmiddleware.NewContextualLoggerMiddleware(),
clientmiddleware.NewLoggerMiddleware(cfg, log.New("plugin.instrumentation"), features),
clientmiddleware.NewTracingHeaderMiddleware(),
clientmiddleware.NewClearAuthHeadersMiddleware(),
clientmiddleware.NewOAuthTokenMiddleware(oAuthTokenService),