refactor: extract repeated OTEL span-start patterns into named tracing helpers#6684
Conversation
Add StartToolCallSpan, StartBackendExecuteSpan, StartDIFCPipelineSpan, and StartProxyForwardSpan helpers to internal/tracing/span_helpers.go. Replace the four inline span-start call sites in server/unified.go and proxy/handler.go with the new helpers. Add tests for each helper in span_helpers_test.go.
There was a problem hiding this comment.
Pull request overview
This PR extracts four repeated tracer.Start(...) call patterns from server/unified.go and proxy/handler.go into named constructor helpers in internal/tracing/span_helpers.go, reducing duplication and centralizing span name, kind, and attribute conventions. Tests verify each helper's span name, kind, and attributes via an in-memory exporter.
Changes:
- Added four new span-start helpers (
StartToolCallSpan,StartBackendExecuteSpan,StartDIFCPipelineSpan,StartProxyForwardSpan). - Replaced inline
GetTracer().Start(...)blocks at four call sites and removed unusedoteltraceimports. - Added unit tests covering each helper.
Show a summary per file
| File | Description |
|---|---|
| internal/tracing/span_helpers.go | New named span constructors encoding name/attrs/kind. |
| internal/tracing/span_helpers_test.go | In-memory exporter tests for each new helper. |
| internal/server/unified.go | Replaces two inline span starts with helpers; drops oteltrace import. |
| internal/proxy/handler.go | Replaces two inline span starts with helpers; drops oteltrace import. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 0
Fixed in d620dc1 by removing an unused |
Four near-identical
GetTracer().Start(...)blocks acrossserver/unified.goandproxy/handler.goshared the same attribute sets and span-kind declarations with no shared abstraction, creating attribute-ordering drift and a maintenance burden for future spec updates.Changes
internal/tracing/span_helpers.go— four new named constructors, each encoding span name, attributes, and kind:StartToolCallSpan→mcp.tool_call(Internal)StartBackendExecuteSpan→gateway.backend.execute(Client)StartDIFCPipelineSpan→proxy.difc_pipeline(Internal)StartProxyForwardSpan→proxy.backend.forward(Client)internal/server/unified.go/internal/proxy/handler.go— inline span blocks replaced with the new helpers; now-unusedoteltraceimports removed.internal/tracing/span_helpers_test.go— tests for each helper asserting span name, kind, and attribute values.