tracing: Add span attributes to tracing module (#7269)

* WIP tracing span attributes

* better test

* only write attributes after other middleware (and request)

* Fix test to use header response placeholders

modules/caddyhttp/tracing/module.go

@@ -27,6 +27,9 @@ type Tracing struct {
 	// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#span
 	SpanName string `json:"span"`
 
+	// SpanAttributes are custom key-value pairs to be added to spans
+	SpanAttributes map[string]string `json:"span_attributes,omitempty"`
+
 	// otel implements opentelemetry related logic.
 	otel openTelemetryWrapper
 
@@ -46,7 +49,7 @@ func (ot *Tracing) Provision(ctx caddy.Context) error {
 	ot.logger = ctx.Logger()
 
 	var err error
-	ot.otel, err = newOpenTelemetryWrapper(ctx, ot.SpanName)
+	ot.otel, err = newOpenTelemetryWrapper(ctx, ot.SpanName, ot.SpanAttributes)
 
 	return err
 }
@@ -69,6 +72,10 @@ func (ot *Tracing) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyh
 //
 //	tracing {
 //	    [span <span_name>]
+//		[span_attributes {
+//			attr1 value1
+//			attr2 value2
+//		}]
 //	}
 func (ot *Tracing) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 	setParameter := func(d *caddyfile.Dispenser, val *string) error {
@@ -94,12 +101,30 @@ func (ot *Tracing) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 	}
 
 	for d.NextBlock(0) {
-		if dst, ok := paramsMap[d.Val()]; ok {
-			if err := setParameter(d, dst); err != nil {
-				return err
+		switch d.Val() {
+		case "span_attributes":
+			if ot.SpanAttributes == nil {
+				ot.SpanAttributes = make(map[string]string)
+			}
+			for d.NextBlock(1) {
+				key := d.Val()
+				if !d.NextArg() {
+					return d.ArgErr()
+				}
+				value := d.Val()
+				if d.NextArg() {
+					return d.ArgErr()
+				}
+				ot.SpanAttributes[key] = value
+			}
+		default:
+			if dst, ok := paramsMap[d.Val()]; ok {
+				if err := setParameter(d, dst); err != nil {
+					return err
+				}
+			} else {
+				return d.ArgErr()
 			}
-		} else {
-			return d.ArgErr()
 		}
 	}
 	return nil

modules/caddyhttp/tracing/module_test.go

@@ -2,12 +2,16 @@ package tracing
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"net/http"
 	"net/http/httptest"
 	"strings"
 	"testing"
 
+	"go.opentelemetry.io/otel/sdk/trace"
+	"go.opentelemetry.io/otel/sdk/trace/tracetest"
+
 	"github.com/caddyserver/caddy/v2"
 	"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
 	"github.com/caddyserver/caddy/v2/modules/caddyhttp"
@@ -15,17 +19,26 @@ import (
 
 func TestTracing_UnmarshalCaddyfile(t *testing.T) {
 	tests := []struct {
-		name     string
-		spanName string
-		d        *caddyfile.Dispenser
-		wantErr  bool
+		name           string
+		spanName       string
+		spanAttributes map[string]string
+		d              *caddyfile.Dispenser
+		wantErr        bool
 	}{
 		{
 			name:     "Full config",
 			spanName: "my-span",
+			spanAttributes: map[string]string{
+				"attr1": "value1",
+				"attr2": "value2",
+			},
 			d: caddyfile.NewTestDispenser(`
 tracing {
 	span my-span
+	span_attributes {
+		attr1 value1
+		attr2 value2
+	}
 }`),
 			wantErr: false,
 		},
@@ -42,6 +55,21 @@ tracing {
 			name: "Empty config",
 			d: caddyfile.NewTestDispenser(`
 tracing {
+}`),
+			wantErr: false,
+		},
+		{
+			name: "Only span attributes",
+			spanAttributes: map[string]string{
+				"service.name":    "my-service",
+				"service.version": "1.0.0",
+			},
+			d: caddyfile.NewTestDispenser(`
+tracing {
+	span_attributes {
+		service.name my-service
+		service.version 1.0.0
+	}
 }`),
 			wantErr: false,
 		},
@@ -56,6 +84,20 @@ tracing {
 			if ot.SpanName != tt.spanName {
 				t.Errorf("UnmarshalCaddyfile() SpanName = %v, want SpanName %v", ot.SpanName, tt.spanName)
 			}
+
+			if len(tt.spanAttributes) > 0 {
+				if ot.SpanAttributes == nil {
+					t.Errorf("UnmarshalCaddyfile() SpanAttributes is nil, expected %v", tt.spanAttributes)
+				} else {
+					for key, expectedValue := range tt.spanAttributes {
+						if actualValue, exists := ot.SpanAttributes[key]; !exists {
+							t.Errorf("UnmarshalCaddyfile() SpanAttributes missing key %v", key)
+						} else if actualValue != expectedValue {
+							t.Errorf("UnmarshalCaddyfile() SpanAttributes[%v] = %v, want %v", key, actualValue, expectedValue)
+						}
+					}
+				}
+			}
 		})
 	}
 }
@@ -79,6 +121,26 @@ func TestTracing_UnmarshalCaddyfile_Error(t *testing.T) {
 			d: caddyfile.NewTestDispenser(`
 tracing {
 	span
+}`),
+			wantErr: true,
+		},
+		{
+			name: "Span attributes missing value",
+			d: caddyfile.NewTestDispenser(`
+tracing {
+	span_attributes {
+		key
+	}
+}`),
+			wantErr: true,
+		},
+		{
+			name: "Span attributes too many arguments",
+			d: caddyfile.NewTestDispenser(`
+tracing {
+	span_attributes {
+		key value extra
+	}
 }`),
 			wantErr: true,
 		},
@@ -181,6 +243,160 @@ func TestTracing_ServeHTTP_Next_Error(t *testing.T) {
 	}
 }
 
+func TestTracing_JSON_Configuration(t *testing.T) {
+	// Test that our struct correctly marshals to and from JSON
+	original := &Tracing{
+		SpanName: "test-span",
+		SpanAttributes: map[string]string{
+			"service.name":    "test-service",
+			"service.version": "1.0.0",
+			"env":             "test",
+		},
+	}
+
+	jsonData, err := json.Marshal(original)
+	if err != nil {
+		t.Fatalf("Failed to marshal to JSON: %v", err)
+	}
+
+	var unmarshaled Tracing
+	if err := json.Unmarshal(jsonData, &unmarshaled); err != nil {
+		t.Fatalf("Failed to unmarshal from JSON: %v", err)
+	}
+
+	if unmarshaled.SpanName != original.SpanName {
+		t.Errorf("Expected SpanName %s, got %s", original.SpanName, unmarshaled.SpanName)
+	}
+
+	if len(unmarshaled.SpanAttributes) != len(original.SpanAttributes) {
+		t.Errorf("Expected %d span attributes, got %d", len(original.SpanAttributes), len(unmarshaled.SpanAttributes))
+	}
+
+	for key, expectedValue := range original.SpanAttributes {
+		if actualValue, exists := unmarshaled.SpanAttributes[key]; !exists {
+			t.Errorf("Expected span attribute %s to exist", key)
+		} else if actualValue != expectedValue {
+			t.Errorf("Expected span attribute %s = %s, got %s", key, expectedValue, actualValue)
+		}
+	}
+
+	t.Logf("JSON representation: %s", string(jsonData))
+}
+
+func TestTracing_OpenTelemetry_Span_Attributes(t *testing.T) {
+	// Create an in-memory span recorder to capture actual span data
+	spanRecorder := tracetest.NewSpanRecorder()
+	provider := trace.NewTracerProvider(
+		trace.WithSpanProcessor(spanRecorder),
+	)
+
+	// Create our tracing module with span attributes that include placeholders
+	ot := &Tracing{
+		SpanName: "test-span",
+		SpanAttributes: map[string]string{
+			"static":               "test-service",
+			"request-placeholder":  "{http.request.method}",
+			"response-placeholder": "{http.response.header.X-Some-Header}",
+			"mixed":                "prefix-{http.request.method}-{http.response.header.X-Some-Header}",
+		},
+	}
+
+	// Create a specific request to test against
+	req, _ := http.NewRequest("POST", "https://api.example.com/v1/users?id=123", nil)
+	req.Host = "api.example.com"
+
+	w := httptest.NewRecorder()
+
+	// Set up the replacer
+	repl := caddy.NewReplacer()
+	ctx := context.WithValue(req.Context(), caddy.ReplacerCtxKey, repl)
+	ctx = context.WithValue(ctx, caddyhttp.VarsCtxKey, make(map[string]any))
+	req = req.WithContext(ctx)
+
+	// Set up request placeholders
+	repl.Set("http.request.method", req.Method)
+	repl.Set("http.request.uri", req.URL.RequestURI())
+
+	// Handler to generate the response
+	var handler caddyhttp.HandlerFunc = func(writer http.ResponseWriter, request *http.Request) error {
+		writer.Header().Set("X-Some-Header", "some-value")
+		writer.WriteHeader(200)
+
+		// Make response headers available to replacer
+		repl.Set("http.response.header.X-Some-Header", writer.Header().Get("X-Some-Header"))
+
+		return nil
+	}
+
+	// Set up Caddy context
+	caddyCtx, cancel := caddy.NewContext(caddy.Context{Context: context.Background()})
+	defer cancel()
+
+	// Override the global tracer provider with our test provider
+	// This is a bit hacky but necessary to capture the actual spans
+	originalProvider := globalTracerProvider
+	globalTracerProvider = &tracerProvider{
+		tracerProvider:         provider,
+		tracerProvidersCounter: 1, // Simulate one user
+	}
+	defer func() {
+		globalTracerProvider = originalProvider
+	}()
+
+	// Provision the tracing module
+	if err := ot.Provision(caddyCtx); err != nil {
+		t.Errorf("Provision error: %v", err)
+		t.FailNow()
+	}
+
+	// Execute the request
+	if err := ot.ServeHTTP(w, req, handler); err != nil {
+		t.Errorf("ServeHTTP error: %v", err)
+	}
+
+	// Get the recorded spans
+	spans := spanRecorder.Ended()
+	if len(spans) == 0 {
+		t.Fatal("Expected at least one span to be recorded")
+	}
+
+	// Find our span (should be the one with our test span name)
+	var testSpan trace.ReadOnlySpan
+	for _, span := range spans {
+		if span.Name() == "test-span" {
+			testSpan = span
+			break
+		}
+	}
+
+	if testSpan == nil {
+		t.Fatal("Could not find test span in recorded spans")
+	}
+
+	// Verify that the span attributes were set correctly with placeholder replacement
+	expectedAttributes := map[string]string{
+		"static":               "test-service",
+		"request-placeholder":  "POST",
+		"response-placeholder": "some-value",
+		"mixed":                "prefix-POST-some-value",
+	}
+
+	actualAttributes := make(map[string]string)
+	for _, attr := range testSpan.Attributes() {
+		actualAttributes[string(attr.Key)] = attr.Value.AsString()
+	}
+
+	for key, expectedValue := range expectedAttributes {
+		if actualValue, exists := actualAttributes[key]; !exists {
+			t.Errorf("Expected span attribute %s to be set", key)
+		} else if actualValue != expectedValue {
+			t.Errorf("Expected span attribute %s = %s, got %s", key, expectedValue, actualValue)
+		}
+	}
+
+	t.Logf("Recorded span attributes: %+v", actualAttributes)
+}
+
 func createRequestWithContext(method string, url string) *http.Request {
 	r, _ := http.NewRequest(method, url, nil)
 	repl := caddy.NewReplacer()

modules/caddyhttp/tracing/tracer.go

@@ -8,6 +8,7 @@ import (
 	"go.opentelemetry.io/contrib/exporters/autoexport"
 	"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
 	"go.opentelemetry.io/contrib/propagators/autoprop"
+	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/propagation"
 	"go.opentelemetry.io/otel/sdk/resource"
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
@@ -37,20 +38,23 @@ type openTelemetryWrapper struct {
 
 	handler http.Handler
 
-	spanName string
+	spanName       string
+	spanAttributes map[string]string
 }
 
 // newOpenTelemetryWrapper is responsible for the openTelemetryWrapper initialization using provided configuration.
 func newOpenTelemetryWrapper(
 	ctx context.Context,
 	spanName string,
+	spanAttributes map[string]string,
 ) (openTelemetryWrapper, error) {
 	if spanName == "" {
 		spanName = defaultSpanName
 	}
 
 	ot := openTelemetryWrapper{
-		spanName: spanName,
+		spanName:       spanName,
+		spanAttributes: spanAttributes,
 	}
 
 	version, _ := caddy.Version()
@@ -99,8 +103,22 @@ func (ot *openTelemetryWrapper) serveHTTP(w http.ResponseWriter, r *http.Request
 			extra.Add(zap.String("spanID", spanID))
 		}
 	}
+
 	next := ctx.Value(nextCallCtxKey).(*nextCall)
 	next.err = next.next.ServeHTTP(w, r)
+
+	// Add custom span attributes to the current span
+	span := trace.SpanFromContext(ctx)
+	if span.IsRecording() && len(ot.spanAttributes) > 0 {
+		replacer := ctx.Value(caddy.ReplacerCtxKey).(*caddy.Replacer)
+		attributes := make([]attribute.KeyValue, 0, len(ot.spanAttributes))
+		for key, value := range ot.spanAttributes {
+			// Allow placeholder replacement in attribute values
+			replacedValue := replacer.ReplaceAll(value, "")
+			attributes = append(attributes, attribute.String(key, replacedValue))
+		}
+		span.SetAttributes(attributes...)
+	}
 }
 
 // ServeHTTP propagates call to the by wrapped by `otelhttp` next handler.

modules/caddyhttp/tracing/tracer_test.go

@@ -16,6 +16,7 @@ func TestOpenTelemetryWrapper_newOpenTelemetryWrapper(t *testing.T) {
 
 	if otw, err = newOpenTelemetryWrapper(ctx,
 		"",
+		nil,
 	); err != nil {
 		t.Errorf("newOpenTelemetryWrapper() error = %v", err)
 		t.FailNow()