feat: enhance logging configuration and add request logging context
This commit is contained in:
@@ -2,7 +2,6 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
from dataclasses import dataclass
|
||||
from datetime import timedelta
|
||||
from typing import TYPE_CHECKING, Literal
|
||||
@@ -11,6 +10,7 @@ from fastapi import Depends, Header, HTTPException, Request, status
|
||||
from sqlmodel import col, select
|
||||
|
||||
from app.core.agent_tokens import verify_agent_token
|
||||
from app.core.logging import get_logger
|
||||
from app.core.time import utcnow
|
||||
from app.db.session import get_session
|
||||
from app.models.agents import Agent
|
||||
@@ -18,7 +18,7 @@ from app.models.agents import Agent
|
||||
if TYPE_CHECKING:
|
||||
from sqlmodel.ext.asyncio.session import AsyncSession
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
logger = get_logger(__name__)
|
||||
|
||||
_LAST_SEEN_TOUCH_INTERVAL = timedelta(seconds=30)
|
||||
_SAFE_METHODS = frozenset({"GET", "HEAD", "OPTIONS"})
|
||||
|
||||
@@ -2,7 +2,6 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
from dataclasses import dataclass
|
||||
from typing import TYPE_CHECKING, Literal
|
||||
|
||||
@@ -17,6 +16,7 @@ from pydantic import BaseModel, ValidationError
|
||||
from starlette.concurrency import run_in_threadpool
|
||||
|
||||
from app.core.config import settings
|
||||
from app.core.logging import get_logger
|
||||
from app.db import crud
|
||||
from app.db.session import get_session
|
||||
from app.models.users import User
|
||||
@@ -25,7 +25,7 @@ if TYPE_CHECKING:
|
||||
from clerk_backend_api.models.user import User as ClerkUser
|
||||
from sqlmodel.ext.asyncio.session import AsyncSession
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
logger = get_logger(__name__)
|
||||
security = HTTPBearer(auto_error=False)
|
||||
SECURITY_DEP = Depends(security)
|
||||
SESSION_DEP = Depends(get_session)
|
||||
@@ -331,9 +331,8 @@ async def _get_or_sync_user(
|
||||
)
|
||||
else:
|
||||
logger.debug(
|
||||
"auth.user.sync clerk_user_id=%s updated=%s fetched_profile=%s",
|
||||
"auth.user.sync.noop clerk_user_id=%s fetched_profile=%s",
|
||||
clerk_user_id_log,
|
||||
changed,
|
||||
should_fetch_profile,
|
||||
)
|
||||
if not user.email:
|
||||
|
||||
@@ -42,6 +42,8 @@ class Settings(BaseSettings):
|
||||
log_level: str = "INFO"
|
||||
log_format: str = "text"
|
||||
log_use_utc: bool = False
|
||||
request_log_slow_ms: int = Field(default=1000, ge=0)
|
||||
request_log_include_health: bool = False
|
||||
|
||||
@model_validator(mode="after")
|
||||
def _defaults(self) -> Self:
|
||||
|
||||
@@ -2,8 +2,8 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
from collections.abc import Awaitable, Callable
|
||||
from time import perf_counter
|
||||
from typing import TYPE_CHECKING, Any, Final
|
||||
from uuid import uuid4
|
||||
|
||||
@@ -13,12 +13,23 @@ from fastapi.responses import JSONResponse
|
||||
from starlette.exceptions import HTTPException as StarletteHTTPException
|
||||
from starlette.responses import Response
|
||||
|
||||
from app.core.config import settings
|
||||
from app.core.logging import (
|
||||
TRACE_LEVEL,
|
||||
get_logger,
|
||||
reset_request_id,
|
||||
reset_request_route_context,
|
||||
set_request_id,
|
||||
set_request_route_context,
|
||||
)
|
||||
|
||||
if TYPE_CHECKING: # pragma: no cover
|
||||
from starlette.types import ASGIApp, Message, Receive, Scope, Send
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
logger = get_logger(__name__)
|
||||
|
||||
REQUEST_ID_HEADER: Final[str] = "X-Request-Id"
|
||||
_HEALTH_CHECK_PATHS: Final[frozenset[str]] = frozenset({"/health", "/healthz", "/readyz"})
|
||||
|
||||
ExceptionHandler = Callable[[Request, Exception], Response | Awaitable[Response]]
|
||||
|
||||
@@ -31,6 +42,8 @@ class RequestIdMiddleware:
|
||||
self._app = app
|
||||
self._header_name = header_name
|
||||
self._header_name_bytes = header_name.lower().encode("latin-1")
|
||||
self._slow_request_ms = settings.request_log_slow_ms
|
||||
self._include_health_logs = settings.request_log_include_health
|
||||
|
||||
async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
|
||||
"""Inject request-id into request state and response headers."""
|
||||
@@ -38,18 +51,80 @@ class RequestIdMiddleware:
|
||||
await self._app(scope, receive, send)
|
||||
return
|
||||
|
||||
method = str(scope.get("method") or "UNKNOWN").upper()
|
||||
path = str(scope.get("path") or "")
|
||||
client = scope.get("client")
|
||||
client_ip: str | None = None
|
||||
if isinstance(client, tuple) and client and isinstance(client[0], str):
|
||||
client_ip = client[0]
|
||||
should_log = self._include_health_logs or path not in _HEALTH_CHECK_PATHS
|
||||
started_at = perf_counter()
|
||||
status_code: int | None = None
|
||||
|
||||
request_id = self._get_or_create_request_id(scope)
|
||||
context_token = set_request_id(request_id)
|
||||
route_context_tokens = set_request_route_context(method, path)
|
||||
if should_log:
|
||||
logger.log(
|
||||
TRACE_LEVEL,
|
||||
"http.request.start",
|
||||
extra={
|
||||
"method": method,
|
||||
"path": path,
|
||||
"client_ip": client_ip,
|
||||
},
|
||||
)
|
||||
|
||||
async def send_with_request_id(message: Message) -> None:
|
||||
nonlocal status_code
|
||||
if message["type"] == "http.response.start":
|
||||
# Starlette uses `list[tuple[bytes, bytes]]` here.
|
||||
headers: list[tuple[bytes, bytes]] = message.setdefault("headers", [])
|
||||
if not any(key.lower() == self._header_name_bytes for key, _ in headers):
|
||||
request_id_bytes = request_id.encode("latin-1")
|
||||
headers.append((self._header_name_bytes, request_id_bytes))
|
||||
status = message.get("status")
|
||||
status_code = status if isinstance(status, int) else 500
|
||||
if should_log:
|
||||
duration_ms = int((perf_counter() - started_at) * 1000)
|
||||
extra = {
|
||||
"method": method,
|
||||
"path": path,
|
||||
"status_code": status_code,
|
||||
"duration_ms": duration_ms,
|
||||
"client_ip": client_ip,
|
||||
}
|
||||
if status_code >= 500:
|
||||
logger.error("http.request.complete", extra=extra)
|
||||
elif status_code >= 400:
|
||||
logger.warning("http.request.complete", extra=extra)
|
||||
else:
|
||||
logger.debug("http.request.complete", extra=extra)
|
||||
if self._slow_request_ms and duration_ms >= self._slow_request_ms:
|
||||
logger.warning(
|
||||
"http.request.slow",
|
||||
extra={
|
||||
**extra,
|
||||
"slow_threshold_ms": self._slow_request_ms,
|
||||
},
|
||||
)
|
||||
await send(message)
|
||||
|
||||
await self._app(scope, receive, send_with_request_id)
|
||||
try:
|
||||
await self._app(scope, receive, send_with_request_id)
|
||||
finally:
|
||||
if should_log and status_code is None:
|
||||
logger.warning(
|
||||
"http.request.incomplete",
|
||||
extra={
|
||||
"method": method,
|
||||
"path": path,
|
||||
"duration_ms": int((perf_counter() - started_at) * 1000),
|
||||
"client_ip": client_ip,
|
||||
},
|
||||
)
|
||||
reset_request_route_context(route_context_tokens)
|
||||
reset_request_id(context_token)
|
||||
|
||||
def _get_or_create_request_id(self, scope: Scope) -> str:
|
||||
# Accept a client-provided request id if present.
|
||||
|
||||
@@ -7,6 +7,7 @@ import logging
|
||||
import os
|
||||
import sys
|
||||
import time
|
||||
from contextvars import ContextVar, Token
|
||||
from datetime import UTC, datetime
|
||||
from types import TracebackType
|
||||
from typing import Any
|
||||
@@ -17,6 +18,9 @@ from app.core.version import APP_NAME, APP_VERSION
|
||||
TRACE_LEVEL = 5
|
||||
EXC_INFO_TUPLE_SIZE = 3
|
||||
logging.addLevelName(TRACE_LEVEL, "TRACE")
|
||||
_REQUEST_ID_CONTEXT: ContextVar[str | None] = ContextVar("request_id", default=None)
|
||||
_REQUEST_METHOD_CONTEXT: ContextVar[str | None] = ContextVar("request_method", default=None)
|
||||
_REQUEST_PATH_CONTEXT: ContextVar[str | None] = ContextVar("request_path", default=None)
|
||||
|
||||
|
||||
def _coerce_exc_info(
|
||||
@@ -75,6 +79,53 @@ def _trace(self: logging.Logger, message: str, *args: object, **kwargs: object)
|
||||
|
||||
logging.Logger.trace = _trace # type: ignore[attr-defined]
|
||||
|
||||
|
||||
def set_request_id(request_id: str | None) -> Token[str | None]:
|
||||
"""Bind request-id to logging context for the current task."""
|
||||
normalized = (request_id or "").strip() or None
|
||||
return _REQUEST_ID_CONTEXT.set(normalized)
|
||||
|
||||
|
||||
def reset_request_id(token: Token[str | None]) -> None:
|
||||
"""Reset request-id context to a previous token value."""
|
||||
_REQUEST_ID_CONTEXT.reset(token)
|
||||
|
||||
|
||||
def get_request_id() -> str | None:
|
||||
"""Return request-id currently bound to logging context."""
|
||||
return _REQUEST_ID_CONTEXT.get()
|
||||
|
||||
|
||||
def set_request_route_context(
|
||||
method: str | None,
|
||||
path: str | None,
|
||||
) -> tuple[Token[str | None], Token[str | None]]:
|
||||
"""Bind request method/path to logging context for the current task."""
|
||||
normalized_method = (method or "").strip().upper() or None
|
||||
normalized_path = (path or "").strip() or None
|
||||
return (
|
||||
_REQUEST_METHOD_CONTEXT.set(normalized_method),
|
||||
_REQUEST_PATH_CONTEXT.set(normalized_path),
|
||||
)
|
||||
|
||||
|
||||
def reset_request_route_context(tokens: tuple[Token[str | None], Token[str | None]]) -> None:
|
||||
"""Reset request method/path context to previously-bound values."""
|
||||
method_token, path_token = tokens
|
||||
_REQUEST_METHOD_CONTEXT.reset(method_token)
|
||||
_REQUEST_PATH_CONTEXT.reset(path_token)
|
||||
|
||||
|
||||
def get_request_method() -> str | None:
|
||||
"""Return request method currently bound to logging context."""
|
||||
return _REQUEST_METHOD_CONTEXT.get()
|
||||
|
||||
|
||||
def get_request_path() -> str | None:
|
||||
"""Return request path currently bound to logging context."""
|
||||
return _REQUEST_PATH_CONTEXT.get()
|
||||
|
||||
|
||||
_STANDARD_LOG_RECORD_ATTRS = {
|
||||
"args",
|
||||
"asctime",
|
||||
@@ -117,6 +168,18 @@ class AppLogFilter(logging.Filter):
|
||||
"""Attach app metadata fields to each emitted record."""
|
||||
record.app = self._app_name
|
||||
record.version = self._version
|
||||
if not getattr(record, "request_id", None):
|
||||
request_id = get_request_id()
|
||||
if request_id:
|
||||
record.request_id = request_id
|
||||
if not getattr(record, "method", None):
|
||||
method = get_request_method()
|
||||
if method:
|
||||
record.method = method
|
||||
if not getattr(record, "path", None):
|
||||
path = get_request_path()
|
||||
if path:
|
||||
record.path = path
|
||||
return True
|
||||
|
||||
|
||||
@@ -227,6 +290,18 @@ class AppLogger:
|
||||
logger = logging.getLogger(name)
|
||||
logger.disabled = True
|
||||
|
||||
logging.getLogger(__name__).info(
|
||||
"logging.configured level=%s format=%s use_utc=%s",
|
||||
level_name,
|
||||
format_name,
|
||||
settings.log_use_utc,
|
||||
)
|
||||
logging.getLogger(__name__).debug(
|
||||
"logging.libraries uvicorn_level=%s sql_enabled=%s",
|
||||
level_name,
|
||||
level_name == "TRACE",
|
||||
)
|
||||
|
||||
cls._configured = True
|
||||
|
||||
@classmethod
|
||||
@@ -240,3 +315,8 @@ class AppLogger:
|
||||
def configure_logging() -> None:
|
||||
"""Configure global application logging once during startup."""
|
||||
AppLogger.configure()
|
||||
|
||||
|
||||
def get_logger(name: str | None = None) -> logging.Logger:
|
||||
"""Return an app logger from the centralized logger configuration."""
|
||||
return AppLogger.get_logger(name)
|
||||
|
||||
Reference in New Issue
Block a user