pal-e-docs

Scope Review: board-landscaping-observability#1308 -- Tempo + OpenTelemetry

review-1308-2026-06-04 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Forgejo Issue: ldraney/landscaping-assistant #85 -- Deploy Grafana Tempo + OpenTelemetry for distributed tracing
Board Item: #1308 (8 points, backlog, labels: type:feature, arch:platform, arch:rails-app, arch:k8s-deploy, story:observability, parent:43, devops)
Reviewed: 2026-06-04 by agent-qa

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- Child of #43 (Observability & DORA metrics stack), Phase 1 of observability roadmap
  • [x] Repo -- Three repos listed (but owner is stale, see Recommendation #2)
  • [x] User Story -- Clear "As a platform operator / I want distributed tracing / So that I can trace requests end-to-end"
  • [x] Context -- Thorough. Explains zero tracing today, MinIO backend, correlation goals, relationship to observability roadmap
  • [ ] File Targets -- Six targets listed across three repos, but Terraform paths follow wrong pattern (see Recommendation #1)
  • [x] Acceptance Criteria -- Seven criteria, all testable and specific
  • [x] Test Expectations -- Three items (infrastructure-level only; see Recommendation #4)
  • [x] Constraints -- Four constraints, all reasonable and actionable
  • [x] Checklist -- Per-repo PR checkboxes
  • [ ] Related -- References project-pal-e-platform but ticket lives on board-landscaping-observability (see Recommendation #5)

Traceability

  • [x] story:observability -- present on board item
  • [x] arch:platform, arch:rails-app, arch:k8s-deploy -- present on board item
  • [x] Forgejo issue #85 -- exists and is open
  • [x] parent:43 -- label present, parent issue exists and is open

File Targets

  • [ ] terraform/tempo.tf (pal-e-platform) -- WRONG PATH. pal-e-platform uses modular Terraform structure under terraform/modules/. The monitoring module lives at terraform/modules/monitoring/ with its own main.tf/variables.tf/outputs.tf/versions.tf. There are NO top-level .tf files for individual Helm releases. An agent following this target literally would create orphaned files outside the module structure.
  • [ ] terraform/otel-collector.tf (pal-e-platform) -- Same issue. Should be inside a module.
  • [ ] terraform/grafana.tf (pal-e-platform) -- Same issue. Grafana config modifications should go in the existing monitoring module.
  • [x] Gemfile (landscaping-assistant) -- Verified exists. Currently has yabeda-rails, yabeda-prometheus, yabeda-puma-plugin, lograge. Adding opentelemetry-sdk and opentelemetry-instrumentation-all here is correct.
  • [x] config/initializers/opentelemetry.rb (landscaping-assistant) -- New file. Parent directory exists. Existing config/initializers/yabeda.rb is a separate file, no conflict.
  • [?] overlays/production/patches/env.yaml (pal-e-deployments) -- Cannot verify; repo is private. Path pattern is consistent with Kustomize conventions.
Assessment: Three of six file targets point to the wrong location in the Terraform repo. An agent cannot execute reliably without corrected paths.

Repo Placement

This is a multi-repo ticket spanning three repositories:
  • pal-e-platform (Terraform) -- Helm releases for Tempo and OTel Collector, Grafana datasource config
  • landscaping-assistant (Rails) -- Gem additions, OTel initializer
  • pal-e-deployments (Kustomize) -- OTLP endpoint env var
The issue is filed on landscaping-assistant, which is reasonable as the primary user-facing repo, with the checklist tracking PRs across all three repos. This pattern works -- the issue serves as a tracking issue with per-repo PRs.
Repo owner mismatch: Issue references forgejo_admin/pal-e-platform and forgejo_admin/pal-e-deployments. The canonical owner is ldraney (forgejo_admin 301-redirects to ldraney). Agent API calls using the old owner could fail depending on Forgejo API behavior with redirects.

Dependencies

  • [x] MinIO -- satisfied. Already deployed per project-pal-e-platform status and observability roadmap.
  • [x] Grafana -- satisfied. 26 dashboards deployed, monitoring module exists.
  • [x] Loki -- satisfied. 7d retention, Promtail DaemonSet running. Needed for trace-log correlation.
  • [x] Lograge -- satisfied. Already in Gemfile. Needed for TraceID injection into structured logs.
  • [x] yabeda metrics -- satisfied. Constraint says "must not break existing yabeda Prometheus metrics" -- coexistence is possible since OTel and yabeda operate on different telemetry signals (traces vs metrics).
  • [ ] Tempo Helm chart repo -- ATTENTION. The Grafana Tempo chart migrated from grafana/helm-charts to grafana-community/helm-charts as of January 2026. OCI path is now oci://ghcr.io/grafana-community/helm-charts/tempo. Issue does not mention this. An agent using the old repo would pull a stale chart.

Acceptance Criteria

All seven AC are testable and specific:
  • "Tempo is deployed and healthy in the monitoring namespace" -- verifiable via kubectl
  • "OTel Collector DaemonSet is running and receiving OTLP" -- verifiable via kubectl + logs
  • "Rails requests produce traces visible in Grafana Explore -> Tempo" -- verifiable via Grafana UI
  • "ActiveRecord, ActionPack, Net::HTTP, Puma spans auto-instrumented" -- verifiable via trace detail in Grafana. The opentelemetry-instrumentation-all gem (v0.94.0) bundles all four of these instrumentations.
  • "TraceID appears in structured log output (Lograge)" -- verifiable via log inspection
  • "Clicking a TraceID in Loki opens the trace in Tempo" -- verifiable via Grafana Explore. Requires derived fields config in Loki datasource.
  • "Grafana NodeGraph panel shows service dependency map from trace data" -- verifiable via Grafana. Note: with only one service instrumented initially, the dependency map will be minimal.
Assessment: AC are well-crafted. No "works correctly" ambiguity.

Blast Radius

  • Files touched: ~6 files across 3 repos
  • Services affected: Monitoring namespace (new pods), landscaping-assistant (new gems + initializer), Grafana (new datasource)
  • What could break: (a) Rails boot failure if OTel initializer has errors. (b) Yabeda /metrics endpoint regression if gems conflict. (c) Monitoring namespace resource pressure from new pods (Tempo + OTel Collector DaemonSet).
  • Rollback: Straightforward. Revert Helm releases via Terraform, remove gems from Gemfile, remove initializer. Independent rollback per repo.

Decomposition Assessment

This ticket has 3 discrete changes across 3 repos:
  • Platform infra (Tempo + OTel Collector Helm releases + Grafana datasource) -- pal-e-platform
  • App instrumentation (gems + initializer) -- landscaping-assistant
  • Env var injection (OTLP endpoint) -- pal-e-deployments
These are sequentially dependent (infra must exist before app can send traces, env var must exist before app can find the collector). The three-thing limit is met exactly. An agent could execute this as one ticket with ordered PRs: platform first, then deployments, then app.
At 8 points this is on the large side but not decomposition-worthy -- the changes are tightly coupled and individually small.

Recommendation

  • (MUST FIX) Correct Terraform file targets. Replace terraform/tempo.tf, terraform/otel-collector.tf, and terraform/grafana.tf with paths that match the modular structure. Options: (a) Add resources to terraform/modules/monitoring/main.tf, or (b) Create a new terraform/modules/tracing/ module with its own main.tf/variables.tf/outputs.tf/versions.tf. Explicitly state which approach to use.
  • (MUST FIX) Update repo owner from forgejo_admin to ldraney. The canonical owner is ldraney. API calls and PR submissions should target ldraney/pal-e-platform and ldraney/pal-e-deployments.
  • (MUST FIX) Add Tempo Helm chart migration note. Add a constraint: "Use the Tempo chart from grafana-community/helm-charts (OCI: oci://ghcr.io/grafana-community/helm-charts/tempo), not the deprecated grafana/helm-charts repo."
  • (SHOULD FIX) Add application-level test expectation. Add at minimum: "Rails boots cleanly with OTel gems (RAILS_ENV=test bundle exec rails runner 'puts OpenTelemetry.tracer_provider')" to catch initializer errors before deployment.
  • (SHOULD FIX) Add landscaping-observability to Related section. This ticket lives on board-landscaping-observability. The Related section should reference the project it belongs to.
  • (SHOULD FIX) Add "do not touch" file targets. List config/initializers/yabeda.rb, config/routes.rb, and config/puma.rb as files agents should NOT modify, reinforcing the "must not break yabeda" constraint.