feat(llm): add shared model telemetry and instrumented decorator - #2214
feat(llm): add shared model telemetry and instrumented decorator#2214Pouyanpi wants to merge 3 commits into
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThe change centralizes LLM telemetry in a new reusable instrumented model wrapper, adds dedicated LLM telemetry helpers, integrates wrappers into ChangesLLM instrumentation and telemetry
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant EngineRegistry
participant InstrumentedLLMModel
participant ModelEngine
participant LLMTelemetry
Caller->>EngineRegistry: model_call or stream_model_call
EngineRegistry->>InstrumentedLLMModel: generate_async or stream_async
InstrumentedLLMModel->>LLMTelemetry: start span and record request data
InstrumentedLLMModel->>ModelEngine: execute model request
ModelEngine-->>InstrumentedLLMModel: response or streamed chunks
InstrumentedLLMModel->>LLMTelemetry: record response data, content, and metrics
InstrumentedLLMModel-->>Caller: response or streamed chunks
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR extracts LLM-call OpenTelemetry instrumentation from
|
| Filename | Overview |
|---|---|
| nemoguardrails/llm/models/instrumented.py | New InstrumentedLLMModel decorator with correct stream lifecycle, idempotency guard with warnings, and clean delegation to the wrapped model. No issues found. |
| nemoguardrails/llm/telemetry.py | New engine-neutral GenAI telemetry helpers migrated from guardrails/telemetry.py with added suppress(Exception) resilience guards. |
| nemoguardrails/guardrails/engine_registry.py | Commit 2 (temporary): replaces inline LLM telemetry in _run_model_call/_run_model_stream with InstrumentedLLMModel delegation via _ModelEngineAdapter. Not for merge. |
| nemoguardrails/guardrails/telemetry.py | Removes LLM-span helpers now living in llm/telemetry.py; imports record_span_error from there. Clean refactor. |
| tests/llm/models/test_instrumented.py | Comprehensive test suite covering delegation, error identity, stream lifecycle (natural exhaustion, consumer close, cancellation), content capture, metrics, and idempotency. Well-structured. |
Sequence Diagram
sequenceDiagram
participant Caller
participant ILM as InstrumentedLLMModel
participant Span as llm_call_span
participant Dur as llm_operation_duration
participant Model as Wrapped LLMModel
participant Metrics as OTEL Metrics
Caller->>ILM: "generate_async(prompt, **kwargs)"
ILM->>Span: open CLIENT span
ILM->>Span: set_llm_request_attributes
ILM->>Dur: start duration timer
ILM->>Model: "generate_async(prompt, stop, **kwargs)"
Model-->>ILM: LLMResponse
ILM->>Dur: record duration (stop timer)
ILM->>Span: set_llm_response_attributes
ILM->>Span: set_llm_call_content (if capture enabled)
ILM->>Span: close span
ILM->>Metrics: record_token_usage
ILM-->>Caller: LLMResponse
Note over ILM,Span: On error: record_span_error re-raise
Caller->>ILM: "stream_async(prompt, **kwargs)"
ILM->>Span: open CLIENT span
ILM->>Span: "set_llm_request_attributes (stream=True)"
ILM->>Dur: start duration timer
loop each chunk
ILM->>Model: yield chunk
Model-->>ILM: LLMResponseChunk
ILM->>Metrics: record_time_to_first_chunk / per_chunk
ILM-->>Caller: yield chunk
end
ILM->>Dur: record duration (stop timer)
ILM->>Model: aclose() [finally]
ILM->>Span: set_llm_response_attributes
ILM->>Span: set_llm_call_content (if capture enabled)
ILM->>Span: close span
ILM->>Metrics: record_token_usage
Reviews (4): Last reviewed commit: "docs(llm): document model telemetry surf..." | Re-trigger Greptile
There was a problem hiding this comment.
🧹 Nitpick comments (1)
nemoguardrails/llm/models/instrumented.py (1)
177-192: 🩺 Stability & Availability | 🔵 TrivialConfirm marking consumer cancellation / early close as span
ERRORis intended.When the consumer closes the stream early (
GeneratorExit) or the task is cancelled (CancelledError), the exception propagates throughllm_call_span, which setserror.typeandStatusCode.ERRORon the CLIENT span (as the tests assert). This conflates normal client-initiated cancellation with genuine provider failures and can inflate error rates on dashboards/alerts derived from span status. If that's not desired, consider treatingGeneratorExit/CancelledErroras non-error terminations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nemoguardrails/llm/models/instrumented.py` around lines 177 - 192, Update the streaming cleanup/error handling around llm_call_span so consumer-initiated GeneratorExit and CancelledError terminations are treated as normal cancellations rather than marking the CLIENT span with error.type or StatusCode.ERROR. Preserve error recording for genuine provider failures and retain the existing stream close and token-usage behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@nemoguardrails/llm/models/instrumented.py`:
- Around line 177-192: Update the streaming cleanup/error handling around
llm_call_span so consumer-initiated GeneratorExit and CancelledError
terminations are treated as normal cancellations rather than marking the CLIENT
span with error.type or StatusCode.ERROR. Preserve error recording for genuine
provider failures and retain the existing stream close and token-usage behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b3e7b59e-0716-48f2-a40b-d464477f10c6
📒 Files selected for processing (9)
nemoguardrails/__init__.pynemoguardrails/guardrails/engine_registry.pynemoguardrails/guardrails/telemetry.pynemoguardrails/llm/models/instrumented.pynemoguardrails/llm/telemetry.pynemoguardrails/tracing/constants.pytests/guardrails/test_telemetry_content_capture.pytests/guardrails/test_telemetry_spans.pytests/llm/models/test_instrumented.py
Add nemoguardrails/llm/telemetry.py with engine-neutral OpenTelemetry GenAI helpers (span lifecycle, request/response attributes, content capture) and InstrumentedLLMModel in nemoguardrails/llm/models/instrumented.py, a transparent wrapper that emits CLIENT spans and GenAI client metrics around any LLMModel. Export InstrumentedLLMModel from the package root.
…corator Replace the inline instrumentation loop in the IORails engine registry with instrument_llm_model, and drop the model-call helpers from guardrails/telemetry.py (re-exporting record_span_error from the shared module for existing importers). De-scope the metric-name comments from IORails to Guardrails engines and retarget the telemetry tests at the engine-neutral helpers.
da1db40 to
71ca58d
Compare
Signed-off-by: Pouyanpi <13303554+Pouyanpi@users.noreply.github.com>
Description
Adds shared, engine-neutral OpenTelemetry GenAI instrumentation for model calls.
Commit 1 (the deliverable):
feat(llm): add shared model telemetry and instrumented decorator2b680f0nemoguardrails/llm/telemetry.py: engine-neutral helpers for GenAI CLIENT spans and request/response/usage attributes.InstrumentedLLMModel: a transparent decorator wrapping anyLLMModelto emit GenAI spans and client metrics. No-op when tracing and metrics are both off.Commit 2 (temporary):
refactor(guardrails): route IORails model calls through the shared decoratorRelated Issue(s)
Verification
gpt-4o-mini): unchanged spans plus bothguardrails.*andgen_ai.client.*metrics. Examples were run against every IORails telemetry doc:docs/observability/metrics/:enable-metrics.mdx,index.mdx,opentelemetry-integration.mdx,reference.mdxdocs/observability/tracing/:content-capture.mdx,span-reference.mdx,index.mdxAI Assistance
Checklist