Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,16 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

### Fixed

- **`DISABLED` no longer claims to be a metrics no-op.** `State.DISABLED` and
`disable()` were documented as "admit all traffic, record nothing", but only
the sliding window ever went quiet: every admitted call is still classified,
timed and delivered to the `EventListener` as `on_call`. So an operator who
reached for `disable()` to silence a noisy exporter kept seeing its metrics,
and one who switched a rollout from `METRICS_ONLY` to `DISABLED` had no
documented promise that listener-fed dashboards would survive it. The
docstrings and the states guide now separate the two surfaces: `DISABLED`
records no outcome — thresholds are never evaluated and `snapshot()` gets
nothing new — and leaves listener events flowing. Behaviour is unchanged.
- **A bug in your own code no longer opens the circuit of a healthy
dependency.** The HTTP integrations counted every exception raised inside the
guarded call as the dependency failing, including the ones the client library
Expand Down
28 changes: 27 additions & 1 deletion docs/guides/states.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ Three special states are set manually and stay until you `reset()`:
| Method | State | Behaviour |
|--------|-------|-----------|
| `breaker.force_open()` | `FORCED_OPEN` | Reject all traffic regardless of metrics. |
| `breaker.disable()` | `DISABLED` | Admit all traffic, record nothing — the breaker is a no-op. |
| `breaker.disable()` | `DISABLED` | Admit all traffic but record no outcome — thresholds are never evaluated and `snapshot()` gets nothing new. Listener `on_call` events still fire. |
| `breaker.metrics_only()` | `METRICS_ONLY` | Admit all traffic, record metrics, but never trip. |
| `breaker.reset()` | `CLOSED` | Return to closed with a fresh, empty window. In coordinated mode, resume the cached shared state instead. |

Expand All @@ -74,6 +74,32 @@ breaker.metrics_only() # observe in production without enforcing
breaker.reset() # start enforcing with a clean window
```

### What an override does to your metrics

Two observability surfaces are in play, and an override does not move them
together:

- the sliding **window** — what `snapshot()` reports and what the thresholds
read. `METRICS_ONLY` keeps filling it (that is the whole point of shadow
mode); `DISABLED` records nothing and `FORCED_OPEN` admits nothing to record,
so neither feeds it. A count-based window then keeps its last contents
unchanged; a time-based one drains as its buckets expire;
- the **`EventListener`**, which observes calls rather than the window.
`on_call` fires whenever an admitted call settles, whatever the state —
`DISABLED` included; `on_rejected` fires for every rejected call,
`FORCED_OPEN` included.

So `disable()` is not a way to silence a listener: `LoggingEventListener`, the
`OTelEventListener` or a Prometheus exporter keeps reporting outcomes and
durations for a disabled breaker, with the classifier still deciding success
from failure. That is deliberate — dashboards going dark the moment an operator
disables a breaker looks exactly like an outage. To stop the events, drop the
listener instead (construct the breaker without one).

Switching a rollout from `metrics_only()` to `disable()` therefore keeps
listener-exported dashboards alive, and only stops threshold evaluation and
`snapshot()`.
Comment on lines +99 to +101

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Clarify that snapshot() remains available.

disable() stops new window data and threshold evaluation. It does not stop snapshot(). Count-based data remains, and time-based data can expire.

The generated mirror repeats this wording in docs/llms-full.txt Lines 1671-1673.

Proposed wording
-`snapshot()`.
+further `snapshot()` updates.

As per path instructions: user-facing documentation must describe current behavior accurately and generated mirrors must stay synchronized.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Switching a rollout from `metrics_only()` to `disable()` therefore keeps
listener-exported dashboards alive, and only stops threshold evaluation and
`snapshot()`.
Switching a rollout from `metrics_only()` to `disable()` therefore keeps
listener-exported dashboards alive, and only stops threshold evaluation and
further `snapshot()` updates.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/guides/states.md` around lines 99 - 101, Update the rollout behavior
description in the states documentation to explicitly state that disable() stops
new window data and threshold evaluation but leaves snapshot() available;
clarify that count-based data remains while time-based data may expire. Apply
the same wording to the corresponding generated section in docs/llms-full.txt so
both documents stay synchronized.

Source: Path instructions


### Safe rollout

Shadow mode is the key to introducing a breaker without risk: it records the
Expand Down
28 changes: 27 additions & 1 deletion docs/llms-full.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1636,7 +1636,7 @@ Three special states are set manually and stay until you `reset()`:
| Method | State | Behaviour |
|--------|-------|-----------|
| `breaker.force_open()` | `FORCED_OPEN` | Reject all traffic regardless of metrics. |
| `breaker.disable()` | `DISABLED` | Admit all traffic, record nothing — the breaker is a no-op. |
| `breaker.disable()` | `DISABLED` | Admit all traffic but record no outcome — thresholds are never evaluated and `snapshot()` gets nothing new. Listener `on_call` events still fire. |
| `breaker.metrics_only()` | `METRICS_ONLY` | Admit all traffic, record metrics, but never trip. |
| `breaker.reset()` | `CLOSED` | Return to closed with a fresh, empty window. In coordinated mode, resume the cached shared state instead. |

Expand All @@ -1646,6 +1646,32 @@ breaker.metrics_only() # observe in production without enforcing
breaker.reset() # start enforcing with a clean window
```

### What an override does to your metrics

Two observability surfaces are in play, and an override does not move them
together:

- the sliding **window** — what `snapshot()` reports and what the thresholds
read. `METRICS_ONLY` keeps filling it (that is the whole point of shadow
mode); `DISABLED` records nothing and `FORCED_OPEN` admits nothing to record,
so neither feeds it. A count-based window then keeps its last contents
unchanged; a time-based one drains as its buckets expire;
- the **`EventListener`**, which observes calls rather than the window.
`on_call` fires whenever an admitted call settles, whatever the state —
`DISABLED` included; `on_rejected` fires for every rejected call,
`FORCED_OPEN` included.

So `disable()` is not a way to silence a listener: `LoggingEventListener`, the
`OTelEventListener` or a Prometheus exporter keeps reporting outcomes and
durations for a disabled breaker, with the classifier still deciding success
from failure. That is deliberate — dashboards going dark the moment an operator
disables a breaker looks exactly like an outage. To stop the events, drop the
listener instead (construct the breaker without one).

Switching a rollout from `metrics_only()` to `disable()` therefore keeps
listener-exported dashboards alive, and only stops threshold evaluation and
`snapshot()`.

### Safe rollout

Shadow mode is the key to introducing a breaker without risk: it records the
Expand Down
6 changes: 5 additions & 1 deletion interlock/_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,11 @@ def force_open(self) -> None:
self._override(self._machine.force_open)

def disable(self) -> None:
"""Override to ``DISABLED``: admit all traffic, record nothing."""
"""Override to ``DISABLED``: admit all traffic, record no outcome.

``_settle`` still classifies the outcome and notifies ``on_call``; only
the window — and with it the thresholds and ``snapshot()`` — goes quiet.
"""
self._override(self._machine.disable)

def metrics_only(self) -> None:
Expand Down
11 changes: 8 additions & 3 deletions interlock/_state_machine.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
then closes or reopens based on the probes' rates.

Three special states are operator overrides: ``FORCED_OPEN`` (reject all),
``DISABLED`` (admit all, no metrics), ``METRICS_ONLY`` (admit all, record
metrics, never trip).
``DISABLED`` (admit all, record nothing in the window), ``METRICS_ONLY``
(admit all, record metrics, never trip).
"""

from interlock._initial_state import validate_initial_state
Expand Down Expand Up @@ -167,7 +167,12 @@ def force_open(self) -> None:
self._reset_probes()

def disable(self) -> None:
"""Override to ``DISABLED``: admit all traffic, record nothing."""
"""Override to ``DISABLED``: admit all traffic, record nothing.

Nothing reaches the window, so no threshold is ever evaluated and
``snapshot()`` gets nothing new. What the call layer does with an
outcome — notifying a listener, say — is not this machine's business.
"""
self._state = State.DISABLED
self._generation += 1
self._reset_probes()
Expand Down
8 changes: 7 additions & 1 deletion interlock/breaker.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,13 @@ def force_open(self) -> None:
self._engine.force_open()

def disable(self) -> None:
"""Disable the breaker: admit all traffic and record nothing."""
"""Disable the breaker: admit all traffic, record no outcome.

Thresholds are never evaluated and ``snapshot()`` gets nothing new (a
time-based window drains as its buckets expire). Listener ``on_call``
events keep firing — ``disable()`` silences the breaker, not its
observability.
"""
self._engine.disable()

def metrics_only(self) -> None:
Expand Down
4 changes: 3 additions & 1 deletion interlock/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ class State(StrEnum):
overrides for safe rollout and manual control:

- ``FORCED_OPEN``: rejects all traffic regardless of metrics.
- ``DISABLED``: passes all traffic, no metrics, breaker is a no-op.
- ``DISABLED``: passes all traffic but records no outcome — thresholds are
never evaluated and ``snapshot()`` gets nothing new. Listener ``on_call``
events still fire, so external dashboards stay live.
- ``METRICS_ONLY``: shadow/observe mode — passes all traffic and records
metrics, but never trips. The key to tuning thresholds before enforcing.

Expand Down
15 changes: 15 additions & 0 deletions tests/test_observability.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,21 @@ def boom() -> None:
assert breaker.snapshot().total_calls == 0


def test__disable__admitted_call__still_reports_on_call(
breaker: CircuitBreaker, fake_clock: FakeClock, listener: RecordingListener
) -> None:
breaker.disable()

def work() -> int:
fake_clock.advance(0.25)
return 1

breaker.call(work)

assert listener.calls == [(Outcome.SUCCESS, 0.25)]
assert breaker.snapshot().total_calls == 0


def test__metrics_only__records_but_never_trips(breaker: CircuitBreaker) -> None:
breaker.metrics_only()

Expand Down
Loading