Repository navigation
feat(openai-agents): map Trace.group_id to session.id on the root span - #3901
robertwitt wants to merge 1 commit into
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
chrikrah
left a comment
There was a problem hiding this comment.
@robertwitt the code at c365061 works as measured: trace(group_id=...) now puts session.id on the root span. I am leaving a comment and not an approval, because this is feature work and CONTRIBUTING says features are not being accepted right now and asks for an issue first on a non-trivial change.
Probe with the real SDK (openai-agents==0.11.0): one trace("wf", group_id=...) around one custom_span("child"), in-memory exporter, instrumentor source exported from each commit.
$ python probe_group.py # c365061
A group_id only: child session.id= None
A group_id only: wf session.id= conv-1
B using_session only: child session.id= ctx-1
B using_session only: wf session.id= ctx-1
C both: child session.id= ctx-1
C both: wf session.id= ctx-1
$ python probe_group.py # 400013c, the merge base
A group_id only: child session.id= None
A group_id only: wf session.id= None
B using_session only: child session.id= ctx-1
B using_session only: wf session.id= ctx-1
C both: child session.id= ctx-1
C both: wf session.id= ctx-1
For Phoenix the root span should be enough, since src/phoenix/db/insertion/span.py:62 reads session.id from any span and attaches the whole trace (phoenix 7556cee, read, not run).
$ cd python/instrumentation/openinference-instrumentation-openai-agents && python -m pytest tests -q
299 passed in 2.12s # c365061
295 passed in 1.91s # 400013c, the merge base
# _processor.py from 400013c, tests kept
FAILED tests/test_processor_span_attributes.py::test_group_id_mapped_to_session_id
FAILED tests/test_processor_span_attributes.py::test_empty_group_id_mapped_to_session_id
2 failed, 297 passed in 1.72s
# not run: ruff, mypy, a live Phoenix server
non-blocking: CONTRIBUTING asks for a trace screenshot when emitted spans change, and the body has none yet.
non-blocking: _realtime.py:733 puts the provider session id on every span through the context and lets using_session win. This PR matches that precedence (case C), but only because of the call order in _tracers.py:199, with no test pinning it, and it stamps only the root (case A, child None).
@mikeldking, do you want trace group_id mapped to session.id at all? If yes, on the root span only, or on every span the way _realtime.py does it?
The OpenAI Agents SDK lets callers group related traces into a single conversation via
trace(group_id=...). This is the intended mechanism for multi-turn sessions — for example, a chat interface where each user message produces oneRunner.runcall. Before this changeOpenInferenceTracingProcessorbuilt the root span's attributes fromtrace.nameandtrace.trace_idonly;trace.group_idwas silently dropped. Each turn produced an independent OTel trace with no shared attribute, making it impossible for a backend such as Phoenix to group them into a session.What changed
on_trace_startnow readstrace.group_idand, when present, stampssession.idon the root span:group_idis set inTraceImpl.__init__beforestart()fires the hook, so it is always available at this point. Agetattrguard keeps older SDK versions andNonevalues safe — in both casessession.idis simply omitted. An explicit empty string is preserved as a present value.Tests added (
test_processor_span_attributes.py)test_group_id_mapped_to_session_id— present string →session.idsettest_none_group_id_omits_session_id—None→ attribute absenttest_empty_group_id_mapped_to_session_id—""→ mapped verbatimtest_missing_group_id_attr_no_crash— trace with nogroup_idattribute (older SDK) → no crash, attribute absentNo behavior change when
group_idis unset. Fully backward compatible.