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
10 changes: 5 additions & 5 deletions litellm/integrations/opentelemetry.py
Original file line number Diff line number Diff line change
Expand Up @@ -1611,11 +1611,11 @@ def _create_guardrail_span(
"masked_entity_count", safe_dumps(masked_entity_count)
)

self.safe_set_attribute(
span=guardrail_span,
key="guardrail_response",
value=guardrail_information.get("guardrail_response"),
)
guardrail_response = guardrail_information.get("guardrail_response")
if guardrail_response is not None:
guardrail_span.set_attribute(
"guardrail_response", safe_dumps(guardrail_response)
)
Comment on lines +1614 to +1618

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 String values are double-encoded by safe_dumps. When guardrail_response is already a plain string (e.g. "filtered_content"), safe_dumps wraps it in JSON quotes producing '"filtered_content"'. Downstream consumers reading the attribute expecting a bare string will now get an extra layer of quoting. The primary bug was with dict/object payloads; strings didn't need to be re-serialized. Consider only applying safe_dumps when the value is not already a str.

Suggested change
guardrail_response = guardrail_information.get("guardrail_response")
if guardrail_response is not None:
guardrail_span.set_attribute(
"guardrail_response", safe_dumps(guardrail_response)
)
guardrail_response = guardrail_information.get("guardrail_response")
if guardrail_response is not None:
guardrail_span.set_attribute(
"guardrail_response",
guardrail_response if isinstance(guardrail_response, str) else safe_dumps(guardrail_response),
)


self._set_team_attributes_from_kwargs(guardrail_span, kwargs)

Expand Down
63 changes: 61 additions & 2 deletions tests/test_litellm/integrations/test_opentelemetry.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def test_create_guardrail_span_with_valid_info(self, mock_datetime):
mock_span.set_attribute.assert_any_call("guardrail_name", "test_guardrail")
mock_span.set_attribute.assert_any_call("guardrail_mode", "input")
mock_span.set_attribute.assert_any_call(
"guardrail_response", "filtered_content"
"guardrail_response", safe_dumps("filtered_content")
)
mock_span.set_attribute.assert_any_call(
"masked_entity_count", safe_dumps({"CREDIT_CARD": 2})
Expand All @@ -87,6 +87,65 @@ def test_create_guardrail_span_with_no_info(self):
# Verify that start_span was never called
otel.tracer.start_span.assert_not_called()

@patch("litellm.integrations.opentelemetry.datetime")
def test_guardrail_response_dict_is_json_serialized(self, mock_datetime):
"""Dict guardrail_response (e.g. OpenAI moderation result) must reach
the span as a JSON string so downstream pipelines can parse it for
metric extraction — this is the bug the PR fixes."""
otel = OpenTelemetry()
otel.tracer = MagicMock()
mock_span = MagicMock()
otel.tracer.start_span.return_value = mock_span

moderation_payload = {
"id": "modr-7740",
"model": "omni-moderation-latest",
"results": [{"categories": {"harassment": False}}],
}
guardrail_info = {
"guardrail_name": "test_guardrail",
"guardrail_mode": "input",
"guardrail_response": moderation_payload,
"start_time": 1609459200.0,
"end_time": 1609459201.0,
}
kwargs = {
"standard_logging_object": {"guardrail_information": [guardrail_info]}
}

otel._create_guardrail_span(kwargs=kwargs, context=None)

mock_span.set_attribute.assert_any_call(
"guardrail_response", safe_dumps(moderation_payload)
)

@patch("litellm.integrations.opentelemetry.datetime")
def test_guardrail_response_none_is_skipped(self, mock_datetime):
"""When guardrail_response is None, the attribute must not be set —
guards against round-tripping ``"null"`` into traces."""
otel = OpenTelemetry()
otel.tracer = MagicMock()
mock_span = MagicMock()
otel.tracer.start_span.return_value = mock_span

guardrail_info = {
"guardrail_name": "test_guardrail",
"guardrail_mode": "input",
"guardrail_response": None,
"start_time": 1609459200.0,
"end_time": 1609459201.0,
}
kwargs = {
"standard_logging_object": {"guardrail_information": [guardrail_info]}
}

otel._create_guardrail_span(kwargs=kwargs, context=None)

attribute_keys = [
call.args[0] for call in mock_span.set_attribute.call_args_list
]
self.assertNotIn("guardrail_response", attribute_keys)


class TestOpenTelemetryTeamAttributesOnChildSpans(unittest.TestCase):
"""team_id / team_alias must land on every child span of a
Expand Down Expand Up @@ -1169,7 +1228,7 @@ def test_create_guardrail_span_with_valid_info(self, mock_datetime):
mock_span.set_attribute.assert_any_call("guardrail_name", "test_guardrail")
mock_span.set_attribute.assert_any_call("guardrail_mode", "input")
mock_span.set_attribute.assert_any_call(
"guardrail_response", "filtered_content"
"guardrail_response", safe_dumps("filtered_content")
)
mock_span.set_attribute.assert_any_call(
"masked_entity_count", safe_dumps({"CREDIT_CARD": 2})
Expand Down
Loading