feat(server): canonical gen_ai.*/mcp.* attributes on MCP tool spans (RFC0038.7) - #623
Conversation
…RFC0038.7)
The MCP `execute_tool` spans now carry the canonical OTel GenAI/MCP
semantic-convention attributes — `gen_ai.operation.name = execute_tool`,
`gen_ai.tool.name`, `mcp.method.name = tools/call`, and `mcp.session.id`
(recorded from the forwarded `mcp-session-id` header) — so Ourios's
agent-facing tool calls interoperate with GenAI-aware backends and an
agent's calls within a session correlate.
These attributes moved out of core semantic-conventions to the separate
semantic-conventions-genai repo; in our pinned semconv v1.42.0 dependency
they survive only as deprecated "Moved to …" stubs, which `weaver
registry live-check` reports as violations. weaver cannot take a second
registry dependency ("Multiple dependencies is not supported yet"), and
v1.42.0 still ships the gen-ai/mcp model, so a second dependency would
also collide on group ids. The live-check job therefore gates on a
filtered violation count that exempts only the genai-relocation
deprecation for the gen_ai.*/mcp.* namespaces; every other violation
(including any other deprecation on those keys) still fails. #622 tracks
collapsing this into a single genai dependency once upstream deletes its
v1.42 copies.
Driving an MCP tools/call through live-check also surfaces rmcp's own
internal instrumentation (bare session_id/peer_info/notification fields
on its events) — non-semconv transport noise, not Ourios signal. That is
muted at the source in `ourios-telemetry`'s guarded_env_filter
(`rmcp=off`), alongside the export-stack loop guard; the `execute_tool`
span (target `ourios_server::mcp`) is unaffected.
Verified end-to-end locally: real server + weaver live-check with an MCP
call driven → non-exempt violations = 0.
Refs #622.
Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughMCP tool tracing now records session identifiers and canonical GenAI/MCP attributes. The RFC and integration test define the span contract, while CI exercises MCP calls and validates Weaver’s JSON report with a narrow relocation-deprecation exemption. ChangesMCP telemetry validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CI
participant MCPClient
participant ourios-server
participant Weaver
CI->>ourios-server: Start with MCP enabled
MCPClient->>ourios-server: Initialize session and call list_templates
ourios-server-->>Weaver: Emit MCP execute-tool telemetry
Weaver-->>CI: Write live-check JSON report
CI->>CI: Filter sanctioned advisory and evaluate violations
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
393-465: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake MCP emission a required live-check assertion.
The
|| truefallbacks let the MCP handshake ortools/callfail while the HTTP query still supplies telemetry entities. With no emitted GenAI/MCP attributes,unexpected=0and CI passes—so this gate does not enforce RFC0038.7. Fail on a missing session or either MCP request failure, and require the report to contain the sanctioned relocation findings for all four expected attributes before excluding them.🤖 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 @.github/workflows/ci.yml around lines 393 - 465, Make the MCP live-check mandatory by removing the best-effort fallbacks around mcp_session creation, notifications/initialized, and tools/call, failing when the session is missing or either request fails. Extend report validation before calculating unexpected so the Weaver report contains the sanctioned relocation findings for all four expected gen_ai/mcp attributes; fail if any are absent. Preserve the existing exemption only after this completeness check.
🤖 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.
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 393-465: Make the MCP live-check mandatory by removing the
best-effort fallbacks around mcp_session creation, notifications/initialized,
and tools/call, failing when the session is missing or either request fails.
Extend report validation before calculating unexpected so the Weaver report
contains the sanctioned relocation findings for all four expected gen_ai/mcp
attributes; fail if any are absent. Preserve the existing exemption only after
this completeness check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 86c35577-a607-46c1-92aa-312209a44436
📒 Files selected for processing (5)
.github/workflows/ci.ymlcrates/ourios-server/src/mcp.rscrates/ourios-server/tests/rfc0038_1_mcp_span.rscrates/ourios-telemetry/src/lib.rsdocs/rfcs/0038-self-tracing.md
There was a problem hiding this comment.
Pull request overview
Updates Ourios’s MCP tool-execution tracing to emit the canonical OpenTelemetry GenAI/MCP semantic-convention attributes (RFC0038.7), and adjusts the semconv live-check CI gate to narrowly exempt the upstream “moved to semantic-conventions-genai” deprecations while still failing on any other drift.
Changes:
- Add
gen_ai.*andmcp.*attributes (includingmcp.session.idfrommcp-session-id) toexecute_tool {tool}spans for all MCP tools. - Silence
rmcp’s internal, non-semconv instrumentation from exported telemetry viaguarded_env_filter. - Extend the CI live-check job to enable MCP, drive a
tools/call, and gate on a filtered violation count that only exempts the GenAI relocation deprecations.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| docs/rfcs/0038-self-tracing.md | Documents RFC0038.7: canonical GenAI/MCP attributes on tool spans and the narrow live-check exemption. |
| crates/ourios-telemetry/src/lib.rs | Extends the exported-telemetry filter to mute rmcp internal instrumentation noise. |
| crates/ourios-server/tests/rfc0038_1_mcp_span.rs | Asserts the four GenAI/MCP span attributes (including session id) on MCP tool spans. |
| crates/ourios-server/src/mcp.rs | Emits canonical gen_ai.*/mcp.* attributes on MCP tool spans and records mcp.session.id from request headers. |
| .github/workflows/ci.yml | Enables MCP in live-check, drives an MCP tools/call, and gates on filtered violation count exempting only the relocation deprecations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The MCP handshake driving the `execute_tool` span is best-effort (its curls are `|| true`), so a failed `tools/call` could let the live-check gate pass while never exercising the gen_ai.*/mcp.* attribute + exemption path — a silent false negative (Copilot, #623). Confirm the outcome instead of the curl exit: after weaver writes its report, check whether the `execute_tool` span actually reached it (`gen_ai.operation.name` present) and emit a `::warning::` if not. This catches every miss (curl hiccup, session failure, span not exported), not just a non-zero curl exit. Kept a warning, not a hard failure: a missed handshake is a coverage gap for that run (the HTTP path still validates the rest), and failing on a transient handshake would be flaky. Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
… test
RFC0038.7's §3.6 said the span name is "the derived {gen_ai.operation.name}
{gen_ai.tool.name} pair, not a duplicate source of truth", but
`#[tracing::instrument]` requires a static name literal, so the name and
the two attributes are in fact written separately per tool (Copilot,
#623). Acknowledge that: the MCP-span unit test asserts both the name and
the attribute values, so a drift between them fails the test.
Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
The exempted gen_ai.*/mcp.* violations make weaver exit non-zero on every clean run, so status alone is no signal — but a non-zero status with zero violations in the report means weaver failed for a non-policy reason (internal error) after writing a report, which the filtered gate would otherwise mask (Copilot, #623). Warn on exactly that case. Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
What
RFC0038.7 — the MCP
execute_tooltool spans now carry the canonical OTelGenAI/MCP semantic-convention attributes, the agent-observability payload:
gen_ai.operation.nameexecute_toolgen_ai.tool.namequery_logs/list_templates/template_drift)mcp.method.nametools/callmcp.session.idmcp-session-idheaderSo Ourios's agent-facing tool calls interoperate with GenAI-aware backends, and
an agent's calls within one session correlate — the agent-observability thesis.
Why it isn't a one-line registry dependency
These attributes moved out of core semantic-conventions to the separate
semantic-conventions-genairepo. In our pinned dependency (semconv v1.42.0) they survive only as
deprecated "Moved to …" stubs, which
weaver registry live-checkreports ashard
violations. Two facts (both verified with the local weaver 0.23.0) ruleout simply pinning the genai registry:
dependenciesentries panics:not yet implemented: Multiple dependencies is not supported yet.model/gen-ai/+model/mcp/, so a seconddependency would define the same group ids twice (the genai repo's own
manifest works around this only via a build-time filtered copy of upstream).
Approach: emit + narrowly exempt
The
live-checkCI job now gates on a filtered violation count that exemptsonly the genai-relocation
deprecatedadvisory for thegen_ai.*/mcp.*namespaces — every other violation (including any other deprecation on those
keys, or any non-conformant attribute) still fails the gate. #622 tracks
collapsing this into a single genai dependency once upstream deletes its v1.42
copies (then it's a one-line swap and the exemption is deleted).
Driving an MCP
tools/callthrough live-check (the job now enables MCP andperforms a handshake) also surfaces
rmcp's own internal instrumentation —bare
session_id/peer_info/notificationfields on its events. Thatnon-semconv transport noise is muted at the source in
ourios-telemetry'sguarded_env_filter(rmcp=off), alongside the export-stack loop guard; ourown
execute_toolspan (targetourios_server::mcp) is unaffected.§3 invariant / hazard note
No pillar or on-disk invariant touched. This is the OTel traces signal (RFC
0038) gaining span attributes. Per the project's OTel-alignment rule, every name
here was confirmed against the OpenTelemetry MCP: the
gen_ai.*/mcp.*keys arethe canonical (relocated) OTel names, and
execute_toolis the GenAIwell-known
gen_ai.operation.namevalue; no collision with a differently-meantsemconv name.
Verification
cargo fmt --all --check,cargo clippy --all-targets --all-features -D warnings,cargo test -p ourios-server -p ourios-telemetry,mdbook build—all green.
ourios-server+weaver registry live-checkwith an MCPtools/calldriven → theexecute_toolspancarries all four attributes and non-exempt violations = 0 (the only
remaining 4 are the sanctioned gen_ai/mcp "moved" deprecations the gate
exempts).
rfc0038_1_mcp_spanasserts the four attributes (incl. the sessionid) on the emitted span.
Refs #622.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation