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)