diff --git a/docs/requirements/Component-HealthMonitoring.md b/docs/requirements/Component-HealthMonitoring.md index 6b612d4c..1d58ee2e 100644 --- a/docs/requirements/Component-HealthMonitoring.md +++ b/docs/requirements/Component-HealthMonitoring.md @@ -44,7 +44,7 @@ Site clusters (metric collection and reporting). Central cluster (aggregation an - Each report is a **flat snapshot** containing the current values of all monitored metrics, a **monotonic sequence number**, and the **report timestamp** from the site. Central replaces the previous state for that site only if the incoming sequence number is higher than the last received — this prevents stale reports (e.g., delayed in transit or from a pre-failover node) from overwriting newer state. - Central tracks two **distinct** signals per site — do not conflate them: - **Liveness (online/offline)**: driven by the **last signal of any kind** (a full report *or* a transport heartbeat, the latter arriving every ~5s from any reachable site node). If central receives no signal within `OfflineTimeout` (default: **60 seconds**), the site is marked **offline**. Because heartbeats are frequent, this only fires on genuine total loss of contact — not during a single-node failover, when the surviving node keeps heartbeating. The synthetic *central* self-report site has no heartbeat source and uses the longer `CentralOfflineTimeout` (default: **3 minutes**) so a single missed self-report does not flap it offline. - - **Metrics staleness**: an **online** site (heartbeats still arriving) that has produced no full **report** within `MetricsStaleTimeout` (default: **2 minutes**) is flagged **metrics stale**. This surfaces the failure mode where a site's report pipeline (`HealthReportSender`) has died but the node is otherwise reachable — previously such a site showed "online with frozen metrics forever". `MetricsStaleTimeout` must be positive and no shorter than the report interval. + - **Metrics staleness**: an **online** site (heartbeats still arriving) that has produced no full **report** within `MetricsStaleTimeout` (default: **2 minutes**) is flagged **metrics stale**. This surfaces the failure mode where a site's report pipeline (`HealthReportSender`) has died but the node is otherwise reachable — previously such a site showed "online with frozen metrics forever". A site that has **never** delivered a report is anchored on its first-contact time (`FirstSeenAt`), so a report pipeline dead from first boot is also flagged after `MetricsStaleTimeout` (previously such a site showed online-with-no-metrics indefinitely). `MetricsStaleTimeout` must be positive and no shorter than the report interval. - **Online recovery**: when central receives a health report from a site that was marked offline, the site is automatically marked **online** and the metrics-stale flag cleared (a fresh report proves the pipeline is alive); a heartbeat alone also restores **online** but leaves the metrics-stale flag untouched. No manual acknowledgment required. - **Status-transition timestamp**: each online↔offline flip stamps `LastStatusChangeAt`, so the UI can answer "since when has this site been offline/online" rather than only "is it offline now". Report/heartbeat traffic that leaves the status unchanged does not move it. diff --git a/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/CentralHealthAggregator.cs b/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/CentralHealthAggregator.cs index 1cd236e1..811a6e5f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/CentralHealthAggregator.cs +++ b/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/CentralHealthAggregator.cs @@ -47,6 +47,7 @@ public class CentralHealthAggregator : BackgroundService, ICentralHealthAggregat LatestReport = report, LastReportReceivedAt = now, LastHeartbeatAt = now, + FirstSeenAt = now, LastSequenceNumber = report.SequenceNumber, IsOnline = true, IsMetricsStale = false @@ -117,6 +118,7 @@ public class CentralHealthAggregator : BackgroundService, ICentralHealthAggregat LatestReport = null, LastReportReceivedAt = null, LastHeartbeatAt = receivedAt, + FirstSeenAt = receivedAt, LastSequenceNumber = 0, IsOnline = true }; @@ -297,16 +299,22 @@ public class CentralHealthAggregator : BackgroundService, ICentralHealthAggregat // when no report has arrived within MetricsStaleTimeout. Distinct from // offline — a site whose HealthReportSender crashed keeps heartbeating // and would otherwise show "online with frozen metrics forever". - if (state.LastReportReceivedAt is { } lastReport - && now - lastReport > _options.MetricsStaleTimeout + // A site that has NEVER reported (heartbeat-only registration, + // LastReportReceivedAt == null) anchors on FirstSeenAt instead — + // otherwise a pipeline dead from first boot is never flagged (N3). + var reportAnchor = state.LastReportReceivedAt ?? state.FirstSeenAt; + if (reportAnchor is { } anchor + && now - anchor > _options.MetricsStaleTimeout && !state.IsMetricsStale) { var stale = state with { IsMetricsStale = true }; if (_siteStates.TryUpdate(kvp.Key, stale, state)) { _logger.LogWarning( - "Site {SiteId} metrics are stale — online (heartbeats) but no report for {Elapsed}s (timeout: {Timeout}s)", - state.SiteId, (now - lastReport).TotalSeconds, _options.MetricsStaleTimeout.TotalSeconds); + "Site {SiteId} metrics are stale — online (heartbeats) but no report for {Elapsed}s{NeverNote} (timeout: {Timeout}s)", + state.SiteId, (now - anchor).TotalSeconds, + state.LastReportReceivedAt is null ? " (never reported since first contact)" : string.Empty, + _options.MetricsStaleTimeout.TotalSeconds); } // CAS loss ⇒ a fresh report/heartbeat swapped in ⇒ correct to skip. } diff --git a/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/SiteHealthState.cs b/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/SiteHealthState.cs index 3ef9c184..7767aef2 100644 --- a/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/SiteHealthState.cs +++ b/src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/SiteHealthState.cs @@ -40,6 +40,17 @@ public sealed record SiteHealthState /// public DateTimeOffset LastHeartbeatAt { get; init; } + /// + /// Gets the instant this site was first observed by the aggregator (via a + /// report or a heartbeat); null only for states not built by the + /// aggregator (hand-constructed fixtures). Anchors metrics-staleness for a + /// site that has NEVER delivered a report (review 01 round-2 N3): with + /// null the staleness check previously + /// skipped the site forever. cannot anchor + /// this — every heartbeat advances it. + /// + public DateTimeOffset? FirstSeenAt { get; init; } + /// Gets the sequence number of the last accepted health report, used to reject out-of-order duplicates. public long LastSequenceNumber { get; init; } /// Gets a value indicating whether the site is currently considered online. diff --git a/tests/ZB.MOM.WW.ScadaBridge.HealthMonitoring.Tests/CentralHealthAggregatorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.HealthMonitoring.Tests/CentralHealthAggregatorTests.cs index bc340ef6..25c3f8fb 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.HealthMonitoring.Tests/CentralHealthAggregatorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.HealthMonitoring.Tests/CentralHealthAggregatorTests.cs @@ -513,4 +513,35 @@ public class CentralHealthAggregatorTests aggregator.MarkHeartbeat("site-a", time.GetUtcNow()); Assert.NotEqual(offlineAt, aggregator.GetSiteState("site-a")!.LastStatusChangeAt); } + + /// + /// Round-2 N3: a site known only via heartbeats (LastReportReceivedAt == null) + /// previously skipped the staleness check forever — a report pipeline dead + /// from FIRST BOOT showed "online with no metrics" indefinitely. FirstSeenAt + /// anchors the window instead (LastHeartbeatAt cannot: heartbeats advance it). + /// + [Fact] + public void HeartbeatOnlySite_NeverReported_IsFlaggedMetricsStale() + { + var (aggregator, time) = NewAggregator(metricsStaleTimeout: TimeSpan.FromMinutes(2)); + aggregator.MarkHeartbeat("site-a", time.GetUtcNow()); // registered via heartbeat only + time.Advance(TimeSpan.FromMinutes(3)); + aggregator.MarkHeartbeat("site-a", time.GetUtcNow()); // still heartbeating => online + aggregator.CheckForOfflineSites(); + var state = aggregator.GetSiteState("site-a")!; + Assert.True(state.IsOnline); // liveness unchanged + Assert.True(state.IsMetricsStale); // was impossible before the fix + } + + /// A heartbeat-only site inside the window is NOT falsely flagged. + [Fact] + public void HeartbeatOnlySite_WithinStaleWindow_IsNotFlagged() + { + var (aggregator, time) = NewAggregator(metricsStaleTimeout: TimeSpan.FromMinutes(2)); + aggregator.MarkHeartbeat("site-a", time.GetUtcNow()); + time.Advance(TimeSpan.FromMinutes(1)); + aggregator.MarkHeartbeat("site-a", time.GetUtcNow()); + aggregator.CheckForOfflineSites(); + Assert.False(aggregator.GetSiteState("site-a")!.IsMetricsStale); + } }