Context
#22 / PR #23 add bounded, asynchronous log emission. The failure handling is deliberately fail-open and one-way: after DESTINATION_FAILURE_LIMIT (3) consecutive failures, a destination is disabled — the async wrapper trips, the manager removes the destination and falls back to stdout — and it stays disabled until something calls restart_observability(), which rebuilds all destinations from config with fresh clients.
That manual-recovery design is the right first step (predictable, no thundering probes during an outage), but it means a transient Cloud Logging incident permanently downgrades a long-lived process to stdout-only until a deploy, snapshot restore, or explicit restart call.
Proposal
Add an automatic half-open recovery path on top of the existing breaker:
- After a destination is disabled, start a cooldown (e.g. 60s, exponential up to a cap).
- When the cooldown elapses, the next emit (or a lightweight timer) sends a single probe record through a freshly built destination (half-open state).
- Probe succeeds → destination is reinstated and the cooldown resets. Probe fails → destination stays disabled and the cooldown doubles.
Design constraints carried over from #22:
- Fail-open above all: probing must never add latency or exceptions to the caller's path (probe from the background worker or a timer thread, never the request thread).
- Rebuild, don't poke: reinstatement should construct a fresh destination (fresh client/transport) exactly like
restart() does, not resurrect the tripped instance — tripped BackgroundEmitDestination wrappers are terminal by design.
- One probe at a time per destination (no thundering herd from concurrent emits observing an elapsed cooldown).
- Telemetry: report state transitions (
half_open_probe, destination_reinstated) through the same internal-error channel as destination_disabled.
Why not in #23
The breaker + manual restart_observability() covers the immediate consumers: Modal workers call restart in their post-snapshot hook anyway, and Cloud Run instances are short-lived enough that a disabled destination rarely outlives the incident. Automatic recovery adds a state machine (closed/open/half-open, cooldown clock, probe serialization) that deserves its own review.
Context
#22 / PR #23 add bounded, asynchronous log emission. The failure handling is deliberately fail-open and one-way: after
DESTINATION_FAILURE_LIMIT(3) consecutive failures, a destination is disabled — the async wrapper trips, the manager removes the destination and falls back to stdout — and it stays disabled until something callsrestart_observability(), which rebuilds all destinations from config with fresh clients.That manual-recovery design is the right first step (predictable, no thundering probes during an outage), but it means a transient Cloud Logging incident permanently downgrades a long-lived process to stdout-only until a deploy, snapshot restore, or explicit restart call.
Proposal
Add an automatic half-open recovery path on top of the existing breaker:
Design constraints carried over from #22:
restart()does, not resurrect the tripped instance — trippedBackgroundEmitDestinationwrappers are terminal by design.half_open_probe,destination_reinstated) through the same internal-error channel asdestination_disabled.Why not in #23
The breaker + manual
restart_observability()covers the immediate consumers: Modal workers call restart in their post-snapshot hook anyway, and Cloud Run instances are short-lived enough that a disabled destination rarely outlives the incident. Automatic recovery adds a state machine (closed/open/half-open, cooldown clock, probe serialization) that deserves its own review.