amadan
host: discover instantiated units in their per-template cgroup slice
listCarlosUnits read only the top level of /sys/fs/cgroup/system.slice.
systemd puts an instantiated unit that declares no Slice= — which
carlos-app@.service does not — in the implicit per-template slice
system.slice/system-carlos\x2dapp.slice/, so in production the per-unit
table and every owner's app-metrics row would have seen zero tenants
while the fixed platform units read fine.
Scan both levels: the top level (fixed units are genuinely flat) and one
level down inside each system-*.slice child. listCarlosUnits now takes
its root as an argument and returns {name, dir} pairs, since a tenant
unit's directory is no longer <root>/<name>; readUnitRaw reads the
discovered directory. Pinned by a temp-dir fixture covering flat,
nested, and non-carlos decoys at both levels.
Spec and plan carried the flat-layout assumption; both now note the
real layout.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
5 files changed,
+199
−27
docs/superpowers/plans/2026-08-02-host-process-monitoring.md+12 −0docs/superpowers/specs/2026-08-02-host-process-monitoring-design.md+8 −0internal/host/metrics_loop.go+8 −6internal/host/metrics_probes.go+81 −21internal/host/metrics_probes_test.go+90 −0
diff --git a/docs/superpowers/plans/2026-08-02-host-process-monitoring.md b/docs/superpowers/plans/2026-08-02-host-process-monitoring.md| index a68b35ddc..805fb5086 100644 |
| --- a/docs/superpowers/plans/2026-08-02-host-process-monitoring.md |
| +++ b/docs/superpowers/plans/2026-08-02-host-process-monitoring.md |
| @@ -366,6 +366,18 @@ type unitRaw struct { |
| func readUnitRaw(unit string) unitRaw // reads cpu.stat, memory.current, memory.peak, io.stat under cgroupSliceDir/<unit>/ |
| ``` |
| +**Implementation note (post-review, 2026-08-03):** the flat-layout assumption |
| +above is wrong for tenant units and the shipped code differs. systemd places an |
| +instantiated unit that sets no `Slice=` (i.e. `carlos-app@.service`) in the |
| +implicit per-template slice |
| +`system.slice/system-carlos\x2dapp.slice/carlos-app@<host>.service`, so a |
| +top-level-only `ReadDir` sees every fixed platform unit and no tenant. Shipped: |
| +`listCarlosUnits(root string) []unitCgroup` (`{name, dir}` pairs) scans the top |
| +level **and** one level down inside each `system-*.slice` child, and |
| +`readUnitRaw(unitPath string)` reads from the discovered directory rather than |
| +rebuilding `<root>/<unit>`. The walk takes its root as an argument and is |
| +pinned by `internal/host/metrics_probes_test.go` against a temp-dir fixture. |
| + |
| - [ ] **Step 1: Implement** (no TDD cycle — parsers are tested; this is `os.ReadFile` + `filepath.Join` glue; each file read failure yields nil for that field only). |
| - [ ] **Step 2: `go build ./... && go vet ./...`** — clean. |
| - [ ] **Step 3: Commit** `host: cgroup and /proc readers for the metrics sampler` |
diff --git a/docs/superpowers/specs/2026-08-02-host-process-monitoring-design.md b/docs/superpowers/specs/2026-08-02-host-process-monitoring-design.md| index fc9b15049..0f404ca76 100644 |
| --- a/docs/superpowers/specs/2026-08-02-host-process-monitoring-design.md |
| +++ b/docs/superpowers/specs/2026-08-02-host-process-monitoring-design.md |
| @@ -38,6 +38,14 @@ cgroup v2 files, never `ps` scraping: |
| keymail are themselves `carlos-app@` instances) plus the platform's |
| fixed units: `carlos-edge.service` (the agent), `carlos-litestream`, |
| `carlos-adopt`, `carlos-restore-verify`. |
| + **Implementation note:** those two kinds do not sit at the same depth. |
| + The fixed units are directly under `system.slice`, but an instantiated |
| + unit that declares no `Slice=` (which `carlos-app@.service` does not) |
| + is placed by systemd in the implicit per-template slice — |
| + `system.slice/system-carlos\x2dapp.slice/carlos-app@<host>.service`. |
| + Discovery therefore scans the top level of `system.slice` **and** one |
| + level down inside each `system-*.slice` child; a top-level-only scan |
| + finds every platform unit and no tenant at all. |
| - Per unit: CPU fraction (`cpu.stat` usage_usec delta over wall clock), |
| `memory.current`, `memory.peak`, and cumulative read/write bytes from |
| `io.stat`. |
diff --git a/internal/host/metrics_loop.go b/internal/host/metrics_loop.go| index 86e6deb9f..dab133a13 100644 |
| --- a/internal/host/metrics_loop.go |
| +++ b/internal/host/metrics_loop.go |
| @@ -543,15 +543,17 @@ func realSampleProbes() sampleProbes { |
| // realUnitReadings lists the box's currently active CARLOS units and reads |
| // each one's cgroup usage — the production units func sampleProbes.units |
| -// wraps. |
| +// wraps. Keyed by UNIT NAME (what the console matches against), read from the |
| +// unit's own cgroup DIRECTORY (which for a tenant instance is a level down; |
| +// see listCarlosUnits). |
| func realUnitReadings() map[string]unitRaw { |
| - names := listCarlosUnits() |
| - if names == nil { |
| + units := listCarlosUnits(cgroupSliceDir) |
| + if units == nil { |
| return nil |
| } |
| - out := make(map[string]unitRaw, len(names)) |
| - for _, name := range names { |
| - out[name] = readUnitRaw(name) |
| + out := make(map[string]unitRaw, len(units)) |
| + for _, u := range units { |
| + out[u.name] = readUnitRaw(u.dir) |
| } |
| return out |
| } |
diff --git a/internal/host/metrics_probes.go b/internal/host/metrics_probes.go| index eb31b486e..899247438 100644 |
| --- a/internal/host/metrics_probes.go |
| +++ b/internal/host/metrics_probes.go |
| @@ -1,14 +1,19 @@ |
| // The box-side half of the metrics sampler: every function here reads local |
| // kernel/OS state (cgroup v2 and /proc files) and MUST fail toward nil, never |
| // a guessed zero. This file is thin OS glue — os.ReadFile and os.ReadDir |
| -// wrapping the pure parsers in internal/metrics — and is deliberately untested |
| -// by the same convention as probes.go readers; internal/metrics tests exercise |
| -// the parsers with fixtures instead. |
| +// wrapping the pure parsers in internal/metrics — and the readers are |
| +// deliberately untested by the same convention as probes.go readers; |
| +// internal/metrics tests exercise the parsers with fixtures instead. The one |
| +// exception is listCarlosUnits: it is a directory WALK, not a single read, and |
| +// which directories it walks is a decision that has already been wrong once |
| +// (see its doc comment), so it takes its root as an argument and is pinned by |
| +// metrics_probes_test.go against a temp-dir fixture. |
| package host |
| import ( |
| "os" |
| "path/filepath" |
| + "strings" |
| "github.com/carlosframework/platform/internal/metrics" |
| ) |
| @@ -36,31 +41,87 @@ func readLoadAvg() (l1, l5, l15 *float64) { |
| } |
| // cgroupSliceDir is the cgroup v2 directory where CARLOS units report their |
| -// resource usage. |
| +// resource usage. It is the default root for listCarlosUnits, which takes the |
| +// root as an argument only so the discovery walk below can be tested against |
| +// a temp-dir fixture instead of the live kernel. |
| const cgroupSliceDir = "/sys/fs/cgroup/system.slice" |
| -// listCarlosUnits lists all active CARLOS systemd units by reading the cgroup |
| -// slice directory and filtering for names with the "carlos-" prefix and |
| -// ".service" suffix. Read failure -> nil. |
| -func listCarlosUnits() []string { |
| - entries, err := os.ReadDir(cgroupSliceDir) |
| +// unitCgroup is one discovered CARLOS unit: its systemd unit name (the key |
| +// everything downstream — sample maps, the console's activator.UnitName |
| +// match — uses) and the cgroup directory that unit's files actually live in. |
| +// The two are NOT interchangeable: only flat units sit at |
| +// <root>/<name>, and rebuilding the path from the name would miss every |
| +// tenant instance (see listCarlosUnits). |
| +type unitCgroup struct { |
| + name string |
| + dir string |
| +} |
| + |
| +// listCarlosUnits lists all active CARLOS systemd units under root by |
| +// filtering directory names for the "carlos-" prefix and ".service" suffix. |
| +// Read failure -> nil. |
| +// |
| +// Two levels are scanned, because systemd puts CARLOS's two kinds of unit in |
| +// two different places: |
| +// |
| +// - Flat, directly under root: the platform's fixed units |
| +// (carlos-edge.service, carlos-litestream, carlos-adopt, |
| +// carlos-restore-verify) — plain non-templated services land in |
| +// system.slice itself. |
| +// - Nested one level down, inside a "system-*.slice" child: INSTANTIATED |
| +// units. carlos-app@<host>.service sets no Slice= (see |
| +// internal/host/units/carlos-app@.service), so systemd places every |
| +// instance of the template in the implicit per-template slice |
| +// system.slice/system-carlos\x2dapp.slice/. Scanning only the top level |
| +// therefore finds every platform unit and not a single tenant — the exact |
| +// set the per-unit table and the owner app-metrics page exist to show. |
| +// |
| +// Only these two levels are walked: a unit's own cgroup directory has |
| +// children of its own on some systems, and nothing of ours ever nests deeper |
| +// than one slice. |
| +func listCarlosUnits(root string) []unitCgroup { |
| + entries, err := os.ReadDir(root) |
| if err != nil { |
| return nil |
| } |
| - var units []string |
| + var units []unitCgroup |
| for _, entry := range entries { |
| + if !entry.IsDir() { |
| + continue |
| + } |
| name := entry.Name() |
| - // Match directories with prefix "carlos-" and suffix ".service" |
| - if entry.IsDir() && len(name) > len("carlos-.service") { |
| - if name[:7] == "carlos-" && name[len(name)-8:] == ".service" { |
| - units = append(units, name) |
| + if isCarlosUnitName(name) { |
| + units = append(units, unitCgroup{name: name, dir: filepath.Join(root, name)}) |
| + continue |
| + } |
| + if !strings.HasPrefix(name, "system-") || !strings.HasSuffix(name, ".slice") { |
| + continue |
| + } |
| + sliceDir := filepath.Join(root, name) |
| + nested, err := os.ReadDir(sliceDir) |
| + if err != nil { |
| + continue // an unreadable slice loses its units, never the whole scan |
| + } |
| + for _, child := range nested { |
| + if !child.IsDir() || !isCarlosUnitName(child.Name()) { |
| + continue |
| } |
| + units = append(units, unitCgroup{name: child.Name(), dir: filepath.Join(sliceDir, child.Name())}) |
| } |
| } |
| return units |
| } |
| +// isCarlosUnitName reports whether a cgroup directory name is one of ours: |
| +// the "carlos-" prefix and the ".service" suffix, with at least one character |
| +// between them. |
| +func isCarlosUnitName(name string) bool { |
| + return len(name) > len("carlos-.service") && |
| + strings.HasPrefix(name, "carlos-") && |
| + strings.HasSuffix(name, ".service") |
| +} |
| + |
| // unitRaw is a single reading of a systemd unit's resource usage from its |
| // cgroup v2 files. Each field represents a metric that may have failed to |
| // read independently. |
| @@ -72,13 +133,12 @@ type unitRaw struct { |
| writeBytes *int64 |
| } |
| -// readUnitRaw reads a unit's resource usage from its cgroup v2 files under |
| -// cgroupSliceDir/<unit>/. Each file read failure sets only that field to nil; |
| -// the others still read independently. Parse failures also yield nil for that |
| -// field only. |
| -func readUnitRaw(unit string) unitRaw { |
| - unitPath := filepath.Join(cgroupSliceDir, unit) |
| - |
| +// readUnitRaw reads a unit's resource usage from the cgroup v2 files in |
| +// unitPath — the directory listCarlosUnits reported for that unit, which for |
| +// an instantiated unit is NOT <cgroupSliceDir>/<name>. Each file read failure |
| +// sets only that field to nil; the others still read independently. Parse |
| +// failures also yield nil for that field only. |
| +func readUnitRaw(unitPath string) unitRaw { |
| // Read cpu.stat for usage_usec |
| raw := unitRaw{} |
| if b, err := os.ReadFile(filepath.Join(unitPath, "cpu.stat")); err == nil { |
diff --git a/internal/host/metrics_probes_test.go b/internal/host/metrics_probes_test.go| new file mode 100644 |
| index 000000000..8ea66188d |
| --- /dev/null |
| +++ b/internal/host/metrics_probes_test.go |
| @@ -0,0 +1,90 @@ |
| +package host |
| + |
| +import ( |
| + "os" |
| + "path/filepath" |
| + "testing" |
| +) |
| + |
| +// TestListCarlosUnitsFindsFlatAndNestedUnits pins the cgroup layout systemd |
| +// actually produces: the platform's fixed units sit directly under |
| +// system.slice, but an INSTANTIATED unit (carlos-app@<host>.service, and |
| +// internal/host/units/carlos-app@.service sets no Slice=) is placed by |
| +// systemd inside an implicit per-template slice — |
| +// system.slice/system-carlos\x2dapp.slice/carlos-app@<host>.service. Reading |
| +// only the top level would find every platform unit and not one tenant, which |
| +// is exactly the set the per-unit tables and owner metrics exist for. A |
| +// non-CARLOS directory at either level must stay invisible. |
| +func TestListCarlosUnitsFindsFlatAndNestedUnits(t *testing.T) { |
| + root := t.TempDir() |
| + mk := func(parts ...string) string { |
| + t.Helper() |
| + dir := filepath.Join(append([]string{root}, parts...)...) |
| + if err := os.MkdirAll(dir, 0o755); err != nil { |
| + t.Fatal(err) |
| + } |
| + return dir |
| + } |
| + |
| + flatDir := mk("carlos-edge.service") |
| + nestedDir := mk(`system-carlos\x2dapp.slice`, "carlos-app@x.example.com.service") |
| + // Decoys: a non-CARLOS unit at each level, and a nested slice that holds |
| + // nothing of ours. |
| + mk("sshd.service") |
| + mk(`system-carlos\x2dapp.slice`, "sshd.service") |
| + mk("system-getty.slice", "getty@tty1.service") |
| + |
| + got := listCarlosUnits(root) |
| + dirs := map[string]string{} |
| + for _, u := range got { |
| + if _, dup := dirs[u.name]; dup { |
| + t.Fatalf("unit %q discovered twice: %+v", u.name, got) |
| + } |
| + dirs[u.name] = u.dir |
| + } |
| + |
| + if len(got) != 2 { |
| + t.Fatalf("discovered %d units, want 2 (one flat, one nested): %+v", len(got), got) |
| + } |
| + if dirs["carlos-edge.service"] != flatDir { |
| + t.Errorf("carlos-edge.service dir = %q, want %q", dirs["carlos-edge.service"], flatDir) |
| + } |
| + if dirs["carlos-app@x.example.com.service"] != nestedDir { |
| + t.Errorf("carlos-app@x.example.com.service dir = %q, want %q (the per-template slice)", dirs["carlos-app@x.example.com.service"], nestedDir) |
| + } |
| +} |
| + |
| +// TestReadUnitRawReadsFromUnitDir pins that a unit's cgroup files are read |
| +// from the directory listCarlosUnits reported for it — the nested case, since |
| +// a reader that rebuilt the path as <root>/<unit> would find nothing there. |
| +func TestReadUnitRawReadsFromUnitDir(t *testing.T) { |
| + root := t.TempDir() |
| + dir := filepath.Join(root, `system-carlos\x2dapp.slice`, "carlos-app@x.example.com.service") |
| + if err := os.MkdirAll(dir, 0o755); err != nil { |
| + t.Fatal(err) |
| + } |
| + write := func(name, body string) { |
| + t.Helper() |
| + if err := os.WriteFile(filepath.Join(dir, name), []byte(body), 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + write("cpu.stat", "usage_usec 12345\nuser_usec 1\n") |
| + write("memory.current", "4096\n") |
| + // memory.peak and io.stat deliberately absent: each read fails on its own. |
| + |
| + units := listCarlosUnits(root) |
| + if len(units) != 1 { |
| + t.Fatalf("discovered %d units, want 1: %+v", len(units), units) |
| + } |
| + raw := readUnitRaw(units[0].dir) |
| + if raw.cpuUsec == nil || *raw.cpuUsec != 12345 { |
| + t.Errorf("cpuUsec = %v, want 12345", raw.cpuUsec) |
| + } |
| + if raw.memBytes == nil || *raw.memBytes != 4096 { |
| + t.Errorf("memBytes = %v, want 4096", raw.memBytes) |
| + } |
| + if raw.memPeakBytes != nil || raw.readBytes != nil || raw.writeBytes != nil { |
| + t.Errorf("absent files must stay nil, got peak=%v r=%v w=%v", raw.memPeakBytes, raw.readBytes, raw.writeBytes) |
| + } |
| +} |