fix: serialize guardrail_response to JSON in OTEL traces#28362
Conversation
|
|
|
@greptileai please review |
Greptile SummaryThis PR fixes
Confidence Score: 5/5Safe to merge — the change is a one-liner in a non-critical observability path, consistent with the existing masked_entity_count pattern, and covered by targeted new tests. The fix is isolated to the guardrail OTEL span writer, follows an already-established pattern in the same method, introduces no new dependencies, and is backed by two new tests that directly exercise the corrected behaviour plus updated assertions on the existing ones. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/integrations/opentelemetry.py | Switches guardrail_response from safe_set_attribute to safe_dumps + direct set_attribute, matching the existing pattern for masked_entity_count; adds a None guard to omit the attribute when absent. |
| tests/test_litellm/integrations/test_opentelemetry.py | Updates two existing assertions to expect safe_dumps-serialized values; adds two new focused tests covering dict payloads and the None-skipping behaviour. |
Reviews (3): Last reviewed commit: "test: cover dict-serialization and None-..." | Re-trigger Greptile
| guardrail_response = guardrail_information.get("guardrail_response") | ||
| if guardrail_response is not None: | ||
| guardrail_span.set_attribute( | ||
| "guardrail_response", safe_dumps(guardrail_response) | ||
| ) |
There was a problem hiding this comment.
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.
| 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), | |
| ) |
Greptile SummaryThis PR fixes
Confidence Score: 5/5Safe to merge — a small, targeted serialization fix with no side effects on the broader request path. The change is minimal and self-contained: one guard-and-serialize block replaces one No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/integrations/opentelemetry.py | Replaces safe_set_attribute (which produced Python repr strings via str()) with safe_dumps for guardrail_response, and skips the attribute when the value is None — consistent with the existing masked_entity_count pattern two lines above. |
| tests/test_litellm/integrations/test_opentelemetry.py | Two test assertions updated to expect safe_dumps("filtered_content") instead of the bare string, accurately reflecting the new serialization behavior; no test coverage weakened. |
Reviews (2): Last reviewed commit: "fix: serialize guardrail_response to JSO..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Address Greptile feedback on #28362 — add explicit coverage for the two behavioral guarantees of this fix: - Dict payloads (the OpenAI moderation case in the report) reach the span as a JSON string, not a Python repr. - ``None`` guardrail_response skips the attribute entirely, so no ``"null"`` leaks into traces. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@greptileai I addressed your feedback by adding two new tests: one verifying that dict payloads (the OpenAI moderation case from the report) are JSON-serialized, and one verifying that |
Guardrail spans previously set the `guardrail_response` attribute via `safe_set_attribute`, which let dict payloads reach the OTEL exporter as Python repr strings. Downstream log pipelines could not parse those as JSON, breaking metric creation from guardrail traces. Serialize `guardrail_response` with `safe_dumps` before setting the attribute, matching how `masked_entity_count` is already handled. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address Greptile feedback on #28362 — add explicit coverage for the two behavioral guarantees of this fix: - Dict payloads (the OpenAI moderation case in the report) reach the span as a JSON string, not a Python repr. - ``None`` guardrail_response skips the attribute entirely, so no ``"null"`` leaks into traces. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
152bcd2 to
e4946b5
Compare
35520ad
into
litellm_internal_staging
* fix: serialize guardrail_response to JSON in OTEL traces Guardrail spans previously set the `guardrail_response` attribute via `safe_set_attribute`, which let dict payloads reach the OTEL exporter as Python repr strings. Downstream log pipelines could not parse those as JSON, breaking metric creation from guardrail traces. Serialize `guardrail_response` with `safe_dumps` before setting the attribute, matching how `masked_entity_count` is already handled. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: cover dict-serialization and None-skip for guardrail_response Address Greptile feedback on BerriAI#28362 — add explicit coverage for the two behavioral guarantees of this fix: - Dict payloads (the OpenAI moderation case in the report) reach the span as a JSON string, not a Python repr. - ``None`` guardrail_response skips the attribute entirely, so no ``"null"`` leaks into traces. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Yassin Kortam <yassinkortam@g.ucla.edu> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Guardrail OTEL spans previously set
guardrail_responseviasafe_set_attribute, letting dict payloads reach the exporter as Python repr strings (e.g."{'id': 'modr-7740', ...}"). Downstream log pipelines could not parse those as JSON, breaking metric creation from guardrail traces.Serialize
guardrail_responsewithsafe_dumpsbefore setting the span attribute — same treatmentmasked_entity_countalready gets two lines above. Drop the attribute entirely when the value isNone.Resolves LIT-3233
Changes
litellm/integrations/opentelemetry.py— JSON-serializeguardrail_response, skip onNone.tests/test_litellm/integrations/test_opentelemetry.py— update bothtest_create_guardrail_span_with_valid_infoassertions to expectsafe_dumps("filtered_content").Test plan
uv run pytest tests/test_litellm/integrations/test_opentelemetry.py -v— 219 passeduv run ruff check litellm/— cleanuv run black --checkon both files — cleanuv run mypy litellm/integrations/opentelemetry.py --ignore-missing-imports— clean@greptileaireview with confidence score >= 4/5🤖 Generated with Claude Code