amadan

carlos / platform Public

host: metrics sweep samples registry-stamped bespoke units

The 60s sweep discovered units by NAME CONVENTION only (carlos-*.service
plus the delegated exec cgroups), so every route stamped with a bespoke
unit — Route.Unit, the #110 column: an operator's console.service, tito's
titogo-canary@<tenant>.service — reported no readings at all, and the
console had no way to tell "not running" from "never sampled".

listStampedUnits walks the same two systemd levels as listCarlosUnits (a
flat unit under root, a template instance inside its implicit
system-*.slice) against the wanted set, because a stamped NAME says
nothing about where systemd put its cgroup. A stamped unit with no cgroup
dir yields nothing: absence is the honest report for a unit that isn't
running, never a guessed zero.

The stamped set reaches the sweep as a func, not a registry handle:
realSampleProbes is built ONCE and its units probe read every tick, so a
snapshot would pin the set to agent-start and a route stamped this
afternoon would go unsampled until the next restart. stampedUnitNames
does the one List per tick; a read failure costs that tick's stamped
units (logged), never the sweep or the agent. A nil func means none —
darwin, dev runs, and every existing caller.

Wire shape is additive: stamped readings are extra entries in the
existing metrics.Sample.Units map, keyed by the REAL unit name, and the
map write dedupes a stamped unit that also matches the carlos-
convention. internal/metrics is untouched.

Gate: build, vet, test ./... (23 pkgs) and GOOS=darwin build all clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell 69f73fadba8d2c987e88c1267dd1e8200082f80f parent 8ebc685
5 files changed, +227 −11
  • internal/host/agent.go +37 −1
  • internal/host/agent_test.go +45 −0
  • internal/host/metrics_loop.go +35 −9
  • internal/host/metrics_probes.go +60 −1
  • internal/host/metrics_probes_test.go +50 −0
