diff --git a/rules/group.go b/rules/group.go index 9ad9aab093..a83c33a8e8 100644 --- a/rules/group.go +++ b/rules/group.go @@ -1110,9 +1110,6 @@ func buildDependencyMap(rules []Rule) dependencyMap { return dependencies } - inputs := make(map[string][]Rule, len(rules)) - outputs := make(map[string][]Rule, len(rules)) - var indeterminate bool for _, rule := range rules { @@ -1120,26 +1117,46 @@ func buildDependencyMap(rules []Rule) dependencyMap { break } - name := rule.Name() - outputs[name] = append(outputs[name], rule) - parser.Inspect(rule.Query(), func(node parser.Node, path []parser.Node) error { if n, ok := node.(*parser.VectorSelector); ok { + // Find the name matcher for the rule. + var nameMatcher *labels.Matcher + if n.Name != "" { + nameMatcher = labels.MustNewMatcher(labels.MatchEqual, model.MetricNameLabel, n.Name) + } else { + for _, m := range n.LabelMatchers { + if m.Name == model.MetricNameLabel { + nameMatcher = m + break + } + } + } + // A wildcard metric expression means we cannot reliably determine if this rule depends on any other, // which means we cannot safely run any rules concurrently. - if n.Name == "" && len(n.LabelMatchers) > 0 { + if nameMatcher == nil { indeterminate = true return nil } // Rules which depend on "meta-metrics" like ALERTS and ALERTS_FOR_STATE will have undefined behaviour // if they run concurrently. - if n.Name == alertMetricName || n.Name == alertForStateMetricName { + if nameMatcher.Matches(alertMetricName) || nameMatcher.Matches(alertForStateMetricName) { indeterminate = true return nil } - inputs[n.Name] = append(inputs[n.Name], rule) + // Find rules which depend on the output of this rule. + for _, other := range rules { + if other == rule { + continue + } + + otherName := other.Name() + if nameMatcher.Matches(otherName) { + dependencies[other] = append(dependencies[other], rule) + } + } } return nil }) @@ -1149,13 +1166,5 @@ func buildDependencyMap(rules []Rule) dependencyMap { return nil } - for output, outRules := range outputs { - for _, outRule := range outRules { - if inRules, found := inputs[output]; found && len(inRules) > 0 { - dependencies[outRule] = append(dependencies[outRule], inRules...) - } - } - } - return dependencies } diff --git a/rules/manager_test.go b/rules/manager_test.go index 6c32b6d0f1..9ef1fcf074 100644 --- a/rules/manager_test.go +++ b/rules/manager_test.go @@ -1601,10 +1601,14 @@ func TestDependencyMap(t *testing.T) { require.NoError(t, err) rule4 := NewRecordingRule("user:requests:increase1h", expr, labels.Labels{}) + expr, err = parser.ParseExpr(`sum by (user) ({__name__=~"user:requests.+5m"})`) + require.NoError(t, err) + rule5 := NewRecordingRule("user:requests:sum5m", expr, labels.Labels{}) + group := NewGroup(GroupOptions{ Name: "rule_group", Interval: time.Second, - Rules: []Rule{rule, rule2, rule3, rule4}, + Rules: []Rule{rule, rule2, rule3, rule4, rule5}, Opts: opts, }) @@ -1619,13 +1623,17 @@ func TestDependencyMap(t *testing.T) { require.Equal(t, []Rule{rule}, depMap.dependencies(rule2)) require.False(t, depMap.isIndependent(rule2)) - require.Zero(t, depMap.dependents(rule3)) + require.Equal(t, []Rule{rule5}, depMap.dependents(rule3)) require.Zero(t, depMap.dependencies(rule3)) - require.True(t, depMap.isIndependent(rule3)) + require.False(t, depMap.isIndependent(rule3)) require.Zero(t, depMap.dependents(rule4)) require.Equal(t, []Rule{rule}, depMap.dependencies(rule4)) require.False(t, depMap.isIndependent(rule4)) + + require.Zero(t, depMap.dependents(rule5)) + require.Equal(t, []Rule{rule3}, depMap.dependencies(rule5)) + require.False(t, depMap.isIndependent(rule5)) } func TestNoDependency(t *testing.T) {