Plugins: Chore: Renamed instrumentation middleware to metrics middleware #3
Conversation
…are (#76186) * Plugins: Chore: Renamed instrumentation middleware to metrics middleware * Removed repeated logger attributes in middleware and contextual logger * renamed loggerParams to logParams * PR review suggestion * Add contextual logger middleware * Removed unused params from logRequest * Removed unwanted changes * Safer FromContext method * Removed traceID from logParams
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR refactors the plugin middleware architecture by renaming "instrumentation middleware" to "metrics middleware" and introducing a dedicated contextual logger middleware. The changes separate concerns that were previously handled by a single middleware component.
The core change renames InstrumentationMiddleware to MetricsMiddleware across the codebase, making its purpose clearer - it exclusively handles Prometheus metrics collection (request counts, durations, and sizes). The middleware file is renamed from instrumentation_middleware.go to metrics_middleware.go, and related test files are updated accordingly.
A new ContextualLoggerMiddleware is introduced to handle the logging responsibilities that were previously mixed into the instrumentation middleware. This new middleware extracts plugin context information (plugin ID, datasource details, user info) and adds it to the logging context using structured logging attributes.
The existing LoggerMiddleware is simplified to focus solely on basic request timing and status logging, removing plugin-specific metadata collection that is now handled by the contextual logger middleware. It now relies on FromContext() to access contextual information added by upstream middlewares.
To support the new contextual logging pattern, the Logger interface is updated with a FromContext(ctx context.Context) Logger method, and implementations in both TestLogger and grafanaInfraLogWrapper are added. The interface also modernizes by replacing interface{} with the Go 1.18+ any type alias.
The middleware chain is reordered in pluginsintegration.go to: MetricsMiddleware → ContextualLoggerMiddleware → LoggerMiddleware, ensuring contextual information is available for downstream logging.
Confidence score: 1/5
• This PR has critical compilation errors that will prevent it from building successfully
• The newly added ContextualLoggerMiddleware references undefined constants (endpointQueryData, endpointCallResource, etc.) that exist in other middleware files but are not imported or defined in the new file
• Additionally, there's an inconsistent test function name in metrics_middleware_test.go that still uses the old "Instrumentation" naming convention
The file pkg/services/pluginsintegration/clientmiddleware/contextual_logger_middleware.go needs immediate attention to resolve the undefined constant references before this can be merged.
8 files reviewed, 5 comments
| clientmiddleware.NewInstrumentationMiddleware(promRegisterer, registry), | ||
| clientmiddleware.NewMetricsMiddleware(promRegisterer, registry), | ||
| clientmiddleware.NewContextualLoggerMiddleware(), | ||
| clientmiddleware.NewLoggerMiddleware(cfg, log.New("plugin.instrumentation")), |
There was a problem hiding this comment.
style: Logger still references 'plugin.instrumentation' in logger name - consider updating to 'plugin.metrics' for consistency
| clientmiddleware.NewLoggerMiddleware(cfg, log.New("plugin.instrumentation")), | |
| clientmiddleware.NewLoggerMiddleware(cfg, log.New("plugin.metrics")), |
| } | ||
|
|
||
| func (m *ContextualLoggerMiddleware) QueryData(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) { | ||
| ctx = instrumentContext(ctx, endpointQueryData, req.PluginContext) |
There was a problem hiding this comment.
logic: endpointQueryData is not defined in this file or imported. This will cause a compilation error.
| } | ||
|
|
||
| func (m *ContextualLoggerMiddleware) CallResource(ctx context.Context, req *backend.CallResourceRequest, sender backend.CallResourceResponseSender) error { | ||
| ctx = instrumentContext(ctx, endpointCallResource, req.PluginContext) |
There was a problem hiding this comment.
logic: endpointCallResource is not defined in this file or imported. This will cause a compilation error.
| } | ||
|
|
||
| func (m *ContextualLoggerMiddleware) CheckHealth(ctx context.Context, req *backend.CheckHealthRequest) (*backend.CheckHealthResult, error) { | ||
| ctx = instrumentContext(ctx, endpointCheckHealth, req.PluginContext) |
There was a problem hiding this comment.
logic: endpointCheckHealth is not defined in this file or imported. This will cause a compilation error.
| } | ||
|
|
||
| func (m *ContextualLoggerMiddleware) CollectMetrics(ctx context.Context, req *backend.CollectMetricsRequest) (*backend.CollectMetricsResult, error) { | ||
| ctx = instrumentContext(ctx, endpointCollectMetrics, req.PluginContext) |
There was a problem hiding this comment.
logic: endpointCollectMetrics is not defined in this file or imported. This will cause a compilation error.
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 2 weeks if no further activity occurs. Please feel free to give a status update or ping for review. Thank you for your contributions! |
|
This pull request has been automatically closed because it has not had any further activity in the last 2 weeks. Thank you for your contributions! |
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
1 participant
Test 3