feat(observability): add dual local logging sinks and static environment context
Why:
- Wanted human-readable console output while developing locally, without
losing a machine-parseable log for later grepping/parsing. A single
renderer chosen by a flag can't do both at once.
- ADR-0011 had no way to correlate an issue with a specific deployment
(build/region/instance) independent of any one request.
Changes:
- configure_logging() now builds two independent handlers: console (always
on, colored unless LOG_JSON_FORMAT=true) and an optional rotating JSON file
(LOG_FILE_PATH, unset by default) -- the same structlog event fans out to
both, so call sites are unaffected.
- A static structlog processor binds env/service_version onto every event.
Deliberately not a contextvar: RequestIdMiddleware's clear_contextvars()
would wipe a value bound there before the first request.
- New settings: APP_SERVICE_VERSION, LOG_FILE_PATH/LOG_FILE_MAX_BYTES/
LOG_FILE_BACKUP_COUNT.
- ADR-0011 amended with both decisions ("console and file are independent
sinks locally"; "bind process-level environment context once at startup").
Impact:
- configure_logging() signature changed to (logging_settings, app_settings);
both call sites (lifespan, qdrant_bootstrap CLI) updated.
This commit is contained in:
@@ -70,17 +70,30 @@ logger.info(
|
||||
Do not build log messages by interpolating operational metadata into prose.
|
||||
Prefer fields over long strings because fields are queryable.
|
||||
|
||||
### Emit JSON logs by default in production
|
||||
### Emit JSON logs by default in production; console and file are independent sinks locally
|
||||
|
||||
Production logs are JSON on stdout so process managers, container runtimes, and
|
||||
log collectors can ingest them directly. Local development may use a colored
|
||||
console renderer controlled by configuration.
|
||||
log collectors can ingest them directly. This does not change.
|
||||
|
||||
File logging is optional and mainly for local development. If enabled, it must
|
||||
use explicit rotation settings such as `maxBytes` and `backupCount`. Do not rely
|
||||
on a default `RotatingFileHandler` with no rotation parameters. In containerized
|
||||
production, stdout/stderr collection is preferred over writing `logs/app.log`
|
||||
inside the application container.
|
||||
Locally, stdout and an optional file are two **independent, simultaneous**
|
||||
handlers on the same logger, not a single renderer chosen by a flag — the same
|
||||
structlog event fans out to both:
|
||||
|
||||
- **Console handler**: always on, `structlog.dev.ConsoleRenderer(colors=True)`.
|
||||
This is what a developer reads while the process runs, so it stays
|
||||
human-readable regardless of whether file logging is also enabled.
|
||||
- **File handler**: off by default, enabled by setting `LOG_FILE_PATH`. Always
|
||||
renders JSON (`structlog.processors.JSONRenderer()`), independent of the
|
||||
console handler's renderer, so a saved log is machine-parseable even though
|
||||
the terminal output next to it is not. Must use explicit rotation
|
||||
(`RotatingFileHandler` with `maxBytes`/`backupCount` — never an unrotated
|
||||
handler).
|
||||
|
||||
In containerized production, stdout/stderr collection remains preferred over
|
||||
writing `logs/app.log` inside the application container, so `LOG_FILE_PATH` is
|
||||
expected to be unset there; the file handler exists for local development,
|
||||
where reading a colored terminal *and* keeping a JSON trail to grep/parse later
|
||||
are both useful at once.
|
||||
|
||||
### Configure stdlib and structlog together
|
||||
|
||||
@@ -188,6 +201,36 @@ Notes:
|
||||
- `structlog.contextvars.merge_contextvars` ensures request-bound fields appear
|
||||
on both structlog and stdlib logs processed through the formatter.
|
||||
|
||||
### Bind process-level environment context once at startup
|
||||
|
||||
Deployment identity — which build is running, in which environment, on which
|
||||
instance — answers a different question than request correlation: "is this
|
||||
issue specific to one deployment / one region / one instance?" rather than "is
|
||||
this issue specific to one request?" It does not vary per request, so it must
|
||||
not go through `structlog.contextvars`, which `RequestIdMiddleware` clears on
|
||||
every request; a value bound there before the first request would be wiped the
|
||||
moment that middleware runs.
|
||||
|
||||
Instead, add a static structlog **processor** — a plain closure over values read
|
||||
once at `configure_logging()` time — so it runs on every event regardless of
|
||||
request context:
|
||||
|
||||
```python
|
||||
def _bind_environment(settings: AppLimitSettings):
|
||||
def processor(logger, method_name, event_dict):
|
||||
event_dict["env"] = settings.env
|
||||
event_dict["service_version"] = settings.service_version
|
||||
return event_dict
|
||||
|
||||
return processor
|
||||
```
|
||||
|
||||
`service_version` should be the deployed commit SHA or release tag (e.g. from a
|
||||
`GIT_SHA`/`APP_VERSION` build-time env var — not computed at runtime by
|
||||
shelling out to `git`). This makes "is this only happening on the new
|
||||
deployment?" answerable directly from logs, without cross-referencing a
|
||||
separate deployment record.
|
||||
|
||||
### Bind request context with contextvars
|
||||
|
||||
At FastAPI ingress, clear stale context, bind request identifiers, and return the
|
||||
|
||||
Reference in New Issue
Block a user