Merge pull request #9 from avlitman/rules-registry

Allow recording rules with same name but different expression
machadovilaca · Apr 9, 2024 · 6aa8144 · 6aa8144
2 parents 50ee142 + 8849a1e
commit 6aa8144
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 14 deletions.
diff --git a/pkg/docs/metrics.go b/pkg/docs/metrics.go
@@ -81,14 +81,19 @@ func BuildMetricsDocs(metrics []operatormetrics.Metric, recordingRules []operato
 }
 
 func buildMetricsDocs[T docOptions](items []T) []metricDocs {
-	metricsDocs := make([]metricDocs, len(items))
-	for i, metric := range items {
+	uniqueNames := make(map[string]struct{})
+	var metricsDocs []metricDocs
+
+	for _, metric := range items {
 		metricOpts := metric.GetOpts()
-		metricsDocs[i] = metricDocs{
-			Name:        metricOpts.Name,
-			Help:        metricOpts.Help,
-			Type:        getAndConvertMetricType(metric.GetType()),
-			ExtraFields: metricOpts.ExtraFields,
+		if _, exists := uniqueNames[metricOpts.Name]; !exists {
+			uniqueNames[metricOpts.Name] = struct{}{}
+			metricsDocs = append(metricsDocs, metricDocs{
+				Name:        metricOpts.Name,
+				Help:        metricOpts.Help,
+				Type:        getAndConvertMetricType(metric.GetType()),
+				ExtraFields: metricOpts.ExtraFields,
+			})
 		}
 	}
 

diff --git a/pkg/operatorrules/registry.go b/pkg/operatorrules/registry.go
@@ -25,7 +25,8 @@ func newRegistry() operatorRegisterer {
 func RegisterRecordingRules(recordingRules ...[]RecordingRule) error {
 	for _, recordingRuleList := range recordingRules {
 		for _, recordingRule := range recordingRuleList {
-			operatorRegistry.registeredRecordingRules[recordingRule.MetricsOpts.Name] = recordingRule
+			key := recordingRule.MetricsOpts.Name + ":" + recordingRule.Expr.String()
+			operatorRegistry.registeredRecordingRules[key] = recordingRule
 		}
 	}
 
@@ -51,7 +52,10 @@ func ListRecordingRules() []RecordingRule {
 	}
 
 	slices.SortFunc(rules, func(a, b RecordingRule) int {
-		return cmp.Compare(a.GetOpts().Name, b.GetOpts().Name)
+		aKey := a.GetOpts().Name + ":" + a.Expr.String()
+		bKey := b.GetOpts().Name + ":" + b.Expr.String()
+
+		return cmp.Compare(aKey, bKey)
 	})
 
 	return rules

diff --git a/pkg/operatorrules/registry_test.go b/pkg/operatorrules/registry_test.go
@@ -36,15 +36,15 @@ var _ = Describe("OperatorRules", func() {
 			Expect(registeredRules).To(ConsistOf(recordingRules))
 		})
 
-		It("should replace recording rules with the same name in the same RegisterRecordingRules call", func() {
+		It("should replace recording rule with the same name and expression", func() {
 			recordingRules := []RecordingRule{
 				{
 					MetricsOpts: operatormetrics.MetricOpts{Name: "ExampleRecordingRule1"},
 					Expr:        intstr.FromString("sum(rate(http_requests_total[5m]))"),
 				},
 				{
 					MetricsOpts: operatormetrics.MetricOpts{Name: "ExampleRecordingRule1"},
-					Expr:        intstr.FromString("sum(rate(http_requests_total[10m]))"),
+					Expr:        intstr.FromString("sum(rate(http_requests_total[5m]))"),
 				},
 			}
 
@@ -53,10 +53,10 @@ var _ = Describe("OperatorRules", func() {
 
 			registeredRules := ListRecordingRules()
 			Expect(registeredRules).To(HaveLen(1))
-			Expect(registeredRules[0].Expr.String()).To(Equal("sum(rate(http_requests_total[10m]))"))
+			Expect(registeredRules[0].Expr.String()).To(Equal("sum(rate(http_requests_total[5m]))"))
 		})
 
-		It("should replace recording rules with the same name in different RegisterRecordingRules calls", func() {
+		It("should create 2 recording rules when registered with the same name but different expressions", func() {
 			recordingRules := []RecordingRule{
 				{
 					MetricsOpts: operatormetrics.MetricOpts{Name: "ExampleRecordingRule1"},
@@ -78,8 +78,9 @@ var _ = Describe("OperatorRules", func() {
 			Expect(err).To(BeNil())
 
 			registeredRules := ListRecordingRules()
-			Expect(registeredRules).To(HaveLen(1))
+			Expect(registeredRules).To(HaveLen(2))
 			Expect(registeredRules[0].Expr.String()).To(Equal("sum(rate(http_requests_total[10m]))"))
+			Expect(registeredRules[1].Expr.String()).To(Equal("sum(rate(http_requests_total[5m]))"))
 		})
 	})