fix(r2-10): OnClosed records breaker-close on the status tracker

This commit is contained in:
Joseph Doherty
2026-07-13 10:37:12 -04:00
parent 205d398055
commit 2cce61fc04
3 changed files with 66 additions and 8 deletions
@@ -3,7 +3,7 @@
"lastUpdated": "2026-07-12",
"tasks": [
{ "id": "T1", "subject": "Tracker: LastBreakerClosedUtc + IsBreakerOpen + RecordBreakerClose (Core, TDD in DriverResilienceStatusTrackerTests)", "status": "completed", "blockedBy": [] },
{ "id": "T2", "subject": "Builder: OnClosed records breaker-close on the tracker (TDD in DriverResiliencePipelineBuilderTests)", "status": "pending", "blockedBy": ["T1"] },
{ "id": "T2", "subject": "Builder: OnClosed records breaker-close on the tracker (TDD in DriverResiliencePipelineBuilderTests)", "status": "completed", "blockedBy": ["T1"] },
{ "id": "T3", "subject": "Invoker: successful Execute* resets ConsecutiveFailures (TDD in CapabilityInvokerTests)", "status": "pending", "blockedBy": ["T1"] },
{ "id": "T4", "subject": "Commons: DriverResilienceStatusChanged record + driver-resilience-status TopicName", "status": "pending", "blockedBy": [] },
{ "id": "T5", "subject": "Host: DriverResilienceStatusPublisherService (BuildMessages mapping + 5s DPS timer shell; TDD mapping in Host.IntegrationTests)", "status": "pending", "blockedBy": ["T1", "T4"] },
@@ -38,14 +38,16 @@ public sealed class DriverResiliencePipelineBuilder
/// <param name="timeProvider">Clock source for pipeline timeouts + breaker sampling. Defaults to system.</param>
/// <param name="statusTracker">When non-null, every built pipeline wires Polly telemetry into
/// the tracker — retries increment <c>ConsecutiveFailures</c>, breaker-open stamps
/// <c>LastBreakerOpenUtc</c>, breaker-close resets failures. Feeds Admin <c>/hosts</c> +
/// <c>LastBreakerOpenUtc</c>, breaker-close resets failures and stamps
/// <c>LastBreakerClosedUtc</c> (so <c>IsBreakerOpen</c> reads false again). Feeds the AdminUI
/// resilience panel via the <c>driver-resilience-status</c> DPS publisher +
/// the in-flight-call-depth column. Absent tracker means no telemetry (unit tests +
/// deployments that don't care about resilience observability).</param>
/// <param name="logger">When non-null, retry / circuit-breaker-open / breaker-close events are
/// logged (instance + host + capability + attempt/exception). This is the operator-facing
/// observability surface for the resilience pipeline until the Admin <c>/hosts</c> reader
/// (Phase 6.1 Stream E.2/E.3) ships — without it the pipeline runs silently. Absent logger =
/// no resilience logging (unit tests).</param>
/// logged (instance + host + capability + attempt/exception). This is the structured-log
/// drill-down for the resilience pipeline, complementing the AdminUI resilience panel fed by
/// the tracker — without it the pipeline runs silently. Absent logger = no resilience logging
/// (unit tests).</param>
public DriverResiliencePipelineBuilder(
TimeProvider? timeProvider = null,
DriverResilienceStatusTracker? statusTracker = null,
@@ -186,8 +188,11 @@ public sealed class DriverResiliencePipelineBuilder
breakerOptions.OnClosed = args =>
{
// Closing the breaker means the target recovered — reset the consecutive-
// failure counter so Admin UI stops flashing red for this host.
tracker?.RecordSuccess(driverInstanceId, hostName, timeProvider.GetUtcNow().UtcDateTime);
// failure counter AND stamp the close time so IsBreakerOpen reads false again
// (the resilience panel stops showing "Breaker OPEN" for this host).
var closedUtc = timeProvider.GetUtcNow().UtcDateTime;
tracker?.RecordSuccess(driverInstanceId, hostName, closedUtc);
tracker?.RecordBreakerClose(driverInstanceId, hostName, closedUtc);
logger?.LogInformation(
"Driver resilience circuit-breaker closed (recovered): instance={DriverInstanceId} host={HostName} capability={Capability}",
driverInstanceId, hostName, capability);
@@ -277,6 +277,44 @@ public sealed class DriverResiliencePipelineBuilderTests
snap!.LastBreakerOpenUtc.ShouldNotBeNull();
}
/// <summary>
/// R2-10 truthfulness fix: after the breaker opens and later recovers (a successful probe
/// past <c>BreakDuration</c>), the pipeline builder's <c>OnClosed</c> hook must record the
/// breaker close on the tracker — so <see cref="ResilienceStatusSnapshot.IsBreakerOpen"/>
/// reads false again and <see cref="ResilienceStatusSnapshot.LastBreakerClosedUtc"/> is stamped.
/// Uses a controllable <see cref="TimeProvider"/> so the 15 s break duration elapses deterministically.
/// </summary>
[Fact]
public async Task Tracker_RecordsBreakerClose_OnRecovery()
{
var clock = new ManualTimeProvider(new DateTime(2026, 7, 13, 0, 0, 0, DateTimeKind.Utc));
var tracker = new DriverResilienceStatusTracker();
var builder = new DriverResiliencePipelineBuilder(timeProvider: clock, statusTracker: tracker);
var pipeline = builder.GetOrCreate("drv-trk", "host-c", DriverCapability.Write, TierAOptions);
// Drive the breaker open with the tier's throughput of failures.
var threshold = TierAOptions.Resolve(DriverCapability.Write).BreakerFailureThreshold;
for (var i = 0; i < threshold; i++)
{
await Should.ThrowAsync<InvalidOperationException>(async () =>
await pipeline.ExecuteAsync(async _ =>
{
await Task.Yield();
throw new InvalidOperationException("boom");
}));
}
tracker.TryGet("drv-trk", "host-c")!.IsBreakerOpen.ShouldBeTrue("breaker must be open after the failure threshold");
// Elapse the break duration (15 s) so the breaker half-opens, then let a probe succeed → OnClosed.
clock.Advance(TimeSpan.FromSeconds(16));
await pipeline.ExecuteAsync(async _ => await Task.Yield());
var snap = tracker.TryGet("drv-trk", "host-c")!;
snap.LastBreakerClosedUtc.ShouldNotBeNull("OnClosed must stamp the close time on recovery");
snap.IsBreakerOpen.ShouldBeFalse("the breaker must read closed after a successful recovery probe");
}
/// <summary>Verifies that the tracker isolates counters per host.</summary>
[Fact]
public async Task Tracker_IsolatesCounters_PerHost()
@@ -404,6 +442,21 @@ public sealed class DriverResiliencePipelineBuilderTests
attempts.ShouldBe(1, "the new generation must honor optsB (retryCount 0), not the stale optsA re-cache");
}
/// <summary>
/// A controllable <see cref="TimeProvider"/> whose clock advances only on <see cref="Advance"/>.
/// Drives both wall-clock (<see cref="GetUtcNow"/>) and monotonic (<see cref="GetTimestamp"/>)
/// reads off the same tick counter so Polly's circuit-breaker break-duration timing is deterministic.
/// </summary>
private sealed class ManualTimeProvider : TimeProvider
{
private long _ticks;
public ManualTimeProvider(DateTime start) => _ticks = start.Ticks;
public void Advance(TimeSpan by) => _ticks += by.Ticks;
public override DateTimeOffset GetUtcNow() => new(_ticks, TimeSpan.Zero);
public override long GetTimestamp() => _ticks;
public override long TimestampFrequency => TimeSpan.TicksPerSecond;
}
/// <summary>Minimal in-memory <see cref="ILogger"/> capturing (level, formatted-message) pairs so the
/// resilience logging contract can be asserted without a real logging provider.</summary>
private sealed class CapturingLogger : ILogger