[mashkovd]

Adopts Tempo as the permanent trace backend: the #1280 verdict (ADR 0001, ADOPT TEMPO / mctl-native only), deployed as a separate permanent deployment, not the bake-off sandbox promoted in place. Plan: docs/plans/tempo-adoption.md. Runbook: docs/runbooks/tempo.md.

Refs #1280. The teardown (#2073) merged first (Option A).

Owner approval (2026-10-22)

#2072 approved as written. Choose Option A, teardown first. I explicitly accept the emptyDir WAL standing risk. The partial-read anomaly does not block deployment, but it blocks any claim or use that assumes complete span enumeration until explained.

Known limitation: partial reads

In the soak a bounded trace-by-id over all 139 traces returned 5,092 of 6,930 in-window spans, while ingest counters agree to the span. It is unexplained. Until it is: no promise of complete trace/span enumeration, and Tempo is not used for completeness-critical counts (count ingest with collector / distributor counters instead). Recorded in the runbook §1, the values comment and the datasource template.

What changes

• tracing namespace + Application tracing-tempo (bootstrap/templates/observability/tracing.yaml): chart tempo 2.4.0, Tempo 2.10.8, reuses R2 bucket tempo-traces (ExternalSecret tempo-r2 from Vault platform/r2-tempo), retention 336h, 1 replica, no PVC (WAL on a 2Gi emptyDir: accepted standing risk), limits 2 CPU / 2Gi, GOMEMLIMIT=1600MiB.
• Read-path bounds after the soak's unranged read pinned the pod: query_frontend.api_timeout: 1m (default is none), max_outstanding_per_tenant: 100, trace_by_id.concurrent_shards: 4, querier.max_concurrent_queries: 4. Keys checked against Tempo v2.10.8 source.
• Deadlock avoidance (gitops#1925): root-app renders only the cluster-scoped Namespace and the Application in argocd. Every namespaced object in tracing (default-deny + allow NetworkPolicies, ResourceQuota refusing PVCs, LimitRange, ExternalSecret, VMServiceScrape) lives in the Application's manifestsPath infra-components/observability/tracing/tempo/ with negative sync waves. tracing and the chart repo are derived into AppProject default.
• Collector: otlp/backend → tempo.tracing.svc.cluster.local:4317, with a new opt-in otelCollector.backendInsecure (Tempo's receiver is plaintext in-cluster).
• Grafana: datasource "Tempo", uid tempo, same tracesToLogsV2; provisioning deleteDatasources removes the leftover "Tempo (eval)" (the API refuses read-only datasources, so this is the §10 cleanup of chore(observability): tear down the #1280 Tempo eval sandbox #2073). Loki's trace_id link now points at tempo.
• Alerts: TempoEval* → Tempo* (vm-rules/tempo-alerts.yaml, namespace tracing, exporter otlp(_grpc)?/backend), Telegram route Tempo.*. promtool tests pass.
• Producers: only the three DevLoop workers and admins-mctl-agent get otel.enabled: true.
• Docs: new docs/runbooks/tempo.md; bake-off runbook marked closed and §9 corrected (datasource removal via provisioning); plan status → approved.

Tests

• tests/test_otel_collector_backends_render.py: eval-machinery checks now render with the permanent backend off (golden unchanged); new "Committed default" section asserts the shipped render: otlp/backend endpoint + TLS, nothing else added to the collector, Namespace/Application/project, no namespaced root-app resource in tracing, manifests (default-deny, PVC-refusing quota, negative waves), chart pin and values, datasource identity + deleteDatasources, Loki link. Mutation-checked (wrong port, wrong delete name → FAIL).
• validate-argocd-project-scope.py --selftest --strict default (both renders), validate-eval-teardown-date.py, kubeconform, test_adr_rubric_frozen, test_otel_collector_alert_windows, test_base_service_otel_env: pass locally. Tempo chart rendered with these values.

After merge

Live verification: tracing-tempo Synced/Healthy, tempo-0 Running, tempo-r2 Ready, collector config has otlp/backend, producers carry OTEL_*, spans arrive, Grafana lists "Tempo" and no "Tempo (eval)". If root-app does deadlock anyway, selective sync of the project needs owner OK.

🤖 Generated with Claude Code

[claude]

Claude finished @mashkovd's task in 4m 57s —— View job

---

Review of PR #2072 — Tempo permanent trace backend

• Gather context (diff, CLAUDE.md, repo structure)
• Review ArgoCD Application / bootstrap wiring (tracing.yaml, project-default.yaml, values.yaml)
• Review NetworkPolicy / ResourceQuota / LimitRange / ExternalSecret manifests
• Review collector / datasource / alert changes
• Review tests
• Post inline findings and submit verdict

Note: Bash tool calls in this run required interactive approval that wasn't available, so I could not execute helm template, helm lint, the Python test suite, or kubeconform to verify renders directly. The review below is a thorough static read of the diff (git diff d4add9025...feb91c5c0) against every changed file; if you'd like the render/test suite actually executed, allow helm/python3 in --allowedTools for this job.

Summary: this is a large, carefully-scoped change, and it reads as correct. Highlights verified:

• Deadlock avoidance (gitops#1925): tracing.yaml root-app renders only the cluster-scoped Namespace + Application; every namespaced object (default-deny + scoped NetworkPolicies, PVC-refusing ResourceQuota, LimitRange, ExternalSecret, VMServiceScrape) lives in the Application's manifestsPath with negative sync-waves (-2/-1), ahead of the StatefulSet (wave 0). Matches the pattern already used in eval-namespace.yaml/eval-candidates.yaml.
• Resource math checks out: ResourceQuota limits.cpu: 4 / limits.memory: 4Gi exactly covers two Tempo pods at the chart's 2 CPU / 2Gi limits (room for one rolling replacement, as the comment states); requests.cpu: 1 / requests.memory: 2Gi covers two pods at 200m/768Mi requests. persistentvolumeclaims: "0" + requests.storage: "0" correctly refuses PVCs for the accepted emptyDir-WAL risk.
• NetworkPolicy layering: namespace-wide default-deny + intra-namespace + cluster-egress (DNS, pod/service CIDRs) + ingress from monitoring on 4317/4318 only; Tempo-specific policy scopes HTTP (3200, for Grafana/vmagent) and R2 egress (443, Cloudflare IPv4 ranges only, pod-scoped not {}) to the Tempo pod specifically — never widens the namespace-wide {} ingress/egress deny.
• Secrets: tempo-r2 ExternalSecret pulls from secret/data/platform/r2-tempo via the existing vault-backend ClusterSecretStore; no plaintext secret material in the repo, consistent with CLAUDE.md's Vault+ExternalSecrets convention.
• AppProject: tracing namespace and the chart's repoURL are correctly appended to project-default.yaml's derived lists, gated on .Values.tracing.enabled, deduped via uniq.
• Collector/datasource/alerts/Loki link are all mutually consistent: otlp/backend → tempo.tracing.svc.cluster.local:4317 (+ tls.insecure only when backendInsecure is set) matches the Grafana datasource URL and the VMServiceScrape's job="tempo" matches vm-rules/tempo-alerts.yaml. TempoEval* → Tempo* rename is applied consistently across the alert file, the eval NetworkPolicy/servicescrape comments, and the Telegram route regex.
• Known risks (WAL on emptyDir, partial-read anomaly) are both pre-existing, explicitly owner-accepted (quoted in the PR description), and thoroughly documented in docs/runbooks/tempo.md §1/§3, the values comment, and the datasource template comment — not new gaps introduced silently.
• Tests: test_otel_collector_backends_render.py's new "Committed default" section asserts the shipped render end-to-end (collector exporter config, Namespace/Application shape, project scope, manifestsPath contents incl. sync-wave ordering, Tempo values for retention/resources/read-path bounds, datasource identity + deleteDatasources, Loki derived-field target) and mutation-checks a few of those (wrong port/delete-name → FAIL). Good coverage for a config-only change.

No P1 or P2 findings. No inline comments posted.

gh api repos/mctlhq/mctl-gitops/pulls/2072/reviews --method POST -f commit_id="feb91c5c0380961b294a7e5a6f6ff42cda6e8099" -f event="APPROVE" -f body="No P1/P2 findings (0 P3). Thorough manifest/test coverage; known risks (WAL emptyDir, partial-read anomaly) are pre-existing and owner-accepted. Static review only — could not execute helm/python3 test suite in this sandbox."

[claude]

No P1/P2 findings (0 P3). Thorough manifest/test coverage; known risks (WAL emptyDir, partial-read anomaly) are pre-existing and owner-accepted. Static review only - could not execute helm/python3 test suite in this sandbox.

[github-actions]

Antigravity (agy) review — informational

Model: gemini-3.8-flash-high · commit feb91c5.

No significant issues found.

[claude]

Claude finished @mashkovd's task in 1m 56s —— View job

---

Review of PR #2072 — commit 456526b9

• Gather context (diff since last review, pinned commits)
• Review new commit 456526b95 ("reserve the tracing namespace and lint-ignore its template")
• Confirm nothing else changed vs. the already-reviewed feb91c5c0
• Post findings / verdict

Scope of this run: git diff d4add9025...456526b95 is the full PR diff, but git diff feb91c5c0...456526b95 isolates what's actually new since the prior Claude review (which already approved the full feature) — exactly 4 files, 10/-7 lines:

• platform-gitops/argo-workflows/cluster-templates/wft-create-tenant.yaml and wft-delete-tenant-safe.yaml: both add tracing to the exact-match arm of tenant_name_reserved(), in both occurrences in each file (create has 2, delete-safe has 2). Confirmed the literal tracing matches the namespace tracing.yaml actually creates (platform-gitops/bootstrap/templates/observability/tracing.yaml:53), and all four occurrences are consistent.
• .github/workflows/yamllint.yml: adds platform-gitops/bootstrap/templates/observability/tracing.yaml to the lint-ignore list (bare-template-directive files) and updates the comment to explain why. Verified tracing.yaml does lead with {{- /* ... */}} followed by {{- if .Values.tracing.enabled }}, matching the stated rationale.
• tests/test_wft_create_tenant_existing.py: adds "tracing" to MUST_REJECT, covering the new reserved name.

No P1/P2/P3 findings. This is a small, mechanical, internally-consistent follow-up fix — reserving the new permanent-Tempo namespace against tenant creation and keeping yamllint/tests in sync. The rest of the diff (collector, datasource, alerts, manifests, NetworkPolicies, ResourceQuota, tests) is unchanged from the version already reviewed and approved in the prior run.

Verdict: Approved.
gh api repos/mctlhq/mctl-gitops/pulls/2072/reviews → APPROVE bound to commit 456526b95e94a7752be75bb80f2eb135a7fe4da1.

[claude]

No P1/P2/P3 findings. This sync's delta vs. feb91c5 (already reviewed/approved) is a small, consistent fix: adds tracing to the reserved-tenant-name exact-match group in both wft-create-tenant.yaml and wft-delete-tenant-safe.yaml (both occurrences, matching the namespace tracing.yaml creates), adds tracing.yaml to the yamllint template-comment ignore list with an accurate rationale (verified the file does lead with {{- /* ... */}}), and extends test_wft_create_tenant_existing.py's MUST_REJECT list to cover it. Verified consistent across all four touched files; rest of the diff is unchanged from the prior full review.

[github-actions]

Antigravity (agy) review — informational

Model: gemini-3.8-flash-high · commit 456526b.

No significant issues found.