diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index 20fd82c066..5a0febf39b 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -461,6 +461,20 @@ operator-visible message evidence. Window changes never change the canonical `metric-threshold:` identity, so start time, acknowledgement, history, timeline, escalation, and recovery stay one occurrence. +Current metric observations must be finite in both instantaneous and rolling +evaluation, including a manager without a history provider. After selecting +in-range observations and last-duplicate authority, any non-finite History +sample or non-finite weighted result makes the whole window unknown. Invalid +samples are not dropped to manufacture coverage, substituted with an older +duplicate, clamped, or treated as zero. Both legacy and canonical evaluation +retain the last trusted incident value, identity, acknowledgement and delivery +state; no new lifecycle or notification evidence is emitted. Fresh finite +evidence resumes normal evaluation, and explicitly disabling a rule still +clears it independently of telemetry. `windowed_metric_test.go` verifies +NaN/infinities, arithmetic overflow, duplicate authority, input isolation, +JSON-safe incident snapshots, transition-ledger continuity and local dispatch. +These source controls do not prove installed or external recipient acceptance. + Active-alert restore is opt-out at construction. `NewManagerWithDataDir` accepts `ManagerOption` values, and `WithoutPersistedAlertRestore` starts the manager with an empty active-alert set instead of reading `active-alerts.json`. Mock diff --git a/internal/alerts/windowed_metric.go b/internal/alerts/windowed_metric.go index f305d1b73c..457b695637 100644 --- a/internal/alerts/windowed_metric.go +++ b/internal/alerts/windowed_metric.go @@ -2,6 +2,7 @@ package alerts import ( "fmt" + "math" "sort" "strings" "time" @@ -66,6 +67,12 @@ func (m *Manager) evaluateMetricWindow(resourceID, resourceType, metricType stri provider := m.metricWindowProvider m.mu.RUnlock() + // Unknown telemetry is neither a trigger nor recovery evidence, including + // instantaneous evaluation and embedders without a history provider. + if !finiteMetricValue(current) { + return metricWindowObservation{CurrentValue: current, WindowSeconds: windowSeconds} + } + instant := metricWindowObservation{Value: current, CurrentValue: current, Ready: true} if windowSeconds <= 0 { return instant @@ -92,6 +99,9 @@ func (m *Manager) evaluateMetricWindow(resourceID, resourceType, metricType stri func calculateMetricWindow(points []MetricWindowPoint, start, end time.Time, current float64, windowSeconds int) metricWindowObservation { result := metricWindowObservation{CurrentValue: current, WindowSeconds: windowSeconds} + if !finiteMetricValue(current) { + return result + } filtered := make([]MetricWindowPoint, 0, len(points)) for _, point := range points { if point.Timestamp.Before(start) || point.Timestamp.After(end) { @@ -112,6 +122,14 @@ func calculateMetricWindow(points []MetricWindowPoint, start, end time.Time, cur unique = append(unique, point) } result.SampleCount = len(unique) + // Validate after duplicate authority is resolved. Dropping bad values would + // invent continuous coverage across unavailable observations; using an older + // finite duplicate would override the latest evidence. + for _, point := range unique { + if !finiteMetricValue(point.Value) { + return result + } + } if len(unique) < minimumMetricWindowSamples { return result } @@ -141,11 +159,19 @@ func calculateMetricWindow(points []MetricWindowPoint, start, end time.Time, cur if weightedDuration <= 0 { return result } - result.Value = weightedTotal / weightedDuration.Seconds() + value := weightedTotal / weightedDuration.Seconds() + if !finiteMetricValue(value) { + return result + } + result.Value = value result.Ready = true return result } +func finiteMetricValue(value float64) bool { + return !math.IsNaN(value) && !math.IsInf(value, 0) +} + func metricWindowOptions(opts *metricOptions, metricType, resourceType string, observation metricWindowObservation) *metricOptions { if observation.WindowSeconds <= 0 { return opts diff --git a/internal/alerts/windowed_metric_test.go b/internal/alerts/windowed_metric_test.go index 664b1199dd..402ae8c89c 100644 --- a/internal/alerts/windowed_metric_test.go +++ b/internal/alerts/windowed_metric_test.go @@ -1,9 +1,15 @@ package alerts import ( + "bytes" + "encoding/json" "math" + "sync/atomic" "testing" "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/alerts/eventlog" + "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" ) func TestCalculateMetricWindowUsesTimeWeightedAverage(t *testing.T) { @@ -133,3 +139,237 @@ func TestWindowedMetricUnknownHistoryPreservesActiveIncident(t *testing.T) { t.Fatalf("expected unknown history to preserve the incident, got %+v", active) } } + +func nonFiniteMetricSamples() map[string]float64 { + return map[string]float64{"NaN": math.NaN(), "positive infinity": math.Inf(1), "negative infinity": math.Inf(-1)} +} + +func TestMetricEvaluationRejectsNonFiniteCurrent(t *testing.T) { + for _, mode := range []string{"instant", "window without provider", "window with provider"} { + for name, value := range nonFiniteMetricSamples() { + t.Run(mode+"/"+name, func(t *testing.T) { + m := NewManagerWithDataDir(t.TempDir(), WithoutPersistedAlertRestore()) + t.Cleanup(m.Stop) + window := 0 + if mode != "instant" { + window = 300 + } + m.mu.Lock() + m.config.MetricEvaluationWindows = map[string]map[string]int{"all": {"cpu": window}} + m.mu.Unlock() + calls := 0 + if mode == "window with provider" { + m.SetMetricWindowProvider(func(MetricWindowRequest) ([]MetricWindowPoint, error) { + calls++ + return nil, nil + }) + } + got := m.evaluateMetricWindow("vm-1", "vm", "cpu", value, time.Now()) + if got.Ready || calls != 0 { + t.Fatalf("invalid current is not trustworthy or a reason to query history: ready=%v calls=%d", got.Ready, calls) + } + }) + } + } +} + +func TestMetricWindowRejectsNonFiniteEvidence(t *testing.T) { + end := time.Date(2026, 10, 4, 8, 0, 0, 0, time.UTC) + start := end.Add(-4 * time.Minute) + for name, value := range nonFiniteMetricSamples() { + for _, position := range []string{"first", "middle", "last", "current"} { + t.Run(name+"/"+position, func(t *testing.T) { + points := []MetricWindowPoint{{start, 90}, {start.Add(2 * time.Minute), 90}, {end, 90}} + current := 90.0 + switch position { + case "first": + points[0].Value = value + case "middle": + points[1].Value = value + case "last": + points[2].Value = value + case "current": + current = value + } + if got := calculateMetricWindow(points, start, end, current, 240); got.Ready { + t.Fatalf("non-finite %s was marked ready: %+v", position, got) + } + }) + } + } + for name, value := range map[string]float64{"positive overflow": math.MaxFloat64, "negative overflow": -math.MaxFloat64} { + t.Run(name, func(t *testing.T) { + points := []MetricWindowPoint{{start, value}, {start.Add(2 * time.Minute), value}, {end, value}} + if got := calculateMetricWindow(points, start, end, value, 240); got.Ready { + t.Fatalf("non-finite weighted result was marked ready: %+v", got) + } + }) + } +} + +func TestMetricWindowFiniteAuthorityAndInputIsolation(t *testing.T) { + end := time.Date(2026, 10, 4, 8, 0, 0, 0, time.UTC) + start := end.Add(-4 * time.Minute) + for name, invalid := range nonFiniteMetricSamples() { + t.Run(name, func(t *testing.T) { + // Unordered input and obsolete invalid duplicates are not evidence. + points := []MetricWindowPoint{{end, invalid}, {start, 90}, {start.Add(2 * time.Minute), 90}, {end, 90}, {start.Add(-time.Second), invalid}, {end.Add(time.Second), invalid}} + before := append([]MetricWindowPoint(nil), points...) + got := calculateMetricWindow(points, start, end, 90, 240) + if !got.Ready || got.Value != 90 || got.SampleCount != 3 || got.CoverageSeconds != 240 { + t.Fatalf("last in-range duplicate must remain authoritative: %+v", got) + } + for i := range points { + if points[i].Timestamp != before[i].Timestamp || math.Float64bits(points[i].Value) != math.Float64bits(before[i].Value) { + t.Fatal("window evaluation mutated provider evidence") + } + } + points = append(points, MetricWindowPoint{end, invalid}) + if got := calculateMetricWindow(points, start, end, 90, 240); got.Ready { + t.Fatal("invalid latest duplicate must not resurrect the older finite value") + } + }) + } + for _, value := range []float64{-20, 0, 90, 1e300} { + points := []MetricWindowPoint{{start, value}, {start.Add(2 * time.Minute), value}, {end, value}} + got := calculateMetricWindow(points, start, end, value, 240) + if !got.Ready || math.Abs(got.Value-value) > math.Abs(value)*1e-14 { + t.Fatalf("finite values must not be clipped or made unknown: value=%v result=%+v", value, got) + } + } +} + +func TestNonFiniteMetricPreservesIncidentAndDeliveryEvidence(t *testing.T) { + for _, mode := range []string{"legacy instant", "canonical instant", "legacy history", "canonical history"} { + for name, invalid := range nonFiniteMetricSamples() { + t.Run(mode+"/"+name, func(t *testing.T) { + m := newEventLogManager(t) + m.mu.Lock() + m.config.TimeThresholds = map[string]int{} + m.config.MinimumDelta = 0 + m.config.SuppressionWindow = 0 + m.config.FlappingEnabled = false + window := 0 + withHistory := mode == "legacy history" || mode == "canonical history" + if withHistory { + window = 300 + } + m.config.MetricEvaluationWindows = map[string]map[string]int{"all": {"cpu": window}} + m.mu.Unlock() + now := time.Date(2026, 10, 4, 8, 0, 0, 0, time.UTC) + m.now = func() time.Time { return now } + historyValue := 85.0 + m.SetMetricWindowProvider(func(request MetricWindowRequest) ([]MetricWindowPoint, error) { + return []MetricWindowPoint{{request.Start, historyValue}, {request.Start.Add(2 * time.Minute), historyValue}, {request.Start.Add(4 * time.Minute), historyValue}}, nil + }) + threshold := &HysteresisThreshold{Trigger: 80, Clear: 75} + spec, err := buildCanonicalMetricSpec("vm-1", "web-1", unifiedresources.ResourceTypeVM, "cpu", threshold) + if err != nil { + t.Fatal(err) + } + var deliveries atomic.Int64 + fired := make(chan struct{}, 8) + m.SetAlertCallback(func(*Alert) { deliveries.Add(1); fired <- struct{}{} }) + evaluate := func(value float64) { + if mode == "canonical instant" || mode == "canonical history" { + m.evaluateCanonicalMetricAlert(spec, "web-1", "node-1", "pve-1", "vm", value, threshold, nil) + } else { + m.checkMetric("vm-1", "web-1", "node-1", "pve-1", "vm", "cpu", value, threshold, nil) + } + } + evaluate(85) + select { + case <-fired: + case <-time.After(5 * time.Second): + t.Fatal("finite trigger did not reach the asynchronous dispatch callback") + } + active := m.GetActiveAlerts() + if len(active) != 1 || deliveries.Load() != 1 { + t.Fatalf("finite trigger must fire and dispatch exactly once: active=%d deliveries=%d", len(active), deliveries.Load()) + } + if err := m.AcknowledgeAlert(active[0].ID, "operator"); err != nil { + t.Fatal(err) + } + before, err := json.Marshal(m.GetActiveAlerts()) + if err != nil { + t.Fatal(err) + } + eventsBefore := len(queryAlertEvents(t, m, eventlog.Filter{})) + now = now.Add(time.Minute) + if withHistory { + historyValue = invalid + evaluate(10) + } else { + evaluate(invalid) + } + after, err := json.Marshal(m.GetActiveAlerts()) + if err != nil || !bytes.Equal(before, after) || deliveries.Load() != 1 { + t.Fatalf("unknown evidence changed acknowledged incident/delivery or broke JSON: error=%v before=%s after=%s deliveries=%d", err, before, after, deliveries.Load()) + } + if got := len(queryAlertEvents(t, m, eventlog.Filter{})); got != eventsBefore { + t.Fatalf("unknown evidence changed durable transition ledger: before=%d after=%d", eventsBefore, got) + } + historyValue = 10 + now = now.Add(time.Minute) + evaluate(10) + evaluate(10) + if len(m.GetActiveAlerts()) != 0 || len(queryAlertEvents(t, m, eventlog.Filter{Types: []string{eventlog.TypeResolved}})) != 1 { + t.Fatal("fresh finite evidence must recover the same incident exactly once") + } + }) + } + } +} + +func TestNonFiniteMetricCannotActivateIncident(t *testing.T) { + for _, window := range []int{0, 300} { + for name, invalid := range nonFiniteMetricSamples() { + t.Run((time.Duration(window)*time.Second).String()+"/"+name, func(t *testing.T) { + m := newEventLogManager(t) + m.mu.Lock() + m.config.TimeThresholds = map[string]int{} + m.config.MetricEvaluationWindows = map[string]map[string]int{"all": {"cpu": window}} + m.mu.Unlock() + m.SetMetricWindowProvider(func(request MetricWindowRequest) ([]MetricWindowPoint, error) { + return []MetricWindowPoint{{request.Start, invalid}, {request.Start.Add(2 * time.Minute), invalid}, {request.Start.Add(4 * time.Minute), invalid}}, nil + }) + var deliveries atomic.Int64 + m.SetAlertCallback(func(*Alert) { deliveries.Add(1) }) + value := invalid + if window > 0 { + value = 85 + } + m.checkMetric("vm-1", "web-1", "node-1", "pve-1", "vm", "cpu", value, &HysteresisThreshold{Trigger: 80, Clear: 75}, nil) + if len(m.GetActiveAlerts()) != 0 || deliveries.Load() != 0 || len(queryAlertEvents(t, m, eventlog.Filter{})) != 0 { + t.Fatal("invalid evidence opened an incident or dispatched a notification") + } + }) + } + } +} + +func TestNonFiniteMetricDoesNotPreventExplicitDisable(t *testing.T) { + for _, canonical := range []bool{false, true} { + m := newEventLogManager(t) + m.mu.Lock() + m.config.TimeThresholds = map[string]int{} + m.config.MetricEvaluationWindows = map[string]map[string]int{"all": {"cpu": 0}} + m.mu.Unlock() + threshold := &HysteresisThreshold{Trigger: 80, Clear: 75} + spec, err := buildCanonicalMetricSpec("vm-1", "web-1", unifiedresources.ResourceTypeVM, "cpu", threshold) + if err != nil { + t.Fatal(err) + } + if canonical { + m.evaluateCanonicalMetricAlert(spec, "web-1", "node-1", "pve-1", "vm", 85, threshold, nil) + spec.Disabled = true + m.evaluateCanonicalMetricAlert(spec, "web-1", "node-1", "pve-1", "vm", math.NaN(), threshold, nil) + } else { + m.checkMetric("vm-1", "web-1", "node-1", "pve-1", "vm", "cpu", 85, threshold, nil) + m.checkMetric("vm-1", "web-1", "node-1", "pve-1", "vm", "cpu", math.NaN(), nil, nil) + } + if len(m.GetActiveAlerts()) != 0 || len(queryAlertEvents(t, m, eventlog.Filter{Types: []string{eventlog.TypeFired}})) != 1 || len(queryAlertEvents(t, m, eventlog.Filter{Types: []string{eventlog.TypeResolved}})) != 1 { + t.Fatal("explicit disable must still resolve the active rule independently of telemetry") + } + } +}