feat(health): metrics-stale signal + status-transition timestamps; spec now matches heartbeat-liveness code

docs/requirements/Component-HealthMonitoring.md

@@ -42,8 +42,11 @@ Site clusters (metric collection and reporting). Central cluster (aggregation an
 
 - Sites send a **health report message** to central at a configurable interval (default: **30 seconds**).
 - 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.
-- **Offline detection**: If central does not receive a report within a configurable timeout window (default: **60 seconds** — 2x the report interval), the site is marked as **offline**. This gives one missed report as grace before marking offline.
-- **Online recovery**: When central receives a health report from a site that was marked offline, the site is automatically marked **online**. No manual acknowledgment required — the metrics in the report provide immediate visibility into the site's condition.
+- 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.
+- **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.
 
 ## Error Rate Metrics
 

src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/CentralHealthAggregator.cs

@@ -48,7 +48,9 @@ public class CentralHealthAggregator : BackgroundService, ICentralHealthAggregat
                     LastReportReceivedAt = now,
                     LastHeartbeatAt = now,
                     LastSequenceNumber = report.SequenceNumber,
-                    IsOnline = true
+                    IsOnline = true,
+                    IsMetricsStale = false
+                    // LastStatusChangeAt stays null: first observation is not a flip.
                 };
 
                 if (_siteStates.TryAdd(report.SiteId, registered))
@@ -76,7 +78,12 @@ public class CentralHealthAggregator : BackgroundService, ICentralHealthAggregat
                 LastReportReceivedAt = now,
                 LastHeartbeatAt = now,
                 LastSequenceNumber = report.SequenceNumber,
-                IsOnline = true
+                IsOnline = true,
+                // A fresh report is proof the metrics pipeline is alive again.
+                IsMetricsStale = false,
+                // Stamp a flip only when this report brings the site back online;
+                // a report from an already-online site leaves the timestamp be.
+                LastStatusChangeAt = existing.IsOnline ? existing.LastStatusChangeAt : now
             };
 
             if (_siteStates.TryUpdate(report.SiteId, updated, existing))
@@ -155,7 +162,14 @@ public class CentralHealthAggregator : BackgroundService, ICentralHealthAggregat
             var updated = existing with
             {
                 LastHeartbeatAt = newHeartbeat,
-                IsOnline = true
+                IsOnline = true,
+                // Stamp the flip when this heartbeat promotes an offline site back
+                // online. IsMetricsStale is deliberately left untouched — a
+                // heartbeat says nothing about whether the metrics pipeline
+                // recovered; only a full report clears staleness.
+                LastStatusChangeAt = existing.IsOnline
+                    ? existing.LastStatusChangeAt
+                    : _timeProvider.GetUtcNow()
             };
 
             if (_siteStates.TryUpdate(siteId, updated, existing))
@@ -246,18 +260,41 @@ public class CentralHealthAggregator : BackgroundService, ICentralHealthAggregat
                 : _options.OfflineTimeout;
 
             var elapsed = now - state.LastHeartbeatAt;
-            if (elapsed <= timeout)
-                continue;
-
-            // Atomically swap to an offline copy. If the CAS loses to a
-            // concurrent report/heartbeat the site was just heard from, so
-            // leaving it online is the correct outcome — no retry needed.
-            var offline = state with { IsOnline = false };
-            if (_siteStates.TryUpdate(kvp.Key, offline, state))
+            if (elapsed > timeout)
             {
-                _logger.LogWarning(
-                    "Site {SiteId} marked offline — no signal for {Elapsed}s (timeout: {Timeout}s)",
-                    state.SiteId, elapsed.TotalSeconds, timeout.TotalSeconds);
+                // Atomically swap to an offline copy, stamping the flip. If the
+                // CAS loses to a concurrent report/heartbeat the site was just
+                // heard from, so leaving it online is correct — no retry needed.
+                var offline = state with { IsOnline = false, LastStatusChangeAt = now };
+                if (_siteStates.TryUpdate(kvp.Key, offline, state))
+                {
+                    _logger.LogWarning(
+                        "Site {SiteId} marked offline — no signal for {Elapsed}s (timeout: {Timeout}s)",
+                        state.SiteId, elapsed.TotalSeconds, timeout.TotalSeconds);
+                }
+
+                // Whether the CAS won or lost, do not also evaluate staleness on
+                // this stale snapshot — offline supersedes metrics-stale.
+                continue;
+            }
+
+            // Second signal: the site is still live (heartbeats within timeout)
+            // but its full-report pipeline may have died. Flag it metrics-stale
+            // 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
+                && !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);
+                }
+                // CAS loss ⇒ a fresh report/heartbeat swapped in ⇒ correct to skip.
             }
         }
     }

src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/HealthMonitoringOptions.cs

@@ -19,4 +19,17 @@ public class HealthMonitoringOptions
     /// the design doc grants real sites. Default: 3x the report interval.
     /// </summary>
     public TimeSpan CentralOfflineTimeout { get; set; } = TimeSpan.FromMinutes(3);
