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
|
cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Bugbot free trial expires on August 11, 2025
Learn more in the Cursor dashboard.
|
|
||
| func (m *ContextualLoggerMiddleware) CollectMetrics(ctx context.Context, req *backend.CollectMetricsRequest) (*backend.CollectMetricsResult, error) { | ||
| ctx = instrumentContext(ctx, endpointCollectMetrics, req.PluginContext) | ||
| return m.next.CollectMetrics(ctx, req) |
There was a problem hiding this comment.
Bug: Middleware Methods Panic on Nil Requests
The ContextualLoggerMiddleware methods (QueryData, CallResource, CheckHealth, CollectMetrics) panic when a nil request is received. This occurs because they directly access req.PluginContext (via the instrumentContext function) without first checking if req is nil. This is a regression, as previous middleware layers gracefully handled nil requests.
Locations (1)
| } | ||
|
|
||
| m.logger.Info("Plugin Request Completed", logParams...) | ||
| m.logger.FromContext(ctx).Info("Plugin Request Completed", logParams...) |
There was a problem hiding this comment.
Bug: Plugin Requests Missing TraceID in Logs
The traceID is no longer logged for plugin requests. During a refactoring, the tracing import and the logic to extract and add traceID from the context to log parameters were removed from the LoggerMiddleware. The newly introduced ContextualLoggerMiddleware does not add this information, resulting in missing traceID in plugin request logs and impacting debugging and request tracing capabilities.
Locations (3)
|
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! |
2 participants
Test 3