feat(iorails): Telemetry - full non-streaming trace - #1794
Merged
Conversation
tgasser-nv
changed the base branch from
develop
to
feat/iorails-telemetry-initial-e2e
April 15, 2026 21:28
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
Greptile SummaryThis PR instruments the non-streaming
|
| Filename | Overview |
|---|---|
| nemoguardrails/guardrails/telemetry.py | New OTEL instrumentation module with span helpers for request, rail, action, LLM, and API calls; includes graceful no-op paths when OTEL is unavailable. |
| nemoguardrails/guardrails/iorails.py | Adds traced_request wrapper around generate_async; minor redundancy in the local tracer variable (always equals self._tracer). |
| nemoguardrails/guardrails/rails_manager.py | Threads tracer through constructor and wraps _run_rail with rail_span; mark_rail_stop correctly called after action completes. |
| nemoguardrails/guardrails/rail_action.py | Wraps run() with action_span; record_span_error is called in the except block before swallowing the exception, so action spans correctly show ERROR status on failures. |
| nemoguardrails/guardrails/engine_registry.py | Adds tracer param, wraps model_call with llm_call_span and api_call with api_call_span; _get_engine is correctly resolved before llm_call_span so the model name is available for the span name. |
| nemoguardrails/tracing/constants.py | Adds SpanNames, GuardrailsAttributes, OperationNames, and GenAIAttributes constants following OTEL GenAI semantic conventions. |
| tests/guardrails/test_iorails_telemetry.py | Comprehensive integration tests covering span hierarchy, error recording, and concurrent trace isolation; TestOtelNotInstalled directly mutates the tracer singleton rather than using patch.object for exception-safe teardown. |
| tests/guardrails/test_telemetry.py | Unit tests for all telemetry helpers; uses autouse fixture to reset the singleton between tests. |
| tests/guardrails/test_telemetry_spans.py | Unit tests for rail_span, action_span, llm_call_span, and api_call_span; covers attributes, no-op with None tracer, and exception propagation. |
Sequence Diagram
sequenceDiagram
participant Client
participant IORails
participant RailsManager
participant RailAction
participant EngineRegistry
Client->>IORails: generate_async(messages)
IORails->>IORails: traced_request → guardrails.request span (SERVER)
loop Each input rail
IORails->>RailsManager: is_input_safe(messages)
RailsManager->>RailsManager: rail_span → guardrails.rail [Input]
RailsManager->>RailAction: action.run(flow, messages)
RailAction->>RailAction: action_span → guardrails.action
RailAction->>EngineRegistry: model_call / api_call
EngineRegistry->>EngineRegistry: llm_call_span / api_call_span (CLIENT)
EngineRegistry-->>RailAction: response
RailAction-->>RailsManager: RailResult
RailsManager->>RailsManager: mark_rail_stop if blocked
end
IORails->>EngineRegistry: model_call("main", messages)
EngineRegistry->>EngineRegistry: llm_call_span (CLIENT, parent=request)
EngineRegistry-->>IORails: response_text
loop Each output rail
IORails->>RailsManager: is_output_safe(messages, response)
RailsManager->>RailsManager: rail_span → guardrails.rail [Output]
RailsManager->>RailAction: action.run(flow, messages, bot_response)
RailAction->>RailAction: action_span → guardrails.action
RailAction->>EngineRegistry: model_call / api_call
EngineRegistry->>EngineRegistry: llm_call_span / api_call_span (CLIENT)
EngineRegistry-->>RailAction: response
RailAction-->>RailsManager: RailResult
end
IORails-->>Client: {"role": "assistant", "content": ...}
Prompt To Fix All With AI
This is a comment left during a code review.
Path: nemoguardrails/guardrails/iorails.py
Line: 139
Comment:
**Redundant local `tracer` variable**
`self._tracer` is already `None` when tracing is disabled (set in `__init__` as `get_tracer() if self._tracing_enabled else None`), so the local rebinding here always equals `self._tracer`. Simplifying to `with traced_request(self._tracer) as req_id:` removes the indirection without changing behaviour.
```suggestion
with traced_request(self._tracer) as req_id:
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: tests/guardrails/test_iorails_telemetry.py
Line: 645-646
Comment:
**Direct singleton mutation not restored by `patch.object`**
`telemetry._tracer = None` is a bare assignment; if the test fails after line 646 and before the `with patch.object(telemetry, "_OTEL_AVAILABLE", False)` block exits, the singleton remains `None` for subsequent tests in this module. The rest of the file uses `patch.object(telemetry, "_tracer", ...)` which handles teardown atomically. Using the same pattern here makes cleanup exception-safe:
```suggestion
with patch.object(telemetry, "_OTEL_AVAILABLE", False), patch.object(telemetry, "_tracer", None):
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (5): Last reviewed commit: "Add docstring to explain hard-coded chat..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
tgasser-nv
marked this pull request as draft
April 16, 2026 01:36
tgasser-nv
force-pushed
the
feat/iorails-telemetry-full-e2e
branch
from
April 16, 2026 18:47
22964c1 to
11d16e2
Compare
…de and record exception
Collaborator
Author
|
@greptile Review latest commit and update summary and score |
Contributor
📝 WalkthroughWalkthroughOpenTelemetry instrumentation is integrated into the guardrails engine to trace execution flows. New helper functions wrap model calls, API calls, rail actions, and rail execution with telemetry spans. A telemetry module provides span context managers and error recording utilities. Comprehensive tests validate span hierarchy, attributes, and behavior across the instrumented components. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/guardrails/test_iorails_telemetry.py (1)
598-600: Strengthen this assertion to avoid false positives.Line 599 currently passes for any non-empty set, so it won’t catch wrong model attribution. Prefer asserting the expected model (or an explicit allowlist) directly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/guardrails/test_iorails_telemetry.py` around lines 598 - 600, The assertion on models_seen is too weak and can pass for any non-empty set; update the test to assert that the expected model(s) are present explicitly (e.g., require "nvidia/llama-3.1-nemoguard-8b-content-safety" to be in models_seen) or validate models_seen against a small explicit allowlist; locate the models_seen set (built from llm_spans using s.attributes["gen_ai.request.model"]) and replace the current assert with one that checks membership or that models_seen is subset/equal to the expected models collection.nemoguardrails/guardrails/telemetry.py (1)
162-273: Consider consolidating repeated span error handling viarecord_span_error.Lines 185-188, 209-212, 243-247, and 269-273 duplicate the same exception/status pattern. Reusing
record_span_errorin these blocks will keep behavior consistent and reduce drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nemoguardrails/guardrails/telemetry.py` around lines 162 - 273, The except blocks in rail_span, action_span, llm_call_span, and api_call_span duplicate the same exception recording and status-setting logic; replace each duplicated except Exception as exc: block with a single call to the shared helper record_span_error(span, exc) (and then re-raise), making sure record_span_error handles span.record_exception(exc), span.set_status(StatusCode.ERROR, str(exc)) and sets "error.type" where needed for client spans; update the four functions (rail_span, action_span, llm_call_span, api_call_span) to call record_span_error(span, exc) instead of repeating the three lines so behavior is centralized and consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nemoguardrails/guardrails/engine_registry.py`:
- Around line 140-143: The code unconditionally calls get_tracer() and creates
llm_call_span around engine.chat_completion which can emit OTEL spans even when
Guardrails tracing is disabled; update the logic in engine_registry.py to only
call get_tracer() and enter llm_call_span when the runtime flag _tracing_enabled
(the same flag used by IORails) is true, or change the API to accept an optional
tracer passed in from the caller and only use it when provided; apply the same
gated change to the other occurrence around lines 175-179 so tracer lookup and
span creation are skipped when tracing is disabled.
In `@nemoguardrails/guardrails/rails_manager.py`:
- Around line 147-153: The code currently infers direction with "direction =
RailDirection.INPUT if flow in self.input_flows else RailDirection.OUTPUT",
which can misclassify rails present in both lists; change the method to accept
an explicit direction argument from the caller and use that value when calling
rail_span(tracer, flow, direction) instead of checking self.input_flows. Update
the caller sites to pass the correct RailDirection, keep the rest of the logic
unchanged (retain action = self._actions[flow], await action.run(...), and the
span.set_attribute(GuardrailsAttributes.RAIL_STOP, True) handling), and remove
the membership-based inference to prevent incorrect telemetry.
---
Nitpick comments:
In `@nemoguardrails/guardrails/telemetry.py`:
- Around line 162-273: The except blocks in rail_span, action_span,
llm_call_span, and api_call_span duplicate the same exception recording and
status-setting logic; replace each duplicated except Exception as exc: block
with a single call to the shared helper record_span_error(span, exc) (and then
re-raise), making sure record_span_error handles span.record_exception(exc),
span.set_status(StatusCode.ERROR, str(exc)) and sets "error.type" where needed
for client spans; update the four functions (rail_span, action_span,
llm_call_span, api_call_span) to call record_span_error(span, exc) instead of
repeating the three lines so behavior is centralized and consistent.
In `@tests/guardrails/test_iorails_telemetry.py`:
- Around line 598-600: The assertion on models_seen is too weak and can pass for
any non-empty set; update the test to assert that the expected model(s) are
present explicitly (e.g., require "nvidia/llama-3.1-nemoguard-8b-content-safety"
to be in models_seen) or validate models_seen against a small explicit
allowlist; locate the models_seen set (built from llm_spans using
s.attributes["gen_ai.request.model"]) and replace the current assert with one
that checks membership or that models_seen is subset/equal to the expected
models collection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a30f5773-ac43-447f-b00e-b76c86d54935
📒 Files selected for processing (8)
nemoguardrails/guardrails/engine_registry.pynemoguardrails/guardrails/iorails.pynemoguardrails/guardrails/rail_action.pynemoguardrails/guardrails/rails_manager.pynemoguardrails/guardrails/telemetry.pytests/guardrails/test_iorails_telemetry.pytests/guardrails/test_telemetry.pytests/guardrails/test_telemetry_spans.py
…d improve test_no_child_spans_when_tracing_disabled test
… *_span() helpers
…ting in most cases, no need for more docstrings
…gistry to keep all spans consistent under parent ID
Collaborator
Author
|
@CodeRabbit go through all comments, re-evaluate based on latest commit, and resolve if any issue has been fixed |
This comment was marked as resolved.
This comment was marked as resolved.
Collaborator
Author
|
@greptile re-evaluate latest commit and update summary and score |
…ction updating OTEL correctly
This was referenced Apr 17, 2026
Pouyanpi
approved these changes
Apr 20, 2026
Pouyanpi
left a comment
Pouyanpi
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, thanks! please merge after reviewing the comments.
@Pouyanpi note for refactor:
Each helper (
rail_span,action_span,llm_call_span,api_call_span) callsspan.set_attribute(GuardrailsAttributes.RAIL_TYPE, )directly, with the same constants Pydantic models use into_otel_attributes(). This is exactly whattracing/span_attributes.pywas designed to avoid. when the refactor comes, these helpers should delegate toapply_attributes(span, rail_attributes(...)).
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR builds on top of the scaffolding introduced in #1793 to instrument the non-streaming inference path from
generate_async(). It adds spans to reflect work units at different levels of hierarchy:rail_span: Includes the rail type, direction and flow.action_span: Includes action name being executed.llm_call_span: Includes LLM provider, model, and operation.api_call_span: Operation name set to API.As part of the test-plan, I integration-tested IORails calls to
generate_async()for the nemoguards config. See files and test results below.Related Issue(s)
Preceeding PRs in the stack:
Test Plan
Pre-commit
Unit-test
Integration test (uses trace_e2e_test.py)
Attaching logs and JSON files with traces
20260416_otel.log
trace_e2e_test.json
Checklist
Summary by CodeRabbit
Release Notes
New Features
Tests