amadan

carlos / platform Public

geoip: alert mail tracks the last reason told per recipient; a confirmed reason is reported even when the feed is unreadable (Astra code review R5)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude c24b64d29a28c40046192ee6d6bb07bf4a26f8f1 parent 9cd8e78
4 files changed, +48 −3
  • internal/console/alertwatch.go +9 −2
  • internal/console/alertwatch_test.go +6 −0
  • internal/edge/geoipmirror.go +10 −1
  • internal/edge/geoipmirror_test.go +23 −0

CI passed — run details

diff --git a/internal/console/alertwatch.go b/internal/console/alertwatch.go
index c7a643b3b..afcd77f21 100644
--- a/internal/console/alertwatch.go
+++ b/internal/console/alertwatch.go
@@ -236,10 +236,17 @@ func (s *Server) alertDueFor(box, rule, address string, rs metrics.RuleState) bo
return due
}
+// recordAlertSeenFor records the transition and, for geoip, which reason
+// this address was LAST told about: exactly one reason row is true while
+// firing and none after a clear, so a reason seen before (2, 3, back to 2,
+// or a later episode) is news again (code review R5 on R4-3).
func (s *Server) recordAlertSeenFor(box, rule, address string, rs metrics.RuleState) {
s.recordAlertSeen(box, rule, address, rs.Firing)
- if rule == "geoip" && rs.Firing {
- s.recordAlertSeen(box, geoipReasonRule(rs.Value), address, true)
+ if rule != "geoip" {
+ return
+ }
+ for code := 1; code <= 5; code++ {
+ s.recordAlertSeen(box, geoipReasonRule(float64(code)), address, rs.Firing && int(rs.Value) == code)
}
}
diff --git a/internal/console/alertwatch_test.go b/internal/console/alertwatch_test.go
index 100406c86..9fd983d42 100644
--- a/internal/console/alertwatch_test.go
+++ b/internal/console/alertwatch_test.go
@@ -343,4 +343,10 @@ func TestAlertPassMailsAGeoIPReasonChange(t *testing.T) {
step(geoipAlert(2, true), 0, "")
step(geoipAlert(3, true), 1, "expired and removed")
step(geoipAlert(0, false), 1, "")
+ // A later episode, and a return to an earlier reason within one: each
+ // is news (code review R5 on R4-3).
+ step(geoipAlert(2, true), 1, "expires within 9 days")
+ step(geoipAlert(3, true), 1, "expired and removed")
+ step(geoipAlert(2, true), 1, "expires within 9 days")
+ step(geoipAlert(2, true), 0, "")
}
diff --git a/internal/edge/geoipmirror.go b/internal/edge/geoipmirror.go
index 5a1b008cb..3420aff1e 100644
--- a/internal/edge/geoipmirror.go
+++ b/internal/edge/geoipmirror.go
@@ -551,13 +551,22 @@ const (
func (m *geoMirror) AlertCode(now time.Time, box string) (code float64, known bool) {
m.genMu.Lock()
defer m.genMu.Unlock()
+ // A reason established from what IS known is reported even when the
+ // other input is not (code review R5-1): an unreadable feed.json must
+ // not hide a confirmed expiry. Only "healthy" needs everything known.
+ if c := m.alertCodeLocked(now, box); c != GeoAlertNone {
+ return c, true
+ }
if !m.pointerKnown || !m.feedKnown {
return GeoAlertNone, false
}
- return m.alertCodeLocked(now, box), true
+ return GeoAlertNone, true
}
func (m *geoMirror) alertCodeLocked(now time.Time, box string) float64 {
+ // pointerPresent is only ever true after a successful read, and
+ // feedFailing only after a parsed feed.json, so each reason below rests
+ // on state that was actually read.
expired := m.pointerPresent && (m.pointerMarked || geoip.Expired(m.pointerType, m.pointerBuild, now))
switch {
case expired:
diff --git a/internal/edge/geoipmirror_test.go b/internal/edge/geoipmirror_test.go
index 85cd35a72..7f3b385e4 100644
--- a/internal/edge/geoipmirror_test.go
+++ b/internal/edge/geoipmirror_test.go
@@ -15,6 +15,7 @@ import (
"github.com/carlosframework/platform/internal/geo"
"github.com/carlosframework/platform/internal/geoip"
+ "github.com/carlosframework/platform/internal/metrics"
"github.com/carlosframework/platform/internal/release"
)
@@ -537,3 +538,25 @@ func TestMirrorAlertUnknownUntilStateIsRead(t *testing.T) {
t.Fatalf("after both reads = (%v, %v)", c, known)
}
}
+
+// Code review R5-1: a confirmed expiry is reported even when feed.json is
+// unreadable, through the evaluator.
+func TestMirrorExpiryReportedWithAnUnreadableFeed(t *testing.T) {
+ now := time.Date(2026, 10, 6, 0, 0, 0, 0, time.UTC)
+ m, s, _ := newTestMirror(t)
+ m.now = func() time.Time { return now }
+ s.Put(context.Background(), geoip.FeedKey, []byte("garbled"), "")
+ publishFixture(t, s, geoFixture(t, "GeoIP2-City-Test.mmdb"), "GeoLite2-City", now.Add(-31*24*time.Hour))
+ m.Poll(context.Background())
+ c, known := m.AlertCode(now, "box")
+ if !known || c != GeoAlertExpired {
+ t.Fatalf("expired with an unreadable feed = (%v, %v), want (3, true)", c, known)
+ }
+ r := metrics.DefaultRules()
+ st := metrics.AlertState{V: 1, Box: "box"}
+ st, _ = metrics.Evaluate(r, st, metrics.Observation{At: now, GeoIP: &c})
+ _, ev := metrics.Evaluate(r, st, metrics.Observation{At: now.Add(30 * time.Minute), GeoIP: &c})
+ if len(ev) != 1 || ev[0].Value != GeoAlertExpired {
+ t.Fatalf("evaluator = %v", ev)
+ }
+}