Homelab/infra-core

Fork 2

HOMELAB-1850: fix(tempo): span-metrics histogram buckets + phase dimension — OPERATOR REVIEW (protected ns) #595

Merged

aaron merged 1 commit from plane/HOMELAB-1850-tempo into epic/LD4

2026-10-03 17:14:32 +00:00

aaron commented

2026-10-03 03:41:28 +00:00

Owner

⚠ OPERATOR REVIEW REQUIRED — do not auto-merge. monitoring is a protected
namespace (AGENTS.md), so this change is class E-adjacent. It is deliberately its own PR,
separate from HOMELAB-1850's routines/ leg (Homelab/infra-core#594), so the class boundary
is visible on the board.

What

core/charts/platform/tempo/values.yaml:

HOMELAB-1593 — histogram_buckets. The premise measured worse than the ticket
stated: Tempo's default ladder tops out at 16.384 s while subagent runs have a measured
mean of ~1640 s, so p99 was not coarse but constant (16.38 s, one saturated
bucket) — stall-outlier detection could not work because the stall signature IS the top
bucket. The ladder here keeps the sub-second regime (tool:* spans run 2 ms–64 ms) and
extends to 1 h for agent spans. The ladder is global to the metrics generator, not
per span name, so dropping the small buckets would coarsen every tool:* span and
both Pi dashboards that chart it.
HOMELAB-1850 — phase in span_metrics.dimensions, so the call/duration side of
the Costs pane can be split by stage the way the token side already is. phase is a
closed enum (build|review|verify|unphased, ≤ 4 values), so its cardinality cost is
bounded by the existing dimension set.

