Merge branch 'master' into feat/span-first

Author: sentrivana

sentry_sdk/integrations/openai_agents/patches/models.py

@@ -133,8 +133,6 @@ async def wrapped_get_response(*args: "Any", **kwargs: "Any") -> "Any":
 
         @wraps(original_stream_response)
         async def wrapped_stream_response(*args: "Any", **kwargs: "Any") -> "Any":
-            # Uses explicit try/finally instead of context manager to ensure cleanup
-            # even if the consumer abandons the stream (GeneratorExit).
             span_kwargs = dict(kwargs)
             if len(args) > 0:
                 span_kwargs["system_instructions"] = args[0]

sentry_sdk/scope.py

@@ -34,7 +34,7 @@
     make_sampling_decision,
     PropagationContext,
 )
-from sentry_sdk.traces import StreamedSpan, NoOpStreamedSpan
+from sentry_sdk.traces import _DEFAULT_PARENT_SPAN, StreamedSpan, NoOpStreamedSpan
 from sentry_sdk.tracing import (
     BAGGAGE_HEADER_NAME,
     SENTRY_TRACE_HEADER_NAME,
@@ -1224,9 +1224,9 @@ def start_span(
     def start_streamed_span(
         self,
         name: str,
-        attributes: "Optional[Attributes]" = None,
-        parent_span: "Optional[StreamedSpan]" = None,
-        active: bool = True,
+        attributes: "Optional[Attributes]",
+        parent_span: "Optional[StreamedSpan]",
+        active: bool,
     ) -> "StreamedSpan":
         # TODO: rename to start_span once we drop the old API
         if isinstance(parent_span, NoOpStreamedSpan):
@@ -1236,7 +1236,9 @@ def start_streamed_span(
                 "currently active span instead."
             )
 
-        if parent_span is None or isinstance(parent_span, NoOpStreamedSpan):
+        if parent_span is _DEFAULT_PARENT_SPAN or isinstance(
+            parent_span, NoOpStreamedSpan
+        ):
             parent_span = self.span  # type: ignore
 
         # If no eligible parent_span was provided and there is no currently

sentry_sdk/traces.py

@@ -84,18 +84,24 @@ def __str__(self) -> str:
 }
 
 
+# Sentinel value for an unset parent_span to be able to distinguish it from
+# a None set by the user
+_DEFAULT_PARENT_SPAN = object()
+
+
 def start_span(
     name: str,
     attributes: "Optional[Attributes]" = None,
-    parent_span: "Optional[StreamedSpan]" = None,
+    parent_span: "Optional[StreamedSpan]" = _DEFAULT_PARENT_SPAN,  # type: ignore[assignment]
     active: bool = True,
 ) -> "StreamedSpan":
     """
     Start a span.
 
     The span's parent, unless provided explicitly via the `parent_span` argument,
     will be the current active span, if any. If there is none, this span will
-    become the root of a new span tree.
+    become the root of a new span tree. If you explicitly want this span to be
+    top-level without a parent, set `parent_span=None`.
 
     `start_span()` can either be used as context manager or you can use the span
     object it returns and explicitly end it via `span.end()`. The following is
@@ -130,7 +136,8 @@ def start_span(
 
     :param parent_span: A span instance that the new span should consider its
         parent. If not provided, the parent will be set to the currently active
-        span, if any.
+        span, if any. If set to `None`, this span will become a new root-level
+        span.
     :type parent_span: "Optional[StreamedSpan]"
 
     :param active: Controls whether spans started while this span is running
@@ -593,6 +600,18 @@ def trace_id(self) -> str:
     def sampled(self) -> "Optional[bool]":
         return False
 
+    @property
+    def status(self) -> "str":
+        return SpanStatus.OK.value
+
+    @status.setter
+    def status(self, status: "Union[SpanStatus, str]") -> None:
+        pass
+
+    @property
+    def active(self) -> bool:
+        return True
+
     def _start(self) -> None:
         if self._scope is None:
             return self
@@ -643,7 +662,7 @@ def remove_attribute(self, key: str) -> None:
     def is_segment(self) -> bool:
         return False
 
-    def to_traceparent(self) -> str:
+    def _to_traceparent(self) -> str:
         propagation_context = (
             sentry_sdk.get_current_scope().get_active_propagation_context()
         )

sentry_sdk/utils.py

@@ -2005,7 +2005,11 @@ def serialize_item(
 
     try:
         serialized = serialize_item(data)
-        return json.dumps(serialized, default=str)
+        return (
+            json.dumps(serialized, default=str)
+            if not isinstance(serialized, str)
+            else serialized
+        )
     except Exception:
         return str(data)
 

tests/integrations/google_genai/test_google_genai.py

@@ -1625,7 +1625,7 @@ def test_generate_content_with_function_response(
     assert messages[0]["role"] == "tool"
     assert messages[0]["content"]["toolCallId"] == "call_123"
     assert messages[0]["content"]["toolName"] == "get_weather"
-    assert messages[0]["content"]["output"] == '"Sunny, 72F"'
+    assert messages[0]["content"]["output"] == "Sunny, 72F"
 
 
 def test_generate_content_with_mixed_string_and_content(
@@ -1891,7 +1891,7 @@ def test_extract_contents_messages_function_response():
     assert result[0]["role"] == "tool"
     assert result[0]["content"]["toolCallId"] == "call_123"
     assert result[0]["content"]["toolName"] == "get_weather"
-    assert result[0]["content"]["output"] == '"sunny"'
+    assert result[0]["content"]["output"] == "sunny"
 
 
 def test_extract_contents_messages_function_response_with_output_key():
@@ -1908,7 +1908,7 @@ def test_extract_contents_messages_function_response_with_output_key():
     assert result[0]["content"]["toolCallId"] == "call_456"
     assert result[0]["content"]["toolName"] == "get_time"
     # Should prefer "output" key
-    assert result[0]["content"]["output"] == '"3:00 PM"'
+    assert result[0]["content"]["output"] == "3:00 PM"
 
 
 def test_extract_contents_messages_mixed_parts():

tests/integrations/mcp/test_mcp.py

@@ -241,15 +241,12 @@ async def test_tool_async(tool_name, arguments):
     assert span["data"][SPANDATA.MCP_TRANSPORT] == "http"
     assert span["data"][SPANDATA.MCP_REQUEST_ID] == "req-456"
     assert span["data"][SPANDATA.MCP_SESSION_ID] == session_id
-    assert span["data"]["mcp.request.argument.data"] == '"test"'
+    assert span["data"]["mcp.request.argument.data"] == "test"
 
     # Check PII-sensitive data
     if send_default_pii and include_prompts:
-        # TODO: Investigate why tool result is double-serialized.
         assert span["data"][SPANDATA.MCP_TOOL_RESULT_CONTENT] == json.dumps(
-            json.dumps(
-                {"status": "completed"},
-            )
+            {"status": "completed"}
         )
     else:
         assert SPANDATA.MCP_TOOL_RESULT_CONTENT not in span["data"]
@@ -366,8 +363,8 @@ async def test_prompt(name, arguments):
     assert span["data"][SPANDATA.MCP_METHOD_NAME] == "prompts/get"
     assert span["data"][SPANDATA.MCP_TRANSPORT] == "stdio"
     assert span["data"][SPANDATA.MCP_REQUEST_ID] == "req-prompt"
-    assert span["data"]["mcp.request.argument.name"] == '"code_help"'
-    assert span["data"]["mcp.request.argument.language"] == '"python"'
+    assert span["data"]["mcp.request.argument.name"] == "code_help"
+    assert span["data"]["mcp.request.argument.language"] == "python"
 
     # Message count is always captured
     assert span["data"][SPANDATA.MCP_PROMPT_RESULT_MESSAGE_COUNT] == 1
@@ -752,7 +749,7 @@ def test_tool_unstructured(tool_name, arguments):
     # Should extract and join text from content blocks only with PII
     if send_default_pii and include_prompts:
         assert (
-            span["data"][SPANDATA.MCP_TOOL_RESULT_CONTENT] == '"First part Second part"'
+            span["data"][SPANDATA.MCP_TOOL_RESULT_CONTENT] == "First part Second part"
         )
     else:
         assert SPANDATA.MCP_TOOL_RESULT_CONTENT not in span["data"]
@@ -959,7 +956,7 @@ def test_tool_complex(tool_name, arguments):
     assert span["data"]["mcp.request.argument.nested"] == json.dumps(
         {"key": "value", "list": [1, 2, 3]}
     )
-    assert span["data"]["mcp.request.argument.string"] == '"test"'
+    assert span["data"]["mcp.request.argument.string"] == "test"
     assert span["data"]["mcp.request.argument.number"] == "42"
 
 

tests/integrations/openai_agents/test_openai_agents.py

@@ -12,7 +12,7 @@
 from sentry_sdk.integrations.logging import LoggingIntegration
 from sentry_sdk.integrations.openai_agents import OpenAIAgentsIntegration
 from sentry_sdk.integrations.openai_agents.utils import _set_input_data, safe_serialize
-from sentry_sdk.utils import parse_version
+from sentry_sdk.utils import parse_version, package_version
 
 from openai import AsyncOpenAI
 from agents.models.openai_responses import OpenAIResponsesModel
@@ -37,6 +37,8 @@
 from agents.exceptions import MaxTurnsExceeded, ModelBehaviorError
 from agents.version import __version__ as OPENAI_AGENTS_VERSION
 
+OPENAI_VERSION = package_version("openai")
+
 from openai.types.responses import (
     ResponseCreatedEvent,
     ResponseTextDeltaEvent,
@@ -1256,18 +1258,22 @@ def simple_test_tool(message: str) -> str:
     assert ai_client_span1["data"]["gen_ai.usage.output_tokens"] == 5
     assert ai_client_span1["data"]["gen_ai.usage.output_tokens.reasoning"] == 0
     assert ai_client_span1["data"]["gen_ai.usage.total_tokens"] == 15
-    assert ai_client_span1["data"]["gen_ai.response.tool_calls"] == safe_serialize(
-        [
-            {
-                "arguments": '{"message": "hello"}',
-                "call_id": "call_123",
-                "name": "simple_test_tool",
-                "type": "function_call",
-                "id": "call_123",
-                "status": None,
-            }
-        ]
-    )
+
+    tool_call = {
+        "arguments": '{"message": "hello"}',
+        "call_id": "call_123",
+        "name": "simple_test_tool",
+        "type": "function_call",
+        "id": "call_123",
+        "status": None,
+    }
+
+    if OPENAI_VERSION >= (2, 25, 0):
+        tool_call["namespace"] = None
+
+    assert json.loads(ai_client_span1["data"]["gen_ai.response.tool_calls"]) == [
+        tool_call
+    ]
 
     assert tool_span["description"] == "execute_tool simple_test_tool"
     assert tool_span["data"]["gen_ai.agent.name"] == "test_agent"
@@ -2507,75 +2513,6 @@ def calculator(a: int, b: int) -> int:
     assert invoke_agent_span["data"]["gen_ai.usage.output_tokens.reasoning"] == 3
 
 
-@pytest.mark.asyncio
-async def test_response_model_not_set_when_unavailable(
-    sentry_init, capture_events, test_agent
-):
-    """
-    Test that response model is not set if the API response doesn't have a model field.
-    The request model should still be set correctly.
-    """
-
-    with patch.dict(os.environ, {"OPENAI_API_KEY": "test-key"}):
-        with patch(
-            "agents.models.openai_responses.OpenAIResponsesModel._fetch_response"
-        ) as mock_fetch_response:
-            # Create a mock response without a model field
-            mock_response = MagicMock()
-            mock_response.model = None  # No model in response
-            mock_response.id = "resp_123"
-            mock_response.output = [
-                ResponseOutputMessage(
-                    id="msg_123",
-                    type="message",
-                    status="completed",
-                    content=[
-                        ResponseOutputText(
-                            text="Response without model field",
-                            type="output_text",
-                            annotations=[],
-                        )
-                    ],
-                    role="assistant",
-                )
-            ]
-            mock_response.usage = MagicMock()
-            mock_response.usage.input_tokens = 10
-            mock_response.usage.output_tokens = 20
-            mock_response.usage.total_tokens = 30
-            mock_response.usage.input_tokens_details = InputTokensDetails(
-                cached_tokens=0
-            )
-            mock_response.usage.output_tokens_details = OutputTokensDetails(
-                reasoning_tokens=0
-            )
-
-            mock_fetch_response.return_value = mock_response
-
-            sentry_init(
-                integrations=[OpenAIAgentsIntegration()],
-                traces_sample_rate=1.0,
-            )
-
-            events = capture_events()
-
-            result = await agents.Runner.run(
-                test_agent, "Test input", run_config=test_run_config
-            )
-
-            assert result is not None
-
-    (transaction,) = events
-    spans = transaction["spans"]
-    _, ai_client_span = spans
-
-    # Response model should NOT be set when API doesn't return it
-    assert "gen_ai.response.model" not in ai_client_span["data"]
-    # But request model should still be set
-    assert "gen_ai.request.model" in ai_client_span["data"]
-    assert ai_client_span["data"]["gen_ai.request.model"] == "gpt-4"
-
-
 @pytest.mark.asyncio
 async def test_invoke_agent_span_includes_response_model(
     sentry_init, capture_events, test_agent

tests/test_utils.py

@@ -35,6 +35,7 @@
     exc_info_from_error,
     get_lines_from_file,
     package_version,
+    safe_serialize,
 )
 
 
@@ -1062,5 +1063,36 @@ def fake_getlines(filename):
         assert result == expected_result
 
 
+def test_safe_serialize_plain_string():
+    assert safe_serialize("already a string") == "already a string"
+
+
+def test_safe_serialize_json_string():
+    assert safe_serialize('{"key": "value"}') == '{"key": "value"}'
+
+
+def test_safe_serialize_dict():
+    assert safe_serialize({"key": "value"}) == '{"key": "value"}'
+
+
+def test_safe_serialize_callable():
+    def my_func():
+        pass
+
+    result = safe_serialize(my_func)
+    assert result.startswith("<function")
+    assert '"' not in result[:1]  # no wrapping quotes from json.dumps
+
+
+def test_safe_serialize_object():
+    class MyClass:
+        def __init__(self):
+            self.x = 1
+
+    result = safe_serialize(MyClass())
+    assert result.startswith("<MyClass")
+    assert '"' not in result[:1]  # no wrapping quotes from json.dumps
+
+
 def test_package_version_is_none():
     assert package_version("non_existent_package") is None