From f6f7704b3772d7e69e38812ed254993bc7e5999e Mon Sep 17 00:00:00 2001 From: Sung-Kyu Yoo Date: Sat, 25 Jul 2026 00:52:48 +0900 Subject: [PATCH] perf: skip log collector on hot path when admin API is disabled Build the admin.LogCollector only when admin is enabled and pass nil to gateway.New otherwise. The logging middleware is now installed only when a collector exists, so a disabled admin API no longer pays a per-request mutex lock + ring-buffer write that no consumer could read. Fixes #112 --- .../unreleased/Changed-20260725-000001.yaml | 5 +++ cmd/devcloud/main.go | 6 ++- internal/gateway/gateway.go | 38 +++++++++++-------- internal/gateway/gateway_test.go | 20 ++++++++++ 4 files changed, 52 insertions(+), 17 deletions(-) create mode 100644 changes/unreleased/Changed-20260725-000001.yaml diff --git a/changes/unreleased/Changed-20260725-000001.yaml b/changes/unreleased/Changed-20260725-000001.yaml new file mode 100644 index 0000000..4e9aff5 --- /dev/null +++ b/changes/unreleased/Changed-20260725-000001.yaml @@ -0,0 +1,5 @@ +kind: Changed +body: Skip request log collection on the AWS request hot path when the admin API is disabled (the default). The log collector is now built only when `admin.enabled` is true, avoiding a per-request mutex lock and ring-buffer write that no consumer could read. +time: 2026-07-25T00:00:01.000000+09:00 +custom: + Issue: "112" diff --git a/cmd/devcloud/main.go b/cmd/devcloud/main.go index 3031d0e..a984ae1 100644 --- a/cmd/devcloud/main.go +++ b/cmd/devcloud/main.go @@ -104,9 +104,13 @@ func main() { // opted in via admin.enabled. Otherwise expose a 404 handler so the admin // routes don't leak service internals. This binary serves no web UI; the // dashboard frontend lives in a separate repository and talks to this API. - logCollector := admin.NewLogCollector(1000) + // The log collector is only built when admin is enabled; otherwise no + // consumer can ever read it, so a nil collector keeps Add off the request + // hot path (see gateway.New). + var logCollector *admin.LogCollector adminHandler := http.NotFoundHandler() if cfg.Admin.Enabled { + logCollector = admin.NewLogCollector(1000) adminAPI := admin.NewAPI(registry, logCollector) hub := admin.NewHub(eventbus.New()) go hub.Start() diff --git a/internal/gateway/gateway.go b/internal/gateway/gateway.go index 6063c12..cfb506d 100644 --- a/internal/gateway/gateway.go +++ b/internal/gateway/gateway.go @@ -26,27 +26,33 @@ type Gateway struct { // - Requests starting with /devcloud/api/ → adminAPI handler // - Everything else → service router (AWS API) // -// A logging middleware wraps the service router and records each request to -// logCollector after the response has been written. +// When logCollector is non-nil, a logging middleware wraps the service router +// and records each request after the response has been written. A nil +// collector (admin API disabled) skips that wrapper entirely, keeping the +// mutex lock + ring-buffer write off the request hot path. func New(port int, registry *plugin.Registry, adminAPI http.Handler, logCollector *admin.LogCollector) *Gateway { router := NewServiceRouter(registry) - // Logging middleware: records AWS API requests to logCollector. - loggedRouter := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - start := time.Now() - rec := newStatusRecorder(w) - router.ServeHTTP(rec, r) - logCollector.Add(admin.RequestLog{ - Method: r.Method, - Path: r.URL.Path, - Status: rec.statusCode, - Duration: time.Since(start).String(), - Timestamp: start, - Service: detectService(r.URL.Path), + // Logging middleware: records AWS API requests to logCollector. Only + // installed when there is a collector to read them. + var serviceHandler http.Handler = router + if logCollector != nil { + serviceHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + start := time.Now() + rec := newStatusRecorder(w) + router.ServeHTTP(rec, r) + logCollector.Add(admin.RequestLog{ + Method: r.Method, + Path: r.URL.Path, + Status: rec.statusCode, + Duration: time.Since(start).String(), + Timestamp: start, + Service: detectService(r.URL.Path), + }) }) - }) + } - awsHandler := ChainMiddleware(loggedRouter, + awsHandler := ChainMiddleware(serviceHandler, ErrorRecoveryMiddleware, BodyLimitMiddleware, CORSMiddleware, diff --git a/internal/gateway/gateway_test.go b/internal/gateway/gateway_test.go index 9b8f9e2..81de638 100644 --- a/internal/gateway/gateway_test.go +++ b/internal/gateway/gateway_test.go @@ -46,6 +46,26 @@ func TestGateway_Integration(t *testing.T) { assert.Equal(t, "*", res.Header.Get("Access-Control-Allow-Origin"), "CORS middleware must set Access-Control-Allow-Origin") } +func TestGateway_NilLogCollector(t *testing.T) { + resp := &plugin.Response{ + StatusCode: http.StatusOK, + Headers: map[string]string{}, + Body: []byte(""), + ContentType: "application/xml", + } + reg := newRegistryWithStub("s3", resp) + + // admin disabled → nil collector: the request must succeed without the + // logging middleware (no panic, no Add). + gw := New(0, reg, http.NotFoundHandler(), nil) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/", nil) + gw.server.Handler.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) +} + func TestGateway_StartShutdown(t *testing.T) { reg := plugin.NewRegistry() lc := admin.NewLogCollector(100)