diff --git a/cmd/pimonitor/main.go b/cmd/pimonitor/main.go index 35bcfb7..799f6bf 100644 --- a/cmd/pimonitor/main.go +++ b/cmd/pimonitor/main.go @@ -47,6 +47,7 @@ func run(args []string) error { if err != nil { return fmt.Errorf("configure alert webhooks: %w", err) } + warnIfNotifierInert(log, cfg.Alerts, notifier) collCfg := collector.Config{ FastInterval: cfg.FastInterval(), @@ -132,6 +133,16 @@ func run(args []string) error { } } +// warnIfNotifierInert logs a startup warning when webhooks are configured +// (notifier is non-nil) but alerts.enabled is false: a disabled alert engine +// never produces the transition events those webhooks would need, so the +// combination silently never fires without this hint. +func warnIfNotifierInert(log *slog.Logger, alerts config.Alerts, notifier *alert.Notifier) { + if notifier != nil && !alerts.Enabled { + log.Warn("alert webhooks configured but alerts.enabled is false — no notifications will be sent") + } +} + func newLogger(level string) *slog.Logger { var lvl slog.Level switch level { diff --git a/cmd/pimonitor/main_test.go b/cmd/pimonitor/main_test.go new file mode 100644 index 0000000..016fb67 --- /dev/null +++ b/cmd/pimonitor/main_test.go @@ -0,0 +1,56 @@ +package main + +import ( + "bytes" + "log/slog" + "testing" + + "github.com/larslaskowski/pimonitor/internal/alert" + "github.com/larslaskowski/pimonitor/internal/config" +) + +func newTestNotifier(t *testing.T) *alert.Notifier { + t.Helper() + n, err := alert.NewNotifier(config.Alerts{ + Webhooks: []config.Webhook{{URL: "http://example.invalid/webhook"}}, + }, nil) + if err != nil { + t.Fatalf("NewNotifier: %v", err) + } + if n == nil { + t.Fatal("expected a non-nil notifier for a configured webhook") + } + return n +} + +func TestWarnIfNotifierInert(t *testing.T) { + tests := []struct { + name string + enabled bool + withNotif bool + wantWarning bool + }{ + {"no webhooks configured, alerts disabled", false, false, false}, + {"webhooks configured, alerts enabled", true, true, false}, + {"webhooks configured, alerts disabled", false, true, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var notifier *alert.Notifier + if tt.withNotif { + notifier = newTestNotifier(t) + } + + var buf bytes.Buffer + log := slog.New(slog.NewTextHandler(&buf, nil)) + + warnIfNotifierInert(log, config.Alerts{Enabled: tt.enabled}, notifier) + + gotWarning := bytes.Contains(buf.Bytes(), []byte("no notifications will be sent")) + if gotWarning != tt.wantWarning { + t.Fatalf("warnIfNotifierInert: got warning=%v, want %v (log: %q)", gotWarning, tt.wantWarning, buf.String()) + } + }) + } +} diff --git a/internal/collector/collector.go b/internal/collector/collector.go index bfc52ee..30a9a8a 100644 --- a/internal/collector/collector.go +++ b/internal/collector/collector.go @@ -92,7 +92,9 @@ type Collector struct { // alerts is nil when alerting is disabled. alerts *alert.Engine - // notifier is nil when no webhooks are configured. + // notifier is nil when no webhooks are configured, or when alerting is + // disabled (a disabled engine never produces events, so starting the + // worker would only leave it idling forever). notifier *alert.Notifier log *slog.Logger @@ -124,13 +126,15 @@ func New(cfg Config, log *slog.Logger) *Collector { log = slog.Default() } var alerts *alert.Engine + var notifier *alert.Notifier if cfg.AlertsEnabled { alerts = alert.New(cfg.Thresholds, cfg.AlertFor) + notifier = cfg.Notifier } c := &Collector{ cfg: cfg, alerts: alerts, - notifier: cfg.Notifier, + notifier: notifier, cpu: NewCPUCollector(), cpuFreq: NewCPUFreqCollector(), loadAvg: NewLoadAvgCollector(), diff --git a/internal/collector/collector_test.go b/internal/collector/collector_test.go index 7185464..419ace5 100644 --- a/internal/collector/collector_test.go +++ b/internal/collector/collector_test.go @@ -5,6 +5,7 @@ import ( "testing" "time" + "github.com/larslaskowski/pimonitor/internal/alert" "github.com/larslaskowski/pimonitor/internal/config" ) @@ -175,6 +176,33 @@ func TestCollector_Alerts_EvaluatedOnFastTick(t *testing.T) { } } +// TestCollector_Notifier_NotWiredWhenAlertsDisabled ensures a notifier built +// from configured webhooks is never wired in while the alert engine is +// disabled, since a disabled engine never produces events to deliver. +func TestCollector_Notifier_NotWiredWhenAlertsDisabled(t *testing.T) { + notifier, err := alert.NewNotifier(config.Alerts{ + Webhooks: []config.Webhook{{URL: "http://example.invalid/webhook"}}, + }, nil) + if err != nil { + t.Fatalf("NewNotifier: %v", err) + } + if notifier == nil { + t.Fatal("expected a non-nil notifier for a configured webhook") + } + + c := New(Config{ + FastInterval: time.Second, + SlowInterval: time.Minute, + HistoryCapacity: 10, + AlertsEnabled: false, + Notifier: notifier, + }, nil) + + if c.notifier != nil { + t.Fatal("expected notifier to stay unwired when AlertsEnabled is false") + } +} + func TestEvictStaleSeries(t *testing.T) { const window = 3 * time.Second now := time.Unix(1_700_000_000, 0)