From 6c9a319a832e13f99c4e1d2cc4fe0ec6b71d0e1b Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Fri, 18 Sep 2026 11:15:54 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(backend)=20add=20opt-in=20prometheus?= =?UTF-8?q?=20metrics=20on=20`/metrics`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We want to export metrics to prometheus. We install for this django-prometheus. By default the feature is disabled and must be explicitly enabled. A complete documentation is available in documentation/metrics.md --- CHANGELOG.md | 3 + documentation/env.md | 4 + documentation/metrics.md | 122 +++++++++++++ src/backend/core/metrics.py | 58 ++++++ src/backend/core/middleware.py | 42 ++++- .../core/tests/test_prometheus_metrics.py | 170 ++++++++++++++++++ src/backend/core/tests/test_settings.py | 149 +++++++++++++++ src/backend/impress/settings.py | 94 ++++++++++ src/backend/impress/urls.py | 12 ++ src/backend/pyproject.toml | 2 + src/backend/uv.lock | 30 +++- src/helm/impress/README.md | 10 ++ .../impress/templates/ingress_metrics.yaml | 90 ++++++++++ src/helm/impress/values.yaml | 27 +++ 14 files changed, 809 insertions(+), 4 deletions(-) create mode 100644 documentation/metrics.md create mode 100644 src/backend/core/metrics.py create mode 100644 src/backend/core/tests/test_prometheus_metrics.py create mode 100644 src/helm/impress/templates/ingress_metrics.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index c764d5aed..d895f3e7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ and this project adheres to - 🐛(frontend) reduce PostHog volume from web vitals and opt_in spam #2701 - ✨(backend) expose the attachment max size in the config endpoint #2577 - ✨(frontend) warn before uploading an attachment over the size limit #2577 +- ✨(backend) add opt-in prometheus metrics on `/metrics`, protected by a + bearer token +- ✨(helm) add a dedicated ingress for the prometheus metrics - ✨(backend) add a service generating cached RS256 JWT tokens - ✨(backend) publish the JWT public key on a JWKS endpoint - 🔧(dev) generate the JWT signing key when bootstrapping the dev stack diff --git a/documentation/env.md b/documentation/env.md index baa60ef96..008384900 100644 --- a/documentation/env.md +++ b/documentation/env.md @@ -133,6 +133,10 @@ These are the environment variables you can set for the `impress-backend` contai | OIDC_USERINFO_SHORTNAME_FIELD | OIDC token claims to create shortname | first_name | | OIDC_USE_NONCE | Use nonce for OIDC | true | | POSTHOG_KEY | Posthog key for analytics | | +| PROMETHEUS_API_KEY | Bearer token required on `/metrics`. Required when the metrics are enabled (the application refuses to start without it). Can be given as a file with PROMETHEUS_API_KEY_FILE | | +| PROMETHEUS_DB_METRICS_ENABLED | When the metrics are enabled, also count and time the SQL queries (swaps the postgresql engine for the instrumented one of django-prometheus) | True | +| PROMETHEUS_METRICS_ENABLED | Instrument the application with django-prometheus and serve `/metrics`. OFF by default. See documentation/metrics.md | False | +| PROMETHEUS_MULTIPROC_DIR | Directory the uvicorn workers share their metrics through. Local to the host or pod, owned by the user running the application | `/impress-prometheus-` | | POSTHOG_HOST | Posthog host for analytics | | | REDIS_URL | Cache url | redis://redis:6379/1 | | SEARCH_INDEXER_BATCH_SIZE | Size of each batch for indexation of all documents | 100000 | diff --git a/documentation/metrics.md b/documentation/metrics.md new file mode 100644 index 000000000..ce90ae078 --- /dev/null +++ b/documentation/metrics.md @@ -0,0 +1,122 @@ +# Prometheus metrics of the backend + +The backend ships an **opt-in** instrumentation, +[django-prometheus](https://github.com/django-commons/django-prometheus). It is +**disabled by default**: when `PROMETHEUS_METRICS_ENABLED` is unset, +`django_prometheus` is not in `INSTALLED_APPS`, its middlewares are not +installed, the database engine is left alone and `/metrics` is not routed. + +## Enabling it + +```bash +PROMETHEUS_METRICS_ENABLED=True +PROMETHEUS_API_KEY= # or PROMETHEUS_API_KEY_FILE +``` + +The application **refuses to start** with the metrics enabled and no key: the +endpoint would otherwise answer to anybody. + +Then scrape `GET /metrics` with the key as a bearer token: + +```yaml +scrape_configs: + - job_name: docs-backend + scheme: https + metrics_path: /metrics + authorization: + type: Bearer + credentials_file: /etc/prometheus/docs-api-key + static_configs: + - targets: ["metrics.docs.example.com"] +``` + +## What is measured + +- HTTP requests: counts and latency histograms, labelled by **view name**, + method and status + (`django_http_requests_latency_seconds_by_view_method`, + `django_http_responses_total_by_status_view_method_total`, ...). +- SQL (unless `PROMETHEUS_DB_METRICS_ENABLED=False`): queries, errors and query + duration (`django_db_execute_total`, `django_db_query_duration_seconds`, ...). + With `DB_PSYCOPG_POOL_ENABLED`, `django_db_new_connections_total` counts the + connections taken from the pool, not the connections opened to Postgres. + +No label ever carries a path, a user or a document identifier. A request that +matches no route is counted under ``. + +Every sample carries a `hostname` label (the pod name in Kubernetes), see +[Several replicas](#several-replicas-behind-one-address). + +## How the endpoint is protected + +| Layer | What it guarantees | +|---|---| +| Off by default | Nothing is installed, and `/metrics` is a 404, unless the deployment asks for it | +| Served on `/metrics`, not under `/api/` | The ingress of the application publishes `/api` and `/external_api` only: the metrics are not reachable from outside until a route is added on purpose | +| Bearer token | `core.middleware.PrometheusAuthMiddleware` refuses anything but `Authorization: Bearer `, compared in constant time. It fails closed: no key, no answer | +| First middleware | A refused call stops there: no session is created, no database or cache access is made | +| Dedicated ingress (chart) | One exact path on a host of its own, where the callers are filtered by address | +| Labels | No path, user or document identifier leaves the application | + +The key is a long-lived shared secret, which is what a Prometheus scraper can +present. Treat it as such: give it through `PROMETHEUS_API_KEY_FILE` or a +Kubernetes secret, keep TLS on wherever it travels, and rotate it by changing +the value on both sides. + +## uvicorn workers + +uvicorn runs several worker processes (`WEB_CONCURRENCY`) and a scrape is +answered by one of them. The workers therefore all write their numbers to one +directory (prometheus_client's multiprocess mode), and whichever answers adds +them up. Nothing has to be configured: the directory defaults to +`/impress-prometheus-`, is created with mode `0700`, and the +application refuses a directory owned by another user or a symbolic link. Set +`PROMETHEUS_MULTIPROC_DIR` to put it elsewhere — it must be writable, and local +to the host or the pod: it is **not** shared between replicas. + +Known limit: uvicorn recycles its workers (`--limit-max-requests`) and has no +hook to tell when one is gone. Every new worker writes two new 64 KiB files, +and the files of the workers that are gone must stay — their counters are part +of the totals. The directory grows, and the scrape gets slower, until the +container is replaced. Watch `scrape_duration_seconds` on long runs. Outside of +a container, empty the directory when the application is stopped. + +## Several replicas behind one address + +A scrape that goes through a load balancer — the dedicated ingress in front of +several backend pods — is answered by a different replica each time. The +`hostname` label keeps the replicas apart: each has its own series, and its +counters never go backwards. Each series is only sampled when its replica +happens to answer, so with `N` replicas expect one sample every `N` scrapes on +average: use a short scrape interval and rate windows of several minutes, and +aggregate with `sum without (hostname) (rate(...[5m]))`. + +This degrades as the number of replicas grows. A Prometheus running inside the +cluster should scrape each pod directly instead (a `PodMonitor` or pod +discovery on the `http` port, same path, same bearer token). + +## Kubernetes (Helm chart) + +```yaml +backend: + envVars: + PROMETHEUS_METRICS_ENABLED: "True" + PROMETHEUS_API_KEY: + secretKeyRef: + name: backend + key: PROMETHEUS_API_KEY + DJANGO_ALLOWED_HOSTS: docs.example.com,metrics.docs.example.com + +ingressMetrics: + enabled: true + host: metrics.docs.example.com + annotations: + nginx.ingress.kubernetes.io/whitelist-source-range: "203.0.113.10/32" +``` + +`ingressMetrics` routes the exact path `/metrics` of that host to the backend +and nothing else. Its host has to be in `DJANGO_ALLOWED_HOSTS`. + +The celery worker receives `backend.envVars` too. It serves no request, so its +metrics are never read: turn them off there with +`backend.celery.envVars.PROMETHEUS_METRICS_ENABLED: "False"`. diff --git a/src/backend/core/metrics.py b/src/backend/core/metrics.py new file mode 100644 index 000000000..cdf5718d9 --- /dev/null +++ b/src/backend/core/metrics.py @@ -0,0 +1,58 @@ +"""Prometheus metrics of the application, as served on /metrics. + +Enabled by PROMETHEUS_METRICS_ENABLED and protected by +`core.middleware.PrometheusAuthMiddleware` — see documentation/metrics.md. +""" + +import socket + +from django.http import HttpResponse +from django.views.decorators.http import require_safe + +from prometheus_client import CONTENT_TYPE_LATEST, CollectorRegistry, generate_latest +from prometheus_client.core import Metric +from prometheus_client.multiprocess import MultiProcessCollector + +HOSTNAME_LABEL = "hostname" + + +class HostnameLabelCollector: + """ + The metrics of every worker process of this host, labelled with its name. + + uvicorn runs several workers and the scrape is answered by one of them: the + multiprocess collector reads what they all wrote to PROMETHEUS_MULTIPROC_DIR + and adds it up, instead of reporting the numbers of that worker alone. + + The label is for a scraper that goes through a load balancer (an ingress in + front of several replicas): each scrape is then answered by a different + replica, and without it their counters would land in the same series, which + would jump from the numbers of one replica to those of another. With it, + each replica has its own series, sampled whenever it happens to answer. + """ + + def __init__(self): + self.hostname = socket.gethostname() + + def collect(self): + """Yield the aggregated metrics with the hostname added to every sample.""" + # a registry of its own: the collector registers itself in the one it is given + for metric in MultiProcessCollector(CollectorRegistry()).collect(): + labelled = Metric(metric.name, metric.documentation, metric.type) + for sample in metric.samples: + labelled.add_sample( + sample.name, + {**sample.labels, HOSTNAME_LABEL: self.hostname}, + sample.value, + sample.timestamp, + sample.exemplar, + ) + yield labelled + + +@require_safe +def metrics_view(request): + """Serve the metrics in the Prometheus text format.""" + registry = CollectorRegistry() + registry.register(HostnameLabelCollector()) + return HttpResponse(generate_latest(registry), content_type=CONTENT_TYPE_LATEST) diff --git a/src/backend/core/middleware.py b/src/backend/core/middleware.py index b54f56868..c4720426e 100644 --- a/src/backend/core/middleware.py +++ b/src/backend/core/middleware.py @@ -1,13 +1,24 @@ -"""Force session creation for all requests.""" +"""Custom middlewares of the impress core application.""" +from secrets import compare_digest + +from django.conf import settings +from django.http import HttpResponse from django.utils.deprecation import MiddlewareMixin +# Where the Prometheus metrics are served when PROMETHEUS_METRICS_ENABLED is set. +# Defined here rather than next to the view so that this module, which is always +# loaded, does not import prometheus_client when the metrics are disabled. +METRICS_PATH = "/metrics" + # Paths that must never touch the session store. Liveness and readiness # probes should never create a new session in redis. Also, the liveness probe -# should never reach redis before returning its answer. +# should never reach redis before returning its answer. A scraper carries no +# cookie either: each of its calls would leave one more session behind. SESSION_EXEMPT_PATHS = ( "/__lbheartbeat__", "/__heartbeat__", + METRICS_PATH, ) @@ -37,3 +48,30 @@ class SaveRawBodyMiddleware(MiddlewareMixin): """Save the raw request body in the request to use it later.""" if request.path.endswith(("/ai-proxy/", "/ai-proxy")): request.raw_body = request.body + + +class PrometheusAuthMiddleware(MiddlewareMixin): + """ + Require PROMETHEUS_API_KEY as a bearer token on the metrics endpoint. + + Installed first, and only when PROMETHEUS_METRICS_ENABLED is set. It fails + closed: with no key configured nothing is served, whatever is presented. + """ + + def process_request(self, request): + """Refuse a call to the metrics endpoint that does not present the key.""" + if request.path.rstrip("/") != METRICS_PATH: + return None + + api_key = settings.PROMETHEUS_API_KEY + authorization = request.headers.get("Authorization") or "" + # compared as bytes: `compare_digest` refuses non-ASCII strings, and + # what a caller sends in a header is not ours to trust + if not api_key or not compare_digest( + authorization.encode(), f"Bearer {api_key}".encode() + ): + response = HttpResponse("Unauthorized", status=401) + response["WWW-Authenticate"] = 'Bearer realm="metrics"' + return response + + return None diff --git a/src/backend/core/tests/test_prometheus_metrics.py b/src/backend/core/tests/test_prometheus_metrics.py new file mode 100644 index 000000000..a361547a4 --- /dev/null +++ b/src/backend/core/tests/test_prometheus_metrics.py @@ -0,0 +1,170 @@ +""" +Test the Prometheus metrics endpoint and the instrumentation of the application. +""" + +import socket +from importlib import reload + +from django.test import override_settings +from django.urls import clear_url_caches, resolve + +import pytest +from prometheus_client import REGISTRY, Counter, values +from rest_framework.test import APIClient + +from impress import urls + +pytestmark = pytest.mark.django_db + +API_KEY = "test-prometheus-api-key" +AUTH_MIDDLEWARE = "core.middleware.PrometheusAuthMiddleware" +BEFORE_MIDDLEWARE = "django_prometheus.middleware.PrometheusBeforeMiddleware" +AFTER_MIDDLEWARE = "django_prometheus.middleware.PrometheusAfterMiddleware" + + +def _reload_urls(): + """The route only exists when the metrics are enabled at import time.""" + reload(urls) + clear_url_caches() + + +@pytest.fixture(name="metrics_enabled") +def metrics_enabled_fixture(settings, monkeypatch, tmp_path): + """Configure the application the way PROMETHEUS_METRICS_ENABLED does at startup.""" + settings.PROMETHEUS_METRICS_ENABLED = True + settings.PROMETHEUS_API_KEY = API_KEY + settings.MIDDLEWARE = [ + AUTH_MIDDLEWARE, + BEFORE_MIDDLEWARE, + *settings.MIDDLEWARE, + AFTER_MIDDLEWARE, + ] + # what a worker started with PROMETHEUS_MULTIPROC_DIR does on its own: + # write the values to that directory instead of keeping them in memory + monkeypatch.setenv("PROMETHEUS_MULTIPROC_DIR", str(tmp_path)) + monkeypatch.setattr(values, "ValueClass", values.MultiProcessValue()) + _reload_urls() + yield + settings.PROMETHEUS_METRICS_ENABLED = False + _reload_urls() + + +@pytest.mark.parametrize("path", ["/metrics", "/metrics/", "/prometheus/metrics"]) +def test_prometheus_metrics_disabled_by_default(path): + """Nothing should be served unless the metrics are enabled, key or not.""" + response = APIClient().get(path, HTTP_AUTHORIZATION=f"Bearer {API_KEY}") + + assert response.status_code == 404 + + +@pytest.mark.usefixtures("metrics_enabled") +@pytest.mark.parametrize( + "authorization", + [ + None, + "", + "Bearer", + "Bearer ", + "Bearer wrong-key", + f"Bearer {API_KEY} ", + f"bearer {API_KEY}", + f"Token {API_KEY}", + f"Basic {API_KEY}", + API_KEY, + "Bearer clé-non-ascii", + ], +) +def test_prometheus_metrics_requires_the_api_key(authorization): + """Anything but the exact bearer token should be refused, before any other work.""" + headers = {} if authorization is None else {"HTTP_AUTHORIZATION": authorization} + + response = APIClient().get("/metrics", **headers) + + assert response.status_code == 401 + assert response["WWW-Authenticate"] == 'Bearer realm="metrics"' + assert response.content == b"Unauthorized" + # refused before the session middleware: a scraper must not fill the session store + assert not response.cookies + + +@pytest.mark.usefixtures("metrics_enabled") +@pytest.mark.parametrize("api_key", [None, ""]) +@pytest.mark.parametrize("authorization", ["Bearer ", "Bearer None", "Bearer"]) +def test_prometheus_metrics_fails_closed_without_api_key( + settings, api_key, authorization +): + """With no key configured nothing should be served, whatever is presented.""" + settings.PROMETHEUS_API_KEY = api_key + + response = APIClient().get("/metrics", HTTP_AUTHORIZATION=authorization) + + assert response.status_code == 401 + + +@pytest.mark.usefixtures("metrics_enabled") +def test_prometheus_metrics_served_with_the_api_key(): + """ + The right key should get what every worker wrote to the shared directory, labelled + with the name of the host, and the call should not create a session. + """ + counter = Counter( + "docs_metrics_test", "A counter written by a worker.", registry=None + ) + counter.inc(3) + + response = APIClient().get("/metrics", HTTP_AUTHORIZATION=f"Bearer {API_KEY}") + + assert response.status_code == 200 + assert response["Content-Type"].startswith("text/plain") + assert ( + f'docs_metrics_test_total{{hostname="{socket.gethostname()}"}} 3.0' + in response.content.decode() + ) + assert not response.cookies + + +@pytest.mark.usefixtures("metrics_enabled") +def test_prometheus_metrics_is_read_only(): + """Only GET and HEAD should be answered.""" + response = APIClient().post("/metrics", HTTP_AUTHORIZATION=f"Bearer {API_KEY}") + + assert response.status_code == 405 + + +@pytest.mark.usefixtures("metrics_enabled") +@pytest.mark.parametrize( + "path", + ["/api/v1.0/metrics", "/api/v1.0/metrics/", "/api/v1.0/prometheus/metrics"], +) +def test_prometheus_metrics_not_served_under_the_api(path): + """The ingress publishes /api/: the metrics should not be found under it.""" + response = APIClient().get(path, HTTP_AUTHORIZATION=f"Bearer {API_KEY}") + + assert response.status_code == 404 + + +@pytest.mark.usefixtures("metrics_enabled") +def test_prometheus_metrics_other_routes_need_no_api_key(): + """The key should only be asked for on the metrics endpoint.""" + assert APIClient().get("/api/v1.0/config/").status_code == 200 + + +@override_settings(MIDDLEWARE=[BEFORE_MIDDLEWARE, AFTER_MIDDLEWARE]) +def test_prometheus_metrics_requests_are_labelled_by_view(): + """A request should be counted under the name of its view, never under its path.""" + path = "/api/v1.0/config/" + labels = {"status": "200", "view": resolve(path).view_name, "method": "GET"} + metric = "django_http_responses_total_by_status_view_method_total" + before = REGISTRY.get_sample_value(metric, labels) or 0 + + response = APIClient().get(path) + + assert response.status_code == 200 + assert REGISTRY.get_sample_value(metric, labels) == before + 1 + # no label of any sample carries the path that was requested + assert not any( + path in value + for family in REGISTRY.collect() + for sample in family.samples + for value in sample.labels.values() + ) diff --git a/src/backend/core/tests/test_settings.py b/src/backend/core/tests/test_settings.py index 44c2a4ce1..3f55d6583 100644 --- a/src/backend/core/tests/test_settings.py +++ b/src/backend/core/tests/test_settings.py @@ -2,6 +2,9 @@ Unit tests for the User model """ +import os +import tempfile + import pytest from impress.settings import Base @@ -64,3 +67,149 @@ def test_settings_psycopg_pool_enabled(monkeypatch): "timeout": 3, } } + + +PROMETHEUS_AUTH_MIDDLEWARE = "core.middleware.PrometheusAuthMiddleware" +PROMETHEUS_BEFORE_MIDDLEWARE = "django_prometheus.middleware.PrometheusBeforeMiddleware" +PROMETHEUS_AFTER_MIDDLEWARE = "django_prometheus.middleware.PrometheusAfterMiddleware" + + +def _prometheus_settings(**attributes): + """ + Build fake settings enabling the metrics. + + post_setup edits the lists and the database in place, which is the only way for Django + to see the change: copies keep it away from the settings the other tests run with. + """ + return type( + "TestSettings", + (Base,), + { + "PROMETHEUS_METRICS_ENABLED": True, + "PROMETHEUS_API_KEY": "a-key", + "INSTALLED_APPS": list(Base.INSTALLED_APPS), + "MIDDLEWARE": list(Base.MIDDLEWARE), + "DATABASES": {"default": {"ENGINE": "django.db.backends.postgresql"}}, + **attributes, + }, + ) + + +def test_settings_prometheus_metrics_not_enabled(monkeypatch): + """django-prometheus should be absent from the settings unless it is asked for.""" + monkeypatch.delenv("PROMETHEUS_MULTIPROC_DIR", raising=False) + + class TestSettings(Base): + """Fake test settings without enabling the metrics.""" + + TestSettings.post_setup() + + assert "django_prometheus" not in TestSettings.INSTALLED_APPS + assert PROMETHEUS_AUTH_MIDDLEWARE not in TestSettings.MIDDLEWARE + assert PROMETHEUS_BEFORE_MIDDLEWARE not in TestSettings.MIDDLEWARE + assert PROMETHEUS_AFTER_MIDDLEWARE not in TestSettings.MIDDLEWARE + assert "PROMETHEUS_MULTIPROC_DIR" not in os.environ + + +@pytest.mark.parametrize("api_key", [None, ""]) +def test_settings_prometheus_metrics_enabled_requires_api_key( + monkeypatch, tmp_path, api_key +): + """Enabling the metrics without a key should be refused: they would be public.""" + monkeypatch.setenv("PROMETHEUS_MULTIPROC_DIR", str(tmp_path)) + test_settings = _prometheus_settings(PROMETHEUS_API_KEY=api_key) + + with pytest.raises(ValueError) as excinfo: + test_settings.post_setup() + + assert str(excinfo.value) == ( + "PROMETHEUS_METRICS_ENABLED requires PROMETHEUS_API_KEY to be set." + ) + assert PROMETHEUS_BEFORE_MIDDLEWARE not in test_settings.MIDDLEWARE + + +def test_settings_prometheus_metrics_enabled(monkeypatch, tmp_path): + """ + Enabling the metrics should create the directory and hand it to prometheus_client, + install the application, wrap the middlewares and instrument the database. + """ + multiproc_dir = tmp_path / "prometheus" + monkeypatch.setenv("PROMETHEUS_MULTIPROC_DIR", "overwritten-below") + test_settings = _prometheus_settings(PROMETHEUS_MULTIPROC_DIR=str(multiproc_dir)) + installed_apps = test_settings.INSTALLED_APPS + middleware = test_settings.MIDDLEWARE + default_database = test_settings.DATABASES["default"] + + test_settings.post_setup() + + assert multiproc_dir.is_dir() + assert multiproc_dir.stat().st_mode & 0o777 == 0o700 + assert os.environ["PROMETHEUS_MULTIPROC_DIR"] == str(multiproc_dir) + + # the very objects Django was handed before post_setup ran + assert test_settings.INSTALLED_APPS is installed_apps + assert test_settings.MIDDLEWARE is middleware + assert test_settings.DATABASES["default"] is default_database + + assert installed_apps[-1] == "django_prometheus" + # the authentication first: a refused scrape must not reach anything else + assert middleware[0] == PROMETHEUS_AUTH_MIDDLEWARE + assert middleware[1] == PROMETHEUS_BEFORE_MIDDLEWARE + assert middleware[-1] == PROMETHEUS_AFTER_MIDDLEWARE + assert middleware[2:-1] == Base.MIDDLEWARE + assert default_database["ENGINE"] == "django_prometheus.db.backends.postgresql" + + # running it again must not wrap the middlewares a second time + test_settings.post_setup() + assert installed_apps.count("django_prometheus") == 1 + assert middleware.count(PROMETHEUS_AUTH_MIDDLEWARE) == 1 + assert middleware.count(PROMETHEUS_BEFORE_MIDDLEWARE) == 1 + assert middleware.count(PROMETHEUS_AFTER_MIDDLEWARE) == 1 + + +def test_settings_prometheus_multiproc_dir_default(monkeypatch, tmp_path): + """ + Without a directory, a fixed one should be used: every uvicorn worker computes it on + its own and they have to agree. + """ + # set then unset, so that what post_setup writes to the environment is undone + monkeypatch.setenv("PROMETHEUS_MULTIPROC_DIR", "unset-below") + monkeypatch.delenv("PROMETHEUS_MULTIPROC_DIR") + monkeypatch.setattr(tempfile, "tempdir", str(tmp_path)) + expected = tmp_path / f"impress-prometheus-{os.getuid()}" + + _prometheus_settings().post_setup() + assert os.environ["PROMETHEUS_MULTIPROC_DIR"] == str(expected) + assert expected.is_dir() + + _prometheus_settings().post_setup() + assert os.environ["PROMETHEUS_MULTIPROC_DIR"] == str(expected) + + +def test_settings_prometheus_multiproc_dir_must_not_be_a_link(monkeypatch, tmp_path): + """A link planted in a shared temporary directory should not be followed.""" + monkeypatch.setenv("PROMETHEUS_MULTIPROC_DIR", "unset-below") + monkeypatch.delenv("PROMETHEUS_MULTIPROC_DIR") + target = tmp_path / "elsewhere" + target.mkdir() + link = tmp_path / "link" + link.symlink_to(target, target_is_directory=True) + + with pytest.raises(ValueError, match="must be a directory owned by the user"): + _prometheus_settings(PROMETHEUS_MULTIPROC_DIR=str(link)).post_setup() + + assert "PROMETHEUS_MULTIPROC_DIR" not in os.environ + + +def test_settings_prometheus_db_metrics_can_be_disabled(monkeypatch, tmp_path): + """The database engine should be left alone when its metrics are not wanted.""" + monkeypatch.setenv("PROMETHEUS_MULTIPROC_DIR", "overwritten-below") + test_settings = _prometheus_settings( + PROMETHEUS_MULTIPROC_DIR=str(tmp_path), PROMETHEUS_DB_METRICS_ENABLED=False + ) + + test_settings.post_setup() + + assert ( + test_settings.DATABASES["default"]["ENGINE"] == "django.db.backends.postgresql" + ) diff --git a/src/backend/impress/settings.py b/src/backend/impress/settings.py index 9791d9289..9ca7da6ef 100755 --- a/src/backend/impress/settings.py +++ b/src/backend/impress/settings.py @@ -11,6 +11,8 @@ https://docs.djangoproject.com/en/3.1/ref/settings/ """ import os +import stat +import tempfile import tomllib from socket import gethostbyname, gethostname @@ -1233,6 +1235,39 @@ class Base(Configuration): SILKY_MAX_REQUEST_BODY_SIZE = 0 SILKY_MAX_RESPONSE_BODY_SIZE = 0 + # -- Metrics (django-prometheus) ----------------------------------------- + # Opt-in Prometheus instrumentation, OFF by default. When enabled, + # `django_prometheus` is added to INSTALLED_APPS and its two middlewares + # wrap MIDDLEWARE (see setup_prometheus_metrics below) to count and time + # every request, labelled by view name, method and status — never by path, + # user or document, so no identifier leaves the application through a label. + # + # The metrics are served on /metrics, outside of /api/ so that the ingress + # of the application does not publish them: a deployment that wants them + # reachable from outside routes that path on purpose, and filters who may + # call it. Whoever reaches it still has to present PROMETHEUS_API_KEY as a + # bearer token (core.middleware.PrometheusAuthMiddleware), and the + # application refuses to start with the metrics enabled and no key. + PROMETHEUS_METRICS_ENABLED = values.BooleanValue( + False, environ_name="PROMETHEUS_METRICS_ENABLED", environ_prefix=None + ) + PROMETHEUS_API_KEY = SecretFileValue( + None, environ_name="PROMETHEUS_API_KEY", environ_prefix=None + ) + # Also count and time the SQL queries, by swapping the database engine for + # django-prometheus' instrumented subclass of it. + PROMETHEUS_DB_METRICS_ENABLED = values.BooleanValue( + True, environ_name="PROMETHEUS_DB_METRICS_ENABLED", environ_prefix=None + ) + # uvicorn runs several worker processes, and a scrape is answered by one of + # them: they all write their numbers to this directory so that whichever + # answers can add them up. It has to be the same for every worker, hence a + # fixed default rather than a random one. Defaults to a directory of the + # system's temporary directory, named after the user running the application. + PROMETHEUS_MULTIPROC_DIR = values.Value( + None, environ_name="PROMETHEUS_MULTIPROC_DIR", environ_prefix=None + ) + # pylint: disable=invalid-name @property def ENVIRONMENT(self): @@ -1263,6 +1298,62 @@ class Base(Configuration): }, } + @classmethod + def setup_prometheus_metrics(cls): + """Wire django-prometheus into the settings (PROMETHEUS_METRICS_ENABLED).""" + if not cls.PROMETHEUS_API_KEY: + # fail closed: without a key the endpoint would answer to anybody + raise ValueError( + "PROMETHEUS_METRICS_ENABLED requires PROMETHEUS_API_KEY to be set." + ) + + # prometheus_client decides whether the workers share their numbers + # when it is first imported, by looking for this variable in the + # environment of the process: a setting alone would come too late. + multiproc_dir = cls.PROMETHEUS_MULTIPROC_DIR or os.path.join( + tempfile.gettempdir(), f"impress-prometheus-{os.getuid()}" + ) + try: + os.makedirs(multiproc_dir, mode=0o700, exist_ok=True) + stat_result = os.lstat(multiproc_dir) + except OSError as error: + raise ValueError( + f"PROMETHEUS_MULTIPROC_DIR ({multiproc_dir}) cannot be created: {error}" + ) from error + # A shared temporary directory is writable by every local user: refuse a + # directory somebody else prepared, or a link to somewhere else. + if not stat.S_ISDIR(stat_result.st_mode) or stat_result.st_uid != os.getuid(): + raise ValueError( + f"PROMETHEUS_MULTIPROC_DIR ({multiproc_dir}) must be a directory " + "owned by the user running the application." + ) + cls.PROMETHEUS_MULTIPROC_DIR = multiproc_dir + os.environ["PROMETHEUS_MULTIPROC_DIR"] = multiproc_dir + + # Edited in place, like silk above: post_setup runs once the settings + # have been handed to Django, which only sees an assignment made + # here through the objects it already holds. + if "django_prometheus" not in cls.INSTALLED_APPS: + cls.INSTALLED_APPS.append("django_prometheus") + # The measuring middlewares go first and last, so that the time spent in + # every other middleware is part of what is measured. The authentication + # goes before them all: a refused scrape costs nothing and touches + # neither the session store nor the database. + auth = "core.middleware.PrometheusAuthMiddleware" + before = "django_prometheus.middleware.PrometheusBeforeMiddleware" + after = "django_prometheus.middleware.PrometheusAfterMiddleware" + if before not in cls.MIDDLEWARE: + cls.MIDDLEWARE.insert(0, before) + cls.MIDDLEWARE.insert(0, auth) + cls.MIDDLEWARE.append(after) + + default_database = cls.DATABASES["default"] + if ( + cls.PROMETHEUS_DB_METRICS_ENABLED + and default_database.get("ENGINE") == "django.db.backends.postgresql" + ): + default_database["ENGINE"] = "django_prometheus.db.backends.postgresql" + @classmethod def post_setup(cls): """Post setup configuration. @@ -1354,6 +1445,9 @@ class Base(Configuration): if "silk.middleware.SilkyMiddleware" not in cls.MIDDLEWARE: cls.MIDDLEWARE.insert(1, "silk.middleware.SilkyMiddleware") + if cls.PROMETHEUS_METRICS_ENABLED: + cls.setup_prometheus_metrics() + class Build(Base): """Settings used when the application is built. diff --git a/src/backend/impress/urls.py b/src/backend/impress/urls.py index fd594dc61..852d78695 100644 --- a/src/backend/impress/urls.py +++ b/src/backend/impress/urls.py @@ -17,6 +17,18 @@ urlpatterns = [ path("", include("core.urls")), ] +# Serve the Prometheus metrics only when they are enabled for this environment +# (PROMETHEUS_METRICS_ENABLED=1). Outside of /api/ on purpose, so that the ingress +# of the application does not publish them, and behind a bearer token +# (core.middleware.PrometheusAuthMiddleware) — see documentation/metrics.md. +if settings.PROMETHEUS_METRICS_ENABLED: + from core.metrics import metrics_view + from core.middleware import METRICS_PATH + + urlpatterns += [ + path(METRICS_PATH.lstrip("/"), metrics_view, name="prometheus-metrics") + ] + # Serve the django-silk profiling UI at /silk/ only when profiling is enabled # for this environment (SILK_ENABLED=1). The view itself is further gated behind # a staff session by SILKY_AUTHENTICATION / SILKY_AUTHORISATION. diff --git a/src/backend/pyproject.toml b/src/backend/pyproject.toml index bfe6abafe..b45bc06d7 100644 --- a/src/backend/pyproject.toml +++ b/src/backend/pyproject.toml @@ -36,6 +36,7 @@ dependencies = [ "django-filter==26.1", "django-lasuite[all]==0.0.27", "django-parler==2.4", + "django-prometheus==2.5.0", "django-redis==7.0.0", "django-silk==5.5.2", "django-storages[s3]==1.14.6", @@ -59,6 +60,7 @@ dependencies = [ "nested-multipart-parser==1.6.0", "openai==2.48.0", "posthog==7.29.0", + "prometheus-client==0.26.0", "psycopg[binary,pool]==3.3.4", "pycrdt==0.14.1", "pydantic==2.13.4", diff --git a/src/backend/uv.lock b/src/backend/uv.lock index b1e764a7a..c913e6de3 100644 --- a/src/backend/uv.lock +++ b/src/backend/uv.lock @@ -570,6 +570,19 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/8b/87/6d6129e14a7815bb383b61e7f86ffa85a4f43132eafa15de4a3a30ff9424/django_parler-2.4-py3-none-any.whl", hash = "sha256:0fff7185d581790d7063cbe0f7c57a399517d5ec976536b6c6bc06cf8ad4b956", size = 112605, upload-time = "2026-05-14T08:58:06.545Z" }, ] +[[package]] +name = "django-prometheus" +version = "2.5.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "django" }, + { name = "prometheus-client" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/7b/c7/dc39c4c19f7b35e827a486d08376de1fad31c50decb26c56e32668314f13/django_prometheus-2.5.0.tar.gz", hash = "sha256:4837b3c3734d8350880839ab8235aafd250b668c348e159d4aecc3cbefeee53e", size = 26465, upload-time = "2026-05-26T19:04:00.77Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/5d/5d/6ec3083ba69545696c962ae505a0e52e280e7592d4c278c2f3803cabb688/django_prometheus-2.5.0-py2.py3-none-any.whl", hash = "sha256:f15efb526cd53f9cf12da72dc55506322f5566b017a819ff27be1da302303134", size = 31801, upload-time = "2026-05-26T19:03:59.505Z" }, +] + [[package]] name = "django-redis" version = "7.0.0" @@ -965,6 +978,7 @@ dependencies = [ { name = "django-filter" }, { name = "django-lasuite", extra = ["all"] }, { name = "django-parler" }, + { name = "django-prometheus" }, { name = "django-redis" }, { name = "django-silk" }, { name = "django-storages", extra = ["s3"] }, @@ -987,11 +1001,12 @@ dependencies = [ { name = "nested-multipart-parser" }, { name = "openai" }, { name = "posthog" }, + { name = "prometheus-client" }, { name = "psycopg", extra = ["binary", "pool"] }, { name = "pycrdt" }, { name = "pydantic" }, { name = "pydantic-ai-slim", extra = ["logfire", "mistral", "openai", "web"] }, - { name = "pyjwt" , extra = ["crypto"] }, + { name = "pyjwt", extra = ["crypto"] }, { name = "python-magic" }, { name = "redis" }, { name = "requests" }, @@ -1038,6 +1053,7 @@ requires-dist = [ { name = "django-filter", specifier = "==26.1" }, { name = "django-lasuite", extras = ["all"], specifier = "==0.0.27" }, { name = "django-parler", specifier = "==2.4" }, + { name = "django-prometheus", specifier = "==2.5.0" }, { name = "django-redis", specifier = "==7.0.0" }, { name = "django-silk", specifier = "==5.5.2" }, { name = "django-storages", extras = ["s3"], specifier = "==1.14.6" }, @@ -1056,7 +1072,7 @@ requires-dist = [ { name = "gunicorn", specifier = "==26.0.0" }, { name = "ipdb", marker = "extra == 'dev'", specifier = "==0.13.13" }, { name = "ipython", marker = "extra == 'dev'", specifier = "==9.15.0" }, - { name = "joserfc", specifier = "==1.6.5" }, + { name = "joserfc", specifier = "==1.6.5" }, { name = "jsonschema", specifier = "==4.26.0" }, { name = "langfuse", specifier = "==3.11.2" }, { name = "lxml", specifier = "==6.1.1" }, @@ -1065,6 +1081,7 @@ requires-dist = [ { name = "nested-multipart-parser", specifier = "==1.6.0" }, { name = "openai", specifier = "==2.48.0" }, { name = "posthog", specifier = "==7.29.0" }, + { name = "prometheus-client", specifier = "==0.26.0" }, { name = "psycopg", extras = ["binary", "pool"], specifier = "==3.3.4" }, { name = "pycrdt", specifier = "==0.14.1" }, { name = "pydantic", specifier = "==2.13.4" }, @@ -1723,6 +1740,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/4e/d1/e4ed95fdd3ef13b78630280d9e9e240aeb65cc7c544ec57106149c3942fb/pprintpp-0.4.0-py2.py3-none-any.whl", hash = "sha256:b6b4dcdd0c0c0d75e4d7b2f21a9e933e5b2ce62b26e1a54537f9651ae5a5c01d", size = 16952, upload-time = "2018-07-01T01:42:36.496Z" }, ] +[[package]] +name = "prometheus-client" +version = "0.26.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/52/73/f1334c29c2af4cd9dba6c7817e61b611bd0215e2eb5565c6064a4de18802/prometheus_client-0.26.0.tar.gz", hash = "sha256:04a91bcf94e2cf74a44a1a874d651a2e853ed354b6e822f3b7487751465d5c2b", size = 92910, upload-time = "2026-07-24T19:36:41.893Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/eb/a3/b69efbf4143b5b9859b977770bbbabcc2796b702fa69dc40271e45cd5a56/prometheus_client-0.26.0-py3-none-any.whl", hash = "sha256:fa93d06737aa02bacd05794768508bb97d2fbee28cb3bca04eaae92f0ca953d6", size = 64494, upload-time = "2026-07-24T19:36:40.854Z" }, +] + [[package]] name = "prompt-toolkit" version = "3.0.52" diff --git a/src/helm/impress/README.md b/src/helm/impress/README.md index 72aac31ca..3e54eb902 100644 --- a/src/helm/impress/README.md +++ b/src/helm/impress/README.md @@ -66,6 +66,16 @@ | `ingressAdmin.tls.secretName` | Secret name for TLS config | `nil` | | `ingressAdmin.tls.additional[].secretName` | Secret name for additional TLS config | | | `ingressAdmin.tls.additional[].hosts[]` | Hosts for additional TLS config | | +| `ingressMetrics.enabled` | whether to enable the Ingress publishing the Prometheus metrics of the backend (requires PROMETHEUS_METRICS_ENABLED and PROMETHEUS_API_KEY in backend.envVars) | `false` | +| `ingressMetrics.className` | IngressClass to use for the Ingress | `nil` | +| `ingressMetrics.host` | Host for the Ingress. Use a host of its own, and add it to DJANGO_ALLOWED_HOSTS | `metrics.impress.example.com` | +| `ingressMetrics.path` | Path to use for the Ingress, matched exactly | `/metrics` | +| `ingressMetrics.hosts` | Additional host to configure for the Ingress | `[]` | +| `ingressMetrics.tls.enabled` | Whether to enable TLS for the Ingress | `true` | +| `ingressMetrics.tls.secretName` | Secret name for TLS config | `nil` | +| `ingressMetrics.tls.additional[].secretName` | Secret name for additional TLS config | | +| `ingressMetrics.tls.additional[].hosts[]` | Hosts for additional TLS config | | +| `ingressMetrics.annotations` | Annotations of the Ingress. Restrict the callers by address here | `{}` | | `ingressMedia.enabled` | whether to enable the Ingress or not | `false` | | `ingressMedia.className` | IngressClass to use for the Ingress | `nil` | | `ingressMedia.host` | Host for the Ingress | `impress.example.com` | diff --git a/src/helm/impress/templates/ingress_metrics.yaml b/src/helm/impress/templates/ingress_metrics.yaml new file mode 100644 index 000000000..d5a0f5fd7 --- /dev/null +++ b/src/helm/impress/templates/ingress_metrics.yaml @@ -0,0 +1,90 @@ +{{- /* +Publishes the Prometheus metrics of the backend (PROMETHEUS_METRICS_ENABLED) and +nothing else: one exact path, on a host of its own. The main ingress sends every +path but /api and /external_api to the frontend, so without this one the metrics +are not reachable from outside the cluster. Restrict who may call it with the +annotations of your ingress controller; the bearer token is the second lock. +*/ -}} +{{- if .Values.ingressMetrics.enabled -}} +{{- $fullName := include "impress.fullname" . -}} +{{- if and .Values.ingressMetrics.className (not (semverCompare ">=1.18-0" .Capabilities.KubeVersion.GitVersion)) }} + {{- if not (hasKey .Values.ingressMetrics.annotations "kubernetes.io/ingress.class") }} + {{- $_ := set .Values.ingressMetrics.annotations "kubernetes.io/ingress.class" .Values.ingressMetrics.className}} + {{- end }} +{{- end }} +{{- if semverCompare ">=1.19-0" .Capabilities.KubeVersion.GitVersion -}} +apiVersion: networking.k8s.io/v1 +{{- else if semverCompare ">=1.14-0" .Capabilities.KubeVersion.GitVersion -}} +apiVersion: networking.k8s.io/v1beta1 +{{- else -}} +apiVersion: extensions/v1beta1 +{{- end }} +kind: Ingress +metadata: + name: {{ $fullName }}-metrics + namespace: {{ .Release.Namespace | quote }} + labels: + {{- include "impress.labels" . | nindent 4 }} + {{- with .Values.ingressMetrics.annotations }} + annotations: + {{- toYaml . | nindent 4 }} + {{- end }} +spec: + {{- if and .Values.ingressMetrics.className (semverCompare ">=1.18-0" .Capabilities.KubeVersion.GitVersion) }} + ingressClassName: {{ .Values.ingressMetrics.className }} + {{- end }} + {{- if .Values.ingressMetrics.tls.enabled }} + tls: + {{- if .Values.ingressMetrics.host }} + - secretName: {{ .Values.ingressMetrics.tls.secretName | default (printf "%s-tls" $fullName) | quote }} + hosts: + - {{ .Values.ingressMetrics.host | quote }} + {{- end }} + {{- range .Values.ingressMetrics.tls.additional }} + - hosts: + {{- range .hosts }} + - {{ . | quote }} + {{- end }} + secretName: {{ .secretName }} + {{- end }} + {{- end }} + rules: + {{- if .Values.ingressMetrics.host }} + - host: {{ .Values.ingressMetrics.host | quote }} + http: + paths: + - path: {{ .Values.ingressMetrics.path | quote }} + {{- if semverCompare ">=1.18-0" $.Capabilities.KubeVersion.GitVersion }} + pathType: Exact + {{- end }} + backend: + {{- if semverCompare ">=1.19-0" $.Capabilities.KubeVersion.GitVersion }} + service: + name: {{ include "impress.backend.fullname" . }} + port: + number: {{ .Values.backend.service.port }} + {{- else }} + serviceName: {{ include "impress.backend.fullname" . }} + servicePort: {{ .Values.backend.service.port }} + {{- end }} + {{- end }} + {{- range .Values.ingressMetrics.hosts }} + - host: {{ . | quote }} + http: + paths: + - path: {{ $.Values.ingressMetrics.path | quote }} + {{- if semverCompare ">=1.18-0" $.Capabilities.KubeVersion.GitVersion }} + pathType: Exact + {{- end }} + backend: + {{- if semverCompare ">=1.19-0" $.Capabilities.KubeVersion.GitVersion }} + service: + name: {{ include "impress.backend.fullname" $ }} + port: + number: {{ $.Values.backend.service.port }} + {{- else }} + serviceName: {{ include "impress.backend.fullname" $ }} + servicePort: {{ $.Values.backend.service.port }} + {{- end }} + {{- end }} +{{- end }} diff --git a/src/helm/impress/values.yaml b/src/helm/impress/values.yaml index 37348a888..e9096b8cc 100644 --- a/src/helm/impress/values.yaml +++ b/src/helm/impress/values.yaml @@ -186,6 +186,33 @@ ingressAdmin: secretName: null additional: [] +## @param ingressMetrics.enabled whether to enable the Ingress publishing the Prometheus metrics of the backend (requires PROMETHEUS_METRICS_ENABLED and PROMETHEUS_API_KEY in backend.envVars) +## @param ingressMetrics.className IngressClass to use for the Ingress +## @param ingressMetrics.host Host for the Ingress. Use a host of its own, and add it to DJANGO_ALLOWED_HOSTS +## @param ingressMetrics.path Path to use for the Ingress, matched exactly +ingressMetrics: + enabled: false + className: null + host: metrics.impress.example.com + path: /metrics + ## @param ingressMetrics.hosts Additional host to configure for the Ingress + hosts: [] + ## @param ingressMetrics.tls.enabled Whether to enable TLS for the Ingress + ## @param ingressMetrics.tls.secretName Secret name for TLS config + ## @skip ingressMetrics.tls.additional + ## @extra ingressMetrics.tls.additional[].secretName Secret name for additional TLS config + ## @extra ingressMetrics.tls.additional[].hosts[] Hosts for additional TLS config + tls: + enabled: true + secretName: null + additional: [] + + ## @param ingressMetrics.annotations Annotations of the Ingress. Restrict the callers by address here + ## The bearer token travels in a header: keep TLS on, and filter the source + ## addresses, e.g. with ingress-nginx: + ## nginx.ingress.kubernetes.io/whitelist-source-range: "203.0.113.10/32" + annotations: {} + ## @param ingressMedia.enabled whether to enable the Ingress or not ## @param ingressMedia.className IngressClass to use for the Ingress ## @param ingressMedia.host Host for the Ingress