mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
fix(ai): backport retained incident occurrence projection bounds
Backport of a41c60597b. Named candidate regression: resolved incident reads reopen after recurrence. Both 2m and 500ms dispatcher baselines fail on 601d061; focused incident and dispatcher tests pass with race count=20 after repair. No persistence optimisation, duplicate migration or release-readiness claim.
Change-source: pulse-maintainer
This commit is contained in:
@@ -0,0 +1,36 @@
|
||||
# Retained incident occurrence projection backport
|
||||
|
||||
Named regression on supplied release/v6.4 base
|
||||
601d061dadcba23410b0b251bcb3736ab4fffe5a: canonical timeline reads absorb
|
||||
later recurrences, making an older resolved incident appear open. Release
|
||||
Train rules 2/4 permit this corrective backport of Core
|
||||
a41c60597b3f1282803bedba7b2cba16f04b6357. It is distinct from the
|
||||
already-present shell identity repair. Runtime delta is identical to Core;
|
||||
the monitoring test additionally imports testify/require on this line.
|
||||
|
||||
Baseline with new tests:
|
||||
- CanonicalProjectionOccurrenceBounds fails at both 2m and 500ms.
|
||||
- Dispatcher initially failed compilation (missing test-only require import);
|
||||
after adding that import it fails expected resolved / actual open at both
|
||||
timings. No runtime change was present for either baseline.
|
||||
|
||||
Focused repaired verification command:
|
||||
go test -race ./internal/ai/memory ./internal/monitoring -run '^TestIncident|^TestMonitorLifecycleReplayPreservesOccurrenceTimelines$' -count=20
|
||||
|
||||
Logs are retained beside the lane outcome in
|
||||
/var/lib/pulse-maintainer/queue/staging/20260907T221516Z-release-line/
|
||||
(baseline.log, dispatcher-baseline.log, repaired.log); the coordinator may
|
||||
archive this directory. These tests use in-memory canonical stores.
|
||||
|
||||
Scope: retained-shell reads are bounded by exact start and next retained
|
||||
start for the same alert/resource; historical acknowledgement stays separate.
|
||||
No shell-less/evicted-history repair, duplicate migration, write-byte reduction,
|
||||
installed restart or recipient-delivery acceptance is asserted. Issue #1966
|
||||
remains unresolved. No new feature or persistence optimisation is included.
|
||||
|
||||
This is source backport evidence, not selected-snapshot qualification. Normal
|
||||
protected review and steward version/candidate disposition remain required;
|
||||
do not substitute this head for an immutable selected release. Preserve prior
|
||||
failed latency qualification and excluded crash evidence. No release approval.
|
||||
|
||||
Result: PASS, memory 7.117s and monitoring 4.444s, race-enabled count=20.
|
||||
@@ -36,6 +36,19 @@ make acknowledgement event replay idempotent, or prove aggregate write-byte
|
||||
reductions or recipient delivery. An evicted occurrence can still be recreated
|
||||
by replay; exact identity is guaranteed only while its shell is retained.
|
||||
|
||||
Canonical timeline reads for retained incident shells select events from the
|
||||
shell's exact opening time up to (but not including) the next retained opening
|
||||
for the same alert/resource. A newer firing must not reopen an older resolved
|
||||
incident, and a subsecond recurrence must not inherit its predecessor's
|
||||
resolution or acknowledgement. Local analysis annotations remain attached to
|
||||
their shell. This is a read-projection boundary, not event deletion or a change
|
||||
to notification delivery. Canonical-only fallback when no shell is retained,
|
||||
missing recurrence boundaries after retention, and already-duplicated shells
|
||||
remain outside this guarantee. Legacy shell lookup retains its time tolerance;
|
||||
events preceding the selected shell's exact opening are not projected into it.
|
||||
`TestIncidentStore_CanonicalProjectionOccurrenceBounds` verifies both boundaries,
|
||||
subsecond starts, unordered successors and unrelated alert/resource isolation.
|
||||
|
||||
|
||||
Incident timeline summaries must distinguish provider conditions from metric
|
||||
threshold evidence. For `resource-incident` alert events, numeric value and
|
||||
|
||||
@@ -1011,6 +1011,20 @@ func (s *IncidentStore) loadProjectedIncidentEvents(incident *Incident, timeline
|
||||
since = since.Add(-incidentStartMatchTolerance)
|
||||
}
|
||||
|
||||
// Retained shells identify exact occurrences. Canonical history is keyed by
|
||||
// alert identifier, so without an upper bound an old incident absorbs every
|
||||
// later recurrence and can appear open again. Do not use the legacy read
|
||||
// tolerance to include events from a preceding subsecond occurrence either.
|
||||
var nextStart time.Time
|
||||
s.mu.RLock()
|
||||
for _, shell := range s.incidents {
|
||||
if shell != nil && shell.AlertIdentifier == alertIdentifier && shell.ResourceID == resourceID &&
|
||||
shell.OpenedAt.After(incident.OpenedAt) && (nextStart.IsZero() || shell.OpenedAt.Before(nextStart)) {
|
||||
nextStart = shell.OpenedAt
|
||||
}
|
||||
}
|
||||
s.mu.RUnlock()
|
||||
|
||||
changes, err := timelineStore.GetRecentChanges(resourceID, since, projectedIncidentChangeLimit)
|
||||
if err != nil || len(changes) == 0 {
|
||||
return nil
|
||||
@@ -1025,6 +1039,10 @@ func (s *IncidentStore) loadProjectedIncidentEvents(incident *Incident, timeline
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
if (!incident.OpenedAt.IsZero() && event.Timestamp.Before(incident.OpenedAt)) ||
|
||||
(!nextStart.IsZero() && !event.Timestamp.Before(nextStart)) {
|
||||
continue
|
||||
}
|
||||
events = append(events, event)
|
||||
hydrateIncidentFromCanonicalChange(incident, change)
|
||||
}
|
||||
|
||||
@@ -1152,3 +1152,59 @@ func TestIncidentStore_LifecycleRapidRecurrence(t *testing.T) {
|
||||
t.Fatal("old resolution closed rapid recurrence")
|
||||
}
|
||||
}
|
||||
|
||||
func TestIncidentStore_CanonicalProjectionOccurrenceBounds(t *testing.T) {
|
||||
for _, gap := range []time.Duration{2 * time.Minute, 500 * time.Millisecond} {
|
||||
t.Run(gap.String(), func(t *testing.T) {
|
||||
store := NewIncidentStore(IncidentStoreConfig{})
|
||||
canonical := unifiedresources.NewMemoryStore()
|
||||
store.SetResourceTimelineStore(canonical)
|
||||
start := time.Now().UTC().Add(-time.Hour)
|
||||
old := &alerts.Alert{ID: "alert-bounds", ResourceID: "pbs", StartTime: start}
|
||||
store.RecordAlertFired(old)
|
||||
// Deliberately insert successors out of time order. The earliest
|
||||
// matching next start, not insertion order, is the upper boundary.
|
||||
for _, offset := range []time.Duration{3 * gap, gap} {
|
||||
next := old.Clone()
|
||||
next.StartTime = start.Add(offset)
|
||||
store.RecordAlertFired(next)
|
||||
}
|
||||
// Neither another alert nor another resource may shorten the window.
|
||||
other := old.Clone()
|
||||
other.ID = "unrelated-alert"
|
||||
other.StartTime = start.Add(gap / 4)
|
||||
store.RecordAlertFired(other)
|
||||
other.ID = old.ID
|
||||
other.ResourceID = "other-resource"
|
||||
other.StartTime = start.Add(gap / 3)
|
||||
store.RecordAlertFired(other)
|
||||
|
||||
end := start.Add(gap - time.Nanosecond)
|
||||
for _, event := range []struct {
|
||||
at time.Time
|
||||
kind unifiedresources.ChangeKind
|
||||
}{
|
||||
{start.Add(-time.Nanosecond), unifiedresources.ChangeAlertResolved},
|
||||
{start, unifiedresources.ChangeAlertFired},
|
||||
{end, unifiedresources.ChangeAlertResolved},
|
||||
{start.Add(gap), unifiedresources.ChangeAlertFired},
|
||||
{start.Add(2 * gap), unifiedresources.ChangeAlertAcknowledged},
|
||||
} {
|
||||
change := unifiedresources.BuildAlertTimelineChange(old.ResourceID, event.kind, event.at, "", unifiedresources.AlertTimelineChange{AlertIdentifier: old.ID})
|
||||
if err := canonical.RecordChange(*change); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
projected := store.GetTimelineByAlertAt(old.ID, start)
|
||||
if projected == nil || len(projected.Events) != 2 {
|
||||
t.Fatalf("expected only this occurrence's fired/resolved events, got %+v", projected)
|
||||
}
|
||||
if !projected.Events[0].Timestamp.Equal(start) || !projected.Events[1].Timestamp.Equal(end) {
|
||||
t.Fatalf("wrong inclusive lower/exclusive upper boundary: %+v", projected.Events)
|
||||
}
|
||||
if projected.Status != IncidentStatusResolved || projected.ClosedAt == nil || !projected.ClosedAt.Equal(end) || projected.Acknowledged {
|
||||
t.Fatalf("another occurrence changed historical state: %+v", projected)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -9,6 +9,8 @@ import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/ai/memory"
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/alerts"
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/alerts/eventlog"
|
||||
@@ -988,3 +990,76 @@ func TestMonitor_HandleAlertEscalated_BypassesDeliveryCooldown(t *testing.T) {
|
||||
t.Fatal("expected escalated notification delivery despite active cooldown")
|
||||
}
|
||||
}
|
||||
|
||||
// Exercise the dispatcher used by lifecycle replay with both projections
|
||||
// attached. Store-only tests cannot detect cross-occurrence canonical history.
|
||||
func TestMonitorLifecycleReplayPreservesOccurrenceTimelines(t *testing.T) {
|
||||
for _, gap := range []time.Duration{2 * time.Minute, 500 * time.Millisecond} {
|
||||
t.Run(gap.String(), func(t *testing.T) {
|
||||
store := unifiedresources.NewMemoryStore()
|
||||
incidents := memory.NewIncidentStore(memory.IncidentStoreConfig{})
|
||||
m := &Monitor{
|
||||
incidentStore: incidents,
|
||||
resourceStore: unifiedresources.NewMonitorAdapter(unifiedresources.NewRegistry(store)),
|
||||
}
|
||||
incidents.SetResourceTimelineStore(m.resourceStore.(memory.IncidentTimelineStore))
|
||||
start := time.Now().UTC().Add(-time.Hour).Truncate(time.Second)
|
||||
old := &alerts.Alert{ID: "pbs-connectivity", ResourceID: "pbs", StartTime: start}
|
||||
end := start.Add(gap / 2)
|
||||
fired := alerts.LifecycleEvent{Type: eventlog.TypeFired, OccurredAt: start, Alert: old}
|
||||
resolved := alerts.LifecycleEvent{Type: eventlog.TypeResolved, OccurredAt: end, Alert: old}
|
||||
m.handleAlertLifecycleEvent(fired)
|
||||
m.handleAlertLifecycleEvent(resolved)
|
||||
first := incidents.GetTimelineByAlertAt(old.ID, start)
|
||||
require.NotNil(t, first)
|
||||
for i := 0; i < 10; i++ {
|
||||
m.handleAlertLifecycleEvent(fired)
|
||||
m.handleAlertLifecycleEvent(resolved)
|
||||
}
|
||||
require.Len(t, incidents.ListIncidentsByResource(old.ResourceID, 0), 1)
|
||||
replayed := incidents.GetTimelineByAlertAt(old.ID, start)
|
||||
require.Equal(t, first.ID, replayed.ID)
|
||||
require.Equal(t, memory.IncidentStatusResolved, replayed.Status)
|
||||
require.Len(t, replayed.Events, 2)
|
||||
|
||||
next := old.Clone()
|
||||
next.StartTime = start.Add(gap)
|
||||
m.handleAlertLifecycleEvent(alerts.LifecycleEvent{Type: eventlog.TypeFired, OccurredAt: next.StartTime, Alert: next})
|
||||
// A historical resolution arriving after recurrence must not close it;
|
||||
// conversely, the recurrence must not reopen the historical timeline.
|
||||
m.handleAlertLifecycleEvent(resolved)
|
||||
require.Len(t, incidents.ListIncidentsByResource(old.ResourceID, 0), 2)
|
||||
historical := incidents.GetTimelineByAlertAt(old.ID, start)
|
||||
current := incidents.GetTimelineByAlertAt(next.ID, next.StartTime)
|
||||
require.NotNil(t, historical)
|
||||
require.NotNil(t, current)
|
||||
require.Equal(t, first.ID, historical.ID)
|
||||
require.NotEqual(t, historical.ID, current.ID)
|
||||
require.Equal(t, memory.IncidentStatusResolved, historical.Status)
|
||||
require.Equal(t, &end, historical.ClosedAt)
|
||||
require.Len(t, historical.Events, 2)
|
||||
require.Equal(t, memory.IncidentStatusOpen, current.Status)
|
||||
require.Nil(t, current.ClosedAt)
|
||||
require.Len(t, current.Events, 1)
|
||||
changes, err := store.GetRecentChanges(old.ResourceID, time.Time{}, 100)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, changes, 3)
|
||||
|
||||
ackAt := start.Add(gap / 4)
|
||||
old.AckTime = &ackAt
|
||||
m.handleAlertLifecycleEvent(alerts.LifecycleEvent{
|
||||
Type: eventlog.TypeAcknowledged, OccurredAt: ackAt, Alert: old,
|
||||
Details: map[string]string{"user": "historical-operator"},
|
||||
})
|
||||
historical = incidents.GetTimelineByAlertAt(old.ID, start)
|
||||
current = incidents.GetTimelineByAlertAt(next.ID, next.StartTime)
|
||||
require.Equal(t, memory.IncidentStatusResolved, historical.Status)
|
||||
require.True(t, historical.Acknowledged)
|
||||
require.Equal(t, "historical-operator", historical.AckUser)
|
||||
require.Len(t, historical.Events, 3)
|
||||
require.False(t, current.Acknowledged)
|
||||
require.Equal(t, memory.IncidentStatusOpen, current.Status)
|
||||
require.Len(t, current.Events, 1)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user