From 63f4e387c33e42864ad80ed704595299c3f8128b Mon Sep 17 00:00:00 2001 From: Julien Voisin Date: Tue, 11 Aug 2026 13:21:33 +0200 Subject: [PATCH] caddyhttp: allocate the request UUID lazily instead of on every request (#7936) addHTTPVarsToReplacer ran SetVar(ctx, "uuid", new(requestID)) for every request during replacer setup, allocating a *requestID unconditionally even though it is only ever read by the {http.request.uuid} placeholder. The UUID value itself was already generated lazily (requestID.String caches on first call), but the container was not. Allocate the *requestID on first access instead: the uuid placeholder now creates and stores it in the request's vars table when absent, so repeated references within a request still share one instance (and thus one UUID), while requests that never reference the UUID pay nothing. Add a benchmark for the per-request replacer setup and a regression test for the uuid placeholder, which previously had no coverage. Benchmark (BenchmarkAddHTTPVarsToReplacer, linux/arm64, count=6), common path where the UUID is never referenced: before: ~700 ns/op 352 B/op 9 allocs/op after: ~690 ns/op 336 B/op 8 allocs/op One 16-byte allocation removed per request; a GC-pressure win rather than a latency one. --- modules/caddyhttp/replacer.go | 11 ++++++--- modules/caddyhttp/replacer_test.go | 39 ++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/modules/caddyhttp/replacer.go b/modules/caddyhttp/replacer.go index 65f9dd475..c510938c1 100644 --- a/modules/caddyhttp/replacer.go +++ b/modules/caddyhttp/replacer.go @@ -57,7 +57,6 @@ func NewTestReplacer(req *http.Request) *caddy.Replacer { func addHTTPVarsToReplacer(repl *caddy.Replacer, req *http.Request, w http.ResponseWriter) { SetVar(req.Context(), "start_time", time.Now()) - SetVar(req.Context(), "uuid", new(requestID)) httpVars := func(key string) (any, bool) { if req != nil { @@ -213,8 +212,14 @@ func addHTTPVarsToReplacer(repl *caddy.Replacer, req *http.Request, w http.Respo return time.Since(start).Seconds() * 1e3, true // multiply seconds to preserve decimal (see #4666) case "http.request.uuid": - // fetch the UUID for this request - id := GetVar(req.Context(), "uuid").(*requestID) + // fetch the UUID for this request, generating and caching it + // on first access so requests that never reference the UUID + // don't pay for the allocation + id, ok := GetVar(req.Context(), "uuid").(*requestID) + if !ok { + id = new(requestID) + SetVar(req.Context(), "uuid", id) + } // set it to this request's access log extra := req.Context().Value(ExtraLogFieldsCtxKey).(*ExtraLogFields) diff --git a/modules/caddyhttp/replacer_test.go b/modules/caddyhttp/replacer_test.go index 4f8d8f0b2..b77b599dc 100644 --- a/modules/caddyhttp/replacer_test.go +++ b/modules/caddyhttp/replacer_test.go @@ -296,3 +296,42 @@ func TestHTTPProtoNameNormalization(t *testing.T) { } } } + +// BenchmarkAddHTTPVarsToReplacer measures the per-request replacer setup, which +// is the common path where the request UUID is never referenced. +func BenchmarkAddHTTPVarsToReplacer(b *testing.B) { + req := httptest.NewRequest(http.MethodGet, "http://example.com/foo?a=b", nil) + req.Header.Set("User-Agent", "test-agent") + ctx := context.WithValue(req.Context(), VarsCtxKey, make(map[string]any)) + ctx = context.WithValue(ctx, ExtraLogFieldsCtxKey, new(ExtraLogFields)) + req = req.WithContext(ctx) + + b.ReportAllocs() + for b.Loop() { + repl := caddy.NewReplacer() + addHTTPVarsToReplacer(repl, req, nil) + } +} + +// TestHTTPVarReplacementUUID verifies the lazily-allocated request UUID is +// generated on first access and stays stable across references. +func TestHTTPVarReplacementUUID(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) + repl := caddy.NewReplacer() + ctx := context.WithValue(req.Context(), caddy.ReplacerCtxKey, repl) + ctx = context.WithValue(ctx, VarsCtxKey, make(map[string]any)) + ctx = context.WithValue(ctx, ExtraLogFieldsCtxKey, new(ExtraLogFields)) + req = req.WithContext(ctx) + addHTTPVarsToReplacer(repl, req, nil) + + first, ok := repl.GetString("http.request.uuid") + if !ok || first == "" { + t.Fatalf("expected a non-empty uuid, got %q (ok=%t)", first, ok) + } + + second, _ := repl.GetString("http.request.uuid") + if first != second { + t.Errorf("expected stable uuid across references: %q != %q", first, second) + } +} +