Skip to content

improve tracing for openai agents #205

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

Merged
merged 8 commits into from
Jul 23, 2025
Merged

Conversation

jssmith
Copy link
Contributor

@jssmith jssmith commented Jul 9, 2025

What was changed

Added tracing to all of the OpenAI Agents examples.

Why?

Some examples had incomplete tracing.

Checklist

  1. Closes

  2. How was this tested:
    Manual validation via OpenAI platform UI.

  3. Any docs updates needed?
    N/A

@jssmith jssmith requested a review from tconley1428 July 9, 2025 22:01
@jssmith jssmith requested a review from a team as a code owner July 9, 2025 22:01
@jssmith
Copy link
Contributor Author

jssmith commented Jul 9, 2025

Note that we appear to have a tracing bug where some spans don't get closed. We will address this as a separate issue.

Comment on lines 72 to 73
trace_id = gen_trace_id()
with trace("Orchestrator evaluator", trace_id=trace_id):
Copy link
Member

Choose a reason for hiding this comment

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

Does trace ID have to be explicitly generated here? It is the default correct? (this calls our deterministic UUID generator, and have it on our TODO to stop using the RNG shared with the user so we don't affect their RNG use with our own)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have confirmed that the default behavior is correct (get uuid from workflow) and removed the explicit trace_id generation.

Added non-shared RNG as a TODO.

instructions="You only respond in haikus.",
)
trace_id = gen_trace_id()
with trace("Hello World", trace_id=trace_id):
Copy link
Member

@cretz cretz Jul 10, 2025

Choose a reason for hiding this comment

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

I was under the impression samples like these were intentionally meant to look like https://github.com/openai/openai-agents-python/blob/main/examples/basic/hello_world.py for comparison. OpenAI chooses not to demonstrate tracing in their hello world, is it important that Temporal demonstrate OpenAI tracing in the same sample? Should we upstream a PR to OpenAI samples if we feel demonstrating tracing in hello world is important? I think it may be clearer if we are wanting to demonstrate something beyond just showing OpenAI sample equivalents, we do it in a separate sample.

EDIT: This was removed from hello_world sample but still remains on tool sample. Do we want to upstream a PR to add a trace around https://github.com/openai/openai-agents-python/blob/b09d37d8326c5d21760aaf10e9ff2eb8a01ebdaa/examples/basic/tools.py#L20?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made our sample match.

We can stick with matching the samples as closely as possible and can add additional samples/examples where we want to show more.

@jssmith
Copy link
Contributor Author

jssmith commented Jul 23, 2025

@cretz thanks for your review. I have addressed the issues you pointed out. Please have another look.

@cretz
Copy link
Member

cretz commented Jul 23, 2025

Thanks! I still have the concerns about deviating from OpenAI samples at #205 (comment) (can discuss on that thread). Also would like @dandavison or @tconley1428 to review if possible (I am doing less Python things these days).

@jssmith jssmith merged commit 0b42324 into main Jul 23, 2025
12 checks passed
@jssmith jssmith deleted the openai-agents-update-logging branch July 23, 2025 19:45
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.

3 participants