Skip to content

[go-fan] Go Module Review: go.opentelemetry.io/otel (and suite)ย #7569

Description

@github-actions

๐Ÿน Go Fan Report: OpenTelemetry Go SDK

Module Overview

The OpenTelemetry Go SDK is the official Go implementation of OpenTelemetry โ€” the vendor-neutral observability framework for distributed tracing, metrics, and logging. The project uses it for distributed trace export over OTLP/HTTP to observability backends.

Four packages from the OTel suite are direct dependencies:

  • go.opentelemetry.io/otel v1.44.0 โ€” core API (tracers, context propagation)
  • go.opentelemetry.io/otel/trace v1.44.0 โ€” trace types and interfaces
  • go.opentelemetry.io/otel/sdk v1.44.0 โ€” SDK implementation (providers, exporters, samplers)
  • go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.44.0 โ€” OTLP/HTTP exporter

Current Usage in gh-aw

The tracing integration is well-structured, living primarily in internal/tracing/:

  • Files: 9 production files, 5 test files across internal/tracing/, internal/server/, internal/proxy/
  • Import Count: ~30 OTel imports across production code
  • Key APIs Used:
    • otel.SetTracerProvider() / otel.Tracer() โ€” global tracer registration
    • sdktrace.NewTracerProvider() with WithBatcher, WithResource, WithSampler
    • otlptracehttp.New() with WithEndpointURL, WithTimeout, WithHeaders
    • trace.ContextWithRemoteSpanContext() โ€” W3C parent context injection
    • span.RecordError(), span.SetStatus(), span.SetAttributes() โ€” span enrichment
    • propagation.NewCompositeTextMapPropagator(TraceContext{}, Baggage{}) โ€” W3C propagation
    • resource.New() with telemetry SDK, container, host, PID detectors
    • semconv/v1.41.0 โ€” HTTP, URL, server, error type attributes
    • semconv/v1.34.0 โ€” GenAI semantic convention attributes

Span hierarchy implemented:

gateway.request (server span, per HTTP request)
โ””โ”€โ”€ mcp.tool_call (internal span, tool call lifecycle)
    โ””โ”€โ”€ gateway.backend.execute (client span, backend RPC)

proxy.difc_pipeline (internal span, DIFC phases 0-6)
โ””โ”€โ”€ proxy.backend.forward (client span, upstream GitHub API)

Research Findings

The OTel Go SDK at v1.44.0 is the current stable release. The project is fully up-to-date on the SDK. Notable features in use:

Recent Updates (v1.40.0โ€“v1.44.0)

  • sdktrace.WithBatcher is the recommended batch exporter (used โœ“)
  • otlptracehttp.WithEndpointURL replaces the deprecated WithEndpoint (used โœ“)
  • resource.WithContainer() auto-detects container environment (used โœ“)
  • trace/noop package for zero-overhead no-op provider (used โœ“)

Best Practices

  • Register global W3C propagator at startup to enable cross-service trace continuation (implemented โœ“)
  • Use sdktrace.AlwaysSample()/NeverSample()/TraceIDRatioBased() based on rate config (implemented โœ“)
  • Prefer otel.Tracer(instrumentationName) with a stable scope name (implemented โœ“)
  • Use span.RecordError(err, WithStackTrace(true)) + span.SetStatus(codes.Error, msg) together (implemented via RecordSpanError helper โœ“)
  • Use resource.Merge() on error to preserve partial resource detection (implemented โœ“)

Improvement Opportunities

๐Ÿ› Bug: Missing Span Error Recording in proxy/handler.go

Three error paths in handleWithDIFC end spans without marking them as errors. This causes spans to appear successful in tracing backends even when requests fail with 502/503.

Case 1: Guard not initialized (returns HTTP 503):

// internal/proxy/handler.go ~L184
if !s.guardInitialized {
    // difcSpan ends via defer without error status โ† gap
    httputil.WriteErrorResponse(w, http.StatusServiceUnavailable, ...)
    return
}

Case 2: RunPipelinePrePhases non-access-denied failure (returns HTTP 502):

// internal/proxy/handler.go ~L213
logHandler.Printf("[DIFC] Phase 1 failed: %v", err)
httputil.WriteErrorResponse(w, http.StatusBadGateway, "bad_gateway", "resource labeling failed")
return  // difcSpan ends without error status โ† gap

Case 3: forwardAndReadBody returns nil/network error (returns HTTP 502):

// internal/proxy/handler.go ~L233
if resp == nil {
    return  // both fwdSpan AND difcSpan end without error status โ† gap
}

Fix pattern (using the existing tracing.RecordSpanError helper):

// Case 1
if !s.guardInitialized {
    err := errors.New("proxy enforcement not configured")
    tracing.RecordSpanError(difcSpan, err, "service unavailable")
    httputil.WriteErrorResponse(w, http.StatusServiceUnavailable, ...)
    return
}

// Case 2
tracing.RecordSpanError(difcSpan, err, "resource labeling failed")
httputil.WriteErrorResponse(w, http.StatusBadGateway, ...)
return

