[Peeja]

Written by Claude.

Part of FIL-1503.

Note added after merge. The body says below that this has no effect on staging until infra-nodes#172's relabel is applied by hand on the staging host. It has been: on 2026-10-23 the staging Piris report instance="staging/eu-central-3/piri-0", piri-1 and piri-2, visible in both their Loki stream labels and their trace resource attributes. So this is taking effect there.

Appliance nodes can now run several Piris. Each reports instance as <node>/piri-<n>, according to infra-nodes' docs/decisions/2026-10-multiple-piri-per-appliance.md (fil-forge/infra-nodes#165). Dev and staging each run three: staging since fil-forge/infra-nodes#171 merged on 2026-10-20 (FIL-1503), with the three in their own directories on one shared disk.

Five Piri rules aggregate by (appliance, region, node) alone, so on a node with several Piris a healthy one hides a stuck one, or the alert can't say which Piri it means:

Rule	Before	Effect with three Piris
Piri's chain head is stale	time() - max by (…, node) (head timestamp)	The freshest head wins, so one Piri with a stalled head doesn't fire
Piri is reporting but has no chain head	… unless on (…, node) head series	Any Piri's head series suppresses the rule for the whole node
Piri's Prove task has not run in 1.5 proving periods	max by (…, node) (last Prove success)	One Piri's recent Prove hides another that has stopped proving
Piri CommP claims have stalled (#291)	max by (…, node) (oldest due blob age)	Fires, but as one alert per node that doesn't say which Piri's task engine stalled
Piri Postgres pool is saturated (#300)	sum by (…, node) (waited) / sum by (…, node) (acquires)	Pools every Piri's acquires, so two idle pools can dilute a saturated one below the 10% threshold

Each now groups and matches by instance as well. A Piri that reports its DID as instance, as it does until an image with fil-forge/piri#173 reaches its node, is still one series per Piri. Where Alloy overwrites instance with the node name, a node's Piris merge into one series, as before.

The rules keyed by proof set are unchanged ("proving period has not advanced", "proving is failing"), because each Piri has its own proof set. So is "proof set is unrecoverable": it fires on any rise in the node's count, which a second Piri can't hide.

Alerts only evaluate staging and prod (alert_stages), so staging is where this matters, and staging now runs three Piris. Staging's Piri image already includes fil-forge/piri#173, so its Piris report <node>/piri-<n>, not their DIDs. But staging's Alloy relabel, as infra-nodes' docs/RUNBOOK.md gives it on main, sets instance to staging/eu-central-3 for every series, which would merge the three Piris into one series. fil-forge/infra-nodes#172 keeps Piri's instance there, as dev's config does. It merged on 2026-10-21, but its relabel has to be applied by hand on the staging host; until it is, this change has no effect on staging. Where instance survives, the firing alert's labels carry it, so the alert says which Piri is affected.

Left alone, to avoid conflicting with #249 (merged since), which rewrote the comments and descriptions next to these rules:

• The rules' summary text, which still names only the node.
• The Prove-task rule's "Per node, not per proof set" comment.

One new paragraph in the comment above the Piri proving rules explains the grouping, and the CommP and pool rules each gained a sentence saying why they group by instance. The forge-regions panels the pool alert links to (31, acquires that waited, and 32, mean wait) now chart one line per instance too, so the linked panel shows the ratio the rule fires on. Panel 33 (connections in use) does too, so each Piri's in-use line sits beside its own pool size. Once this merges, a small follow-up can name the Piri in each summary and update that comment.

Not verified: I haven't run these queries against live series. The no-head rule matches target_info, piri_pdp_proofsets_unrecoverable and the head timestamp on instance. That relies on all three carrying the same instance, which they should, since all come from one Piri's resource.

Checks

• make check passes (at 94696a7), including the dashboard normalise check.
• git diff --check is clean.

✴️

https://claude.ai/code/session_01JCF8gyb8jNRmHmWqQmVhRJ

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

[Peeja]

Claude reviewer says… (1st round, 15920ae)

Approve. Nothing in this PR is blocking. One should-fix sits outside it, in infra-nodes, and there are two nits.

Verified

• PromQL: in all three rules every by and on is (appliance, region, node, instance). Both sides of each and, unless and / have the same label sets, so every match is one-to-one.
• Label provenance: on infra-nodes main, dev's config.alloy saves Piri's instance, overwrites it with the node name and restores it. That covers target_info too, so all the matched series carry the same instance per Piri.
• Rollout: old node-name series age out together, so neither the stale-head rule nor the no-head unless fires spuriously. This assumes no staleness markers from the OTLP conversion, which is speculation. Routing groups by alertname, node, stage, severity (routing.tf:35), so three firing Piris on one node still send one notification.
• Downstream: no rule or dashboard reads these rules' output. The Prove rule is still paused.
• Checks: make check exits 0 and git diff --check is clean. A trial merge with Make alert severities match their definitions #249's head merges cleanly.

Findings

1. should-fix (infra-nodes, not this PR): infra-nodes' docs/RUNBOOK.md (staging Alloy relabel, about lines 635–639) sets instance to staging/eu-central-3 for every series. The staging decision doc says staging already runs three Piris. If the host matches the runbook, the three Piris' samples interleave into one series, and this PR can't help staging, the only alerted stage with several Piris. I couldn't check the host itself. Fix: a follow-up in infra-nodes that gives the staging relabel dev's save/restore, plus a check of the host.
2. nit (PR body): "this matters once staging runs three Piris". Staging already does, and the benefit depends on finding 1.
3. nit (alerts.tf:1686–1691): the comment's only fallback is "still reports the node name". The multi-Piri decision doc says a Piri reports its DID as instance until an image with feat(telemetry): read resource attributes from env piri#173 reaches its node. That's still one series per Piri, but worth naming.

Note (speculation): I couldn't confirm whether Piri exports piri_pdp_task_last_success_timestamp_seconds as 0 before a fresh proof set's first Prove. If it does, grouping per instance would surface a new Piri that a per-node max used to hide. That's harmless while the rule is paused, but worth checking before unpausing it.

✴️

---

Generated by Claude Code

[Peeja]

Claude author says…

The fixes are in a2db52c.

1. Staging relabel: confirmed on infra-nodes main. The staging section of docs/RUNBOOK.md sets instance to staging/eu-central-3 unconditionally, with the comment "Piri reports its DID as service.instance.id; name the node". docs/decisions/2026-09-staging-bare-metal-appliance.md says staging runs three Piris. The fix belongs in infra-nodes, not here. I've raised it with Petra, along with a proposed follow-up there. The PR body now says that staging gains nothing from this PR until that follow-up lands.
2. Body: reworded as above. It also covers the DID case.
3. Comment: it now names both other cases. The DID is still one series per Piri. The node name, where Alloy overwrites instance, merges a node's Piris, as before.

The note about Prove-timestamp zeros stays a note: the rule is paused, and checking it belongs with unpausing.

Checks: make check exits 0 at a2db52c, and git diff --check is clean.

The only should-fix is outside this PR, and this PR's own changes were nits. So there won't be another round here. The infra-nodes follow-up will get its own review.

✴️

---

Generated by Claude Code

[Peeja]

Claude author says…

A correction to my round-1 reply. Staging does not yet run three Piris. On infra-nodes main, nodes/staging/eu-central-3/apps/compose.yml defines one piri service. The three Piris are in fil-forge/infra-nodes#171, which is still a draft. The decision doc's "UPDATE: Staging now runs three Piris" line was written ahead of that.

The relabel finding is already handled. fil-forge/infra-nodes#172 gives staging's filone_forge relabel the same save/restore of Piri's instance that dev has, so I'm not opening a separate PR.

I've updated the description to match. There's no code change, so the head stays a2db52c.

✴️

---

Generated by Claude Code

[Peeja]

Claude reviewer says… (round after #171, 14a9e44)

The new comment is accurate and nothing in the code needs to change. The PR description had gone stale, and one operational gap outside this PR still decides whether staging gets any benefit.

Verified

• make check passes and git diff --check d6562e5 14a9e44 is clean. The diff is still only alerts.tf: the comment, plus instance in the three rules' by/on clauses.
• On infra-nodes main (b7ed531, after Bump hilt in dev to sha256:80076f3 #171), the dev and staging composes both set service.instance.id=${FILONE_NODE}/piri-<n> for piri-0 to piri-2. FILONE_NODE is the value Alloy writes to node, so <node>/piri-<n> lines up with the alert's node label.
• Staging's three Piris have their own directories on one disk, so "a node can run several Piris" is right and "one per storage volume" was not.
• Staging's Piri pin (sha256:1e91500…, promoted in Promote dev's images to staging infra-nodes#167 from grafana: note the query grant every rule's data source needs, and rename the Regions groups #166) was published from feat(telemetry): read resource attributes from env piri#173. So staging's Piris already report <node>/piri-<n>, not their DIDs.

Findings

1. should-fix, outside this PR. infra-nodes' docs/RUNBOOK.md on main (about lines 635–639) gives staging an Alloy relabel that sets instance = "staging/eu-central-3" for every series. If the host matches, the three staging Piris still merge into one series, and this change does nothing on staging. docs: describe nodes that run several Piris infra-nodes#172 fixes the relabel; it is still open. The host's Alloy config is managed by hand, so I couldn't check it from the repo.
2. nit, PR description. It still said "one Piri per storage volume", and that staging would run three Piris "once Bump hilt in dev to sha256:80076f3 #171 merges". Bump hilt in dev to sha256:80076f3 #171 merged on 2026-10-20.
3. nit, alerts.tf summaries. The three rules' summaries name only {{ $labels.node }}. The description already defers this to a follow-up after Make alert severities match their definitions #249, so this is settled.

✴️

---

Generated by Claude Code

[Peeja]

Claude author says…

No code change; the head stays at 14a9e44.

1. Staging's relabel. Agreed. It's docs: describe nodes that run several Piris infra-nodes#172's to fix, and it isn't mine. The description now says plainly that this change has no effect on staging until Record the decisions behind the first prod stack #172 is merged and applied on the host. After that, count by (instance) (target_info{job="forge/piri", node="staging/eu-central-3"}) should return three series.
2. Description. Updated: nodes "can run several Piris"; staging runs three since Bump hilt in dev to sha256:80076f3 #171 merged on 2026-10-20; and staging's image already includes feat(telemetry): read resource attributes from env piri#173.
3. Summaries. Left for the planned follow-up after Make alert severities match their definitions #249, as the description says.

Checks: make check and git diff --check at 14a9e44. This round found nothing to change in this PR's code, so there won't be another round.

✴️

---

Generated by Claude Code

[github-actions]

Previews for this pull request — in Dashboard PR previews, deleted when it closes. Read-only, and tagged preview.

• Forge Regions — preview · current

[Peeja]

Claude reviewer says… (3rd round, 22a82ff)

Approve. The two regrouped rules are correct. One should-fix sits next to them, in the dashboard the pool alert links to, and there are two nits.

Verified

• CommP rule: one vector, max by (appliance, region, node, instance), with no matching to get wrong.
• Pool rule: both sides of the ratio are sum by (appliance, region, node, instance), so a plain / matches one to one.
• The three rules regrouped in earlier rounds still include instance in every by and on.
• No other rule using piri_metric_matcher groups by node alone, apart from the settled proof-set rules. The other node-grouped rules watch Ingot, cAdvisor, Caddy hosts or OpenBao.
• The updated PR description is accurate.
• make check and git diff --check pass.
• Not checked: live series. Whether the CommP and pool metrics carry a per-Piri instance on dev or staging isn't verifiable from here.

Findings

1. should-fix: the pool alert's dashboard_url opens panel 31 of forge-regions.json. That panel still computes sum by (node) (waited) / sum by (node) (acquires), the dilution this commit removes from the rule. A saturated Piri beside two idle ones would fire the alert and look under 10% on the linked panel. Panel 32 (mean wait) also groups by node.
2. nit: the pool rule's no-data comment says an idle Piri's 0/0 "produces no series". In PromQL it produces NaN, which never crosses the threshold; that comment came in with Chart Piri's Postgres pool and alert when it saturates #300.
3. nit: the comment above the proving rules says "the three rules … also group by instance". It's accurate where it sits, but it doesn't mention the two rules further down that now follow it.

✴️

---

Generated by Claude Code

[Peeja]

Claude author says…

All three are fixed in 503ee6d.

1. Panels 31 (acquires that waited) and 32 (mean wait) now use sum by (node, instance) on both sides and label each line {{instance}}, so the panel the alert links to shows the same per-Piri ratio as the rule. I left panel 28 (CommP queue) alone: it takes a max, which can't dilute, and the CommP alert links there only for context.
2. The no-data comment now says an idle Piri's 0/0 gives NaN, which never crosses the threshold, and that only a Piri with no pool counters at all gives no series.
3. The comment above the proving rules now ends by saying the CommP and Postgres pool rules group by instance for the same reason.

Checks: make check, scripts/normalise-dashboard.sh --check and git diff --check all pass. The PR description now mentions the panels.

Round 3 had a should-fix, so a round 4 follows.

✴️

---

Generated by Claude Code

[Peeja]

Claude reviewer says… (4th round, 503ee6d)

Approve. 503ee6d fixes round 3's should-fix and both nits. Nothing blocking or should-fix; two optional nits.

Verified

• 503ee6d touches only two comments in alerts.tf and the expr and legendFormat of panels 31 and 32. The dashboard JSON parses and passes the normalise check.
• Panels 31 and 32 group both sides by (node, instance), matching the rule's ratio once the dashboard's own appliance/region filters are taken into account. The pool alert still links to panel 31.
• The panels' descriptions already said "each Piri's" and "per Piri", so they now match their queries.
• {{instance}} already contains the node (<node>/piri-<n>), so it reads well as a legend.
• The NaN comment and the proving-rules cross-reference are accurate.
• All five rules carry instance in every by and on. The PR description matches the diff.
• make check and git diff --check pass. Live Prometheus and Grafana aren't checkable from here.

Findings

1. nit: panel 33 ("connections in use"), in the same pool group, still takes max by (node) of in-use and of pool size separately. The two lines can come from different Piris, so one Piri at its limit beside a bigger pool could be hidden. It's harmless while every Piri has the same pool_max_conns, and the alert doesn't link to it.
2. nit: where a Piri still reports its DID as instance (before feat(telemetry): read resource attributes from env piri#173 reaches its node), the legend of panels 31 and 32 shows a bare DID. The tooltip still shows node, and the case is transitional, so no change is needed.

✴️

---

Generated by Claude Code

[Peeja]

Claude author says…

1. Fixed in 94696a7: panel 33's two queries now use max by (node, instance), with legends {{instance}} in use and {{instance}} max, so each Piri's in-use line sits beside its own pool size, like panels 31 and 32.
2. Left as is, as the review suggests: the DID-as-instance case is transitional, and {{node}} {{instance}} would print the node twice in the normal case.

Checks: make check (including the dashboard normalise check) and git diff --check pass.

This round found only nits, so there won't be another.

✴️

---

Generated by Claude Code