Call Laminar.set_trace_user_id in RootSpan alongside set_trace_session_id#3242
Draft
juanmichelini wants to merge 2 commits into
Draft
Call Laminar.set_trace_user_id in RootSpan alongside set_trace_session_id#3242juanmichelini wants to merge 2 commits into
juanmichelini wants to merge 2 commits into
Conversation
…n_id Thread user_id through: - RootSpan.__init__ → calls Laminar.set_trace_user_id(user_id) - start_root_span() - BaseConversation._start_observability_span() - Conversation factory, LocalConversation, RemoteConversation Fixes #3241 Co-authored-by: openhands <openhands@all-hands.dev>
Contributor
Python API breakage checks — ✅ PASSEDResult: ✅ PASSED |
Contributor
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
all-hands-bot
approved these changes
May 13, 2026
Collaborator
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
🟢 Good taste - Clean implementation that follows existing patterns.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟢 LOW
This is a straightforward observability enhancement that adds user_id support to Laminar tracing. The implementation correctly mirrors the existing session_id pattern, maintains full backward compatibility with optional parameters, and includes appropriate test coverage. No agent behavior changes, so no eval risk.
VERDICT:
✅ Worth merging: Solves a real Laminar integration need with minimal, well-tested code.
KEY INSIGHT:
Parameter threading done right—consistent with existing session_id flow and fully backward-compatible.
Collaborator
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
✅ QA Report: PASS
Verified that Laminar.set_trace_user_id() is now properly called when user_id is provided, enabling Laminar's user-level trace features as intended.
Does this PR achieve its stated goal?
Yes. The PR successfully fixes the bug where user_id was only saved in metadata but never registered via Laminar's set_trace_user_id() API. The fix properly threads the user_id parameter through the entire conversation stack and calls the Laminar API when a user ID is provided.
Evidence from functional verification:
- Created a conversation with
user_id="test-user-123"and confirmedLaminar.set_trace_user_id()is called with the correct value - Created a conversation without
user_idand confirmed the API is NOT called (backward compatibility preserved) - Verified the parameter flows correctly through:
RootSpan.__init__→start_root_span()→BaseConversation._start_observability_span()→LocalConversation.__init__/RemoteConversation.__init__→Conversationfactory
| Phase | Result |
|---|---|
| Environment Setup | ✅ Dependencies installed with uv sync --dev |
| CI Status | |
| Functional Verification | ✅ user_id parameter properly threaded; Laminar API called correctly |
Functional Verification
Test Setup
Created a functional test script that mocks the Laminar library and verifies the integration without requiring a real Laminar backend.
Baseline Behavior (Before Fix)
Code from origin/main:
def __init__(self, name: str, session_id: str | None = None) -> None:
from lmnr import Laminar
self.span = Laminar.start_span(name)
if session_id:
with contextlib.suppress(Exception):
with Laminar.use_span(self.span):
Laminar.set_trace_session_id(session_id)
self._ended = FalseProblem: No user_id parameter exists, and Laminar.set_trace_user_id() is never called, so Laminar's user-level trace features don't work.
Fixed Behavior (With PR Changes)
New implementation:
def __init__(
self,
name: str,
session_id: str | None = None,
user_id: str | None = None, # NEW PARAMETER
) -> None:
from lmnr import Laminar
self.span = Laminar.start_span(name)
if session_id or user_id: # CHANGED CONDITION
with contextlib.suppress(Exception):
with Laminar.use_span(self.span):
if session_id:
Laminar.set_trace_session_id(session_id)
if user_id:
Laminar.set_trace_user_id(user_id) # NEW API CALL
self._ended = FalseVerification Results
Test 1: RootSpan with user_id
Ran: RootSpan("test-operation", session_id="session-123", user_id="user-456")
Result:
✓ Laminar.start_span called with: call('test-operation')
✓ Laminar.set_trace_session_id called with: call('session-123')
✓ Laminar.set_trace_user_id called with: call('user-456')
✓ Correct user_id value: user-456
Test 2: RootSpan without user_id (backward compatibility)
Ran: RootSpan("test-operation", session_id="session-789")
Result:
✓ Laminar.set_trace_session_id called
✓ Laminar.set_trace_user_id NOT called (correct for backward compatibility)
Test 3: Parameter threading verification
Confirmed user_id parameter exists in all required signatures:
- ✅
RootSpan.__init__(self, name, session_id, user_id) - ✅
start_root_span(name, session_id, user_id) - ✅
BaseConversation._start_observability_span(self, session_id, user_id) - ✅
LocalConversation.__init__(..., user_id, ...) - ✅
RemoteConversation.__init__(..., user_id, ...) - ✅
Conversation.__new__(..., user_id, ...)
Test 4: End-to-end integration
from openhands.sdk import Agent, LLM, Conversation
llm = LLM(model="anthropic/claude-sonnet-4-5-20250929", api_key="test-key")
agent = Agent(llm=llm, tools=[])
# With user_id
conv = Conversation(agent=agent, workspace="/tmp/test", user_id="user-123")
# Result: Laminar.set_trace_user_id("user-123") was called ✓
# Without user_id (backward compatibility)
conv = Conversation(agent=agent, workspace="/tmp/test")
# Result: Laminar.set_trace_user_id() was NOT called ✓Full test output:
======================================================================
FUNCTIONAL TEST: user_id Parameter Integration
======================================================================
Test 1: Verify RootSpan.__init__ signature
----------------------------------------------------------------------
RootSpan.__init__ parameters: ['self', 'name', 'session_id', 'user_id']
✓ user_id parameter exists in RootSpan.__init__
Test 2: Verify start_root_span signature
----------------------------------------------------------------------
start_root_span parameters: ['name', 'session_id', 'user_id']
✓ user_id parameter exists in start_root_span
Test 3: RootSpan calls Laminar.set_trace_user_id
----------------------------------------------------------------------
✓ Laminar.start_span called with: call('test-operation')
✓ Laminar.set_trace_session_id called with: call('session-123')
✓ Correct session_id value: session-123
✓ Laminar.set_trace_user_id called with: call('user-456')
✓ Correct user_id value: user-456
Test 4: RootSpan without user_id (backward compatibility)
----------------------------------------------------------------------
✓ Laminar.set_trace_session_id called
✓ Laminar.set_trace_user_id NOT called (correct for backward compatibility)
Test 5: Verify Conversation factory signature
----------------------------------------------------------------------
Conversation.__new__ parameters: [..., 'user_id']
✓ user_id parameter exists in Conversation factory
Test 6: Verify LocalConversation signature
----------------------------------------------------------------------
LocalConversation.__init__ parameters: [..., 'user_id', ...]
✓ user_id parameter exists in LocalConversation.__init__
Test 7: Verify RemoteConversation signature
----------------------------------------------------------------------
RemoteConversation.__init__ parameters: [..., 'user_id', ...]
✓ user_id parameter exists in RemoteConversation.__init__
Test 8: Verify BaseConversation._start_observability_span signature
----------------------------------------------------------------------
BaseConversation._start_observability_span parameters: ['self', 'session_id', 'user_id']
✓ user_id parameter exists in _start_observability_span
======================================================================
ALL TESTS PASSED ✓
======================================================================
SUMMARY:
--------
✓ user_id parameter is properly threaded through the entire stack
✓ Laminar.set_trace_user_id() IS called when user_id is provided
✓ Laminar.set_trace_user_id() is NOT called when user_id is None
✓ Backward compatibility maintained (existing code without user_id works)
Issues Found
None.
Collaborator
Author
- Add user_id field to StartConversationRequest so callers (e.g. the enterprise server) can supply the authenticated user's ID. - Pass user_id from StoredConversation to LocalConversation in EventService so the observability span actually receives it. Co-authored-by: openhands <openhands@all-hands.dev>
Contributor
Coverage Report •
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
neubig
approved these changes
May 15, 2026
Contributor
neubig
left a comment
neubig
left a comment
There was a problem hiding this comment.
The code seems reasonable to me if we can QA it and make sure that it works!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Laminar traces need both
set_trace_session_idandset_trace_user_idset on the span context. Currently, user_id is only saved in metadata but not registered via the dedicatedLaminar.set_trace_user_id()API, which means Laminar's user-level trace features don't work properly.See: https://laminar.sh/docs/tracing/structure/user-id
Summary
user_idparameter toRootSpan.__init__and callLaminar.set_trace_user_id(user_id)right afterset_trace_session_iduser_idthroughstart_root_span(),BaseConversation._start_observability_span(), and theConversationfactoryuser_idinLocalConversationandRemoteConversationconstructorsIssue Number
Fixes #3241
How to Test
The new unit tests verify that
Laminar.set_trace_user_idis called whenuser_idis provided and skipped whenNone:uv run pytest tests/sdk/observability/test_laminar.py -v # All 32 tests passType
Notes
All new parameters are optional (
str | None = None) so this is fully backward-compatible. The deprecatedstart_active_span/SpanManagershims are unchanged since they don't support user_id (they're slated for removal in 1.27.0).@juanmichelini can click here to continue refining the PR
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.13-nodejs22-slimgolang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:23e84b3-pythonRun
All tags pushed for this build
About Multi-Architecture Support
23e84b3-python) is a multi-arch manifest supporting both amd64 and arm6423e84b3-python-amd64) are also available if needed