fix(ingest): rank gen_ai.conversation.id before session.id for OpenInference-style vendors - #1177
Conversation
…ference-style vendors
Maple review🟢 Confidence 4/5 · likely safe to merge Ranks
What was checked
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughSession-key lookup precedence changes across several OpenInference and native dialects. The updated tests check when both identifiers exist, when only ChangesSession key precedence
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to This change only alters which session key is preferred for newly ingested spans. Tests cover the new ordering and the fallback, and no merge-blocking risk was found. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Conversation identifiers now take precedence for affected vendors without replacing tenant access controls. The main compatibility effect is that historical data and mixed-version ingestion can retain different session groupings. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 Clippy (1.98.1)Clippy execution failed Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Why
Several vendors read bare
session.idbeforegen_ai.conversation.idwhen grouping spans into agent sessions. The two keys usually carry the same value, because OpenInference's dual-write copies it. When they differ, the GenAI-standard conversation id should win.session.idis a generic key: a customer's own middleware can stamp a web-login session on every span, and Maple's browser SDK stamps its replay session there. Either one can span several conversations and merge them into one Maple session.Change
apps/ingest/src/ai_session.rs: a newCONVERSATION_THEN_SESSION_IDkey list (gen_ai.conversation.id, thensession.id), used by the vendors below. A span with onlysession.idstill groups by it. Because these lists now include the conversation id,run_predicatesno longer appends it as a trailing fallback for them (the fallback added in #1139).dspy,agno,openinference-openai,crewai,smolagents,strandssession.id, then the fallbacksession.idhaystack(OpenInference scope),llamaindex,openai_agents_sdksession.id, conversation idsession.idlangchainsession.id, conversation idsession.idunknown:openinferenceUnchanged:
claude_agent_sdk: itssession.idis Claude Code's own session id, which is the correct session key.openrouter: itssession.idechoes the caller's OpenRoutersession_idrequest field. That is OpenRouter's own session key, and it is how a call joins the framework session that tagged it.google_adk,pydantic_ai: these already read the conversation id beforesession.id.session.id(maple,eve,spring_ai,vercel_ai_sdk, the conversation-id-only dialects).Tests
openinference_session_id_ranks_below_the_conversation_idchecks every affected vendor two ways: with both keys set, the conversation id wins; with onlysession.id, the span still groups by it. It also checks thatclaude_agent_sdkandopenrouterkeep theirsession.idwhen both keys are set. It replacesvendors_with_their_own_key_fall_back_to_the_conversation_id, which asserted the old order. The OpenInference Haystack case insession_key_order_follows_the_emitting_dialectis dropped because the new test covers it.cargo test --libinapps/ingest: 204 passed.Note
This is an ingest stamp, so it applies only to newly ingested spans. Rows already stamped keep their session id.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit