From fec18f343f561735852a75b8ee0dbe9477b0372b Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Thu, 27 Aug 2026 21:22:12 +0100 Subject: [PATCH] Preserve fresh rolling-window metric authority Keep in-memory observations authoritative over duplicate persisted history, add direct provider and cache reset coverage, and register the rolling-window proof files with canonical governance.\n\nChange-source: pulse-maintainer --- .../v6/internal/subsystems/monitoring.md | 11 ++-- .../v6/internal/subsystems/registry.json | 7 +- internal/monitoring/metric_window_provider.go | 4 +- .../monitoring/metric_window_provider_test.go | 65 +++++++++++++++++++ .../canonical_completion_guard_test.py | 1 + .../release_control/registry_audit_test.py | 7 ++ .../release_control/subsystem_lookup_test.py | 1 + 7 files changed, 90 insertions(+), 6 deletions(-) create mode 100644 internal/monitoring/metric_window_provider_test.go diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index eb03af73e..1136a132d 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -437,10 +437,13 @@ identity and durable history as charts. `internal/monitoring/metric_window_provi resolves the unified resource metrics target, reads the fresh in-memory tail, and falls back to the SQLite metrics store when that tail lacks the requested coverage. Persistent fallbacks are briefly cached to bound restart-time query -load, merged by timestamp, and returned as observations only; alert policy owns -averaging, readiness, hysteresis, and lifecycle decisions. Missing target, -query failure, shallow history, or gapped history must remain unknown at the -alerts boundary rather than being replaced by a synthetic healthy value. +load, merged by timestamp, and returned as observations only. When the durable +series and fresh in-memory tail contain the same timestamp, the in-memory value +is authoritative so an older persisted or rolled-up value cannot replace the +latest observation. Alert policy owns averaging, readiness, hysteresis, and +lifecycle decisions. Missing target, query failure, shallow history, or gapped +history must remain unknown at the alerts boundary rather than being replaced +by a synthetic healthy value. Monitoring ingest keeps mock mode hermetic. The unified read path already substitutes the mock snapshot wholesale, so anything that runs after that diff --git a/docs/release-control/v6/internal/subsystems/registry.json b/docs/release-control/v6/internal/subsystems/registry.json index 744326178..4e1f938e6 100644 --- a/docs/release-control/v6/internal/subsystems/registry.json +++ b/docs/release-control/v6/internal/subsystems/registry.json @@ -2334,7 +2334,8 @@ "internal/alerts/unified_eval_parity_test.go", "internal/alerts/unified_eval_test.go", "internal/alerts/unified_incident_confirmation_test.go", - "internal/alerts/unified_incidents_test.go" + "internal/alerts/unified_incidents_test.go", + "internal/alerts/windowed_metric_test.go" ] }, { @@ -2540,6 +2541,7 @@ "internal/alerts/callback_config_coverage_test.go", "internal/alerts/canonical_override_migration_test.go", "internal/alerts/canonical_stateful_test.go", + "internal/alerts/config/evaluation_windows_test.go", "internal/alerts/config_validation_test.go", "internal/alerts/eventlog/eventlog_snapshot_test.go", "internal/alerts/external_probe_test.go", @@ -2564,6 +2566,7 @@ "internal/alerts/threshold_resolution_shared_test.go", "internal/alerts/unified_incident_confirmation_test.go", "internal/alerts/update_alerts_test.go", + "internal/alerts/windowed_metric_test.go", "internal/monitoring/deadman_test.go", "internal/monitoring/monitor_alert_override_migration_test.go" ] @@ -5715,6 +5718,7 @@ "allow_same_subsystem_tests": false, "test_prefixes": [], "exact_files": [ + "internal/monitoring/metric_window_provider_test.go", "internal/monitoring/metrics_history_concurrency_test.go", "internal/monitoring/metrics_history_memory_regression_test.go", "internal/monitoring/metrics_history_test.go", @@ -6055,6 +6059,7 @@ "internal/monitoring/issue1595_collection_trust_test.go", "internal/monitoring/issue1613_contract_test.go", "internal/monitoring/issue1638_dns_cache_test.go", + "internal/monitoring/metric_window_provider_test.go", "internal/monitoring/monitor_additional_test.go", "internal/monitoring/monitor_alert_intent_test.go", "internal/monitoring/monitor_alert_override_migration_test.go", diff --git a/internal/monitoring/metric_window_provider.go b/internal/monitoring/metric_window_provider.go index 70c6fbe0f..339c94b62 100644 --- a/internal/monitoring/metric_window_provider.go +++ b/internal/monitoring/metric_window_provider.go @@ -58,7 +58,9 @@ func (m *Monitor) metricWindowPoints(request alerts.MetricWindowRequest) ([]aler points := m.inMemoryMetricWindow(resourceType, resourceID, metric, duration) if metricWindowCoverage(points) < duration*8/10 { stored := m.persistentMetricWindow(resourceType, resourceID, metric, request.Start, request.End) - points = mergeMetricWindowPoints(points, stored, request.Start, request.End) + // Append the fresh in-memory tail last so it remains authoritative when + // SQLite contains an older value at the same timestamp. + points = mergeMetricWindowPoints(stored, points, request.Start, request.End) } result := make([]alerts.MetricWindowPoint, 0, len(points)) diff --git a/internal/monitoring/metric_window_provider_test.go b/internal/monitoring/metric_window_provider_test.go new file mode 100644 index 000000000..d0e012cde --- /dev/null +++ b/internal/monitoring/metric_window_provider_test.go @@ -0,0 +1,65 @@ +package monitoring + +import ( + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/alerts" +) + +func TestMetricWindowPointsUsesInMemoryMetricAlias(t *testing.T) { + now := time.Now().UTC() + history := NewMetricsHistory(32, time.Hour) + history.AddGuestMetric("vm-1", "netin", 12, now.Add(-4*time.Minute)) + history.AddGuestMetric("vm-1", "netin", 18, now.Add(-2*time.Minute)) + monitor := &Monitor{metricsHistory: history} + + points, err := monitor.metricWindowPoints(alerts.MetricWindowRequest{ + ResourceID: "vm-1", + ResourceType: "vm", + Metric: "networkIn", + Start: now.Add(-5 * time.Minute), + End: now, + }) + if err != nil { + t.Fatalf("metricWindowPoints returned error: %v", err) + } + if len(points) != 2 || points[0].Value != 12 || points[1].Value != 18 { + t.Fatalf("metricWindowPoints = %+v, want canonical netin history", points) + } +} + +func TestMetricWindowMergePrefersFreshInMemoryDuplicate(t *testing.T) { + now := time.Now().UTC() + stored := []MetricPoint{ + {Timestamp: now.Add(-4 * time.Minute), Value: 40}, + {Timestamp: now.Add(-2 * time.Minute), Value: 99}, + } + inMemory := []MetricPoint{ + {Timestamp: now.Add(-2 * time.Minute), Value: 55}, + {Timestamp: now.Add(-time.Minute), Value: 60}, + } + + got := mergeMetricWindowPoints(stored, inMemory, now.Add(-5*time.Minute), now) + if len(got) != 3 { + t.Fatalf("mergeMetricWindowPoints returned %d points, want 3: %+v", len(got), got) + } + if got[1].Value != 55 { + t.Fatalf("duplicate timestamp value = %.1f, want fresh in-memory value 55", got[1].Value) + } +} + +func TestMetricsHistoryResetClearsMetricWindowCache(t *testing.T) { + history := NewMetricsHistory(32, time.Hour) + history.metricWindowCache = map[string]metricWindowCacheEntry{ + "vm\x00vm-1\x00cpu\x00300": { + points: []alerts.MetricWindowPoint{{Timestamp: time.Now().UTC(), Value: 42}}, + expiresAt: time.Now().Add(time.Minute), + }, + } + + history.Reset() + if history.metricWindowCache != nil { + t.Fatalf("metric window cache survived reset: %+v", history.metricWindowCache) + } +} diff --git a/scripts/release_control/canonical_completion_guard_test.py b/scripts/release_control/canonical_completion_guard_test.py index 90e1c3dc5..50a1b7819 100644 --- a/scripts/release_control/canonical_completion_guard_test.py +++ b/scripts/release_control/canonical_completion_guard_test.py @@ -304,6 +304,7 @@ class CanonicalCompletionGuardTest(unittest.TestCase): "internal/monitoring/issue1595_collection_trust_test.go", "internal/monitoring/issue1613_contract_test.go", "internal/monitoring/issue1638_dns_cache_test.go", + "internal/monitoring/metric_window_provider_test.go", "internal/monitoring/monitor_additional_test.go", "internal/monitoring/monitor_alert_intent_test.go", "internal/monitoring/monitor_alert_override_migration_test.go", diff --git a/scripts/release_control/registry_audit_test.py b/scripts/release_control/registry_audit_test.py index 83cab9190..4241d97ac 100644 --- a/scripts/release_control/registry_audit_test.py +++ b/scripts/release_control/registry_audit_test.py @@ -87,10 +87,13 @@ class RegistryAuditTest(unittest.TestCase): }, ("alerts", "canonical-alert-runtime"): { "internal/alerts/intent_policy_test.go", + "internal/alerts/windowed_metric_test.go", }, ("alerts", "alerts-runtime-support"): { "internal/alerts/canonical_override_migration_test.go", + "internal/alerts/config/evaluation_windows_test.go", "internal/alerts/intent_policy_test.go", + "internal/alerts/windowed_metric_test.go", "internal/monitoring/monitor_alert_override_migration_test.go", }, ("api-contracts", "alert-intent-policy-api"): { @@ -126,9 +129,13 @@ class RegistryAuditTest(unittest.TestCase): ("monitoring", "monitoring-runtime"): { "internal/monitoring/availability_udp_test.go", "internal/monitoring/issue1595_collection_trust_test.go", + "internal/monitoring/metric_window_provider_test.go", "internal/monitoring/monitor_alert_intent_test.go", "internal/monitoring/monitor_alert_override_migration_test.go", }, + ("monitoring", "metrics-history-runtime"): { + "internal/monitoring/metric_window_provider_test.go", + }, ("monitoring", "agent-fleet-diagnostics-runtime"): { "internal/api/agent_fleet_doctor_test.go", "internal/api/connections_aggregator_test.go", diff --git a/scripts/release_control/subsystem_lookup_test.py b/scripts/release_control/subsystem_lookup_test.py index 9f2d7a436..47df008d3 100644 --- a/scripts/release_control/subsystem_lookup_test.py +++ b/scripts/release_control/subsystem_lookup_test.py @@ -4230,6 +4230,7 @@ class SubsystemLookupTest(unittest.TestCase): self.assertEqual( match["verification_requirement"]["exact_files"], [ + "internal/monitoring/metric_window_provider_test.go", "internal/monitoring/metrics_history_concurrency_test.go", "internal/monitoring/metrics_history_memory_regression_test.go", "internal/monitoring/metrics_history_test.go",