+
+    /// <summary>
+    /// Duration an online site may go without a full health <em>report</em>
+    /// (heartbeats notwithstanding) before it is flagged
+    /// <see cref="SiteHealthState.IsMetricsStale"/>. Distinct from
+    /// <see cref="OfflineTimeout"/>, which is heartbeat-based liveness: a site
+    /// whose HealthReportSender has died keeps heartbeating and stays "online",
+    /// but its metrics freeze — this window surfaces that. Must be positive and
+    /// no shorter than <see cref="ReportInterval"/> (a window shorter than one
+    /// report interval would flag every healthy site). Default: 2 minutes
+    /// (~4 missed 30s reports).
+    /// </summary>
+    public TimeSpan MetricsStaleTimeout { get; set; } = TimeSpan.FromMinutes(2);
 }

src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/HealthMonitoringOptionsValidator.cs

@@ -45,5 +45,24 @@ public sealed class HealthMonitoringOptionsValidator : OptionsValidatorBase<Heal
             $"must be >= OfflineTimeout ({options.OfflineTimeout}): the synthetic 'central' site has " +
             "no heartbeat source and is fed only by the slower self-report loop, so it needs at " +
             "least as much offline grace as a real site.");
+
+        builder.RequireThat(options.MetricsStaleTimeout > TimeSpan.Zero,
+            $"ScadaBridge:HealthMonitoring:MetricsStaleTimeout must be a positive duration " +
+            $"(was {options.MetricsStaleTimeout}); it is the window an online site may go without " +
+            "a full report before being flagged metrics-stale.");
+
+        // A stale window shorter than one report interval flags every healthy
+        // site (reports simply can't arrive faster than they are emitted). Same
+        // De Morgan guard shape as the Central/Offline pair above: stay silent
+        // when either field is non-positive so the dedicated positive-duration
+        // checks own that failure rather than double-firing here.
+        builder.RequireThat(
+            !(options.ReportInterval > TimeSpan.Zero
+              && options.MetricsStaleTimeout > TimeSpan.Zero
+              && options.MetricsStaleTimeout < options.ReportInterval),
+            $"ScadaBridge:HealthMonitoring:MetricsStaleTimeout ({options.MetricsStaleTimeout}) " +
+            $"must be >= ReportInterval ({options.ReportInterval}): a stale window shorter than one " +
+            "report interval would flag every healthy site, since reports cannot arrive faster than " +
+            "they are emitted.");
     }
 }

src/ZB.MOM.WW.ScadaBridge.HealthMonitoring/SiteHealthState.cs

@@ -44,4 +44,23 @@ public sealed record SiteHealthState
     public long LastSequenceNumber { get; init; }
     /// <summary>Gets a value indicating whether the site is currently considered online.</summary>
     public bool IsOnline { get; init; }
+
+    /// <summary>
+    /// Gets a value indicating whether the site is online (heartbeats still
+    /// arriving) but has sent no full <see cref="SiteHealthReport"/> within
+    /// <see cref="HealthMonitoringOptions.MetricsStaleTimeout"/>. This is a signal
+    /// <em>distinct</em> from liveness: a site whose HealthReportSender died keeps
+    /// heartbeating and would otherwise show "online with frozen metrics forever".
+    /// Cleared whenever a fresh report is processed. Heartbeats never set or clear
+    /// it — they say nothing about the metrics pipeline.
+    /// </summary>
+    public bool IsMetricsStale { get; init; }
+
+    /// <summary>
+    /// Gets the instant of the last online↔offline flip, or <c>null</c> if the
+    /// site has never changed status since it was first observed. Answers "when
+    /// did this site drop / come back". Not moved by report/heartbeat traffic that
+    /// leaves the online/offline status unchanged, nor by the metrics-stale flag.
+    /// </summary>
+    public DateTimeOffset? LastStatusChangeAt { get; init; }
 }

tests/ZB.MOM.WW.ScadaBridge.HealthMonitoring.Tests/CentralHealthAggregatorTests.cs

@@ -40,6 +40,23 @@ public class CentralHealthAggregatorTests
             _timeProvider);
     }
 
+    /// <summary>
+    /// Builds a fresh aggregator + manual time provider with an explicit
+    /// <paramref name="metricsStaleTimeout"/> so the metrics-staleness tests can
+    /// pick a window independent of the shared 60s <c>OfflineTimeout</c>.
+    /// </summary>
+    private static (CentralHealthAggregator Aggregator, TestTimeProvider Time) NewAggregator(
+        TimeSpan? metricsStaleTimeout = null)
+    {
+        var time = new TestTimeProvider(DateTimeOffset.UtcNow);
+        var opts = new HealthMonitoringOptions { OfflineTimeout = TimeSpan.FromSeconds(60) };
+        if (metricsStaleTimeout is { } mst)
+            opts.MetricsStaleTimeout = mst;
+        var aggregator = new CentralHealthAggregator(
+            Options.Create(opts), NullLogger<CentralHealthAggregator>.Instance, time);
+        return (aggregator, time);
+    }
+
     private static SiteHealthReport MakeReport(string siteId, long seq) =>
         new(
             SiteId: siteId,
@@ -423,4 +440,56 @@ public class CentralHealthAggregatorTests
         _aggregator.ProcessReport(MakeReport("site-1", 11));
         Assert.Equal(11, _aggregator.GetSiteState("site-1")!.LastSequenceNumber);
     }
