carlos / platform Public

review pass: svc-convention loop param, phantom-unit-name honesty

- metricsLoop/realSampleProbes take the nilable *activator.Service like
  every sibling activator-fed loop, dropping agent.go's one-off wrapper.
- UnitRow's doc comment and the design doc now own the two edges review
  surfaced: an exec row's unit-name key is identity, not a systemd
  promise (systemctl status finds nothing; the agent journal has the
  logs), and the counter-reset guard can blend one tick on a
  same-window respawn — the same self-correcting edge the cgroup path
  already had.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Paul Campbell 2 months ago 8a361c058fbb0c6c43a457508045ed21c3536d0c parent ae795ae

4 files changed, +39 −22

docs/superpowers/specs/2026-08-08-exec-instance-metrics-design.md +18 −2
internal/console/metricsview.go +9 −3
internal/host/agent.go +1 −8
internal/host/metrics_loop.go +11 −9

diff --git a/docs/superpowers/specs/2026-08-08-exec-instance-metrics-design.md b/docs/superpowers/specs/2026-08-08-exec-instance-metrics-design.md

index f48fd4c55..d52cbbc25 100644

--- a/docs/superpowers/specs/2026-08-08-exec-instance-metrics-design.md

+++ b/docs/superpowers/specs/2026-08-08-exec-instance-metrics-design.md

@@ -78,8 +78,15 @@ the same convention as the other readers):

the /proc tick unit at 100 on every architecture CARLOS runs on,

independent of CONFIG_HZ. Parsed after the last `)` (comm may contain

spaces and parens). Feeds the existing `UnitCPUFrac` delta machinery

- unchanged; a respawned child's counter reset yields a nil fraction

- for that tick (`curUsec < prevUsec` guard, already present).

+ unchanged; a respawned child's counter reset normally yields a nil

+ fraction for that tick (`curUsec < prevUsec` guard, already present).

+ The guard is not absolute: a same-window respawn whose new process

+ has already accumulated more CPU than the dead one's last reading

+ slips past it and blends one tick's fraction across the two

+ processes — the identical, self-correcting edge the cgroup path has

+ when a unit restart recreates its cgroup (prev-tick state is keyed

+ by unit name, not by pid or cgroup inode, on both paths). One

+ transient tick in a 60s-sampled view; accepted.

- `/proc/<pid>/status` → `VmRSS` (memory), `VmHWM` (peak), kB → bytes.

RSS, not `memory.current`: no page-cache attribution, which for "what

your app uses" is the more honest number a single-process reading can

@@ -113,6 +120,15 @@ zeros.

sub-millisecond within a 60s tick, the activator excludes

already-exited procs, and the failure mode is one sample's nils or

one stray reading — accepted.

+- **The operator dashboard shows the unit-name key for a unit systemd

+ does not know.** An exec-backed instance's row reads

+ `carlos-app@<host>.service` — its identity, and what the console

+ matches — but `systemctl status` with that name on the box comes up

+ empty (the process is a `carlos agent` child; its logs are in the

+ agent's own journal). Accepted as the cost of the additive keying;

+ `UnitRow`'s doc comment says so, and if it bites in practice the

+ upgrade is an additive per-unit source marker in `UnitSample`, not a

+ key change.

## Testing

diff --git a/internal/console/metricsview.go b/internal/console/metricsview.go

index 7ee92eb31..4dd763e2a 100644

--- a/internal/console/metricsview.go

+++ b/internal/console/metricsview.go

@@ -24,9 +24,15 @@ import (

"github.com/carlosframework/platform/internal/metrics"

)

-// UnitRow is one systemd unit's latest reading, ready to render, plus its

-// own 48h sparklines (the owner app-metrics view renders a unit on its own,

-// with no host row to borrow a shape from).

+// UnitRow is one per-instance reading, ready to render, plus its own 48h

+// sparklines (the owner app-metrics view renders a unit on its own, with no

+// host row to borrow a shape from). Keyed by unit name, but the name is the

+// reading's IDENTITY, not a promise that systemd knows it: an exec-backed

+// instance has no unit at all — it is a child of `carlos agent`, sampled by

+// pid — and publishes under the carlos-app@<host>.service name it WOULD

+// have (issue #118), so `systemctl status` on a box can come up empty for a

+// row that is live here. The route host inside the name is still the thing

+// to grep the agent's journal for.

type UnitRow struct {

Unit string // full unit name

CPUFrac *float64

diff --git a/internal/host/agent.go b/internal/host/agent.go

index fe0e848c3..7c4404aee 100644

--- a/internal/host/agent.go

+++ b/internal/host/agent.go

@@ -162,14 +162,7 @@ func RunAgent(args []string) {

go statusLoop(reg)

go domainsLoop(reg, peekAcmeCache(args))

go instancesLoop(reg)

- // The metrics loop reads awake exec children from the activator (issue

- // #118: they live in the agent's own cgroup, invisible to the unit walk);

- // with no activator armed there are no exec children to read.

- var execPids func() map[string]int

- if svc != nil {

- execPids = svc.AwakeExecPids

- }

- go metricsLoop(execPids)

+ go metricsLoop(svc)

go configSyncLoop(reg, svc, dd)

go configGenLoop(reg, svc, dd)

go socketAdvisoryLoop(reg, svc)

diff --git a/internal/host/metrics_loop.go b/internal/host/metrics_loop.go

index 4c237e6d7..4d0b3753d 100644

--- a/internal/host/metrics_loop.go

+++ b/internal/host/metrics_loop.go

@@ -20,6 +20,7 @@ import (

"path/filepath"

"time"

+ "github.com/carlosframework/platform/internal/activator"

"github.com/carlosframework/platform/internal/metrics"

"github.com/carlosframework/platform/internal/release"

"github.com/carlosframework/platform/internal/status"

@@ -617,15 +618,16 @@ func httpPostWebhook(url string, body []byte) error {

// realSampleProbes wires sampleProbes to the actual OS/cgroup readers in

// metrics_probes.go and probes.go — the production seam metricsLoop uses,

// never called from a test (host_test.go's convention: tests inject

-// fixtures, never touch /proc or a syscall). execPids is the activator's

-// awake exec-children source (Service.AwakeExecPids); nil — no activator

+// fixtures, never touch /proc or a syscall). svc feeds the units probe the

+// activator's awake exec children (AwakeExecPids); nil — no activator

// armed, e.g. plain `carlos edge` — keeps the units probe cgroup-only,

-// exactly the pre-#118 behavior.

-func realSampleProbes(execPids func() map[string]int) sampleProbes {

+// exactly the pre-#118 behavior, the same nilable-svc convention every

+// other activator-fed loop in this package uses.

+func realSampleProbes(svc *activator.Service) sampleProbes {

units := realUnitReadings

- if execPids != nil {

+ if svc != nil {

units = func() map[string]unitRaw {

- return mergeUnitReadings(realUnitReadings(), execInstanceReadings(execPids(), readExecInstanceRaw))

+ return mergeUnitReadings(realUnitReadings(), execInstanceReadings(svc.AwakeExecPids(), readExecInstanceRaw))

}

return sampleProbes{

@@ -707,10 +709,10 @@ func loadAlertState(path, box string) metrics.AlertState {

// still a valid deployment" shape every other optional loop in this package

// uses. Flush runs every 5th tick (5 minutes at the 60s tick cadence);

// alertTick runs every tick, since a threshold breach deserves faster

-// detection than a metrics flush does. execPids feeds the units probe the

+// detection than a metrics flush does. svc feeds the units probe the

// activator's awake exec children (nil when no activator is armed) — see

// realSampleProbes.

-func metricsLoop(execPids func() map[string]int) {

+func metricsLoop(svc *activator.Service) {

// This loop is always on and shares a process with the agent that serves

// this box's traffic, so a panic in it — an unreadable cgroup shape, a

// nil no one expected — must not take the box down with it. Monitoring

@@ -743,7 +745,7 @@ func metricsLoop(execPids func() map[string]int) {

log.Printf("agent: metrics loop armed (60s ticks, 5min flush), publishing to bucket %s", bucket)

sm := &sampler{}

- probes := realSampleProbes(execPids)

+ probes := realSampleProbes(svc)

tick := 0

for {

time.Sleep(60 * time.Second)