amadan

carlos / platform Public

console: owner metrics match exec-backed instances by their cgroup key

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell 084fdb0a4fe8a172d0c442ad17d7585849f9873a parent d9e64b6
2 files changed, +127 −12
  • internal/console/metricsview.go +29 −12
  • internal/console/metricsview_test.go +98 −0
diff --git a/internal/console/metricsview.go b/internal/console/metricsview.go
index 0d519279a..45789388c 100644
--- a/internal/console/metricsview.go
+++ b/internal/console/metricsview.go
@@ -629,8 +629,9 @@ type InstanceMetrics struct {
// is false, where it decides what the absence of readings may claim: a
// hibernating instance is presumed asleep, anything else is NOT — it may
// be serving right now from outside per-instance accounting (a bespoke
- // unit like keymail-relay.service, or an exec child of the agent), and
- // the template must not assert a sleep state nobody observed.
+ // unit like keymail-relay.service, or an exec child on a box whose agent
+ // has no delegated subtree yet), and the template must not assert a
+ // sleep state nobody observed.
Hibernate bool
}
@@ -643,11 +644,12 @@ type InstanceMetrics struct {
// was unreadable, which renders as the usual em-dash.)
func (m InstanceMetrics) Reporting() bool { return m.Box != "" }
-// OwnerAppMetrics filters the cached 48h sweep down to EXACTLY the units
-// carlos-app@<host>.service for the given hosts. It returns unit-level rows
-// only — no host totals, no other units, no alert data. The allowlist is the
-// hosts slice the caller derived from the app's OWN routes; nothing else can
-// enter the view.
+// OwnerAppMetrics filters the cached 48h sweep down to EXACTLY the given
+// hosts' own per-instance readings — carlos-app@<host>.service for a
+// unit-backed tenant, carlos-exec@<host> for an exec-backed one. It returns
+// unit-level rows only — no host totals, no other units, no alert data. The
+// allowlist is the hosts slice the caller derived from the app's OWN routes;
+// nothing else can enter the view.
//
// Every allowlisted host gets exactly one row, in host order, whether or not
// the sweep saw it: a host with no unit data is an UNKNOWN row (nil
@@ -657,12 +659,18 @@ func (s *Server) OwnerAppMetrics(ctx context.Context, hosts []string) ([]Instanc
// The allowlist, keyed by the one unit name each host can legally match.
// Matching is exact-string on this map: a unit the caller did not ask
// for cannot be reached by any path below.
- want := make(map[string]string, len(hosts))
+ want := make(map[string]string, 2*len(hosts))
for _, h := range hosts {
if h == "" {
continue
}
+ // Two legal keys per host: the systemd unit a unit-backed tenant
+ // reports under, and the delegated exec cgroup an exec-backed one
+ // does. One route, one backing — a host can't legally report under
+ // both at once; if both ever appear in a sweep, the freshest-box
+ // rule below picks deterministically, no special case.
want[activator.UnitName(h)] = h
+ want[activator.ExecName(h)] = h
}
if len(want) == 0 {
return nil, nil
@@ -706,12 +714,21 @@ func (s *Server) OwnerAppMetrics(ctx context.Context, hosts []string) ([]Instanc
}
}
- rows := make([]InstanceMetrics, 0, len(want))
- for _, host := range want {
- if row, ok := found[host]; ok {
+ // Iterate the callers' hosts, not want's values: want now carries TWO
+ // keys per host, and "every allowlisted host gets exactly one row" is a
+ // promise about hosts, not keys. seen collapses duplicate input hosts,
+ // which the old unique-keyed map did implicitly.
+ rows := make([]InstanceMetrics, 0, len(hosts))
+ seen := make(map[string]bool, len(hosts))
+ for _, h := range hosts {
+ if h == "" || seen[h] {
+ continue
+ }
+ seen[h] = true
+ if row, ok := found[h]; ok {
rows = append(rows, row)
} else {
- rows = append(rows, InstanceMetrics{Host: host})
+ rows = append(rows, InstanceMetrics{Host: h})
}
}
sort.Slice(rows, func(i, j int) bool { return rows[i].Host < rows[j].Host })
diff --git a/internal/console/metricsview_test.go b/internal/console/metricsview_test.go
index 27370f67b..73ec4a2b4 100644
--- a/internal/console/metricsview_test.go
+++ b/internal/console/metricsview_test.go
@@ -1251,3 +1251,101 @@ func TestSparkPointsDownsampleKeepsNilUnknown(t *testing.T) {
}
}
}
+
+// TestOwnerAppMetricsSeesExecBackedInstances pins the second allowlist key:
+// an exec-backed host reports under carlos-exec@<host> (the delegated
+// cgroup's verbatim name, no .service), and its readings reach the owner
+// row exactly like a unit-backed tenant's. Another tenant's exec cgroup
+// stays invisible — the allowlist boundary holds for the new key form too.
+func TestOwnerAppMetricsSeesExecBackedInstances(t *testing.T) {
+ srv, rel := newMetricsFixture(t)
+ ctx := context.Background()
+ at := time.Now().UTC().Add(-time.Minute)
+ if err := metrics.WriteRaw(ctx, rel, metrics.RawChunk{
+ V: 1, Box: "box-a", Hour: metrics.HourOf(at),
+ Samples: []metrics.Sample{{
+ At: at,
+ Units: map[string]metrics.UnitSample{
+ "carlos-exec@keymail.example.com": {CPUFrac: p(0.22), MemBytes: pi64(2048)},
+ "carlos-exec@other.example.com": {CPUFrac: p(0.44), MemBytes: pi64(4096)},
+ },
+ }},
+ }); err != nil {
+ t.Fatal(err)
+ }
+
+ rows, err := srv.OwnerAppMetrics(ctx, []string{"keymail.example.com"})
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(rows) != 1 {
+ t.Fatalf("rows = %+v, want exactly the one allowlisted host", rows)
+ }
+ got := rows[0]
+ if got.Host != "keymail.example.com" || !got.Reporting() {
+ t.Fatalf("exec-backed host not reporting: %+v", got)
+ }
+ if got.CPUFrac == nil || *got.CPUFrac != 0.22 {
+ t.Fatalf("CPU = %v, want 0.22", got.CPUFrac)
+ }
+ if got.MemBytes == nil || *got.MemBytes != 2048 {
+ t.Fatalf("mem = %v, want 2048", got.MemBytes)
+ }
+ if got.SparkCPU == "" || got.SparkMem == "" {
+ t.Fatalf("sparklines empty: %+v", got)
+ }
+ if got.CPUFrac != nil && *got.CPUFrac == 0.44 {
+ t.Fatalf("row carries another tenant's exec reading: %+v", got)
+ }
+}
+
+// TestOwnerAppMetricsExecAndUnitKeysNeverDoubleReport pins the spec's
+// no-double-report property: a host can't legally report under both key
+// forms at once (one route, one backing), but if a sweep ever carries both
+// — a backing flip mid-window, two boxes — the existing freshest-box rule
+// picks ONE deterministically. No special case, still exactly one row.
+func TestOwnerAppMetricsExecAndUnitKeysNeverDoubleReport(t *testing.T) {
+ srv, rel := newMetricsFixture(t)
+ ctx := context.Background()
+ fresher := time.Now().UTC().Add(-time.Minute)
+ older := fresher.Add(-2 * time.Minute)
+ // The older box reports the host as a systemd unit, the fresher box as
+ // an exec cgroup — the flip-mid-window shape.
+ if err := metrics.WriteRaw(ctx, rel, metrics.RawChunk{
+ V: 1, Box: "box-old", Hour: metrics.HourOf(older),
+ Samples: []metrics.Sample{{
+ At: older,
+ Units: map[string]metrics.UnitSample{
+ "carlos-app@flip.example.com.service": {CPUFrac: p(0.99), MemBytes: pi64(9999)},
+ },
+ }},
+ }); err != nil {
+ t.Fatal(err)
+ }
+ if err := metrics.WriteRaw(ctx, rel, metrics.RawChunk{
+ V: 1, Box: "box-new", Hour: metrics.HourOf(fresher),
+ Samples: []metrics.Sample{{
+ At: fresher,
+ Units: map[string]metrics.UnitSample{
+ "carlos-exec@flip.example.com": {CPUFrac: p(0.11), MemBytes: pi64(1111)},
+ },
+ }},
+ }); err != nil {
+ t.Fatal(err)
+ }
+
+ rows, err := srv.OwnerAppMetrics(ctx, []string{"flip.example.com"})
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(rows) != 1 {
+ t.Fatalf("rows = %+v, want exactly one row for the host, never two", rows)
+ }
+ got := rows[0]
+ if got.Box != "box-new" {
+ t.Fatalf("Box = %q, want the freshest box-new: %+v", got.Box, got)
+ }
+ if got.CPUFrac == nil || *got.CPUFrac != 0.11 {
+ t.Fatalf("CPU = %v, want the freshest box's 0.11", got.CPUFrac)
+ }
+}