Not taken here, deliberately: the sandbox_env dimension (LD4.4's key). It multiplies
with subagent.type × llm.model and the LD4.5d brief makes it a measured decision
(§8 R4) — enable it once the series count after this change is known.

Evidence

helm template grafana/tempo --version 1.24.2 -f core/charts/platform/tempo/values.yaml
renders both keys at metrics_generator.processor.span_metrics.{dimensions,histogram_buckets}
— the chart passes processor through verbatim (chart 1.24.2 values.yaml lines 53–63),
and that path is the Tempo 2.9 span-metrics schema, so the key names are the chart's own
rather than a guess. Rendered output checked locally (21 buckets, phase last in
dimensions).
Rollout: infra-core epic/LD4 → live → prod apps/tempo.yaml ($values reference,
targetRevision: live) → ArgoCD sync. The metrics-generator restarts with the change;
the new ladder starts fresh (a step, not a backfill) and the saturated default-ladder
series age out of Prometheus' 10.03 d retention.

Reviewer questions worth answering explicitly

Is the 21-bucket ladder acceptable for the metrics-generator's memory/CPU profile
(buckets × dimensions is the cost), or should the 1 h tail stop at 1800 s?
Should the sandbox_env dimension ride along while the operator is already reviewing
this diff, or stay a separate measured decision?
Is a global ladder the right call, or should this wait for Tempo's per-processor
bucketing if 2.9 has it?

**⚠ OPERATOR REVIEW REQUIRED — do not auto-merge.** `monitoring` is a protected namespace (AGENTS.md), so this change is class E-adjacent. It is deliberately its own PR, separate from HOMELAB-1850's routines/ leg (Homelab/infra-core#594), so the class boundary is visible on the board. ## What `core/charts/platform/tempo/values.yaml`: 1. **HOMELAB-1593 — `histogram_buckets`.** The premise measured worse than the ticket stated: Tempo's default ladder tops out at 16.384 s while subagent runs have a measured **mean of ~1640 s**, so p99 was not coarse but *constant* (16.38 s, one saturated bucket) — stall-outlier detection could not work because the stall signature IS the top bucket. The ladder here keeps the sub-second regime (`tool:*` spans run 2 ms–64 ms) and extends to 1 h for agent spans. The ladder is **global to the metrics generator, not per span name**, so dropping the small buckets would coarsen every `tool:*` span and both Pi dashboards that chart it. 2. **HOMELAB-1850 — `phase` in `span_metrics.dimensions`**, so the call/duration side of the Costs pane can be split by stage the way the token side already is. `phase` is a closed enum (`build|review|verify|unphased`, ≤ 4 values), so its cardinality cost is bounded by the existing dimension set. **Not taken here, deliberately:** the `sandbox_env` dimension (LD4.4's key). It multiplies with `subagent.type × llm.model` and the LD4.5d brief makes it a *measured* decision (§8 R4) — enable it once the series count after this change is known. ## Evidence * `helm template grafana/tempo --version 1.24.2 -f core/charts/platform/tempo/values.yaml` renders both keys at `metrics_generator.processor.span_metrics.{dimensions,histogram_buckets}` — the chart passes `processor` through verbatim (chart 1.24.2 `values.yaml` lines 53–63), and that path is the Tempo 2.9 span-metrics schema, so the key names are the chart's own rather than a guess. Rendered output checked locally (21 buckets, `phase` last in `dimensions`). * Rollout: infra-core `epic/LD4` → `live` → prod `apps/tempo.yaml` (`$values` reference, `targetRevision: live`) → ArgoCD sync. The metrics-generator restarts with the change; the new ladder starts fresh (a step, **not** a backfill) and the saturated default-ladder series age out of Prometheus' 10.03 d retention. ## Reviewer questions worth answering explicitly 1. Is the 21-bucket ladder acceptable for the metrics-generator's memory/CPU profile (buckets × dimensions is the cost), or should the 1 h tail stop at 1800 s? 2. Should the `sandbox_env` dimension ride along while the operator is already reviewing this diff, or stay a separate measured decision? 3. Is a global ladder the right call, or should this wait for Tempo's per-processor bucketing if 2.9 has it?

aaron added 1 commit

2026-10-03 03:41:29 +00:00

HOMELAB-1850: fix(tempo): span-metrics histogram buckets + the phase dimension (OPERATOR REVIEW)

0/0 projects applied successfully.

CI Review / helm-validate (pull_request) Successful in 13s

Details

CI Review / receiver-test (pull_request) Successful in 10s

Details

CI Review / alert-receiver-test (pull_request) Successful in 24s

Details

CI Review / dispatcher-test (pull_request) Successful in 4m10s

Details

ebdb5266bf

CLASS: E-adj / DANGEROUS. `monitoring` is a protected namespace (AGENTS.md), so
this commit must not auto-merge and is deliberately kept SEPARATE from the
routines/ and pi-extensions/ legs of HOMELAB-1850 so the class boundary is
visible on the board. Operator review required.

1. HOMELAB-1593 — histogram_buckets. The premise measured worse than stated:
   Tempo's default ladder tops out at 16.384s while subagent runs have a measured
   MEAN of ~1640s, so p99 was not coarse but CONSTANT (16.38s, one saturated
   bucket) and stall-outlier detection could not work, because the stall
   signature IS the top bucket. The ladder added here keeps the sub-second regime
   (tool spans run 2ms-64ms) and extends to 1h for agent spans — the ladder is
   GLOBAL to the metrics generator, not per span name, so dropping the small
   buckets would coarsen every `tool:*` span and both Pi dashboards.

2. HOMELAB-1850 — `phase` in `span_metrics.dimensions`, so the call/duration side
   of the Costs pane can be split by stage the way the token side already is.
   `phase` is a closed enum (build|review|verify|unphased; <= 4 values), so its
   cardinality cost is bounded by the existing dimension set.

Not taken here, deliberately: the `sandbox_env` dimension (LD4.4's key). It
multiplies with subagent.type x llm.model and the brief makes it a MEASURED
decision (R4) — enable it once the series count after this change is known.

Verified: `helm template grafana/tempo --version 1.24.2 -f <values>` renders both
keys at `metrics_generator.processor.span_metrics.{dimensions,histogram_buckets}`
(the chart passes `processor` through verbatim — chart 1.24.2 values lines 53-63),
i.e. the Tempo 2.9 span-metrics schema, so the keys are the chart's own and not a
guess.

Rollout: infra-core `epic/LD4` → `live` → prod `apps/tempo.yaml` ($values
reference, targetRevision: live) → ArgoCD sync. The metrics-generator restarts
with the change and the new ladder starts fresh (a step, not a backfill); the
default-ladder series age out of Prometheus' 10.03d retention.

aaron commented

2026-10-03 03:52:37 +00:00

Author

Owner

VERDICT: APPROVED WITH OPERATOR REVIEW REQUIRED (protected monitoring ns)

Fresh-context review (landing worker, HOMELAB-1850). Reviewed at head ebdb5266bf6edff772f854f4d12be78588851013; CI run #1777 green (helm-validate, dispatcher-test, receiver-test, alert-receiver-test). No blocker findings. Two factual corrections and one non-blocking concern below. Did not merge: monitoring is a protected namespace.

1. Diff scope — PASS (exactly the two promised changes)

One file, core/charts/platform/tempo/values.yaml, +22/-0, of which only two lines are effective:

- phase appended to metricsGenerator.processor.span_metrics.dimensions
histogram_buckets: [0.001 … 3600] (21 buckets)

Everything else is comment. Untouched and verified untouched: retention: 72h, queryFrontend.search.query_ingesters_until, queryFrontend.trace_by_id.query_shards, overrides.defaults.metrics_generator.processors, storage, resources, receivers, probes, persistence. No retention / query / ingestion change smuggled in.

Verified by render, not by reading: helm template grafana/tempo --version 1.24.2 -f <pr values> emits both keys at metrics_generator.processor.span_metrics.{dimensions,histogram_buckets}, 21 buckets, phase last. The chart passes processor verbatim (1.24.2 values.yaml processor: {}).

2. "Prod pulls Tempo values from infra-core live, no prod PR needed" — CLAIM TRUE, with a path nuance

Confirmed from prod apps/tempo.yaml:

- repoURL: https://grafana.github.io/helm-charts
  chart: tempo
  targetRevision: "1.24.2"           # chart pinned
  helm:
    valueFiles: ["$values/core/charts/platform/tempo/values.yaml"]
- repoURL: https://forgejo.aaron.reynoza.org/Homelab/infra-core.git
  targetRevision: live                # <-- the values source
  ref: values

So prod consumes infra-core live; no prod PR is needed — TRUE.

Nuance the PR body does not state: this PR's base is epic/LD4, not live. Merging #595 changes nothing in the cluster. The production effect arrives only via PR #591 (live <- epic/LD4). That is also precisely why there is nothing agent-executable here (§8).

3. Effect statement — premise CONFIRMED quantitatively; two corrections

Measured against the live Prometheus (24 h, last_over_time — note instant queries look back only ~5 min and show nothing while agents idle, which is a trap here):

Check	Measured	Verdict
`histogram_quantile(0.99, …latency_bucket{subagent_type=~".+"})`	16.38 s	saturated at the 16.384 s top bucket — p99 is constant, exactly as claimed
`le` boundaries present	15 (14 buckets + `+Inf`)	matches Tempo 2.9 default ladder
subagent mean, `general-purpose`	839.6 s	≫ 16.384 s
subagent mean, `verifier` / `scout-codebase` / `scout-web`	216.3 / 264.2 / 121.9 s	≫ 16.384 s

HOMELAB-1593's premise is real and reproduced: the stall signature is the top bucket. Default ladder confirmed independently in Tempo v2.9.0 source — docs/sources/tempo/configuration/_index.md line 467: histogram_buckets: <list of float> | default = 0.002 … 16.384.

Correction a (minor). "a measured MEAN of ~1640 s" is not reproducible today: 24 h overall subagent mean ≈ 635 s, and no per-subagent-type mean is near 1640 s (max 839.6 s). The order of magnitude holds and the conclusion is unaffected, but the figure should be re-derived or attributed to the window/mission it came from.

Correction b (minor). "Prometheus' 10.03 d retention" — the live Prometheus reports prometheus_tsdb_retention_limit_seconds = 1 296 000 (15 d) and …_limit_bytes = 30 GiB. Series do age out; at 15 d, not 10.03 d.

The phase half is inert until the emitter ships. No span carries a phase attribute yet (Homelab/pi-extensions#60, open), and pi_agent_tokens_total today has no phase label (count(pi_agent_tokens_total) by (phase) → one empty bucket, 68 series) — so the PR body's "the way the token side already is" is loose: the token side gets phase from the same sibling PR. Per Tempo 2.9 docs a dimension "is added to the metrics if present", so a dimension declared with no matching attribute is a no-op. The observable change from this PR alone is the histogram ladder; the call/duration phase split requires #60 deployed.

4. Reviewer Q1 — 21-bucket ladder acceptable; low risk, measured

No cap to trip: tempo_metrics_generator_registry_max_active_series = 0 (unset/∞), …_series_limited_total = 0, registry peak 798 active series in 24 h.
Headroom: tempo container working set 557 MiB against a 2 GiB limit and 512 MiB request → ~1.4 GiB.
Cost is bounded, not multiplied globally: buckets are processor-global (Config.HistogramBuckets []float64 — single list, confirmed in v2.9.0 spanmetrics/config.go), so the ladder is a +50 % (21/14) on all latency_bucket series: 1725 → ~2590. The phase multiplier applies only to spans carrying the attribute — subagent-labelled bucket series are 150 of 1725, so worst case ≈ +450 series, not 4×.
Keep 3600 as the top bucket. Measured general-purpose mean already exceeds 1800 s; stopping at 1800 s would re-introduce the saturation this PR exists to remove. (This answers reviewer Q3 too: global is the only option in 2.9 — there is no per-span-name bucketing in the config schema; the per-processor route would need a second span-metrics processor with a filter policy, and the current ladder keeps 0.001–1 s so tool:* spans are still covered.)

5. CONCERN (non-blocking) — the tool-latency grid does change

"The ladder keeps the sub-second regime" is directionally right but not exact. Tempo's default has boundaries at 2/4/8/16/32/64 ms; the new ladder has 1/2/5/10/25/50/100 ms. Tool spans (2–64 ms) get a different grid, not the same one — expect small baseline shifts in tool-latency panels. Checked both Pi dashboards that chart spanmetrics: pi-tools.yaml uses only latency_count / latency_sum / calls_total (bucket-insensitive) and pi-subagents.yaml uses histogram_quantile with no hardcoded le, so nothing breaks. Recorded so the claim is not read as "unchanged".

6. `sandbox_env` deferral — ENDORSED

It multiplies with subagent.type × llm.model, LD4.5d brief §8 R4 makes it a measured decision, and LD4.4 owns the key. Keeping it out of a protected-ns PR is correct. Do not let it ride along.

7. Rollback story — sound, but not instant

Revert the two keys → ArgoCD sync → tempo-0 restarts on Tempo's defaults (top 16.384 s, no phase dimension). No trace data is touched (Tempo retention: 72h local backend is not in the diff); no migration, no backfill — the PR's own "a step, not a backfill" is accurate. Prometheus: the old-ladder and phase-bearing series identities go stale and age out over 15 d.

Friction worth naming: the only rollback path is the same three-tier flow (revert on epic/LD4 → #591 → live), and ArgoCD has selfHeal: true, so hand-editing the live ConfigMap is reverted — there is no in-cluster hot-patch escape hatch. Also: because the ladder creates a new series grid, capture the pre-change p99 before rolling back or the before/after comparison is lost.

8. Nothing agent-executable — PASS

No auto-merge workflow in .forgejo/workflows. release.yaml fires only on PRs merged to main — not epic/LD4. No image is built from a values change. Agents never kubectl into clusters. Merging #595 into epic/LD4 has zero cluster effect; there is no automated rollout to watch.

One honesty note: helm-validate is a YAML-syntax-only yaml.safe_load — green CI does not validate Tempo's key schema. The real validation is the chart render (§1) plus Tempo v2.9.0 source (HistogramBuckets []float64 \yaml:"histogram_buckets"``). Residual risk: Tempo fails to start on an unknown config key, so a wrong key's failure mode is a tempo-0 rollout failure, not a silent no-op — which is the reason this belongs in operator review.

## VERDICT: APPROVED WITH OPERATOR REVIEW REQUIRED (protected monitoring ns) Fresh-context review (landing worker, HOMELAB-1850). Reviewed at head `ebdb5266bf6edff772f854f4d12be78588851013`; CI run #1777 green (helm-validate, dispatcher-test, receiver-test, alert-receiver-test). **No blocker findings.** Two factual corrections and one non-blocking concern below. Did **not** merge: `monitoring` is a protected namespace. --- ### 1. Diff scope — PASS (exactly the two promised changes) One file, `core/charts/platform/tempo/values.yaml`, +22/-0, of which only two lines are effective: - `- phase` appended to `metricsGenerator.processor.span_metrics.dimensions` - `histogram_buckets: [0.001 … 3600]` (21 buckets) Everything else is comment. Untouched and verified untouched: `retention: 72h`, `queryFrontend.search.query_ingesters_until`, `queryFrontend.trace_by_id.query_shards`, `overrides.defaults.metrics_generator.processors`, `storage`, `resources`, receivers, probes, persistence. **No retention / query / ingestion change smuggled in.** Verified by render, not by reading: `helm template grafana/tempo --version 1.24.2 -f <pr values>` emits both keys at `metrics_generator.processor.span_metrics.{dimensions,histogram_buckets}`, 21 buckets, `phase` last. The chart passes `processor` verbatim (1.24.2 `values.yaml` `processor: {}`). ### 2. "Prod pulls Tempo values from infra-core live, no prod PR needed" — CLAIM TRUE, with a path nuance Confirmed from `prod` `apps/tempo.yaml`: ```yaml - repoURL: https://grafana.github.io/helm-charts chart: tempo targetRevision: "1.24.2" # chart pinned helm: valueFiles: ["$values/core/charts/platform/tempo/values.yaml"] - repoURL: https://forgejo.aaron.reynoza.org/Homelab/infra-core.git targetRevision: live # <-- the values source ref: values ``` So prod consumes infra-core `live`; **no prod PR is needed** — TRUE. **Nuance the PR body does not state:** this PR's base is `epic/LD4`, not `live`. Merging #595 changes **nothing** in the cluster. The production effect arrives only via PR **#591** (`live <- epic/LD4`). That is also precisely *why* there is nothing agent-executable here (§8). ### 3. Effect statement — premise CONFIRMED quantitatively; two corrections Measured against the live Prometheus (24 h, `last_over_time` — note instant queries look back only ~5 min and show nothing while agents idle, which is a trap here): | Check | Measured | Verdict | |---|---|---| | `histogram_quantile(0.99, …latency_bucket{subagent_type=~".+"})` | **16.38 s** | saturated at the 16.384 s top bucket — p99 is *constant*, exactly as claimed | | `le` boundaries present | **15** (14 buckets + `+Inf`) | matches Tempo 2.9 default ladder | | subagent mean, `general-purpose` | 839.6 s | ≫ 16.384 s | | subagent mean, `verifier` / `scout-codebase` / `scout-web` | 216.3 / 264.2 / 121.9 s | ≫ 16.384 s | HOMELAB-1593's premise is **real and reproduced**: the stall signature *is* the top bucket. Default ladder confirmed independently in Tempo **v2.9.0** source — `docs/sources/tempo/configuration/_index.md` line 467: `histogram_buckets: <list of float> | default = 0.002 … 16.384`. **Correction a (minor).** "a measured **MEAN of ~1640 s**" is not reproducible today: 24 h overall subagent mean ≈ **635 s**, and no per-subagent-type mean is near 1640 s (max 839.6 s). The order of magnitude holds and the conclusion is unaffected, but the figure should be re-derived or attributed to the window/mission it came from. **Correction b (minor).** "Prometheus' **10.03 d** retention" — the live Prometheus reports `prometheus_tsdb_retention_limit_seconds` = 1 296 000 (**15 d**) and `…_limit_bytes` = 30 GiB. Series do age out; at 15 d, not 10.03 d. **The `phase` half is inert until the emitter ships.** No span carries a `phase` attribute yet (`Homelab/pi-extensions#60`, open), and `pi_agent_tokens_total` today has **no `phase` label** (`count(pi_agent_tokens_total) by (phase)` → one empty bucket, 68 series) — so the PR body's "the way the token side already is" is loose: the token side gets `phase` from the same sibling PR. Per Tempo 2.9 docs a dimension *"is added to the metrics if present"*, so a dimension declared with no matching attribute is a no-op. **The observable change from this PR alone is the histogram ladder**; the call/duration phase split requires #60 deployed. ### 4. Reviewer Q1 — 21-bucket ladder acceptable; low risk, measured - No cap to trip: `tempo_metrics_generator_registry_max_active_series` = 0 (unset/∞), `…_series_limited_total` = 0, registry peak **798** active series in 24 h. - Headroom: tempo container working set **557 MiB** against a 2 GiB limit and 512 MiB request → ~1.4 GiB. - Cost is bounded, not multiplied globally: buckets are processor-global (`Config.HistogramBuckets []float64` — single list, confirmed in v2.9.0 `spanmetrics/config.go`), so the ladder is a **+50 % (21/14)** on all `latency_bucket` series: **1725 → ~2590**. The `phase` multiplier applies **only to spans carrying the attribute** — subagent-labelled bucket series are **150 of 1725**, so worst case ≈ +450 series, not 4×. - **Keep 3600 as the top bucket.** Measured `general-purpose` mean already exceeds 1800 s; stopping at 1800 s would re-introduce the saturation this PR exists to remove. (This answers reviewer Q3 too: global is the only option in 2.9 — there is no per-span-name bucketing in the config schema; the per-processor route would need a second span-metrics processor with a filter policy, and the current ladder keeps 0.001–1 s so `tool:*` spans are still covered.) ### 5. CONCERN (non-blocking) — the tool-latency grid *does* change "The ladder keeps the sub-second regime" is directionally right but not exact. Tempo's default has boundaries at **2/4/8/16/32/64 ms**; the new ladder has **1/2/5/10/25/50/100 ms**. Tool spans (2–64 ms) get a *different* grid, not the same one — expect small baseline shifts in tool-latency panels. Checked both Pi dashboards that chart spanmetrics: `pi-tools.yaml` uses only `latency_count` / `latency_sum` / `calls_total` (bucket-insensitive) and `pi-subagents.yaml` uses `histogram_quantile` with **no hardcoded `le`**, so nothing breaks. Recorded so the claim is not read as "unchanged". ### 6. `sandbox_env` deferral — ENDORSED It multiplies with `subagent.type × llm.model`, LD4.5d brief §8 R4 makes it a measured decision, and LD4.4 owns the key. Keeping it out of a protected-ns PR is correct. Do not let it ride along. ### 7. Rollback story — sound, but not instant Revert the two keys → ArgoCD sync → `tempo-0` restarts on Tempo's defaults (top 16.384 s, no `phase` dimension). No trace data is touched (Tempo `retention: 72h` local backend is not in the diff); no migration, no backfill — the PR's own "a step, not a backfill" is accurate. Prometheus: the old-ladder and phase-bearing series identities go stale and age out over 15 d. **Friction worth naming:** the only rollback path is the same three-tier flow (`revert on epic/LD4 → #591 → live`), and ArgoCD has `selfHeal: true`, so hand-editing the live ConfigMap is *reverted* — there is no in-cluster hot-patch escape hatch. Also: because the ladder creates a new series grid, **capture the pre-change p99 before rolling back** or the before/after comparison is lost. ### 8. Nothing agent-executable — PASS No auto-merge workflow in `.forgejo/workflows`. `release.yaml` fires only on PRs merged to **`main`** — not `epic/LD4`. No image is built from a values change. Agents never kubectl into clusters. **Merging #595 into `epic/LD4` has zero cluster effect; there is no automated rollout to watch.** One honesty note: `helm-validate` is a YAML-syntax-only `yaml.safe_load` — **green CI does not validate Tempo's key schema**. The real validation is the chart render (§1) plus Tempo v2.9.0 source (`HistogramBuckets []float64 \`yaml:"histogram_buckets"\``). Residual risk: Tempo fails to start on an unknown config key, so a wrong key's failure mode is a **tempo-0 rollout failure, not a silent no-op** — which is the reason this belongs in operator review.

aaron commented

2026-10-03 03:53:28 +00:00

Author

Owner

RIDER — correction to §3 (ordering fact), verdict UNCHANGED

Re-verified after posting; my §3 said pi-extensions#60 was "open". That was stale and is wrong.

Homelab/pi-extensions#60 is MERGED — merged_at 2026-10-03T03:49:21Z, merge commit 79671ee, head 59518ae.
But it is merged and NOT deployed. Verified on disk: the live checkout ~/.pi/agent/extensions/pi-otel-telemetry sits at 93a0b5b (pre-#60) and phase.ts is absent. The sibling landing worker for #60 independently reported the same, blocked on 24 pre-existing uncommitted modifications in that checkout.

So phase is still inert at runtime today — for a different reason than I stated: the emitter is merged-but-not-deployed, not unwritten. The conclusion is unchanged and now sharper:

The phase dimension becomes observable only after the live extension checkout is advanced to pi-extensions main @ ≥ 79671ee. That is a separate operator action, not part of this PR and not part of #591.
Replace operator-checklist item 5 with: "the call/duration phase split needs the live ~/.pi/agent/extensions checkout advanced to main @ ≥79671ee; currently blocked on 24 local uncommitted edits in that checkout."
pi_agent_tokens_total having no phase label today remains accurate (still 93a0b5b).

Everything else in the review is unaffected and stands: the diff scope, the prod $values verification, the reproduced p99 = 16.38 s / measured means, the cardinality measurement, the tool-latency grid concern, the rollback story, sandbox_env deferral, and "nothing agent-executable".

VERDICT UNCHANGED: APPROVED WITH OPERATOR REVIEW REQUIRED (protected monitoring ns). No blocker findings.

## RIDER — correction to §3 (ordering fact), verdict UNCHANGED Re-verified after posting; my §3 said pi-extensions#60 was "open". **That was stale and is wrong.** - `Homelab/pi-extensions#60` is **MERGED** — `merged_at` 2026-10-03T03:49:21Z, merge commit **`79671ee`**, head `59518ae`. - **But it is merged and NOT deployed.** Verified on disk: the live checkout `~/.pi/agent/extensions/pi-otel-telemetry` sits at **`93a0b5b`** (pre-#60) and **`phase.ts` is absent**. The sibling landing worker for #60 independently reported the same, blocked on 24 pre-existing uncommitted modifications in that checkout. **So `phase` is still inert at runtime today — for a different reason than I stated:** the emitter is merged-but-not-deployed, not unwritten. The conclusion is unchanged and now sharper: - The `phase` dimension becomes observable only after the live extension checkout is advanced to `pi-extensions` `main` @ ≥ `79671ee`. That is a **separate operator action**, not part of this PR and not part of #591. - Replace operator-checklist item 5 with: *"the call/duration `phase` split needs the live `~/.pi/agent/extensions` checkout advanced to `main` @ ≥`79671ee`; currently blocked on 24 local uncommitted edits in that checkout."* - `pi_agent_tokens_total` having no `phase` label today remains accurate (still `93a0b5b`). Everything else in the review is unaffected and stands: the diff scope, the prod `$values` verification, the reproduced p99 = 16.38 s / measured means, the cardinality measurement, the tool-latency grid concern, the rollback story, sandbox_env deferral, and "nothing agent-executable". **VERDICT UNCHANGED: APPROVED WITH OPERATOR REVIEW REQUIRED (protected monitoring ns).** No blocker findings.

aaron referenced this pull request

2026-10-03 03:54:26 +00:00

LD4 (HOMELAB-1713): The Loops — intake, El Director, routines, readiness, plan-freshness, meetings, alert audit → live #591

aaron commented

2026-10-03 17:14:16 +00:00

Author

Owner

Orchestrator rulings on the review questions (operator-delegated merge authority, 2026-10-03): (1) the 21-bucket ladder ships — cardinality is bounded (buckets × the existing dimension set), and the tail can be trimmed later against measured series counts; (2) sandbox_env stays a separate measured decision exactly as the PR disciplines itself; (3) global ladder is correct for Tempo 2.9 (no per-processor bucketing at this pin). Merging so the change rides #591.

Orchestrator rulings on the review questions (operator-delegated merge authority, 2026-10-03): (1) the 21-bucket ladder ships — cardinality is bounded (buckets × the existing dimension set), and the tail can be trimmed later against measured series counts; (2) `sandbox_env` stays a separate measured decision exactly as the PR disciplines itself; (3) global ladder is correct for Tempo 2.9 (no per-processor bucketing at this pin). Merging so the change rides #591.

aaron merged commit 74029341e8 into epic/LD4

2026-10-03 17:14:32 +00:00

aaron referenced this pull request from a commit

2026-10-03 17:14:32 +00:00

Merge pull request 'HOMELAB-1850: fix(tempo): span-metrics histogram buckets + phase dimension — OPERATOR REVIEW (protected ns)' (#595) from plane/HOMELAB-1850-tempo into epic/LD4

No reviewers

No labels

operator-review

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Homelab/infra-core!595

No description provided.

Rows
Columns

HOMELAB-1850: fix(tempo): span-metrics histogram buckets + phase dimension — OPERATOR REVIEW (protected ns) #595

What

Evidence

Reviewer questions worth answering explicitly

VERDICT: APPROVED WITH OPERATOR REVIEW REQUIRED (protected monitoring ns)

1. Diff scope — PASS (exactly the two promised changes)

2. "Prod pulls Tempo values from infra-core live, no prod PR needed" — CLAIM TRUE, with a path nuance

3. Effect statement — premise CONFIRMED quantitatively; two corrections

4. Reviewer Q1 — 21-bucket ladder acceptable; low risk, measured

5. CONCERN (non-blocking) — the tool-latency grid does change

6. sandbox_env deferral — ENDORSED

7. Rollback story — sound, but not instant

8. Nothing agent-executable — PASS

RIDER — correction to §3 (ordering fact), verdict UNCHANGED

6. `sandbox_env` deferral — ENDORSED