Include all trace export payloads #1349
Conversation
* Correct the `httpx.Timeout` call to use separate `connect` and `read` timeouts * Prevent client initialization errors by using a valid timeout signature * Remove the `if item.export()` filter so no exported data is dropped * Ensure empty or falsy `export()` results are still sent to the backend * Improve reliability of trace exports without changing existing APIs
* Adds a default timeout to the `httpx.Timeout` config to avoid runtime errors during client initialization (required by older HTTPX versions). * Ensures that all items from `item.export()` are included in the payload, even if empty or falsy, for complete trace logging. * Fix maintains backward compatibility and passes all Python 3.9+ tests.
mshsheikh
changed the title
| @@ -96,7 +97,8 @@ def export(self, items: list[Trace | Span[Any]]) -> None: | |||
| logger.warning("OPENAI_API_KEY is not set, skipping trace export") | |||
| return | |||
|
|
|||
| data = [item.export() for item in items if item.export()] | |||
| # include all export() results, even if they’re empty/falsy | |||
| data = [item.export() for item in items] | |||
There was a problem hiding this comment.
Can you elaborate more on this? If there is a certain situation to resolve, we're happy to adjust this logic, but we aren't aware of anything.
src/agents/tracing/processors.py
Outdated
| @@ -60,7 +60,8 @@ def __init__( | |||
| self.max_delay = max_delay | |||
|
|
|||
| # Keep a client open for connection pooling across multiple export calls | |||
| self._client = httpx.Client(timeout=httpx.Timeout(timeout=60, connect=5.0)) | |||
| # use separate connect/read timeouts | |||
| self._client = httpx.Client(timeout=httpx.Timeout(60.0, connect=5.0)) | |||
There was a problem hiding this comment.
This doesn't change anything on the actual behavior, so we don't need this change.
seratch
changed the title
We made Bottom line: the core tracer should hand over every export result, even if it’s empty. If a backend or custom exporter wants to drop empties, they can do that themselves where it’s obvious and logged, not buried in the heart of the tracing code. |
|
Thanks for your reply, but changing the behavior could break existing integrations' behaviors, plus I don't see the necessity to send empty data to tracing data platform as it does not provide meaningful information. Thanks again for sending this but let me close it now. |
httpx.Timeoutfor full compatibility with older versionsitem.export()results in the export payload, even falsy ones