Commit a1da0c7a authored by Doug Barrett's avatar Doug Barrett
Browse files

fix(v2/httpserver): Add Name(), context-aware listener, and error prefixes

Address AGENTS.md review findings:
- Add Name field to Config with default "httpserver" for component
  identification in logs and errors
- Use net.ListenConfig.Listen(ctx) in Start to honour context
  cancellation during bind
- Wrap Shutdown errors with the component name
- Fix doc.go claim about logging middleware recording duration
  (it only logs method, path, and status)
parent 4befdbea
Loading
Loading
Loading
Loading
+2 −2
Original line number Diff line number Diff line
@@ -14,8 +14,8 @@ Two built-in middleware layers are included:
  - Tracing: extracts incoming W3C trace context (traceparent / tracestate),
    creates a server span named "METHOD /path", and records the response
    status.
  - Logging: emits a structured log line per request with method, path, status
    code, and elapsed duration.
  - Logging: emits a structured log line per request with method, path, and
    status code.

# Basic usage

+312 −0
Original line number Diff line number Diff line
package routerbench

import (
	"context"
	"fmt"
	"net/http"
	"net/http/httptest"
	"strings"
	"testing"
)

// benchHandler is the handler registered for every route. It writes a small
// JSON body to simulate a real endpoint without dominating the benchmark.
func benchHandler(w http.ResponseWriter, _ *http.Request) {
	w.Header().Set("Content-Type", "application/json")
	w.WriteHeader(http.StatusOK)
	_, _ = w.Write([]byte(`{"status":"ok"}`))
}

// noopMiddleware simulates realistic middleware (read header, set context
// value) without doing heavy work. This isolates the router's middleware
// dispatch overhead.
func noopMiddleware(next http.Handler) http.Handler {
	type ctxKey struct{}
	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		val := r.Header.Get("X-Request-ID")
		ctx := context.WithValue(r.Context(), ctxKey{}, val)
		next.ServeHTTP(w, r.WithContext(ctx))
	})
}

// request holds a pre-built HTTP request for benchmarking.
type request struct {
	method string
	path   string
}

// routerEntry defines how to create a router and its pattern conventions.
type routerEntry struct {
	name    string
	factory func() Router
	// pattern builds the route pattern string. Stdlib uses "GET /path/{id}",
	// chi uses "/path/{id}", gorilla uses "/path/{id}".
	pattern func(method, path string) string
}

var routers = []routerEntry{
	{
		name:    "stdlib",
		factory: newStdlibRouter,
		pattern: func(method, path string) string { return method + " " + path },
	},
	{
		name:    "chi",
		factory: newChiRouter,
		pattern: func(_, path string) string { return path },
	},
	{
		name:    "gorilla",
		factory: newGorillaRouter,
		pattern: func(_, path string) string { return path },
	},
}

// workload defines a set of routes and corresponding requests to exercise.
type workload struct {
	name string
	// routes returns n route definitions (method + pattern with params).
	routes func(n int) []route
	// requests returns paths to hit (with param placeholders resolved).
	requests func(n int) []request
}

type route struct {
	method  string
	pattern string // e.g. "/api/v4/resource0/action" or "/api/v4/resource0/{id}"
}