+
+    /// <summary>
+    /// Review 01 [Medium]: a site whose HealthReportSender died keeps
+    /// heartbeating and previously showed "online with frozen metrics forever".
+    /// Metrics staleness is a distinct signal from liveness: heartbeats keep the
+    /// site online, but the absence of a full report for
+    /// <see cref="HealthMonitoringOptions.MetricsStaleTimeout"/> flags it stale.
+    /// </summary>
+    [Fact]
+    public void HeartbeatingSiteWithStaleReports_IsFlaggedMetricsStale_ButStaysOnline()
+    {
+        var (aggregator, time) = NewAggregator(metricsStaleTimeout: TimeSpan.FromMinutes(2));
+        aggregator.ProcessReport(MakeReport("site-a", 1));
+        time.Advance(TimeSpan.FromMinutes(3));
+        aggregator.MarkHeartbeat("site-a", time.GetUtcNow()); // heartbeats keep flowing
+        aggregator.CheckForOfflineSites();
+        var state = aggregator.GetSiteState("site-a")!;
+        Assert.True(state.IsOnline);        // heartbeat-based liveness unchanged
+        Assert.True(state.IsMetricsStale);  // NEW distinct signal
+    }
+
+    /// <summary>A fresh full report clears the metrics-stale flag.</summary>
+    [Fact]
+    public void FreshReport_ClearsMetricsStale()
+    {
+        var (aggregator, time) = NewAggregator(metricsStaleTimeout: TimeSpan.FromMinutes(2));
+        aggregator.ProcessReport(MakeReport("site-a", 1));
+        time.Advance(TimeSpan.FromMinutes(3));
+        aggregator.CheckForOfflineSites();
+        aggregator.ProcessReport(MakeReport("site-a", 2));
+        Assert.False(aggregator.GetSiteState("site-a")!.IsMetricsStale);
+    }
+
+    /// <summary>
+    /// Review 01 underdeveloped #7: "when did the site drop" was unanswerable.
+    /// Every online↔offline flip stamps <see cref="SiteHealthState.LastStatusChangeAt"/>.
+    /// </summary>
+    [Fact]
+    public void OnlineOfflineTransitions_RecordLastStatusChangeAt()
+    {
+        var (aggregator, time) = NewAggregator();
+        aggregator.ProcessReport(MakeReport("site-a", 1));
+        time.Advance(TimeSpan.FromMinutes(5)); // > OfflineTimeout
+        aggregator.CheckForOfflineSites();
+        var offlineAt = aggregator.GetSiteState("site-a")!.LastStatusChangeAt;
+        Assert.Equal(time.GetUtcNow(), offlineAt);
+        // Advance so the recovery flip lands at a demonstrably later instant than
+        // the offline flip — proves LastStatusChangeAt is re-stamped on each flip.
+        time.Advance(TimeSpan.FromSeconds(10));
+        aggregator.MarkHeartbeat("site-a", time.GetUtcNow());
+        Assert.NotEqual(offlineAt, aggregator.GetSiteState("site-a")!.LastStatusChangeAt);
+    }
 }

tests/ZB.MOM.WW.ScadaBridge.HealthMonitoring.Tests/HealthMonitoringOptionsValidatorTests.cs

@@ -70,4 +70,27 @@ public class HealthMonitoringOptionsValidatorTests
         Assert.True(result.Failed);
         Assert.Contains("CentralOfflineTimeout", result.FailureMessage);
     }
+
+    [Fact]
+    public void ZeroMetricsStaleTimeout_IsRejected()
+    {
+        var result = Validate(new HealthMonitoringOptions { MetricsStaleTimeout = TimeSpan.Zero });
+
+        Assert.True(result.Failed);
+        Assert.Contains("MetricsStaleTimeout", result.FailureMessage);
+    }
+
+    [Fact]
+    public void MetricsStaleTimeout_ShorterThanReportInterval_IsRejected()
+    {
+        // A stale window shorter than one report interval flags every site.
+        var result = Validate(new HealthMonitoringOptions
+        {
+            ReportInterval = TimeSpan.FromSeconds(30),
+            MetricsStaleTimeout = TimeSpan.FromSeconds(10)
+        });
+
+        Assert.True(result.Failed);
+        Assert.Contains("MetricsStaleTimeout", result.FailureMessage);
+    }
 }