amadan

carlos / platform Public

console: render each alert value in its own rule's unit

Every alert value went through the percent formatter, on the dashboard's
alerts table and in the alert mail. Every rule but one carries a
fraction; lag is keymail relay seconds, so the first lag alert to fire
would have read "92000%" instead of "920s". alertValueText renders
disk/mem/cpu as percent, lag as seconds, and an unknown future rule as a
bare number rather than giving it someone else's unit.

The alerts section had also never actually been rendered in a test: the
new one seeds a firing frac rule and a firing lag rule, and pins that
the section appears above the first box card with each value in its own
unit.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell 6313bc3113d1a99ec1392d0ad3fd73b6275452bd parent 378c252
5 files changed, +96 −8
  • internal/console/alertwatch.go +24 −2
  • internal/console/alertwatch_test.go +3 −1
  • internal/console/server.go +4 −4
  • internal/console/server_test.go +64 −0
  • internal/console/templates/metrics.html +1 −1
diff --git a/internal/console/alertwatch.go b/internal/console/alertwatch.go
index e0023ebad..f34212779 100644
--- a/internal/console/alertwatch.go
+++ b/internal/console/alertwatch.go
@@ -6,6 +6,7 @@ import (
"fmt"
"log"
"sort"
+ "strconv"
"strings"
"time"
@@ -244,6 +245,27 @@ func (s *Server) recordAlertSeen(box, rule, address string, firing bool) {
}
}
+// alertValueText renders one rule's transition value in that rule's own
+// unit. Every rule internal/metrics evaluates carries a fraction EXCEPT lag,
+// which is keymail relay seconds (see metrics.Rules.LagSecs) — so a single
+// percent formatter for all of them turns the first lag alert to fire into
+// "92000%". Used by both operator-facing surfaces: the dashboard's alerts
+// table and the alert mail below.
+//
+// An unknown rule name (a future rule this console has not been taught) is
+// rendered as a bare number rather than being given someone else's unit: a
+// unitless figure is honest, a wrong unit is not.
+func alertValueText(rule string, value float64) string {
+ switch rule {
+ case "disk", "mem", "cpu":
+ return fmt.Sprintf("%.0f%%", value*100)
+ case "lag":
+ return fmt.Sprintf("%.0fs", value)
+ default:
+ return strconv.FormatFloat(value, 'g', -1, 64)
+ }
+}
+
// alertMessage is the operator-facing copy. Plain sentences, and no needle
// beyond the value that triggered the transition — this mail lands in an
// inbox, not on the operator dashboard.
@@ -255,8 +277,8 @@ func alertMessage(box, rule string, rs metrics.RuleState, updated time.Time) (su
stamp := at.UTC().Format(time.RFC3339)
if rs.Firing {
subject = fmt.Sprintf("[carlos] %s: %s alert", box, rule)
- body = fmt.Sprintf("The %s alert is firing on %s.\n\nValue at the transition: %g\nTime (UTC): %s\n\n"+
- "You are getting this because you hold the CARLOS fleet-operator role.\n", rule, box, rs.Value, stamp)
+ body = fmt.Sprintf("The %s alert is firing on %s.\n\nValue at the transition: %s\nTime (UTC): %s\n\n"+
+ "You are getting this because you hold the CARLOS fleet-operator role.\n", rule, box, alertValueText(rule, rs.Value), stamp)
return subject, body
}
subject = fmt.Sprintf("[carlos] %s: %s cleared", box, rule)
diff --git a/internal/console/alertwatch_test.go b/internal/console/alertwatch_test.go
index 75ac99fa2..34997cc5a 100644
--- a/internal/console/alertwatch_test.go
+++ b/internal/console/alertwatch_test.go
@@ -119,7 +119,9 @@ func TestAlertPassFiresOnceThenGoesQuiet(t *testing.T) {
if got.subject != "[carlos] box-a: disk alert" {
t.Fatalf("subject = %q", got.subject)
}
- for _, want := range []string{"disk", "box-a", "0.95", "2026-08-02T10:00:00Z"} {
+ // "95%", not "0.95": the mail carries the rule's own unit, the same as
+ // the operator dashboard's alerts table (see alertValueText).
+ for _, want := range []string{"disk", "box-a", "95%", "2026-08-02T10:00:00Z"} {
if !strings.Contains(got.body, want) {
t.Fatalf("body is missing %q:\n%s", want, got.body)
}
diff --git a/internal/console/server.go b/internal/console/server.go
index 724c07a92..bae9a5f18 100644
--- a/internal/console/server.go
+++ b/internal/console/server.go
@@ -74,10 +74,10 @@ var templates = template.Must(template.New("").Funcs(template.FuncMap{
}
return fmt.Sprintf("%.0f%%", *v*100)
},
- // pctVal is pct's non-pointer twin for AlertRow.Value: LoadMetrics only
- // ever returns firing rules, so an alert row always carries a value —
- // there is no nil case to dash out.
- "pctVal": func(v float64) string { return fmt.Sprintf("%.0f%%", v*100) },
+ // alertVal renders one AlertRow's value in ITS OWN rule's unit —
+ // see alertValueText. LoadMetrics only ever returns firing rules, so an
+ // alert row always carries a value; there is no nil case to dash out.
+ "alertVal": alertValueText,
// bytesOrDash renders a byte count in the smallest unit that keeps it
// readable; nil (no reading, from a unit that reported CPU but not
// memory/IO that tick) is "—", never a fabricated 0 B.
diff --git a/internal/console/server_test.go b/internal/console/server_test.go
index 79f301de3..74da16298 100644
--- a/internal/console/server_test.go
+++ b/internal/console/server_test.go
@@ -1401,3 +1401,67 @@ func TestAppPageOwnerMetricsStaleBoxIsNotCurrent(t *testing.T) {
}
}
}
+
+// TestFleetMetricsRendersFiringAlertsWithUnits pins two things the alerts
+// section had never actually been rendered in a test for: it appears ABOVE
+// the box cards (an operator opening the page sees what is on fire before
+// scrolling anything), and each rule's value carries ITS OWN unit. Every
+// alert value used to render through the percent formatter, so the first lag
+// alert to fire — lag is seconds, not a fraction — would have read
+// "92000%" instead of "920s".
+func TestFleetMetricsRendersFiringAlertsWithUnits(t *testing.T) {
+ fx := newConsoleFixture(t)
+ ctx := context.Background()
+
+ at := time.Now().UTC().Add(-2 * time.Minute)
+ if err := metrics.WriteRaw(ctx, fx.rel, metrics.RawChunk{
+ V: 1, Box: "box-alerting", Hour: metrics.HourOf(at),
+ Samples: []metrics.Sample{{At: at, Host: metrics.HostSample{CPUFrac: p(0.42), DiskFrac: p(0.95)}}},
+ }); err != nil {
+ t.Fatal(err)
+ }
+ state := metrics.AlertState{
+ V: 1, Box: "box-alerting", UpdatedAt: at,
+ Rules: map[string]metrics.RuleState{
+ "disk": {Firing: true, Value: 0.95, FiredAt: at},
+ "lag": {Firing: true, Value: 920, FiredAt: at},
+ },
+ }
+ body, err := json.Marshal(state)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if _, err := fx.rel.Put(ctx, metrics.AlertKey("box-alerting"), body, ""); err != nil {
+ t.Fatal(err)
+ }
+
+ c := signInPaul(t, fx)
+ if err := fx.st.SetOperator("paul@keymail.dev", true); err != nil {
+ t.Fatal(err)
+ }
+ status, _, page := get(t, c, fx.ts.URL+"/fleet/metrics")
+ if status != 200 {
+ t.Fatalf("/fleet/metrics = %d: %s", status, page)
+ }
+
+ alertsAt := strings.Index(page, ">Alerts<")
+ cardAt := strings.Index(page, "<h2>box-alerting</h2>")
+ if alertsAt < 0 {
+ t.Fatalf("no alerts section rendered for a firing box:\n%s", page)
+ }
+ if cardAt < 0 {
+ t.Fatalf("no box card rendered:\n%s", page)
+ }
+ if alertsAt > cardAt {
+ t.Errorf("alerts section renders after the first box card (%d > %d) — firing alerts belong at the top", alertsAt, cardAt)
+ }
+ if !strings.Contains(page, "95%") {
+ t.Errorf("disk alert value not rendered as a percent:\n%s", page)
+ }
+ if !strings.Contains(page, "920s") {
+ t.Errorf("lag alert value not rendered in seconds:\n%s", page)
+ }
+ if strings.Contains(page, "92000%") {
+ t.Errorf("lag alert value rendered as a percentage — lag is seconds:\n%s", page)
+ }
+}
diff --git a/internal/console/templates/metrics.html b/internal/console/templates/metrics.html
index 978724047..b11fcbec6 100644
--- a/internal/console/templates/metrics.html
+++ b/internal/console/templates/metrics.html
@@ -10,7 +10,7 @@
<tr>
<td>{{.Box}}</td>
<td>{{.Rule}}</td>
- <td class="error">{{pctVal .Value}}</td>
+ <td class="error">{{alertVal .Rule .Value}}</td>
<td>{{fmtTime .FiredAt}}</td>
</tr>
{{end}}