docs(archreview): R2-10 resilience observability shipped
This commit is contained in:
Joseph Doherty
@@ -175,9 +175,8 @@ Closed since 9cad9ed0: **hard-kill failover** (`HardKillFailoverTests`, in-proce
|
||||
|
||||
**U9 — LOW: `EnsureVariable` silently ignores changed historize-intent on an existing node** (`OtOpcUaNodeManager.cs:1345-1349`) — invariant still lives in two files' comments rather than an assert.
|
||||
|
||||
**U10 — LOW (NEW): The `DriverResilienceStatusTracker` has no reader — pipeline observability is log-only.**
|
||||
The tracker is registered, fed by every retry/breaker event, and consumed by nothing (Admin `/hosts` Stream E.2/E.3 never shipped). The interim mitigation (retry/breaker-open/breaker-close logging in `DriverResiliencePipelineBuilder.cs:124-170`, added with #10) is real and correctly gated, but breaker state — now a production behavior that can reject calls (`BrokenCircuitException`) — is invisible to operators except in logs. Known residual from FOLLOWUP-10; recorded here so the review tracks it.
|
||||
*Recommendation:* ship the `/hosts` resilience column, or at minimum a metric per breaker-open.
|
||||
**U10 — LOW (NEW): The `DriverResilienceStatusTracker` has no reader — pipeline observability is log-only.** — ✅ **REMEDIATED** (R2-10, branch `r2/10-resilience-observability`). The tracker now has a production reader: a periodic (5 s) full-snapshot publisher (`DriverResilienceStatusPublisherService`, driver nodes, registered next to the tracker with a wiring guard `ResilienceStatusReaderRegistrationTests`) → `driver-resilience-status` DPS topic → per-admin-node `DriverResilienceStatusBridge` → `IDriverResilienceStatusStore` → a resilience section on `DriverStatusPanel.razor` (breaker-OPEN / consecutive-failures / in-flight / staleness chips, read in-process per the self-HubConnection ban). Two tracker truthfulness gaps fixed along the way: `RecordBreakerClose` + `IsBreakerOpen` (breaker-open state is now derivable) and `RecordSuccess` wired onto the invoker success paths (`ConsecutiveFailures` is now a true consecutive counter). Mirrors the driver-health flow one-for-one. *(Original finding:)* The tracker was registered, fed by every retry/breaker event, and consumed by nothing (Admin `/hosts` Stream E.2/E.3 never shipped); the interim mitigation was retry/breaker logging in `DriverResiliencePipelineBuilder.cs`.
|
||||
*Live gate (T10) pending:* docker-dev breaker-open observation on both centrals + staleness dim.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -240,7 +240,9 @@ There is **no bUnit** — 77 razor components have no render/binding coverage, w
|
||||
|
||||
~31 `aria-*`/`role=` attributes across 77 components, concentrated in the modals (`role="dialog"`, `btn-close` aria-labels). Gaps: text-glyph expander buttons (`▼`/`▶` in `UnsTree.razor:49`) with no `aria-expanded`/label; modals have no focus trap or Escape handling; live tails (`Alerts`) have no `aria-live` region; tables lack captions/scope. Form labeling is decent (most inputs have `for`/`id` pairs). Acceptable for an internal ops tool; worth a targeted pass on the tree and the modals if operator diversity matters.
|
||||
|
||||
### U-5 (NEW) — Resilience runtime status has no AdminUI surface; overrides are authored blind
|
||||
### U-5 (NEW) — Resilience runtime status has no AdminUI surface; overrides are authored blind — ✅ REMEDIATED (R2-10)
|
||||
|
||||
**Remediated** on branch `r2/10-resilience-observability`: `DriverResilienceStatusTracker` now flows to the AdminUI through exactly the recommended idiom — a per-node DPS bridge (`DriverResilienceStatusBridge`) → in-process store (`IDriverResilienceStatusStore`, registered beside `IDriverStatusSnapshotStore` in `AddOtOpcUaDriverStatusServices`) → a **resilience section on `DriverStatusPanel.razor`** (embedded on all 8 driver pages, right where the `ResilienceConfig` overrides are authored — closing the feedback loop). It renders breaker-OPEN / consecutive-failures / in-flight / staleness chips, read in-process (self-HubConnection ban honored). The driver-node feed is a periodic 5 s full-snapshot publisher (`DriverResilienceStatusPublisherService`) with a wiring guard. Live gate (T10) — docker-dev breaker-open on both centrals + staleness — pending. *(Original finding below.)*
|
||||
|
||||
The remediation merges made per-driver resilience live in production: #10 (`bacea1a4`) wired `CapabilityInvoker` into every `DriverInstanceActor` dispatch site, and #13 (`75403caa`) applies the AdminUI-authored `ResilienceConfig` overrides from the deploy artifact. But `DriverResilienceStatusTracker` (`src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResilienceStatusTracker.cs`) still has **no reader** — no AdminUI component, hub, or endpoint consumes it (`grep -rn Resilience src/Server/ZB.MOM.WW.OtOpcUa.AdminUI` matches only the authoring form + driver pages). The planned `/hosts` Stream E.2/E.3 status panels never shipped; the interim observability surface is retry/breaker **log lines** added during #10. `archreview/plans/STATUS.md` explicitly records this as "still open as an Admin-UI follow-on."
|
||||
|
||||
|
||||
@@ -39,11 +39,19 @@ the concrete invoker in would reverse that architectural decision.
|
||||
ResilienceConfig change. Per-instance overrides now reach the invoker. See
|
||||
[`FOLLOWUP-13`](FOLLOWUP-13-resilience-config-artifact.md).
|
||||
|
||||
**Observability finding + fix.** The resilience-status tracker has **no reader** (Admin `/hosts`, Phase 6.1 Stream
|
||||
**Observability finding + fix.** The resilience-status tracker had **no reader** (Admin `/hosts`, Phase 6.1 Stream
|
||||
E.2/E.3, was never built) and the pipeline builder logged nothing — so the pipeline ran **invisibly**. Added
|
||||
retry / breaker-open / breaker-close **structured logging** to `DriverResiliencePipelineBuilder` (optional ILogger,
|
||||
wired in the Host DI). This is the operator-facing observability surface AND the prerequisite for any live-verify.
|
||||
|
||||
> **UPDATE (R2-10, branch `r2/10-resilience-observability`):** the tracker now HAS a proper reader. A periodic
|
||||
> 5 s DPS publisher (`DriverResilienceStatusPublisherService`, driver nodes) mirrors each snapshot →
|
||||
> `driver-resilience-status` topic → per-admin `DriverResilienceStatusBridge` → `IDriverResilienceStatusStore` →
|
||||
> breaker/retry/in-flight/staleness chips on `DriverStatusPanel.razor` (mirrors the driver-health flow). Tracker
|
||||
> truthfulness fixed too: `IsBreakerOpen`/`RecordBreakerClose` + `RecordSuccess` on the invoker success paths. The
|
||||
> log-line remains the drill-down; the panel is the primary surface. **The live behavioral gate below (observing the
|
||||
> `circuit-breaker OPENED` line on a dead endpoint) is now folded into R2-10's T10 live-`/run` — still pending.**
|
||||
|
||||
**Verification done (deterministic + local):**
|
||||
- Negative control: unwrapping any site fails the Runtime build with OTOPCUA0001.
|
||||
- Recording-invoker Runtime test: write routes via `ExecuteWriteAsync` with the `IPerCallHostResolver` host;
|
||||
|
||||
@@ -191,3 +191,14 @@ Subscription lifecycle copies the existing pattern exactly: inject the store, su
|
||||
1. Mark U10/U-5 remediated with the branch/commit; update STATUS.md; annotate FOLLOWUP-10 that the reader shipped and (if observed in T10) the live behavioral log-line gate closed. Commit: `docs(archreview): R2-10 resilience observability shipped`.
|
||||
|
||||
**Full-suite gate before merge:** `dotnet test ZB.MOM.WW.OtOpcUa.slnx` green (integration suites need their fixtures per CLAUDE.md; the new tests themselves are fixture-free).
|
||||
|
||||
---
|
||||
|
||||
## Execution deviations (R2-10)
|
||||
|
||||
Executed on branch `r2/10-resilience-observability` (off master `46fedda3`). T1–T9 + T11 completed; T10 deferred-live.
|
||||
|
||||
- **T2 breaker-close test uses a controllable `TimeProvider` (`ManualTimeProvider`), not wall-clock waits.** Driving the real Polly breaker to `OnClosed` requires elapsing the 15 s `BreakDuration` then a successful half-open probe. Rather than a 15 s `Task.Delay`, the test injects a `TimeProvider` that advances both `GetUtcNow` and `GetTimestamp` off one tick counter (`Advance(16 s)`), making the close deterministic and instant. Same self-contained-fake style the repo already uses (`TriePermissionEvaluatorTests`); no new package. This is the one deviation from the plan's "extend the existing breaker test" wording — the existing test only reaches breaker-open, and open→close needs time control.
|
||||
- **T9 panel renders healthy hosts quietly** rather than one always-visible row per host. Per the plan's "healthy is quiet" intent, the resilience section shows: `No resilience activity recorded yet.` when the store has no entries for the instance; `All targets healthy.` when entries exist but none has an active signal; and a chip row **only** for hosts with breaker-OPEN / consecutive-failures>0 / in-flight>0 / stale. Keeps the panel silent under normal operation (the tracker populates an entry for every host that has ever had a successful call, so an always-on per-host row would be noisy).
|
||||
- **T10 (live-`/run` gate) deferred-live.** Code-complete + offline-verified (unit + full-solution build). The docker-dev breaker-open observation on both centrals + recovery-clear + staleness-dim is a heavy serial pass left for the consolidated live session (also the first live sighting of the #10 `circuit-breaker OPENED` log line, closing FOLLOWUP-10's behavioral gate). Marked `deferred-live` in `.tasks.json`.
|
||||
- **T6/T8 registration:** the publisher's `Func<ActorSystem>` is registered via `TryAddSingleton` (idempotent with the one `Program.cs` registers for the alarm-command router, per plan). The resilience bridge intentionally has **no SignalR hub** (the panel reads the store in-process; resilience has no browser-JS consumer) — a deliberate simplification the plan calls for.
|
||||
|
||||
@@ -11,7 +11,7 @@
|
||||
{ "id": "T7", "subject": "AdminUI: IDriverResilienceStatusStore + InMemory impl + AddOtOpcUaDriverStatusServices registration (TDD mirroring DriverStatusSnapshotStoreTests)", "status": "completed", "blockedBy": ["T4"] },
|
||||
{ "id": "T8", "subject": "AdminUI: DriverResilienceStatusBridge DPS actor + spawn in WithOtOpcUaSignalRBridges", "status": "completed", "blockedBy": ["T7"] },
|
||||
{ "id": "T9", "subject": "AdminUI: resilience chip section in DriverStatusPanel.razor (in-process store read; no bUnit — verified by T10)", "status": "completed", "blockedBy": ["T7"] },
|
||||
{ "id": "T10", "subject": "Live-/run gate on docker-dev: rebuild BOTH centrals, S7 dead-endpoint breaker-open, both-replica chips, recovery clear, staleness dim", "status": "pending", "blockedBy": ["T6", "T8", "T9"] },
|
||||
{ "id": "T11", "subject": "Docs + review ledger: mark 03/U10 + 04/U-5 remediated; update STATUS.md + FOLLOWUP-10", "status": "pending", "blockedBy": ["T10"] }
|
||||
{ "id": "T10", "subject": "Live-/run gate on docker-dev: rebuild BOTH centrals, S7 dead-endpoint breaker-open, both-replica chips, recovery clear, staleness dim", "status": "deferred-live", "blockedBy": ["T6", "T8", "T9"] },
|
||||
{ "id": "T11", "subject": "Docs + review ledger: mark 03/U10 + 04/U-5 remediated; update STATUS.md + FOLLOWUP-10", "status": "completed", "blockedBy": ["T10"] }
|
||||
]
|
||||
}
|
||||
|
||||
@@ -174,6 +174,7 @@ what the unit leg cannot; (c) migrate the 3 xunit v2 holdouts once Akka ships an
|
||||
| **R2-01** | 05/STAB-14 (High regression, rank 1), 05/STAB-15 (rank 3), 05/STAB-8 seam | `r2/01-s7-fault-hardening` (off `1676c8f4`) | `fcdcf0e2`…`bdb7ed41` (13 task commits) | **Code-complete, offline-verified.** All prod changes in `S7Driver.cs`. STAB-14: connect-timeout surfaces as `TimeoutException` (not an escaping OCE) at the `EnsureConnectedAsync`/`InitializeAsync` source + `when(token.IsCancellationRequested)` filters on the read/write ensure-wrappers and all three poll-loop OCE catches — subscription poll loops survive an unreachable-host outage and recover (T2 regression RED→GREEN). STAB-15: `IsS7ConnectionFatal` broadened to the ISO-on-TCP framing surface (TPKT/TPDU/WrongNumberOfBytes/`PlcException{WrongNumberReceivedBytes}`, NOT `InvalidDataException`); mid-PDU cancel marks handle dead; probe classifies+marks-dead under the gate. STAB-8 (S7 part): `EnsureConnectedAsync` documented as the R2-09 connect-throttle seam (note-only). Driver.S7.Tests 246/246. **Live outage gate deferred (T11)** — `S7_1500ConnectTimeoutOutageTests` (env `S7_TIMEOUT_OUTAGE_START_CMD`/`STOP_CMD`, SYN-blackhole) skips clean offline; the connect-*timeout* class the `docker restart` bounce can't produce. (Pre-existing unrelated CLI docs-scan test fails at base — out of scope.) |
|
||||
| **R2-03** | 02/S13 (rank 5), 02/S12, 02/P7 | `r2/03-vt-failure-quality` (off `1676c8f4`) | `b9775485`…`6bb91e8d` (12 task commits) | **Code-complete, offline-verified.** S13: VT script failure/timeout degrades the node to `OpcUaQuality.Bad` (carrying last-known value) instead of freezing on last-Good forever — additive `EvaluationResult.Quality`, bridge historizes `0x80020000u` BadInternalError, published once per Good→Bad transition gated on all deps arrived (inputs-ready, no cold-start flash), recovery force-publishes next Good. S12: apply-boundary `ClearCompiledScripts` racing an in-flight eval into ObjectDisposedException retried once in `RoslynVirtualTagEvaluator` (re-fetch→recompile into current generation); stress guard reproduces pre-fix, deterministically green post-fix. P7: apply-boundary clear gated on a changed deployed expression set (`HashSet.SetEquals`, ordinal) — identical redeploy / rename-with-same-expressions no longer forces full VT recompile. Runtime.Tests 372/372, RoslynVirtualTagEvaluatorTests 20/20. **Deferred (T11/T12):** full cross-suite sweep + docker-dev live-`/run` (fail→Bad→recover + P7 no-recompile spot-check). |
|
||||
| **R2-02** | 01/S-6 (High, rank 2), 01/U-6, 01/S-8=03/S12 (rank 4), 04/C-7, 01/S-7=03/S13 | `r2/02-resilience-config-hardening` (off `1676c8f4`) | `4be13429`…`dd82e740` (17 task commits) | **Code-complete, offline-verified.** **S-6:** parser clamps timeout/retry/breaker overrides (timeout≤0→tier default, breaker==1→2, caps) with diagnostics + belt-and-braces `Build` clamp so pipeline construction can never throw Polly `ValidationException` (new `ResiliencePolicyRanges` in Core.Abstractions); brick-repro + `Build_NeverThrows_ForHostileDirectOptions` + factory-seam wiring proof; AdminUI `Validate()` range warnings. **S-8:** parser forces Write/AlarmAck retry→0 (spec invariant) **and** dispatch flips to `isIdempotent:false` (RED-first on the Runtime wiring test); non-idempotent arm caches the no-retry snapshot once/invoker (01/P-4) + tracker in-flight accounting; `WriteIdempotentAttribute` false claim corrected; retry cells disabled. **U-6:** bulkhead knob deleted end-to-end (migration-free; stored keys ignored) + `Options_properties_are_exactly_the_pipeline_wired_set` inertness guard. **C-7:** `ResilienceFormModel` JsonObject-bag round-trip preserves unknown keys; malformed JSON → `ParseFailed` lock + verbatim `ToJson` + discard button. **S-7:** per-`Create` `Interlocked` options-generation in the pipeline-cache key kills the respawn stale-pipeline race. Core resilience 121/121, AdminUI form 10/10, Analyzers 32/32, Runtime 364/364; 0 production OTOPCUA0001. **Deferred (T15/T18):** docker-dev `:9200` `/run` (warning banner, input lockout, unknown-key survival) + whole-solution sweep. |
|
||||
| **R2-10** | 03/U10=04/U-5 (Medium/Low, action #10) | `r2/10-resilience-observability` (off `46fedda3`) | `205d3980`…`<docs>` (11 task commits) | **Code-complete, offline-verified.** The resilience status tracker now has a production reader, mirroring the driver-health flow. Core truthfulness: `RecordBreakerClose` + derived `IsBreakerOpen` (breaker-open state derivable), `OnClosed` records the close, and `RecordSuccess` wired onto the invoker success paths (`ConsecutiveFailures` is a true consecutive counter, not stuck-forever). Transport: `DriverResilienceStatusChanged` Commons contract → periodic 5 s full-snapshot publisher `DriverResilienceStatusPublisherService` (driver nodes, registered in `AddOtOpcUaDriverFactories` with wiring guard `ResilienceStatusReaderRegistrationTests` — closes the built-but-never-wired class) → `driver-resilience-status` DPS topic → per-admin-node `DriverResilienceStatusBridge` → `IDriverResilienceStatusStore` → resilience chips (breaker-OPEN / consecutive-failures / in-flight / staleness) on `DriverStatusPanel.razor`, read in-process (self-HubConnection ban honored). Core.Tests resilience 31/31 (incl. deterministic breaker-close via a controllable TimeProvider), Host.IntegrationTests publisher+guard 6/6, AdminUI.Tests store 5/5; full-solution build clean. **Deferred (T10):** docker-dev live-`/run` — S7 dead-endpoint breaker-open observed on BOTH centrals + recovery-clear + staleness-dim (also the first live sighting of the #10 `circuit-breaker OPENED` log line, closing FOLLOWUP-10's behavioral gate). |
|
||||
|
||||
**Report anchor drifts to fold into the next report refresh** (from the R2-02 verification pass): (1)
|
||||
`01-core-composition.md` U-6 anchor `DriverResilienceOptions.cs:23-25` → the two bulkhead properties sat at
|
||||
|
||||
Reference in New Issue
Block a user