Skip to content

Commit 7ba2f2e

Browse files
committed
fix: emit circuit breaker events outside the lock and count rejections per outage
Breaker transitions queue their events and the drivers emit them after releasing the sync lock, so a logging handler that sends a request through the same breaker no longer deadlocks. Rejection accounting restarts with each outage: the first rejection is always emitted and circuit.closed reports the outage's total in a new rejected attribute.
1 parent ed8e87d commit 7ba2f2e

6 files changed

Lines changed: 385 additions & 50 deletions

File tree

‎docs/observability.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ The logger and event names below are a stable public contract:
1111
| `httpware.circuit_breaker` | `circuit.opened` (WARNING), `circuit.rejected` (WARNING), `circuit.half_open` (INFO), `circuit.closed` (INFO) |
1212
| `httpware.timeout` | `timeout.exceeded` (WARNING) |
1313

14-
Each log record has an `event` field holding the event name, such as `event="circuit.opened"`, which you can filter on in your log aggregator. `circuit.rejected` is emitted at most once per second per circuit, so an open circuit under heavy traffic does not flood your logs; its `suppressed` attribute counts the rejections that were left out since the previous one. Events from `AsyncKeyedCircuitBreaker` and `KeyedCircuitBreaker` also carry `circuit_key`, the circuit they belong to. [Resilience](resilience.md) describes when each event fires.
14+
Each log record has an `event` field holding the event name, such as `event="circuit.opened"`, which you can filter on in your log aggregator. `circuit.rejected` is emitted at most once per second per circuit, so an open circuit under heavy traffic does not flood your logs. The first rejection of each outage is always emitted, `suppressed` counts the rejections left out since the previous one, and `circuit.closed` reports the outage's total in `rejected`. Events from `AsyncKeyedCircuitBreaker` and `KeyedCircuitBreaker` also carry `circuit_key`, the circuit they belong to. [Resilience](resilience.md) describes when each event fires.
1515

