Skip to content

fix: python openai agent sdk integration fix#83

Closed
Nightingalelyy wants to merge 6 commits intorespanai:mainfrom
Nightingalelyy:openai-agent-sdk-fix
Closed

fix: python openai agent sdk integration fix#83
Nightingalelyy wants to merge 6 commits intorespanai:mainfrom
Nightingalelyy:openai-agent-sdk-fix

Conversation

@Nightingalelyy
Copy link
Contributor

fix the span_type, serializing, usage count of openai agent sdk integration, create tests for openai agent sdk integration

fix the span_type, serializing, usage count of openai agent sdk integration, create tests for openai agent sdk integration
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 14, 2026

Greptile Summary

This PR fixes several issues in the OpenAI Agents SDK integration: corrects span_type assignments for all span data types, replaces a thread-unsafe warnings.catch_warnings() per-call approach with a single module-level filter, improves token count extraction to correctly handle both Responses API (input_tokens/output_tokens) and Chat Completions API (prompt_tokens/completion_tokens) key formats, and adds a new LocalSpanCollector class for in-process span collection in self-hosted deployments. A comprehensive test suite is added covering all 7 span types and LocalSpanCollector lifecycle.

Key changes:

  • Added module-level constants for item types, roles, field names, and usage keys — eliminating scattered string literals
  • Extracted per-span-type converter helpers (_response_data_to_respan_log, _function_data_to_respan_log, etc.) and a shared convert_to_respan_log entry point reused by both RespanSpanExporter and LocalSpanCollector
  • Added safe_attr / safe_serialize to respan_sdk.utils public API (__init__.py and serialization.py)
  • New LocalSpanCollector stores converted spans in memory keyed by trace_id with root-first ordering (insert(0, ...) for the trace record)
  • _custom_data_to_respan_log passthrough for prompt_tokens/completion_tokens skips the int() coercion that all other converter functions apply — minor inconsistency flagged
  • export() method uses bare list[Union[...]] annotation (Python 3.9+ only) while the rest of the file uses List from typing

Confidence Score: 4/5

  • This PR is safe to merge; it fixes real integration bugs and introduces well-tested new functionality with only minor style inconsistencies remaining.
  • The core logic — span type mapping, token count extraction with identity checks for zero values, Responses API → Chat Completions format conversion, and thread-safe LocalSpanCollector — is correct and thoroughly tested. The two flagged issues are style-level: missing int() coercion in the custom span passthrough (relies on Pydantic to coerce, which it likely does) and a list[Union[...]] annotation that is inconsistent with the rest of the file. No critical logic or security issues found.
  • respan_openai_agents_exporter.py — minor token coercion inconsistency in _custom_data_to_respan_log and the list[Union[...]] annotation on export().

Important Files Changed

Filename Overview
python-sdks/respan-exporter-openai-agents/src/respan_exporter_openai_agents/respan_openai_agents_exporter.py Major refactor: adds module-level constants, per-span-type converter helpers, a shared convert_to_respan_log function, and a new LocalSpanCollector class. Two minor style issues flagged: list[Union[...]] annotation should be List[Union[...]] for Python <3.9 compatibility, and _custom_data_to_respan_log omits int() coercion for token count passthrough unlike all other converter functions.
python-sdks/respan-exporter-openai-agents/tests/test_converters.py Comprehensive new test suite covering all 7 span types, LocalSpanCollector lifecycle, edge cases (zero tokens, errors, fallback IDs), and helper functions. LocalSpanCollector tests are now present, addressing a concern from a previous review round.
python-sdks/respan-sdk/src/respan_sdk/utils/serialization.py Adds safe_attr helper. Implementation is correct for all non-None values, but cannot distinguish an explicit None stored value from a missing key — this is a documented design choice confirmed by tests, but callers should be aware of the limitation.
python-sdks/respan-sdk/src/respan_sdk/utils/init.py Re-exports safe_attr and safe_serialize in __all__, making them part of the public API of respan_sdk.utils. No issues.
python-sdks/respan-exporter-openai-agents/tests/init.py Empty __init__.py to make the tests/ directory a Python package for pytest discovery. No issues.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Trace / Span[Any]"] --> B{isinstance Trace?}
    B -- yes --> C["Return root log\n(trace_id, name, LOG_TYPE_AGENT)"]
    B -- no --> D{isinstance SpanImpl?}
    D -- no --> E["Return None"]
    D -- yes --> F["Build base RespanTextLogParams\n(ids, timestamps, error fields)"]
    F --> G{span_data type?}
    G -- ResponseSpanData --> H["_response_data_to_respan_log\n(Responses API → Chat Completions format,\nusage: input_tokens / output_tokens)"]
    G -- FunctionSpanData --> I["_function_data_to_respan_log\n(LOG_TYPE_TOOL, span_tools)"]
    G -- GenerationSpanData --> J["_generation_data_to_respan_log\n(prompt_tokens / input_tokens fallback)"]
    G -- HandoffSpanData --> K["_handoff_data_to_respan_log\n(from_agent → to_agent)"]
    G -- AgentSpanData --> L["_agent_data_to_respan_log\n(LOG_TYPE_AGENT, span_workflow_name)"]
    G -- GuardrailSpanData --> M["_guardrail_data_to_respan_log\n(has_warnings, warnings_dict)"]
    G -- CustomSpanData --> N["_custom_data_to_respan_log\n(metadata + passthrough keys)"]
    G -- unknown --> O["Log warning, return None"]
    H & I & J & K & L & M & N --> P["model_dump(mode='json')"]
    P --> Q{Caller}
    Q -- RespanSpanExporter --> R["HTTP POST to Respan backend\n(with retry + backoff)"]
    Q -- LocalSpanCollector --> S["_traces[trace_id].append(data)\n(Trace record inserted at index 0)"]
Loading

Last reviewed commit: 6dd1b0b

Nightingalelyy and others added 3 commits March 14, 2026 16:49
…s.py

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
…openai_agents/respan_openai_agents_exporter.py

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@Nightingalelyy
Copy link
Contributor Author

@greptileai I fixed the problem and add test, please review

@Nightingalelyy
Copy link
Contributor Author

@greptileai problem fixed. review

Comment on lines +55 to +58
val = getattr(obj, key, None)
if val is None and isinstance(obj, dict):
val = obj.get(key, default)
return val if val is not None else default No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

safe_attr cannot distinguish an explicit None value from a missing key

Because both getattr(obj, key, None) and obj.get(key, default) map to None for a stored None value, the final guard return val if val is not None else default always substitutes default when the stored value is None. For callers that need to detect an explicitly-set None (as opposed to an absent key), there is no way to do so — they always receive default back instead.

This is a documented, tested design choice (see test_none_value_returns_default), but it is worth keeping in mind when using safe_attr in contexts where None is a meaningful value (e.g. response.instructions = None intentionally overrides a default). If that distinction ever matters, callers should fall back to direct attribute/key access rather than safe_attr.

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