From 57d606450e9fc1b681481e42a5a8792a1b8b786e Mon Sep 17 00:00:00 2001 From: Deluan Date: Mon, 12 Jan 2026 23:46:38 -0500 Subject: [PATCH] fix: don't record metrics for plugin calls that aren't implemented at all Signed-off-by: Deluan --- plugins/host_scheduler_test.go | 1 + plugins/host_websocket_test.go | 1 + plugins/manager_call.go | 23 ++++++++--------------- plugins/plugins_suite_test.go | 8 +++++++- 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/plugins/host_scheduler_test.go b/plugins/host_scheduler_test.go index 0233ce781..51311f1c4 100644 --- a/plugins/host_scheduler_test.go +++ b/plugins/host_scheduler_test.go @@ -79,6 +79,7 @@ var _ = Describe("SchedulerService", Ordered, func() { plugins: make(map[string]*plugin), ds: dataStore, subsonicRouter: http.NotFoundHandler(), + metrics: noopMetricsRecorder{}, } err = manager.Start(GinkgoT().Context()) Expect(err).ToNot(HaveOccurred()) diff --git a/plugins/host_websocket_test.go b/plugins/host_websocket_test.go index a10cc089f..d359ff27e 100644 --- a/plugins/host_websocket_test.go +++ b/plugins/host_websocket_test.go @@ -71,6 +71,7 @@ var _ = Describe("WebSocketService", Ordered, func() { plugins: make(map[string]*plugin), ds: dataStore, subsonicRouter: http.NotFoundHandler(), + metrics: noopMetricsRecorder{}, } err = manager.Start(GinkgoT().Context()) Expect(err).ToNot(HaveOccurred()) diff --git a/plugins/manager_call.go b/plugins/manager_call.go index 50b1f0797..957d552f7 100644 --- a/plugins/manager_call.go +++ b/plugins/manager_call.go @@ -59,42 +59,35 @@ func callPluginFunction[I any, O any](ctx context.Context, plugin *plugin, funcN startCall := time.Now() exit, output, err := p.CallWithContext(ctx, funcName, inputBytes) + elapsed := time.Since(startCall) if err != nil { - elapsed := time.Since(startCall).Milliseconds() // If context was cancelled, return that error instead of the plugin error if ctx.Err() != nil { - log.Debug(ctx, "Plugin call cancelled", "plugin", plugin.name, "function", funcName, "pluginDuration", time.Since(startCall)) + log.Debug(ctx, "Plugin call cancelled", "plugin", plugin.name, "function", funcName, "pluginDuration", elapsed) return result, ctx.Err() } - if plugin.metrics != nil { - plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, false, elapsed) - } - log.Trace(ctx, "Plugin call failed", "plugin", plugin.name, "function", funcName, "pluginDuration", time.Since(startCall), "navidromeDuration", startCall.Sub(start), err) + plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, false, elapsed.Milliseconds()) + log.Trace(ctx, "Plugin call failed", "plugin", plugin.name, "function", funcName, "pluginDuration", elapsed, "navidromeDuration", startCall.Sub(start), err) return result, fmt.Errorf("plugin call failed: %w", err) } if exit != 0 { - elapsed := time.Since(startCall).Milliseconds() - if plugin.metrics != nil { - plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, false, elapsed) - } if exit == notImplementedCode { + plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, false, elapsed.Milliseconds()) return result, fmt.Errorf("%w: %s", errNotImplemented, funcName) } + plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, false, elapsed.Milliseconds()) return result, fmt.Errorf("plugin call exited with code %d", exit) } if len(output) > 0 { err = json.Unmarshal(output, &result) if err != nil { - log.Trace(ctx, "Plugin call failed", "plugin", plugin.name, "function", funcName, "pluginDuration", time.Since(startCall), "navidromeDuration", startCall.Sub(start), err) + log.Trace(ctx, "Plugin call failed", "plugin", plugin.name, "function", funcName, "pluginDuration", elapsed, "navidromeDuration", startCall.Sub(start), err) } } // Record metrics for successful calls (or JSON unmarshal failures) - if plugin.metrics != nil { - elapsed := time.Since(startCall).Milliseconds() - plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, err == nil, elapsed) - } + plugin.metrics.RecordPluginRequest(ctx, plugin.name, funcName, err == nil, elapsed.Milliseconds()) log.Trace(ctx, "Plugin call succeeded", "plugin", plugin.name, "function", funcName, "pluginDuration", time.Since(startCall), "navidromeDuration", startCall.Sub(start)) return result, err diff --git a/plugins/plugins_suite_test.go b/plugins/plugins_suite_test.go index 459a7fac7..7f2fdf395 100644 --- a/plugins/plugins_suite_test.go +++ b/plugins/plugins_suite_test.go @@ -3,6 +3,7 @@ package plugins import ( + "context" "crypto/sha256" "encoding/hex" "encoding/json" @@ -61,7 +62,7 @@ func createTestManager(pluginConfig map[string]map[string]string) (*Manager, str // and specified plugins. It creates a temp directory, copies the specified plugins, and starts the manager. // Returns the manager and temp directory path. func createTestManagerWithPlugins(pluginConfig map[string]map[string]string, plugins ...string) (*Manager, string) { - return createTestManagerWithPluginsAndMetrics(pluginConfig, nil, plugins...) + return createTestManagerWithPluginsAndMetrics(pluginConfig, noopMetricsRecorder{}, plugins...) } // createTestManagerWithPluginsAndMetrics creates a new plugin Manager with the given plugin config, @@ -155,3 +156,8 @@ var _ = AfterSuite(func() { _ = os.RemoveAll(tmpPluginsDir) } }) + +// noopMetricsRecorder is a no-op implementation of PluginMetricsRecorder for tests +type noopMetricsRecorder struct{} + +func (noopMetricsRecorder) RecordPluginRequest(context.Context, string, string, bool, int64) {}