fix(otel): emit guardrail span on violation, surface status + categories - #28364
Conversation
|
|
Greptile SummaryThis PR fixes two gaps in guardrail OTEL tracing: the
Confidence Score: 5/5Safe to merge — all changed paths are additive observability instrumentation with no effect on request routing or auth. All previously identified blocking findings have been addressed: Bedrock-specific parsing now lives in No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/integrations/opentelemetry.py | Adds _emit_guardrail_spans_from_request_data to emit guardrail child spans from the failure hook using the same metadata dict for dedup; enriches every guardrail span with guardrail_status, guardrail_action, and guardrail_violation_categories read from normalised fields on StandardLoggingGuardrailInformation. |
| litellm/proxy/guardrails/guardrail_hooks/bedrock_guardrails.py | Adds _extract_violation_category_names to build a safe, non-sensitive label list before redaction; propagates both violation_categories and guardrail_action via GuardrailTracingDetail into StandardLoggingGuardrailInformation. |
| litellm/types/utils.py | Adds violation_categories and guardrail_action fields to both StandardLoggingGuardrailInformation and GuardrailTracingDetail TypedDicts. |
| tests/test_litellm/integrations/test_otel_guardrail_violation_spans.py | New test file covering failure-path span emission, dedup via shared metadata dict identity, attribute presence for all GuardrailStatus values, guardrail_action population, and an end-to-end flow through a real CustomGuardrail subclass. Correctly uses a _run() helper that closes the event loop. |
| tests/test_litellm/proxy/guardrails/guardrail_hooks/test_bedrock_guardrails.py | Adds unit tests for _extract_violation_category_names (mixed policies, security/no-leak, named-regex, ANONYMIZED skip, empty assessments) and two async tests verifying guardrail_action forwarding through tracing_detail. |
Reviews (8): Last reviewed commit: "fix(otel): emit guardrail_action span at..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@greptileai re-review please — addressed feedback: pushed Bedrock parsing into BedrockGuardrail (added |
|
Note for human reviewers: the 3 inline Greptile comments still attached to commit
Bedrock-specific parsing has been moved out of |
PR overviewGuardrail telemetry spans expandedThis PR adds failure-path guardrail span emission and surfaces Bedrock guardrail status/action/category attributes in OTel. I checked the new span data flow from Bedrock responses through standard guardrail logging, including redaction boundaries and the request metadata fallback path, and did not find a concrete security issue. Security review
Risk: 2/10 |
|
@greptileai please re-review on commit |
c5ddb60 to
ab47640
Compare
|
@greptileai review |
|
@greptileai please re-review on commit |
ac18aa4 to
3145196
Compare
…n pre-call blocks - Fix missing guardrail child spans when a pre-call guardrail blocks the request before reaching the LLM provider; `async_post_call_failure_hook` now calls `_emit_guardrail_spans_from_request_data` to emit spans from `request_data["metadata"]` regardless of whether `_handle_failure` already fired - Add `guardrail_status`, `guardrail_action`, and `guardrail_violation_categories` as queryable top-level OTEL span attributes so trace backends can filter/group by violation type without parsing the redacted `guardrail_response` blob - Introduce `_emit_guardrail_spans_from_request_data` helper that constructs minimal kwargs from `request_data["metadata"]` and routes through `_create_guardrail_span`, sharing the same dedupe state to prevent double-emitting when both failure hooks fire - Extend `BedrockGuardrail` with `_build_tracing_detail` and `_extract_violation_category_names` which flatten BLOCKED assessments into human-readable category labels (topic names, content-filter types, PII entity types, named regex names) before redaction, and surface Bedrock's raw `action` field via `tracing_detail` - Security: violation category extraction deliberately omits `customWords.match` and unnamed regex `match` values because those fields carry the user-submitted content that triggered the rule; only operator-defined `name`/`type` labels are emitted - Add `violation_categories` and `guardrail_action` fields to `StandardLoggingGuardrailInformation` and `GuardrailTracingDetail` TypedDicts to carry the pre-redaction metadata through the logging pipeline - Add comprehensive test suite covering: guardrail span creation on failure, dedupe between `_handle_failure` and `async_post_call_failure_hook`, per-span status attributes for multi-guardrail sequences, Bedrock category extraction for all policy types, security leak prevention, and end-to-end `CustomGuardrail` violation path
3145196 to
1188d73
Compare
10bd740
into
litellm_internal_staging
* feat: add guardrail violation span attributes and fix missing spans on pre-call blocks (#28364) - Fix missing guardrail child spans when a pre-call guardrail blocks the request before reaching the LLM provider; `async_post_call_failure_hook` now calls `_emit_guardrail_spans_from_request_data` to emit spans from `request_data["metadata"]` regardless of whether `_handle_failure` already fired - Add `guardrail_status`, `guardrail_action`, and `guardrail_violation_categories` as queryable top-level OTEL span attributes so trace backends can filter/group by violation type without parsing the redacted `guardrail_response` blob - Introduce `_emit_guardrail_spans_from_request_data` helper that constructs minimal kwargs from `request_data["metadata"]` and routes through `_create_guardrail_span`, sharing the same dedupe state to prevent double-emitting when both failure hooks fire - Extend `BedrockGuardrail` with `_build_tracing_detail` and `_extract_violation_category_names` which flatten BLOCKED assessments into human-readable category labels (topic names, content-filter types, PII entity types, named regex names) before redaction, and surface Bedrock's raw `action` field via `tracing_detail` - Security: violation category extraction deliberately omits `customWords.match` and unnamed regex `match` values because those fields carry the user-submitted content that triggered the rule; only operator-defined `name`/`type` labels are emitted - Add `violation_categories` and `guardrail_action` fields to `StandardLoggingGuardrailInformation` and `GuardrailTracingDetail` TypedDicts to carry the pre-redaction metadata through the logging pipeline - Add comprehensive test suite covering: guardrail span creation on failure, dedupe between `_handle_failure` and `async_post_call_failure_hook`, per-span status attributes for multi-guardrail sequences, Bedrock category extraction for all policy types, security leak prevention, and end-to-end `CustomGuardrail` violation path Co-authored-by: Yassin Kortam <yassinkortam@g.ucla.edu> * test(proxy): behavior-pinning matrix for team management endpoints (#28441) * test(proxy): behavior-pinning matrix for team management endpoints PR2 (Team Tier-1) of the management-endpoint behavior-pinning effort. Extends the tests/proxy_behavior/management/ harness PR1 built and adds the actor x target-resource authz matrix for the 7 team endpoints: /team/new, /team/info, /team/list, /team/update, /team/member_add, /team/member_delete, /team/member_update. Tests-only, no production code changes. Harness extensions: - actors.py: ORG_B_ADMIN actor (org admin of ORG_B) and TEAM_GAMMA (an ORG_A team with no actor members), so team-targeting endpoints get a clean own / same-org-other / cross-org target axis. - conftest.py: create_scratch_team() raw-seeds target teams without /team/new side effects; the scratch teardown now also strips dangling scratch-team refs from LiteLLM_UserTable.teams. 156 new scenarios; status codes pinned to observed handler behavior. * test(proxy): record mutmut run blockers in PR2 triage doc Attempted a scoped local mutmut run for G5; it did not complete. Record the three concrete blockers in mutmut_triage/pr2-team-tier1.md so the next attempt has a head start: 1. mutmut's mutants/ sandbox is import-shadowed by the worktree source. 2. the legacy mock suite and the real-DB behavior suite cannot share a pytest session (mock suite globally patches prisma_client). 3. the CI mutation-test.yml workflow starts no Postgres, so its stats phase now aborts on the behavior-suite tests PR1 added to tests_dir. mutmut stays a deferred follow-up (as in PR1); the binding pre-merge signal remains the behavior matrix (G1) and the G4 regression-replay. * test(proxy): drop suite README + triage doc, trim test comments Remove the two prose docs from the behavior suite (README.md and mutmut_triage/pr2-team-tier1.md) and tighten the comment blocks on the team test files + harness down to the load-bearing parts (the gate each matrix pins, plus genuinely surprising results). No behavior change — all 286 scenarios still pass. * test(proxy): remove mutmut tests_dir comment * test(vertex_ai): tolerate transient 500 in google maps grounding test (#28503) test_gemini_google_maps_tool_simple makes live calls to Vertex AI's Google Maps grounding backend, which intermittently returns 500 INTERNAL ("Please retry") — a transient Google-side failure, not a LiteLLM bug. The request LiteLLM emits matches Google's published googleMaps grounding spec field-for-field, and the maps-platform 500 only occurs after Vertex accepts the request. The test already passes on RateLimitError; treat InternalServerError the same way so transient Vertex-side failures don't fail CI. * fix(docker): restore npm to non_root builder image (#28519) The non_root builder stage installs `nodejs` but not `npm`. Without `npm` on PATH, prisma-python falls back to downloading a Node runtime via nodeenv from nodejs.org, and that downloaded binary fails to load `libatomic.so.1` — breaking `prisma generate` and the image build. `npm` was dropped from this apk list in ca52e34. Restoring it lets prisma-python use the system Node + npm, matching docker/Dockerfile which already installs `npm` for the same reason. * build(deps): bump next from 16.2.4 to 16.2.6 in /ui/litellm-dashboard (#27665) (#28524) Bumps [next](https://github.com/vercel/next.js) from 16.2.4 to 16.2.6. - [Release notes](https://github.com/vercel/next.js/releases) - [Changelog](https://github.com/vercel/next.js/blob/canary/release.js) - [Commits](vercel/next.js@v16.2.4...v16.2.6) --- updated-dependencies: - dependency-name: next dependency-version: 16.2.6 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps-dev): bump black to 26.3.1 and apply formatting (#28525) * build(deps-dev): bump black 24.10.0 -> 26.3.1 * style: apply black 26.3.1 formatting * chore: authorize black 26.3.1 license in liccheck.ini * chore(deps): bump deps (#28528) * build(deps): bump next from 16.2.4 to 16.2.6 in /ui/litellm-dashboard (#27665) Bumps [next](https://github.com/vercel/next.js) from 16.2.4 to 16.2.6. - [Release notes](https://github.com/vercel/next.js/releases) - [Changelog](https://github.com/vercel/next.js/blob/canary/release.js) - [Commits](vercel/next.js@v16.2.4...v16.2.6) --- updated-dependencies: - dependency-name: next dependency-version: 16.2.6 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump protobufjs in /tests/pass_through_tests (#28296) Bumps [protobufjs](https://github.com/protobufjs/protobuf.js) from 7.5.6 to 7.6.0. - [Release notes](https://github.com/protobufjs/protobuf.js/releases) - [Changelog](https://github.com/protobufjs/protobuf.js/blob/protobufjs-v7.6.0/CHANGELOG.md) - [Commits](protobufjs/protobuf.js@protobufjs-v7.5.6...protobufjs-v7.6.0) --- updated-dependencies: - dependency-name: protobufjs dependency-version: 7.6.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump ws from 8.20.0 to 8.20.1 in /tests/pass_through_tests (#28303) Bumps [ws](https://github.com/websockets/ws) from 8.20.0 to 8.20.1. - [Release notes](https://github.com/websockets/ws/releases) - [Commits](websockets/ws@8.20.0...8.20.1) --- updated-dependencies: - dependency-name: ws dependency-version: 8.20.1 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * test(e2e): forward LITELLM_LICENSE to UI e2e proxy (#28398) * test(e2e): forward LITELLM_LICENSE to UI e2e proxy The UI e2e job ran without LITELLM_LICENSE, so premium_user was always false in the issued login JWT and premium-gated UI surfaces (Team-BYOK Model switch, etc.) couldn't be driven through the UI. Forward the env var from run_e2e.sh and the CircleCI e2e_ui_testing job, and add a sanity test that decodes the admin storage state token and asserts premium_user=true so the wiring fails loudly if it ever regresses. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Update ui/litellm-dashboard/e2e_tests/tests/proxy-admin/license.spec.ts Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * Add granian as a ASGI compliant web server. Provider better throughput stability, (#26027) * Add granian as a ASGI compliant web server. Provides better stability, 10-20 RPS improvement under standard LT conditions. TODO: Verify poetry lock details and add locust numbers to PR * Update granian version in license_cache.json and pyproject.toml to 2.5.7 * Enhance proxy CLI tests by adding SSL initialization checks for Granian server. Remove Python version skip conditions and implement tests to ensure SSL certificate and key are required for server initialization. * update uv lock to fix granian import error --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Yassin Kortam <yassin@berri.ai> Co-authored-by: Yassin Kortam <yassinkortam@g.ucla.edu> Co-authored-by: yuneng-jiang <yuneng@berri.ai> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: ryan-crabbe-berri <ryan@berri.ai> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Co-authored-by: harish-berri <harish@berri.ai>
…n pre-call blocks (BerriAI#28364) - Fix missing guardrail child spans when a pre-call guardrail blocks the request before reaching the LLM provider; `async_post_call_failure_hook` now calls `_emit_guardrail_spans_from_request_data` to emit spans from `request_data["metadata"]` regardless of whether `_handle_failure` already fired - Add `guardrail_status`, `guardrail_action`, and `guardrail_violation_categories` as queryable top-level OTEL span attributes so trace backends can filter/group by violation type without parsing the redacted `guardrail_response` blob - Introduce `_emit_guardrail_spans_from_request_data` helper that constructs minimal kwargs from `request_data["metadata"]` and routes through `_create_guardrail_span`, sharing the same dedupe state to prevent double-emitting when both failure hooks fire - Extend `BedrockGuardrail` with `_build_tracing_detail` and `_extract_violation_category_names` which flatten BLOCKED assessments into human-readable category labels (topic names, content-filter types, PII entity types, named regex names) before redaction, and surface Bedrock's raw `action` field via `tracing_detail` - Security: violation category extraction deliberately omits `customWords.match` and unnamed regex `match` values because those fields carry the user-submitted content that triggered the rule; only operator-defined `name`/`type` labels are emitted - Add `violation_categories` and `guardrail_action` fields to `StandardLoggingGuardrailInformation` and `GuardrailTracingDetail` TypedDicts to carry the pre-redaction metadata through the logging pipeline - Add comprehensive test suite covering: guardrail span creation on failure, dedupe between `_handle_failure` and `async_post_call_failure_hook`, per-span status attributes for multi-guardrail sequences, Bedrock category extraction for all policy types, security leak prevention, and end-to-end `CustomGuardrail` violation path Co-authored-by: Yassin Kortam <yassinkortam@g.ucla.edu>
Summary
Resolves LIT-3234
When a pre-call guardrail blocked a request, the production failure path was missing the
guardrailchild span entirely, and the guardrail span on the_handle_failurepath didn't surfaceguardrail_statusor the violated categories as queryable attributes — that data was buried inside the serialisedguardrail_responseblob.This PR:
Emits
guardrailspans fromasync_post_call_failure_hookby readingrequest_data["metadata"]["standard_logging_guardrail_information"]and routing through_create_guardrail_span. Dedupe via_emit_oncekeeps this safe even when_handle_failurealready emitted the span on the same kwargs.Enriches every guardrail span with
guardrail_status(success/guardrail_intervened/guardrail_failed_to_respond/not_run),guardrail_action(the provider's top-level action — Bedrock'sGUARDRAIL_INTERVENED/NONE), and — when the guardrail intervened —guardrail_violation_categoriesextracted from Bedrockassessments[*]policy items whose action isBLOCKED/ANONYMIZED.Repro
A pre-call guardrail block from a Bedrock guardrail looks like this:
pre_call.OTEL_EXPORTER=console)./chat/completionsrequest that triggers a topic-policy / content-policy block.The same path is exercised in a standalone script that does not require AWS:
Before / after —
async_post_call_failure_hook(prod failure path)Before: no
guardrailspan emitted at all.[ {"name": "Failed Proxy Server Request", "status": "ERROR", "parent": "Received Proxy Server Request"}, {"name": "Received Proxy Server Request", "status": "ERROR", "parent": "<root>"} ]After:
guardrailspan emitted as a child of the proxy span, carrying status + violated categories.[ {"name": "Failed Proxy Server Request", "status": "ERROR", "parent": "Received Proxy Server Request"}, { "name": "guardrail", "parent": "Received Proxy Server Request", "attributes": { "guardrail_name": "bedrock-policy", "guardrail_mode": "pre_call", "guardrail_status": "guardrail_intervened", "guardrail_action": "GUARDRAIL_INTERVENED", "guardrail_violation_categories": "[\"Fiduciary Advice\", \"VIOLENCE\"]" } }, {"name": "Received Proxy Server Request", "status": "ERROR", "parent": "<root>"} ]Before / after —
_handle_failure(logging-handler path)Before: guardrail span existed but lacked status / action / categories.
{ "name": "guardrail", "attributes": { "guardrail_name": "bedrock-policy", "guardrail_mode": "pre_call", "guardrail_response": "{...full Bedrock JSON, only inspectable by parsing...}" } }After: queryable status + action + categories attributes on the same span.
{ "name": "guardrail", "attributes": { "guardrail_name": "bedrock-policy", "guardrail_mode": "pre_call", "guardrail_response": "{...}", "guardrail_status": "guardrail_intervened", "guardrail_action": "GUARDRAIL_INTERVENED", "guardrail_violation_categories": "[\"Fiduciary Advice\", \"VIOLENCE\"]" } }Test plan
uv run pytest tests/test_litellm/integrations/test_otel_guardrail_violation_spans.py(new file, 10 tests covering: failure-path span creation, post-call-failure-hook span emission, attribute presence for everyGuardrailStatus, category extraction from Bedrock topic/content/word/PII/grounding policies, multi-guardrail flow where only the last one blocks, end-to-end with a realCustomGuardrailsubclass)uv run pytest tests/test_litellm/integrations/test_opentelemetry.py(full existing suite still passes — 219 tests)🤖 Generated with Claude Code