From 10404e669998ab60af983015bf8493b461742545 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Hild=C3=A9n?= Date: Sat, 20 Sep 2025 19:43:35 +0300 Subject: [PATCH 1/3] WIP tracing span attributes --- README.md | 9 +- modules/caddyhttp/tracing/module.go | 37 ++++- modules/caddyhttp/tracing/module_test.go | 184 ++++++++++++++++++++++- modules/caddyhttp/tracing/tracer.go | 22 ++- modules/caddyhttp/tracing/tracer_test.go | 1 + 5 files changed, 240 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 4c091f71410..a54dd438dde 100644 --- a/README.md +++ b/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 ``` diff --git a/modules/caddyhttp/tracing/module.go b/modules/caddyhttp/tracing/module.go index 85fd630020e..c312bfd8a2b 100644 --- a/modules/caddyhttp/tracing/module.go +++ b/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_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 diff --git a/modules/caddyhttp/tracing/module_test.go b/modules/caddyhttp/tracing/module_test.go index 2a775fc18b2..2cbbad74327 100644 --- a/modules/caddyhttp/tracing/module_test.go +++ b/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() diff --git a/modules/caddyhttp/tracing/tracer.go b/modules/caddyhttp/tracing/tracer.go index 261952aa65e..8e4b1162fcf 100644 --- a/modules/caddyhttp/tracing/tracer.go +++ b/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) } diff --git a/modules/caddyhttp/tracing/tracer_test.go b/modules/caddyhttp/tracing/tracer_test.go index 36a32ff46e0..5ca423aa9cf 100644 --- a/modules/caddyhttp/tracing/tracer_test.go +++ b/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() From 168f0e02b854aba91a1ffb0a2d424380cd6978ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Hild=C3=A9n?= Date: Sat, 20 Sep 2025 19:57:24 +0300 Subject: [PATCH 2/3] better test --- modules/caddyhttp/tracing/module_test.go | 156 ++++++++++++++--------- 1 file changed, 93 insertions(+), 63 deletions(-) diff --git a/modules/caddyhttp/tracing/module_test.go b/modules/caddyhttp/tracing/module_test.go index 2cbbad74327..e0975a9adad 100644 --- a/modules/caddyhttp/tracing/module_test.go +++ b/modules/caddyhttp/tracing/module_test.go @@ -9,6 +9,9 @@ import ( "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" @@ -240,18 +243,65 @@ func TestTracing_ServeHTTP_Next_Error(t *testing.T) { } } -func TestTracing_Span_Attributes_With_Placeholders(t *testing.T) { - ot := &Tracing{ +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{ - "http.method": "{http.request.method}", "service.name": "test-service", - "mixed.attribute": "prefix-{http.request.method}-suffix", + "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{ + "placeholder": "{http.request.method}", + "static": "test-service", + "mixed": "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, _ := http.NewRequest("POST", "https://api.example.com/v1/users?id=123", nil) req.Host = "api.example.com" // Set up the request context with proper replacer and vars @@ -259,23 +309,32 @@ func TestTracing_Span_Attributes_With_Placeholders(t *testing.T) { 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 + // Handler that ensures the request gets processed var handler caddyhttp.HandlerFunc = func(writer http.ResponseWriter, request *http.Request) error { - // Just ensure the request gets processed writer.WriteHeader(200) 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() @@ -286,75 +345,46 @@ func TestTracing_Span_Attributes_With_Placeholders(t *testing.T) { 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", + // Get the recorded spans + spans := spanRecorder.Ended() + if len(spans) == 0 { + t.Fatal("Expected at least one span to be recorded") } - 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) + // 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 } } - // 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", + if testSpan == nil { + t.Fatal("Could not find test span in recorded spans") } - 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) - } + // Verify that the span attributes were set correctly with placeholder replacement + expectedAttributes := map[string]string{ + "placeholder": "POST", + "static": "test-service", + "mixed": "prefix-POST-suffix", } -} -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", - }, + actualAttributes := make(map[string]string) + for _, attr := range testSpan.Attributes() { + actualAttributes[string(attr.Key)] = attr.Value.AsString() } - 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) + 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("JSON representation: %s", string(jsonData)) + t.Logf("Recorded span attributes: %+v", actualAttributes) } func createRequestWithContext(method string, url string) *http.Request { From d28346cbdd7bc9a59d9f918df6357364972a00ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Hild=C3=A9n?= Date: Sun, 21 Sep 2025 15:22:40 +0300 Subject: [PATCH 3/3] only write attributes after other middleware (and request) --- modules/caddyhttp/tracing/module_test.go | 7 ++++--- modules/caddyhttp/tracing/tracer.go | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/modules/caddyhttp/tracing/module_test.go b/modules/caddyhttp/tracing/module_test.go index e0975a9adad..0e677334a9c 100644 --- a/modules/caddyhttp/tracing/module_test.go +++ b/modules/caddyhttp/tracing/module_test.go @@ -366,9 +366,10 @@ func TestTracing_OpenTelemetry_Span_Attributes(t *testing.T) { // Verify that the span attributes were set correctly with placeholder replacement expectedAttributes := map[string]string{ - "placeholder": "POST", - "static": "test-service", - "mixed": "prefix-POST-suffix", + "placeholder": "POST", + "static": "test-service", + "mixed": "prefix-POST-suffix", + "http.response.status_code": "200", // OTEL default } actualAttributes := make(map[string]string) diff --git a/modules/caddyhttp/tracing/tracer.go b/modules/caddyhttp/tracing/tracer.go index 8e4b1162fcf..d4b7c1d4c7e 100644 --- a/modules/caddyhttp/tracing/tracer.go +++ b/modules/caddyhttp/tracing/tracer.go @@ -104,6 +104,9 @@ func (ot *openTelemetryWrapper) serveHTTP(w http.ResponseWriter, r *http.Request } } + 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 { @@ -116,9 +119,6 @@ func (ot *openTelemetryWrapper) serveHTTP(w http.ResponseWriter, r *http.Request } span.SetAttributes(attributes...) } - - next := ctx.Value(nextCallCtxKey).(*nextCall) - next.err = next.next.ServeHTTP(w, r) } // ServeHTTP propagates call to the by wrapped by `otelhttp` next handler.