Skip to content

fix(tracing): semconv v1.27.0→v1.34.0, fix mcp.tool_call attribute misuse, add server.address#6683

Merged
lpcox merged 5 commits into
mainfrom
copilot/go-fan-go-module-review
May 29, 2026
Merged

fix(tracing): semconv v1.27.0→v1.34.0, fix mcp.tool_call attribute misuse, add server.address#6683
lpcox merged 5 commits into
mainfrom
copilot/go-fan-go-module-review

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 29, 2026

Addresses several OTel correctness and best-practice issues identified in the go-fan module review: a semantic attribute mismatch on an internal span, manual gen_ai key definitions that duplicated official semconv, and minor span hygiene gaps.

Semconv upgrade (v1.27.0v1.34.0)

Updated the import alias in all four files that reference semconv. v1.34.0 is bundled with the already-pinned go.opentelemetry.io/otel@v1.43.0 — no module version bump required.

genai_attrs.go: replace manual constants with official semconv aliases

The five gen_ai.* keys were hand-rolled strings. They're now aliases to the official semconv/v1.34.0 constants, eliminating drift risk. Call sites are unchanged — the alias type is identical (attribute.Key).

// Before
GenAIToolName = attribute.Key("gen_ai.tool.name")

// After
GenAIToolName = semconv.GenAIToolNameKey  // "gen_ai.tool.name" — official semconv/v1.34.0

Added MCPResponseStatus = attribute.Key("mcp.response.status") for the conceptual HTTP status on internal MCP spans.

unified.go: fix semantic mismatch on mcp.tool_call span

mcp.tool_call is SpanKindInternal — setting http.response.status_code on it violates the OTel spec (that attribute is reserved for HTTP client/server spans). Replaced with the new mcp.response.status key.

// Before — semantically wrong on an internal span
toolSpan.SetAttributes(semconv.HTTPResponseStatusCodeKey.Int(httpStatusCode))

// After
toolSpan.SetAttributes(tracing.MCPResponseStatus.Int(httpStatusCode))

proxy/handler.go: span hygiene

  • Added semconv.ServerAddressKey (upstream hostname) to the proxy.backend.forward span for upstream visibility in trace backends. Extracted via a new Server.upstreamHost() helper.
  • Switched fwdSpan.End() to defer fwdSpan.End() to prevent span leaks if early-return paths are introduced by future refactors.

Copilot AI linked an issue May 29, 2026 that may be closed by this pull request
5 tasks
…ibute, add server.address, defer fwdSpan.End()
Copilot AI changed the title [WIP] Review Go module go.opentelemetry.io/otel integration fix(tracing): semconv v1.27.0→v1.34.0, fix mcp.tool_call attribute misuse, add server.address May 29, 2026
Copilot finished work on behalf of lpcox May 29, 2026 12:51
Copilot AI requested a review from lpcox May 29, 2026 12:51
@lpcox lpcox marked this pull request as ready for review May 29, 2026 12:52
Copilot AI review requested due to automatic review settings May 29, 2026 12:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the gateway’s OpenTelemetry instrumentation to align with newer semantic conventions (semconv v1.34.0) and fixes attribute usage on non-HTTP/internal spans, while improving proxy forwarding span metadata for upstream visibility.

Changes:

  • Upgraded semconv imports from v1.27.0 to v1.34.0 across tracing/proxy code.
  • Replaced hand-rolled gen_ai.* attribute keys with official semconv constants and introduced mcp.response.status for internal MCP spans.
  • Improved proxy forwarding span lifecycle handling and added server.address attribution for upstream tracing.
Show a summary per file
File Description
internal/tracing/provider.go Bumps semconv import to v1.34.0 for provider/resource attributes.
internal/tracing/provider_test.go Updates tests to match semconv v1.34.0 import.
internal/tracing/http.go Updates HTTP span wrapper to semconv v1.34.0 import.
internal/tracing/genai_attrs.go Re-exports official gen_ai keys and adds mcp.response.status.
internal/server/unified.go Stops using HTTP status semconv on an internal span; uses mcp.response.status instead.
internal/proxy/proxy.go Adds Server.upstreamHost() helper for server.address attribution.
internal/proxy/handler.go Adds server.address to forward span and defers span end for safety.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 7/7 changed files
  • Comments generated: 2

Comment thread internal/proxy/handler.go Outdated
Comment thread internal/proxy/proxy.go
lpcox and others added 2 commits May 29, 2026 05:55
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 29, 2026

@copilot fix this failing ci checks https://github.com/github/gh-aw-mcpg/actions/runs/26638527237/job/78504913214?pr=6683 https://github.com/github/gh-aw-mcpg/actions/runs/26638527237/job/78504913249...

Fixed in the latest commit. The upstreamHost() function was missing its closing }, which caused a parse error cascading into forwardToGitHub's signature. Build and proxy tests now pass locally.

Copilot finished work on behalf of lpcox May 29, 2026 13:01
@lpcox lpcox merged commit f3b485a into main May 29, 2026
16 checks passed
@lpcox lpcox deleted the copilot/go-fan-go-module-review branch May 29, 2026 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[go-fan] Go Module Review: go.opentelemetry.io/otel

3 participants