diff --git a/services/backend/app/main.py b/services/backend/app/main.py index 3579403..5bc7562 100644 --- a/services/backend/app/main.py +++ b/services/backend/app/main.py @@ -10,9 +10,13 @@ from app.crud import create_item, delete_item, list_items from app.deps import get_db -from app.metrics import HTTP_REQUESTS_TOTAL, REQUEST_LATENCY_SECONDS +from app.metrics import FE_WEB_VITAL, HTTP_REQUESTS_TOTAL, REQUEST_LATENCY_SECONDS from app.schemas import ItemCreate, ItemOut +# Known web-vitals names; the gauge label stays a bounded set by +# construction, anything else from the network is rejected +ALLOWED_WEB_VITALS = {"CLS", "FCP", "INP", "LCP", "TTFB"} + # Logging configuration logging.basicConfig(level=logging.INFO, format="%(asctime)s - %(name)s - %(levelname)s - %(message)s", datefmt="%Y-%m-%d %H:%M:%S") logger = logging.getLogger(__name__) @@ -23,6 +27,18 @@ app.add_middleware(CORSMiddleware, allow_origins=["*"], allow_methods=["*"], allow_headers=["*"]) +def _endpoint_label(request: Request) -> str: + """Metric label for the endpoint: the matched route template. + + Never the raw URL path -- raw paths (/items/1, /items/2, scanner + noise) create unbounded label cardinality, and path parameters make + routes like DELETE /items/{item_id} invisible to SLO rules that + filter on the endpoint label. + """ + route = request.scope.get("route") + return getattr(route, "path", None) or "__unmatched__" + + @app.middleware("http") async def metrics_mw(request: Request, call_next) -> Response: """Middleware for HTTP metrics and logging""" @@ -31,8 +47,9 @@ async def metrics_mw(request: Request, call_next) -> Response: try: response: Response = await call_next(request) duration = time.time() - start - REQUEST_LATENCY_SECONDS.labels(request.method, request.url.path).observe(duration) - HTTP_REQUESTS_TOTAL.labels(request.method, request.url.path, str(response.status_code)).inc() + endpoint = _endpoint_label(request) + REQUEST_LATENCY_SECONDS.labels(request.method, endpoint).observe(duration) + HTTP_REQUESTS_TOTAL.labels(request.method, endpoint, str(response.status_code)).inc() logger.info(f"Response: {request.method} {request.url.path} - {response.status_code} ({duration:.3f}s)") return response except Exception as e: @@ -92,13 +109,19 @@ def metrics() -> PlainTextResponse: return PlainTextResponse(generate_latest(), media_type=CONTENT_TYPE_LATEST) -# Receive web-vitals from frontend (if added) +# Receive web-vitals reports from the frontend @app.post("/metrics/frontend") async def metrics_frontend(req: Request) -> dict[str, bool]: - data = await req.json() - from app.metrics import FE_WEB_VITAL # import here to avoid cycles - - name = str(data.get("name")) - value = float(data.get("value", 0)) + try: + data = await req.json() + except Exception: + raise HTTPException(status_code=400, detail="Invalid JSON") from None + name = data.get("name") + if name not in ALLOWED_WEB_VITALS: + raise HTTPException(status_code=400, detail="Unknown metric name") + try: + value = float(data.get("value")) + except (TypeError, ValueError): + raise HTTPException(status_code=400, detail="Invalid metric value") from None FE_WEB_VITAL.labels(name=name).set(value) return {"ok": True} diff --git a/services/backend/pyproject.toml b/services/backend/pyproject.toml index aa63c8b..45dd28d 100644 --- a/services/backend/pyproject.toml +++ b/services/backend/pyproject.toml @@ -39,7 +39,11 @@ packages = ["app"] [tool.pytest.ini_options] # Use auto mode for compatibility with anyio (Starlette) asyncio_mode = "auto" -asyncio_default_fixture_loop_scope = "function" +# One event loop for the whole suite: the engine (and its connection +# pool) is module-global, and pooled asyncpg connections are bound to +# the loop they were created on -- per-test loops would poison the pool +asyncio_default_fixture_loop_scope = "session" +asyncio_default_test_loop_scope = "session" [tool.ruff] target-version = "py312" diff --git a/services/backend/tests/conftest.py b/services/backend/tests/conftest.py index 1cfc70f..fe7c459 100644 --- a/services/backend/tests/conftest.py +++ b/services/backend/tests/conftest.py @@ -38,13 +38,17 @@ async def _cleanup_db_pool(): warnings.warn(f"Failed to cleanup items: {e}", stacklevel=2) - # Then close all connections from pool before closing event loop - # This helps avoid "Event loop is closed" errors - from contextlib import suppress - with suppress(Exception): - # Close all active connections from pool - await engine.dispose() +@pytest_asyncio.fixture(scope="session", autouse=True) +async def _dispose_engine_pool(): + """Dispose the shared engine pool once, after the whole session. + + Runs on the session-scoped event loop (see pyproject pytest config), + the same loop every pooled connection was created on. The previous + per-test dispose existed only to survive per-test loops. + """ + yield + await engine.dispose() @pytest_asyncio.fixture diff --git a/services/backend/tests/test_metrics.py b/services/backend/tests/test_metrics.py new file mode 100644 index 0000000..8b668db --- /dev/null +++ b/services/backend/tests/test_metrics.py @@ -0,0 +1,56 @@ +import pytest + + +@pytest.mark.asyncio +async def test_web_vital_accepted(client): + resp = await client.post("/metrics/frontend", json={"name": "LCP", "value": 1234.5}) + assert resp.status_code == 200 + assert resp.json() == {"ok": True} + + metrics = (await client.get("/metrics")).text + assert 'fe_web_vital{name="LCP"} 1234.5' in metrics + + +@pytest.mark.asyncio +async def test_web_vital_unknown_name_rejected(client): + resp = await client.post("/metrics/frontend", json={"name": "EVIL" * 10, "value": 1}) + assert resp.status_code == 400 + + metrics = (await client.get("/metrics")).text + assert "EVIL" not in metrics + + +@pytest.mark.asyncio +async def test_web_vital_bad_value_rejected(client): + resp = await client.post("/metrics/frontend", json={"name": "CLS", "value": "not-a-number"}) + assert resp.status_code == 400 + + resp = await client.post("/metrics/frontend", json={"name": "CLS"}) + assert resp.status_code == 400 + + +@pytest.mark.asyncio +async def test_web_vital_invalid_json_rejected(client): + resp = await client.post("/metrics/frontend", content=b"not json", headers={"Content-Type": "application/json"}) + assert resp.status_code == 400 + + +@pytest.mark.asyncio +async def test_endpoint_label_uses_route_template(client): + # 404 on a real route: still the template, not the raw path + resp = await client.delete("/items/999999") + assert resp.status_code == 404 + + metrics = (await client.get("/metrics")).text + assert 'endpoint="/items/{item_id}"' in metrics + assert 'endpoint="/items/999999"' not in metrics + + +@pytest.mark.asyncio +async def test_endpoint_label_bounded_for_unmatched_paths(client): + resp = await client.get("/definitely/not/a/route") + assert resp.status_code == 404 + + metrics = (await client.get("/metrics")).text + assert 'endpoint="__unmatched__"' in metrics + assert 'endpoint="/definitely/not/a/route"' not in metrics