mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-24 04:33:17 +00:00
Keep non-finite metric evidence out of alert decisions
Treat non-finite current observations, authoritative rolling-history samples and overflowing averages as unknown. Preserve trusted incident and notification state instead of opening, recovering or poisoning JSON snapshots. Retain timestamp/duplicate authority, normal finite evaluation and explicit rule disable. Change-source: pulse-maintainer
This commit is contained in:
@@ -461,6 +461,20 @@ operator-visible message evidence. Window changes never change the canonical
|
||||
`metric-threshold:<metric>` 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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user