From b590d2708dcdf44ff1c27cde77c499aa7012fb49 Mon Sep 17 00:00:00 2001 From: Mathias Date: Fri, 12 Jun 2026 08:27:56 +0200 Subject: [PATCH] =?UTF-8?q?docs(adr):=20ADR-030=20observability=20?= =?UTF-8?q?=E2=80=94=20slog=20+=20Prometheus=20metrics=20(issue=20#15)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- DECISIONS.md | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/DECISIONS.md b/DECISIONS.md index 0c6d1e8..5798e51 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1162,6 +1162,63 @@ re-login now rare (30-day idle or explicit logout), it matters far less. --- +## ADR-030 — Observability: slog timing + Prometheus metrics (AI-focused) + +**Status:** Proposed (2026-06-11). Issue #15. **Draft for review — no code yet.** + +**Context / requirements.** Nothing measures the activities that drive Tapir's performance and +UX, and the Stage-0 eval gate (ADR-016) needs a *performance* dimension to sit beside the +return-usage one. We need timing for: caption fetches (the scarce op), summarization (which model +won, how long, fallbacks), Q&A latency, LLM token spend, and basic session/usage (request rate, +latency by route, logins). Requirements: +- R1: structured `slog` timing at each AI call site (human-readable, already the logging stack). +- R2: Prometheus metrics for the same, scrapeable by the cluster's prometheus-operator. +- R3: **AI metrics are the priority** — summarize latency by `model`/`outcome`/`fallback`, + caption-fetch latency by `outcome`, chat latency by `model`, and LLM `tokens` by model+kind. +- R4: HTTP/session metrics via middleware — request count + latency by route, logins. +- R5: bounded label cardinality (no per-user, no raw-path labels). +- R6: `/metrics` must NOT be publicly exposed. + +**Decision / architecture.** +1. **New package `internal/metrics`** owns all Prometheus collectors + a typed API + (`ObserveSummarize`, `ObserveCaptionFetch`, `ObserveChat`, `RecordTokens`, `IncLogin`, + `HTTPMiddleware`, `Handler`). Adapters call this API; they never import prometheus types. +2. **New dependency `github.com/prometheus/client_golang`.** Justification: it is *the* standard + Go Prometheus client and the cluster already runs prometheus-operator; hand-rolling exposition + is not worth it. (Needs the dep-justification note in the commit per repo rules.) +3. **The copied `llm` package stays stdlib-only (ADR-004).** It must not import `internal/metrics`. + Token usage is surfaced via an **optional callback** `llm.WithUsageHook(func(model string, prompt, completion int))` + set at wiring time (`buildSummarizer`/`buildChat`) to `metrics.RecordTokens`; `llm.Client` only + gains parsing of the response `usage` block. Our own adapters (`summarizer`, `youtube`, `chat`) + may import `internal/metrics` directly. +4. **HTTP middleware** reads `r.Pattern` AFTER routing (Go 1.22 sets it during ServeMux match), so + the `route` label is the bounded registered pattern (`GET /v/{videoId}`), satisfying R5; + unmatched → `other`. +5. **Dedicated metrics port** (`TAPIR_METRICS_ADDR`, default `:9090`) served by a second + `http.Server` in `cmdServe`; `/metrics` is never on the public app mux (R6). A **PodMonitor** + in `mathias/infra` scrapes it; the deployment exposes the port. +6. **slog** elapsed fields are emitted alongside each metric at the call sites (R1). + +**Hook points (where the instrumentation lands).** +- `summarizer.Summarize` — per-endpoint timing + outcome (`success`/`parse_error`/`error`) + fallback flag. +- `youtube.FetchTranscript` — fetch timing + outcome from `domain.Transcript.Source`. +- `chat.Service` answer — timing by model. +- `llm.Client.Complete` — parse `usage`, fire the usage hook. +- `oidc.handleCallback` — `IncLogin`. +- `cmdServe` — wrap `Router()` in `metrics.HTTPMiddleware`; start the metrics server. + +**Out of scope / later.** Persisting per-summary latency into Postgres for `tapir report` +(derive UX latency — publish/discovery → summary — from existing timestamps first; only persist +op-latency if the scrape proves insufficient). SPA view (#16) and visual refresh (#17). + +**Reversibility.** Additive: a new package + middleware + a metrics port. Removing the PodMonitor +stops scraping; the app is unaffected. No schema change. + +**Next steps (gated):** on approval of this ADR → BDD scenarios (`docs/use-cases/observability.feature` ++ scenario-coverage map) → TDD → implement → SemVer + docs + PodMonitor. + +--- + ## Rejected alternatives Approaches considered during the 2026-06-02 planning + grill session and **deliberately not