Skip to main content

ADR — Silent-failure defaults across the platform

Date: 2026-09-20 · Status: Proposed (ADR-4 partially Accepted) Evidence: deploy-availability-and-event-delivery.md · AB#14565, AB#14567, AB#14568, AB#14569, AB#14576

Context — the observation that produced these decisions​

A probe of two questions ("does a PROD deploy drop requests?" and "is the cross-service event sync guaranteed?") surfaced defects that initially looked unrelated. They are not. Every one of them shares a single property: the system reported success while not doing the thing.

SurfaceReportedActual
ArgoCDSynced, Succeededthe config change had not applied
Prometheus alerts9 rules inactivethe metrics they query do not exist
git push origin masterEverything up-to-datepushed nothing (checkout on another branch)
KafkaService.publish()returns normallyevent dropped, transaction already committed
A watchdog scriptexit 0, "complete"it had lost cluster access

These are not five bugs. They are five instances of one architectural property: the platform's failure modes are silent by default. A system whose failures are silent cannot be operated by reading its dashboards, and cannot be improved by measurement, because the measurements themselves fail silently.

The decisions below correct that property at its four structural sources.


ADR-1 — Observability is a structural guarantee, not an optional per-service dependency​

Status: Proposed. Tracked as AB#14568.

Context​

KafkaService declares @Optional() metricsService. MessagingModule is @Global() and provides KafkaService. In NestJS a global module's exports resolve only for modules instantiated after it — so an app module listing MessagingModule before MetricsModule constructs KafkaService while MetricsService does not yet exist. The optional injection resolves to undefined and every messaging metric in that service becomes a permanent no-op, including the 30-second gauge probe, which is scheduled only inside if (this.metricsService).

⚠ CORRECTED 2026-09-20. The ordering account above is withdrawn. The real cause is that MessagingMetrics is a bare TypeScript interface, so design:paramtypes emits Object, nothing is registered under that token, and @Optional() yields undefined in every service, always. Every metric call inside @hmp/messaging has therefore been a no-op since it was written, and probeGaugeMetrics() has never run anywhere. The one series on prod comes from analytics' own dispatcher (event-dispatcher.service.ts:401), which injects MetricsService — a class token — correctly. The ordering correlation was coincidental. Reordering the app modules would have fixed nothing.

Nine of ten messaging alert rules query series that consequently do not exist, and read inactive forever.

This was diagnosed once before. hmp-analytics-service/src/app.module.ts still carries the comment "AB#9459 — MetricsModule (@Global) must load before domain modules". Three services were fixed; thirteen were never swept.

Decision​

A cross-cutting concern must not be an optional dependency that each service wires correctly by hand. Observability for the message bus becomes structural:

  1. The metrics dependency gets a real runtime injection token. @hmp/messaging exports one; KafkaService declares @Optional() @Inject(MESSAGING_METRICS); the composing application provides { provide: MESSAGING_METRICS, useExisting: MetricsService }. An interface may never be used as an injection type — it compiles, it is silently unresolvable, and @Optional() converts that into undefined rather than an error.
  2. The absence of a metrics provider is logged at WARN on boot, once, by name. An absent cross-cutting concern must announce itself.
  3. A gate asserts the property by composing the real module graph and checking that KafkaService actually received a metrics provider — not by grepping import order, which passes on a renamed symbol and cannot see behaviour.

The immediate remediation (reordering the 13 app modules) is a stopgap that restores telemetry without a package release. It is not the fix — it leaves the trap intact for service 17.

Consequences​

  • 13 services regain messaging telemetry; 9 dead alert rules become able to fire.
  • @hmp/messaging gains a ModuleRef dependency and needs a release + fleet bump.
  • Ordering ceases to be load-bearing, so the class of defect cannot recur.
  • Every other event-bus improvement depends on this one. A durability fix cannot be verified with no delivery telemetry.

ADR-2 — Durable publish is the default; fire-and-forget is explicit​

Status: Proposed.

Context​

publish() and publishMessage() never throw. On !isConnected, or on an XADD error, they log and return. The caller's Postgres transaction has already committed, so the event is silently dropped with no compensation.

The correct pattern already exists and works: a transactional outbox, with a relay polling every 5 seconds in 14 services. Measured split across the 17 backends: 195 direct publish sites, 49 outbox sites. The durable path is the one a developer must remember to reach for; the lossy path is the default.

hmp-property-service is the starkest case — 20 direct publishes, 0 outbox rows.

Decision​

Invert the default. The ordinary publish() writes an outbox row inside the caller's transaction. A separately-named method (publishEphemeral() or equivalent) exists for events that are genuinely allowed to be lost — realtime UI nudges, cache invalidations. Losing an event then requires an explicit, reviewable choice rather than an omission.

This decision does not ship alone. There is currently no retention sweep on any Outbox model in any of the 14 services — no deleteMany, no TTL. Rows reach PUBLISHED and persist forever, each carrying a payload Json. Routing 195 more call sites through the outbox without pruning trades silent event loss for unbounded RDS growth. The sweep ships in the same change.

