feat(pydantic_ai): propagate conversation_id as LLMObs session_id - #19238
feat(pydantic_ai): propagate conversation_id as LLMObs session_id#19238mlorthiois wants to merge 1 commit into
Conversation
Agent.run/run_stream/iter's conversation_id kwarg is now forwarded as the LLMObs session_id, so multi-turn conversations don't need a separate LLMObs.workflow(session_id=...) wrapper. `None` (default) and the `'new'` sentinel are skipped: `None` is the common no-op case, and `'new'` tells pydantic-ai to fork a fresh conversation into an id we don't have access to at span-creation time — propagating the literal string would incorrectly group unrelated conversations under one shared session. Fixes DataDog#19237
0983ba6 to
a853c56
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a853c56592
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "ddtrace_global_config", | ||
| [dict(_llmobs_enabled=True, _llmobs_ml_app="<ml-app-name>")], | ||
| ) | ||
| class TestLLMObsPydanticAISessionId: |
There was a problem hiding this comment.
The pydantic_ai riot env still runs this test module against pydantic-ai-slim==0.8.1, ==1.0.0, and ==1.63.0 (see riotfile.py), but conversation_id is not accepted by Agent.run/run_stream/iter in those existing pinned versions. As written, this whole new class raises TypeError before the integration is exercised in those jobs; please version-gate these tests or update the supported/tested version matrix.
Useful? React with 👍 / 👎.
| """ | ||
| if not self.llmobs_enabled or not conversation_id or conversation_id == "new": | ||
| return | ||
| _annotate_llmobs_span_data(span, session_id=conversation_id) |
There was a problem hiding this comment.
When this is the first session id in a trace and the agent makes an outbound HTTP call, this only writes the value into the LLMObs metastruct/span store; unlike the core LLMObs span path, it never mirrors it into span.context._meta[PROPAGATED_SESSION_ID_KEY], which is what the HTTP propagator injects. Downstream services therefore won't inherit the conversation_id session even though the local agent/tool spans do, so please also stamp the propagated session key when accepting the explicit conversation_id.
Useful? React with 👍 / 👎.
|
I tried adding a
So properly supporting v2 in the matrix means reworking the model strings across the whole module (and likely re-recording cassettes), which feels like a separate compatibility effort rather than part of this session-id change. For this PR I can instead make Does officially supporting pydantic-ai v2 in the test matrix sound like it should be its own follow-up, out of scope here? |
1 participant
Summary
conversation_idkwarg passed toAgent.run/run_stream/iteras the LLMObssession_id, mirroring the pattern already used by thegoogle_adkintegration (PydanticAIIntegration.set_session_id, called right after span creation).conversation_id=None(the default, i.e. not passed explicitly) and the'new'sentinel are intentionally not propagated:Noneis just the common no-op case (most calls don't pass it).'new'tells pydantic-ai to fork a fresh conversation into an id generated internally (a fresh UUID7) that isn't available to the integration at span-creation time. Propagating the literal string'new'would incorrectly group every unrelated fresh conversation under one shared session.AgentRun.conversation_id, which also accounts for inheritance frommessage_history) — resolving it would require entering the run before the session id could be set, which is after child tool spans already need it.Test plan
TestLLMObsPydanticAISessionIdtotests/contrib/pydantic_ai/test_pydantic_ai_llmobs.py, covering: explicitconversation_idviarun/run_stream/iter, the'new'sentinel being skipped, and an omitted-but-inherited-from-message_historycase also being skipped (documenting the raw-kwarg-only design choice).Closes #19237