feat(tracing): HTTP response status on spans, sanitize error recording, bump semconv to v1.27.0 (#4879)

Three OTel best-practice gaps identified in the Go Fan module review:
`WrapHTTPHandler` never recorded the HTTP response status code,
`SetStatus` and `RecordError` were leaking raw error strings to trace
backends, and all files were pinned to semconv `v1.26.0` despite
`v1.27.0` being available.

## Changes

- **`tracing/http.go` — capture HTTP response status on spans**
- Added `statusResponseWriter` wrapper intercepting `WriteHeader` and
`Write` (handles implicit 200 when only `Write` is called)
- Added `Unwrap() http.ResponseWriter` to preserve optional interfaces
(`http.Flusher`, `http.Hijacker`, etc.) so streaming/SSE handlers are
not broken
- Sets `http.response.status_code` attribute and `codes.Error` span
status for 5xx via `defer`, ensuring attributes are recorded even if the
handler panics

- **`server/unified.go` — sanitize error recording**
- `execSpan.SetStatus(codes.Error, err.Error())` →
`execSpan.SetStatus(codes.Error, "tool execution failed")`
- `execSpan.RecordError(err)` → `execSpan.RecordError(fmt.Errorf("tool
execution failed"))` — prevents raw upstream error text from appearing
as `exception.message` in trace backends; the full error is still
returned to the caller via `mcp.NewErrorCallToolResult(err)`

- **semconv `v1.26.0` → `v1.27.0`** across all four affected files
(`tracing/http.go`, `tracing/provider.go`, `server/unified.go`,
`proxy/handler.go`); no go.mod/go.sum changes required as semconv is a
sub-package of the already-imported `go.opentelemetry.io/otel v1.43.0`

- **Tests** — added three new test cases to
`internal/tracing/provider_test.go` covering the HTTP status recording
behaviour:
- Explicit `WriteHeader(404)` is reflected in the span attribute; 4xx
does not set `codes.Error`
  - Implicit 200 via `Write` without `WriteHeader` is correctly recorded
  - 5xx sets span status to `codes.Error` with a non-empty description

Author: lpcox

internal/proxy/handler.go

@@ -12,7 +12,7 @@ import (
 
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
-	semconv "go.opentelemetry.io/otel/semconv/v1.26.0"
+	semconv "go.opentelemetry.io/otel/semconv/v1.27.0"
 	oteltrace "go.opentelemetry.io/otel/trace"
 
 	"github.com/github/gh-aw-mcpg/internal/difc"

internal/server/unified.go

@@ -11,7 +11,7 @@ import (
 
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
-	semconv "go.opentelemetry.io/otel/semconv/v1.26.0"
+	semconv "go.opentelemetry.io/otel/semconv/v1.27.0"
 	oteltrace "go.opentelemetry.io/otel/trace"
 
 	"github.com/github/gh-aw-mcpg/internal/config"
@@ -552,8 +552,10 @@ func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName
 		// Transport errors (connection failure, JSON parse error, etc.) are not rate-limit
 		// events and must not affect the consecutive rate-limit counter. Leave the circuit
 		// breaker state unchanged so genuine rate-limit history is preserved.
-		execSpan.RecordError(err)
-		execSpan.SetStatus(codes.Error, err.Error())
+		// Use a generic error message for trace recording to avoid leaking internal details
+		// to trace backends; the full error is returned to the caller and logged separately.
+		execSpan.RecordError(fmt.Errorf("tool execution failed"))
+		execSpan.SetStatus(codes.Error, "tool execution failed")
 		httpStatusCode = 500
 		return mcp.NewErrorCallToolResult(err)
 	}

internal/tracing/http.go

@@ -7,11 +7,39 @@ import (
 
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
+	"go.opentelemetry.io/otel/codes"
 	"go.opentelemetry.io/otel/propagation"
-	semconv "go.opentelemetry.io/otel/semconv/v1.26.0"
+	semconv "go.opentelemetry.io/otel/semconv/v1.27.0"
 	oteltrace "go.opentelemetry.io/otel/trace"
 )
 
+// statusResponseWriter wraps http.ResponseWriter to capture the HTTP response status code.
+// Unwrap allows http.ResponseController and other callers to access the underlying
+// writer's optional interfaces (e.g. http.Flusher, http.Hijacker) transparently.
+type statusResponseWriter struct {
+	http.ResponseWriter
+	statusCode int
+}
+
+func (rw *statusResponseWriter) WriteHeader(code int) {
+	rw.statusCode = code
+	rw.ResponseWriter.WriteHeader(code)
+}
+
+// Write captures an implicit 200 status when Write is called without a prior WriteHeader.
+func (rw *statusResponseWriter) Write(b []byte) (int, error) {
+	if rw.statusCode == 0 {
+		rw.statusCode = http.StatusOK
+	}
+	return rw.ResponseWriter.Write(b)
+}
+
+// Unwrap returns the underlying http.ResponseWriter so that callers can type-assert
+// optional interfaces such as http.Flusher or http.Hijacker against the real writer.
+func (rw *statusResponseWriter) Unwrap() http.ResponseWriter {
+	return rw.ResponseWriter
+}
+
 // WrapHTTPHandler wraps an http.Handler with an OpenTelemetry server span.
 // A span named spanName is created for every request, with
 // http.request.method and url.path set automatically. Extra attrs are merged in.
@@ -48,6 +76,13 @@ func WrapHTTPHandler(next http.Handler, spanName string, extraAttrs ...attribute
 
 		logTracing.Printf("Span started: span=%s, traceID=%s", spanName, span.SpanContext().TraceID())
 
-		next.ServeHTTP(w, r.WithContext(ctx))
+		srw := &statusResponseWriter{ResponseWriter: w, statusCode: http.StatusOK}
+		defer func() {
+			span.SetAttributes(semconv.HTTPResponseStatusCodeKey.Int(srw.statusCode))
+			if srw.statusCode >= 500 {
+				span.SetStatus(codes.Error, http.StatusText(srw.statusCode))
+			}
+		}()
+		next.ServeHTTP(srw, r.WithContext(ctx))
 	})
 }

