Skip to content

tracing: fix StartProxyForwardSpan dead code by adding serverAddress param#7536

Merged
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-resolution
Jun 14, 2026
Merged

tracing: fix StartProxyForwardSpan dead code by adding serverAddress param#7536
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-resolution

Conversation

Copilot AI commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

StartProxyForwardSpan in internal/tracing/span_helpers.go was dead code — it omitted semconv.ServerAddressKey, causing internal/proxy/handler.go to inline the span creation with a diverged attribute set instead of using the helper.

Changes

  • internal/tracing/span_helpers.go — Add serverAddress string parameter to StartProxyForwardSpan; include semconv.ServerAddressKey in span attributes to match the live inline version
  • internal/proxy/handler.go — Replace inline "proxy.backend.forward" span creation with tracing.StartProxyForwardSpan; remove now-unused oteltrace import
  • internal/tracing/span_helpers_test.go — Update call site and add assertion for semconv.ServerAddressKey
// Before: inline in handler.go
fwdCtx, fwdSpan := h.GetTracer().Start(ctx, "proxy.backend.forward",
    oteltrace.WithAttributes(
        semconv.URLPathKey.String(r.URL.Path),
        semconv.ServerAddressKey.String(h.server.upstreamHost()),
        tracing.GenAIToolName.String(toolName),
    ),
    oteltrace.WithSpanKind(oteltrace.SpanKindClient),
)

// After: via helper
fwdCtx, fwdSpan := tracing.StartProxyForwardSpan(ctx, h.GetTracer(), toolName, r.URL.Path, h.server.upstreamHost())

GitHub Advanced Security started work on behalf of lpcox June 14, 2026 17:40 View session
GitHub Advanced Security finished work on behalf of lpcox June 14, 2026 17:41
Copilot AI changed the title [WIP] Remove dead code for StartProxyForwardSpan helper tracing: fix StartProxyForwardSpan dead code by adding serverAddress param Jun 14, 2026
Copilot finished work on behalf of lpcox June 14, 2026 17:45
Copilot AI requested a review from lpcox June 14, 2026 17:45
GitHub Advanced Security started work on behalf of lpcox June 14, 2026 17:46 View session
@lpcox lpcox marked this pull request as ready for review June 14, 2026 17:46
Copilot AI review requested due to automatic review settings June 14, 2026 17:46
@lpcox

lpcox commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator

@copilot resolve the merge conflicts in this pull request

GitHub Advanced Security finished work on behalf of lpcox June 14, 2026 17:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 standardizes OpenTelemetry span creation for proxied backend forwarding by fixing the StartProxyForwardSpan helper to include the server.address attribute, then reusing the helper from the proxy handler to avoid duplicated/diverged span attribute sets.

Changes:

  • Extend tracing.StartProxyForwardSpan to accept serverAddress and set semconv.ServerAddressKey.
  • Replace the inline "proxy.backend.forward" span creation in the proxy handler with the tracing helper (and remove the now-unused otel/trace import).
  • Update tracing helper tests to pass a server address and assert the server.address attribute is recorded.
Show a summary per file
File Description
internal/tracing/span_helpers.go Adds serverAddress parameter and includes semconv.ServerAddressKey on the forward span.
internal/proxy/handler.go Switches forward-span creation to tracing.StartProxyForwardSpan and drops the unused oteltrace import.
internal/tracing/span_helpers_test.go Updates the helper call and asserts the new server.address span attribute.

Copilot's findings

Tip

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

  • Files reviewed: 3/3 changed files
  • Comments generated: 0

Copilot AI commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

Conflicts are resolved in commit 016d132e by merging origin/main into this branch and resolving the internal/tracing/span_helpers.go conflict while preserving the server.address span attribute.

GitHub Advanced Security started work on behalf of lpcox June 14, 2026 17:54 View session
Copilot finished work on behalf of lpcox June 14, 2026 17:54
GitHub Advanced Security finished work on behalf of lpcox June 14, 2026 17:55
@lpcox lpcox merged commit 7834629 into main Jun 14, 2026
27 checks passed
@lpcox lpcox deleted the copilot/duplicate-code-resolution branch June 14, 2026 17:58
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.

3 participants