-
Notifications
You must be signed in to change notification settings - Fork 1k
fix(openai): finalize completion spans on early stream close #4394
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
CTWalk
wants to merge
1
commit into
traceloop:main
Choose a base branch
from
CTWalk:fix/completion-stream-early-close
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+101
−24
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: traceloop/openllmetry
Length of output: 50377
🏁 Script executed:
Repository: traceloop/openllmetry
Length of output: 4653
🏁 Script executed:
Repository: traceloop/openllmetry
Length of output: 5290
🌐 Web query:
openai python 1.99.7 Stream close AsyncStream aclose iterator close source💡 Result:
In the OpenAI Python library, specifically regarding the AsyncStream class and its closing behavior, the following technical details apply: 1. Handling of aclose: In version 1.99.7 and earlier, AsyncStream did not expose an aclose method, which often led to an AttributeError when instrumentation libraries or standard async patterns attempted to use await stream.aclose [1][2]. This was subsequently addressed in later versions of the library by adding aclose as an alias for the existing close method to ensure compatibility with PEP 525 conventions and standard async cleanup patterns [1][2]. 2. Stream Closing Mechanism: The internal logic for closing an asynchronous stream is designed to ensure the underlying network connection is managed correctly [3][4]. The AsyncStream.stream generator includes a finally block that calls await response.aclose to guarantee the response is closed even if the consumer does not iterate to completion [3]. 3. Resource Management and Connection Draining: A known issue related to streaming involves connection management after the [DONE] event is received [5][4]. When an SSE stream reaches the [DONE] sentinel, the generator typically breaks out of its iteration loop [3][4]. If the remaining bytes—specifically the HTTP/1.1 chunked terminator—are not drained from the iterator before the connection is closed, it can result in a force-close of the connection rather than a graceful return to the connection pool [5][4]. Subsequent patches introduced logic to "drain" these remaining bytes in the finally block (using async for _ in iterator: pass) to prevent these connection-related issues [5]. Users experiencing issues with streaming lifecycle or connection errors are encouraged to ensure they are using a more recent version of the library where these compatibility and connection-draining fixes have been integrated [5].
Citations:
[DONE]SSE event because chunked terminator is not drained — regression from6132922c** openai/openai-python#3440🌐 Web query:
openai-python issue 763 unclosed streams leak connections💡 Result:
Issue
#763in the openai-python GitHub repository addressed a bug where network connections were not being returned to the HTTP connection pool when using streaming requests [1]. This resulted in connection exhaustion (often manifesting asAPITimeoutErrororhttpx.PoolTimeout) in high-concurrency environments because the underlying connections remained occupied [1][2]. The issue was resolved in version 1.2.3 of the OpenAI Python SDK [1]. For users encountering similar behaviors or connection management issues, the recommended practices include: 1. Manual Response Closure: If you are streaming responses and not consuming the entire stream, you can explicitly close the response object usingawait stream.response.aclose()(for async) orstream.response.close()(for sync) to ensure the connection is returned to the pool [1][3][4]. 2. Use Context Managers: When working with the client, using it within a context manager (async with AsyncOpenAI() as client:orwith OpenAI() as client:) ensures that the client and its underlying connection pool are cleaned up appropriately when the block exits [5][6]. 3. Client Reuse: The SDK is designed for the client instance to be reused across requests rather than instantiated repeatedly. Creating new clients per request can lead to resource leaks and socket exhaustion [5][6][7]. 4. Adjust Pool Limits: In high-scale production environments, you may need to explicitly configure the connection pool limits to be larger than the defaults [8][7]. This can be done by passing a customhttp_clientto the OpenAI client constructor: from openai import AsyncOpenAI import httpx client = AsyncOpenAI( http_client=httpx.AsyncClient( limits=httpx.Limits(max_connections=1000, max_keepalive_connections=100))) [7] These approaches address the connection leaks and pool timeouts reported in issue#763and related discussions [1][2][7].Citations:
🌐 Web query:
openai python 1.99.7 AsyncStream aclose attribute source code💡 Result:
In the OpenAI Python library, specifically as of version 1.99.7, the AsyncStream class contains an aclose method [1][2][3]. Historically, AsyncStream only provided a close method, which led to an AttributeError when external instrumentation tools (such as Langfuse) or standard Python async cleanup patterns expected an aclose method—a convention established by PEP 525 for async generators and used by libraries like httpx and asyncio [1][2][4]. This issue was addressed by adding aclose as an alias for the existing close method, ensuring compatibility with standard asynchronous resource cleanup workflows [1][2]. In the source code (src/openai/_streaming.py), the implementation essentially delegates the cleanup to the underlying HTTP response object [5][6][7]: async def aclose(self) -> None: await self.close async def close(self) -> None: """ Close the response and release the connection. Automatically called if the response body is read to completion. """ await self.response.aclose The primary purpose of this change was to satisfy the PEP 525 async cleanup contract, preventing crashes in environments where wrappers or proxy objects might attempt to call aclose on the stream [1][2][4].
Citations:
🌐 Web query:
langfuse github close async streaming openai Python AttributeError 1.99.7 aclose💡 Result:
The AttributeError: 'AsyncStream' object has no attribute 'aclose' that occurred in older versions of the Langfuse Python SDK (such as around version 1.99.7) was caused by an incompatibility between Langfuse’s streaming adapter and the OpenAI Python SDK's async stream management [1][2][3]. When using Langfuse to instrument OpenAI's async streaming API—particularly with structured outputs—the Langfuse wrapper was historically passed as the raw stream object to OpenAI's internal stream manager [1][4]. OpenAI’s stream manager would then attempt to call.aclose on the underlying response object [1][5][4]. Because the Langfuse adapter was incorrectly exposing an AsyncStream object (which lacked the aclose method) as the response, the Python interpreter raised an AttributeError [1][3][5]. This issue was addressed through several rounds of improvements: 1. Initial Hotfixes: Earlier versions of Langfuse and the OpenAI SDK worked to resolve the mismatch by ensuring the adapter properly delegated cleanup or by adding aclose as an alias for close on the OpenAI AsyncStream class itself to better comply with PEP 525 async generator conventions [5][6][7]. 2. Robust Architectural Change: In more recent updates (such as those represented in PR
#1627, merged in April 2026), Langfuse moved away from wrapping the response in a custom generator class that replaced the native OpenAI stream objects [8][9]. Instead, the SDK now preserves the native v1 stream contract by monkey-patching the existing OpenAI Stream/AsyncStream objects to collect data while keeping the original methods (like close and aclose) intact [8][9]. If you are encountering this error, it is recommended to update the langfuse package to the latest version, as this architectural change ensures compatibility with the native OpenAI client contract [8][9].Citations:
Close the OpenAI stream before ending the span. Both
_build_from_streaming_responseand_abuild_from_streaming_responseonly callspan.end()infinally; if the wrapper generator exits before full consumption, OpenAI’s stream iterator is abandoned without returning its HTTP connection to the client pool. Add a raw stream cleanup path before ending the span: callresponse.close()for syncStreamresponses andawait response.aclose()/response.aclose()for asyncAsyncStreamresponses, with an appropriate guard if the wrapper is used with streams that may not expose the async method.📍 Affects 1 file
packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/completion_wrappers.py#L209-L230(this comment)packages/opentelemetry-instrumentation-openai/opentelemetry/instrumentation/openai/shared/completion_wrappers.py#L235-L253🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks — I've left this out of scope rather than folding it in. The change
alters when the span is ended, not when the stream is released; an
early-closed Completions stream was abandoned the same way before it. The chat
path from #3155 has the same gap on its
__del__→_ensure_cleanupabandonment path, so it looks repo-wide rather than specific to this file.
One correction for anyone acting on the suggestion: on
openai1.99.7,AsyncStreamhas noaclose. The async close isawait response.close(), andStream.close()is sync.Happy to open a separate PR for connection cleanup across both paths if that
would be useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CTWalk, agreed. This pull request changes span finalization only. It does not introduce the raw stream cleanup gap.Thank you for the
openai1.99.7 correction. A follow-up must useawait response.close()forAsyncStreamandresponse.close()forStream.Would you like me to open a GitHub issue for repository-wide stream connection cleanup across the Completions and chat wrapper paths?
🐇 💭
✏️ Learnings added
You are interacting with an AI system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not for now — worth measuring whether connections are actually held open before filing anything. If it holds up I'll raise it separately with a reproduction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CTWalk, acknowledged. I will not open an issue now.Please raise a separate issue if measurement confirms that early stream abandonment retains connections. Include a minimal reproduction if available.
🧠 Learnings used
You are interacting with an AI system.