-
Notifications
You must be signed in to change notification settings - Fork 3.4k
fix(tracing): prevent RecursionError in truncation of truncated previews #2857
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -283,6 +283,8 @@ def _truncate_span_field_value(self, value: Any) -> Any: | |
|
|
||
| return self._truncate_json_value_for_limit(sanitized_value, max_bytes) | ||
|
|
||
| _TRUNCATED_PREVIEW_KEY = "__truncated_preview__" | ||
|
|
||
| def _truncate_json_value_for_limit(self, value: Any, max_bytes: int) -> Any: | ||
| if self._value_json_size_bytes(value) <= max_bytes: | ||
| return value | ||
|
|
@@ -291,6 +293,14 @@ def _truncate_json_value_for_limit(self, value: Any, max_bytes: int) -> Any: | |
| return self._truncate_string_for_json_limit(value, max_bytes) | ||
|
|
||
| if isinstance(value, dict): | ||
| # Avoid re-truncating our own truncated preview dicts, which | ||
| # would cause infinite recursion (the preview dict contains bool | ||
| # values that get wrapped into new preview dicts, and so on). | ||
| # Use a sentinel key to reliably identify SDK-generated previews | ||
| # without matching arbitrary user data that happens to use the | ||
| # same key names. | ||
| if self._TRUNCATED_PREVIEW_KEY in value: | ||
| return value | ||
|
Comment on lines
+302
to
+303
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Fresh evidence: although this guard now also checks Useful? React with 👍 / 👎. |
||
| return self._truncate_mapping_for_json_limit(value, max_bytes) | ||
|
|
||
| if isinstance(value, list): | ||
|
|
@@ -356,6 +366,7 @@ def _truncated_preview(self, value: Any) -> dict[str, Any]: | |
| preview = f"<{type_name} bytes={len(value)} truncated>" | ||
|
|
||
| return { | ||
| self._TRUNCATED_PREVIEW_KEY: True, | ||
| "truncated": True, | ||
| "original_type": type_name, | ||
| "preview": preview, | ||
|
|
||
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.
Fresh evidence: the new guard now returns early for any mapping that merely contains
__truncated_preview__, regardless of its value or shape. If user span data includes that key (for example{"__truncated_preview__": false, ...}) and the field exceeds_OPENAI_TRACING_MAX_FIELD_BYTES,_truncate_json_value_for_limitwill return the oversized dict unchanged, which can still be rejected by tracing export limits and drop trace content. Restrict the bypass to SDK-generated preview objects (for example, requirevalue.get(__truncated_preview__) is Trueplus expected preview fields).Useful? React with 👍 / 👎.