Unique identifiers (#552)

* add relation information to the identifier so it is unique * alerts[identifier] will always be overwritten later in the method anyway * fix tests
canonical · Nov 23, 2023 · c25f7ce · c25f7ce
1 parent 26c43ef
commit c25f7ce
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 7 deletions.
diff --git a/lib/charms/prometheus_k8s/v0/prometheus_scrape.py b/lib/charms/prometheus_k8s/v0/prometheus_scrape.py
@@ -1015,7 +1015,6 @@ def alerts(self) -> dict:
                 try:
                     scrape_metadata = json.loads(relation.data[relation.app]["scrape_metadata"])
                     identifier = JujuTopology.from_dict(scrape_metadata).identifier
-                    alerts[identifier] = self._tool.apply_label_matchers(alert_rules)  # type: ignore
 
                 except KeyError as e:
                     logger.debug(
@@ -1030,6 +1029,10 @@ def alerts(self) -> dict:
                 )
                 continue
 
+            # We need to append the relation info to the identifier. This is to allow for cases for there are two
+            # relations which eventually scrape the same application. Issue #551.
+            identifier = f"{identifier}_{relation.name}_{relation.id}"
+
             alerts[identifier] = alert_rules
 
             _, errmsg = self._tool.validate_alert_rules(alert_rules)

diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
@@ -413,7 +413,9 @@ def test_charm_writes_meaningful_alerts_filename_1(self, *_):
         files = container.list_files("/etc/prometheus/rules")
         self.assertEqual(
             {file.path for file in files},
-            {"/etc/prometheus/rules/juju_ZZZ-model_a5edc336_zzz-app.rules"},
+            {
+                f"/etc/prometheus/rules/juju_ZZZ-model_a5edc336_zzz-app_{RELATION_NAME}_{self.rel_id}.rules"
+            },
         )
 
     @k8s_resource_multipatch
@@ -436,7 +438,9 @@ def test_charm_writes_meaningful_alerts_filename_2(self, *_):
         files = container.list_files("/etc/prometheus/rules")
         self.assertEqual(
             {file.path for file in files},
-            {"/etc/prometheus/rules/juju_ZZZ-model_a5edc336_zzz-app.rules"},
+            {
+                f"/etc/prometheus/rules/juju_ZZZ-model_a5edc336_zzz-app_{RELATION_NAME}_{self.rel_id}.rules"
+            },
         )
 
     @k8s_resource_multipatch
@@ -458,7 +462,9 @@ def test_charm_writes_meaningful_alerts_filename_3(self, *_):
         files = container.list_files("/etc/prometheus/rules")
         self.assertEqual(
             {file.path for file in files},
-            {"/etc/prometheus/rules/juju_remote-model_be44e4b8_remote-app.rules"},
+            {
+                f"/etc/prometheus/rules/juju_remote-model_be44e4b8_remote-app_{RELATION_NAME}_{self.rel_id}.rules"
+            },
         )
 
     @k8s_resource_multipatch
@@ -480,7 +486,8 @@ def test_charm_writes_meaningful_alerts_filename_4(self, *_):
         container = self.harness.charm.unit.get_container(self.harness.charm._name)
         files = container.list_files("/etc/prometheus/rules")
         self.assertEqual(
-            {file.path for file in files}, {"/etc/prometheus/rules/juju_ZZZ_group_alerts.rules"}
+            {file.path for file in files},
+            {f"/etc/prometheus/rules/juju_ZZZ_group_alerts_{RELATION_NAME}_{self.rel_id}.rules"},
         )
 
 

diff --git a/tests/unit/test_endpoint_consumer.py b/tests/unit/test_endpoint_consumer.py
@@ -499,8 +499,9 @@ def test_consumer_accepts_rules_with_no_identifier(self):
                 messages[1],
             )
         alerts = self.harness.charm.prometheus_consumer.alerts
-        self.assertIn("unlabeled_external_cpu_alerts", alerts.keys())
-        self.assertEqual(UNLABELED_ALERT_RULES, alerts["unlabeled_external_cpu_alerts"])
+        identifier = f"unlabeled_external_cpu_alerts_{RELATION_NAME}_{rel_id}"
+        self.assertIn(identifier, alerts.keys())
+        self.assertEqual(UNLABELED_ALERT_RULES, alerts[identifier])
 
     def test_bad_scrape_job(self):
         self.harness.set_leader(True)