internal/tracing/provider.go

@@ -30,7 +30,7 @@ import (
 	"go.opentelemetry.io/otel/propagation"
 	"go.opentelemetry.io/otel/sdk/resource"
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
-	semconv "go.opentelemetry.io/otel/semconv/v1.26.0"
+	semconv "go.opentelemetry.io/otel/semconv/v1.27.0"
 	"go.opentelemetry.io/otel/trace"
 	"go.opentelemetry.io/otel/trace/noop"
 

internal/tracing/provider_test.go

@@ -11,9 +11,11 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.opentelemetry.io/otel"
+	"go.opentelemetry.io/otel/codes"
 	"go.opentelemetry.io/otel/propagation"
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
 	"go.opentelemetry.io/otel/sdk/trace/tracetest"
+	semconv "go.opentelemetry.io/otel/semconv/v1.27.0"
 	"go.opentelemetry.io/otel/trace"
 	"go.opentelemetry.io/otel/trace/noop"
 
@@ -614,3 +616,128 @@ func TestParentContext_AllZerosTraceID(t *testing.T) {
 	assert.Equal(t, ctx, parentCtx,
 		"ParentContext with an all-zeros traceId must return the original context unchanged")
 }
+
+// newInMemoryProvider creates a fresh in-process SDK tracer provider with an in-memory
+// exporter and registers it globally. The returned cleanup func restores the noop provider.
+func newInMemoryProvider(t *testing.T) (*sdktrace.TracerProvider, *tracetest.InMemoryExporter, func()) {
+	t.Helper()
+	exporter := tracetest.NewInMemoryExporter()
+	sp := sdktrace.NewSimpleSpanProcessor(exporter)
+	tp := sdktrace.NewTracerProvider(
+		sdktrace.WithSpanProcessor(sp),
+		sdktrace.WithSampler(sdktrace.AlwaysSample()),
+	)
+	otel.SetTracerProvider(tp)
+	return tp, exporter, func() { otel.SetTracerProvider(noop.NewTracerProvider()) }
+}
+
+// TestWrapHTTPHandler_RecordsExplicitStatusCode verifies that an explicit WriteHeader call
+// is reflected in the http.response.status_code span attribute.
+func TestWrapHTTPHandler_RecordsExplicitStatusCode(t *testing.T) {
+	ctx := context.Background()
+
+	provider, err := tracing.InitProvider(ctx, nil)
+	require.NoError(t, err)
+	defer provider.Shutdown(ctx)
+
+	_, exporter, cleanup := newInMemoryProvider(t)
+	defer cleanup()
+
+	req := httptest.NewRequest(http.MethodGet, "/mcp", nil)
+	rr := httptest.NewRecorder()
+
+	inner := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusNotFound) // explicit 404
+	})
+
+	tracing.WrapHTTPHandler(inner, "test.status").ServeHTTP(rr, req)
+
+	spans := exporter.GetSpans()
+	require.Len(t, spans, 1, "expected exactly one span")
+	span := spans[0]
+
+	// Span must carry http.response.status_code = 404
+	var found bool
+	for _, attr := range span.Attributes {
+		if attr.Key == semconv.HTTPResponseStatusCodeKey {
+			assert.Equal(t, int64(http.StatusNotFound), attr.Value.AsInt64(), "status code attribute must be 404")
+			found = true
+		}
+	}
+	assert.True(t, found, "http.response.status_code attribute must be present on the span")
+
+	// 4xx must NOT set span status to Error
+	assert.Equal(t, codes.Unset, span.Status.Code, "4xx responses must not set span status to Error")
+}
+
+// TestWrapHTTPHandler_RecordsImplicit200 verifies that a handler that calls Write
+// without an explicit WriteHeader is recorded as status 200.
+func TestWrapHTTPHandler_RecordsImplicit200(t *testing.T) {
+	ctx := context.Background()
+
+	provider, err := tracing.InitProvider(ctx, nil)
+	require.NoError(t, err)
+	defer provider.Shutdown(ctx)
+
+	_, exporter, cleanup := newInMemoryProvider(t)
+	defer cleanup()
+
+	req := httptest.NewRequest(http.MethodGet, "/mcp", nil)
+	rr := httptest.NewRecorder()
+
+	inner := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		_, _ = w.Write([]byte("ok")) // implicit 200 — no WriteHeader call
+	})
+
+	tracing.WrapHTTPHandler(inner, "test.implicit200").ServeHTTP(rr, req)
+
+	spans := exporter.GetSpans()
+	require.Len(t, spans, 1, "expected exactly one span")
+
+	var found bool
+	for _, attr := range spans[0].Attributes {
+		if attr.Key == semconv.HTTPResponseStatusCodeKey {
+			assert.Equal(t, int64(http.StatusOK), attr.Value.AsInt64(), "implicit Write must record status 200")
+			found = true
+		}
+	}
+	assert.True(t, found, "http.response.status_code attribute must be present on the span")
+}
+
+// TestWrapHTTPHandler_5xxSetsSpanStatusError verifies that a 5xx response sets span
+// status to codes.Error and leaves a non-empty description.
+func TestWrapHTTPHandler_5xxSetsSpanStatusError(t *testing.T) {
+	ctx := context.Background()
+
+	provider, err := tracing.InitProvider(ctx, nil)
+	require.NoError(t, err)
+	defer provider.Shutdown(ctx)
+
+	_, exporter, cleanup := newInMemoryProvider(t)
+	defer cleanup()
+
+	req := httptest.NewRequest(http.MethodPost, "/mcp", nil)
+	rr := httptest.NewRecorder()
+
+	inner := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError) // 500
+	})
+
+	tracing.WrapHTTPHandler(inner, "test.5xx").ServeHTTP(rr, req)
+
+	spans := exporter.GetSpans()
+	require.Len(t, spans, 1, "expected exactly one span")
+	span := spans[0]
+
+	assert.Equal(t, codes.Error, span.Status.Code, "5xx must set span status to Error")
+	assert.NotEmpty(t, span.Status.Description, "5xx must provide a non-empty status description")
+
+	var found bool
+	for _, attr := range span.Attributes {
+		if attr.Key == semconv.HTTPResponseStatusCodeKey {
+			assert.Equal(t, int64(http.StatusInternalServerError), attr.Value.AsInt64())
+			found = true
+		}
+	}
+	assert.True(t, found, "http.response.status_code attribute must be present on the span")
+}