Add support for rule group labels #6665
Conversation
dosubot
bot
added
the
component/rules
mustafain117
force-pushed
the
rule-group-labels
branch
3 times, most recently
from
49e9849 to
c469509
Compare
| @@ -30,6 +30,10 @@ message RuleGroupDesc { | |||
| int64 limit =10; | |||
| google.protobuf.Duration queryOffset = 11 | |||
| [(gogoproto.nullable) = true, (gogoproto.stdduration) = true]; | |||
There was a problem hiding this comment.
Unrelated, but interesting that queryOffset is using nullable = true while all the rest is not. I was reading through the implications https://pkg.go.dev/github.com/gogo/protobuf/gogoproto#hdr-More_Canonical_Go_Structures
integration/ruler_test.go
Outdated
| // Wait until ruler has loaded the group. | ||
| require.NoError(t, ruler.WaitSumMetricsWithOptions(e2e.Equals(1), []string{"cortex_prometheus_rule_group_rules"}, e2e.WithLabelMatchers(m), e2e.WaitMissingMetrics)) | ||
|
|
||
| groups, _, err := c.GetPrometheusRules(e2ecortex.RuleFilter{ |
There was a problem hiding this comment.
we could also get the rule group using the client GerRuleGroup method to assert that the duplicate_label is really in the rule group level.
|
LGTM. Thanks! |
There was a problem hiding this comment.
Line 1253 in 8668b1c
I wonder if we miss this one. When API backup is enabled.
integration/ruler_test.go
Outdated
| @@ -1739,6 +1739,106 @@ func TestRulerEvalWithQueryFrontend(t *testing.T) { | |||
| } | |||
| } | |||
There was a problem hiding this comment.
Can we try to cover group labels in the API backup test case? Just to make sure it works for that scenario
TestRulerAPIShardingWithAPIRulesBackupEnabled
Signed-off-by: Mustafain Ali Khan <mustalik@amazon.com>
mustafain117
force-pushed
the
rule-group-labels
branch
from
c469509 to
2dc3a5a
Compare
Signed-off-by: Mustafain Ali Khan <mustalik@amazon.com>
mustafain117
force-pushed
the
rule-group-labels
branch
from
2dc3a5a to
f56d3b8
Compare
What this PR does:
Adds support for labels at rule group level.
Which issue(s) this PR fixes:
This feature was added in a recent Prometheus release, cortex should support this.
related PR: prometheus/prometheus#11474
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]