pal-e-docs

Scope Review: board-landscaping-observability#1307 (Forgejo #17)

review-1307-2026-06-04 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Third scope review of this ticket. Two prior reviews (2026-06-01, 2026-06-02) flagged wrong repo and wrong metric names. Neither issue has been fixed. The issue body is unchanged since original filing. This ticket cannot be dispatched to an agent in its current state.

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- Standalone, observability gap audit
  • [ ] Repo -- WRONG. Says ldraney/pal-e-deployments. Should be ldraney/pal-e-platform. All existing PrometheusRules are Terraform kubernetes_manifest resources in terraform/modules/monitoring/main.tf, not Kustomize overlays. Verified: blackbox_alerts, embedding_alerts, payment_pipeline_alerts, gmail_oauth_expiry_alert all live there.
  • [x] User Story -- Clear operator perspective, well-formed As/I want/So that
  • [x] Context -- Good background, mentions ServiceMonitor dependency
  • [ ] File Targets -- WRONG. Points to overlays/landscaping-assistant/prod/prometheusrule.yaml and overlays/landscaping-assistant/prod/kustomization.yaml in pal-e-deployments. Should point to terraform/modules/monitoring/main.tf in pal-e-platform (append new kubernetes_manifest resource blocks).
  • [ ] Acceptance Criteria -- INCOMPLETE. References "5xx error rate" and "p95 latency" without specifying actual metric names or PromQL. Agent will guess wrong. See Acceptance Criteria section below for details.
  • [ ] Test Expectations -- WRONG. Says kustomize build overlays/landscaping-assistant/prod/. Should say tofu plan against the monitoring module.
  • [x] Constraints -- Good: for: 5m, API version monitoring.coreos.com/v1, tuning caveat
  • [x] Checklist -- Present
  • [x] Related -- References project-landscaping-assistant

Traceability

  • [x] story:observability -- present on board item labels
  • [x] arch:k8s-deploy -- present on board item labels
  • [x] Forgejo issue -- exists and is open (landscaping-assistant#17)
  • [ ] Board item title -- says "(needs refinement)" which is accurate, but the board item URL still points to landscaping-assistant/issues/17 despite the work targeting pal-e-platform

File Targets

  • [ ] overlays/landscaping-assistant/prod/prometheusrule.yaml -- This file does not exist and should not be created here. The platform convention is to define PrometheusRules as Terraform kubernetes_manifest resources in pal-e-platform.
  • [x] overlays/landscaping-assistant/prod/kustomization.yaml -- This file exists in pal-e-deployments, but should NOT be modified for this ticket. It is the wrong location for alerts.
  • [ ] Missing correct target: terraform/modules/monitoring/main.tf in ldraney/pal-e-platform -- this is where the new kubernetes_manifest resource blocks should be appended, following the pattern of existing alert resources.

Repo Placement

The issue is filed on ldraney/landscaping-assistant (Rails app) but the work targets infrastructure code. Two options:
  • Preferred: Re-create the issue on ldraney/pal-e-platform where the Terraform code lives. Update the board item URL. Close this issue as "moved."
  • Acceptable: Keep the issue here but correct the ### Repo field to ldraney/pal-e-platform and update file targets. The implementing agent branch would be created on pal-e-platform.
Currently the issue, the repo field, and the file targets all disagree about where the work lives. This is the fundamental blocker.

Dependencies

  • [x] ServiceMonitor -- SATISFIED. Base template bases/standard/servicemonitor.yaml exists. Prod kustomization.yaml patches it for landscaping-assistant namespace and label selectors.
  • [x] /metrics endpoint -- SATISFIED. Gemfile includes yabeda-rails, yabeda-prometheus, yabeda-puma-plugin. Initializer at config/initializers/yabeda.rb confirms auto-configuration.
  • [x] Prometheus scraping -- SATISFIED. ServiceMonitor scrapes /metrics on port http every 30s.

Acceptance Criteria

The AC says "high 5xx error rate (>5% over 5m), p95 latency spike (>2s over 5m), and zero available pods" but does not specify the actual PromQL. The app uses yabeda-rails which exposes:
  • Counter: rails_requests_total with labels: controller, action, status, format, method
  • Histogram: rails_request_duration (seconds) -- Prometheus exposes as rails_request_duration_bucket, rails_request_duration_sum, rails_request_duration_count
Common wrong guesses an agent would make: http_requests_total (generic Prometheus convention), http_request_duration_seconds_bucket (Go/generic convention). Neither metric exists on this app.
The AC should include explicit PromQL or at minimum the metric names. Suggested expressions:
  • 5xx rate: sum(rate(rails_requests_total{namespace="landscaping-assistant", status=~"5.."}[5m])) / sum(rate(rails_requests_total{namespace="landscaping-assistant"}[5m])) > 0.05
  • p95 latency: histogram_quantile(0.95, sum(rate(rails_request_duration_bucket{namespace="landscaping-assistant"}[5m])) by (le)) > 2
  • Zero pods: kube_deployment_status_replicas_available{namespace="landscaping-assistant", deployment="landscaping-assistant"} == 0
Note: the status label in yabeda-rails is a string (e.g., "200", "500"), so regex matching status=~"5.." is correct.

Blast Radius

  • One file modified: terraform/modules/monitoring/main.tf (append-only, 3 new resource blocks)
  • Low risk: adding PrometheusRule resources is additive. Does not modify existing alerts or application code.
  • Rollback: tofu apply with the resource blocks removed, or kubectl delete prometheusrule.
  • Failure mode: wrong PromQL means alerts never fire (silent) or always fire (noisy). Neither breaks the application.

Decomposition Assessment

  • 3 discrete changes: one resource block per alert (error rate, latency, availability). Exactly at the three-thing limit.
  • All three belong in the same file and same PrometheusRule resource (or separate resources in the same file). No need to split.
  • Estimated agent time: well under 5 minutes once the ticket is correctly scoped.
  • No parallelization needed.

Recommendation

  • Fix the Repo field: Change from ldraney/pal-e-deployments to ldraney/pal-e-platform.
  • Fix the File Targets: Replace kustomize overlay paths with terraform/modules/monitoring/main.tf. Note this is an append to an existing file following the pattern of blackbox_alerts, embedding_alerts, etc.
  • Add metric names to AC: Specify that alerts must use rails_requests_total (counter, labels: controller/action/status/format/method) and rails_request_duration_bucket (histogram). Include the suggested PromQL expressions or at minimum the metric names.
  • Fix Test Expectations: Replace kustomize build with tofu plan (or tofu validate) against the monitoring module.
  • Clarify Pyrra relationship: The observability roadmap (docs/observability-roadmap.md) places PrometheusRules under Pyrra in Phase 6, where Pyrra would auto-generate multi-window burn rate alerting from SLO CRDs. Clarify whether these are interim manual rules that Pyrra will eventually replace, or intended to coexist.