From dbf3554bafa5f1ec6aaf6a83a1a4e0a6428c5fca Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Wed, 30 Sep 2026 12:22:28 +0200 Subject: [PATCH] fix(prometheus): resolve the native-metrics scrape target per deployment mode `native_metrics_target` was a Jinja filter, so it could not read DEPLOYMENT_MODE: filters see no Ansible variables. It returned `.name>:` unconditionally, which is `matrix-synapse:9000` for matrix. Compose serves that name; swarm does not, because `docker stack deploy` ignores `container_name` and the alias never exists. This repository says as much itself, through `lookup('compose_only', 'container_name', ...)`. The target was therefore scraped and refused, and `playwright.spec.js:274` reported `web-app-matrix: 1 matching target(s) reporting up != 1`, meaning one matching target whose `up` value is not 1. It becomes a lookup that delegates host resolution to `scrape_target`, which was already mode-aware, so exactly one plugin knows the difference between the two modes. The port has to be handed over explicitly: matrix exposes metrics on 9000 while `services.synapse.ports.internal.http` is 8008, so the existing internal-port lookup cannot serve this case on its own. `scrape_target` now refuses `port` together with an explicit `port_kind`. Before, `port` won silently and the error text still named the `ports.internal.` path that the call had never consulted. `native_metrics_apps` drops an app that pins itself to compose through `compose_mode_force` while the cluster runs swarm, because the scrape target would then name the wrong object. Only the literal case: matrix declares that override as a Jinja expression over its flavour, which this lookup cannot resolve from the scraping role's variable scope, so variant 0 on swarm stays a latent mismatch. The two comment lines claiming `container_name:port` went with the change; after it they state the opposite of what the code does. Verified by mutation: with the swarm branch of `scrape_target` disabled, the new test emits `matrix-synapse:9000`, the exact string CI reported. Co-Authored-By: Claude Opus 5 (1M context) --- plugins/filter/native_metrics_target.py | 51 -------- plugins/lookup/native_metrics_apps.py | 32 ++++- plugins/lookup/native_metrics_target.py | 68 ++++++++++ plugins/lookup/scrape_target.py | 17 ++- .../web-app-gitea/templates/prometheus.yml.j2 | 2 +- .../templates/prometheus.yml.j2 | 4 +- .../templates/prometheus.yml.j2 | 4 +- .../templates/prometheus.yml.j2 | 2 +- .../filter/test_native_metrics_target.py | 111 ---------------- .../lookup/test_native_metrics_target.py | 119 ++++++++++++++++++ 10 files changed, 234 insertions(+), 176 deletions(-) delete mode 100644 plugins/filter/native_metrics_target.py create mode 100644 plugins/lookup/native_metrics_target.py delete mode 100644 tests/unit/python/plugins/filter/test_native_metrics_target.py create mode 100644 tests/unit/python/plugins/lookup/test_native_metrics_target.py diff --git a/plugins/filter/native_metrics_target.py b/plugins/filter/native_metrics_target.py deleted file mode 100644 index cade81dc24..0000000000 --- a/plugins/filter/native_metrics_target.py +++ /dev/null @@ -1,51 +0,0 @@ -from __future__ import annotations - -from ansible.errors import AnsibleFilterError - -from utils.roles.entity.name import get_entity_name - - -def native_metrics_target(app_id: str, applications: dict) -> str: - """Return the container-to-container Prometheus scrape target for a native-metrics app. - - Prometheus connects to the app's Docker network (declared in prometheus's - compose.yml.j2 via the native_metrics_apps loop) and scrapes by container - name and internal port — no host port binding required, no nginx/OAuth2 in - the path. - - Usage in a per-app prometheus.yml.j2 fragment: - targets: ["{{ native_prometheus_application_id | native_metrics_target(lookup('applications')) }}"] - - When the service key differs from the app's entity name (e.g. matrix - uses service key "synapse"), set services.prometheus.native_metrics.service_key - in the app's meta/services.yml to override. - """ - app_conf = applications.get(app_id, {}) - services = app_conf.get("services", {}) if isinstance(app_conf, dict) else {} - native_metrics_conf = ( - services.get("prometheus", {}) if isinstance(services, dict) else {} - ).get("native_metrics", {}) - service_key = native_metrics_conf.get("service_key") or get_entity_name(app_id) - - container = ( - services.get(service_key, {}).get("name") - if isinstance(services, dict) - else None - ) - if not container: - raise AnsibleFilterError( - f"native_metrics_target: no services.{service_key}.name for '{app_id}'" - ) - - port = native_metrics_conf.get("port") - if port is None: - raise AnsibleFilterError( - f"native_metrics_target: no services.prometheus.native_metrics.port for '{app_id}'" - ) - - return f"{container}:{port}" - - -class FilterModule: - def filters(self): - return {"native_metrics_target": native_metrics_target} diff --git a/plugins/lookup/native_metrics_apps.py b/plugins/lookup/native_metrics_apps.py index 6a186041d3..68bc93548f 100644 --- a/plugins/lookup/native_metrics_apps.py +++ b/plugins/lookup/native_metrics_apps.py @@ -1,12 +1,35 @@ from __future__ import annotations +from collections.abc import Mapping from typing import Any from ansible.plugins.loader import lookup_loader from ansible.plugins.lookup import LookupBase from utils.cache import ROLES_DIR +from utils.cache.yaml import load_yaml_any from utils.roles.applications.config import get as get_app_conf +from utils.roles.mapping import ROLE_FILE_VARS_MAIN + + +def compose_mode_force(app_id: str) -> str: + """Return the role's own deploy-mode override. + + Args: + app_id: the role directory name. + + Returns: + The literal ``compose_mode_force`` from the role's vars, or ``""`` when + it is absent or a Jinja expression, which cannot be resolved from the + scraping role's variable scope and so leaves the override invisible here. + """ + path = ROLES_DIR / app_id / ROLE_FILE_VARS_MAIN + if not path.is_file(): + return "" + data = load_yaml_any(str(path), default_if_missing={}) + raw = data.get("compose_mode_force") if isinstance(data, Mapping) else None + value = "" if raw is None else str(raw).strip() + return "" if "{{" in value else value class LookupModule(LookupBase): @@ -16,8 +39,10 @@ class LookupModule(LookupBase): 2. a prometheus.yml.j2 template at roles//templates/ 3. reachable from this prometheus: NOT on a node-local force_bridge network under swarm, which a swarm prometheus can neither join nor - resolve. This is the single point that gates force_bridge apps out of - the native-metrics precreate + scrape. + resolve, and NOT pinned to compose by its own compose_mode_force while + the cluster runs swarm, which would make the scrape target name the + wrong object. This is the single point that gates unreachable apps out + of the native-metrics precreate + scrape. Used by web-app-prometheus/templates/configuration/prometheus.yml.j2 to auto-discover apps that expose a native /metrics endpoint without hardcoding each app name. @@ -78,6 +103,9 @@ class LookupModule(LookupBase): if is_swarm and bool(force_bridge): continue + if is_swarm and compose_mode_force(app_id) == "compose": + continue + result.append(app_id) return [result] diff --git a/plugins/lookup/native_metrics_target.py b/plugins/lookup/native_metrics_target.py new file mode 100644 index 0000000000..93140ee56e --- /dev/null +++ b/plugins/lookup/native_metrics_target.py @@ -0,0 +1,68 @@ +"""Lookup ``native_metrics_target``: scrape target for an app that exposes its +own metrics port. Reads ``services.prometheus.native_metrics`` for the service +key and port, then delegates host resolution to ``scrape_target`` so swarm and +compose stay on one implementation. + +Usage in a per-app prometheus.yml.j2 fragment: + targets: ["{{ lookup('native_metrics_target', native_prometheus_application_id) }}"] +""" + +from __future__ import annotations + +from typing import Any + +from ansible.errors import AnsibleError +from ansible.plugins.loader import lookup_loader +from ansible.plugins.lookup import LookupBase + +from utils.roles.applications.config import get +from utils.roles.entity.name import get_entity_name + + +class LookupModule(LookupBase): + def run( + self, + terms: list[Any] | None, + variables: dict[str, Any] | None = None, + **kwargs: Any, + ) -> list[str]: + if not terms: + raise AnsibleError( + "native_metrics_target lookup requires the application_id term" + ) + + application_id = str(terms[0]).strip() + if not application_id: + raise AnsibleError("native_metrics_target: application_id must be non-empty") + + vars_ = variables or getattr(self._templar, "available_variables", {}) or {} + applications = lookup_loader.get( + "applications", loader=self._loader, templar=getattr(self, "_templar", None) + ).run([], variables=vars_)[0] + + def conf(key: str) -> str: + value = get( + applications=applications, + application_id=application_id, + config_path=f"services.prometheus.native_metrics.{key}", + strict=False, + default="", + ) + return "" if value is None else str(value).strip() + + port = conf("port") + if not port: + raise AnsibleError( + "native_metrics_target: no " + f"services.prometheus.native_metrics.port for {application_id!r}" + ) + + service_key = conf("service_key") or get_entity_name(application_id) + if not service_key: + raise AnsibleError( + f"native_metrics_target: cannot derive service key for {application_id!r}" + ) + + return lookup_loader.get( + "scrape_target", loader=self._loader, templar=getattr(self, "_templar", None) + ).run([application_id, service_key], variables=vars_, port=port) diff --git a/plugins/lookup/scrape_target.py b/plugins/lookup/scrape_target.py index d99674e8d7..06190f50be 100644 --- a/plugins/lookup/scrape_target.py +++ b/plugins/lookup/scrape_target.py @@ -1,6 +1,7 @@ """Lookup ``scrape_target``: mode-aware ``:`` for a Prometheus scrape target. Swarm resolves ``tasks._`` (per-task DNS); -compose uses ``compose_host`` or the service container name.""" +compose uses ``compose_host`` or the service container name. ``port`` overrides +the ``services..ports.internal.`` lookup.""" from __future__ import annotations @@ -37,12 +38,20 @@ class LookupModule(LookupBase): if not service_key: raise AnsibleError("scrape_target: service_key must be non-empty") - port_kind = ( + explicit_port_kind = ( _as_str(terms[2]) if len(terms) > 2 else _as_str(kwargs.get("port_kind")) - ) or "http" + ) + port_kind = explicit_port_kind or "http" compose_host = ( _as_str(terms[3]) if len(terms) > 3 else _as_str(kwargs.get("compose_host")) ) + port_override = _as_str(kwargs.get("port")) + if port_override and explicit_port_kind: + raise AnsibleError( + f"scrape_target: {application_id!r} passed both port={port_override!r} " + f"and port_kind={explicit_port_kind!r}; port wins and port_kind would be " + "silently dropped, so pass only one" + ) vars_ = variables or getattr(self._templar, "available_variables", {}) or {} templar = getattr(self, "_templar", None) @@ -57,7 +66,7 @@ class LookupModule(LookupBase): "applications", loader=self._loader, templar=getattr(self, "_templar", None) ).run([], variables=vars_)[0] - port = _as_str( + port = port_override or _as_str( get( applications=applications, application_id=application_id, diff --git a/roles/web-app-gitea/templates/prometheus.yml.j2 b/roles/web-app-gitea/templates/prometheus.yml.j2 index 3583badebf..ba2fc8151c 100644 --- a/roles/web-app-gitea/templates/prometheus.yml.j2 +++ b/roles/web-app-gitea/templates/prometheus.yml.j2 @@ -9,6 +9,6 @@ bearer_token: "{{ _token }}" {% endif %} static_configs: - - targets: ["{{ native_prometheus_application_id | native_metrics_target(lookup('applications')) }}"] + - targets: ["{{ lookup('native_metrics_target', native_prometheus_application_id) }}"] labels: app: "{{ native_prometheus_application_id }}" diff --git a/roles/web-app-matrix/templates/prometheus.yml.j2 b/roles/web-app-matrix/templates/prometheus.yml.j2 index 79e65ead8f..ac5712173f 100644 --- a/roles/web-app-matrix/templates/prometheus.yml.j2 +++ b/roles/web-app-matrix/templates/prometheus.yml.j2 @@ -1,12 +1,10 @@ # Matrix Synapse — server and federation metrics. # Requires enable_metrics: true in homeserver.yaml. # Enable via native_metrics.enabled: true in inventory. - # Scraped container-to-container: prometheus joins the matrix Docker network and - # reaches synapse directly at container_name:port — no host binding, no nginx/OAuth2. - job_name: "matrix-synapse" scheme: http metrics_path: /_synapse/metrics static_configs: - - targets: ["{{ native_prometheus_application_id | native_metrics_target(lookup('applications')) }}"] + - targets: ["{{ lookup('native_metrics_target', native_prometheus_application_id) }}"] labels: app: "{{ native_prometheus_application_id }}" diff --git a/roles/web-app-mattermost/templates/prometheus.yml.j2 b/roles/web-app-mattermost/templates/prometheus.yml.j2 index a3b4a71621..67a7e7ae66 100644 --- a/roles/web-app-mattermost/templates/prometheus.yml.j2 +++ b/roles/web-app-mattermost/templates/prometheus.yml.j2 @@ -1,11 +1,9 @@ # Mattermost — application and Go runtime metrics. - # Scraped container-to-container: prometheus joins the mattermost Docker network and - # reaches the app directly at container_name:port — no host binding, no nginx/OAuth2. # Requires MM_METRICSSETTINGS_ENABLE=true and MM_METRICSSETTINGS_LISTENADDRESS (set in env.j2 when enabled). - job_name: "mattermost" scheme: http metrics_path: /metrics static_configs: - - targets: ["{{ native_prometheus_application_id | native_metrics_target(lookup('applications')) }}"] + - targets: ["{{ lookup('native_metrics_target', native_prometheus_application_id) }}"] labels: app: "{{ native_prometheus_application_id }}" diff --git a/roles/web-app-openbao/templates/prometheus.yml.j2 b/roles/web-app-openbao/templates/prometheus.yml.j2 index 3dd99470c9..9eedd90c6f 100644 --- a/roles/web-app-openbao/templates/prometheus.yml.j2 +++ b/roles/web-app-openbao/templates/prometheus.yml.j2 @@ -4,6 +4,6 @@ params: format: ["prometheus"] static_configs: - - targets: ["{{ native_prometheus_application_id | native_metrics_target(lookup('applications')) }}"] + - targets: ["{{ lookup('native_metrics_target', native_prometheus_application_id) }}"] labels: app: "{{ native_prometheus_application_id }}" diff --git a/tests/unit/python/plugins/filter/test_native_metrics_target.py b/tests/unit/python/plugins/filter/test_native_metrics_target.py deleted file mode 100644 index e2d773c2e5..0000000000 --- a/tests/unit/python/plugins/filter/test_native_metrics_target.py +++ /dev/null @@ -1,111 +0,0 @@ -from __future__ import annotations - -import unittest -from unittest.mock import patch - -from ansible.errors import AnsibleFilterError - -from plugins.filter.native_metrics_target import native_metrics_target - - -def _make_applications( - app_id: str, - container: str, - port: int, - entity_name: str, - service_key: str | None = None, -) -> dict: - native_metrics: dict = {"port": port} - if service_key is not None: - native_metrics["service_key"] = service_key - return { - app_id: { - "services": { - entity_name: {"name": container}, - "prometheus": {"native_metrics": native_metrics}, - } - } - } - - -class TestNativeMetricsTargetSuccess(unittest.TestCase): - def test_returns_container_colon_port(self): - apps = _make_applications("web-app-gitea", "gitea", 3000, "gitea") - with patch( - "plugins.filter.native_metrics_target.get_entity_name", return_value="gitea" - ): - result = native_metrics_target("web-app-gitea", apps) - self.assertEqual(result, "gitea:3000") - - def test_different_app(self): - apps = _make_applications( - "web-app-mattermost", "mattermost", 8067, "mattermost" - ) - with patch( - "plugins.filter.native_metrics_target.get_entity_name", - return_value="mattermost", - ): - result = native_metrics_target("web-app-mattermost", apps) - self.assertEqual(result, "mattermost:8067") - - def test_service_key_override(self): - apps = _make_applications( - "web-app-matrix", "matrix-synapse", 9000, "synapse", service_key="synapse" - ) - with patch( - "plugins.filter.native_metrics_target.get_entity_name", - return_value="matrix", - ): - result = native_metrics_target("web-app-matrix", apps) - self.assertEqual(result, "matrix-synapse:9000") - - -class TestNativeMetricsTargetErrors(unittest.TestCase): - def test_raises_when_container_name_missing(self): - apps = { - "web-app-gitea": { - "services": { - "prometheus": {"native_metrics": {"port": 3000}}, - } - } - } - with ( - patch( - "plugins.filter.native_metrics_target.get_entity_name", - return_value="gitea", - ), - self.assertRaises(AnsibleFilterError), - ): - native_metrics_target("web-app-gitea", apps) - - def test_raises_when_port_missing(self): - apps = { - "web-app-gitea": { - "services": { - "gitea": {"name": "gitea"}, - "prometheus": {"native_metrics": {}}, - } - } - } - with ( - patch( - "plugins.filter.native_metrics_target.get_entity_name", - return_value="gitea", - ), - self.assertRaises(AnsibleFilterError), - ): - native_metrics_target("web-app-gitea", apps) - - def test_raises_when_app_not_in_applications(self): - with ( - patch( - "plugins.filter.native_metrics_target.get_entity_name", - return_value="gitea", - ), - self.assertRaises(AnsibleFilterError), - ): - native_metrics_target("web-app-gitea", {}) - - -if __name__ == "__main__": - unittest.main() diff --git a/tests/unit/python/plugins/lookup/test_native_metrics_target.py b/tests/unit/python/plugins/lookup/test_native_metrics_target.py new file mode 100644 index 0000000000..e3abbdb9dd --- /dev/null +++ b/tests/unit/python/plugins/lookup/test_native_metrics_target.py @@ -0,0 +1,119 @@ +"""Tests for the native_metrics_target lookup. + +The lookup delegates host resolution to scrape_target, so these run the real +scrape_target rather than a stub: the regression being guarded is that swarm +must not receive a compose container name, and only the delegation proves it. +""" + +from __future__ import annotations + +import unittest +from typing import Any +from unittest.mock import MagicMock, patch + +from ansible.errors import AnsibleError + +from plugins.lookup.native_metrics_target import LookupModule +from plugins.lookup.scrape_target import LookupModule as ScrapeTargetLookup + +ENTITIES = {"web-app-matrix": "matrix", "web-app-gitea": "gitea"} + + +class _Templar: + def __init__(self, vars_=None): + self.available_variables = vars_ or {} + + def template(self, value, **_): + return value + + +def _apps(native_metrics: dict[str, Any]) -> dict[str, Any]: + return { + "web-app-matrix": { + "services": { + "prometheus": {"native_metrics": native_metrics}, + "synapse": { + "name": "matrix-synapse", + "ports": {"internal": {"http": 8008}}, + }, + }, + }, + } + + +def _run(terms, *, mode, apps): + variables = {"DEPLOYMENT_MODE": mode} + lookup = LookupModule() + lookup._templar = _Templar(variables) + lookup._loader = MagicMock() + + def _dispatch(name, **_kwargs): + if name == "applications": + return MagicMock(run=lambda *_a, **_k: [apps]) + scrape = ScrapeTargetLookup() + scrape._templar = _Templar(variables) + scrape._loader = MagicMock() + return scrape + + with ( + patch( + "plugins.lookup.native_metrics_target.lookup_loader" + ) as native_loader_mock, + patch("plugins.lookup.scrape_target.lookup_loader") as scrape_loader_mock, + patch( + "plugins.lookup.native_metrics_target.get_entity_name", + side_effect=lambda app_id: ENTITIES.get(app_id, ""), + ), + patch( + "plugins.lookup.scrape_target.get_entity_name", + side_effect=lambda app_id: ENTITIES.get(app_id, ""), + ), + ): + native_loader_mock.get.side_effect = _dispatch + scrape_loader_mock.get.return_value = MagicMock(run=lambda *_a, **_k: [apps]) + return lookup.run(terms, variables=variables) + + +class TestNativeMetricsTargetLookup(unittest.TestCase): + def test_swarm_uses_per_task_dns_not_the_container_name(self): + apps = _apps({"port": 9000, "service_key": "synapse"}) + result = _run(["web-app-matrix"], mode="swarm", apps=apps) + self.assertEqual(result, ["tasks.matrix_synapse:9000"]) + + def test_compose_uses_the_container_name(self): + apps = _apps({"port": 9000, "service_key": "synapse"}) + result = _run(["web-app-matrix"], mode="compose", apps=apps) + self.assertEqual(result, ["matrix-synapse:9000"]) + + def test_service_key_defaults_to_the_entity_name(self): + apps = { + "web-app-gitea": { + "services": { + "prometheus": {"native_metrics": {"port": 3000}}, + "gitea": { + "name": "gitea", + "ports": {"internal": {"http": 3000}}, + }, + }, + }, + } + result = _run(["web-app-gitea"], mode="swarm", apps=apps) + self.assertEqual(result, ["tasks.gitea_gitea:3000"]) + + def test_native_metrics_port_takes_precedence_over_the_internal_port(self): + apps = _apps({"port": 9000, "service_key": "synapse"}) + result = _run(["web-app-matrix"], mode="compose", apps=apps) + self.assertNotIn("8008", result[0]) + + def test_missing_port_raises(self): + apps = _apps({"service_key": "synapse"}) + with self.assertRaises(AnsibleError): + _run(["web-app-matrix"], mode="compose", apps=apps) + + def test_missing_application_id_raises(self): + with self.assertRaises(AnsibleError): + _run([], mode="compose", apps=_apps({"port": 9000})) + + +if __name__ == "__main__": + unittest.main()