Consequences​

  • Event loss on a Redis or ElastiCache failover stops being silent and stops being possible for business events.
  • Added latency on the durable path: the relay's @Interval(5000) adds 0–5s (≈2.5s median) before the XADD. This is a real cost and must be stated in the SLO (see ADR-3's note on measurement).
  • Write amplification on RDS: one INSERT + one UPDATE per event, plus relay polling. Bounded by the retention sweep.
  • Migration is incremental — call sites move in batches, starting with hmp-property-service.

ADR-3 — Error posture is declared per concern, not applied uniformly​

Status: Proposed.

Context​

The platform applies one error posture — swallow and continue — to concerns with opposite requirements:

  • probeGaugeMetrics() wraps its work in two bare catch {} blocks. Correct in spirit: a telemetry probe must never kill message processing. But it means the gauge path fails with zero signal, which is why consumer_lag and pel_depth emit nothing even in the service where metrics are wired.
  • publish() swallows identically — and there the swallowed failure is a lost business event.

The same construct is right in one place and catastrophic in the other.

Decision​

Classify every failure path as one of two postures, and make the choice visible at the call site:

PostureApplies toBehaviour
Degrade quietlytelemetry, gauge probes, cache warmingcatch, increment an error counter, continue — never a bare catch {}
Must not losebusiness events, money, compliance writespropagate to the caller, who holds the transaction and can roll back

A bare catch {} with no counter and no log is banned outright: it is the construct that makes a subsystem fail invisibly.

Additionally: a metric that measures the wrong interval is a silent failure of its own. propagationSeconds() parses the Redis stream entry-ID, i.e. XADD time — so the 0–5s an event waits in the outbox is invisible to the declared p95 ≤ 2s SLO. Propagation is measured from business-event time, which build-event-payload.ts already stamps.

Consequences​

  • Telemetry failures become visible without becoming fatal.
  • The 2s SLO starts measuring the interval it claims to measure, and outbox-routed events will initially appear to breach it — correctly, because they do.

ADR-4 — A declaration must be verifiable against reality​

Status: Proposed. The verification discipline is Accepted and already in use.

Context​

Three independent instances of the repo describing a world that does not exist:

  • The QA environment is gone — no EKS cluster, no RDS, zero EC2 in account 851236939056 — while the repo carries 22 qa overlays, an ArgoCD ApplicationSet and terraform/envs/qa, and the root CLAUDE.md documents a QA seed procedure. A change "promoted to qa" applies to nothing and reports success (AB#14576).
  • ci-cd/argocd/keynova-prod.yaml states "MANUAL sync for new git revisions — prod requires human approval" while setting syncPolicy.automated. Verified live: all 20 prod Applications are automated=true.
  • startupProbe.failureThreshold: 360 was sized on 2026-05-14 for an O(n²) cold boot that was fixed on 2026-05-16. Measured reality: median 33s. The constant was ~103× its justification and left a wedged pod unrestarted for an hour.

Decision​

  1. A declared environment must prove it has a cluster. A gate enumerates kubernetes/overlays/* and ci-cd/argocd/* and fails when a declared environment has no reachable control plane. Removal is then a decision, not a drift.
  2. A tuning constant carries the measurement that justifies it — the value, the date, and the reading. A constant whose justification names a fixed defect is thereby visible as stale.
  3. Verification reads the effect, not the actor's self-report. Synced means ArgoCD finished its step, not that the intended state is true. An inactive alert may mean the condition is absent or that its metric does not exist. A green run that never asserted anything is not evidence. Concretely: verify the live object, assert the series exists, and keep a positive control — an alert that has never fired is unproven.

Consequences​

  • Point 3 is already being applied and has repeatedly changed conclusions in this programme — it caught a config change that ArgoCD reported as applied and had not been, and a git push that reported success having pushed nothing.
  • Points 1 and 2 are new gates and add CI surface.
  • Some existing declarations will fail the gate on first run. That is the point.

What has actually been corrected so far​

Honesty matters more here than a tidy status column.

State
Probe right-sizing (initialDelaySeconds 30→1, failureThreshold 360→18)Shipped to test + staging, 17/17 each, 0 restarts, 16h soak. Prod pending approval.
ADR-1 metrics wiringDiagnosed, root cause proven. Not built.
ADR-2 durable-by-defaultDecided. Not built.
ADR-3 error postureDecided. Not built.
ADR-4 declaration gatesDiscipline in use; gates not built.

The probe change is a tuning constant, not an architectural correction. Its real value was diagnostic: shipping two numbers surfaced three separate silent-failure modes in one afternoon. That is the argument for the four decisions above, not a substitute for them.

Order of work​

  1. ADR-1 — nothing else is verifiable without it.
  2. ADR-2 — the one actively losing data.
  3. ADR-3 — small, and prevents ADR-1 regressing into a new silent hole.
  4. ADR-4 — gates, once the above are stable.