From f0eadd0c7c6c2d2a23b8c2ef5ccaae8f2ae325ca Mon Sep 17 00:00:00 2001 From: Pulse Monitor Date: Wed, 3 Sep 2025 10:04:37 +0000 Subject: [PATCH] fix: storage alerts and UI improvements - Added CheckStorage calls in parallel storage polling (was missing, causing storage alerts to not trigger) - Fixed node cleanup logic to use alert.Node field directly instead of parsing IDs - Removed auto-acknowledge on alert click - now only acknowledge button toggles state - Added unacknowledge button for acknowledged alerts - Fixed double-toggle issue with acknowledge button using race condition prevention - Fixed tab menu width in Alerts and Settings pages (changed flex-shrink-0 to flex-1) addresses #228 (storage alert threshold issue) --- .../src/components/Settings/Settings.tsx | 2 +- frontend-modern/src/pages/Alerts.tsx | 100 ++++++++---------- frontend-modern/src/stores/websocket.ts | 18 +++- internal/alerts/alerts.go | 39 +++---- internal/monitoring/monitor.go | 11 ++ internal/monitoring/monitor_optimized.go | 5 + 6 files changed, 95 insertions(+), 80 deletions(-) diff --git a/frontend-modern/src/components/Settings/Settings.tsx b/frontend-modern/src/components/Settings/Settings.tsx index 0992a9cd5..a506f83e5 100644 --- a/frontend-modern/src/components/Settings/Settings.tsx +++ b/frontend-modern/src/components/Settings/Settings.tsx @@ -766,7 +766,7 @@ const Settings: Component = () => { {(tab) => ( - + }, 100); + } + }} + > + {processingAlerts().has(alert.id) + ? 'Processing...' + : alert.acknowledged + ? 'Unacknowledge' + : 'Acknowledge'} + diff --git a/frontend-modern/src/stores/websocket.ts b/frontend-modern/src/stores/websocket.ts index 6f335b801..bb63c677d 100644 --- a/frontend-modern/src/stores/websocket.ts +++ b/frontend-modern/src/stores/websocket.ts @@ -48,6 +48,9 @@ export function createWebSocketStore(url: string) { const [activeAlerts, setActiveAlerts] = createStore>({}); const [recentlyResolved, setRecentlyResolved] = createStore>({}); const [updateProgress, setUpdateProgress] = createSignal(null); + + // Track alerts with pending acknowledgment changes to prevent race conditions + const pendingAckChanges = new Set(); let ws: WebSocket | null = null; let reconnectTimeout: number; @@ -179,8 +182,13 @@ export function createWebSocketStore(url: string) { } }); - // Add new alerts + // Add new alerts (skip those with pending ack changes) Object.entries(newAlerts).forEach(([id, alert]) => { + // Skip updating if this alert has a pending acknowledgment change + if (pendingAckChanges.has(id)) { + logger.debug(`Skipping update for alert ${id} - has pending ack change`); + return; + } setActiveAlerts(id, alert); }); @@ -381,6 +389,14 @@ export function createWebSocketStore(url: string) { updateAlert: (alertId: string, updates: Partial) => { const existingAlert = activeAlerts[alertId]; if (existingAlert) { + // Track this alert as having pending changes if acknowledgment is changing + if ('acknowledged' in updates) { + pendingAckChanges.add(alertId); + // Clear the pending flag after a delay to allow server sync + setTimeout(() => { + pendingAckChanges.delete(alertId); + }, 2000); // 2 seconds should be enough for server to sync + } setActiveAlerts(alertId, { ...existingAlert, ...updates }); } } diff --git a/internal/alerts/alerts.go b/internal/alerts/alerts.go index 2494994d6..5a7475daa 100644 --- a/internal/alerts/alerts.go +++ b/internal/alerts/alerts.go @@ -684,6 +684,16 @@ func (m *Manager) CheckStorage(storage models.Storage) { // Check usage if storage has valid data (even if not currently active on this node) // In clusters, storage may show as inactive on nodes where it's not currently mounted // but we still want to alert on high usage + log.Info(). + Str("storage", storage.Name). + Str("id", storage.ID). + Float64("usage", storage.Usage). + Str("status", storage.Status). + Float64("trigger", threshold.Trigger). + Float64("clear", threshold.Clear). + Bool("hasOverride", hasOverride). + Msg("Checking storage thresholds") + if storage.Status != "offline" && storage.Status != "unavailable" && storage.Usage > 0 { m.checkMetric(storage.ID, storage.Name, storage.Node, storage.Instance, "Storage", "usage", storage.Usage, &threshold) } @@ -1975,32 +1985,11 @@ func (m *Manager) CleanupAlertsForNodes(existingNodes map[string]bool) { Msg("Starting alert cleanup for non-existent nodes") removedCount := 0 - for alertID := range m.activeAlerts { - var node string + for alertID, alert := range m.activeAlerts { + // Use the Node field from the alert itself, which is more reliable + node := alert.Node - // Extract node from alert ID - // Format can be either "node:type/id-metric" or "node-storage-name-usage" - if strings.Contains(alertID, ":") { - // Guest alert format: "node:type/id-metric" - parts := strings.Split(alertID, ":") - if len(parts) >= 2 { - node = parts[0] - } - } else if strings.Contains(alertID, "-storage-") { - // Storage alert format: "node-storage-name-usage" - parts := strings.Split(alertID, "-storage-") - if len(parts) >= 1 { - node = parts[0] - } - } else if strings.HasPrefix(alertID, "node-offline-") { - // Node offline alert format: "node-offline-node/nodename" - // Extract the node name after the last slash - if idx := strings.LastIndex(alertID, "/"); idx != -1 { - node = alertID[idx+1:] - } - } - - // If we couldn't extract a node or the node doesn't exist, remove the alert + // If we couldn't get a node or the node doesn't exist, remove the alert if node == "" || !existingNodes[node] { delete(m.activeAlerts, alertID) removedCount++ diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index 8d6c2f90f..8e42110b0 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -2011,6 +2011,12 @@ func (m *Monitor) pollStorageWithNodes(ctx context.Context, instanceName string, m.metricsHistory.AddStorageMetric(modelStorage.ID, "avail", float64(modelStorage.Free), now) // Check thresholds for alerts + log.Info(). + Str("storage", modelStorage.Name). + Str("id", modelStorage.ID). + Float64("usage", modelStorage.Usage). + Str("status", modelStorage.Status). + Msg("Calling CheckStorage for storage device") m.alertManager.CheckStorage(modelStorage) } } @@ -2982,7 +2988,12 @@ func (m *Monitor) checkMockAlerts() { } // Check alerts for storage + log.Info().Int("storageCount", len(state.Storage)).Msg("Checking storage alerts") for _, storage := range state.Storage { + log.Debug(). + Str("name", storage.Name). + Float64("usage", storage.Usage). + Msg("Checking storage for alerts") m.alertManager.CheckStorage(storage) } } diff --git a/internal/monitoring/monitor_optimized.go b/internal/monitoring/monitor_optimized.go index c91a1133e..b612a5cd9 100644 --- a/internal/monitoring/monitor_optimized.go +++ b/internal/monitoring/monitor_optimized.go @@ -582,6 +582,11 @@ func (m *Monitor) pollStorageWithNodesOptimized(ctx context.Context, instanceNam } } + // Check alerts for all storage devices + for _, storage := range allStorage { + m.alertManager.CheckStorage(storage) + } + // Update state with all storage m.state.UpdateStorageForInstance(instanceName, allStorage)