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

review-386-2026-03-26-v2 Review

review needs-refinement

Verdict: NEEDS_REFINEMENT

Second review pass — includes scope correction comment. Original review: review-386-2026-03-26.

Template Completeness

  • [x] Lineage — present ("standalone — discovered during AlertManager triage 2026-03-26")
  • [x] Repo — present in original (wrong), corrected in scope correction comment to pal-e-deployments
  • [ ] User Story — missing
  • [x] Context / What Broke — present, detailed description of 404 on /metrics, 4-day TargetDown alerts
  • [x] File Targets — missing from original, added in scope correction comment
  • [x] Acceptance Criteria — present in original, refined in scope correction (4 criteria)
  • [ ] Test Expectations — missing (no verification commands for agent to run post-fix)
  • [ ] Constraints — missing
  • [ ] Checklist — missing
  • [x] Related — present (project, story, arch labels)

File Targets

  • [x] pal-e-deployments/overlays/pal-e-mail/prod/kustomization.yaml lines 43-53 — verified: ServiceMonitor patch block exists exactly as described (rename base ServiceMonitor from "app" to "pal-e-mail" + selector relabel)
  • [ ] pal-e-deployments/overlays/pal-e-mail/prod/kustomization.yaml line 5 — ISSUE: scope correction says to remove the patch block (lines 43-53) but does NOT mention removing line 5 (- ../../../bases/servicemonitor). If only the patch is removed but the resource reference remains, a ServiceMonitor named "app" with selector "app: app" will still be deployed in the pal-e-mail namespace — orphaned resource. Both line 5 AND lines 43-53 must be removed.
  • [x] pal-e-deployments/bases/servicemonitor/servicemonitor.yaml — verified: base ServiceMonitor scrapes /metrics on port http at 30s interval. Scope correction correctly says DO NOT modify.
  • [x] pal-e-mail codebase — verified: zero files contain "metrics" or "prometheus". Confirms the app has no /metrics endpoint.

Repo Placement

Mismatch identified and partially addressed. The Forgejo issue is filed on forgejo_admin/pal-e-platform, but the fix is entirely in forgejo_admin/pal-e-deployments. The scope correction comment identifies the correct repo but the issue was not moved. The agent will need to create a PR against pal-e-deployments, not pal-e-platform. Cross-repo issue pattern is acceptable per board precedent (items #203, #337, #340, #341 reference pal-e-deployments issues on the pal-e-platform board), but issue body should state PR targets pal-e-deployments explicitly.

Dependencies

No blocking dependencies found. Board item #386 is in the todo column with no depends: or blocked-by: labels. No other board items reference this issue. The fix is self-contained.

Acceptance Criteria

The updated criteria from the scope correction are reasonable but partially untestable by an agent at PR time:
  • [x] "ServiceMonitor reference removed from pal-e-mail kustomization.yaml" — testable: agent can verify file change
  • [ ] "TargetDown alerts clear for pal-e-mail" — post-merge verification only (requires ArgoCD sync + Prometheus scrape cycle)
  • [ ] "ArgoCD syncs successfully after change" — post-merge verification only
  • [x] "Other 3 service ServiceMonitors still functional" — testable: agent can verify no changes to sibling overlays
Missing criterion: "Resource reference to bases/servicemonitor also removed from line 5" — without this, the patch removal is incomplete and leaves an orphaned ServiceMonitor.

Blast Radius

Safe. Verified all three sibling services have working /metrics endpoints:
  • basketball-api — has routes/health.py with metrics, own ServiceMonitor in kustomization
  • mcd-tracker-api — has dedicated metrics.py module
  • pal-e-docs — has metrics in routes/health.py and main.py
Each service includes ../../../bases/servicemonitor independently in its own kustomization.yaml. Removing it from pal-e-mail has zero effect on siblings.

Recommendation

Three items to address before this ticket is READY:
  • Add line 5 to file targets. Scope correction must include removing - ../../../bases/servicemonitor from the resources block (line 5), not just the patch block (lines 43-53). Without this, an orphaned ServiceMonitor deploys into the namespace.
  • Add test expectations. Agent needs: kustomize build overlays/pal-e-mail/prod/ should produce no ServiceMonitor resource. Post-merge: kubectl get servicemonitor -n pal-e-mail should return empty.
  • Add explicit cross-repo note. Issue body or comment should state PR targets forgejo_admin/pal-e-deployments so the agent creates the PR on the correct repo.