Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions openhands-sdk/openhands/sdk/conversation/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.

Expand All @@ -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
Expand All @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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.
Expand Down
9 changes: 8 additions & 1 deletion openhands-sdk/openhands/sdk/observability/laminar.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to move the contextlib.suppress inside the loop.

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.
Expand Down Expand Up @@ -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:
Expand All @@ -314,6 +320,7 @@ def start_root_span(
name,
session_id=session_id,
user_id=user_id,
attributes=attributes,
metadata=metadata,
tags=tags,
)
Expand Down
21 changes: 16 additions & 5 deletions tests/sdk/conversation/test_base_span_management.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 (
Expand All @@ -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",
},
)


Expand Down
20 changes: 20 additions & 0 deletions tests/sdk/conversation/test_conversation_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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="")
Expand Down
24 changes: 24 additions & 0 deletions tests/sdk/observability/test_laminar.py
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Expand Down
Loading