amadan

carlos / platform Public

console: owner metrics — per-account allowlist, freshness bound, downsampled sparks

Three review findings on Task 12.

1. App names are unique per ACCOUNT, not globally, and Assemble groups
   routes by app name alone — so two accounts each owning an app called
   "api" put both accounts' routes in one AppView. handleApp's metrics
   allowlist now takes only routes whose own account matches the account
   being viewed (RouteView carries release.RouteAccount now). It fails
   closed: the half-migrated state yields no metrics, never someone
   else's. (Assemble's route/domain listing is deliberately untouched —
   narrowing it there would also decide what the AccountMismatch banner
   shows, which is a separate call; that pre-existing cross-account
   listing is called out in the task report.)

2. The 48h sweep keeps serving a dead box's final sample for two days,
   which the app page labelled the instance's current figure. A box
   whose newest sample is older than metricsFreshWindow (5m = five
   missed samples) now drops out of the owner view entirely, so its
   instances read as no-recent-readings instead of as stale numbers.
   The operator dashboard, which prints "Latest <time>", is unbounded.

3. sparkPoints now buckets any input longer than the viewBox width down
   to that width, each bucket the mean of its non-nil readings — a
   bucket with nothing readable stays a gap, never a zero. ~2880
   readings into a 240px box was twelve points per pixel nobody can
   see, paid for in cached coordinate strings for every metric of every
   unit on every box. One code path: Task 11's host sparks get it too.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell 29aac10952cc7d2c90d27bb74f5bb36f4b830a9f parent 5b415b2
5 files changed, +266 −5
  • internal/console/metricsview.go +65 −3
  • internal/console/metricsview_test.go +64 −1
  • internal/console/server.go +13 −0
  • internal/console/server_test.go +115 −1
  • internal/console/view.go +9 −0
