feat(observability): catch post-200 stream failures

Merged
GI
2mo ago
+1001-1

Summary

  • Add the bounded omi_client_journey_* family for accepted attempts, one-shot terminal outcomes, bounded issue classes, and duration.
  • Keep the live, alert-backed omi_journey_* family unchanged while adding client segmentation and authenticated /metrics exposure to desktop-backend.
  • Make stream classification fail closed after headers are committed: callers must provide both success_when and failure_when, and an in-band error frame terminalizes immediately so a later [DONE] cannot overwrite it.

The next inventory layer contains 50 semantic capabilities, including 35 that can fail after a committed 2xx response. That inventory folds onto this bounded foundation rather than minting capability labels here.

Post-headers failure class

This catches the incident shape status-code monitoring cannot see: /v2/chat/completions commits HTTP 200/SSE headers, yields data: {"error": {"code": 502, ...}}, then yields data: [DONE] and exhausts normally. The regression test feeds the literal compact desktop_chat frames and requires outcome="failure" with issue_class="upstream_rejected".

ClientJourneyAttempt.observe_stream now requires an explicit failure predicate. A detected failure is terminal before the frame is handed on, so later content, [DONE], clean exhaustion, cancellation, or another exception cannot turn it into success.

Cardinality budget

All label values are coerced through closed vocabularies: 9 journeys, 10 client kinds, 5 outcomes, and 11 issue classes. client_kind is deliberately omitted from the histogram. With the pinned prometheus-client==0.21.1 actually emitting _created samples in this process, zero-initialization exports at most 3,915 series per process: 180 accepted, 900 terminal, 1,980 issue, and 855 histogram series. There are no uid, session, request, content, error-text, model, or revision labels.

Grok review

Addressed the blocking classification defect and the requested mutation gaps:

  • added first-class failure_when precedence and the literal 502-error-then-[DONE] regression;
  • attach stream ownership when observe_stream is called, allowing StreamingResponse consumption after with exits;
  • cancel abandoned, never-iterated, CancelledError, GeneratorExit, ClientDisconnect, and OSError streams;
  • cover normal __exit__, attached-stream __exit__, empty streams, error-only streams, disconnects, and abandonment;
  • replace the tautological pi-mono assertion with two platform populations that become intentionally fused only when the authoritative platform header is absent;
  • collapse non-string platform headers to unknown before string operations;
  • document the required error-frame/[DONE] observer contract.

One review calculation was incorrect in this checkout: 2,340 assumes Prometheus _created samples are disabled, but direct collection with the pinned 0.21.1 environment produced 3,915 samples. The documented budget uses the measured value. The review's separate observation that deployed desktop-backend does not yet receive METRICS_SECRET is correct; this foundation adds no production scrape target or journey call sites, so deployment secret/scrape wiring remains with that follow-up rather than broadening this contract PR.

Product invariants affected

none

Verification

  • backend/.venv/bin/python -m pytest backend/tests/unit/test_client_journey_foundation.py backend/tests/unit/test_desktop_backend_cors.py backend/tests/unit/test_journey_observability.py -q --tb=short — 37 passed in 1.95s.
  • backend/.venv/bin/pyright backend/utils/journey_metrics_contract.py backend/utils/observability/journeys.py — 0 errors.
  • make preflight — passed all selected checks.
  • Direct collection under prometheus-client==0.21.1 — 3,915 bounded series.

No deployed Cloud Run revision was exercised; this PR establishes the metric and observer contract but intentionally adds no production journey call site or scrape target.

Failure class

Added after review: recorders fail open without going silent

Two stacked call-site branches independently reached for the same guard — instrumentation must
never break the request it observes — with different structure, and would have conflicted over it.
It is consolidated here instead, with one change in meaning: both branches used a bare
except Exception: pass.

A recorder that quietly stops writing is indistinguishable from a healthy path with nothing to
report. That is precisely the failure mode this metric family exists to expose, so it must not be
reproduced inside the family itself. _record_fail_open swallows the exception for the caller and
logs it at a bounded rate for the operator, following the pattern already used by
llm_gateway/gateway/metrics.py.

Verified by mutation: making the helper re-raise fails both new tests — one asserting the
operator-visible warning, one proving a broken recorder still delivers every frame to the user.

Failure-Class: none

No registered product failure class describes an additive telemetry contract for post-header in-band failures; the regression is the reusable guard surface for the incident shape.