From b12111cc9e32ce2e5a412abe43804c9f275f4990 Mon Sep 17 00:00:00 2001 From: daavoo Date: Wed, 16 Sep 2026 15:58:32 +0200 Subject: [PATCH 1/5] feat(observability): export database pool stats and fail readiness fast when the pool is full Nothing reported how much of the SQLAlchemy connection pool was in use, so a connection leak stayed invisible until every request answered 503, and the readiness probe then queued for the full pool timeout before reporting a generic database outage. Adds a pool-stats helper over the active engines and uses it twice: four Prometheus gauges filled at scrape time (checked out, idle, overflow, capacity), labeled by pool so the metering pool is covered too; and a side-effect free saturation check ahead of the readiness query, which answers 503 immediately and names pool exhaustion as its own state. Fixes #1240 Co-Authored-By: Claude Opus 5 (1M context) --- docs/public/openapi.json | 2 +- docs/public/otari.postman_collection.json | 2 +- src/gateway/api/routes/health.py | 27 +++- src/gateway/core/database.py | 99 ++++++++++++- src/gateway/metrics.py | 49 ++++++- tests/unit/test_database_pool_stats.py | 171 ++++++++++++++++++++++ tests/unit/test_gateway_metrics.py | 53 +++++++ 7 files changed, 398 insertions(+), 5 deletions(-) create mode 100644 tests/unit/test_database_pool_stats.py diff --git a/docs/public/openapi.json b/docs/public/openapi.json index 878c865a18..25b945deb7 100644 --- a/docs/public/openapi.json +++ b/docs/public/openapi.json @@ -20519,7 +20519,7 @@ }, "/api/v1/health/readiness": { "get": { - "description": "Readiness probe endpoint.\n\nChecks if the gateway is ready to serve requests by validating:\n- Database connectivity\n- Service availability\n\nUsed by Kubernetes/container orchestrators for readiness probes.\nReturns HTTP 503 if any dependency is unavailable.\n\nReturns:\n dict: Status object with health details\n\nRaises:\n HTTPException: 503 if service is not ready", + "description": "Readiness probe endpoint.\n\nChecks if the gateway is ready to serve requests by validating:\n- Database connection pool headroom\n- Database connectivity\n- Service availability\n\nUsed by Kubernetes/container orchestrators for readiness probes.\nReturns HTTP 503 if any dependency is unavailable, or if the database\nconnection pool has no capacity left to serve a request.\n\nReturns:\n dict: Status object with health details\n\nRaises:\n HTTPException: 503 if service is not ready", "operationId": "health-health_readiness", "responses": { "200": { diff --git a/docs/public/otari.postman_collection.json b/docs/public/otari.postman_collection.json index 87bd2fe92f..19e0ddab51 100644 --- a/docs/public/otari.postman_collection.json +++ b/docs/public/otari.postman_collection.json @@ -1953,7 +1953,7 @@ { "name": "Health Readiness", "request": { - "description": "Readiness probe endpoint.\n\nChecks if the gateway is ready to serve requests by validating:\n- Database connectivity\n- Service availability\n\nUsed by Kubernetes/container orchestrators for readiness probes.\nReturns HTTP 503 if any dependency is unavailable.\n\nReturns:\n dict: Status object with health details\n\nRaises:\n HTTPException: 503 if service is not ready", + "description": "Readiness probe endpoint.\n\nChecks if the gateway is ready to serve requests by validating:\n- Database connection pool headroom\n- Database connectivity\n- Service availability\n\nUsed by Kubernetes/container orchestrators for readiness probes.\nReturns HTTP 503 if any dependency is unavailable, or if the database\nconnection pool has no capacity left to serve a request.\n\nReturns:\n dict: Status object with health details\n\nRaises:\n HTTPException: 503 if service is not ready", "header": [], "method": "GET", "url": { diff --git a/src/gateway/api/routes/health.py b/src/gateway/api/routes/health.py index 2aa720cec7..2852e711d2 100644 --- a/src/gateway/api/routes/health.py +++ b/src/gateway/api/routes/health.py @@ -7,11 +7,18 @@ from gateway.api.deps import get_config, get_db_if_needed from gateway.core.config import DEFAULT_PLATFORM_HEALTH_PATH, GatewayConfig +from gateway.core.database import request_pool_stats from gateway.log_config import logger from gateway.version import __version__ router = APIRouter(prefix="/health", tags=["health"]) +# The ``database`` state the readiness probe reports when every pooled +# connection is already checked out. Its own state rather than "unavailable": +# the database is reachable, this process has run out of ways to reach it, and +# an operator restarts or rescales on that rather than paging the database. +POOL_EXHAUSTED = "pool_exhausted" + async def _check_platform_reachability(config: GatewayConfig) -> bool: """Report whether the platform peer serves its health route. @@ -85,11 +92,13 @@ async def health_readiness( """Readiness probe endpoint. Checks if the gateway is ready to serve requests by validating: + - Database connection pool headroom - Database connectivity - Service availability Used by Kubernetes/container orchestrators for readiness probes. - Returns HTTP 503 if any dependency is unavailable. + Returns HTTP 503 if any dependency is unavailable, or if the database + connection pool has no capacity left to serve a request. Returns: dict: Status object with health details @@ -123,6 +132,22 @@ async def health_readiness( detail={"status": "unhealthy", "database": "unavailable", "version": __version__}, ) + pool = request_pool_stats() + if pool is not None and pool.is_saturated: + logger.error( + "Readiness check refused: database pool saturated, %s of %s connections checked out", + pool.checked_out, + pool.capacity, + ) + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail={ + "status": "unhealthy", + "database": POOL_EXHAUSTED, + "version": __version__, + }, + ) + try: await db.execute(text("SELECT 1")) db_status = "connected" diff --git a/src/gateway/core/database.py b/src/gateway/core/database.py index 1c9f282943..fefa049a30 100644 --- a/src/gateway/core/database.py +++ b/src/gateway/core/database.py @@ -6,8 +6,9 @@ import contextlib from collections.abc import AsyncGenerator, AsyncIterator from contextlib import asynccontextmanager +from dataclasses import dataclass from pathlib import Path -from typing import Any +from typing import Any, Protocol, runtime_checkable from alembic import command from alembic.config import Config @@ -196,6 +197,97 @@ async def release_session(session: AsyncSession | None) -> bool: return True + +# The name each pool reports under, in metrics and in anything that iterates +# :func:`pool_stats`. +REQUEST_POOL = "request" +LOG_POOL = "log" + + +@runtime_checkable +class _CountingPool(Protocol): + """The counters a ``QueuePool`` keeps and a ``NullPool`` does not.""" + + def checkedout(self) -> int: ... + def checkedin(self) -> int: ... + def overflow(self) -> int: ... + def size(self) -> int: ... + + +@dataclass(frozen=True, slots=True) +class PoolStats: + """A point-in-time reading of one engine's connection pool.""" + + checked_out: int + checked_in: int + overflow: int + size: int + max_overflow: int + + @property + def capacity(self) -> int: + """The most connections this pool will ever hand out at once.""" + return self.size + self.max_overflow + + @property + def is_saturated(self) -> bool: + """Whether every connection the pool can hand out is already out. + + A caller checking out here would queue for ``db_pool_timeout`` and then + fail, so this is what lets the readiness probe answer immediately + instead of holding the orchestrator open for the full timeout. + """ + return self.capacity > 0 and self.checked_out >= self.capacity + + +def _engine_pool_stats(engine: AsyncEngine | None) -> PoolStats | None: + """Read *engine*'s pool, or ``None`` when there is nothing to read. + + Returns ``None`` for an engine that was never built and for one on + ``NullPool``, which is SQLite's pool here and implements none of the + counters below: it opens a connection per checkout and keeps no pool to + saturate. Callers treat ``None`` as "no pool ceiling applies" rather than + as an error, so this never raises. + + ``max_overflow`` has no public accessor on ``QueuePool``, so it is read + defensively and falls back to ``0``, which understates capacity rather than + inventing it. + """ + if engine is None: + return None + pool = engine.pool + if not isinstance(pool, _CountingPool): + return None + # Negative until the pool has created its full complement of base + # connections, which would read as "overflow in use" the wrong way round. + overflow = max(pool.overflow(), 0) + return PoolStats( + checked_out=pool.checkedout(), + checked_in=pool.checkedin(), + overflow=overflow, + size=pool.size(), + max_overflow=max(getattr(pool, "_max_overflow", 0), 0), + ) + + +def request_pool_stats() -> PoolStats | None: + """Pool stats for the engine that serves request-scoped sessions.""" + return _engine_pool_stats(_engine) + + +def pool_stats() -> dict[str, PoolStats]: + """Pool stats for every active engine, keyed by pool name. + + A pool with nothing to report is omitted, so the result is empty before + :func:`init_db` runs and on SQLite. + """ + readings = { + REQUEST_POOL: _engine_pool_stats(_engine), + LOG_POOL: _engine_pool_stats(_log_engine), + } + return {name: stats for name, stats in readings.items() if stats is not None} + + def engine_kwargs( config: GatewayConfig, *, @@ -387,13 +479,18 @@ async def _dispose_all() -> None: __all__ = [ "DATABASE_ERRORS", + "LOG_POOL", + "REQUEST_POOL", + "PoolStats", "create_log_session", "create_session", "dispose_db", "engine_kwargs", "get_db", "init_db", + "pool_stats", "release_session", + "request_pool_stats", "reset_db", "translate_timeout_error", ] diff --git a/src/gateway/metrics.py b/src/gateway/metrics.py index 882cda1882..94f0bbc84b 100644 --- a/src/gateway/metrics.py +++ b/src/gateway/metrics.py @@ -1,4 +1,4 @@ -"""Prometheus registry, metric types, and HTTP request instrumentation for the gateway. +"""Prometheus registry, metric types, HTTP request instrumentation, and database pool gauges for the gateway. The metric types are re-exported so that code declaring a metric need not depend on ``prometheus_client`` directly. """ @@ -19,6 +19,7 @@ from starlette.responses import Response from gateway.core.config import API_ROOT, API_VERSION +from gateway.core.database import pool_stats if TYPE_CHECKING: from starlette.requests import Request @@ -54,6 +55,34 @@ registry=REGISTRY, ) +DB_POOL_CONNECTIONS_CHECKED_OUT = Gauge( + "gateway_db_pool_connections_checked_out", + "Pooled database connections currently checked out", + ["pool"], + registry=REGISTRY, +) + +DB_POOL_CONNECTIONS_IDLE = Gauge( + "gateway_db_pool_connections_idle", + "Pooled database connections checked in and available", + ["pool"], + registry=REGISTRY, +) + +DB_POOL_OVERFLOW_CONNECTIONS = Gauge( + "gateway_db_pool_overflow_connections", + "Database connections open beyond the base pool size", + ["pool"], + registry=REGISTRY, +) + +DB_POOL_CAPACITY = Gauge( + "gateway_db_pool_capacity", + "Most database connections the pool will hand out at once (size plus max overflow)", + ["pool"], + registry=REGISTRY, +) + _PROMETHEUS_CONTENT_TYPE = "text/plain; version=0.0.4; charset=utf-8" @@ -88,8 +117,26 @@ def _endpoint_label(scope: Scope) -> tuple[str, str]: return template, _NO_VERSION +def refresh_db_pool_metrics() -> None: + """Read the connection pools into their gauges. + + Called at scrape time rather than from a background task: the counters are + already maintained by SQLAlchemy, so reading them costs nothing and a timer + would only add a way for the exported value to lag the pool. A pool that + reports nothing (SQLite's ``NullPool``, or an engine that was never built) + leaves its gauges untouched instead of publishing a zero that would read as + an idle pool. + """ + for name, stats in pool_stats().items(): + DB_POOL_CONNECTIONS_CHECKED_OUT.labels(pool=name).set(stats.checked_out) + DB_POOL_CONNECTIONS_IDLE.labels(pool=name).set(stats.checked_in) + DB_POOL_OVERFLOW_CONNECTIONS.labels(pool=name).set(stats.overflow) + DB_POOL_CAPACITY.labels(pool=name).set(stats.capacity) + + async def metrics_endpoint(request: Request) -> Response: """Serve Prometheus metrics.""" + refresh_db_pool_metrics() body = generate_latest(REGISTRY) return Response(content=body, media_type=_PROMETHEUS_CONTENT_TYPE) diff --git a/tests/unit/test_database_pool_stats.py b/tests/unit/test_database_pool_stats.py new file mode 100644 index 0000000000..f167ab99cc --- /dev/null +++ b/tests/unit/test_database_pool_stats.py @@ -0,0 +1,171 @@ +"""Unit tests for the connection-pool stats helper and the readiness probe.""" + +from collections.abc import Iterator +from types import SimpleNamespace +from typing import Any + +import pytest +from fastapi import HTTPException + +from gateway.api.routes import health +from gateway.core import database +from gateway.core.database import LOG_POOL, REQUEST_POOL, PoolStats, pool_stats, request_pool_stats + + +class _QueuePool: + """Stand-in for ``AsyncAdaptedQueuePool`` exposing the counters read here.""" + + def __init__(self, checked_out: int, checked_in: int, overflow: int, size: int, max_overflow: int) -> None: + self._checked_out = checked_out + self._checked_in = checked_in + self._overflow = overflow + self._size = size + self._max_overflow = max_overflow + + def checkedout(self) -> int: + return self._checked_out + + def checkedin(self) -> int: + return self._checked_in + + def overflow(self) -> int: + return self._overflow + + def size(self) -> int: + return self._size + + +class _NullPool: + """Stand-in for ``NullPool``, which implements none of those counters.""" + + def status(self) -> str: + return "NullPool" + + +def _engine(pool: Any) -> Any: + return SimpleNamespace(pool=pool) + + +@pytest.fixture(autouse=True) +def _restore_engines() -> Iterator[None]: + engine = database._engine + log_engine = database._log_engine + yield + database._engine = engine + database._log_engine = log_engine + + +def test_pool_stats_reads_the_queue_pool_counters() -> None: + database._engine = _engine(_QueuePool(checked_out=7, checked_in=3, overflow=2, size=10, max_overflow=20)) + database._log_engine = None + + stats = request_pool_stats() + + assert stats == PoolStats(checked_out=7, checked_in=3, overflow=2, size=10, max_overflow=20) + assert stats is not None + assert stats.capacity == 30 + assert not stats.is_saturated + + +def test_pool_stats_clamps_a_negative_overflow() -> None: + """A pool that has not created its base connections yet reports a negative overflow.""" + database._engine = _engine(_QueuePool(checked_out=1, checked_in=0, overflow=-9, size=10, max_overflow=20)) + + stats = request_pool_stats() + + assert stats is not None + assert stats.overflow == 0 + + +def test_pool_is_saturated_when_every_connection_is_out() -> None: + database._engine = _engine(_QueuePool(checked_out=30, checked_in=0, overflow=20, size=10, max_overflow=20)) + + stats = request_pool_stats() + + assert stats is not None + assert stats.is_saturated + + +def test_pool_stats_returns_none_for_a_null_pool() -> None: + database._engine = _engine(_NullPool()) + + assert request_pool_stats() is None + + +def test_pool_stats_returns_none_when_the_database_is_not_initialized() -> None: + database._engine = None + database._log_engine = None + + assert request_pool_stats() is None + assert pool_stats() == {} + + +def test_pool_stats_covers_both_engines() -> None: + database._engine = _engine(_QueuePool(checked_out=1, checked_in=2, overflow=0, size=10, max_overflow=20)) + database._log_engine = _engine(_QueuePool(checked_out=0, checked_in=1, overflow=0, size=2, max_overflow=0)) + + readings = pool_stats() + + assert set(readings) == {REQUEST_POOL, LOG_POOL} + assert readings[LOG_POOL].capacity == 2 + + +def test_pool_stats_omits_an_engine_without_a_pool() -> None: + database._engine = _engine(_QueuePool(checked_out=1, checked_in=2, overflow=0, size=10, max_overflow=20)) + database._log_engine = _engine(_NullPool()) + + assert set(pool_stats()) == {REQUEST_POOL} + + +class _Session: + """Session that records whether the readiness query was attempted.""" + + def __init__(self) -> None: + self.executed = False + + async def execute(self, _statement: Any) -> None: + self.executed = True + + +# The route only reads ``is_hybrid_mode``, so a stand-in keeps this a unit test. +_STANDALONE: Any = SimpleNamespace(is_hybrid_mode=False) + + +@pytest.mark.asyncio +async def test_readiness_refuses_immediately_on_a_saturated_pool() -> None: + database._engine = _engine(_QueuePool(checked_out=30, checked_in=0, overflow=20, size=10, max_overflow=20)) + session: Any = _Session() + + with pytest.raises(HTTPException) as excinfo: + await health.health_readiness(config=_STANDALONE, db=session) + + assert excinfo.value.status_code == 503 + detail: Any = excinfo.value.detail + assert detail["status"] == "unhealthy" + assert detail["database"] == health.POOL_EXHAUSTED + assert detail["database"] != "unavailable" + assert "version" in detail + assert not session.executed + + +@pytest.mark.asyncio +async def test_readiness_queries_the_database_when_the_pool_has_headroom() -> None: + database._engine = _engine(_QueuePool(checked_out=1, checked_in=9, overflow=0, size=10, max_overflow=20)) + session: Any = _Session() + + payload = await health.health_readiness(config=_STANDALONE, db=session) + + assert payload["status"] == "healthy" + assert payload["database"] == "connected" + assert session.executed + + +@pytest.mark.asyncio +async def test_readiness_queries_the_database_on_a_null_pool() -> None: + database._engine = _engine(_NullPool()) + session: Any = _Session() + + payload = await health.health_readiness(config=_STANDALONE, db=session) + + assert payload["status"] == "healthy" + assert session.executed diff --git a/tests/unit/test_gateway_metrics.py b/tests/unit/test_gateway_metrics.py index d6529c8f2b..8e42bc13e7 100644 --- a/tests/unit/test_gateway_metrics.py +++ b/tests/unit/test_gateway_metrics.py @@ -13,6 +13,7 @@ MetricsMiddleware, _endpoint_label, metrics_endpoint, + refresh_db_pool_metrics, ) @@ -30,6 +31,10 @@ def _sample(name: str, labels: dict[str, str] | None = None) -> float: ("gateway_active_requests", "gauge", ()), ("gateway_auth_failures", "counter", ("reason",)), ("gateway_budget_exceeded", "counter", ()), + ("gateway_db_pool_capacity", "gauge", ("pool",)), + ("gateway_db_pool_connections_checked_out", "gauge", ("pool",)), + ("gateway_db_pool_connections_idle", "gauge", ("pool",)), + ("gateway_db_pool_overflow_connections", "gauge", ("pool",)), ("gateway_inline_cost_settlements", "counter", ("outcome",)), ("gateway_rate_limit_hits", "counter", ()), ("gateway_request_cost_dollars", "histogram", ("provider", "model")), @@ -238,3 +243,51 @@ def test_metrics_expose_process_memory() -> None: assert "process_resident_memory_bytes" in body assert _sample("process_resident_memory_bytes") > 0 + + +class _FakeQueuePool: + """Stand-in for ``AsyncAdaptedQueuePool``, exposing only the counters read.""" + + _max_overflow = 20 + + def checkedout(self) -> int: + return 4 + + def checkedin(self) -> int: + return 6 + + def overflow(self) -> int: + return 1 + + def size(self) -> int: + return 10 + + +def test_db_pool_gauges_are_refreshed_at_scrape_time(monkeypatch: pytest.MonkeyPatch) -> None: + """A leaked connection has to be visible before requests start failing.""" + from types import SimpleNamespace + + from gateway.core import database + + monkeypatch.setattr(database, "_engine", SimpleNamespace(pool=_FakeQueuePool())) + monkeypatch.setattr(database, "_log_engine", None) + + refresh_db_pool_metrics() + + labels = {"pool": "request"} + assert _sample("gateway_db_pool_connections_checked_out", labels) == 4.0 + assert _sample("gateway_db_pool_connections_idle", labels) == 6.0 + assert _sample("gateway_db_pool_overflow_connections", labels) == 1.0 + assert _sample("gateway_db_pool_capacity", labels) == 30.0 + + +def test_db_pool_gauges_are_left_alone_without_a_pool(monkeypatch: pytest.MonkeyPatch) -> None: + """SQLite runs on NullPool, and an uninitialized engine has no pool at all.""" + from gateway.core import database + + monkeypatch.setattr(database, "_engine", None) + monkeypatch.setattr(database, "_log_engine", None) + + refresh_db_pool_metrics() + + assert _sample("gateway_db_pool_capacity", {"pool": "unbuilt"}) == 0.0 From 08b8f3ca7f8dff084fe3b8d15d10519dcb403ba1 Mon Sep 17 00:00:00 2001 From: daavoo Date: Wed, 16 Sep 2026 15:59:57 +0200 Subject: [PATCH 2/5] docs(deployment): document the database pool metrics and the pool_exhausted readiness state Co-Authored-By: Claude Opus 5 (1M context) --- docs/deployment.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs/deployment.md b/docs/deployment.md index 60bdc7e257..8b15310680 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -19,6 +19,27 @@ A durable standalone deployment should: The default SQLite database is intended for evaluation and single-node local use. +### Watch the connection pool + +On PostgreSQL the gateway serves requests from a fixed pool of database +connections, and running out of them makes every request fail at once. `/metrics` +reports the pool directly, labeled by `pool` (`request` for request traffic, +`log` for the usage-log writer): + +| Metric | Meaning | +| --- | --- | +| `gateway_db_pool_connections_checked_out` | Connections in use right now | +| `gateway_db_pool_connections_idle` | Connections available to hand out | +| `gateway_db_pool_overflow_connections` | Connections open beyond `db_pool_size` | +| `gateway_db_pool_capacity` | Ceiling: `db_pool_size` plus `db_max_overflow` | + +Alert on checked-out connections approaching capacity for a sustained period. +`/api/v1/health/readiness` also answers `503` immediately, with a `database` +state of `pool_exhausted`, once the pool has nothing left to hand out, so an +orchestrator takes the pod out of rotation rather than waiting out the pool +timeout. These metrics do not appear on SQLite, which opens a connection per +use and keeps no pool. + ## Docker Compose The repository Compose stack runs Otari and PostgreSQL: From 2b9051fc115421f364a99b85c51334b09bc4a95d Mon Sep 17 00:00:00 2001 From: daavoo Date: Wed, 16 Sep 2026 16:00:37 +0200 Subject: [PATCH 3/5] style(db): drop a stray blank line Co-Authored-By: Claude Opus 5 (1M context) --- src/gateway/core/database.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/gateway/core/database.py b/src/gateway/core/database.py index fefa049a30..6574e325b6 100644 --- a/src/gateway/core/database.py +++ b/src/gateway/core/database.py @@ -197,7 +197,6 @@ async def release_session(session: AsyncSession | None) -> bool: return True - # The name each pool reports under, in metrics and in anything that iterates # :func:`pool_stats`. REQUEST_POOL = "request" From 0cec51ae7784c5783c775feb69193009c42abfa0 Mon Sep 17 00:00:00 2001 From: daavoo Date: Wed, 16 Sep 2026 16:07:17 +0200 Subject: [PATCH 4/5] chore(web): regenerate the API client for the readiness docstring Co-Authored-By: Claude Opus 5 (1M context) --- web/src/client/schema.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/web/src/client/schema.ts b/web/src/client/schema.ts index 0b8290cbc9..405ccf986a 100644 --- a/web/src/client/schema.ts +++ b/web/src/client/schema.ts @@ -1220,11 +1220,13 @@ export interface paths { * @description Readiness probe endpoint. * * Checks if the gateway is ready to serve requests by validating: + * - Database connection pool headroom * - Database connectivity * - Service availability * * Used by Kubernetes/container orchestrators for readiness probes. - * Returns HTTP 503 if any dependency is unavailable. + * Returns HTTP 503 if any dependency is unavailable, or if the database + * connection pool has no capacity left to serve a request. * * Returns: * dict: Status object with health details From 4a84b0bbdfdba4de585100853408f1197eb55dd2 Mon Sep 17 00:00:00 2001 From: daavoo Date: Wed, 16 Sep 2026 17:31:58 +0200 Subject: [PATCH 5/5] refactor(observability): publish the pool stats from a collector and drop the readiness check The readiness probe judged the pool on one instantaneous reading, so a pool that filled for a moment took the pod out of rotation and a traffic spike across replicas became an outage. Drop the check, along with POOL_EXHAUSTED, request_pool_stats and PoolStats.is_saturated. #1247 tracks a check that judges the pool over time. The gauges move out of metrics.py, which #1205 had just emptied, and become a collector in core/database.py that reads the pools on each scrape. That also removes the metrics -> core.database import, which would have stopped core.database ever declaring a metric of its own. Capacity now uses the max_overflow the gateway configured rather than QueuePool's private _max_overflow, so it cannot silently understate itself. Co-Authored-By: Claude Opus 5 (1M context) --- docs/deployment.md | 16 +-- docs/public/openapi.json | 2 +- docs/public/otari.postman_collection.json | 2 +- src/gateway/api/routes/health.py | 27 +---- src/gateway/core/database.py | 99 +++++++++++------ src/gateway/metrics.py | 63 +++-------- tests/unit/test_database_pool_stats.py | 127 +++++++++------------- tests/unit/test_gateway_metrics.py | 55 +--------- web/src/client/schema.ts | 4 +- 9 files changed, 143 insertions(+), 252 deletions(-) diff --git a/docs/deployment.md b/docs/deployment.md index 8b15310680..2992ef36c7 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -30,15 +30,17 @@ reports the pool directly, labeled by `pool` (`request` for request traffic, | --- | --- | | `gateway_db_pool_connections_checked_out` | Connections in use right now | | `gateway_db_pool_connections_idle` | Connections available to hand out | -| `gateway_db_pool_overflow_connections` | Connections open beyond `db_pool_size` | -| `gateway_db_pool_capacity` | Ceiling: `db_pool_size` plus `db_max_overflow` | +| `gateway_db_pool_overflow_connections` | Connections open beyond the pool's base size | +| `gateway_db_pool_capacity` | Ceiling on connections the pool hands out at once | + +The `request` pool is sized by `db_pool_size` and may open `db_max_overflow` +connections beyond it, so its capacity is the two added together. The `log` +pool is sized by `db_log_pool_size` and has no overflow, so its capacity is +that value. Alert on checked-out connections approaching capacity for a sustained period. -`/api/v1/health/readiness` also answers `503` immediately, with a `database` -state of `pool_exhausted`, once the pool has nothing left to hand out, so an -orchestrator takes the pod out of rotation rather than waiting out the pool -timeout. These metrics do not appear on SQLite, which opens a connection per -use and keeps no pool. +These metrics do not appear on SQLite, which opens a connection per use and +keeps no pool. ## Docker Compose diff --git a/docs/public/openapi.json b/docs/public/openapi.json index 25b945deb7..878c865a18 100644 --- a/docs/public/openapi.json +++ b/docs/public/openapi.json @@ -20519,7 +20519,7 @@ }, "/api/v1/health/readiness": { "get": { - "description": "Readiness probe endpoint.\n\nChecks if the gateway is ready to serve requests by validating:\n- Database connection pool headroom\n- Database connectivity\n- Service availability\n\nUsed by Kubernetes/container orchestrators for readiness probes.\nReturns HTTP 503 if any dependency is unavailable, or if the database\nconnection pool has no capacity left to serve a request.\n\nReturns:\n dict: Status object with health details\n\nRaises:\n HTTPException: 503 if service is not ready", + "description": "Readiness probe endpoint.\n\nChecks if the gateway is ready to serve requests by validating:\n- Database connectivity\n- Service availability\n\nUsed by Kubernetes/container orchestrators for readiness probes.\nReturns HTTP 503 if any dependency is unavailable.\n\nReturns:\n dict: Status object with health details\n\nRaises:\n HTTPException: 503 if service is not ready", "operationId": "health-health_readiness", "responses": { "200": { diff --git a/docs/public/otari.postman_collection.json b/docs/public/otari.postman_collection.json index 19e0ddab51..87bd2fe92f 100644 --- a/docs/public/otari.postman_collection.json +++ b/docs/public/otari.postman_collection.json @@ -1953,7 +1953,7 @@ { "name": "Health Readiness", "request": { - "description": "Readiness probe endpoint.\n\nChecks if the gateway is ready to serve requests by validating:\n- Database connection pool headroom\n- Database connectivity\n- Service availability\n\nUsed by Kubernetes/container orchestrators for readiness probes.\nReturns HTTP 503 if any dependency is unavailable, or if the database\nconnection pool has no capacity left to serve a request.\n\nReturns:\n dict: Status object with health details\n\nRaises:\n HTTPException: 503 if service is not ready", + "description": "Readiness probe endpoint.\n\nChecks if the gateway is ready to serve requests by validating:\n- Database connectivity\n- Service availability\n\nUsed by Kubernetes/container orchestrators for readiness probes.\nReturns HTTP 503 if any dependency is unavailable.\n\nReturns:\n dict: Status object with health details\n\nRaises:\n HTTPException: 503 if service is not ready", "header": [], "method": "GET", "url": { diff --git a/src/gateway/api/routes/health.py b/src/gateway/api/routes/health.py index 2852e711d2..2aa720cec7 100644 --- a/src/gateway/api/routes/health.py +++ b/src/gateway/api/routes/health.py @@ -7,18 +7,11 @@ from gateway.api.deps import get_config, get_db_if_needed from gateway.core.config import DEFAULT_PLATFORM_HEALTH_PATH, GatewayConfig -from gateway.core.database import request_pool_stats from gateway.log_config import logger from gateway.version import __version__ router = APIRouter(prefix="/health", tags=["health"]) -# The ``database`` state the readiness probe reports when every pooled -# connection is already checked out. Its own state rather than "unavailable": -# the database is reachable, this process has run out of ways to reach it, and -# an operator restarts or rescales on that rather than paging the database. -POOL_EXHAUSTED = "pool_exhausted" - async def _check_platform_reachability(config: GatewayConfig) -> bool: """Report whether the platform peer serves its health route. @@ -92,13 +85,11 @@ async def health_readiness( """Readiness probe endpoint. Checks if the gateway is ready to serve requests by validating: - - Database connection pool headroom - Database connectivity - Service availability Used by Kubernetes/container orchestrators for readiness probes. - Returns HTTP 503 if any dependency is unavailable, or if the database - connection pool has no capacity left to serve a request. + Returns HTTP 503 if any dependency is unavailable. Returns: dict: Status object with health details @@ -132,22 +123,6 @@ async def health_readiness( detail={"status": "unhealthy", "database": "unavailable", "version": __version__}, ) - pool = request_pool_stats() - if pool is not None and pool.is_saturated: - logger.error( - "Readiness check refused: database pool saturated, %s of %s connections checked out", - pool.checked_out, - pool.capacity, - ) - raise HTTPException( - status_code=status.HTTP_503_SERVICE_UNAVAILABLE, - detail={ - "status": "unhealthy", - "database": POOL_EXHAUSTED, - "version": __version__, - }, - ) - try: await db.execute(text("SELECT 1")) db_status = "connected" diff --git a/src/gateway/core/database.py b/src/gateway/core/database.py index 6574e325b6..2767620b65 100644 --- a/src/gateway/core/database.py +++ b/src/gateway/core/database.py @@ -4,7 +4,7 @@ import asyncio import contextlib -from collections.abc import AsyncGenerator, AsyncIterator +from collections.abc import AsyncGenerator, AsyncIterator, Iterator from contextlib import asynccontextmanager from dataclasses import dataclass from pathlib import Path @@ -20,6 +20,7 @@ from gateway.core.config import GatewayConfig from gateway.log_config import logger +from gateway.metrics import REGISTRY, Collector, GaugeMetricFamily _engine: AsyncEngine | None = None _SessionLocal: async_sessionmaker[AsyncSession] | None = None @@ -197,11 +198,15 @@ async def release_session(session: AsyncSession | None) -> bool: return True -# The name each pool reports under, in metrics and in anything that iterates -# :func:`pool_stats`. REQUEST_POOL = "request" LOG_POOL = "log" +# The configured ``max_overflow`` of each live pool, by pool name. ``QueuePool`` +# has no public accessor for it, and the gateway is the side that chose the +# value, so it is recorded here at :func:`init_db` time rather than read back +# off the pool. +_pool_max_overflow: dict[str, int] = {} + @runtime_checkable class _CountingPool(Protocol): @@ -228,52 +233,30 @@ def capacity(self) -> int: """The most connections this pool will ever hand out at once.""" return self.size + self.max_overflow - @property - def is_saturated(self) -> bool: - """Whether every connection the pool can hand out is already out. - - A caller checking out here would queue for ``db_pool_timeout`` and then - fail, so this is what lets the readiness probe answer immediately - instead of holding the orchestrator open for the full timeout. - """ - return self.capacity > 0 and self.checked_out >= self.capacity - - -def _engine_pool_stats(engine: AsyncEngine | None) -> PoolStats | None: - """Read *engine*'s pool, or ``None`` when there is nothing to read. - Returns ``None`` for an engine that was never built and for one on - ``NullPool``, which is SQLite's pool here and implements none of the - counters below: it opens a connection per checkout and keeps no pool to - saturate. Callers treat ``None`` as "no pool ceiling applies" rather than - as an error, so this never raises. +def _engine_pool_stats(engine: AsyncEngine | None, max_overflow: int) -> PoolStats | None: + """Read *engine*'s pool. - ``max_overflow`` has no public accessor on ``QueuePool``, so it is read - defensively and falls back to ``0``, which understates capacity rather than - inventing it. + Returns ``None`` when there is no pool to read: no engine yet, or a pool + with no counters (``NullPool``, which SQLite uses). """ if engine is None: return None pool = engine.pool if not isinstance(pool, _CountingPool): return None - # Negative until the pool has created its full complement of base - # connections, which would read as "overflow in use" the wrong way round. + # ``QueuePool`` reports a negative overflow until it has opened its base + # connections. overflow = max(pool.overflow(), 0) return PoolStats( checked_out=pool.checkedout(), checked_in=pool.checkedin(), overflow=overflow, size=pool.size(), - max_overflow=max(getattr(pool, "_max_overflow", 0), 0), + max_overflow=max_overflow, ) -def request_pool_stats() -> PoolStats | None: - """Pool stats for the engine that serves request-scoped sessions.""" - return _engine_pool_stats(_engine) - - def pool_stats() -> dict[str, PoolStats]: """Pool stats for every active engine, keyed by pool name. @@ -281,12 +264,56 @@ def pool_stats() -> dict[str, PoolStats]: :func:`init_db` runs and on SQLite. """ readings = { - REQUEST_POOL: _engine_pool_stats(_engine), - LOG_POOL: _engine_pool_stats(_log_engine), + REQUEST_POOL: _engine_pool_stats(_engine, _pool_max_overflow.get(REQUEST_POOL, 0)), + LOG_POOL: _engine_pool_stats(_log_engine, _pool_max_overflow.get(LOG_POOL, 0)), } return {name: stats for name, stats in readings.items() if stats is not None} +class _PoolCollector(Collector): + """Publish the connection-pool counters at scrape time. + + A collector rather than gauges refreshed on a timer: the counters are + already maintained by SQLAlchemy, so every scrape reads the live pool with + no way to lag it, and a pool that reports nothing emits no series at all + rather than leaving a stale value behind. + """ + + def collect(self) -> Iterator[GaugeMetricFamily]: + checked_out = GaugeMetricFamily( + "gateway_db_pool_connections_checked_out", + "Pooled database connections currently checked out", + labels=["pool"], + ) + idle = GaugeMetricFamily( + "gateway_db_pool_connections_idle", + "Pooled database connections checked in and available", + labels=["pool"], + ) + overflow = GaugeMetricFamily( + "gateway_db_pool_overflow_connections", + "Database connections open beyond the base pool size", + labels=["pool"], + ) + capacity = GaugeMetricFamily( + "gateway_db_pool_capacity", + "Most database connections the pool will hand out at once (size plus max overflow)", + labels=["pool"], + ) + for name, stats in pool_stats().items(): + checked_out.add_metric([name], stats.checked_out) + idle.add_metric([name], stats.checked_in) + overflow.add_metric([name], stats.overflow) + capacity.add_metric([name], stats.capacity) + yield checked_out + yield idle + yield overflow + yield capacity + + +REGISTRY.register(_PoolCollector()) + + def engine_kwargs( config: GatewayConfig, *, @@ -357,6 +384,7 @@ def init_db(config: GatewayConfig) -> None: ) _install_timeout_translation(_engine) _SessionLocal = async_sessionmaker(_engine, expire_on_commit=False) + _pool_max_overflow[REQUEST_POOL] = config.db_max_overflow if is_sqlite: _configure_sqlite_pragmas(_engine) @@ -389,6 +417,7 @@ def init_db(config: GatewayConfig) -> None: ) _install_timeout_translation(_log_engine) _LogSessionLocal = async_sessionmaker(_log_engine, expire_on_commit=False) + _pool_max_overflow[LOG_POOL] = 0 if config.auto_migrate: _run_migrations(database_url) @@ -443,6 +472,7 @@ def _take_engines() -> list[AsyncEngine]: _SessionLocal = None _log_engine = None _LogSessionLocal = None + _pool_max_overflow.clear() return engines @@ -489,7 +519,6 @@ async def _dispose_all() -> None: "init_db", "pool_stats", "release_session", - "request_pool_stats", "reset_db", "translate_timeout_error", ] diff --git a/src/gateway/metrics.py b/src/gateway/metrics.py index 94f0bbc84b..839e29633d 100644 --- a/src/gateway/metrics.py +++ b/src/gateway/metrics.py @@ -1,4 +1,4 @@ -"""Prometheus registry, metric types, HTTP request instrumentation, and database pool gauges for the gateway. +"""Prometheus registry, metric types, and HTTP request instrumentation for the gateway. The metric types are re-exported so that code declaring a metric need not depend on ``prometheus_client`` directly. """ @@ -16,16 +16,26 @@ ProcessCollector, generate_latest, ) +from prometheus_client.core import GaugeMetricFamily +from prometheus_client.registry import Collector from starlette.responses import Response from gateway.core.config import API_ROOT, API_VERSION -from gateway.core.database import pool_stats if TYPE_CHECKING: from starlette.requests import Request from starlette.types import ASGIApp, Message, Receive, Scope, Send -__all__ = ["Counter", "Gauge", "Histogram", "MetricsMiddleware", "REGISTRY", "metrics_endpoint"] +__all__ = [ + "REGISTRY", + "Collector", + "Counter", + "Gauge", + "GaugeMetricFamily", + "Histogram", + "MetricsMiddleware", + "metrics_endpoint", +] REGISTRY = CollectorRegistry() @@ -55,35 +65,6 @@ registry=REGISTRY, ) -DB_POOL_CONNECTIONS_CHECKED_OUT = Gauge( - "gateway_db_pool_connections_checked_out", - "Pooled database connections currently checked out", - ["pool"], - registry=REGISTRY, -) - -DB_POOL_CONNECTIONS_IDLE = Gauge( - "gateway_db_pool_connections_idle", - "Pooled database connections checked in and available", - ["pool"], - registry=REGISTRY, -) - -DB_POOL_OVERFLOW_CONNECTIONS = Gauge( - "gateway_db_pool_overflow_connections", - "Database connections open beyond the base pool size", - ["pool"], - registry=REGISTRY, -) - -DB_POOL_CAPACITY = Gauge( - "gateway_db_pool_capacity", - "Most database connections the pool will hand out at once (size plus max overflow)", - ["pool"], - registry=REGISTRY, -) - - _PROMETHEUS_CONTENT_TYPE = "text/plain; version=0.0.4; charset=utf-8" _UNMATCHED_ENDPOINT = "unmatched" @@ -117,26 +98,8 @@ def _endpoint_label(scope: Scope) -> tuple[str, str]: return template, _NO_VERSION -def refresh_db_pool_metrics() -> None: - """Read the connection pools into their gauges. - - Called at scrape time rather than from a background task: the counters are - already maintained by SQLAlchemy, so reading them costs nothing and a timer - would only add a way for the exported value to lag the pool. A pool that - reports nothing (SQLite's ``NullPool``, or an engine that was never built) - leaves its gauges untouched instead of publishing a zero that would read as - an idle pool. - """ - for name, stats in pool_stats().items(): - DB_POOL_CONNECTIONS_CHECKED_OUT.labels(pool=name).set(stats.checked_out) - DB_POOL_CONNECTIONS_IDLE.labels(pool=name).set(stats.checked_in) - DB_POOL_OVERFLOW_CONNECTIONS.labels(pool=name).set(stats.overflow) - DB_POOL_CAPACITY.labels(pool=name).set(stats.capacity) - - async def metrics_endpoint(request: Request) -> Response: """Serve Prometheus metrics.""" - refresh_db_pool_metrics() body = generate_latest(REGISTRY) return Response(content=body, media_type=_PROMETHEUS_CONTENT_TYPE) diff --git a/tests/unit/test_database_pool_stats.py b/tests/unit/test_database_pool_stats.py index f167ab99cc..d0733f60c4 100644 --- a/tests/unit/test_database_pool_stats.py +++ b/tests/unit/test_database_pool_stats.py @@ -1,26 +1,24 @@ -"""Unit tests for the connection-pool stats helper and the readiness probe.""" +"""Unit tests for the connection-pool stats helper and its Prometheus collector.""" from collections.abc import Iterator from types import SimpleNamespace from typing import Any import pytest -from fastapi import HTTPException -from gateway.api.routes import health from gateway.core import database -from gateway.core.database import LOG_POOL, REQUEST_POOL, PoolStats, pool_stats, request_pool_stats +from gateway.core.database import LOG_POOL, REQUEST_POOL, PoolStats, pool_stats +from gateway.metrics import REGISTRY class _QueuePool: """Stand-in for ``AsyncAdaptedQueuePool`` exposing the counters read here.""" - def __init__(self, checked_out: int, checked_in: int, overflow: int, size: int, max_overflow: int) -> None: + def __init__(self, checked_out: int, checked_in: int, overflow: int, size: int) -> None: self._checked_out = checked_out self._checked_in = checked_in self._overflow = overflow self._size = size - self._max_overflow = max_overflow def checkedout(self) -> int: return self._checked_out @@ -50,59 +48,61 @@ def _engine(pool: Any) -> Any: def _restore_engines() -> Iterator[None]: engine = database._engine log_engine = database._log_engine + overflow = dict(database._pool_max_overflow) yield database._engine = engine database._log_engine = log_engine + database._pool_max_overflow.clear() + database._pool_max_overflow.update(overflow) def test_pool_stats_reads_the_queue_pool_counters() -> None: - database._engine = _engine(_QueuePool(checked_out=7, checked_in=3, overflow=2, size=10, max_overflow=20)) + database._engine = _engine(_QueuePool(checked_out=7, checked_in=3, overflow=2, size=10)) database._log_engine = None + database._pool_max_overflow[REQUEST_POOL] = 20 - stats = request_pool_stats() - - assert stats == PoolStats(checked_out=7, checked_in=3, overflow=2, size=10, max_overflow=20) - assert stats is not None - assert stats.capacity == 30 - assert not stats.is_saturated + readings = pool_stats() + assert readings[REQUEST_POOL] == PoolStats(checked_out=7, checked_in=3, overflow=2, size=10, max_overflow=20) + assert readings[REQUEST_POOL].capacity == 30 -def test_pool_stats_clamps_a_negative_overflow() -> None: - """A pool that has not created its base connections yet reports a negative overflow.""" - database._engine = _engine(_QueuePool(checked_out=1, checked_in=0, overflow=-9, size=10, max_overflow=20)) - stats = request_pool_stats() - - assert stats is not None - assert stats.overflow == 0 +def test_capacity_uses_the_configured_overflow_not_the_pool_object() -> None: + """The gateway chose ``max_overflow``, so capacity comes from the config, not a private field.""" + database._engine = _engine(_QueuePool(checked_out=1, checked_in=1, overflow=0, size=10)) + database._log_engine = None + database._pool_max_overflow[REQUEST_POOL] = 5 + assert pool_stats()[REQUEST_POOL].capacity == 15 -def test_pool_is_saturated_when_every_connection_is_out() -> None: - database._engine = _engine(_QueuePool(checked_out=30, checked_in=0, overflow=20, size=10, max_overflow=20)) - stats = request_pool_stats() +def test_pool_stats_clamps_a_negative_overflow() -> None: + """A pool that has not created its base connections yet reports a negative overflow.""" + database._engine = _engine(_QueuePool(checked_out=1, checked_in=0, overflow=-9, size=10)) + database._log_engine = None - assert stats is not None - assert stats.is_saturated + assert pool_stats()[REQUEST_POOL].overflow == 0 -def test_pool_stats_returns_none_for_a_null_pool() -> None: +def test_pool_stats_omits_a_null_pool() -> None: database._engine = _engine(_NullPool()) + database._log_engine = None - assert request_pool_stats() is None + assert pool_stats() == {} -def test_pool_stats_returns_none_when_the_database_is_not_initialized() -> None: +def test_pool_stats_is_empty_when_the_database_is_not_initialized() -> None: database._engine = None database._log_engine = None - assert request_pool_stats() is None assert pool_stats() == {} def test_pool_stats_covers_both_engines() -> None: - database._engine = _engine(_QueuePool(checked_out=1, checked_in=2, overflow=0, size=10, max_overflow=20)) - database._log_engine = _engine(_QueuePool(checked_out=0, checked_in=1, overflow=0, size=2, max_overflow=0)) + database._engine = _engine(_QueuePool(checked_out=1, checked_in=2, overflow=0, size=10)) + database._log_engine = _engine(_QueuePool(checked_out=0, checked_in=1, overflow=0, size=2)) + database._pool_max_overflow[REQUEST_POOL] = 20 + database._pool_max_overflow[LOG_POOL] = 0 readings = pool_stats() @@ -111,61 +111,32 @@ def test_pool_stats_covers_both_engines() -> None: def test_pool_stats_omits_an_engine_without_a_pool() -> None: - database._engine = _engine(_QueuePool(checked_out=1, checked_in=2, overflow=0, size=10, max_overflow=20)) + database._engine = _engine(_QueuePool(checked_out=1, checked_in=2, overflow=0, size=10)) database._log_engine = _engine(_NullPool()) assert set(pool_stats()) == {REQUEST_POOL} -class _Session: - """Session that records whether the readiness query was attempted.""" - - def __init__(self) -> None: - self.executed = False - - async def execute(self, _statement: Any) -> None: - self.executed = True - - -# The route only reads ``is_hybrid_mode``, so a stand-in keeps this a unit test. -_STANDALONE: Any = SimpleNamespace(is_hybrid_mode=False) - - -@pytest.mark.asyncio -async def test_readiness_refuses_immediately_on_a_saturated_pool() -> None: - database._engine = _engine(_QueuePool(checked_out=30, checked_in=0, overflow=20, size=10, max_overflow=20)) - session: Any = _Session() - - with pytest.raises(HTTPException) as excinfo: - await health.health_readiness(config=_STANDALONE, db=session) - - assert excinfo.value.status_code == 503 - detail: Any = excinfo.value.detail - assert detail["status"] == "unhealthy" - assert detail["database"] == health.POOL_EXHAUSTED - assert detail["database"] != "unavailable" - assert "version" in detail - assert not session.executed - - -@pytest.mark.asyncio -async def test_readiness_queries_the_database_when_the_pool_has_headroom() -> None: - database._engine = _engine(_QueuePool(checked_out=1, checked_in=9, overflow=0, size=10, max_overflow=20)) - session: Any = _Session() +def test_the_collector_publishes_the_live_pool_on_every_scrape() -> None: + database._engine = _engine(_QueuePool(checked_out=4, checked_in=6, overflow=1, size=10)) + database._log_engine = None + database._pool_max_overflow[REQUEST_POOL] = 20 - payload = await health.health_readiness(config=_STANDALONE, db=session) + labels = {"pool": REQUEST_POOL} + assert REGISTRY.get_sample_value("gateway_db_pool_connections_checked_out", labels) == 4.0 + assert REGISTRY.get_sample_value("gateway_db_pool_connections_idle", labels) == 6.0 + assert REGISTRY.get_sample_value("gateway_db_pool_overflow_connections", labels) == 1.0 + assert REGISTRY.get_sample_value("gateway_db_pool_capacity", labels) == 30.0 - assert payload["status"] == "healthy" - assert payload["database"] == "connected" - assert session.executed + database._engine = _engine(_QueuePool(checked_out=9, checked_in=1, overflow=0, size=10)) + assert REGISTRY.get_sample_value("gateway_db_pool_connections_checked_out", labels) == 9.0 -@pytest.mark.asyncio -async def test_readiness_queries_the_database_on_a_null_pool() -> None: - database._engine = _engine(_NullPool()) - session: Any = _Session() - payload = await health.health_readiness(config=_STANDALONE, db=session) +def test_the_collector_publishes_no_series_without_a_pool() -> None: + """SQLite runs on NullPool, and an uninitialized engine has no pool at all.""" + database._engine = None + database._log_engine = None - assert payload["status"] == "healthy" - assert session.executed + assert REGISTRY.get_sample_value("gateway_db_pool_capacity", {"pool": REQUEST_POOL}) is None + assert REGISTRY.get_sample_value("gateway_db_pool_capacity", {"pool": LOG_POOL}) is None diff --git a/tests/unit/test_gateway_metrics.py b/tests/unit/test_gateway_metrics.py index 8e42bc13e7..9d8951498d 100644 --- a/tests/unit/test_gateway_metrics.py +++ b/tests/unit/test_gateway_metrics.py @@ -13,7 +13,6 @@ MetricsMiddleware, _endpoint_label, metrics_endpoint, - refresh_db_pool_metrics, ) @@ -52,7 +51,8 @@ def test_scrape_exposes_the_pinned_families() -> None: """The set of gateway metric families, with their types and label names, is fixed. A labeled family with no series yet shows only its HELP and TYPE lines in a - scrape, so the label names are read off the collectors rather than the text. + scrape, so the label names are read off the collector, or off the family it + yields where the collector is a custom one that keeps no label names. """ import gateway.main # noqa: F401 # imports every module that registers a metric @@ -61,7 +61,8 @@ def test_scrape_exposes_the_pinned_families() -> None: describe = getattr(collector, "describe", collector.collect) for metric in describe(): if metric.name.startswith("gateway_"): - families.add((metric.name, metric.type, tuple(getattr(collector, "_labelnames", ())))) + labelnames = getattr(collector, "_labelnames", ()) or getattr(metric, "_labelnames", ()) + families.add((metric.name, metric.type, tuple(labelnames))) assert families == _EXPOSED_FAMILIES @@ -243,51 +244,3 @@ def test_metrics_expose_process_memory() -> None: assert "process_resident_memory_bytes" in body assert _sample("process_resident_memory_bytes") > 0 - - -class _FakeQueuePool: - """Stand-in for ``AsyncAdaptedQueuePool``, exposing only the counters read.""" - - _max_overflow = 20 - - def checkedout(self) -> int: - return 4 - - def checkedin(self) -> int: - return 6 - - def overflow(self) -> int: - return 1 - - def size(self) -> int: - return 10 - - -def test_db_pool_gauges_are_refreshed_at_scrape_time(monkeypatch: pytest.MonkeyPatch) -> None: - """A leaked connection has to be visible before requests start failing.""" - from types import SimpleNamespace - - from gateway.core import database - - monkeypatch.setattr(database, "_engine", SimpleNamespace(pool=_FakeQueuePool())) - monkeypatch.setattr(database, "_log_engine", None) - - refresh_db_pool_metrics() - - labels = {"pool": "request"} - assert _sample("gateway_db_pool_connections_checked_out", labels) == 4.0 - assert _sample("gateway_db_pool_connections_idle", labels) == 6.0 - assert _sample("gateway_db_pool_overflow_connections", labels) == 1.0 - assert _sample("gateway_db_pool_capacity", labels) == 30.0 - - -def test_db_pool_gauges_are_left_alone_without_a_pool(monkeypatch: pytest.MonkeyPatch) -> None: - """SQLite runs on NullPool, and an uninitialized engine has no pool at all.""" - from gateway.core import database - - monkeypatch.setattr(database, "_engine", None) - monkeypatch.setattr(database, "_log_engine", None) - - refresh_db_pool_metrics() - - assert _sample("gateway_db_pool_capacity", {"pool": "unbuilt"}) == 0.0 diff --git a/web/src/client/schema.ts b/web/src/client/schema.ts index 405ccf986a..0b8290cbc9 100644 --- a/web/src/client/schema.ts +++ b/web/src/client/schema.ts @@ -1220,13 +1220,11 @@ export interface paths { * @description Readiness probe endpoint. * * Checks if the gateway is ready to serve requests by validating: - * - Database connection pool headroom * - Database connectivity * - Service availability * * Used by Kubernetes/container orchestrators for readiness probes. - * Returns HTTP 503 if any dependency is unavailable, or if the database - * connection pool has no capacity left to serve a request. + * Returns HTTP 503 if any dependency is unavailable. * * Returns: * dict: Status object with health details