Merge 1898a907fe into bfdb04912d

README.md

@@ -117,11 +117,18 @@ username ALL=(ALL:ALL) NOPASSWD: /usr/sbin/setcap
 
 replacing `username` with your actual username. Please be careful and only do this if you know what you are doing! We are only qualified to document how to use Caddy, not Go tooling or your computer, and we are providing these instructions for convenience only; please learn how to use your own computer at your own risk and make any needful adjustments.
 
+Then you can run the tests in all modules or a specific one:
+
+````bash
+$ go test ./...
+$ go test ./modules/caddyhttp/tracing/
+```
+
 ### With version information and/or plugins
 
 Using [our builder tool, `xcaddy`](https://github.com/caddyserver/xcaddy)...
 
-```
+```bash
 $ xcaddy build
 ```
 

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()