fix(alert): retain Duration samples for offline rules so they fire

RetentionWindow() trimmed offline-rule samples to 1, but Check's offline
branch reads points[len-Duration:] and needs Duration samples. The window
never filled, boundCheck always returned "passed", and offline alerts never
fired a notification — while other alert types worked. Offline rules now
retain Duration samples (cycle rules still keep 1, matching their last-sample
lookback).

Fixes the prior over-correction in RetentionWindow; corrects the test that
had pinned the buggy offline=1 value, and adds offline + combined-rule
regression tests driving the real trim loop.
This commit is contained in:
naiba
2026-06-08 00:57:14 +00:00
parent 576e6773c0
commit 3d74cd9431
2 changed files with 71 additions and 2 deletions
+4 -1
View File
@@ -199,11 +199,14 @@ func (r *AlertRule) Check(points [][]bool) (int, bool) {
// RetentionWindow 返回保留历史采样所需的长度(各规则窗口的最大值),只依赖 // RetentionWindow 返回保留历史采样所需的长度(各规则窗口的最大值),只依赖
// 规则定义而非 Check 的判定结果——否则窗口未填满时 Check 返回的 max=0 会被 // 规则定义而非 Check 的判定结果——否则窗口未填满时 Check 返回的 max=0 会被
// 误判为"无需历史"而清空采样,使规则永远攒不够样本。 // 误判为"无需历史"而清空采样,使规则永远攒不够样本。
// 各规则类型回看的采样数必须与 Check 中实际读取的窗口一致:
// - 周期流量规则:Check 只读最后 1 个采样点 → 需要 1
// - 离线规则、常规规则:Check 读取 points[len-Duration:] → 需要 Duration
func (r *AlertRule) RetentionWindow() int { func (r *AlertRule) RetentionWindow() int {
window := 0 window := 0
for _, rule := range r.Rules { for _, rule := range r.Rules {
var need int var need int
if rule.IsTransferDurationRule() || rule.IsOfflineRule() { if rule.IsTransferDurationRule() {
need = 1 need = 1
} else if d := int(rule.Duration); d > 0 { } else if d := int(rule.Duration); d > 0 {
need = d need = d
+67 -1
View File
@@ -415,7 +415,7 @@ func TestAlertRule_RetentionWindow(t *testing.T) {
{"single general", &AlertRule{Rules: []*Rule{{Type: "cpu", Duration: 10}}}, 10}, {"single general", &AlertRule{Rules: []*Rule{{Type: "cpu", Duration: 10}}}, 10},
{"zero duration only", &AlertRule{Rules: []*Rule{{Type: "cpu", Duration: 0}}}, 0}, {"zero duration only", &AlertRule{Rules: []*Rule{{Type: "cpu", Duration: 0}}}, 0},
{"mixed picks max", &AlertRule{Rules: []*Rule{{Type: "cpu", Duration: 0}, {Type: "cpu", Duration: 7}}}, 7}, {"mixed picks max", &AlertRule{Rules: []*Rule{{Type: "cpu", Duration: 0}, {Type: "cpu", Duration: 7}}}, 7},
{"offline looks back one", &AlertRule{Rules: []*Rule{{Type: "offline", Duration: 30}}}, 1}, {"offline keeps Duration", &AlertRule{Rules: []*Rule{{Type: "offline", Duration: 30}}}, 30},
{"cycle looks back one", &AlertRule{Rules: []*Rule{{Type: "net_in_speed_cycle"}}}, 1}, {"cycle looks back one", &AlertRule{Rules: []*Rule{{Type: "net_in_speed_cycle"}}}, 1},
} }
for _, c := range cases { for _, c := range cases {
@@ -424,3 +424,69 @@ func TestAlertRule_RetentionWindow(t *testing.T) {
} }
} }
} }
// TestAlertRule_OfflineRuleAccumulatesSamples is a regression guard for offline
// alerts that never fire. Check's offline branch reads points[len-Duration:],
// so it needs Duration samples retained; if RetentionWindow trims to 1 (as an
// earlier fix wrongly did for offline rules), the window never reaches Duration,
// boundCheck keeps returning "passed", and the offline alert never raises.
func TestAlertRule_OfflineRuleAccumulatesSamples(t *testing.T) {
const duration = 10
rule := &AlertRule{Rules: []*Rule{{Type: "offline", Duration: duration}}}
var samples [][]bool
var lastPassed bool
maxLen := 0
for tick := 0; tick < duration*3; tick++ {
samples = append(samples, []bool{false}) // offline sample
_, lastPassed = rule.Check(samples)
samples = trimSamples(samples, rule.RetentionWindow())
if len(samples) > maxLen {
maxLen = len(samples)
}
}
if maxLen < duration {
t.Fatalf("offline samples never accumulated to Duration: max window reached %d, want >= %d", maxLen, duration)
}
if lastPassed {
t.Fatalf("a server offline every tick must eventually fail the offline check (passed=false), got passed=true")
}
}
// TestAlertRule_CombinedRuleAccumulatesSamples drives the real trim loop for
// mixed-type alerts. The verdict is AND-of-failure: an incident fires only once
// every rule's lookback window is full and all fail. RetentionWindow must keep
// enough samples for the largest window (offline/general need Duration, cycle
// needs 1); if any rule type is under-retained the alert never fires.
func TestAlertRule_CombinedRuleAccumulatesSamples(t *testing.T) {
cases := []struct {
msg string
rule *AlertRule
sample []bool
fireAt int // tick index where passed must first become false
wantWindow int
}{
{"general3+general10", &AlertRule{Rules: []*Rule{{Type: "cpu", Duration: 3}, {Type: "memory", Duration: 10}}}, []bool{false, false}, 9, 10},
{"offline5+general10", &AlertRule{Rules: []*Rule{{Type: "offline", Duration: 5}, {Type: "cpu", Duration: 10}}}, []bool{false, false}, 9, 10},
{"transfer+general8", &AlertRule{Rules: []*Rule{{Type: "net_in_speed_cycle"}, {Type: "cpu", Duration: 8}}}, []bool{false, false}, 7, 8},
{"offline3+offline12", &AlertRule{Rules: []*Rule{{Type: "offline", Duration: 3}, {Type: "offline", Duration: 12}}}, []bool{false, false}, 11, 12},
}
for _, c := range cases {
if got := c.rule.RetentionWindow(); got != c.wantWindow {
t.Fatalf("%s: RetentionWindow()=%d want %d", c.msg, got, c.wantWindow)
}
var samples [][]bool
firstFire := -1
for tick := 0; tick < c.wantWindow*3; tick++ {
samples = append(samples, append([]bool(nil), c.sample...))
if _, passed := c.rule.Check(samples); !passed && firstFire < 0 {
firstFire = tick
}
samples = trimSamples(samples, c.rule.RetentionWindow())
}
if firstFire != c.fireAt {
t.Fatalf("%s: alert first fired at tick %d, want %d (never-firing = -1)", c.msg, firstFire, c.fireAt)
}
}
}