Skip to content

fix(backend): metric label hygiene -- bounded cardinality, SLO-visible routes - #146

Merged
vovinacci merged 1 commit into
mainfrom
fix/metric-label-hygiene
Jul 13, 2026
Merged

fix(backend): metric label hygiene -- bounded cardinality, SLO-visible routes#146
vovinacci merged 1 commit into
mainfrom
fix/metric-label-hygiene

Conversation

@vovinacci

Copy link
Copy Markdown
Owner

Why

Repo review round surfaced two metric-label cardinality bombs (HIGH) and confirmed DELETE requests were invisible to every SLO. This is the backend half; the SLO-rule regex update ships in the observability PR.

Change model

  • Affected components:
    • services/backend/app/main.py: middleware labels metrics with the matched route template (never the raw URL path); /metrics/frontend whitelists the five web-vitals names and validates values (400 on garbage).
    • services/backend/tests/: session-scoped event loop (pytest-asyncio config in pyproject), single engine dispose at session end, new test_metrics.py covering the guards and labeling.
  • Hard rules touched (by number, see AGENTS.md): none directly; supports rule 11 (tests ship with the change).
  • Assumptions:
    • Raw-path labels are never depended on: verified -- SLO rules and dashboard filter /health and /items which are literal route templates, unchanged. /items/{item_id} becomes newly visible; rules regex widens in the follow-up observability PR.
    • Unmatched paths (404 noise) collapse into one __unmatched__ label value.
  • Plan: label helper + endpoint guard + test-loop fix, single commit.

Checklist

  • make ci green locally (12 backend tests incl. 6 new, lint, mypy, hooks)
  • Tests, docs, and dashboards ship with this change (tests included; no doc/dashboard claims change here -- SLO doc/rule alignment is the observability PR)
  • Mechanical and logic changes are not mixed in this PR

🤖 Generated with Claude Code

…e routes

Two cardinality bombs and a test-infra fix, from the repo review round:

- middleware labeled metrics with the raw URL path: every /items/<id>
  and every scanner-probed 404 minted a new time series (unbounded
  registry growth), and DELETE /items/{item_id} never matched the SLO
  rules' endpoint=~"/health|/items" filter -- deletes were invisible
  to every SLO while the docs claimed coverage. Metrics now label with
  the matched route template; unmatched requests collapse into
  __unmatched__.
- /metrics/frontend set a gauge label straight from request JSON:
  arbitrary names = unbounded series, non-numeric values = 500. Names
  are now whitelisted to the five web-vitals, values validated, bad
  payloads get 400.
- tests: suite moves to a session-scoped event loop (pytest-asyncio
  config) so the module-global engine pool is used on one loop; the
  per-test engine.dispose() existed only to survive per-test loops and
  is replaced by one dispose at session end. New tests cover the
  whitelist, value validation, and route-template labeling (previously
  zero coverage on this path).

SLO rule filters keep matching: /health and /items are literal route
templates; /items/{item_id} becomes visible to the rules for the first
time (rule-side regex update ships in the observability PR).
@vovinacci
vovinacci merged commit 436824c into main Jul 13, 2026
7 checks passed
@vovinacci
vovinacci deleted the fix/metric-label-hygiene branch July 13, 2026 06:45
vovinacci added a commit that referenced this pull request Jul 13, 2026
…oard (#147)

Repo review round, observability findings:

- dashboard burn panel plotted slo:latency:error_budget7d (~0.05 when
  healthy) on a positive-means-violation scale: permanent false red.
  New slo:latency:error_budget_burn7d rule (budget - 0.05 allowance),
  panel repointed; latency target rule added for symmetry.
- slo:error_rate:ratio7d and error_budget7d were byte-identical to the
  availability rules and unused by the dashboard -- dropped, with a
  NOTE in the rules file; burn now derives from actual_5xx_ratio7d.
- endpoint regex widened to include /items/{item_id}: the backend now
  labels by route template (PR #146), making DELETE visible to SLOs
  for the first time.
- Prometheus scrapes loki and alloy own metrics (previously only their
  containers via cadvisor).
- Loki: deprecated table_manager block dropped (compactor handles
  retention for tsdb); reject_old_samples_max_age aligned to the 24h
  retention -- accepting week-old samples that get deleted immediately
  made no sense.
- dashboard: 5xx stat max cap removed (incidents above 1% looked
  pinned at the gauge edge); log panel filters container_name, the
  label Alloy actually sets, instead of the Loki-synthesized
  service_name.
- SLO.md: formulas synced to the shipped rules (error budget is the
  consumed failure fraction, 1 - ratio), metric name corrected from
  http_request_duration_seconds to http_request_latency_seconds --
  copy-pasted queries now return data; grafana.ini comment matched to
  its value.

Validated: promtool check config + rules (14 rules), loki
-verify-config, dashboard JSON parses.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant