From c25f7ceaa3d4c8da93af704f6704a187663b54ee Mon Sep 17 00:00:00 2001 From: Dylan Stephano-Shachter Date: Thu, 23 Nov 2023 20:29:26 +0200 Subject: [PATCH] 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 --- lib/charms/prometheus_k8s/v0/prometheus_scrape.py | 5 ++++- tests/unit/test_charm.py | 15 +++++++++++---- tests/unit/test_endpoint_consumer.py | 5 +++-- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/lib/charms/prometheus_k8s/v0/prometheus_scrape.py b/lib/charms/prometheus_k8s/v0/prometheus_scrape.py index c954bc06..665af886 100644 --- 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 index bbf440a7..9bc8a3b0 100644 --- 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 index 9aa33415..a8957a8a 100644 --- 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)