From 943e90959cf4e1d1e3e44345583a83c79660dd34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jose=20C=2E=20Mass=C3=B3n?= <939888+Abuelodelanada@users.noreply.github.com> Date: Wed, 29 May 2024 15:03:33 -0300 Subject: [PATCH] Fix juju_unit and juju_application labels (#137) * Fix juju_unit and juju_application labels * tidy up codespell --- lib/charms/nrpe_exporter/v0/nrpe_exporter.py | 33 ++++++++++++++++--- .../prometheus_k8s/v0/prometheus_scrape.py | 21 ++++++------ pyproject.toml | 4 +++ src/charm.py | 2 +- tests/unit/test_relation_monitors.py | 15 ++++++++- tox.ini | 2 +- 6 files changed, 58 insertions(+), 19 deletions(-) diff --git a/lib/charms/nrpe_exporter/v0/nrpe_exporter.py b/lib/charms/nrpe_exporter/v0/nrpe_exporter.py index 7dc7f1f..e36e5fb 100644 --- a/lib/charms/nrpe_exporter/v0/nrpe_exporter.py +++ b/lib/charms/nrpe_exporter/v0/nrpe_exporter.py @@ -50,7 +50,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 5 +LIBPATCH = 6 logger = logging.getLogger(__name__) @@ -441,10 +441,14 @@ def _generate_data(self, relation) -> Tuple[list, list]: ) id = re.sub(r"^juju[-_]", "", id) + nagios_host_context = relation.data[unit].get("nagios_host_context", "") + alerts.append(self._generate_alert(relation, cmd, id, unit)) nrpe_endpoints.append( - self._generate_prometheus_job(relation, unit, cmd, exporter_address, id) + self._generate_prometheus_job( + relation, unit, cmd, exporter_address, id, nagios_host_context + ) ) else: logger.debug("No NRPE check is defined.") @@ -485,13 +489,19 @@ def _generate_alert(self, relation, cmd, id, unit) -> dict: }, } - def _generate_prometheus_job(self, relation, unit, cmd, exporter_address, id) -> dict: + def _generate_prometheus_job( + self, relation, unit, cmd, exporter_address, id, nagios_host_context + ) -> dict: """Generate an on-the-fly Prometheus scrape job.""" # IP address could be 'target-address' OR 'target_address' addr = relation.data[unit].get("target-address", "") or relation.data[unit].get( "target_address", "" ) + # "nagios_host_content" is needed to extract it from the "id" parameter (target-id) + # so that we can correctly relabel juju_application and juju_unit. + nagios_host_context = nagios_host_context + "-" if nagios_host_context else "" + return { "app_name": relation.app.name, "target": { @@ -511,8 +521,21 @@ def _generate_prometheus_job(self, relation, unit, cmd, exporter_address, id) -> }, { "target_label": "juju_unit", - # Turn sql-foo-0 or redis_bar_1 into sql-foo/0 or redis-bar/1 - "replacement": re.sub(r"^(.*?)[-_](\d+)$", r"\1/\2", id.replace("_", "-")), + # Turn nagios-host-context-sql-foo-0 into sql-foo/0 + "replacement": re.sub( + r"^(.*?)[-_](\d+)$", + r"\1/\2", + id.replace("_", "-").replace(nagios_host_context, ""), + ), + }, + { + "target_label": "juju_application", + # Turn nagios-host-context-sql-foo-0 into sql-foo + "replacement": re.sub( + r"^(.*?)[-_](\d+)$", + r"\1", + id.replace("_", "-").replace(nagios_host_context, ""), + ), }, ], "updates": { diff --git a/lib/charms/prometheus_k8s/v0/prometheus_scrape.py b/lib/charms/prometheus_k8s/v0/prometheus_scrape.py index 665af88..e3d35c6 100644 --- a/lib/charms/prometheus_k8s/v0/prometheus_scrape.py +++ b/lib/charms/prometheus_k8s/v0/prometheus_scrape.py @@ -178,7 +178,7 @@ def __init__(self, *args): - `scrape_timeout` - `proxy_url` - `relabel_configs` -- `metrics_relabel_configs` +- `metric_relabel_configs` - `sample_limit` - `label_limit` - `label_name_length_limit` @@ -362,7 +362,7 @@ def _on_scrape_targets_changed(self, event): # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 44 +LIBPATCH = 47 PYDEPS = ["cosl"] @@ -377,7 +377,7 @@ def _on_scrape_targets_changed(self, event): "scrape_timeout", "proxy_url", "relabel_configs", - "metrics_relabel_configs", + "metric_relabel_configs", "sample_limit", "label_limit", "label_name_length_limit", @@ -521,8 +521,8 @@ def expand_wildcard_targets_into_individual_jobs( # for such a target. Therefore labeling with Juju topology, excluding the # unit name. non_wildcard_static_config["labels"] = { - **non_wildcard_static_config.get("labels", {}), **topology.label_matcher_dict, + **non_wildcard_static_config.get("labels", {}), } non_wildcard_static_configs.append(non_wildcard_static_config) @@ -547,9 +547,9 @@ def expand_wildcard_targets_into_individual_jobs( if topology: # Add topology labels modified_static_config["labels"] = { - **modified_static_config.get("labels", {}), **topology.label_matcher_dict, **{"juju_unit": unit_name}, + **modified_static_config.get("labels", {}), } # Instance relabeling for topology should be last in order. @@ -1537,12 +1537,11 @@ def set_scrape_job_spec(self, _=None): relation.data[self._charm.app]["scrape_metadata"] = json.dumps(self._scrape_metadata) relation.data[self._charm.app]["scrape_jobs"] = json.dumps(self._scrape_jobs) - if alert_rules_as_dict: - # Update relation data with the string representation of the rule file. - # Juju topology is already included in the "scrape_metadata" field above. - # The consumer side of the relation uses this information to name the rules file - # that is written to the filesystem. - relation.data[self._charm.app]["alert_rules"] = json.dumps(alert_rules_as_dict) + # Update relation data with the string representation of the rule file. + # Juju topology is already included in the "scrape_metadata" field above. + # The consumer side of the relation uses this information to name the rules file + # that is written to the filesystem. + relation.data[self._charm.app]["alert_rules"] = json.dumps(alert_rules_as_dict) def _set_unit_ip(self, _=None): """Set unit host address. diff --git a/pyproject.toml b/pyproject.toml index 8d112b7..4b0f882 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -43,3 +43,7 @@ pythonPlatfrom = "All" [tool.pytest.ini_options] minversion = "6.0" log_cli_level = "INFO" + +[tool.codespell] +skip = ".git,.tox,build,lib,venv*,.mypy_cache" +ignore-words-list = "assertIn" diff --git a/src/charm.py b/src/charm.py index 13619e0..ac4c011 100755 --- a/src/charm.py +++ b/src/charm.py @@ -130,7 +130,7 @@ def __init__(self, *args): self.metrics_aggregator = MetricsEndpointAggregator(self, resolve_addresses=True) self.cos_agent = COSAgentProvider( self, - scrape_configs=self._get_scrape_configs, + scrape_configs=self._get_scrape_configs(), metrics_rules_dir=RULES_DIR, dashboard_dirs=[COS_PROXY_DASHBOARDS_DIR, DASHBOARDS_DIR], refresh_events=[ diff --git a/tests/unit/test_relation_monitors.py b/tests/unit/test_relation_monitors.py index cf1f14a..47334f1 100644 --- a/tests/unit/test_relation_monitors.py +++ b/tests/unit/test_relation_monitors.py @@ -1,3 +1,4 @@ +import json import tempfile import unittest from pathlib import Path @@ -24,6 +25,7 @@ def setUp(self): "private-address": "10.41.168.226", "target-address": "10.41.168.226", "target-id": "ubuntu-0", + "nagios_host_context": "my-nagios-host-context", } for p in [ @@ -97,10 +99,21 @@ def test_prometheus(self): # THEN alert rules are transferred to prometheus over relation data app_data = self.harness.get_relation_data(rel_id_prom, "cos-proxy") - self.assertIn("alert_rules", app_data) + + self.assertIn("alert_rules", app_data) # pyright: ignore # AND status is "active" self.assertIsInstance( self.harness.model.unit.status, ActiveStatus, ) + # AND relabel configs are ok (we are removing nagios_host_context) + scrape_jobs = json.loads(app_data["scrape_jobs"]) + for job in scrape_jobs: + relabel_configs = job["relabel_configs"] + for config in relabel_configs: + if target_level := config.get("target_label"): + if target_level == "juju_application": + self.assertEquals(config["replacement"], "ubuntu") + elif target_level == "juju_unit": + self.assertEquals(config["replacement"], "ubuntu/0") diff --git a/tox.ini b/tox.ini index 2a52d0a..bfb2358 100644 --- a/tox.ini +++ b/tox.ini @@ -44,7 +44,7 @@ deps = codespell<2.3.0 # https://github.com/codespell-project/codespell/issues/3430 commands = codespell {[vars]lib_path} - codespell . --skip .git --skip .tox --skip build --skip lib --skip venv* --skip .mypy_cache + codespell . ruff {[vars]all_path} black --check --diff {[vars]all_path}