From 9251834343f7f9af68275cd0cb1f47c698b985b7 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 24 Feb 2026 22:44:59 -0500 Subject: [PATCH] fix: wire OTelMiddleware as global HTTP middleware so traces appear in Jaeger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `http.middleware.otel` module was instantiated but its `Process()` method was never called — it had no path into the HTTP handler chain. Two changes fix this: 1. `StandardHTTPRouter.AddGlobalMiddleware()` — new method that appends a middleware to a per-router slice. `ServeHTTP` now wraps the inner mux with all registered global middlewares before dispatching, so every request (health, metrics, pipeline triggers, and static routes) is observed. 2. `observability.otel-middleware` wiring hook (priority 100) — runs at startup and calls `AddGlobalMiddleware` for every `OTelMiddleware` found in the service registry, connecting it to every `StandardHTTPRouter`. The observability plugin manifest and tests are updated to reflect the new wiring hook (5 total) and the already-present `http.middleware.otel` module type (6 total). Co-Authored-By: Claude Opus 4.6 --- module/http_router.go | 41 ++++++++++++++++++++++------ plugins/observability/plugin.go | 1 + plugins/observability/plugin_test.go | 11 +++++--- plugins/observability/wiring.go | 28 +++++++++++++++++++ 4 files changed, 69 insertions(+), 12 deletions(-) diff --git a/module/http_router.go b/module/http_router.go index 88d0ef61..dd542576 100644 --- a/module/http_router.go +++ b/module/http_router.go @@ -48,11 +48,12 @@ type Route struct { // StandardHTTPRouter implements both HTTPRouter and http.Handler interfaces type StandardHTTPRouter struct { - name string - routes []Route - mu sync.RWMutex - serverDeps []string // Names of HTTP server modules this router depends on - serveMux *http.ServeMux + name string + routes []Route + mu sync.RWMutex + serverDeps []string // Names of HTTP server modules this router depends on + serveMux *http.ServeMux + globalMiddlewares []HTTPMiddleware } // NewStandardHTTPRouter creates a new HTTP router @@ -138,6 +139,17 @@ func (r *StandardHTTPRouter) AddRouteWithMiddleware(method, path string, handler } } +// AddGlobalMiddleware appends a middleware that wraps every request served by +// this router, regardless of which route is matched. Global middlewares are +// applied in the order they are added, before any per-route middlewares. +// This is the correct place to attach cross-cutting concerns such as +// distributed tracing that must observe all traffic. +func (r *StandardHTTPRouter) AddGlobalMiddleware(mw HTTPMiddleware) { + r.mu.Lock() + defer r.mu.Unlock() + r.globalMiddlewares = append(r.globalMiddlewares, mw) +} + // HasRoute checks if a route with the given method and path already exists func (r *StandardHTTPRouter) HasRoute(method, path string) bool { r.mu.RLock() @@ -150,16 +162,29 @@ func (r *StandardHTTPRouter) HasRoute(method, path string) bool { return false } -// ServeHTTP implements the http.Handler interface +// ServeHTTP implements the http.Handler interface. +// Global middlewares (e.g. OTEL tracing) are applied around the entire mux so +// every request — including health-checks and pipeline-triggered routes — is +// instrumented, regardless of how the route was registered. func (r *StandardHTTPRouter) ServeHTTP(w http.ResponseWriter, req *http.Request) { r.mu.RLock() defer r.mu.RUnlock() + var base http.Handler if r.serveMux != nil { - r.serveMux.ServeHTTP(w, req) + base = r.serveMux } else { - http.NotFound(w, req) + base = http.NotFoundHandler() } + + // Wrap with global middlewares in reverse registration order so that the + // first-added middleware is outermost (executes first). + handler := base + for i := len(r.globalMiddlewares) - 1; i >= 0; i-- { + handler = r.globalMiddlewares[i].Process(handler) + } + + handler.ServeHTTP(w, req) } // Start compiles all registered routes into the internal ServeMux. diff --git a/plugins/observability/plugin.go b/plugins/observability/plugin.go index 8e1cc242..04fac3ce 100644 --- a/plugins/observability/plugin.go +++ b/plugins/observability/plugin.go @@ -39,6 +39,7 @@ func New() *ObservabilityPlugin { "http.middleware.otel", }, WiringHooks: []string{ + "observability.otel-middleware", "observability.health-endpoints", "observability.metrics-endpoint", "observability.log-endpoint", diff --git a/plugins/observability/plugin_test.go b/plugins/observability/plugin_test.go index 650bd979..ad83a238 100644 --- a/plugins/observability/plugin_test.go +++ b/plugins/observability/plugin_test.go @@ -36,8 +36,8 @@ func TestManifestValidation(t *testing.T) { if m.Name != "observability" { t.Errorf("manifest Name = %q, want %q", m.Name, "observability") } - if len(m.ModuleTypes) != 5 { - t.Errorf("manifest ModuleTypes count = %d, want 5", len(m.ModuleTypes)) + if len(m.ModuleTypes) != 6 { + t.Errorf("manifest ModuleTypes count = %d, want 6", len(m.ModuleTypes)) } } @@ -79,6 +79,7 @@ func TestModuleFactories(t *testing.T) { "log.collector", "observability.otel", "openapi.generator", + "http.middleware.otel", } if len(factories) != len(expectedTypes) { @@ -166,6 +167,7 @@ func TestModuleSchemas(t *testing.T) { "log.collector": false, "observability.otel": false, "openapi.generator": false, + "http.middleware.otel": false, } if len(schemas) != len(expectedTypes) { @@ -235,11 +237,12 @@ func TestWiringHooks(t *testing.T) { p := New() hooks := p.WiringHooks() - if len(hooks) != 4 { - t.Fatalf("WiringHooks() count = %d, want 4", len(hooks)) + if len(hooks) != 5 { + t.Fatalf("WiringHooks() count = %d, want 5", len(hooks)) } expectedNames := map[string]bool{ + "observability.otel-middleware": false, "observability.health-endpoints": false, "observability.metrics-endpoint": false, "observability.log-endpoint": false, diff --git a/plugins/observability/wiring.go b/plugins/observability/wiring.go index e822322f..3506a89b 100644 --- a/plugins/observability/wiring.go +++ b/plugins/observability/wiring.go @@ -13,6 +13,14 @@ import ( // modules to the HTTP router. func wiringHooks() []plugin.WiringHook { return []plugin.WiringHook{ + { + // Run at priority 100 (highest) so OTEL wraps all other middleware + // and every request — including health, metrics, and pipeline routes + // registered later — is captured in a trace span. + Name: "observability.otel-middleware", + Priority: 100, + Hook: wireOTelMiddleware, + }, { Name: "observability.health-endpoints", Priority: 50, @@ -132,6 +140,26 @@ func wireLogEndpoint(app modular.Application, _ *config.WorkflowConfig) error { return nil } +// wireOTelMiddleware registers any OTelMiddleware instances as global middleware +// on every available StandardHTTPRouter so that all inbound HTTP requests are +// wrapped in an OpenTelemetry trace span. +func wireOTelMiddleware(app modular.Application, _ *config.WorkflowConfig) error { + for _, svc := range app.SvcRegistry() { + otelMw, ok := svc.(*module.OTelMiddleware) + if !ok { + continue + } + for _, routerSvc := range app.SvcRegistry() { + router, ok := routerSvc.(*module.StandardHTTPRouter) + if !ok { + continue + } + router.AddGlobalMiddleware(otelMw) + } + } + return nil +} + // wireOpenAPIEndpoints builds OpenAPI specs and registers the JSON/YAML endpoints // on any available router. func wireOpenAPIEndpoints(app modular.Application, cfg *config.WorkflowConfig) error {