Plugins: Chore: Renamed instrumentation middleware to metrics middleware

[marchugon]

What is this feature?

• Renames the InstrumentationMiddleware to MetricsMiddleware
• Moves the contextual logger from the Instrumentation/Metrics middleware to the LoggerMiddleware

Why do we need this feature?

Better naming and consistency.

Who is this feature for?

Plugins platform.

Which issue(s) does this PR fix?:

Fixes #

Special notes for your reviewer:

Please check that:

• It works as expected from a user's perspective.
• If this is a pre-GA feature, it is behind a feature toggle.
• The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

[friendlyreviewer-staging]

Hi there 👋

⛅ Tech	🌤️ Feat
1 medium, 5 low	1 ok, 2 mitigated

The MR cleanly accomplishes both stated goals: the InstrumentationMiddleware has been renamed to MetricsMiddleware (file, package, constructor and wiring all updated, with no remaining NewInstrumentationMiddleware calls), and the contextual-logger concern has been moved out of the metrics middleware into LoggerMiddleware, which now obtains plugin context via Logger.FromContext(ctx). The middleware ordering in pluginsintegration.go (Tracing → Metrics → ContextualLogger → Logger) is correct, and the new metrics tests are well-structured (fresh registry per subtest, correct endpoint/status/size assertions).

The review found no functional regressions — the requested rename and logger relocation behave as intended. However, several small consistency and robustness issues remain: two stale references to the old Instrumentation name survive (a test function name and the base logger scope), the fake TestLogger.FromContext returns a disconnected logger that can silently break test assertions on log output, the real FromContext contains a dead fallback branch from a guaranteed-succeeding type assertion, LoggerMiddleware now has an implicit, ordering-sensitive dependency on ContextualLoggerMiddleware that would silently drop fields if reordered, and a copy-pasted doc comment on the interface. None of these are blocking, but they are worth tidying up.

---

Worth checking

• 🟡 pkg/plugins/log/fake.go (L46)
  TestLogger.FromContext returns a brand-new disconnected TestLogger, discarding the receiver. The real implementation shares the underlying sink with the receiver, so this divergence is a silent testability trap: with the middleware now logging via m.logger.FromContext(ctx).Info(...), tests that set mw.logger = log.NewTestLogger() and then assert on mw.logger.InfoLogs will see zero calls because the log goes to the fresh, unreferenced instance. Prefer returning f itself (mirroring real semantics) or documenting that a fresh instance is intentional.

Small things (take or leave)

• 🔵 pkg/services/pluginsintegration/clientmiddleware/logger_middleware.go (L58)
  logRequest now relies entirely on m.logger.FromContext(ctx) for endpoint/pluginId/dsName/dsUID/uname instead of receiving pluginCtx/endpoint directly. This works today only because ContextualLoggerMiddleware unconditionally precedes LoggerMiddleware; if the two are ever reordered or the contextual middleware is removed while PluginLogBackendRequests stays enabled, those fields would silently vanish from the "Plugin Request Completed" line with no warning. Consider an integration test or documented ordering requirement (not blocking given current wiring).
• 🔵 pkg/plugins/log/logger.go (L48)
  In grafanaInfraLogWrapper.FromContext, the type assertion d.l.FromContext(ctx).(*log.ConcreteLogger) can never fail because d.l is a *log.ConcreteLogger and ConcreteLogger.FromContext always returns a *ConcreteLogger. The if !ok { return d.New() } fallback is dead code and misleading; simplify to a direct assertion, or keep it only with documentation if defense against a future interface return type is desired.
• 🔵 pkg/services/pluginsintegration/clientmiddleware/metrics_middleware_test.go (L21)
  The test function is still named TestInstrumentationMiddleware, referencing the pre-rename symbol that no longer exists. This is the only remaining Instrumentation* reference in the package. Rename it to TestMetricsMiddleware for consistency with the rename.
• 🔵 pkg/services/pluginsintegration/pluginsintegration.go (L161)
  clientmiddleware.NewLoggerMiddleware(cfg, log.New("plugin.instrumentation")) still uses the logger scope/name "plugin.instrumentation", a leftover from the pre-rename instrumentation era. Given the rename intent, rename it to something like "plugin.metrics" or "plugin.request-log". Cosmetic and non-blocking since the runtime FromContext path makes the base name largely irrelevant to output.
• 🔵 pkg/plugins/log/ifaces.go (L22)
  The doc comment // FromContext returns a new contextual Logger that has this logger's context plus the given context. is a verbatim copy of the New comment and is semantically muddled for FromContext, which derives context from a context.Context rather than variadic key/value args. Clarify it, e.g. "returns a Logger carrying this logger's context plus the attributes registered in the given context.Context".

---

✅ Feature-level checklist

Looks good

• ✅ Move the contextual logger from the Instrumentation/Metrics middleware to the LoggerMiddleware.
  Verified end-to-end: the contextual-logger middleware stores attributes via log.WithContextualAttributes, LoggerMiddleware reads them back through m.logger.FromContext(ctx).Info(...), and MetricsMiddleware contains no logger logic. Middleware ordering in pluginsintegration.go guarantees the contextual logger is available when LoggerMiddleware logs.

Partially covered

• ⚠️ Rename the InstrumentationMiddleware to MetricsMiddleware.
  The rename is functionally complete: file/package/constructor/wiring are all updated and no NewInstrumentationMiddleware call remains. However, two stale references to the old name survive (the TestInstrumentationMiddleware test function in metrics_middleware_test.go and the "plugin.instrumentation" base logger name in pluginsintegration.go).
• ⚠️ No stale references to the old Instrumentation/InstrumentationMiddleware name remain in the codebase.
  Grep confirms the only remaining Instrumentation* references are the stale TestInstrumentationMiddleware test function name and the "plugin.instrumentation" base logger scope — both cosmetic.

Review time: 4m 51s