Blind Spots Phase 2: api-server shared middleware builder + global otelhttp (#4510)

<!-- CURSOR_AGENT_PR_BODY_BEGIN -->
## Summary

Create a shared HTTP middleware builder in `go/startup` and apply it to
api-server as the first consumer, removing all per-route
`otelhttp.NewHandler` wrappers. This ensures every HTTP request —
including 404s for unmatched routes — produces telemetry in Honeycomb.

Also removes the broken SDK-level health check sampling infrastructure
(`OvermindSampler`, `healthTp`) from `go/tracing`.

### Middleware Chain (outer → inner)

```
PostHog (outside builder) → sentryhttp → audit → otelhttp → route attribute → ALB trace ID → inner handler
```

PostHog is applied **outside** `WrapHandler` because it clones the
request via `r.WithContext()`, which would break `otelhttp`'s
`http.Request.Pattern` detection.

### Changes

- **`go/startup/middleware.go`** — New `WrapHandler` function with
options for service name, audit logger, and audit exclude paths.
Includes custom `SpanNameFormatter` that uses `http.Request.Pattern` for
matched routes.
- **`go/startup/middleware_test.go`** — 9 tests covering 404 spans,
Pattern-based span naming, `http.route` attribute, ALB trace ID capture,
audit logging, sentry repanic, and Pattern propagation with/without
request cloning.
- **`go/tracing/main.go`** — Removed `healthTp`,
`HealthCheckTracerProvider()`, `HealthCheckTracer()`, `SamplingRule`,
`OvermindSampler`, `UserAgentMatcher`. Simplified `InitTracer` (no
custom sampler — SDK default `ParentBased(AlwaysSample)` is correct).
Simplified `ShutdownTracer` (single provider).
- **`go/tracing/main_test.go`** — Updated tests to remove `healthTp`
references.
- **`go/startup/health.go`** — Removed per-route `otelhttp.NewHandler`
wrapping from `HealthHandler()`.
- **`go/discovery/engine.go`** — Replaced `tracing.HealthCheckTracer()`
with `tracing.Tracer()` in liveness/readiness probes.
- **`services/api-server/service/main.go`** — Removed all 17 per-route
`otelhttp.NewHandler` wrappers. Replaced manual sentry+audit+PostHog
chain with `startup.WrapHandler` + PostHog outside. Removed unused
`otelhttp`, `sentryhttp`, `audit` imports.
- **`go/startup/README.md`** — Documented `WrapHandler`, middleware
chain, PostHog constraint, and updated health check docs.

### Key Design Decisions

1. **Custom `SpanNameFormatter`**: otelhttp's default formatter ignores
`http.Request.Pattern` — it always returns the static operation name.
Our custom formatter prefers `Pattern` when set, falling back to the
service name for 404s.

2. **`routeAttributeMiddleware`**: otelhttp sets `http.route` from
`req.Pattern` at span start, but Pattern is empty at that point with
global wrapping. This middleware sets `http.route` post-handler after
ServeMux populates Pattern.

3. **No replacement sampler**: Removed `OvermindSampler` entirely. The
SDK default `ParentBased(AlwaysSample)` is correct — Phase 5 will add
collector-level `tail_sampling`.

<!-- CURSOR_AGENT_PR_BODY_END -->

<div><a
href="https://cursor.com/agents/bc-c4168a70-b1b6-491e-9d2d-80eec3499421"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/assets/images/open-in-web-light.png"><img
alt="Open in Web" width="114" height="28"
src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a>&nbsp;<a
href="https://cursor.com/background-agent?bcId=bc-c4168a70-b1b6-491e-9d2d-80eec3499421"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img
alt="Open in Cursor" width="131" height="28"
src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a>&nbsp;</div>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
GitOrigin-RevId: 02a81d5de65629cda18089a7932036e799fad7ae

Author: 2 people

go/discovery/engine.go

@@ -854,7 +854,7 @@ func (e *Engine) InitialiseAdapters(ctx context.Context, initFn func(ctx context
 // This checks only engine initialization (NATS connection, heartbeats) and does NOT check adapter-specific health.
 func (e *Engine) LivenessProbeHandlerFunc() func(http.ResponseWriter, *http.Request) {
 	return func(rw http.ResponseWriter, r *http.Request) {
-		ctx, span := tracing.HealthCheckTracer().Start(r.Context(), "healthcheck.liveness")
+		ctx, span := tracing.Tracer().Start(r.Context(), "healthcheck.liveness")
 		defer span.End()
 
 		err := e.LivenessHealthCheck(ctx)
@@ -880,7 +880,7 @@ func (e *Engine) SetReadinessCheck(check func(context.Context) error) {
 // This checks adapter-specific health only (not engine/liveness).
 func (e *Engine) ReadinessProbeHandlerFunc() func(http.ResponseWriter, *http.Request) {
 	return func(rw http.ResponseWriter, r *http.Request) {
-		ctx, span := tracing.HealthCheckTracer().Start(r.Context(), "healthcheck.readiness")
+		ctx, span := tracing.Tracer().Start(r.Context(), "healthcheck.readiness")
 		defer span.End()
 
 		err := e.ReadinessHealthCheck(ctx)

go/tracing/main.go

@@ -6,7 +6,6 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
-	"slices"
 	"time"
 
 	_ "embed"
@@ -27,7 +26,6 @@ import (
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
 	semconv "go.opentelemetry.io/otel/semconv/v1.26.0"
 	"go.opentelemetry.io/otel/trace"
-	"golang.org/x/sync/errgroup"
 )
 
 // logrusOtelErrorHandler routes OpenTelemetry SDK errors through logrus so they
@@ -162,27 +160,6 @@ func tracingResource(component string) *resource.Resource {
 }
 
 var tp *sdktrace.TracerProvider
-var healthTp *sdktrace.TracerProvider
-
-// HealthCheckTracerProvider returns the tracer provider used for health checks. This has a built-in 1:100 sampler for health checks that are not captured by the default UserAgentSampler for ELB and kube-probe requests.
-func HealthCheckTracerProvider() *sdktrace.TracerProvider {
-	if healthTp == nil {
-		panic("tracer providers not initialised")
-	}
-	return healthTp
-}
-
-// healthCheckTracer is the tracer used for health checks. This is heavily sampled to avoid getting spammed by k8s or ELBs
-func HealthCheckTracer() trace.Tracer {
-	return HealthCheckTracerProvider().Tracer(
-		instrumentationName,
-		trace.WithInstrumentationVersion(version),
-		trace.WithSchemaURL(semconv.SchemaURL),
-		trace.WithInstrumentationAttributes(
-			attribute.Bool("ovm.healthCheck", true),
-		),
-	)
-}
 
 // InitTracerWithUpstreams initialises the tracer with uploading directly to Honeycomb and sentry if `honeycombApiKey` and `sentryDSN` is set respectively. `component` is used as the service name.
 func InitTracerWithUpstreams(component, honeycombApiKey, sentryDSN string, opts ...otlptracehttp.Option) error {
@@ -255,18 +232,11 @@ func InitTracer(component string, opts ...otlptracehttp.Option) error {
 		return fmt.Errorf("creating OTLP trace exporter: %w", err)
 	}
 
-	healthExp, err := otlptrace.New(context.Background(), otlptracehttp.NewClient(opts...))
-	if err != nil {
-		return fmt.Errorf("creating health OTLP trace exporter: %w", err)
-	}
-
-	overmindSampler := NewOvermindSampler()
 	res := tracingResource(component)
 
 	tracerOpts := []sdktrace.TracerProviderOption{
 		sdktrace.WithBatcher(otlpExp, batcherOpts...),
 		sdktrace.WithResource(res),
-		sdktrace.WithSampler(sdktrace.ParentBased(overmindSampler)),
 	}
 	if viper.GetBool("stdout-trace-dump") {
 		stdoutExp, err := stdouttrace.New(stdouttrace.WithPrettyPrint())
@@ -279,12 +249,6 @@ func InitTracer(component string, opts ...otlptracehttp.Option) error {
 
 	otel.SetTracerProvider(tp)
 
-	healthTp = sdktrace.NewTracerProvider(
-		sdktrace.WithBatcher(healthExp, batcherOpts...),
-		sdktrace.WithResource(res),
-		sdktrace.WithSampler(sdktrace.ParentBased(sdktrace.TraceIDRatioBased(0.1))),
-	)
-
 	otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{}))
 	return nil
 }
@@ -295,121 +259,16 @@ func ShutdownTracer(ctx context.Context) {
 	ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 10*time.Second)
 	defer cancel()
 
-	var g errgroup.Group
-
-	// Do not nil healthTp or tp here: concurrent callers (e.g. health check
-	// probes via HealthCheckTracerProvider) would panic on the nil guard.
-	// Calling Shutdown on an already-shutdown provider is a safe no-op.
-	if healthTp != nil {
-		localTp := healthTp
-		g.Go(func() error {
-			if err := localTp.ForceFlush(ctx); err != nil {
-				log.WithContext(ctx).WithError(err).Error("Error flushing health tracer provider")
-			}
-			if err := localTp.Shutdown(ctx); err != nil {
-				log.WithContext(ctx).WithError(err).Error("Error shutting down health tracer provider")
-				return err
-			}
-			return nil
-		})
-	}
-
 	if tp != nil {
-		localTp := tp
-		g.Go(func() error {
-			if err := localTp.ForceFlush(ctx); err != nil {
-				log.WithContext(ctx).WithError(err).Error("Error flushing tracer provider")
-			}
-			if err := localTp.Shutdown(ctx); err != nil {
-				log.WithContext(ctx).WithError(err).Error("Error shutting down tracer provider")
-				return err
-			}
-			return nil
-		})
-	}
-
-	if err := g.Wait(); err != nil {
-		log.WithContext(ctx).WithError(err).Error("Error during tracer shutdown")
-	}
-	log.WithContext(ctx).Trace("tracing has shut down")
-}
-
-// SamplingRule defines a single sampling rule with a rate and matching function
-type SamplingRule struct {
-	SampleRate   int
-	ShouldSample func(sdktrace.SamplingParameters) bool
-}
-
-// OvermindSampler is a unified sampler that evaluates multiple sampling rules in order
-type OvermindSampler struct {
-	rules        []SamplingRule
-	ruleSamplers []sdktrace.Sampler
-}
-
-// NewOvermindSampler creates a new unified sampler with the default rules
-func NewOvermindSampler() *OvermindSampler {
-	rules := []SamplingRule{
-		{
-			SampleRate:   200,
-			ShouldSample: UserAgentMatcher("ELB-HealthChecker/2.0", "kube-probe/1.27+"),
-		},
-	}
-
-	// Pre-allocate samplers for each rule
-	ruleSamplers := make([]sdktrace.Sampler, 0, len(rules))
-	for _, rule := range rules {
-		var sampler sdktrace.Sampler
-		switch {
-		case rule.SampleRate <= 0:
-			sampler = sdktrace.NeverSample()
-		case rule.SampleRate == 1:
-			sampler = sdktrace.AlwaysSample()
-		default:
-			sampler = sdktrace.TraceIDRatioBased(1.0 / float64(rule.SampleRate))
-		}
-		ruleSamplers = append(ruleSamplers, sampler)
-	}
-
-	return &OvermindSampler{
-		rules:        rules,
-		ruleSamplers: ruleSamplers,
-	}
-}
-
-// UserAgentMatcher returns a function that matches specific user agents
-func UserAgentMatcher(userAgents ...string) func(sdktrace.SamplingParameters) bool {
-	return func(parameters sdktrace.SamplingParameters) bool {
-		for _, attr := range parameters.Attributes {
-			if (attr.Key == "http.user_agent" || attr.Key == "user_agent.original") &&
-				slices.Contains(userAgents, attr.Value.AsString()) {
-				return true
-			}
+		if err := tp.ForceFlush(ctx); err != nil {
+			log.WithContext(ctx).WithError(err).Error("Error flushing tracer provider")
 		}
-		return false
-	}
-}
-
-// ShouldSample evaluates rules in order and returns the first matching decision
-func (o *OvermindSampler) ShouldSample(parameters sdktrace.SamplingParameters) sdktrace.SamplingResult {
-	for i, rule := range o.rules {
-		if rule.ShouldSample(parameters) {
-			// Use the pre-allocated sampler for this rule
-			result := o.ruleSamplers[i].ShouldSample(parameters)
-			if result.Decision == sdktrace.RecordAndSample {
-				result.Attributes = append(result.Attributes,
-					attribute.Int("SampleRate", rule.SampleRate))
-			}
-			return result
+		if err := tp.Shutdown(ctx); err != nil {
+			log.WithContext(ctx).WithError(err).Error("Error shutting down tracer provider")
 		}
 	}
 
-	// Default to AlwaysSample if no rules match
-	return sdktrace.AlwaysSample().ShouldSample(parameters)
-}
-
-// Description returns information describing the Sampler
-func (o *OvermindSampler) Description() string {
-	return "Unified Overmind sampler combining multiple sampling strategies"
+	log.WithContext(ctx).Trace("tracing has shut down")
 }
 
 // Version returns the version baked into the binary at build time.

go/tracing/main_test.go

@@ -21,34 +21,28 @@ func TestTracingResource(t *testing.T) {
 	}
 }
 
-func TestShutdownBothProviders(t *testing.T) {
+func TestShutdownProvider(t *testing.T) {
 	exp := tracetest.NewInMemoryExporter()
 
 	tp = sdktrace.NewTracerProvider(sdktrace.WithBatcher(exp))
-	healthTp = sdktrace.NewTracerProvider(sdktrace.WithBatcher(exp))
 
-	if tp == nil || healthTp == nil {
-		t.Fatal("expected both tp and healthTp to be non-nil after init")
+	if tp == nil {
+		t.Fatal("expected tp to be non-nil after init")
 	}
 
 	ShutdownTracer(context.Background())
 
-	// After shutdown, calling Shutdown again on the providers should be a
-	// safe no-op (the SDK guards with stopOnce). We do NOT nil the package
-	// vars because concurrent callers (e.g. health probes) would panic.
+	// After shutdown, calling Shutdown again should be a safe no-op
+	// (the SDK guards with stopOnce).
 	if err := tp.Shutdown(context.Background()); err != nil {
 		t.Errorf("second tp.Shutdown should be a no-op, got: %v", err)
 	}
-	if err := healthTp.Shutdown(context.Background()); err != nil {
-		t.Errorf("second healthTp.Shutdown should be a no-op, got: %v", err)
-	}
 }
 
 func TestShutdownIdempotent(t *testing.T) {
 	exp := tracetest.NewInMemoryExporter()
 
 	tp = sdktrace.NewTracerProvider(sdktrace.WithBatcher(exp))
-	healthTp = sdktrace.NewTracerProvider(sdktrace.WithBatcher(exp))
 
 	ShutdownTracer(context.Background())
 	ShutdownTracer(context.Background()) // must not panic