Demonstrate streaming span attributes in the streaming quickstart - #2691
Open
connectsudhindra-gif wants to merge 10 commits into
Open
Demonstrate streaming span attributes in the streaming quickstart#2691connectsudhindra-gif wants to merge 10 commits into
connectsudhindra-gif wants to merge 10 commits into
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
dosubot
Bot
added
size:L
4 tasks
joshreini1
requested changes
Aug 17, 2026
joshreini1
left a comment
joshreini1
left a comment
Collaborator
There was a problem hiding this comment.
Clean streaming instrumentation with comprehensive test coverage. Correctly captures TTFT, chunks, and throughput by wrapping the stream iterator. One blocker: test at line 108 is named test_chunks_received_omitted_without_usage but asserts chunks_received IS present—rename to test_tokens_per_second_omitted_without_usage. Two should-fix items: clarify TTFT docstring (measures stream-available-to-first-chunk, not request-issuance) and add minimum duration guard in tokens/sec calculation to avoid misleading metrics on very fast streams. Request changes: 1 blocker.
| assert attrs[SpanAttributes.GENERATION.TOKENS_PER_SECOND] > 0 | ||
|
|
||
|
|
||
| @pytest.mark.optional |
Collaborator
There was a problem hiding this comment.
Rename to test_tokens_per_second_omitted_without_usage. The test actually verifies chunks_received IS recorded (asserts ==2) but tokens_per_second is not—the current name contradicts the assertion.
| """Whether this generation call used streaming (e.g. `stream=True`).""" | ||
|
|
||
| TIME_TO_FIRST_TOKEN_MS = base + ".time_to_first_token_ms" | ||
| """Milliseconds between issuing the request and receiving the first |
Collaborator
There was a problem hiding this comment.
Change docstring from "issuing the request" to "the streaming response being available" for accuracy. The implementation measures from when handle_response gets the stream object (after HTTP returns) to when next() yields the first chunk, not from initial request send.
| "Could not record streaming span attributes; the span may " | ||
| "have already ended.", | ||
| exc_info=True, | ||
| ) |
Collaborator
There was a problem hiding this comment.
Change duration_s > 0 to duration_s > 0.01 (or similar threshold). Prevents misleadingly high tokens/sec on very fast streams (e.g. single chunk completing in 0.0001s would show 100k tokens/sec). A 10ms minimum ensures the metric represents realistic throughput.
First step toward streaming instrumentation support (truera#2442). Adds the attribute keys the issue asks for as a pure, additive semconv change with no behavior change, so it can land and be reverted independently of the instrumentation work that will populate them: - ai.observability.generation.is_streaming - ai.observability.generation.time_to_first_token_ms - ai.observability.generation.tokens_per_second - ai.observability.generation.chunks_received Follow-up PRs will wire these up for the OpenAI provider's stream=True path and add an example notebook; LangChain streaming, the generic async-generator pattern, mid-stream guardrail evaluation, and the dashboard latency view are each separate, larger efforts tracked against truera#2442 but out of scope here.
connectsudhindra-gif
force-pushed
the
streaming-example-notebook
branch
from
August 18, 2026 02:33
ec1740a to
3092fb9
Compare
added 9 commits
August 17, 2026 21:53
…ument scope of internal-only ones Reviewer flagged 3 blockers: the custom ai.observability.generation.* streaming attributes don't align with the OTel GenAI spec's canonical gen_ai.request.stream (bool) / gen_ai.response.time_to_first_chunk (double, seconds), and the spec models per-chunk throughput as Histogram metrics rather than span attributes. - Added GenAIAttributes.REQUEST.STREAM (gen_ai.request.stream) and a new GenAIAttributes.RESPONSE class with TIME_TO_FIRST_CHUNK (gen_ai.response.time_to_first_chunk), matching this file's existing pattern of emitting gen_ai.* attributes alongside the ai.observability.* ones for interoperability. These are the canonical counterparts to IS_STREAMING and TIME_TO_FIRST_TOKEN_MS. - TOKENS_PER_SECOND and CHUNKS_RECEIVED have no canonical counterpart: the spec wants these as Histogram metrics (gen_ai.client.operation.time_per_output_chunk), and this codebase has no OTel Metrics/MeterProvider pipeline to emit them through yet. Documented them as TruLens-internal (ai.observability.* only) rather than silently leaving them looking like they're meant to be canonical semconv. Still additive/no-behavior-change, consistent with this PR's original scope -- nothing populates any of these attributes yet.
…uplicate them Reviewer's inline comments (not just the summary) explicitly objected to keeping ai.observability.generation.is_streaming and .time_to_first_token_ms alongside the new canonical gen_ai.* ones: "Duplicating it under the ai.observability.* namespace creates confusion and breaks interoperability with OTel-native tooling." Removed GENERATION.IS_STREAMING and GENERATION.TIME_TO_FIRST_TOKEN_MS entirely. GenAIAttributes.REQUEST.STREAM and GenAIAttributes.RESPONSE.TIME_TO_FIRST_CHUNK (from the previous commit) are now the sole representations. This is a breaking rename for the stacked instrumentation PRs (truera#2690/truera#2691/truera#2692), which are being updated to match in the same pass. TOKENS_PER_SECOND and CHUNKS_RECEIVED are intentionally left as-is (TruLens-internal, documented as such) rather than dropped per the reviewer's other comment -- replying on the review thread to explain why, since dropping them removes already-shipped, tested functionality rather than just renaming a key.
Second step toward streaming instrumentation support (truera#2442), built on the semconv attributes from the previous PR. Populates the new GENERATION span attributes for the OpenAI provider's sync stream=True path via OpenAICostComputer.handle_response, the existing hook that already runs on every instrumented openai.*.create() return value: - IS_STREAMING / TIME_TO_FIRST_TOKEN_MS are known synchronously (the existing code already blocks on the first chunk to read its model name for cost attribution, so timing that gives TTFT for free). - CHUNKS_RECEIVED / TOKENS_PER_SECOND require the full stream, which is only consumed later by the caller -- after the span-attribute hook has already returned. To handle this, the stream's iterator is wrapped to count chunks and, once exhausted, write those two attributes directly onto the span that was current when the request was issued (captured via `trace.get_current_span()`). TOKENS_PER_SECOND is only set when the caller requested `stream_options={"include_usage": True}`, since OpenAI doesn't send per-chunk token counts otherwise -- no token count is fabricated from chunk count. This only works when the caller consumes the stream before the enclosing instrumented span closes (the common pattern: an instrumented method that iterates the stream to build its return value). If the raw stream is instead handed off elsewhere uncontained (e.g. returned directly for pass-through streaming), the span has already ended by the time the stream finishes, and this degrades gracefully to a no-op for the two deferred attributes -- verified by test. Does not touch the AsyncOpenAI client path, which goes through a separate, ad-hoc instrumentation function in experimental/otel_tracing/core/session.py rather than OpenAICostComputer.handle_response; that's a separate follow-up.
- Rename test_chunks_received_omitted_without_usage to test_tokens_per_second_omitted_without_usage: the test asserts CHUNKS_RECEIVED is present and TOKENS_PER_SECOND is omitted, the opposite of what the old name said. - Remove the AsyncStream branch in _instrument_stream_span_attributes. handle_response only reaches this helper via the sync `__iter__` check, so an openai.AsyncStream can never reach it; async streaming is explicitly out of scope for this PR and goes through a separate instrumentation path.
…tream/TTFT SpanAttributes.GENERATION.IS_STREAMING and .TIME_TO_FIRST_TOKEN_MS no longer exist (removed upstream per review feedback). Switch to GenAIAttributes.REQUEST.STREAM (bool) and GenAIAttributes.RESPONSE.TIME_TO_FIRST_CHUNK (seconds, not ms -- drop the *1000 conversion). CHUNKS_RECEIVED/TOKENS_PER_SECOND are unaffected, still TruLens-internal.
Third step toward streaming instrumentation support (truera#2442), built on the previous two PRs (semconv attributes + OpenAI stream=True instrumentation). Extends the existing streaming_apps.ipynb (rather than adding a new notebook, since this one already demonstrates the exact pattern: an @instrument-decorated method that consumes an OpenAI stream=True completion) with a section that queries the recorded events for the new GENERATION span attributes -- is_streaming, TTFT, tokens_per_second, chunks_received -- and prints them. Also fixes a now-stale comment ("not yet tracked by trulens" on stream_options={"include_usage": True}) that the previous PR made inaccurate. Verified by running the notebook's actual code path end-to-end against a local Ollama server (OpenAI-compatible endpoint), confirming all four attributes are populated correctly: is_streaming: True time_to_first_token_ms: 2.44 tokens_per_second: 22.3 chunks_received: 4
The basic unit test job doesn't install the openai extra, so the bare `import openai` at module scope was blowing up collection for the whole run before pytest could even see the @pytest.mark.optional markers on the individual tests. Defer the import behind a try/except + OPENAI_AVAILABLE flag and skip the module via pytestmark, matching the existing convention in test_async_openai_capabilities.py.
_instrument_stream_span_attributes divided completion_tokens by elapsed duration with only a `duration_s > 0` guard, which lets a near-instant stream produce a misleadingly huge tokens/sec figure. Added a _MIN_TOKENS_PER_SECOND_DURATION_S floor below which TOKENS_PER_SECOND is simply not recorded. Updated the affected test to fake the clock (real synchronous mock consumption is faster than the new duration floor). (The TTFT docstring clarification from the original version of this commit is dropped: TIME_TO_FIRST_TOKEN_MS no longer exists -- it was replaced by the canonical GenAIAttributes.RESPONSE.TIME_TO_FIRST_CHUNK per truera#2689's review feedback.)
SpanAttributes.GENERATION.IS_STREAMING and .TIME_TO_FIRST_TOKEN_MS no longer exist. Switch the notebook's streaming-metrics cell to GenAIAttributes.REQUEST.STREAM / .RESPONSE.TIME_TO_FIRST_CHUNK.
connectsudhindra-gif
force-pushed
the
streaming-example-notebook
branch
from
August 18, 2026 03:01
3092fb9 to
31d912d
Compare
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
2 participants
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.
Summary
Part of #2442 (streaming LLM instrumentation), PR 3 of several. Stacked on #2690 (populates the streaming span attributes) and #2689 (adds them) -- the diff below includes both until they merge first; the last commit (
Demonstrate streaming span attributes ...) is the new content here.Extends the existing
examples/quickstart/streaming_apps.ipynb(rather than adding a new notebook, since it already demonstrates exactly the right pattern -- an@instrument-decorated method consuming an OpenAIstream=Truecompletion) with a section that queries recorded events for the newGENERATIONspan attributes (is_streaming, TTFT,tokens_per_second,chunks_received) and prints them.Also fixes a comment that PR #2690 made stale:
stream_options={"include_usage": True}, # not yet tracked by trulens-- that's no longer true, so it now explains what the option is for instead.Test plan