fix(pipeline): pump _stream_with_keepalive source from one task, not one per item - #810
Open
sjawhar wants to merge 2 commits into
Open
Conversation
Slow models emit only wire `ping` keepalives before content; the Anthropic SDK's typed stream drops them, so the proxy went silent for the whole pre-content phase and intermediaries (the ALB idle timeout) cut healthy long streams mid-flight. Emit an Anthropic-style ping when the upstream is idle > STREAM_KEEPALIVE_SECONDS.
…one per item Fixes the opentelemetry.context "Failed to detach context" ERROR storm (~60k ERROR lines/48h in production, roughly twice per streaming request). AnthropicClient.stream() and _AnthropicPolicyIO._stream() each hold an OTel span open across every chunk of the upstream response. _stream_with_keepalive wrapped every single upstream __anext__() in a fresh asyncio.ensure_future() Task; asyncio.Task copies contextvars.Context at creation, so a span attach-token created in the first Task could not be detached from whichever Task happened to fetch the last chunk -- contextvars.Context.reset() raises ValueError, which opentelemetry.context.detach() catches and logs at ERROR instead of propagating. Reproduced directly against _stream_with_keepalive with a real TracerProvider and no Sentry involved -- same ERROR line and traceback as production. Sentry initialization order/scope interaction was the leading hypothesis but is not the mechanism: our init_sentry() does not enable any OpenTelemetry-backed Sentry tracing (that path requires the _experiments.otel_powered_performance flag, which we never set), so Sentry cannot be attaching/detaching this context. Fix: pump the upstream generator to completion from a single persistent task (_pump_to_queue) feeding a one-slot queue, instead of a fresh task per item, so every span attach/detach pair stays inside one task context. Preserves the existing never-drop-a-slow-item and cancel-on-close contracts (regression tests included).
sjawhar
marked this pull request as ready for review
August 23, 2026 15:45
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
opentelemetry.contextlogsFailed to detach context(ERROR, with a fulltraceback) roughly twice per streaming request. In production this produced
~60k ERROR lines per 48h, continuously, since the streaming-keepalive change
shipped (#804) — noise that also feeds a Datadog error-rate monitor (which
now excludes this logger by attribute purely because of the volume) and,
separately, exhausted the Sentry error quota when
ignore_loggerwasn'tapplied to it (#807).
Root cause
AnthropicClient.stream()and_AnthropicPolicyIO._stream()each hold anOpenTelemetry span open across every chunk of the upstream response:
_stream_with_keepalive(introduced in #804) drove that generator bywrapping every single
__anext__()call in a freshasyncio.ensure_future(...)Task:asyncio.Taskcopiescontextvars.Contextat creation. The span'scontext-attach token is created in whichever Task fetches the first chunk;
the span's
__exit__(on the last chunk /StopAsyncIteration) runs inwhichever Task fetches the last chunk — a different
Contextobject.contextvars.Context.reset()raisesValueErrorin that case, andopentelemetry.context.detach()catches it and logs it at ERROR rather thanpropagating it — hence the silent-but-noisy storm (the request itself
completes with a 200; otel-collector shows no export failures).
Reproduced directly against
_stream_with_keepalivewith a realTracerProvider, no Sentry involved — sameValueErrorand the sameopentelemetry/context/__init__.py:157traceback line seen in production.Sentry enablement was the leading hypothesis (present in prod, absent in a
Sentry-less staging comparison), but isn't the mechanism: our
init_sentry()doesn't set
_experiments.otel_powered_performance, so Sentry'sOpenTelemetryIntegration(which would install its own OTel context/spanprocessor) never activates, and Sentry's
AsyncioIntegrationtask factorywraps coroutines without touching
opentelemetry.contextat all. The stormis asyncio/OTel-only and would reproduce on any environment carrying real
stream=Truetraffic once #804 is deployed, independent of Sentry.Fix
_stream_with_keepalivenow drivessourcefrom a single persistentbackground task (
_pump_to_queue) that feeds a one-slotasyncio.Queue,instead of a fresh task per item. This keeps every span's attach/detach pair
inside that one task's
contextvars.Contextfor the generator's entirelifetime, no matter how many keepalive timeouts happen in between.
Preserves the existing contract:
consumer's per-iteration timeout).
aclose()) cancels the in-flight pump and propagatescancellation into
source.source(including a spontaneousCancelledError, asopposed to this task being cancelled from the outside) propagate to the
consumer instead of hanging.
Depends on
This branch is based on #804 (
fix/anthropic-stream-keepalive), whichintroduced
_stream_with_keepalive/_anext_or_done. It should land after#804 merges; the diff below includes #804's commits until then.
Verification
(
TestStreamWithKeepalive::test_span_spanning_multiple_yields_detaches_cleanly)reproduces the exact
Failed to detach contextERROR against the pre-fixcode and passes after the fix.
test_source_exception_propagates) covers exception relaythrough the new queue-based pump.
uv run pytest tests/luthien_proxy/unit_tests/pipeline/test_anthropic_processor.py— all pass.uv run pytest tests/luthien_proxy/unit_tests(full unit suite) — all pass.uv run ruff check/uv run ruff format --check— clean on touched files.uv run pyright— 0 errors, 0 warnings, 0 informations../scripts/dev_checks.sh --fast— same 3 pre-existing, environment-orderfailures already called out in fix(sentry): never capture opentelemetry.context detach noise #807's verification notes
(
test_onboard.py::test_find_docker_ports_respects_env_varsand two intest_config_registry.py), each confirmed passing in isolation; unrelatedto this change.
Changelog fragment included.