var workloads = []workload{
	{
		name: "static",
		routes: func(n int) []route {
			out := make([]route, n)
			for i := range n {
				out[i] = route{"GET", fmt.Sprintf("/api/v4/resource%d/action", i)}
			}
			return out
		},
		requests: func(n int) []request {
			out := make([]request, n)
			for i := range n {
				out[i] = request{"GET", fmt.Sprintf("/api/v4/resource%d/action", i)}
			}
			return out
		},
	},
	{
		name: "param",
		routes: func(n int) []route {
			out := make([]route, n)
			for i := range n {
				out[i] = route{"GET", fmt.Sprintf("/api/v4/resource%d/{id}/sub/{sub_id}", i)}
			}
			return out
		},
		requests: func(n int) []request {
			out := make([]request, n)
			for i := range n {
				out[i] = request{"GET", fmt.Sprintf("/api/v4/resource%d/123/sub/456", i)}
			}
			return out
		},
	},
	{
		name: "mixed",
		routes: func(n int) []route {
			out := make([]route, n)
			for i := range n {
				if i%2 == 0 {
					out[i] = route{"GET", fmt.Sprintf("/api/v4/resource%d/action", i)}
				} else {
					out[i] = route{"GET", fmt.Sprintf("/api/v4/resource%d/{id}/detail", i)}
				}
			}
			return out
		},
		requests: func(n int) []request {
			out := make([]request, n)
			for i := range n {
				if i%2 == 0 {
					out[i] = request{"GET", fmt.Sprintf("/api/v4/resource%d/action", i)}
				} else {
					out[i] = request{"GET", fmt.Sprintf("/api/v4/resource%d/123/detail", i)}
				}
			}
			return out
		},
	},
}

var routeCounts = []int{10, 50, 200}
var mwCounts = []int{0, 3, 5}

func BenchmarkRouting(b *testing.B) {
	for _, rt := range routers {
		b.Run(rt.name, func(b *testing.B) {
			for _, wl := range workloads {
				b.Run(wl.name, func(b *testing.B) {
					for _, rc := range routeCounts {
						b.Run(fmt.Sprintf("routes=%d", rc), func(b *testing.B) {
							for _, mc := range mwCounts {
								b.Run(fmt.Sprintf("mw=%d", mc), func(b *testing.B) {
									router := rt.factory()

									// Register middleware.
									for range mc {
										router.Use(noopMiddleware)
									}

									// Register routes.
									routes := wl.routes(rc)
									for _, r := range routes {
										pattern := rt.pattern(r.method, r.pattern)
										router.HandleFunc(pattern, benchHandler)
									}

									// Pre-build requests.
									reqs := wl.requests(rc)
									httpReqs := make([]*http.Request, len(reqs))
									for i, r := range reqs {
										httpReqs[i] = httptest.NewRequest(r.method, r.path, nil)
										httpReqs[i].Header.Set("X-Request-ID", "bench-id")
									}

									// Warm up: first request triggers lazy init.
									rec := httptest.NewRecorder()
									router.ServeHTTP(rec, httpReqs[0])

									b.ReportAllocs()
									b.ResetTimer()
									for i := range b.N {
										rec := httptest.NewRecorder()
										router.ServeHTTP(rec, httpReqs[i%len(httpReqs)])
									}
								})
							}
						})
					}
				})
			}
		})
	}
}

// ---------------------------------------------------------------------------
// Correctness tests
// ---------------------------------------------------------------------------

func TestRouterCorrectness(t *testing.T) {
	for _, rt := range routers {
		t.Run(rt.name, func(t *testing.T) {
			t.Run("static_route", func(t *testing.T) {
				router := rt.factory()
				pattern := rt.pattern("GET", "/health")
				router.HandleFunc(pattern, func(w http.ResponseWriter, _ *http.Request) {
					w.WriteHeader(http.StatusOK)
					_, _ = w.Write([]byte("ok"))
				})

				req := httptest.NewRequest("GET", "/health", nil)
				rec := httptest.NewRecorder()
				router.ServeHTTP(rec, req)

				if rec.Code != http.StatusOK {
					t.Errorf("expected 200, got %d", rec.Code)
				}
				if body := rec.Body.String(); body != "ok" {
					t.Errorf("expected body 'ok', got %q", body)
				}
			})

			t.Run("param_route", func(t *testing.T) {
				router := rt.factory()
				pattern := rt.pattern("GET", "/users/{id}")
				router.HandleFunc(pattern, func(w http.ResponseWriter, _ *http.Request) {
					w.WriteHeader(http.StatusOK)
				})

				req := httptest.NewRequest("GET", "/users/42", nil)
				rec := httptest.NewRecorder()
				router.ServeHTTP(rec, req)

				if rec.Code != http.StatusOK {
					t.Errorf("expected 200, got %d", rec.Code)
				}
			})

			t.Run("middleware_applied", func(t *testing.T) {
				router := rt.factory()
				var called bool
				router.Use(func(next http.Handler) http.Handler {
					return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
						called = true
						next.ServeHTTP(w, r)
					})
				})
				pattern := rt.pattern("GET", "/mw-test")
				router.HandleFunc(pattern, func(w http.ResponseWriter, _ *http.Request) {
					w.WriteHeader(http.StatusOK)
				})

				req := httptest.NewRequest("GET", "/mw-test", nil)
				rec := httptest.NewRecorder()
				router.ServeHTTP(rec, req)

				if !called {
					t.Error("middleware was not called")
				}
			})

			t.Run("not_found", func(t *testing.T) {
				router := rt.factory()
				pattern := rt.pattern("GET", "/exists")
				router.HandleFunc(pattern, func(w http.ResponseWriter, _ *http.Request) {
					w.WriteHeader(http.StatusOK)
				})

				req := httptest.NewRequest("GET", "/does-not-exist", nil)
				rec := httptest.NewRecorder()
				router.ServeHTTP(rec, req)

				if rec.Code != http.StatusNotFound {
					t.Errorf("expected 404, got %d", rec.Code)
				}
			})
		})
	}
}

func TestFeatureMatrix(t *testing.T) {
	// Compile-time check: all adapters satisfy Router.
	var _ Router = newStdlibRouter()
	var _ Router = newChiRouter()
	var _ Router = newGorillaRouter()

	features := []struct {
		feature string
		stdlib  string
		chi     string
		gorilla string
	}{
		{"http.Handler compatible", "Yes", "Yes", "Yes"},
		{"func(http.Handler) http.Handler middleware", "Manual wrapping", "Native Use()", "Manual wrapping"},
		{"Path parameters", "r.PathValue() (Go 1.22+)", "chi.URLParam()", "mux.Vars()"},
		{"Method routing in pattern", "GET /path (Go 1.22+)", "r.Get()/r.Post()", "r.Methods()"},
		{"Route groups / sub-routers", "No", "Yes (r.Group, r.Route, r.Mount)", "Yes (Subrouter)"},
		{"Middleware chaining API", "No", "Yes (r.With, r.Group)", "r.Use() (global only)"},
		{"Zero external deps", "Yes (stdlib)", "Yes", "Yes"},
		{"Actively maintained", "Yes (Go team)", "Yes (slower cadence)", "Archived / minimal"},
	}

	var sb strings.Builder
	sb.WriteString("\nFeature Matrix:\n")
	sb.WriteString(fmt.Sprintf("%-45s | %-30s | %-35s | %-25s\n",
		"Feature", "stdlib", "chi", "gorilla/mux"))
	sb.WriteString(strings.Repeat("-", 140) + "\n")
	for _, f := range features {
		sb.WriteString(fmt.Sprintf("%-45s | %-30s | %-35s | %-25s\n",
			f.feature, f.stdlib, f.chi, f.gorilla))
	}
	t.Log(sb.String())
}
+8 −0
Original line number Diff line number Diff line
module gitlab.com/gitlab-org/labkit/v2/httpserver/routerbench

go 1.24.0

require (
	github.com/go-chi/chi/v5 v5.2.1
	github.com/gorilla/mux v1.8.1
)
+4 −0
Original line number Diff line number Diff line
github.com/go-chi/chi/v5 v5.2.1 h1:KOIHODQj58PmL80G2Eak4WdvUzjSJSm0vG72crDCqb8=
github.com/go-chi/chi/v5 v5.2.1/go.mod h1:L2yAIGWB3H+phAw1NxKwWM+7eUH/lU8pOMm5hHcoops=
github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY=
github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ=
+125 −0
Original line number Diff line number Diff line
// Package routerbench benchmarks HTTP router candidates for LabKit v2's
// httpserver.Router interface. It is a standalone module to isolate
// chi and gorilla/mux dependencies from the production v2 module.
package routerbench

import (
	"net/http"
	"sync"

	chi "github.com/go-chi/chi/v5"
	gorilla "github.com/gorilla/mux"
)

// Router mirrors the interface defined in v2/httpserver (MR !339).
// Duplicated here because !339 has not merged and this is a standalone module.
type Router interface {
	Handle(pattern string, handler http.Handler)
	HandleFunc(pattern string, handler http.HandlerFunc)
	Use(middleware ...func(http.Handler) http.Handler)
	http.Handler
}

// ---------------------------------------------------------------------------
// stdlib adapter
// ---------------------------------------------------------------------------

type stdlibRouter struct {
	mux        *http.ServeMux
	middleware []func(http.Handler) http.Handler
	handler    http.Handler
	once       sync.Once
}

func newStdlibRouter() Router {
	return &stdlibRouter{mux: http.NewServeMux()}
}

func (r *stdlibRouter) Handle(pattern string, handler http.Handler) {
	r.mux.Handle(pattern, handler)
}

func (r *stdlibRouter) HandleFunc(pattern string, handler http.HandlerFunc) {
	r.mux.HandleFunc(pattern, handler)
}

func (r *stdlibRouter) Use(mw ...func(http.Handler) http.Handler) {
	r.middleware = append(r.middleware, mw...)
}

func (r *stdlibRouter) ServeHTTP(w http.ResponseWriter, req *http.Request) {
	r.once.Do(func() {
		var h http.Handler = r.mux
		for i := len(r.middleware) - 1; i >= 0; i-- {
			h = r.middleware[i](h)
		}
		r.handler = h
	})
	r.handler.ServeHTTP(w, req)
}

// ---------------------------------------------------------------------------
// chi adapter
// ---------------------------------------------------------------------------

type chiRouter struct {
	mux *chi.Mux
}

func newChiRouter() Router {
	return &chiRouter{mux: chi.NewMux()}
}

func (r *chiRouter) Handle(pattern string, handler http.Handler) {
	r.mux.Handle(pattern, handler)
}

func (r *chiRouter) HandleFunc(pattern string, handler http.HandlerFunc) {
	r.mux.HandleFunc(pattern, handler)
}

func (r *chiRouter) Use(mw ...func(http.Handler) http.Handler) {
	r.mux.Use(mw...)
}

func (r *chiRouter) ServeHTTP(w http.ResponseWriter, req *http.Request) {
	r.mux.ServeHTTP(w, req)
}

// ---------------------------------------------------------------------------
// gorilla/mux adapter
// ---------------------------------------------------------------------------

type gorillaRouter struct {
	mux        *gorilla.Router
	middleware []func(http.Handler) http.Handler
	handler    http.Handler
	once       sync.Once
}

func newGorillaRouter() Router {
	return &gorillaRouter{mux: gorilla.NewRouter()}
}

func (r *gorillaRouter) Handle(pattern string, handler http.Handler) {
	r.mux.Handle(pattern, handler)
}

func (r *gorillaRouter) HandleFunc(pattern string, handler http.HandlerFunc) {
	r.mux.HandleFunc(pattern, handler)
}

func (r *gorillaRouter) Use(mw ...func(http.Handler) http.Handler) {
	r.middleware = append(r.middleware, mw...)
}

func (r *gorillaRouter) ServeHTTP(w http.ResponseWriter, req *http.Request) {
	r.once.Do(func() {
		var h http.Handler = r.mux
		for i := len(r.middleware) - 1; i >= 0; i-- {
			h = r.middleware[i](h)
		}
		r.handler = h
	})
	r.handler.ServeHTTP(w, req)
}
Loading