// Case 3
if resp == nil {
    netErr := errors.New("upstream request failed")
    tracing.RecordSpanErrorOnAll(netErr, "upstream request failed", fwdSpan, difcSpan)
    return
}

๐Ÿƒ Quick Win: Dual Semconv Versions โ€” Consolidation Opportunity

The codebase uses two different semconv schema versions:

  • go.opentelemetry.io/otel/semconv/v1.34.0 โ€” only in genai_attrs.go for GenAI keys
  • go.opentelemetry.io/otel/semconv/v1.41.0 โ€” everywhere else (HTTP, URL, error, resource)

This means spans like proxy.difc_pipeline and proxy.backend.forward mix attribute constants from both schema versions on the same span (GenAI from v1.34.0, URL/server from v1.41.0).

Recommendation: Verify whether semconv/v1.41.0 exports the same GenAISystemKey, GenAIToolNameKey, GenAIConversationIDKey, GenAIAgentIDKey, and GenAIAgentNameKey constants. If yes, consolidate genai_attrs.go to import from v1.41.0 to match the rest of the codebase. This eliminates the dual-import and ensures schema URL consistency in trace backends.

โœจ Feature Opportunity: Explicit BatchSpanProcessor Configuration

The SDK's batcher is initialized with defaults:

sdktrace.WithBatcher(exporter)  // uses SDK defaults

Default settings: max queue 2048, max batch 512, schedule delay 5s, export timeout 30s.

For a high-throughput MCP gateway, consider explicit tuning:

sdktrace.WithBatcher(exporter,
    sdktrace.WithMaxQueueSize(8192),
    sdktrace.WithMaxExportBatchSize(512),
    sdktrace.WithBatchTimeout(2*time.Second),
    sdktrace.WithExportTimeout(15*time.Second),
)

These could be exposed as optional TracingConfig fields (e.g., gateway.tracing.batch_timeout) so operators can tune without recompilation.

โœจ Feature Opportunity: OTel Metrics for Gateway KPIs

The project currently exports only traces. Adding OTel metric instruments could provide aggregated visibility without per-request overhead:

  • gateway.requests.total (counter, by method/server/status)
  • gateway.tool_calls.duration (histogram)
  • gateway.difc.filtered_items.total (counter, by tool)
  • gateway.rate_limit.hits.total (counter, by server)

The go.opentelemetry.io/otel/metric v1.44.0 is already an indirect dependency โ€” promoting it to direct and adding an OTLP metric exporter would complete the observability picture.

๐Ÿ“ Best Practice: Add mcp.request.id Span Attribute

JSON-RPC request IDs are available at the server SDK boundary (in WithSDKLogging). Propagating the request ID as a span attribute (mcp.request.id) would allow correlating specific tool calls in a trace backend directly with JSON-RPC log entries, enabling faster debugging of protocol errors.

๐Ÿ”ง General: resource.WithProcessPID() in Container Environments

The resource builder includes resource.WithProcessPID() which adds the container-internal PID to all spans. In containerized deployments this is typically PID 1 (or close to it), adding noise without diagnostic value. Consider making this conditional on a debug flag, or replacing with a more meaningful attribute like the container instance ID.

Module Summary

Field Value
Module go.opentelemetry.io/otel
Version v1.44.0
Repository https://github.com/open-telemetry/opentelemetry-go
Latest Release v1.44.0 (matches current pin)
Last Reviewed 2026-06-15

Key Features

  • Zero-overhead noop tracer when OTLP endpoint not configured
  • W3C TraceContext + Baggage propagation for cross-service trace continuation
  • OTLP/HTTP exporter with configurable headers and timeout
  • Pluggable samplers: AlwaysSample, NeverSample, TraceIDRatioBased
  • Semantic conventions versioned as sub-paths (v1.34.0, v1.41.0, etc.)
  • Resource detection: telemetry SDK, container, host, process PID
  • Batcher processor for async span export
  • trace/noop for production no-op path

References

Recommendations (Prioritized)

  1. [P0 - Bug] Fix missing tracing.RecordSpanError calls in proxy/handler.go for the three identified error paths โ€” straightforward 3-line fix using existing helpers
  2. [P1 - Cleanup] Verify GenAI attribute availability in semconv/v1.41.0 and consolidate genai_attrs.go to a single semconv version
  3. [P2 - Enhancement] Add mcp.request.id attribute to tool call spans for JSON-RPC correlation
  4. [P3 - Config] Expose BatchSpanProcessor options in TracingConfig for operator tuning
  5. [P4 - Observability] Add OTel metrics for key gateway KPIs

Next Steps

  • Fix the span error recording gaps in proxy/handler.go (P0)
  • Run make test-all to confirm no regressions after the fix
  • Open a follow-up discussion on OTel metrics requirements with the team

Generated by Go Fan ๐Ÿน โ€” workflow run ยง27536108384

Generated by Go Fan ยท 755.6 AIC ยท โŠž 34.4K ยท โ—ท

  • expires on Jun 22, 2026, 9:24 AM UTC

Metadata

Metadata

Assignees

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions