diff --git a/rules/manager.go b/rules/manager.go index 7fcc57909..0b4661668 100644 --- a/rules/manager.go +++ b/rules/manager.go @@ -221,25 +221,37 @@ func (g *Group) offset() time.Duration { } // copyState copies the alerting rule and staleness related state from the given group. +// +// Rules are matched based on their name. If there are duplicates, the +// first is matched with the first, second with the second etc. func (g *Group) copyState(from *Group) { - g.seriesInPreviousEval = from.seriesInPreviousEval - for _, fromRule := range from.rules { - far, ok := fromRule.(*AlertingRule) + ruleMap := make(map[string][]int, len(from.rules)) + + for fi, fromRule := range from.rules { + l, _ := ruleMap[fromRule.Name()] + ruleMap[fromRule.Name()] = append(l, fi) + } + + for i, rule := range g.rules { + indexes, ok := ruleMap[rule.Name()] + if len(indexes) == 0 { + continue + } + fi := indexes[0] + g.seriesInPreviousEval[i] = from.seriesInPreviousEval[fi] + ruleMap[rule.Name()] = indexes[1:] + + ar, ok := rule.(*AlertingRule) if !ok { continue } - for _, rule := range g.rules { - ar, ok := rule.(*AlertingRule) - if !ok { - continue - } - // TODO(fabxc): forbid same alert definitions that are not unique by - // at least on static label or alertname? - if far.equal(ar) { - for fp, a := range far.active { - ar.active[fp] = a - } - } + far, ok := from.rules[fi].(*AlertingRule) + if !ok { + continue + } + + for fp, a := range far.active { + ar.active[fp] = a } } } diff --git a/rules/manager_test.go b/rules/manager_test.go index eab4139ee..9d7f8a3b9 100644 --- a/rules/manager_test.go +++ b/rules/manager_test.go @@ -183,15 +183,7 @@ func TestStaleness(t *testing.T) { // A time series that has two samples and then goes stale. app, _ := storage.Appender() app.Add(labels.FromStrings(model.MetricNameLabel, "a"), 0, 1) - if err = app.Commit(); err != nil { - t.Fatal(err) - } - app, _ = storage.Appender() app.Add(labels.FromStrings(model.MetricNameLabel, "a"), 1000, 2) - if err = app.Commit(); err != nil { - t.Fatal(err) - } - app, _ = storage.Appender() app.Add(labels.FromStrings(model.MetricNameLabel, "a"), 2000, math.Float64frombits(value.StaleNaN)) if err = app.Commit(); err != nil { t.Fatal(err) @@ -208,8 +200,7 @@ func TestStaleness(t *testing.T) { t.Fatal(err) } matcher, _ := labels.NewMatcher(labels.MatchEqual, model.MetricNameLabel, "a_plus_one") - seriesSet := querier.Select(matcher) - samples, err := readSeriesSet(seriesSet) + samples, err := readSeriesSet(querier.Select(matcher)) if err != nil { t.Fatal(err) } @@ -249,9 +240,53 @@ func readSeriesSet(ss storage.SeriesSet) (map[string][]promql.Point, error) { name := series.Labels().String() result[name] = points - if err := ss.Err(); err != nil { - return nil, err - } } - return result, nil + return result, ss.Err() +} + +func TestCopyState(t *testing.T) { + oldGroup := &Group{ + rules: []Rule{ + NewAlertingRule("alert", nil, 0, nil, nil), + NewRecordingRule("rule1", nil, nil), + NewRecordingRule("rule2", nil, nil), + NewRecordingRule("rule3", nil, nil), + NewRecordingRule("rule3", nil, nil), + }, + seriesInPreviousEval: []map[string]labels.Labels{ + map[string]labels.Labels{"a": nil}, + map[string]labels.Labels{"r1": nil}, + map[string]labels.Labels{"r2": nil}, + map[string]labels.Labels{"r3a": nil}, + map[string]labels.Labels{"r3b": nil}, + }, + } + oldGroup.rules[0].(*AlertingRule).active[42] = nil + newGroup := &Group{ + rules: []Rule{ + NewRecordingRule("rule3", nil, nil), + NewRecordingRule("rule3", nil, nil), + NewRecordingRule("rule3", nil, nil), + NewAlertingRule("alert", nil, 0, nil, nil), + NewRecordingRule("rule1", nil, nil), + NewRecordingRule("rule4", nil, nil), + }, + seriesInPreviousEval: make([]map[string]labels.Labels, 6), + } + newGroup.copyState(oldGroup) + + want := []map[string]labels.Labels{ + map[string]labels.Labels{"r3a": nil}, + map[string]labels.Labels{"r3b": nil}, + nil, + map[string]labels.Labels{"a": nil}, + map[string]labels.Labels{"r1": nil}, + nil, + } + if !reflect.DeepEqual(want, newGroup.seriesInPreviousEval) { + t.Fatalf("seriesInPreviousEval not as expected. Wanted: %+v Got: %+v", want, newGroup.seriesInPreviousEval) + } + if !reflect.DeepEqual(oldGroup.rules[0], newGroup.rules[3]) { + t.Fatalf("Active alerts not as expected. Wanted: %+v Got: %+v", oldGroup.rules[0], oldGroup.rules[3]) + } }