feat(observability): add GenAI semantic attributes - #224
Conversation
WalkthroughThe PR adds W3C trace propagation for server requests and expands tracing across routing, LLM requests, responses, token usage, errors, and streaming telemetry. Tests cover standardized span attributes and deferred stream observation. ChangesObservability instrumentation
Estimated code review effort: 4 (Complex) | ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
crates/libsy/src/observability.rs (2)
198-218: 🚀 Performance & Scalability | 🔵 TrivialStreaming responses keep the
libsy.client_callspan open until the stream finishes.
observe_client_streamretains thelibsy.client_callspan for the lifetime of the returned stream. Until the stream is fully consumed or dropped, the span does not close or export. For a slow consumer, the recorded span duration includes the full stream-consumption time, and the trace does not appear in the backend until then. Confirm this is the intended tradeoff for any alerting or dashboarding that keys onlibsy.client_callduration.🤖 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 `@crates/libsy/src/observability.rs` around lines 198 - 218, Update observe_client_stream and the surrounding libsy.client_call instrumentation to ensure the client-call span closes when the provider response is received, rather than retaining it until the returned stream is consumed or dropped. Preserve the existing chunk-level attribute and error recording while separating stream observation from the span’s lifetime so duration and export reflect the client call itself.
220-250: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the cache-token summation rationale.
record_gen_ai_usagecomputesgen_ai.usage.input_tokensasinput_tokens + cache_read + cache_creation. This matches the OTel GenAI semantic convention for providers whose nativeinput_tokensexcludes cache tokens, but the rationale is not obvious from the code. Add a short comment stating that this follows the GenAI semconv computation, to prevent a future change from "simplifying" this to justusage.input_tokens(a real regression of this exact kind has been reported against other OTel GenAI instrumentations).As per coding guidelines, non-obvious private helper logic should carry an explanatory comment: "document module/file intent, public structs and enums, public methods, non-obvious private helpers... use... block comments before complex validation... logic."
🤖 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 `@crates/libsy/src/observability.rs` around lines 220 - 250, The non-obvious input-token aggregation in record_gen_ai_usage needs documentation. Add a short explanatory comment immediately before the gen_ai.usage.input_tokens calculation stating that summing native input, cache-read, and cache-creation tokens follows the OTel GenAI semantic convention; leave the existing computation unchanged.Source: Coding guidelines
crates/libsy-llm-client/src/client.rs (1)
181-181: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the implicit span contract and add test coverage for
record_gen_ai_request.
record_gen_ai_requestrecords fields ontotracing::Span::current(). This assumes the caller is still inside thelibsy.client_callspan created inlibsy/src/core/algorithm.rs. No function between that span's creation and this call carries its own#[instrument], so it works today, but the dependency is implicit across two crates. If a future change adds an intermediate span, this function will silently record onto the wrong span with no compile or runtime error, sincetracingdrops unrecognized fields.Add a short comment stating this assumption, and add a unit test in this file's test module that enters a test span, calls
record_gen_ai_requestdirectly, and assertsgen_ai.provider.name,gen_ai.request.model,gen_ai.request.stream,server.address, andserver.portare recorded as expected — the existingbackend.rstests and the switchyard-serverrequest_spantest both show the pattern for asserting recorded span fields without a real HTTP call.As per coding guidelines: "document... non-obvious private helpers... use
///doc comments for public items and block comments before complex... async... logic."Also applies to: 605-618
🤖 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 `@crates/libsy-llm-client/src/client.rs` at line 181, Document in record_gen_ai_request that it records fields onto the caller’s existing libsy.client_call span and must be invoked within that span context. Add a unit test in this file’s test module that enters a test span, calls record_gen_ai_request directly, and verifies gen_ai.provider.name, gen_ai.request.model, gen_ai.request.stream, server.address, and server.port using the existing span-field assertion pattern, without making an HTTP request.Source: Coding guidelines
🤖 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 `@crates/libsy-llm-client/src/client.rs`:
- Line 181: Document in record_gen_ai_request that it records fields onto the
caller’s existing libsy.client_call span and must be invoked within that span
context. Add a unit test in this file’s test module that enters a test span,
calls record_gen_ai_request directly, and verifies gen_ai.provider.name,
gen_ai.request.model, gen_ai.request.stream, server.address, and server.port
using the existing span-field assertion pattern, without making an HTTP request.
In `@crates/libsy/src/observability.rs`:
- Around line 198-218: Update observe_client_stream and the surrounding
libsy.client_call instrumentation to ensure the client-call span closes when the
provider response is received, rather than retaining it until the returned
stream is consumed or dropped. Preserve the existing chunk-level attribute and
error recording while separating stream observation from the span’s lifetime so
duration and export reflect the client call itself.
- Around line 220-250: The non-obvious input-token aggregation in
record_gen_ai_usage needs documentation. Add a short explanatory comment
immediately before the gen_ai.usage.input_tokens calculation stating that
summing native input, cache-read, and cache-creation tokens follows the OTel
GenAI semantic convention; leave the existing computation unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 52ad2506-20ea-41b9-94c9-53b46a3b2c46
📒 Files selected for processing (7)
crates/libsy-llm-client/src/backend.rscrates/libsy-llm-client/src/client.rscrates/libsy/src/core/algorithm.rscrates/libsy/src/observability.rscrates/libsy/tests/observability.rscrates/switchyard-server/src/lib.rscrates/switchyard-server/src/observability.rs
Signed-off-by: nachiketb <nachiketb@nvidia.com>
1a7672c to
370ffc3
Compare
Signed-off-by: nachiketb <nachiketb@nvidia.com>
Signed-off-by: nachiketb <nachiketb@nvidia.com>
What
Add OpenTelemetry GenAI semantic attributes to the existing Rust server and libsy spans while preserving the current telemetry surface.
chat <model>and mark them as client spans.traceparentandtracestateat the Rust server boundary.gen_ai.provider.nameremains unset because the current configuration does not identify the provider authoritatively. Wire format, hostname, and model name are not reliable provider identities.Why
OTLP consumers need standard model-call data to reconstruct per-trace and per-session usage without an Intake-specific Rust sink. The existing spans already represent the correct request and model-call boundaries; this adds the portable GenAI projection and inbound trace continuity.
How
libsy.runandlibsy.client_callinstead of creating duplicate GenAI spans.switchyard-llm-clientadd the resolved upstream server address and port.No prompt, completion, raw body, API key, or header content is recorded. Existing metrics, logs, and HTTP endpoints remain unchanged.
What to review
Validation
cargo fmt --all -- --check cargo clippy -p switchyard-libsy -p switchyard-llm-client --all-targets -- -D warnings cargo test -p switchyard-libsyRelated: SWITCH-1096
Follow-up: SWITCH-1142