Skip to content

fix(bedrock): do not report a guardrail activation on every call - #4487

Closed
linhongyu510 wants to merge 1 commit into
traceloop:mainfrom
linhongyu510:fix/bedrock-guardrail-false-activation
Closed

linhongyu510 wants to merge 1 commit into
traceloop:mainfrom
linhongyu510:fix/bedrock-guardrail-false-activation

Conversation

@linhongyu510

@linhongyu510 linhongyu510 commented Sep 21, 2026

Copy link
Copy Markdown

What

is_guardrail_activated ended with:

return response.get("amazon-bedrock-guardrailAction") != "NONE"

When no guardrail is configured, Bedrock omits that key, so .get() returns None, and None != "NONE" is True. Every ordinary response — from users with no guardrail configured at all — was treated as a guardrail activation, and gen_ai.bedrock.guardrail.activation was incremented on every Bedrock call. Fixes #4471.

This bites the converse path in particular: guardrail_converse calls is_guardrail_activated(response) directly, and a normal converse response has no amazon-bedrock-guardrailAction key.

Fix

Two changes in guardrail.py:

  1. Default the key to "NONE" so an absent key is not an activation:

    return response.get("amazon-bedrock-guardrailAction", "NONE") != "NONE"
  2. Guard the two guardrail_activation.add() call sites against None. The issue also notes a crash: when metrics are disabled, _instrument() sets every counter to None, but the two activation call sites dereferenced it unconditionally, so a genuine activation raised AttributeError: 'NoneType' object has no attribute 'add'. The guard skips the counter while still setting the span attributes (span attributes are independent of the metrics toggle).

+5/-3 in one source file.

Reproduction (offline, no AWS)

Detection, before the fix:

is_guardrail_activated({"stopReason": "end_turn"})            -> True   (should be False)
is_guardrail_activated({})                                    -> True   (should be False)
is_guardrail_activated({"amazon-bedrock-guardrailAction":"NONE"}) -> False  (ok)
is_guardrail_activated({"stopReason":"guardrail_intervened"}) -> True   (ok, real activation)

Metrics-disabled crash, before the fix:

metric_params.guardrail_activation = None
guardrail_converse(span, {"stopReason": "guardrail_intervened"}, ...)
-> AttributeError: 'NoneType' object has no attribute 'add'

Both are resolved after the change, and real activations (GUARDRAIL_INTERVENED, stopReason == "guardrail_intervened", completionReason == "CONTENT_FILTERED") are still detected.

Tests

Added tests/test_guardrail_activation.py (pure unit, no cassette / no AWS):

test_is_guardrail_activated[...]                                7 cases PASSED
test_converse_without_guardrail_does_not_record_activation      PASSED
test_converse_with_activation_records_once                      PASSED
test_activation_with_metrics_disabled_does_not_crash            PASSED

Load-bearing, verified by reverting each half:

  • reverting the "NONE" default fails the "no key → False" cases and test_converse_without_guardrail_does_not_record_activation;
  • reverting the None guard fails test_activation_with_metrics_disabled_does_not_crash with the exact AttributeError from the issue.

ruff check clean on both files (repo config).

Summary by CodeRabbit

  • Bug Fixes

    • Fixed Bedrock guardrail detection so responses without a guardrail action are not incorrectly reported as activated.
    • Prevented errors when guardrail activation metrics are unavailable or disabled.
    • Ensured activation metrics are recorded only for genuine guardrail interventions.
  • Tests

    • Added regression coverage for absent, inactive, and active guardrail responses, including scenarios with metrics disabled.

is_guardrail_activated ended with
    response.get("amazon-bedrock-guardrailAction") != "NONE"
When no guardrail is configured Bedrock omits that key, so .get() returns
None and None != "NONE" is True. Every ordinary response was counted as a
guardrail activation and gen_ai.bedrock.guardrail.activation was
incremented on every Bedrock call. Default the key to "NONE".

Also guard the two guardrail_activation.add() call sites: when metrics are
disabled the SDK sets the counter to None, and a genuine activation raised
AttributeError. The span attributes are still set in that case.

Fixes traceloop#4471
@coderabbitai

coderabbitai Bot commented Sep 21, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Understand this PR’s impact

Explore downstream dependencies and potential security impact with Blast Radius.

View blast radius →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 34f38556-9508-40e6-876f-17a8778b9f43

📥 Commits

Reviewing files that changed from the base of the PR and between dac2534 and f5c2aa9.

📒 Files selected for processing (2)
  • packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/guardrail.py
  • packages/opentelemetry-instrumentation-bedrock/tests/test_guardrail_activation.py

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The Bedrock instrumentation now treats missing guardrail actions as inactive. Guardrail metric updates are skipped when metrics are disabled. Regression tests cover inactive responses, activations, metric counts, and disabled metrics.

Changes

Bedrock guardrail handling

Layer / File(s) Summary
Guardrail activation detection
packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/guardrail.py, packages/opentelemetry-instrumentation-bedrock/tests/test_guardrail_activation.py
Missing amazon-bedrock-guardrailAction values default to "NONE". Tests cover inactive, intervened, filtered, and non-filtered responses.
Guardrail activation metric handling
packages/opentelemetry-instrumentation-bedrock/opentelemetry/instrumentation/bedrock/guardrail.py, packages/opentelemetry-instrumentation-bedrock/tests/test_guardrail_activation.py
Metric recording now checks for None before calling add. Tests cover ordinary responses, one activation event, and disabled metrics.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: preventing Bedrock guardrail activation metrics from being reported on every call.
Linked Issues check ✅ Passed Issue [#4471] requires absent amazon-bedrock-guardrailAction to mean no activation and requires genuine activations to avoid an AttributeError when metrics are disabled. is_guardrail_activated d…
Out of Scope Changes check ✅ Passed The changed production code directly fixes the activation detection and disabled-metric failure in [#4471]. The added unit tests verify those behaviors. No unrelated changes are shown in the reviewed …
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Warning

⚠️ This pull request shows signs of AI-generated slop (trivial_assertion). It has been flagged by CodeRabbit slop detection and should be reviewed carefully.


Comment @coderabbitai help to get the list of available commands.

@linhongyu510

Copy link
Copy Markdown
Author

Closing as a duplicate — I opened this without checking for existing PRs first. #4475 (2026-09-16) already fixes the false-positive activation with the same one-line default.

It doesn't yet cover the second half of #4471 (the guardrail_activation.add crash when metrics are disabled), so I've left the details and a minimal guard on #4475 so it can be folded in there and #4471 fully closed. Closing this in favour of #4475.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛 Bug Report: gen_ai.bedrock.guardrail.activation counts every Bedrock call

1 participant