amadan

carlos / platform Public

metrics: correct RuleState's false omitempty claim, pin real JSON wire shape

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell 7167ba8ea91808afe9673ac0ca483280ce593016 parent ff5360a
2 files changed, +50 −2
  • internal/metrics/alert.go +9 −2
  • internal/metrics/alert_test.go +41 −0
diff --git a/internal/metrics/alert.go b/internal/metrics/alert.go
index 8f1f4924a..86979d46a 100644
--- a/internal/metrics/alert.go
+++ b/internal/metrics/alert.go
@@ -75,8 +75,15 @@ type Observation struct {
}
// RuleState is one rule's sustain/hysteresis bookkeeping. Since and FiredAt
-// use omitempty so a never-triggered rule round-trips through JSON as a
-// bare {"firing":false} rather than carrying zero-value time.Time noise.
+// carry `omitempty`, but encoding/json's omitempty never treats a struct
+// (time.Time is a struct) as empty regardless of its zero value — so a
+// never-triggered RuleState does NOT drop these keys. It marshals as
+// {"firing":false,"since":"0001-01-01T00:00:00Z","fired_at":"0001-01-01T00:00:00Z"},
+// both fields always present. The round-trip is still lossless (the zero
+// time reproduces exactly on Unmarshal), so this is a documentation note,
+// not a bug: any consumer of this wire shape (the agent loop's persistence,
+// the console watcher) must treat the zero time value itself — not key
+// absence — as "never set". See TestRuleStateJSONRoundTrip.
type RuleState struct {
Firing bool `json:"firing"`
Since time.Time `json:"since,omitempty"` // continuously-above-threshold since (sustain tracking)
diff --git a/internal/metrics/alert_test.go b/internal/metrics/alert_test.go
index 19c025959..60e0acd12 100644
--- a/internal/metrics/alert_test.go
+++ b/internal/metrics/alert_test.go
@@ -1,10 +1,51 @@
package metrics
import (
+ "encoding/json"
"testing"
"time"
)
+// TestRuleStateJSONRoundTrip pins the true wire shape: `omitempty` on a
+// time.Time field is a no-op in encoding/json (structs are never "empty"),
+// so a zero RuleState's since/fired_at keys are always present, carrying
+// the zero-time string — not omitted. Round-tripping is still lossless for
+// both a zero and a non-zero RuleState, which is what consumers (the agent
+// loop's persistence, the console watcher) actually depend on.
+func TestRuleStateJSONRoundTrip(t *testing.T) {
+ zero := RuleState{Firing: false}
+ b, err := json.Marshal(zero)
+ if err != nil {
+ t.Fatalf("marshal zero RuleState: %v", err)
+ }
+ const wantZero = `{"firing":false,"since":"0001-01-01T00:00:00Z","fired_at":"0001-01-01T00:00:00Z"}`
+ if string(b) != wantZero {
+ t.Fatalf("zero RuleState wire shape changed:\n got: %s\n want: %s", b, wantZero)
+ }
+ var gotZero RuleState
+ if err := json.Unmarshal(b, &gotZero); err != nil {
+ t.Fatalf("unmarshal zero RuleState: %v", err)
+ }
+ if !gotZero.Since.Equal(zero.Since) || !gotZero.FiredAt.Equal(zero.FiredAt) || gotZero != zero {
+ t.Fatalf("zero RuleState did not round-trip: got %+v want %+v", gotZero, zero)
+ }
+
+ since := time.Date(2026, 8, 2, 10, 0, 0, 0, time.UTC)
+ firedAt := since.Add(10 * time.Minute)
+ nonZero := RuleState{Firing: true, Since: since, FiredAt: firedAt, Value: 0.95}
+ b, err = json.Marshal(nonZero)
+ if err != nil {
+ t.Fatalf("marshal non-zero RuleState: %v", err)
+ }
+ var gotNonZero RuleState
+ if err := json.Unmarshal(b, &gotNonZero); err != nil {
+ t.Fatalf("unmarshal non-zero RuleState: %v", err)
+ }
+ if !gotNonZero.Since.Equal(nonZero.Since) || !gotNonZero.FiredAt.Equal(nonZero.FiredAt) || gotNonZero.Firing != nonZero.Firing || gotNonZero.Value != nonZero.Value {
+ t.Fatalf("non-zero RuleState did not round-trip: got %+v want %+v", gotNonZero, nonZero)
+ }
+}
+
func TestEvaluateMemSustainAndHysteresis(t *testing.T) {
r := DefaultRules()
st := AlertState{V: 1, Box: "b", Rules: map[string]RuleState{}}