1616
```python
1717
import logging

‎docs/resilience.md‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,9 +193,13 @@ Emitted on logger `httpware.circuit_breaker`:
193193
| Event | When |
194194
|---|---|
195195
| `circuit.opened` | The failure threshold was reached and the circuit went from CLOSED to OPEN. |
196-
| `circuit.rejected` | A request failed fast because the circuit was OPEN or a HALF_OPEN probe was already running. Emitted at most once per second per circuit; its `suppressed` attribute counts the rejections since the previous `circuit.rejected` that were not emitted. |
196+
| `circuit.rejected` | A request failed fast because the circuit was OPEN or a HALF_OPEN probe was already running. Emitted at most once per second per circuit, and always for the first rejection of an outage. Its `suppressed` attribute counts the rejections in the same outage that were not emitted since the previous `circuit.rejected`. |
197197
| `circuit.half_open` | The reset timeout passed and the circuit went from OPEN to HALF_OPEN. |
198-
| `circuit.closed` | The success threshold was reached and the circuit went from HALF_OPEN to CLOSED. |
198+
| `circuit.closed` | The success threshold was reached and the circuit went from HALF_OPEN to CLOSED. Its `rejected` attribute is the number of requests rejected during the outage, emitted or not. |
199+
200+
An outage runs from the moment the circuit opens until it closes again; a probe failure that reopens it stays in the same outage.
201+
202+
The breaker emits its events after releasing its internal lock, so a logging handler may send requests through a client that uses the same breaker.
199203

200204
### Time-based failure-rate mode
201205

‎src/httpware/middleware/resilience/circuit_breaker.py‎

Lines changed: 100 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@
2222
An outcome applies only to the state its request was admitted under: a request still in
2323
flight when the circuit changes state finishes without touching the circuit.
2424
25-
circuit.rejected is emitted at most once per second per circuit; its `suppressed`
26-
attribute counts the rejections not emitted since the previous one.
25+
An outage episode runs from CLOSED to OPEN until the circuit is CLOSED again. Within one,
26+
circuit.rejected is emitted at most once per second; its `suppressed` attribute counts the
27+
rejections not emitted since the previous one, and the first rejection of an episode is
28+
always emitted. circuit.closed carries `rejected`, the episode's total rejection count.
2729
2830
Trip modes:
2931
Classic (default) — opens when consecutive counted-failures reach failure_threshold.
@@ -37,8 +39,9 @@
3739
The lock-free _CircuitBreakerState holds the transition logic, shared by both wrappers.
3840
AsyncCircuitBreaker relies on asyncio atomicity (no await inside a transition) plus a
3941
single-event-loop guard; CircuitBreaker (sync) serializes transitions with a
40-
threading.Lock. Both are sharable across clients (one shared circuit); a sync instance
41-
cannot be shared with an async one.
42+
threading.Lock and emits events only after releasing it, so a logging handler may send
43+
requests through the same breaker. Both are sharable across clients (one shared circuit);
44+
a sync instance cannot be shared with an async one.
4245
"""
4346

4447
import asyncio
@@ -156,21 +159,32 @@ def clear(self) -> None:
156159
self._failure = [0] * _BUCKET_COUNT
157160

158161

162+
_Event = tuple[str, int, str, dict[str, typing.Any]]
163+
164+
165+
class _HalfOpened(Exception): # noqa: N818
166+
"""Raised by `admit` when it moved the circuit to HALF_OPEN; carries the probe's ticket and events."""
167+
168+
def __init__(self, ticket: int, events: list[_Event] | None) -> None:
169+
self.ticket = ticket
170+
self.events = events
171+
172+
159173
class _RejectionError(Exception):
160-
"""Raised by `admit` to refuse a request; `message` is None when the event is suppressed."""
174+
"""Raised by `admit` to refuse a request, carrying the events to emit once the lock is released."""
161175

162-
def __init__(self, retry_after: float | None, message: str | None, suppressed: int) -> None:
176+
def __init__(self, retry_after: float | None, events: list[_Event] | None) -> None:
163177
self.retry_after = retry_after
164-
self.message = message
165-
self.suppressed = suppressed
178+
self.events = events
166179

167180

168181
class _CircuitBreakerState:
169182
"""Lock-free circuit-breaker state machine shared by the sync + async wrappers.
170183
171-
Every method is synchronous and performs no I/O beyond logging. The async wrapper
172-
calls these directly (atomic under a single event loop because no await occurs
173-
inside a transition); the sync wrapper wraps `admit` and `record` in a threading.Lock.
184+
Every method is synchronous and performs no I/O. The async wrapper calls these directly
185+
(atomic under a single event loop because no await occurs inside a transition); the sync
186+
wrapper wraps `admit` and `record` in a threading.Lock. Transitions queue their events;
187+
the caller takes them with the outcome and emits them outside any lock.
174188
`admit` hands out a ticket: the epoch, which every transition advances by two, plus one
175189
for the probe. `record` ignores a ticket from an earlier epoch.
176190
"""
@@ -223,25 +237,29 @@ def __init__( # noqa: PLR0913 — breaker state has many orthogonal knobs; a da
223237
self._epoch = 0
224238
self._rejected_emitted_at = -math.inf
225239
self._rejections_suppressed = 0
240+
self._episode_rejections = 0
241+
self._events: list[_Event] | None = None
226242

227243
@property
228244
def state(self) -> CircuitState:
229245
"""The circuit's current stored state (raw read; no lazy OPEN→HALF_OPEN transition)."""
230246
return self._state
231247

232248
def admit(self, request: httpx2.Request) -> int:
233-
"""Return the request's ticket, or raise _RejectionError. No await inside."""
249+
"""Return the request's ticket, or raise _RejectionError or _HalfOpened. No await inside."""
234250
if self._state is CircuitState.CLOSED:
235251
return self._epoch
236252
now = self._now()
237253
if self._state is CircuitState.OPEN:
238254
elapsed = now - self._opened_at
239255
if elapsed < self._reset_timeout:
240-
self._reject(now, max(0.0, self._reset_timeout - elapsed), "circuit open — rejecting request")
256+
self._reject(request, now, max(0.0, self._reset_timeout - elapsed), "circuit open — rejecting request")
241257
self._move_to(CircuitState.HALF_OPEN)
242-
self._emit(request, "circuit.half_open", logging.INFO, "circuit half-open — admitting probe", {})
243-
elif self._probe_in_flight:
244-
self._reject(now, None, "circuit half-open — rejecting request (probe in flight)")
258+
self._probe_in_flight = True
259+
self._queue(request, "circuit.half_open", logging.INFO, "circuit half-open — admitting probe", {})
260+
raise _HalfOpened(self._epoch + _PROBE, self.take_events())
261+
if self._probe_in_flight:
262+
self._reject(request, now, None, "circuit half-open — rejecting request (probe in flight)")
245263
self._probe_in_flight = True
246264
return self._epoch + _PROBE
247265

@@ -255,8 +273,11 @@ def classify(self, exc: BaseException | None) -> int:
255273
return _FAILURE
256274
return _FOREIGN
257275

258-
def record(self, ticket: int, outcome: int, request: httpx2.Request) -> None:
259-
"""Apply an admitted request's outcome; a no-op if the circuit has transitioned since."""
276+
def record(self, ticket: int, outcome: int, request: httpx2.Request) -> list[_Event] | None:
277+
"""Apply an admitted request's outcome and return the events it queued.
278+
279+
A no-op if the circuit has transitioned since the ticket was issued.
280+
"""
260281
if ticket == self._epoch:
261282
if outcome == _SUCCESS and not self._rate_mode:
262283
self._consecutive_failures = 0
@@ -268,26 +289,26 @@ def record(self, ticket: int, outcome: int, request: httpx2.Request) -> None:
268289
self._record_probe_success(request)
269290
elif outcome == _FAILURE:
270291
self._open(request, failures=1) # 1 = the single probe failure that re-opened the circuit
271-
272-
def rejected(self, request: httpx2.Request, rejection: _RejectionError) -> CircuitOpenError:
273-
"""Emit the rejection's event unless `admit` suppressed it; return the error to raise."""
274-
if rejection.message is not None:
275-
self._emit(
276-
request,
277-
"circuit.rejected",
278-
logging.WARNING,
279-
rejection.message,
280-
{"retry_after": rejection.retry_after, "suppressed": rejection.suppressed},
281-
)
282-
return CircuitOpenError(retry_after=rejection.retry_after)
283-
284-
def _reject(self, now: float, retry_after: float | None, message: str) -> typing.NoReturn:
292+
events = self._events
293+
if events is not None:
294+
self._events = None
295+
return events
296+
297+
def take_events(self) -> list[_Event] | None:
298+
"""Return and clear the queued events."""
299+
events, self._events = self._events, None
300+
return events
301+
302+
def _reject(self, request: httpx2.Request, now: float, retry_after: float | None, message: str) -> typing.NoReturn:
303+
self._episode_rejections += 1
285304
if now - self._rejected_emitted_at < _REJECTED_EVENT_INTERVAL:
286305
self._rejections_suppressed += 1
287-
raise _RejectionError(retry_after, None, 0)
306+
raise _RejectionError(retry_after, None)
288307
self._rejected_emitted_at = now
289308
suppressed, self._rejections_suppressed = self._rejections_suppressed, 0
290-
raise _RejectionError(retry_after, message, suppressed)
309+
attributes = {"retry_after": retry_after, "suppressed": suppressed}
310+
self._queue(request, "circuit.rejected", logging.WARNING, message, attributes)
311+
raise _RejectionError(retry_after, self.take_events())
291312

292313
def _record_closed(self, request: httpx2.Request, *, failed: bool) -> None:
293314
if self._rate_mode:
@@ -305,18 +326,23 @@ def _record_probe_success(self, request: httpx2.Request) -> None:
305326
self._consecutive_successes = 0
306327
if self._rate_mode:
307328
self._window.clear() # ty: ignore[unresolved-attribute]
308-
self._emit(request, "circuit.closed", logging.INFO, "circuit closed — service recovered", {})
329+
attributes = {"rejected": self._episode_rejections}
330+
self._queue(request, "circuit.closed", logging.INFO, "circuit closed — service recovered", attributes)
309331

310332
def _move_to(self, state: CircuitState) -> None:
311333
self._state = state
312334
self._epoch += _EPOCH_STEP
313335

314336
def _enter_open(self, request: httpx2.Request, message: str, attributes: dict[str, typing.Any]) -> None:
337+
if self._state is CircuitState.CLOSED:
338+
self._rejected_emitted_at = -math.inf
339+
self._rejections_suppressed = 0
340+
self._episode_rejections = 0
315341
self._move_to(CircuitState.OPEN)
316342
self._opened_at = self._now()
317343
self._consecutive_failures = 0
318344
self._consecutive_successes = 0
319-
self._emit(request, "circuit.opened", logging.WARNING, message, attributes)
345+
self._queue(request, "circuit.opened", logging.WARNING, message, attributes)
320346

321347
def _open(self, request: httpx2.Request, *, failures: int) -> None:
322348
self._enter_open(
@@ -346,7 +372,7 @@ def _record_outcome(self, request: httpx2.Request, *, failed: bool) -> None:
346372
if threshold is not None and total >= self._minimum_calls and failures / total >= threshold:
347373
self._open_rate(request, total=total, failures=failures)
348374

349-
def _emit(
375+
def _queue(
350376
self,
351377
request: httpx2.Request,
352378
event_name: str,
@@ -357,20 +383,45 @@ def _emit(
357383
attributes = {**attributes, "method": request.method, "url": str(request.url)}
358384
if self._key is not None:
359385
attributes["circuit_key"] = self._key
386+
if self._events is None:
387+
self._events = []
388+
self._events.append((event_name, level, message, attributes))
389+
390+
391+
def _emit_events(events: list[_Event]) -> None:
392+
for event_name, level, message, attributes in events:
360393
_emit_event(_LOGGER, event_name, level=level, message=message, attributes=attributes)
361394

362395

396+
def _half_opened(half_opened: _HalfOpened) -> int:
397+
if half_opened.events is not None:
398+
_emit_events(half_opened.events)
399+
return half_opened.ticket
400+
401+
402+
def _rejected(rejection: _RejectionError) -> CircuitOpenError:
403+
if rejection.events is not None:
404+
_emit_events(rejection.events)
405+
return CircuitOpenError(retry_after=rejection.retry_after)
406+
407+
363408
async def _call_async(state: _CircuitBreakerState, request: httpx2.Request, next: AsyncNext) -> httpx2.Response: # noqa: A002
364409
try:
365410
ticket = state.admit(request)
366411
except _RejectionError as rejection:
367-
raise state.rejected(request, rejection) from None
412+
raise _rejected(rejection) from None
413+
except _HalfOpened as half_opened:
414+
ticket = _half_opened(half_opened)
368415
try:
369416
response = await next(request)
370417
except BaseException as exc:
371-
state.record(ticket, state.classify(exc), request)
418+
events = state.record(ticket, state.classify(exc), request)
419+
if events is not None:
420+
_emit_events(events)
372421
raise
373-
state.record(ticket, _SUCCESS, request)
422+
events = state.record(ticket, _SUCCESS, request)
423+
if events is not None:
424+
_emit_events(events)
374425
return response
375426

376427

@@ -384,16 +435,22 @@ def _call_sync(
384435
with lock:
385436
ticket = state.admit(request)
386437
except _RejectionError as rejection:
387-
raise state.rejected(request, rejection) from None
438+
raise _rejected(rejection) from None
439+
except _HalfOpened as half_opened:
440+
ticket = _half_opened(half_opened)
388441
try:
389442
response = next(request)
390443
except BaseException as exc:
391444
outcome = state.classify(exc)
392445
with lock:
393-
state.record(ticket, outcome, request)
446+
events = state.record(ticket, outcome, request)
447+
if events is not None:
448+
_emit_events(events)
394449
raise
395450
with lock:
396-
state.record(ticket, _SUCCESS, request)
451+
events = state.record(ticket, _SUCCESS, request)
452+
if events is not None:
453+
_emit_events(events)
397454
return response
398455

399456

‎tests/test_circuit_breaker.py‎

Lines changed: 80 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -744,8 +744,7 @@ async def _call(self) -> None:
744744
async def start(self) -> int:
745745
"""Start a call; return its index once it is blocked in `next` (or already rejected).
746746
747-
The task swallows the breaker's error itself: on 3.11, awaiting a task that failed loses
748-
coverage line events in the awaiting test.
747+
The task swallows the breaker's error itself; on 3.11 awaiting a failed task loses coverage.
749748
"""
750749
self._tasks.append(asyncio.create_task(self._call()))
751750
await asyncio.sleep(0)
@@ -897,3 +896,82 @@ async def _unreachable(request: httpx2.Request) -> httpx2.Response: # pragma: n
897896
await breaker(httpx2.Request("GET", "https://example.test/x"), _unreachable)
898897
rejected = [r for r in caplog.records if r.event == "circuit.rejected"] # ty: ignore[unresolved-attribute]
899898
assert [r.suppressed for r in rejected] == [0, 4] # ty: ignore[unresolved-attribute]
899+
900+
901+
# ── rejection accounting runs per outage episode ──
902+
903+
904+
async def _ok(request: httpx2.Request) -> httpx2.Response:
905+
return httpx2.Response(HTTPStatus.OK, request=request)
906+
907+
908+
async def _reject(breaker: AsyncCircuitBreaker) -> None:
909+
async def _unreachable(request: httpx2.Request) -> httpx2.Response: # pragma: no cover — OPEN never forwards
910+
return httpx2.Response(HTTPStatus.OK, request=request)
911+
912+
with pytest.raises(CircuitOpenError):
913+
await breaker(httpx2.Request("GET", "https://example.test/x"), _unreachable)
914+
915+
916+
def _events(caplog: pytest.LogCaptureFixture, name: str) -> list[logging.LogRecord]:
917+
return [r for r in caplog.records if getattr(r, "event", None) == name]
918+
919+
920+
async def test_first_rejection_of_a_new_outage_is_emitted(caplog: pytest.LogCaptureFixture) -> None:
921+
clock = _Clock()
922+
breaker = AsyncCircuitBreaker(failure_threshold=1, reset_timeout=0.1, _now=clock)
923+
with caplog.at_level(logging.INFO, logger="httpware.circuit_breaker"):
924+
await _open_now(breaker)
925+
await _reject(breaker)
926+
clock.advance(0.1)
927+
await breaker(httpx2.Request("GET", "https://example.test/x"), _ok)
928+
await _open_now(breaker)
929+
await _reject(breaker)
930+
assert [r.suppressed for r in _events(caplog, "circuit.rejected")] == [0, 0] # ty: ignore[unresolved-attribute]
931+
932+
933+
async def test_closed_event_reports_every_rejection_of_the_outage(caplog: pytest.LogCaptureFixture) -> None:
934+
clock = _Clock()
935+
breaker = AsyncCircuitBreaker(failure_threshold=1, reset_timeout=5.0, _now=clock)
936+
with caplog.at_level(logging.INFO, logger="httpware.circuit_breaker"):
937+
await _open_now(breaker)
938+
await _reject(breaker)
939+
await _reject(breaker)
940+
clock.advance(5.0)
941+
await _open_now(breaker)
942+
await _reject(breaker)
943+
await _reject(breaker)
944+
clock.advance(5.0)
945+
await breaker(httpx2.Request("GET", "https://example.test/x"), _ok)
946+
assert [r.suppressed for r in _events(caplog, "circuit.rejected")] == [0, 1] # ty: ignore[unresolved-attribute]
947+
assert [r.rejected for r in _events(caplog, "circuit.closed")] == [4] # ty: ignore[unresolved-attribute]
948+
949+
950+
async def test_suppressed_never_counts_a_previous_outage(caplog: pytest.LogCaptureFixture) -> None:
951+
clock = _Clock()
952+
breaker = AsyncCircuitBreaker(failure_threshold=1, reset_timeout=5.0, _now=clock)
953+
with caplog.at_level(logging.INFO, logger="httpware.circuit_breaker"):
954+
await _open_now(breaker)
955+
for _ in range(3):
956+
await _reject(breaker)
957+
clock.advance(5.0)
958+
await breaker(httpx2.Request("GET", "https://example.test/x"), _ok)
959+
await _open_now(breaker)
960+
await _reject(breaker)
961+
assert [r.suppressed for r in _events(caplog, "circuit.rejected")] == [0, 0] # ty: ignore[unresolved-attribute]
962+
963+
964+
async def test_events_of_one_call_arrive_in_transition_order(caplog: pytest.LogCaptureFixture) -> None:
965+
clock = _Clock()
966+
breaker = AsyncCircuitBreaker(failure_threshold=1, reset_timeout=1.0, _now=clock)
967+
with caplog.at_level(logging.INFO, logger="httpware.circuit_breaker"):
968+
await _open_now(breaker)
969+
await _reject(breaker)
970+
clock.advance(1.0)
971+
await breaker(httpx2.Request("GET", "https://example.test/x"), _ok)
972+
assert [r.event for r in caplog.records] == [ # ty: ignore[unresolved-attribute]
973+
"circuit.opened",
974+
"circuit.rejected",
975+
"circuit.half_open",
976+
"circuit.closed",
977+
]

0 commit comments

Comments
 (0)