fix(health): flag metrics-stale for never-reported sites via FirstSeenAt anchor — null LastReportReceivedAt no longer skips the check (plan R2-01 T8)
This commit is contained in:
@@ -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.
|
- 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:
|
- 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.
|
- **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.
|
- **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.
|
- **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.
|
||||||
|
|
||||||
|
|||||||
@@ -47,6 +47,7 @@ public class CentralHealthAggregator : BackgroundService, ICentralHealthAggregat
|
|||||||
LatestReport = report,
|
LatestReport = report,
|
||||||
LastReportReceivedAt = now,
|
LastReportReceivedAt = now,
|
||||||
LastHeartbeatAt = now,
|
LastHeartbeatAt = now,
|
||||||
|
FirstSeenAt = now,
|
||||||
LastSequenceNumber = report.SequenceNumber,
|
LastSequenceNumber = report.SequenceNumber,
|
||||||
IsOnline = true,
|
IsOnline = true,
|
||||||
IsMetricsStale = false
|
IsMetricsStale = false
|
||||||
@@ -117,6 +118,7 @@ public class CentralHealthAggregator : BackgroundService, ICentralHealthAggregat
|
|||||||
LatestReport = null,
|
LatestReport = null,
|
||||||
LastReportReceivedAt = null,
|
LastReportReceivedAt = null,
|
||||||
LastHeartbeatAt = receivedAt,
|
LastHeartbeatAt = receivedAt,
|
||||||
|
FirstSeenAt = receivedAt,
|
||||||
LastSequenceNumber = 0,
|
LastSequenceNumber = 0,
|
||||||
IsOnline = true
|
IsOnline = true
|
||||||
};
|
};
|
||||||
@@ -297,16 +299,22 @@ public class CentralHealthAggregator : BackgroundService, ICentralHealthAggregat
|
|||||||
// when no report has arrived within MetricsStaleTimeout. Distinct from
|
// when no report has arrived within MetricsStaleTimeout. Distinct from
|
||||||
// offline — a site whose HealthReportSender crashed keeps heartbeating
|
// offline — a site whose HealthReportSender crashed keeps heartbeating
|
||||||
// and would otherwise show "online with frozen metrics forever".
|
// and would otherwise show "online with frozen metrics forever".
|
||||||
if (state.LastReportReceivedAt is { } lastReport
|
// A site that has NEVER reported (heartbeat-only registration,
|
||||||
&& now - lastReport > _options.MetricsStaleTimeout
|
// 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)
|
&& !state.IsMetricsStale)
|
||||||
{
|
{
|
||||||
var stale = state with { IsMetricsStale = true };
|
var stale = state with { IsMetricsStale = true };
|
||||||
if (_siteStates.TryUpdate(kvp.Key, stale, state))
|
if (_siteStates.TryUpdate(kvp.Key, stale, state))
|
||||||
{
|
{
|
||||||
_logger.LogWarning(
|
_logger.LogWarning(
|
||||||
"Site {SiteId} metrics are stale — online (heartbeats) but no report for {Elapsed}s (timeout: {Timeout}s)",
|
"Site {SiteId} metrics are stale — online (heartbeats) but no report for {Elapsed}s{NeverNote} (timeout: {Timeout}s)",
|
||||||
state.SiteId, (now - lastReport).TotalSeconds, _options.MetricsStaleTimeout.TotalSeconds);
|
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.
|
// CAS loss ⇒ a fresh report/heartbeat swapped in ⇒ correct to skip.
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -40,6 +40,17 @@ public sealed record SiteHealthState
|
|||||||
/// </summary>
|
/// </summary>
|
||||||
public DateTimeOffset LastHeartbeatAt { get; init; }
|
public DateTimeOffset LastHeartbeatAt { get; init; }
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Gets the instant this site was first observed by the aggregator (via a
|
||||||
|
/// report or a heartbeat); <c>null</c> 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
|
||||||
|
/// <see cref="LastReportReceivedAt"/> null the staleness check previously
|
||||||
|
/// skipped the site forever. <see cref="LastHeartbeatAt"/> cannot anchor
|
||||||
|
/// this — every heartbeat advances it.
|
||||||
|
/// </summary>
|
||||||
|
public DateTimeOffset? FirstSeenAt { get; init; }
|
||||||
|
|
||||||
/// <summary>Gets the sequence number of the last accepted health report, used to reject out-of-order duplicates.</summary>
|
/// <summary>Gets the sequence number of the last accepted health report, used to reject out-of-order duplicates.</summary>
|
||||||
public long LastSequenceNumber { get; init; }
|
public long LastSequenceNumber { get; init; }
|
||||||
/// <summary>Gets a value indicating whether the site is currently considered online.</summary>
|
/// <summary>Gets a value indicating whether the site is currently considered online.</summary>
|
||||||
|
|||||||
@@ -513,4 +513,35 @@ public class CentralHealthAggregatorTests
|
|||||||
aggregator.MarkHeartbeat("site-a", time.GetUtcNow());
|
aggregator.MarkHeartbeat("site-a", time.GetUtcNow());
|
||||||
Assert.NotEqual(offlineAt, aggregator.GetSiteState("site-a")!.LastStatusChangeAt);
|
Assert.NotEqual(offlineAt, aggregator.GetSiteState("site-a")!.LastStatusChangeAt);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// 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).
|
||||||
|
/// </summary>
|
||||||
|
[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
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>A heartbeat-only site inside the window is NOT falsely flagged.</summary>
|
||||||
|
[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);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user