fix(server/otel): move HttpInstrumentation registration to NodeSDK config for proper metrics handling

apps/server/instrumentation.mjs

@@ -16,7 +16,6 @@
 import { env } from 'node:process'
 
 import { registerInstrumentations } from '@opentelemetry/instrumentation'
-import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'
 import { IORedisInstrumentation } from '@opentelemetry/instrumentation-ioredis'
 import { PgInstrumentation } from '@opentelemetry/instrumentation-pg'
 
@@ -27,8 +26,8 @@ import { PgInstrumentation } from '@opentelemetry/instrumentation-pg'
 // no OLD-name consumer exists, so we go straight to STABLE-only — no `http/dup`
 // transition phase, no doubled cardinality.
 // Source: node_modules/.pnpm/@opentelemetry+instrumentation-http@0.215.0/.../build/src/http.js L25-72
-// MUST run before `new HttpInstrumentation(...)` below — its constructor reads
-// the env var once and caches the result.
+// MUST run before `new HttpInstrumentation(...)` (created in src/libs/otel.ts) —
+// its constructor reads the env var once and caches the result.
 //
 // Use a truthy check (not `??=`): `process.env.X` is `''` when the platform
 // (e.g. Railway) registers the var without a value, and `??=` does NOT override
@@ -48,11 +47,29 @@ if (!env.OTEL_SEMCONV_STABILITY_OPT_IN) {
 // until you query Prometheus and notice STABLE-name series are missing.
 console.info(`[otel-preload] OTEL_SEMCONV_STABILITY_OPT_IN=${env.OTEL_SEMCONV_STABILITY_OPT_IN}`)
 
+// NOTICE:
+// HttpInstrumentation is INTENTIONALLY constructed in src/libs/otel.ts (not
+// here) and passed to NodeSDK's `instrumentations` config. Reason: the OTel
+// metrics API does NOT have a proxy mechanism like traces — instruments
+// created against a NoopMeterProvider stay noop forever. If we register
+// HttpInstrumentation in this preload, its constructor caches a noop meter
+// (because no MeterProvider is set yet), then `_recordServerDuration` writes
+// to NoopHistogram for the entire process lifetime, and
+// `http_server_request_duration_seconds_*` never appears in Prometheus.
+//
+// Putting it in NodeSDK config lets the SDK call `setMeterProvider` with the
+// real provider at start(), which re-runs `_updateMetricInstruments()` and
+// upgrades the histograms to real instruments. The patches it installs are
+// `Server.prototype.emit` (incoming) — prototype-level, race-immune, so it
+// doesn't matter that they install at SDK start instead of preload.
+//
+// pg / ioredis can stay here because they only emit spans, and the trace API
+// DOES have a proxy that upgrades cleanly when the SDK installs its provider.
+//
+// Removal condition: when @opentelemetry/api adds a proxy MeterProvider that
+// upgrades cached meters retroactively, all three can move back here.
 registerInstrumentations({
   instrumentations: [
-    new HttpInstrumentation({
-      ignoreIncomingRequestHook: req => req.url === '/health',
-    }),
     new PgInstrumentation({
       enhancedDatabaseReporting: true,
     }),

apps/server/src/libs/otel.ts

@@ -10,6 +10,7 @@ import { logs, SeverityNumber } from '@opentelemetry/api-logs'
 import { OTLPLogExporter } from '@opentelemetry/exporter-logs-otlp-proto'
 import { OTLPMetricExporter } from '@opentelemetry/exporter-metrics-otlp-proto'
 import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-proto'
+import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'
 import { RuntimeNodeInstrumentation } from '@opentelemetry/instrumentation-runtime-node'
 import { resourceFromAttributes } from '@opentelemetry/resources'
 import { BatchLogRecordProcessor } from '@opentelemetry/sdk-logs'
@@ -214,11 +215,36 @@ export function initOtel(env: Env): OtelInstance | undefined {
       exportTimeoutMillis: 10_000,
     })],
     logRecordProcessors: [new BatchLogRecordProcessor(logExporter)],
-    // NOTICE: HttpInstrumentation, PgInstrumentation, and IORedisInstrumentation
-    // are registered in instrumentation.cjs (loaded via --require) so that
-    // require-in-the-middle can patch the CJS modules before tsx's ESM loader
-    // imports them. Only non-patching instrumentations belong here.
+    // NOTICE: PgInstrumentation and IORedisInstrumentation are registered in
+    // instrumentation.mjs (loaded via --import) so require-in-the-middle can
+    // patch their CJS modules before tsx's ESM loader caches them. They only
+    // emit spans (not metrics), and the trace API has a proxy that upgrades
+    // a noop tracer to the real one when the SDK starts — so registering
+    // early is safe for them.
+    //
+    // HttpInstrumentation MUST be here (NodeSDK config) and NOT in the
+    // preload, because:
+    // - It records `http.server.request.duration` to a Histogram instrument
+    //   created against `this.meter`.
+    // - The OTel metrics API does NOT have a proxy mechanism (see comment
+    //   below at sdk.start()). A meter obtained before the real
+    //   MeterProvider is installed becomes a permanent NoopMeter, and the
+    //   histogram inside it silently swallows every record() call.
+    // - NodeSDK calls setMeterProvider on its config-passed instrumentations
+    //   AT start time, after the real provider is installed. That path
+    //   re-runs `_updateMetricInstruments()` and gives the instrumentation
+    //   a real histogram.
+    // - The patch HttpInstrumentation installs is `Server.prototype.emit`
+    //   (incoming) — prototype-level, race-immune. Patching at SDK-start
+    //   time instead of preload time still catches every Server instance
+    //   created later.
+    // Source: node_modules/.../@opentelemetry+api/.../api/metrics.js
+    // (`getMeterProvider()` returns NoopMeterProvider until setGlobalMeterProvider
+    // is called; cached meters are not retroactively upgraded.)
     instrumentations: [
+      new HttpInstrumentation({
+        ignoreIncomingRequestHook: req => req.url === '/health',
+      }),
       new RuntimeNodeInstrumentation(),
     ],
   })