diff --git a/internal/console/metricsview.go b/internal/console/metricsview.go
index cd00557bc..aab226c37 100644
--- a/internal/console/metricsview.go
+++ b/internal/console/metricsview.go
@@ -63,6 +63,18 @@ const metricsCacheTTL = 60 * time.Second
// 48 UTC hours, days/ rollups untouched (a later task's job).
const metricsSweepHours = 48
+// metricsFreshWindow is how old a box's newest sample may be before its
+// readings stop being anyone's CURRENT figure. The agent samples once a
+// minute, so five minutes is five consecutively missed samples — past any
+// normal hiccup, and the point at which a box has plainly stopped
+// publishing. It matters because the 48h sweep keeps serving a dead box's
+// final sample for two days: without this bound an instance that vanished
+// on Monday still reads "11% / 1.0 KB, current" on Wednesday. Owners get the
+// no-recent-readings state instead (they have no timestamp on the page to
+// judge staleness for themselves; the operator dashboard prints "Latest
+// <time>" and so is deliberately left unbounded).
+const metricsFreshWindow = 5 * time.Minute
+
// metricsBoxesPrefix is where a box publishes its raw chunks and day
// rollups (internal/metrics.RawKey / DayKey); box discovery lists this
// prefix and reads the path segment right after "boxes/".
@@ -340,7 +352,15 @@ func (s *Server) OwnerAppMetrics(ctx context.Context, hosts []string) ([]Instanc
found := make(map[string]InstanceMetrics, len(want))
at := make(map[string]time.Time, len(want))
+ now := time.Now()
for _, box := range boxes {
+ // A box that stopped publishing keeps its final sample inside the
+ // 48h sweep for two days; those numbers are history, not this
+ // instance's current figure, so the whole box drops out of the
+ // owner view and its instances read as no-recent-readings.
+ if box.At.IsZero() || now.Sub(box.At) > metricsFreshWindow {
+ continue
+ }
for _, u := range box.Units {
host, ok := want[u.Unit]
if !ok {
@@ -425,15 +445,21 @@ func (s *Server) sweepAlerts(ctx context.Context) ([]AlertRow, error) {
// index) onto an SVG polyline "points" string on a w x h viewBox. Values are
// fractions and clamp into [0,1] if out of range; y is inverted (SVG's y
// axis points down, so a higher fraction plots at a SMALLER y — visually
-// "up"); x is spread evenly across the width by the sample's index in vals,
-// not by elapsed time. A nil sample contributes no pair at all — its x slot
-// is simply skipped, so the polyline draws a straight line across the gap;
+// "up"); x is spread evenly across the width by the sample's index, not by
+// elapsed time. A nil sample contributes no pair at all — its x slot is
+// simply skipped, so the polyline draws a straight line across the gap;
// breaking the line at gaps is not attempted in v1 (documented here per the
// brief). Empty input or input that is nil throughout returns "".
+//
+// Input longer than w is bucketed down to w points first (see downsample):
+// 48h of minute samples is ~2880 readings into a 240px box — twelve points
+// per pixel of detail nobody can see, paid for in a coordinate string held
+// in the shared cache for every metric of every unit on every box.
func sparkPoints(vals []*float64, w, h int) string {
if len(vals) == 0 {
return ""
}
+ vals = downsample(vals, w)
var b strings.Builder
n := len(vals)
for i, v := range vals {
@@ -460,3 +486,39 @@ func sparkPoints(vals []*float64, w, h int) string {
}
return b.String()
}
+
+// downsample buckets vals into at most max evenly-sized buckets, each
+// bucket's value being the MEAN of its non-nil readings — a spike lifts its
+// bucket rather than disappearing, and a bucket with nothing readable in it
+// stays nil, so "unknown" survives bucketing as a gap and never becomes a
+// zero. Input at or under max is returned untouched (the common case: a box
+// that has been up for minutes, and every unit test's handful of samples).
+func downsample(vals []*float64, max int) []*float64 {
+ if max <= 0 || len(vals) <= max {
+ return vals
+ }
+ n := len(vals)
+ out := make([]*float64, max)
+ for b := 0; b < max; b++ {
+ lo := b * n / max
+ hi := (b + 1) * n / max
+ if hi > n {
+ hi = n
+ }
+ var sum float64
+ var cnt int
+ for _, v := range vals[lo:hi] {
+ if v == nil {
+ continue
+ }
+ sum += *v
+ cnt++
+ }
+ if cnt == 0 {
+ continue // nothing readable in this bucket: a gap, not a zero
+ }
+ mean := sum / float64(cnt)
+ out[b] = &mean
+ }
+ return out
+}
diff --git a/internal/console/metricsview_test.go b/internal/console/metricsview_test.go
index e23e05714..11fc60613 100644
--- a/internal/console/metricsview_test.go
+++ b/internal/console/metricsview_test.go
@@ -212,7 +212,8 @@ func TestOwnerAppMetricsEmptyHosts(t *testing.T) {
func TestOwnerAppMetricsFiltersToOwnUnits(t *testing.T) {
srv, rel := newMetricsFixture(t)
ctx := context.Background()
- at := time.Now().UTC().Add(-5 * time.Minute)
+ // Inside metricsFreshWindow: these are the box's CURRENT readings.
+ at := time.Now().UTC().Add(-time.Minute)
if err := metrics.WriteRaw(ctx, rel, metrics.RawChunk{
V: 1, Box: "box-secret-a", Hour: metrics.HourOf(at),
Samples: []metrics.Sample{{
@@ -325,3 +326,65 @@ func TestSparkPointsEmptyOrAllNil(t *testing.T) {
t.Fatalf("all-nil input = %q, want empty", got)
}
}
+
+// TestSparkPointsDownsamples: 48h of minute samples is ~2880 readings into a
+// 240px-wide viewBox — 12 points per pixel of invisible detail, held in the
+// shared cache for every unit on every box. sparkPoints buckets down to at
+// most the viewBox width, and a bucket's value is the MEAN of its non-nil
+// readings, so a spike still lifts its bucket rather than vanishing.
+func TestSparkPointsDownsamples(t *testing.T) {
+ vals := make([]*float64, 2880)
+ for i := range vals {
+ vals[i] = p(0.1)
+ }
+ vals[1000] = p(1.0) // the spike
+ pts := strings.Fields(sparkPoints(vals, 240, 40))
+ if len(pts) == 0 || len(pts) > 240 {
+ t.Fatalf("got %d coordinate pairs, want 1..240", len(pts))
+ }
+ ys := make([]float64, len(pts))
+ for i, pair := range pts {
+ parts := strings.SplitN(pair, ",", 2)
+ if len(parts) != 2 {
+ t.Fatalf("malformed pair %q", pair)
+ }
+ y, err := strconv.ParseFloat(parts[1], 64)
+ if err != nil {
+ t.Fatalf("bad y in %q: %v", pair, err)
+ }
+ ys[i] = y
+ }
+ // Exactly one bucket contains the spike, and it plots higher (smaller y)
+ // than the flat 0.1 baseline every other bucket sits at.
+ baseline := ys[0]
+ raised := 0
+ for _, y := range ys {
+ if y < baseline-0.001 {
+ raised++
+ }
+ }
+ if raised != 1 {
+ t.Fatalf("%d buckets rose above the baseline, want exactly 1 (the spike's): %v", raised, ys)
+ }
+}
+
+// TestSparkPointsDownsampleKeepsNilUnknown: a bucket with no readable value
+// at all contributes NO point — the gap stays a gap, never a zero, even
+// after bucketing.
+func TestSparkPointsDownsampleKeepsNilUnknown(t *testing.T) {
+ vals := make([]*float64, 1000)
+ for i := range vals {
+ if i < 500 {
+ vals[i] = p(0.5)
+ }
+ }
+ pts := strings.Fields(sparkPoints(vals, 240, 40))
+ if len(pts) == 0 || len(pts) > 120 {
+ t.Fatalf("got %d pairs, want at most the ~120 buckets covering the readable half: %v", len(pts), pts)
+ }
+ for _, pair := range pts {
+ if strings.HasSuffix(pair, ",40.00") {
+ t.Fatalf("an all-nil bucket was plotted as zero: %q", pair)
+ }
+ }
+}
diff --git a/internal/console/server.go b/internal/console/server.go
index 913f68071..724c07a92 100644
--- a/internal/console/server.go
+++ b/internal/console/server.go
@@ -1124,8 +1124,21 @@ func (s *Server) handleApp(w http.ResponseWriter, r *http.Request) {
// is the entire tenant boundary on the metrics read, so it must never be
// widened to "all routes" or taken from the request. A failed sweep
// costs the section, never the page.
+ //
+ // The account comparison is load-bearing and not redundant with the
+ // accountApp gate above: Assemble groups routes by app NAME, and app
+ // names are unique per ACCOUNT, not globally (Store.AppAccounts) — two
+ // accounts may each own an app called "api", and both accounts' routes
+ // arrive in this one AppView. Comparing each route's own account is what
+ // keeps one tenant's cgroup readings off the other's page. It fails
+ // closed: a route stamped with an account the console record disagrees
+ // with (the half-migrated state AccountMismatch describes) yields no
+ // metrics rather than someone else's.
hosts := make([]string, 0, len(views[0].Routes))
for _, rt := range views[0].Routes {
+ if rt.Account != acct.Sqid {
+ continue
+ }
hosts = append(hosts, rt.Host)
}
if am, err := s.OwnerAppMetrics(r.Context(), hosts); err != nil {
diff --git a/internal/console/server_test.go b/internal/console/server_test.go
index a977dcbd4..79f301de3 100644
--- a/internal/console/server_test.go
+++ b/internal/console/server_test.go
@@ -1215,7 +1215,7 @@ func TestAppPageOwnerMetricsBoundary(t *testing.T) {
Status: "ready", App: "kass", Backing: registry.BackingUnit, Hibernate: true})
ctx := context.Background()
- at := time.Now().UTC().Add(-5 * time.Minute)
+ at := time.Now().UTC().Add(-time.Minute) // inside metricsFreshWindow
if err := metrics.WriteRaw(ctx, fx.rel, metrics.RawChunk{
V: 1, Box: "box-secret-a", Hour: metrics.HourOf(at),
Samples: []metrics.Sample{{
@@ -1287,3 +1287,117 @@ func (s failingListStore) List(ctx context.Context, prefix string) ([]string, er
}
return s.Store.List(ctx, prefix)
}
+
+// TestAppPageOwnerMetricsCrossAccountAppName is the app-NAME collision: app
+// names are unique per account, not globally (store.AppAccounts), so two
+// accounts can each own an app called "kass". Assemble groups routes by app
+// name alone, so account B's routes arrive in account A's AppView — and the
+// metrics allowlist must not follow them. A's page must carry none of B's
+// instance readings.
+func TestAppPageOwnerMetricsCrossAccountAppName(t *testing.T) {
+ fx := newConsoleFixture(t)
+ if err := fx.st.CreateAccount(42, "cdef", "Rival", "rival@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := fx.st.CreateAppIn(42, "kass", "rival@keymail.dev", time.Now()); err != nil {
+ t.Fatal(err)
+ }
+ fx.addRoute(t, registry.Route{Host: "owned.example.com", Kind: "instance",
+ Status: "ready", App: "kass", Backing: registry.BackingUnit})
+ // Same app name, different account — paul is not a member of cdef.
+ fx.addRoute(t, registry.Route{Host: "rival.example.com", Kind: "instance",
+ Status: "ready", App: "kass", Account: "cdef", Backing: registry.BackingUnit})
+
+ at := time.Now().UTC().Add(-time.Minute)
+ if err := metrics.WriteRaw(context.Background(), fx.rel, metrics.RawChunk{
+ V: 1, Box: "box-shared", Hour: metrics.HourOf(at),
+ Samples: []metrics.Sample{{
+ At: at,
+ Units: map[string]metrics.UnitSample{
+ "carlos-app@owned.example.com.service": {CPUFrac: p(0.11), MemBytes: pi64(1024)},
+ "carlos-app@rival.example.com.service": {CPUFrac: p(0.33), MemBytes: pi64(8192)},
+ },
+ }},
+ }); err != nil {
+ t.Fatal(err)
+ }
+
+ c := signInPaul(t, fx)
+ status, _, body := get(t, c, fx.u("/apps/kass"))
+ if status != http.StatusOK {
+ t.Fatalf("/apps/kass = %d, want 200", status)
+ }
+ if !strings.Contains(body, "11%") {
+ t.Fatalf("own instance's metrics missing in:\n%s", body)
+ }
+ // The other account's readings, by any spelling.
+ for _, forbidden := range []string{"33%", "8.0 KB"} {
+ if strings.Contains(body, forbidden) {
+ t.Fatalf("app page leaked another account's reading %q in:\n%s", forbidden, body)
+ }
+ }
+ // And the other account's host is not in the Resources section at all.
+ //
+ // NOTE: the assertion is scoped to Resources on purpose. Assemble groups
+ // routes by app NAME with no account filter (view.go), so rival's host
+ // already renders in this page's Domains/ladder tables — a PRE-EXISTING
+ // cross-account leak this task neither introduces nor fixes (fixing it
+ // there would also decide what the half-migrated AccountMismatch banner
+ // shows, which is a separate call). See the task report.
+ if res := resourcesSection(t, body); strings.Contains(res, "rival.example.com") {
+ t.Fatalf("Resources section leaked another account's host:\n%s", res)
+ }
+}
+
+// resourcesSection returns just the app page's Resources block.
+func resourcesSection(t *testing.T, body string) string {
+ t.Helper()
+ i := strings.Index(body, "<h2>Resources</h2>")
+ if i < 0 {
+ t.Fatalf("no Resources section in:\n%s", body)
+ }
+ j := strings.Index(body[i:], "<h2>Channels</h2>")
+ if j < 0 {
+ t.Fatalf("Resources section never ended:\n%s", body[i:])
+ }
+ return body[i : i+j]
+}
+
+// TestAppPageOwnerMetricsStaleBoxIsNotCurrent: a box that stopped publishing
+// keeps its last sample inside the 48h sweep window forever. Rendering that
+// as the instance's CURRENT figure would be a lie with no expiry, so a box
+// whose newest sample is older than the freshness window reads as no recent
+// readings — the same state a hibernating instance gets.
+func TestAppPageOwnerMetricsStaleBoxIsNotCurrent(t *testing.T) {
+ fx := newConsoleFixture(t)
+ fx.addRoute(t, registry.Route{Host: "owned.example.com", Kind: "instance",
+ Status: "ready", App: "kass", Backing: registry.BackingUnit})
+
+ at := time.Now().UTC().Add(-3 * time.Hour) // inside the sweep, far outside fresh
+ if err := metrics.WriteRaw(context.Background(), fx.rel, metrics.RawChunk{
+ V: 1, Box: "box-dead", Hour: metrics.HourOf(at),
+ Samples: []metrics.Sample{{
+ At: at,
+ Units: map[string]metrics.UnitSample{
+ "carlos-app@owned.example.com.service": {CPUFrac: p(0.23), MemBytes: pi64(2048)},
+ },
+ }},
+ }); err != nil {
+ t.Fatal(err)
+ }
+
+ c := signInPaul(t, fx)
+ status, _, body := get(t, c, fx.u("/apps/kass"))
+ if status != http.StatusOK {
+ t.Fatalf("/apps/kass = %d, want 200", status)
+ }
+ if !strings.Contains(body, "asleep — no recent readings") {
+ t.Fatalf("stale box did not render as no-recent-readings:\n%s", resourcesSection(t, body))
+ }
+ for _, forbidden := range []string{"23%", "2.0 KB"} {
+ if strings.Contains(body, forbidden) {
+ t.Fatalf("app page rendered a %v-old reading as current (%q):\n%s",
+ time.Since(at).Truncate(time.Hour), forbidden, resourcesSection(t, body))
+ }
+ }
+}
diff --git a/internal/console/view.go b/internal/console/view.go
index 76fb54068..d20af48d3 100644
--- a/internal/console/view.go
+++ b/internal/console/view.go
@@ -107,6 +107,14 @@ type CanaryView struct {
type RouteView struct {
Host, Kind, Status, Version, Channel string
Hibernate bool
+ // Account is what the ROUTE says its account is (release.RouteAccount:
+ // the stamped account, or the ops account when unstamped). Routes are
+ // grouped by app NAME here, and app names are unique per account rather
+ // than globally (Store.AppAccounts) — so a view's Routes can legitimately
+ // contain another account's routes, and anything drawing a tenant
+ // boundary off this list must compare this field against the account
+ // being viewed. See handleApp's owner-metrics allowlist.
+ Account string
}
// ChannelView is one row of the channels table.
@@ -199,6 +207,7 @@ func Assemble(regPath string, s release.Store, pub ed25519.PublicKey, acct strin
byApp[rt.App] = append(byApp[rt.App], RouteView{
Host: rt.Host, Kind: rt.Kind, Status: rt.Status,
Version: rt.Version, Channel: ch, Hibernate: rt.Hibernate,
+ Account: release.RouteAccount(rt),
})
domainsByApp[rt.App] = append(domainsByApp[rt.App], domainView(rt, edgeIPs))
}