diff --git a/internal/host/agent.go b/internal/host/agent.go
index 11d17dc30..231e0a1e7 100644
--- a/internal/host/agent.go
+++ b/internal/host/agent.go
@@ -116,6 +116,7 @@ import (
"os/exec"
"path"
"path/filepath"
+ "sort"
"strings"
"sync"
"time"
@@ -274,7 +275,9 @@ func RunAgent(args []string) {
go domainsLoop(reg, peekAcmeCache(args))
go instancesLoop(reg)
go retentionLoop(reg)
- go metricsLoop(cgm.Root())
+ // The sweep reads the registry only through this closure — the stamped
+ // bespoke units to sample beside the carlos- ones, re-read each tick.
+ go metricsLoop(cgm.Root(), func() []string { return stampedUnitNames(reg) })
go configSyncLoop(reg, svc, dd)
go configGenLoop(reg, svc, dd)
go socketAdvisoryLoop(reg, svc)
@@ -1481,6 +1484,39 @@ func retentionLoop(reg *registry.Registry) {
}
}
+// stampedUnitNames is the metrics sweep's one registry read: every distinct
+// bespoke unit stamped on a route (Route.Unit, the #110 column — an
+// operator's console.service, tito's titogo-canary@<tenant>.service), which
+// the box's cgroup walk cannot discover by name convention.
+//
+// Called once per 60s tick from metricsLoop's probe, so it is deliberately a
+// plain List + filter with no cache: the registry is a local SQLite file, one
+// read a minute is nothing next to the pass loops already reading it, and a
+// cache would be one more thing to invalidate when an operator stamps a unit.
+//
+// A read failure returns nil — this tick's stamped units are lost, the sweep
+// and the agent are not. The names are returned sorted so the slice handed
+// down each tick is stable (nothing below depends on the order; a stable one
+// just makes a log or a test diff mean something).
+func stampedUnitNames(reg *registry.Registry) []string {
+ routes, err := reg.List()
+ if err != nil {
+ log.Printf("agent: metrics: read stamped units: %v", err)
+ return nil
+ }
+ seen := make(map[string]bool, len(routes))
+ var out []string
+ for _, rt := range routes {
+ if rt.Unit == "" || seen[rt.Unit] {
+ continue
+ }
+ seen[rt.Unit] = true
+ out = append(out, rt.Unit)
+ }
+ sort.Strings(out)
+ return out
+}
+
// resolveStatusBucket applies the status/deployment bucket defaulting
// documented in this file's package comment: an explicit CARLOS_STATUS_BUCKET
// always wins, and when it's unset the status loop falls back to whatever
diff --git a/internal/host/agent_test.go b/internal/host/agent_test.go
index 00917f7a4..682ae7181 100644
--- a/internal/host/agent_test.go
+++ b/internal/host/agent_test.go
@@ -1198,3 +1198,48 @@ func TestFastSweepBouncesAWarmHibernatingCanary(t *testing.T) {
}
}
}
+
+// ---- stamped units for the metrics sweep (#110 column) ----------------------
+
+// TestStampedUnitNamesDedupesAndSkipsUnstamped pins the metrics sweep's one
+// registry read. Route.Unit is a per-ROUTE stamp, so two routes fronted by
+// the same bespoke service (a console and its alias) must yield that unit
+// ONCE — a duplicate would make the box read the same cgroup twice per tick
+// for the same map key. Routes with no unit contribute nothing: the carlos-
+// convention walk owns those, and a "" name would match every unnamed
+// directory the walk compares against.
+func TestStampedUnitNamesDedupesAndSkipsUnstamped(t *testing.T) {
+ reg, err := registry.Open(filepath.Join(t.TempDir(), "registry.db"))
+ if err != nil {
+ t.Fatalf("open registry: %v", err)
+ }
+ for _, rt := range []registry.Route{
+ {Host: "console.example.com", Kind: "instance", App: "console", Unit: "console.service"},
+ {Host: "dashboard.example.com", Kind: "instance", App: "console", Unit: "console.service"},
+ {Host: "vicky.example.com", Kind: "instance", App: "titogo", Unit: "titogo-canary@vicky.service"},
+ {Host: "plain.example.com", Kind: "instance", App: "plain"}, // unstamped
+ } {
+ if err := reg.Upsert(rt); err != nil {
+ t.Fatalf("upsert %s: %v", rt.Host, err)
+ }
+ }
+
+ got := stampedUnitNames(reg)
+ want := []string{"console.service", "titogo-canary@vicky.service"}
+ if !slices.Equal(got, want) {
+ t.Fatalf("stampedUnitNames = %v, want %v (sorted, deduped, unstamped routes skipped)", got, want)
+ }
+}
+
+// TestStampedUnitNamesEmptyRegistry pins the state every box is in today:
+// nothing stamped -> nil, which listStampedUnits turns into no extra scan at
+// all. The sweep must cost nothing on a fleet that has never used the column.
+func TestStampedUnitNamesEmptyRegistry(t *testing.T) {
+ reg, err := registry.Open(filepath.Join(t.TempDir(), "registry.db"))
+ if err != nil {
+ t.Fatalf("open registry: %v", err)
+ }
+ if got := stampedUnitNames(reg); got != nil {
+ t.Fatalf("stampedUnitNames on an empty registry = %v, want nil", got)
+ }
+}
diff --git a/internal/host/metrics_loop.go b/internal/host/metrics_loop.go
index 140b69dd8..9d6e26add 100644
--- a/internal/host/metrics_loop.go
+++ b/internal/host/metrics_loop.go
@@ -618,14 +618,27 @@ func httpPostWebhook(url string, body []byte) error {
// 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).
-func realSampleProbes(execRoot string) sampleProbes {
+//
+// stampedUnits is re-read inside the units probe rather than captured as a
+// slice, because sampleProbes is built ONCE and read every tick: a snapshot
+// taken here would pin the stamped set to whatever the registry held at agent
+// start, and a route stamped this afternoon would go unsampled until the next
+// restart. A nil func means "no stamped units" — the darwin/dev shape, and
+// what every test double passes.
+func realSampleProbes(execRoot string, stampedUnits func() []string) sampleProbes {
return sampleProbes{
procStat: readProcStat,
loadavg: readLoadAvg,
memFrac: readMemFrac,
diskFrac: readDiskFrac,
- units: func() map[string]unitRaw { return realUnitReadings(execRoot) },
- now: time.Now,
+ units: func() map[string]unitRaw {
+ var stamped []string
+ if stampedUnits != nil {
+ stamped = stampedUnits()
+ }
+ return realUnitReadings(execRoot, stamped)
+ },
+ now: time.Now,
}
}
@@ -633,11 +646,18 @@ func realSampleProbes(execRoot string) sampleProbes {
// each one's cgroup usage — the production units func sampleProbes.units
// 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) — plus, when delegation is armed, the per-host
-// exec-child cgroups under execRoot (keyed carlos-exec@<host>; see
-// listExecCgroups).
-func realUnitReadings(execRoot string) map[string]unitRaw {
+// see listCarlosUnits) — plus the operator-stamped bespoke units named in
+// stamped (registry Route.Unit; see listStampedUnits) and, when delegation is
+// armed, the per-host exec-child cgroups under execRoot (keyed
+// carlos-exec@<host>; see listExecCgroups).
+//
+// The three scans may overlap — a stamped unit that also matches the carlos-
+// convention is discovered twice — and the map write below is what dedupes
+// them: both entries name the same unit and the same cgroup dir, so the
+// second read is redundant, never contradictory.
+func realUnitReadings(execRoot string, stamped []string) map[string]unitRaw {
units := listCarlosUnits(cgroupSliceDir)
+ units = append(units, listStampedUnits(cgroupSliceDir, stamped)...)
units = append(units, listExecCgroups(execRoot)...)
if len(units) == 0 {
return nil
@@ -728,7 +748,13 @@ func metricsBoxLabel(getenv func(string) string, machineID string) string {
// 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.
-func metricsLoop(execRoot string) {
+//
+// stampedUnits is the sweep's one registry seam (agent.go's
+// stampedUnitNames): the bespoke units to sample alongside the carlos- ones,
+// re-read once per tick so a freshly stamped route is picked up within a
+// minute. It is a func, not the registry handle, so this loop keeps its "no
+// state but a bucket and /proc" shape; nil = none.
+func metricsLoop(execRoot string, stampedUnits func() []string) {
// 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
@@ -772,7 +798,7 @@ func metricsLoop(execRoot string) {
log.Printf("agent: metrics loop armed (60s ticks, 5min flush), publishing to bucket %s", bucket)
sm := &sampler{}
- probes := realSampleProbes(execRoot)
+ probes := realSampleProbes(execRoot, stampedUnits)
tick := 0
for {
time.Sleep(60 * time.Second)
diff --git a/internal/host/metrics_probes.go b/internal/host/metrics_probes.go
index 30ec0a3bb..4a43dfcc8 100644
--- a/internal/host/metrics_probes.go
+++ b/internal/host/metrics_probes.go
@@ -4,7 +4,7 @@
// 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
-// exceptions are listCarlosUnits and listExecCgroups: they are directory
+// exceptions are listCarlosUnits, listStampedUnits and listExecCgroups: they are directory
// WALKs, not single reads, and which directories they walk is a decision that
// has already been wrong once (see listCarlosUnits' doc comment), so they take
// their roots as arguments and are pinned by metrics_probes_test.go against
@@ -115,6 +115,65 @@ func listCarlosUnits(root string) []unitCgroup {
return units
}
+// listStampedUnits finds the cgroup directories of operator-stamped bespoke
+// units — registry Route.Unit (#110), the routes the carlos- naming
+// convention above cannot see (an operator's console.service, tito's
+// titogo-canary@<tenant>.service). Read failure -> nil, same as
+// listCarlosUnits.
+//
+// Same two-level walk, for the same reason: a flat unit sits under root, a
+// template instance inside its implicit system-*.slice. The stamped NAME says
+// nothing about which of the two systemd chose, so both levels are matched
+// against the same wanted set.
+//
+// A stamped unit with no cgroup dir under root is simply not returned — no
+// reading is the honest report for a unit that isn't running, and the console
+// renders that absence rather than a guessed zero. Names arrive from the
+// registry already shaped by registry.ValidUnitName (the door validates; see
+// SetUnit), so nothing here re-validates them: an unmatchable name matches
+// nothing, which is the same outcome as a stopped unit.
+func listStampedUnits(root string, stamped []string) []unitCgroup {
+ if len(stamped) == 0 {
+ return nil
+ }
+ want := make(map[string]bool, len(stamped))
+ for _, u := range stamped {
+ want[u] = true
+ }
+
+ entries, err := os.ReadDir(root)
+ if err != nil {
+ return nil
+ }
+
+ var units []unitCgroup
+ for _, entry := range entries {
+ if !entry.IsDir() {
+ continue
+ }
+ name := entry.Name()
+ if want[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() || !want[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.
diff --git a/internal/host/metrics_probes_test.go b/internal/host/metrics_probes_test.go
index 0e691d0ca..ee1eaa02f 100644
--- a/internal/host/metrics_probes_test.go
+++ b/internal/host/metrics_probes_test.go
@@ -54,6 +54,56 @@ func TestListCarlosUnitsFindsFlatAndNestedUnits(t *testing.T) {
}
}
+// TestListStampedUnits pins the second discovery walk: units named by the
+// registry's Unit column (#110) rather than by the carlos- convention. The
+// same two systemd levels are in play — a bespoke flat service, and a
+// template instance inside its implicit per-template slice — because the
+// stamped name says nothing about where systemd put its cgroup. A unit
+// present on disk but NOT stamped stays invisible (the convention walk owns
+// what it owns), and a stamped unit with no cgroup dir yields nothing at all:
+// absence is the honest report for a unit that isn't running.
+func TestListStampedUnits(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
+ }
+
+ // Flat bespoke unit.
+ flatDir := mk("console.service")
+ // Template instance inside its implicit per-template slice. The literal
+ // directory name carries systemd's \x2d escaping of the dash.
+ nestedDir := mk(`system-titogo\x2dcanary.slice`, "titogo-canary@demo.service")
+ // Present on disk but NOT stamped — must not be returned.
+ mk("postgres.service")
+
+ got := listStampedUnits(root, []string{"console.service", "titogo-canary@demo.service", "absent@x.service"})
+ want := map[string]string{"console.service": flatDir, "titogo-canary@demo.service": nestedDir}
+ if len(got) != 2 {
+ t.Fatalf("got %d units, want 2 (the stamped ones that exist): %+v", len(got), got)
+ }
+ for _, u := range got {
+ dir, ok := want[u.name]
+ if !ok {
+ t.Fatalf("unexpected unit %q", u.name)
+ }
+ if u.dir != dir {
+ t.Fatalf("unit %q dir = %q, want %q (the real cgroup dir)", u.name, u.dir, dir)
+ }
+ if _, err := os.Stat(u.dir); err != nil {
+ t.Fatalf("dir %q not readable: %v", u.dir, err)
+ }
+ }
+
+ if out := listStampedUnits(root, nil); out != nil {
+ t.Fatalf("no stamped units must scan nothing, got %+v", out)
+ }
+}
+
// 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.