feat(otel): migrate to GenAI semantic conventions v0.31#102
Open
doronkopit5 wants to merge 1 commit into
Open
Conversation
- Upgrade opentelemetry-semantic-conventions 0.27 → 0.31 - Replace gen_ai.system with gen_ai.provider.name using well-known values (openai, azure.ai.openai, anthropic, aws.bedrock, gcp.vertex_ai) - Serialize gen_ai.input.messages and gen_ai.output.messages as JSON with role/parts structure (text, tool_call, tool_call_response) - Add top-level gen_ai.response.finish_reasons array with normalized values (tool_calls/function_call/tool_use → tool_call; end_turn/stop_sequence → stop; max_tokens → length) - Extract record_span helpers for chat and completion message building - Add ~20 unit + streaming tests covering helpers and accumulator path Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR migrates OpenTelemetry span recording to semantic conventions 0.31.0, refactoring chat/completion/embeddings span attributes to emit aggregated JSON for input/output messages and standardized finish reasons, while standardizing vendor names and adding comprehensive test coverage for the new behavior. ChangesOpenTelemetry Semantic Conventions Migration
🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
🔒 Container Vulnerability Scan (hub-migrations - amd64)Click to expand results |
Contributor
🔒 Container Vulnerability Scan (hub-migrations - arm64)Click to expand results |
![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
🤖 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.
Inline comments:
In `@src/pipelines/otel.rs`:
- Around line 762-772: The tests are failing rustfmt checks; run cargo fmt and
fix formatting in the test module so statements like "let v: Value = ..." and
multi-line function calls are properly formatted; specifically reformat the
test_top_level_finish_reasons function and other test helpers referenced around
the affected symbols (e.g., top_level_finish_reasons and any Value
deserializations/assertions) so they conform to rustfmt style and then re-run
CI.
- Line 227: Reformat the single-line conditional that returns an Option from the
local variable mapped so it satisfies rustfmt: replace the inline form "if
mapped.is_empty() { None } else { Some(mapped) }" with a multi-line if/else
block where the if condition is on its own line and each branch (returning None
and Some(mapped)) is on its own line; locate the expression using the identifier
mapped in src/pipelines/otel.rs and update it accordingly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 791eaa3f-30b9-42f8-9daa-d46292767e6a
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
Cargo.tomldocs/superpowers/specs/2026-05-27-otel-messages-finish-reasons-tests-design.mdsrc/pipelines/otel.rssrc/pipelines/pipeline.rssrc/providers/provider.rs
| .map(map_finish_reason) | ||
| .filter(|s| !s.is_empty()) | ||
| .collect(); | ||
| if mapped.is_empty() { None } else { Some(mapped) } |
Contributor
There was a problem hiding this comment.
Fix formatting to pass CI.
Pipeline failure indicates cargo fmt wants this conditional reformatted to multiple lines.
Suggested fix
- if mapped.is_empty() { None } else { Some(mapped) }
+ if mapped.is_empty() {
+ None
+ } else {
+ Some(mapped)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if mapped.is_empty() { None } else { Some(mapped) } | |
| if mapped.is_empty() { | |
| None | |
| } else { | |
| Some(mapped) | |
| } |
🤖 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 `@src/pipelines/otel.rs` at line 227, Reformat the single-line conditional that
returns an Option from the local variable mapped so it satisfies rustfmt:
replace the inline form "if mapped.is_empty() { None } else { Some(mapped) }"
with a multi-line if/else block where the if condition is on its own line and
each branch (returning None and Some(mapped)) is on its own line; locate the
expression using the identifier mapped in src/pipelines/otel.rs and update it
accordingly.
Comment on lines
+762
to
+772
|
|
||
| #[test] | ||
| fn test_top_level_finish_reasons_all_mapped() { | ||
| let out = top_level_finish_reasons(vec![ | ||
| Some("stop"), | ||
| Some("max_tokens"), | ||
| Some("tool_calls"), | ||
| ]) | ||
| .unwrap(); | ||
| assert_eq!(out, vec!["stop", "length", "tool_call"]); | ||
| } |
Contributor
There was a problem hiding this comment.
Fix formatting in tests to pass CI.
The pipeline failures indicate several let v: Value = ... statements and function calls need reformatting per rustfmt. Run cargo fmt to fix all formatting issues in the test module. Affected lines include 762, 1028, 1041, 1073, 1136, 1157, and 1186.
🧰 Tools
🪛 GitHub Actions: build / 0_build.txt
[error] 762-762: cargo fmt --check failed (rustfmt formatting diff in test test_top_level_finish_reasons_all_mapped). Run cargo fmt to reformat.
🪛 GitHub Actions: build / build
[error] 762-762: cargo fmt --check failed: formatting diff in test test_top_level_finish_reasons_all_mapped (rustfmt reflowed top_level_finish_reasons(vec![...]) call).
🤖 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 `@src/pipelines/otel.rs` around lines 762 - 772, The tests are failing rustfmt
checks; run cargo fmt and fix formatting in the test module so statements like
"let v: Value = ..." and multi-line function calls are properly formatted;
specifically reformat the test_top_level_finish_reasons function and other test
helpers referenced around the affected symbols (e.g., top_level_finish_reasons
and any Value deserializations/assertions) so they conform to rustfmt style and
then re-run CI.
Contributor
🔒 Container Vulnerability Scan (hub - amd64)Click to expand results |
Contributor
🔒 Container Vulnerability Scan (hub - arm64)Click to expand results |
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
1 participant
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.
Summary
opentelemetry-semantic-conventionsfrom 0.27 → 0.31 and migratessrc/pipelines/otel.rsto the OTel GenAI spec:gen_ai.provider.namewith well-known values (openai,anthropic,azure.ai.openai,aws.bedrock,gcp.vertex_ai), JSON-serializedgen_ai.input.messages/gen_ai.output.messages, and a top-levelgen_ai.response.finish_reasonsarray with normalized values.RecordSpanimpls and adds ~20 new unit and streaming-integration tests (34 total inpipelines::otel::tests).provider.rsand exercised by the existingpipeline.rsintegration test.Test Plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores