diff --git a/openhands-sdk/openhands/sdk/conversation/base.py b/openhands-sdk/openhands/sdk/conversation/base.py index 1a400356f6..154814df93 100644 --- a/openhands-sdk/openhands/sdk/conversation/base.py +++ b/openhands-sdk/openhands/sdk/conversation/base.py @@ -29,6 +29,14 @@ from openhands.sdk.workspace.base import BaseWorkspace +def _conversation_tag_attributes( + tags: Mapping[str, str] | None, +) -> dict[str, str] | None: + if not tags: + return None + return {f"conversation.tags.{key}": value for key, value in tags.items()} + + if TYPE_CHECKING: from openhands.sdk.agent.base import AgentBase from openhands.sdk.conversation.state import ConversationExecutionStatus @@ -134,6 +142,7 @@ def _start_observability_span( user_id: str | None = None, metadata: dict[str, TraceMetadataValue] | None = None, tags: list[str] | None = None, + conversation_tags: Mapping[str, str] | None = None, ) -> None: """Start a per-conversation observability root span. @@ -142,6 +151,7 @@ def _start_observability_span( user_id: Optional user ID to associate with the trace metadata: Optional trace-level metadata to attach to observability backends tags: Optional span tags to attach to the conversation root span + conversation_tags: Optional conversation tags to add as root span attributes """ if not should_enable_observability(): return @@ -154,6 +164,7 @@ def _start_observability_span( user_id=user_id, metadata=metadata, tags=tags, + attributes=_conversation_tag_attributes(conversation_tags), ) def _end_observability_span(self) -> None: diff --git a/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py b/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py index 9c957aa026..580312a90d 100644 --- a/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py +++ b/openhands-sdk/openhands/sdk/conversation/impl/local_conversation.py @@ -386,6 +386,7 @@ def _default_callback(e): user_id=user_id, metadata=observability_metadata, tags=observability_tags, + conversation_tags=tags, ) self.delete_on_close = delete_on_close diff --git a/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py b/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py index 9eb997936c..2c010d7dcb 100644 --- a/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py +++ b/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py @@ -805,6 +805,8 @@ def __init__( else [], "user_id": user_id, } + if user_id: + payload["user_id"] = user_id if stuck_detection_thresholds is not None: # Convert to StuckDetectionThresholds if dict, then serialize if isinstance(stuck_detection_thresholds, Mapping): @@ -974,6 +976,7 @@ def run_complete_callback(event: Event) -> None: user_id=user_id, metadata=observability_metadata, tags=observability_tags, + conversation_tags=tags, ) # All hooks (including SessionStart/SessionEnd) are executed server-side. # hook_config is sent in the creation payload. diff --git a/openhands-sdk/openhands/sdk/observability/laminar.py b/openhands-sdk/openhands/sdk/observability/laminar.py index 328e3f26dc..7f9ed4bfaa 100644 --- a/openhands-sdk/openhands/sdk/observability/laminar.py +++ b/openhands-sdk/openhands/sdk/observability/laminar.py @@ -4,7 +4,7 @@ import functools import inspect import sys -from collections.abc import Callable, Iterator +from collections.abc import Callable, Iterator, Mapping from typing import TYPE_CHECKING, Any, Final, Literal, cast from openhands.sdk.logger import get_logger @@ -255,6 +255,7 @@ def __init__( name: str, session_id: str | None = None, user_id: str | None = None, + attributes: Mapping[str, str] | None = None, metadata: dict[str, TraceMetadataValue] | None = None, tags: list[str] | None = None, ) -> None: @@ -263,6 +264,10 @@ def __init__( # ``start_span`` returns a span without attaching it as the current # OTel context; we'll restore it on every entry point via ``use_span``. self.span = Laminar.start_span(name) + if attributes: + with contextlib.suppress(Exception): + for key, value in attributes.items(): + self.span.set_attribute(key, value) if session_id or user_id or metadata or tags: # These trace/span helpers require an active span; briefly enter # the span context to apply conversation-level observability data. @@ -300,6 +305,7 @@ def start_root_span( name: str, session_id: str | None = None, user_id: str | None = None, + attributes: Mapping[str, str] | None = None, metadata: dict[str, TraceMetadataValue] | None = None, tags: list[str] | None = None, ) -> RootSpan | None: @@ -314,6 +320,7 @@ def start_root_span( name, session_id=session_id, user_id=user_id, + attributes=attributes, metadata=metadata, tags=tags, ) diff --git a/tests/sdk/conversation/test_base_span_management.py b/tests/sdk/conversation/test_base_span_management.py index 7cf66fd08f..229fa9c844 100644 --- a/tests/sdk/conversation/test_base_span_management.py +++ b/tests/sdk/conversation/test_base_span_management.py @@ -100,6 +100,7 @@ def test_base_conversation_span_management(): user_id=None, metadata=None, tags=None, + attributes=None, ) assert conversation._span_ended is False assert conversation._observability_root_span is fake_root @@ -121,7 +122,8 @@ def test_base_conversation_span_management(): assert conversation._span_ended is True -def test_base_conversation_passes_observability_metadata(): +def test_base_conversation_passes_observability_metadata_and_tag_attributes(): + """Conversation metadata, span tags, and conversation tags reach the root span.""" conversation = MockConversation() with ( @@ -134,18 +136,27 @@ def test_base_conversation_passes_observability_metadata(): metadata: dict[str, TraceMetadataValue] = { "repo_name": "OpenHands/software-agent-sdk" } - tags = ["repo:OpenHands/software-agent-sdk"] + span_tags = ["repo:OpenHands/software-agent-sdk"] + conversation_tags = {"automationid": "auto-1", "automationrunid": "run-1"} conversation._start_observability_span( - "test-session-id", metadata=metadata, tags=tags + "test-session-id", + user_id="user-42", + metadata=metadata, + tags=span_tags, + conversation_tags=conversation_tags, ) mock_start_span.assert_called_once_with( "conversation", session_id="test-session-id", - user_id=None, + user_id="user-42", metadata=metadata, - tags=tags, + tags=span_tags, + attributes={ + "conversation.tags.automationid": "auto-1", + "conversation.tags.automationrunid": "run-1", + }, ) diff --git a/tests/sdk/conversation/test_conversation_factory.py b/tests/sdk/conversation/test_conversation_factory.py index 244c0d112e..9473394059 100644 --- a/tests/sdk/conversation/test_conversation_factory.py +++ b/tests/sdk/conversation/test_conversation_factory.py @@ -102,6 +102,26 @@ def test_conversation_factory_forwards_remote_parameters( assert conversation.max_iteration_per_run == 200 +@patch("openhands.sdk.conversation.impl.remote_conversation.WebSocketCallbackClient") +def test_conversation_factory_forwards_remote_user_id_to_create_payload( + mock_ws_client, agent, remote_workspace +): + """RemoteConversation must send user_id to the agent-server create request.""" + conversation = Conversation( + agent=agent, + workspace=remote_workspace, + tags={"automationid": "auto-1"}, + user_id="user-42", + ) + + assert isinstance(conversation, RemoteConversation) + create_call = remote_workspace._client.request.call_args_list[0] + assert create_call.args[:2] == ("POST", "/api/conversations") + payload = create_call.kwargs["json"] + assert payload["user_id"] == "user-42" + assert payload["tags"] == {"automationid": "auto-1"} + + def test_conversation_factory_string_workspace_creates_local(agent): """Test that string workspace creates LocalConversation.""" conversation = Conversation(agent=agent, workspace="") diff --git a/tests/sdk/observability/test_laminar.py b/tests/sdk/observability/test_laminar.py index 9b3d5385b2..a452411c8e 100644 --- a/tests/sdk/observability/test_laminar.py +++ b/tests/sdk/observability/test_laminar.py @@ -438,6 +438,30 @@ def fake_use_span(span, *args, **kwargs): os.environ.pop("LMNR_PROJECT_API_KEY", None) +def test_root_span_sets_attributes(): + """RootSpan must attach provided attributes to the underlying span.""" + os.environ["LMNR_PROJECT_API_KEY"] = "test-key" + try: + from lmnr import Laminar + + from openhands.sdk.observability import laminar as lam + + mock_span = MagicMock(name="span") + + with patch.object(Laminar, "start_span", return_value=mock_span): + lam._observability_enabled = True + root = lam.RootSpan( + "conversation", + attributes={"conversation.tags.automationid": "auto-1"}, + ) + assert root.span is mock_span + mock_span.set_attribute.assert_called_once_with( + "conversation.tags.automationid", "auto-1" + ) + finally: + os.environ.pop("LMNR_PROJECT_API_KEY", None) + + def test_two_concurrent_conversations_do_not_collide(): """Each conversation must own its own root span (no global stack).