Part of duplicate code analysis: #6070
Summary
Both internal/server/unified.go and internal/proxy/handler.go independently declare a tracer oteltrace.Tracer field, an identical getTracer() method, and repeat the same 5-line OTel span creation block pattern (WithAttributes + WithSpanKind) — twice each, once for an Internal span and once for a Client span.
Duplication Details
Pattern: Embedded tracer field + getTracer() method
- Severity: Medium
- Occurrences: 2 structs with identical field+method, 4 span creation blocks total
- Locations:
internal/server/unified.go (lines 121–126, 389–395, 493–498)
internal/proxy/handler.go (lines 38–44, 150–155, 206–211)
Duplicated field + method (identical in both files):
tracer oteltrace.Tracer
func (x *X) getTracer() oteltrace.Tracer {
return tracing.GetCachedOrGlobal(x.tracer)
}
Duplicated span creation pattern (4 occurrences):
ctx, span := x.getTracer().Start(ctx, "span.name",
oteltrace.WithAttributes(
attribute.String("key", value),
),
oteltrace.WithSpanKind(oteltrace.SpanKindInternal), // or SpanKindClient
)
defer span.End()
Impact Analysis
- Maintainability: Adding a new span attribute (e.g. for a new spec requirement) must be done in both files separately
- Bug Risk: The
getTracer() implementations are trivially identical today, but could diverge silently if one is updated and the other is not
- Code Bloat: ~30 lines of near-identical span plumbing
Refactoring Recommendations
-
Extract a tracerHolder embed struct
- Create
internal/tracing/holder.go with a CachedTracer struct that embeds the field + method:
// CachedTracer holds an optional pre-initialized tracer and falls back to
// the global tracer when nil. Embed this in structs that create OTEL spans.
type CachedTracer struct {
Tracer oteltrace.Tracer
}
func (ct CachedTracer) GetTracer() oteltrace.Tracer {
return GetCachedOrGlobal(ct.Tracer)
}
- Embed in
UnifiedServer and proxyHandler and update callers
- Estimated effort: 1–2 hours
-
Consider a shared span-start helper for the repeated WithAttributes + WithSpanKind boilerplate (lower priority).
Implementation Checklist
Parent Issue
See parent analysis report: #6070
Related to #6070
Generated by Duplicate Code Detector · ● 1.9M · ◷
Part of duplicate code analysis: #6070
Summary
Both
internal/server/unified.goandinternal/proxy/handler.goindependently declare atracer oteltrace.Tracerfield, an identicalgetTracer()method, and repeat the same 5-line OTel span creation block pattern (WithAttributes + WithSpanKind) — twice each, once for anInternalspan and once for aClientspan.Duplication Details
Pattern: Embedded
tracerfield +getTracer()methodinternal/server/unified.go(lines 121–126, 389–395, 493–498)internal/proxy/handler.go(lines 38–44, 150–155, 206–211)Duplicated field + method (identical in both files):
Duplicated span creation pattern (4 occurrences):
Impact Analysis
getTracer()implementations are trivially identical today, but could diverge silently if one is updated and the other is notRefactoring Recommendations
Extract a
tracerHolderembed structinternal/tracing/holder.gowith aCachedTracerstruct that embeds the field + method:UnifiedServerandproxyHandlerand update callersConsider a shared span-start helper for the repeated
WithAttributes+WithSpanKindboilerplate (lower priority).Implementation Checklist
CachedTracerembed vs alternative approachCachedTracerininternal/tracing/UnifiedServerandproxyHandlermake agent-finishedto verify no regressionsParent Issue
See parent analysis report: #6070
Related to #6070