diff --git a/lib/charms/prometheus_k8s/v0/prometheus_scrape.py b/lib/charms/prometheus_k8s/v0/prometheus_scrape.py index e497e57d..84f2f962 100644 --- a/lib/charms/prometheus_k8s/v0/prometheus_scrape.py +++ b/lib/charms/prometheus_k8s/v0/prometheus_scrape.py @@ -311,6 +311,8 @@ def _on_scrape_targets_changed(self, event): """ # noqa: W505 +import copy +import hashlib import ipaddress import json import logging @@ -881,6 +883,8 @@ def jobs(self) -> list: if static_scrape_jobs: scrape_jobs.extend(static_scrape_jobs) + scrape_jobs = _dedupe_job_names(scrape_jobs) + return scrape_jobs def alerts(self) -> dict: @@ -1235,6 +1239,49 @@ def _labeled_unit_config( return static_config +def _dedupe_job_names(jobs: List[dict]): + """Deduplicate a list of dicts by appending a hash to the value of the 'job_name' key. + + Additionally fully dedeuplicate any identical jobs. + + Args: + jobs: A list of prometheus scrape jobs + """ + jobs_copy = copy.deepcopy(jobs) + + # Convert to a dict with job names as keys + # I think this line is O(n^2) but it should be okay given the list sizes + jobs_dict = { + job["job_name"]: list(filter(lambda x: x["job_name"] == job["job_name"], jobs_copy)) + for job in jobs_copy + } + + # If multiple jobs have the same name, convert the name to "name_" + for key in jobs_dict: + if len(jobs_dict[key]) > 1: + for job in jobs_dict[key]: + job_json = json.dumps(job) + hashed = hashlib.sha256(job_json.encode()).hexdigest() + job["job_name"] = "{}_{}".format(job["job_name"], hashed) + new_jobs = [] + for key in jobs_dict: + new_jobs.extend([i for i in jobs_dict[key]]) + + # Deduplicate jobs which are equal + # Again this in O(n^2) but it should be okay + deduped_jobs = [] + seen = [] + for job in new_jobs: + job_json = json.dumps(job) + hashed = hashlib.sha256(job_json.encode()).hexdigest() + if hashed in seen: + continue + seen.append(hashed) + deduped_jobs.append(job) + + return deduped_jobs + + def _resolve_dir_against_charm_path(charm: CharmBase, *path_elements: str) -> str: """Resolve the provided path items against the directory of the main file. diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 74735188..8af24599 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -82,7 +82,7 @@ async def get_prometheus_config(ops_test: OpsTest, app_name: str, unit_num: int) async def get_prometheus_active_targets( - ops_test: OpsTest, app_name: str, unit_num: int + ops_test: OpsTest, app_name: str, unit_num: int = 0 ) -> List[dict]: """Fetch Prometheus active scrape targets. diff --git a/tests/integration/prometheus-tester/config.yaml b/tests/integration/prometheus-tester/config.yaml index 62b606c2..50cd7328 100644 --- a/tests/integration/prometheus-tester/config.yaml +++ b/tests/integration/prometheus-tester/config.yaml @@ -10,3 +10,7 @@ options: default: src/prometheus_alert_rules description: "Path for alert rules passed to the Provider" type: string + scrape_jobs: + default: "[]" + description: "Scrape jobs to pass to the MetricsEndpointProvider constructor" + type: string diff --git a/tests/integration/prometheus-tester/src/charm.py b/tests/integration/prometheus-tester/src/charm.py index 8697d4a4..25897755 100755 --- a/tests/integration/prometheus-tester/src/charm.py +++ b/tests/integration/prometheus-tester/src/charm.py @@ -4,6 +4,7 @@ """A Charm to functionally test the Prometheus Operator.""" +import json import logging from pathlib import Path @@ -24,12 +25,13 @@ def __init__(self, *args): self._name = "prometheus-tester" self._pip_path = "/usr/local/bin/pip" self._metrics_exporter_script = Path("src/metrics.py") - jobs = [ - { - "scrape_interval": self.model.config["scrape-interval"], - "static_configs": [{"targets": ["*:8000"], "labels": {"name": self._name}}], - } - ] + if not (jobs := json.loads(self.config["scrape_jobs"])): + jobs = [ + { + "scrape_interval": self.model.config["scrape-interval"], + "static_configs": [{"targets": ["*:8000"], "labels": {"name": self._name}}], + } + ] logger.warning("Rules path is: %s", self.model.config["alert-rules-path"]) self.prometheus = MetricsEndpointProvider( self, jobs=jobs, alert_rules_path=self.model.config["alert-rules-path"] diff --git a/tests/integration/test_deduplication.py b/tests/integration/test_deduplication.py new file mode 100644 index 00000000..390402bc --- /dev/null +++ b/tests/integration/test_deduplication.py @@ -0,0 +1,59 @@ +#!/usr/bin/env python3 +# Copyright 2022 Canonical Ltd. +# See LICENSE file for licensing details. + +import asyncio +import json + +from helpers import get_prometheus_active_targets, oci_image +from pytest_operator.plugin import OpsTest + +prometheus_app_name = "prometheus" +prometheus_resources = {"prometheus-image": oci_image("./metadata.yaml", "prometheus-image")} +tester_app_name = "tester" +tester_resources = { + "prometheus-tester-image": oci_image( + "./tests/integration/prometheus-tester/metadata.yaml", + "prometheus-tester-image", + ) +} + + +async def test_multiple_scrape_jobs_in_constructor( + ops_test: OpsTest, prometheus_charm, prometheus_tester_charm +): + """Test that job names are properly deduped when in the same consumer unit.""" + jobs = [ + { + "scrape_interval": "10s", + "static_configs": [{"targets": ["*:8000"]}], + }, + { + "scrape_interval": "10s", + "static_configs": [{"targets": ["*:8000"]}], + }, + { + "scrape_interval": "10s", + "static_configs": [{"targets": ["*:8001"]}], + }, + ] + await asyncio.gather( + ops_test.model.deploy( + prometheus_charm, + resources=prometheus_resources, + application_name=prometheus_app_name, + trust=True, + ), + ops_test.model.deploy( + prometheus_tester_charm, + resources=tester_resources, + application_name=tester_app_name, + config={"scrape_jobs": json.dumps(jobs)}, + ), + ) + await ops_test.model.add_relation(prometheus_app_name, tester_app_name) + await ops_test.model.wait_for_idle(status="active") + + targets = await get_prometheus_active_targets(ops_test, prometheus_app_name) + # Two unique jobs above plus an additional an additional job for self scraping. + assert len(targets) == 3 diff --git a/tests/unit/test_functions.py b/tests/unit/test_functions.py new file mode 100644 index 00000000..fb69af9f --- /dev/null +++ b/tests/unit/test_functions.py @@ -0,0 +1,65 @@ +# Copyright 2022 Canonical Ltd. +# See LICENSE file for licensing details. + +import copy +import unittest + +import deepdiff +from charms.prometheus_k8s.v0.prometheus_scrape import _dedupe_job_names + + +class TestFunctions(unittest.TestCase): + def test_dedupe_job_names(self): + jobs = [ + { + "job_name": "job0", + "static_configs": [{"targets": ["localhost:9090"]}], + "scrape_interval": "5s", + }, + { + "job_name": "job0", + "static_configs": [{"targets": ["localhost:9090"]}], + "scrape_interval": "5s", + }, + { + "job_name": "job1", + "static_configs": [{"targets": ["localhost:9090"]}], + "scrape_interval": "5s", + }, + { + "job_name": "job0", + "static_configs": [{"targets": ["localhost:9090"]}], + "scrape_interval": "10s", + }, + { + "job_name": "job0", + "static_configs": [{"targets": ["localhost:9091"]}], + "scrape_interval": "5s", + }, + ] + jobs_original = copy.deepcopy(jobs) + expected = [ + { + "job_name": "job0_6f9f1c305506707b952aef3885fa099fe36158f6359b8a06634068270645aefd", + "scrape_interval": "5s", + "static_configs": [{"targets": ["localhost:9090"]}], + }, + { + "job_name": "job0_c651cf3a8cd1b85abc0cf7620e058b87ef43e2296d1520328ce5a796e9b20993", + "scrape_interval": "10s", + "static_configs": [{"targets": ["localhost:9090"]}], + }, + { + "job_name": "job0_546b5bbb56e719d894b0a557975e0926ed093ea547c87051595d953122d2a7d6", + "scrape_interval": "5s", + "static_configs": [{"targets": ["localhost:9091"]}], + }, + { + "job_name": "job1", + "scrape_interval": "5s", + "static_configs": [{"targets": ["localhost:9090"]}], + }, + ] + self.assertTrue(len(deepdiff.DeepDiff(_dedupe_job_names(jobs), expected)) == 0) + # Make sure the function does not modify its argument + self.assertEqual(jobs, jobs_original)