WIP tracing span attributes

Author: felix-hilden

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,6 +2,7 @@ package tracing
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"net/http"
 	"net/http/httptest"
@@ -15,17 +16,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 +52,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 +81,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 +118,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 +240,123 @@ func TestTracing_ServeHTTP_Next_Error(t *testing.T) {
 	}
 }
 
+func TestTracing_Span_Attributes_With_Placeholders(t *testing.T) {
+	ot := &Tracing{
+		SpanName: "test-span",
+		SpanAttributes: map[string]string{
+			"http.method":     "{http.request.method}",
+			"service.name":    "test-service",
+			"mixed.attribute": "prefix-{http.request.method}-suffix",
+		},
+	}
+
+	// Create a specific request to test against
+	req, _ := http.NewRequest("POST", "https://api.example.com/v1/users?id=123&action=create", nil)
+	req.Host = "api.example.com"
+
+	// Set up the request context with proper replacer and vars
+	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)
+
+	// Manually populate the HTTP variables that would normally be set by the server
+	// This simulates what addHTTPVarsToReplacer would do
+	repl.Set("http.request.method", req.Method)
+
+	w := httptest.NewRecorder()
+
+	// Handler that can verify the context and span attributes
+	var handler caddyhttp.HandlerFunc = func(writer http.ResponseWriter, request *http.Request) error {
+		// Just ensure the request gets processed
+		writer.WriteHeader(200)
+		return nil
+	}
+
+	caddyCtx, cancel := caddy.NewContext(caddy.Context{Context: context.Background()})
+	defer cancel()
+
+	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)
+	}
+
+	// Verify that the span attributes were configured correctly in the otel wrapper
+	expectedRawAttrs := map[string]string{
+		"http.method":     "{http.request.method}",
+		"service.name":    "test-service",
+		"mixed.attribute": "prefix-{http.request.method}-suffix",
+	}
+
+	for key, expectedValue := range expectedRawAttrs {
+		if actualValue, exists := ot.otel.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)
+		}
+	}
+
+	// Now test that the replacement would work correctly if called directly
+	// This verifies that our placeholder values would be replaced correctly
+	expectedReplacements := map[string]string{
+		"{http.request.method}":               "POST",
+		"service.name":                        "service.name",
+		"prefix-{http.request.method}-suffix": "prefix-POST-suffix",
+	}
+
+	for placeholder, expected := range expectedReplacements {
+		replaced := repl.ReplaceAll(placeholder, "")
+		if replaced != expected {
+			t.Errorf("Expected %s to be replaced with %s, got %s", placeholder, expected, replaced)
+		}
+	}
+}
+
+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 createRequestWithContext(method string, url string) *http.Request {
 	r, _ := http.NewRequest(method, url, nil)
 	repl := caddy.NewReplacer()

modules/caddyhttp/tracing/tracer.go

@@ -7,6 +7,7 @@ import (
 
 	"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
 	"go.opentelemetry.io/contrib/propagators/autoprop"
+	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc"
 	"go.opentelemetry.io/otel/propagation"
 	"go.opentelemetry.io/otel/sdk/resource"
@@ -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,6 +103,20 @@ func (ot *openTelemetryWrapper) serveHTTP(w http.ResponseWriter, r *http.Request
 			extra.Add(zap.String("spanID", spanID))
 		}
 	}
+
+	// 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...)
+	}
+
 	next := ctx.Value(nextCallCtxKey).(*nextCall)
 	next.err = next.next.ServeHTTP(w, r)
 }

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