pal-e-docs

Review: Bug: pal-e-mail ServiceMonitor scraping nonexistent /metrics

review-386-2026-03-26 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Lineage — present (standalone, discovered during AlertManager triage)
  • [x] Repo — present but INCORRECT (see Repo Placement below)
  • [ ] User Story — missing. No "As a... I want... So that..." block.
  • [x] Context — present (documented as "What Broke" with clear repro steps)
  • [ ] File Targets — missing. No specific file paths listed for the agent to modify.
  • [x] Acceptance Criteria — present (3 criteria)
  • [ ] Test Expectations — missing. No test commands or verification steps.
  • [ ] Constraints — missing.
  • [ ] Checklist — missing.
  • [x] Related — present (project, story, arch labels)

File Targets

  • [x] /home/ldraney/pal-e-mail/src/pal_e_mail/main.py — verified: only routes are /healthz, /send/*, /log/*. NO /metrics endpoint. NO prometheus dependency.
  • [x] /home/ldraney/pal-e-deployments/bases/servicemonitor/servicemonitor.yaml — verified: base ServiceMonitor scrapes /metrics on port http with 30s interval.
  • [x] /home/ldraney/pal-e-deployments/overlays/pal-e-mail/prod/kustomization.yaml — verified: includes bases/servicemonitor resource and renames it to pal-e-mail. This is the actual source of the bug.
  • [ ] terraform/main.tf in pal-e-platform — ISSUE: ticket claims ServiceMonitor config lives here. Grep confirms no pal-e-mail ServiceMonitor in this file. The only ServiceMonitors in pal-e-platform/terraform are for dora-exporter and embedding-worker.

Repo Placement

MISMATCH. The ticket says the fix is in forgejo_admin/pal-e-platform and/or forgejo_admin/pal-e-mail. The ServiceMonitor is actually deployed via forgejo_admin/pal-e-deployments (kustomize overlay at overlays/pal-e-mail/prod/kustomization.yaml). The Forgejo issue is filed on pal-e-platform but the fix belongs in pal-e-deployments.
If the chosen fix is option (b) — add a real /metrics endpoint — then forgejo_admin/pal-e-mail is also correct as a secondary repo. But option (a) — remove the ServiceMonitor — is purely a pal-e-deployments change.

Dependencies

No blocking dependencies found on the board. Item #386 is in todo column. No other board items reference pal-e-mail ServiceMonitor. The fix is self-contained regardless of which option is chosen.

Acceptance Criteria

Criteria are testable but incomplete:
  • "TargetDown alerts clear for pal-e-mail" — verifiable via kubectl or AlertManager API, but no specific command provided.
  • "Either ServiceMonitor removed OR /metrics returns 200" — verifiable, but the ticket should recommend ONE approach. Both are valid but have different scope.
  • "No regression on pal-e-mail health" — vague. Should specify: curl /healthz returns 200, existing email send/log endpoints unaffected.
  • Missing: no test expectations section. If option (b), need pytest command for the new /metrics endpoint.

Blast Radius

Three other services use the identical bases/servicemonitor kustomize base: basketball-api, mcd-tracker, pal-e-docs. All three were verified to have working /metrics endpoints:
  • basketball-api — has /metrics route in routes/health.py
  • mcd-tracker-api — has prometheus_fastapi_instrumentator in metrics.py + main.py
  • pal-e-docs — has prometheus_fastapi_instrumentator in main.py
No blast radius for sibling services. The bug is isolated to pal-e-mail being the only service that includes the ServiceMonitor base without implementing the /metrics endpoint.

Recommendation

Three issues must be fixed before this ticket is READY:
  • Fix Repo field: Change from forgejo_admin/pal-e-platform to forgejo_admin/pal-e-deployments (and optionally forgejo_admin/pal-e-mail if option (b) is chosen).
  • Add File Targets: For option (a): overlays/pal-e-mail/prod/kustomization.yaml — remove bases/servicemonitor resource and the ServiceMonitor rename patch. For option (b): src/pal_e_mail/main.py — add prometheus_fastapi_instrumentator, plus pyproject.toml for the new dependency.
  • Pick one approach: The ticket offers two options but should recommend one. Option (a) is lower risk and faster. Option (b) adds observability value but requires app code changes + dependency addition + tests.