Skip to content

fix(pipeline): pump _stream_with_keepalive source from one task, not one per item - #810

Open
sjawhar wants to merge 2 commits into
LuthienResearch:mainfrom
trajectory-labs-pbc:fix/streaming-otel-context-detach
Open

fix(pipeline): pump _stream_with_keepalive source from one task, not one per item#810
sjawhar wants to merge 2 commits into
LuthienResearch:mainfrom
trajectory-labs-pbc:fix/streaming-otel-context-detach

Conversation

@sjawhar

@sjawhar sjawhar commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Problem

opentelemetry.context logs Failed to detach context (ERROR, with a full
traceback) 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_logger wasn't
applied to it (#807).

Root cause

AnthropicClient.stream() and _AnthropicPolicyIO._stream() each hold an
OpenTelemetry span open across every chunk of the upstream response:

with tracer.start_as_current_span("anthropic.stream"):
    async for event in stream:
        yield event

_stream_with_keepalive (introduced in #804) drove that generator by
wrapping every single __anext__() call in a fresh
asyncio.ensure_future(...) Task:

if pending is None:
    pending = asyncio.ensure_future(_anext_or_done(iterator))
item = await asyncio.wait_for(asyncio.shield(pending), interval_seconds)
pending = None  # next item gets a brand-new Task

asyncio.Task copies contextvars.Context at creation. The span's
context-attach token is created in whichever Task fetches the first chunk;
the span's __exit__ (on the last chunk / StopAsyncIteration) runs in
whichever Task fetches the last chunk — a different Context object.
contextvars.Context.reset() raises ValueError in that case, and
opentelemetry.context.detach() catches it and logs it at ERROR rather than
propagating 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_keepalive with a real
TracerProvider, no Sentry involved — same ValueError and the same
opentelemetry/context/__init__.py:157 traceback 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's
OpenTelemetryIntegration (which would install its own OTel context/span
processor) never activates, and Sentry's AsyncioIntegration task factory
wraps coroutines without touching opentelemetry.context at all. The storm
is asyncio/OTel-only and would reproduce on any environment carrying real
stream=True traffic once #804 is deployed, independent of Sentry.

Fix

_stream_with_keepalive now drives source from a single persistent
background task (_pump_to_queue) that feeds a one-slot asyncio.Queue,
instead of a fresh task per item. This keeps every span's attach/detach pair
inside that one task's contextvars.Context for the generator's entire
lifetime, no matter how many keepalive timeouts happen in between.

Preserves the existing contract:

  • A slow item is never dropped (the pump keeps running independently of the
    consumer's per-iteration timeout).
  • Closing the consumer (aclose()) cancels the in-flight pump and propagates
    cancellation into source.
  • Exceptions raised by source (including a spontaneous CancelledError, as
    opposed 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), which
introduced _stream_with_keepalive/_anext_or_done. It should land after
#804 merges; the diff below includes #804's commits until then.

Verification

  • New regression test
    (TestStreamWithKeepalive::test_span_spanning_multiple_yields_detaches_cleanly)
    reproduces the exact Failed to detach context ERROR against the pre-fix
    code and passes after the fix.
  • New test (test_source_exception_propagates) covers exception relay
    through 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-order
    failures 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_vars and two in
    test_config_registry.py), each confirmed passing in isolation; unrelated
    to this change.

Changelog fragment included.

sjawhar added 2 commits July 8, 2026 17:55
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
sjawhar marked this pull request as ready for review August 23, 2026 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant