From 7b2bbe315dd432c6f4fc589085893338db2ae984 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 19 Aug 2026 05:50:49 +0000 Subject: [PATCH] feat(spec): give IHttpServer an afterResponse response-observing hook so HTTP metrics are transport-agnostic (#9835) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - contract: optional afterResponse member + HttpResponseObservation/ HttpResponseObserver types + reserved UNMATCHED_ROUTE_PATTERN label; pattern-never-path stated as a hard requirement; runtime-real feature detection; transport-side counter ownership documented (de-dup, #9833); no-seam => no-HTTP-metrics expectation stated plainly per the 2026-08-18 ruling on #9650 - hono adapter: implements the seam (the ruled raw-app middleware becomes its delivery path — reach unchanged); unrouted requests labelled with the reserved unmatched pattern via the notFound marker - observability: armHttpRequestCounter arms http_requests_total through the seam at most once per server (first caller wins) - runtime: dispatcher offers its registry to the seam and suppresses its per-route counter copy when the transport implements the seam (retires the #9833 double count; histogram/request-id/error signals unchanged) - qa/http-conformance: NodeHttpServer implements the seam; cross-adapter conformance suite locks the semantics - docs: OBSERVABILITY.md + production-readiness.mdx state the transport seam and the no-metrics expectation - spec artifacts regenerated (api-surface, export-origins); changeset: spec minor, others patch --- .../http-server-response-observation-seam.md | 45 ++++ .../docs/deployment/production-readiness.mdx | 17 +- docs/OBSERVABILITY.md | 19 +- packages/core/src/index.ts | 5 + .../__tests__/http-transport-metrics.test.ts | 106 +++++++++ .../src/http-transport-metrics.ts | 82 +++++++ packages/observability/src/index.ts | 7 + .../plugins/plugin-hono-server/src/adapter.ts | 165 +++++++++++-- .../src/hono-plugin.test.ts | 9 + .../plugin-hono-server/src/hono-plugin.ts | 16 +- .../src/response-observation-seam.test.ts | 122 ++++++++++ packages/qa/http-conformance/src/adapter.ts | 75 ++++++ .../response-observation.conformance.test.ts | 225 ++++++++++++++++++ packages/runtime/src/dispatcher-plugin.ts | 34 +++ ...-inbound-coverage.hono.integration.test.ts | 175 ++++++++++++-- packages/runtime/src/observability/index.ts | 2 + .../src/observability/instrument.test.ts | 58 +++++ .../runtime/src/observability/instrument.ts | 36 ++- packages/runtime/src/observability/metrics.ts | 2 + packages/spec/api-surface/contracts.json | 3 + packages/spec/export-origins/contracts.json | 3 + .../spec/src/contracts/http-server.test.ts | 147 ++++++++++++ packages/spec/src/contracts/http-server.ts | 126 ++++++++++ 23 files changed, 1424 insertions(+), 55 deletions(-) create mode 100644 .changeset/http-server-response-observation-seam.md create mode 100644 packages/observability/src/__tests__/http-transport-metrics.test.ts create mode 100644 packages/observability/src/http-transport-metrics.ts create mode 100644 packages/plugins/plugin-hono-server/src/response-observation-seam.test.ts create mode 100644 packages/qa/http-conformance/src/response-observation.conformance.test.ts diff --git a/.changeset/http-server-response-observation-seam.md b/.changeset/http-server-response-observation-seam.md new file mode 100644 index 0000000000..ab166352ab --- /dev/null +++ b/.changeset/http-server-response-observation-seam.md @@ -0,0 +1,45 @@ +--- +"@objectstack/spec": minor +"@objectstack/core": patch +"@objectstack/observability": patch +"@objectstack/plugin-hono-server": patch +"@objectstack/runtime": patch +"@objectstack/http-conformance": patch +--- + +feat(spec): `IHttpServer` gains an optional `afterResponse` response-observing +hook so HTTP metrics are transport-agnostic instead of Hono-only (#9835) + +The contract addition (additive — a new optional member plus the +`HttpResponseObservation` / `HttpResponseObserver` types and the reserved +`UNMATCHED_ROUTE_PATTERN` label): a transport invokes each registered observer +exactly once per answered request with `{ method, routePattern, status, +elapsedMs }`, after the response exists — the observation point the `use()` +middleware contract cannot express (it runs before dispatch and never sees a +status). `routePattern` is REQUIRED to be the registered route pattern +(`/api/v1/data/:id`), never the concrete path, so no adapter re-decides metric +cardinality. Optionality is feature-detected runtime-real +(`typeof server.afterResponse === 'function'`); a transport that does not +implement the seam reports **no** HTTP metrics — zero there means "not +instrumented", never "no traffic". + +Implementations and consumers in the same change: + +- `@objectstack/plugin-hono-server`: `HonoHttpServer` implements the seam (the + ruled #9650 raw-app middleware becomes its delivery path — same reach, + including `getRawApp()` mounts and middleware-refused 429s); unrouted + requests are now labelled with the reserved `unmatched` pattern (previously + they could surface as `/*`). +- `@objectstack/observability`: new `armHttpRequestCounter(server, metrics)` + arms the `http_requests_total` counter through the seam at most once per + server (first caller wins), which is what makes "exactly one counter per + server" structural. +- `@objectstack/runtime`: the dispatcher offers its `observability.metrics` + registry to the seam (a host that wires only the dispatcher now counts every + inbound surface) and suppresses its own per-route copy of + `http_requests_total` when the transport implements the seam — retiring the + #9833 double count. Request-id echo, the duration histogram, the error + counter and the error reporter are unchanged. +- `@objectstack/http-conformance`: `NodeHttpServer` implements the seam, and a + new cross-adapter conformance suite locks the semantics for both adapters. +- `@objectstack/core`: re-exports the new contract types/constant. diff --git a/content/docs/deployment/production-readiness.mdx b/content/docs/deployment/production-readiness.mdx index d97980a5a8..88fb0bf560 100644 --- a/content/docs/deployment/production-readiness.mdx +++ b/content/docs/deployment/production-readiness.mdx @@ -55,11 +55,24 @@ That's it — every route the dispatcher mounts now: 1. Sends conservative security headers. 2. Echoes `X-Request-Id` (honored from caller, or freshly minted). -3. Emits `http_requests_total{method,route,status}` and - `http_request_duration_ms` metrics. +3. Emits the `http_request_duration_ms` histogram. 4. Reports 5xx exceptions to Sentry (or your reporter) with the request id attached. +The `http_requests_total{method,route,status}` counter is emitted at the +**transport** (the `IHttpServer.afterResponse` observation seam), so it covers +*every* inbound request on the server — auth, the REST data API and other +raw-app mounts included, not only the dispatcher's own routes. The dispatcher +offers its `metrics` registry to that seam automatically, and exactly one +counter is armed per server, so the wiring above is complete on its own and +wiring the transport plugin too never double-counts. The `route` label is the +registered pattern (`/api/v1/data/:id`), never the concrete path. + +**A transport that does not implement the seam reports no HTTP metrics**: zero +on `http_requests_total` there means "not instrumented", never "no traffic". +The shipped Hono adapter implements it; verify a custom adapter with +`typeof server.afterResponse === 'function'` before trusting its numbers. + Rate limiting is the one piece you wire at the adapter layer (Fastify preHandler, Hono middleware, etc.) because that's where you have reliable access to the caller's IP and authenticated identity. See diff --git a/docs/OBSERVABILITY.md b/docs/OBSERVABILITY.md index 659b5598dd..005a5848c4 100644 --- a/docs/OBSERVABILITY.md +++ b/docs/OBSERVABILITY.md @@ -9,11 +9,28 @@ `createDispatcherPlugin` automatically instruments every route it mounts with: - **Request id** propagation: honors incoming `X-Request-Id` (or mints `req_`); echoes on the response. -- **`http_requests_total{method,route,status}`** counter (1 per request). - **`http_request_duration_ms{method,route}`** histogram (handler latency). - **`http_request_errors_total{method,route}`** counter (incremented on thrown errors). - **Error reporting** for 5xx (handler-thrown or via `errorResponseBase` side channel). +**`http_requests_total{method,route,status}`** (1 per request) is emitted at the +**transport**, through the `IHttpServer.afterResponse` observation seam — so it +counts *every* inbound request on the server (auth, REST data API, raw-app +mounts, requests a middleware refused with 429), not only the routes the +dispatcher registers. The `route` label is always the registered **pattern** +(`/api/v1/data/:id`), never the concrete path; requests no route matched are +labelled `unmatched`. Exactly one counter is armed per server (first wiring +wins), so handing one registry to both the transport plugin and the dispatcher +never double-counts. + +> **A transport that does not implement the `afterResponse` seam reports no +> HTTP metrics.** On such a transport `http_requests_total` stays flat while +> traffic flows: zero there means "not instrumented", never "no traffic". Ask +> with `typeof server.afterResponse === 'function'` before reading absence as +> coverage. (The dispatcher degrades on those transports by counting its own +> routes, which is all it can see.) The shipped Hono adapter and the +> `@objectstack/http-conformance` node adapter both implement the seam. + Defaults are no-op (zero overhead). Inject real adapters via the `observability` config: ```ts diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 9dd9714187..423b673d37 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -88,6 +88,8 @@ export type { IHttpResponse, RouteHandler, Middleware, + HttpResponseObservation, + HttpResponseObserver, IDataEngine, IObjectQLEngine, EngineSchemaRegistryView, @@ -95,3 +97,6 @@ export type { EngineTransactionInfo, IDataDriver, } from '@objectstack/spec/contracts'; +// The reserved route label for unrouted requests on the `afterResponse` +// observation seam (#9835) — a VALUE, so it rides beside the type block above. +export { UNMATCHED_ROUTE_PATTERN } from '@objectstack/spec/contracts'; diff --git a/packages/observability/src/__tests__/http-transport-metrics.test.ts b/packages/observability/src/__tests__/http-transport-metrics.test.ts new file mode 100644 index 0000000000..4f6707a45a --- /dev/null +++ b/packages/observability/src/__tests__/http-transport-metrics.test.ts @@ -0,0 +1,106 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import type { IHttpServer, HttpResponseObserver } from '@objectstack/spec/contracts'; +import { armHttpRequestCounter } from '../http-transport-metrics.js'; +import { InMemoryMetricsRegistry } from '../metrics-exporters.js'; + +/** A minimal `IHttpServer` whose `afterResponse` we can drive by hand. */ +function observableServer() { + const observers: HttpResponseObserver[] = []; + const server: IHttpServer = { + get: () => {}, + post: () => {}, + put: () => {}, + delete: () => {}, + patch: () => {}, + use: () => {}, + listen: async () => {}, + afterResponse: (observer) => { + observers.push(observer); + }, + }; + const deliver = (routePattern: string, status: number) => { + for (const observer of observers) { + observer({ method: 'GET', routePattern, status, elapsedMs: 1 }); + } + }; + return { server, observers, deliver }; +} + +describe('armHttpRequestCounter (#9835)', () => { + it('arms the counter through the afterResponse seam, labelled by PATTERN with stringified status', () => { + const { server, deliver } = observableServer(); + const metrics = new InMemoryMetricsRegistry(); + + expect(armHttpRequestCounter(server, metrics)).toBe('armed'); + deliver('/api/v1/data/:id', 200); + deliver('/api/v1/data/:id', 503); + + expect( + metrics.totalCounter('http_requests_total', { + method: 'GET', + route: '/api/v1/data/:id', + status: '200', + }), + ).toBe(1); + expect( + metrics.totalCounter('http_requests_total', { + route: '/api/v1/data/:id', + status: '503', + }), + ).toBe(1); + }); + + it('latches per server — a second arming call registers NOTHING, whoever brings the registry', () => { + // The contract's one-owner rule made structural: the transport plugin + // (Phase 1) and the dispatcher's registry offer (Phase 2) both route + // through here; first caller wins and one registry handed to both + // layers can never double-count (#9833's shape, closed). + const { server, observers, deliver } = observableServer(); + const first = new InMemoryMetricsRegistry(); + const second = new InMemoryMetricsRegistry(); + + expect(armHttpRequestCounter(server, first)).toBe('armed'); + expect(armHttpRequestCounter(server, first)).toBe('already-armed'); + expect(armHttpRequestCounter(server, second)).toBe('already-armed'); + expect(observers).toHaveLength(1); + + deliver('/counted', 200); + expect(first.totalCounter('http_requests_total', { route: '/counted' })).toBe(1); + // The later registry was refused, not silently added. + expect(second.totalCounter('http_requests_total', { route: '/counted' })).toBe(0); + }); + + it('reports "unsupported" for a transport without the seam and registers nothing', () => { + const server: IHttpServer = { + get: () => {}, + post: () => {}, + put: () => {}, + delete: () => {}, + patch: () => {}, + use: () => {}, + listen: async () => {}, + }; + const metrics = new InMemoryMetricsRegistry(); + + // The documented expectation, surfaced instead of read as coverage: + // this transport reports NO HTTP metrics; the caller decides how to + // degrade (the dispatcher falls back to counting its own routes). + expect(armHttpRequestCounter(server, metrics)).toBe('unsupported'); + expect(metrics.samples).toHaveLength(0); + }); + + it('latches per SERVER, not per process — a second server arms independently', () => { + const a = observableServer(); + const b = observableServer(); + const metrics = new InMemoryMetricsRegistry(); + + expect(armHttpRequestCounter(a.server, metrics)).toBe('armed'); + expect(armHttpRequestCounter(b.server, metrics)).toBe('armed'); + a.deliver('/a', 200); + b.deliver('/b', 200); + expect(metrics.totalCounter('http_requests_total', { route: '/a' })).toBe(1); + expect(metrics.totalCounter('http_requests_total', { route: '/b' })).toBe(1); + }); +}); diff --git a/packages/observability/src/http-transport-metrics.ts b/packages/observability/src/http-transport-metrics.ts new file mode 100644 index 0000000000..82e1cb49bf --- /dev/null +++ b/packages/observability/src/http-transport-metrics.ts @@ -0,0 +1,82 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { IHttpServer } from '@objectstack/spec/contracts'; +import type { MetricsRegistry } from './contracts.js'; +import { RUNTIME_METRICS } from './semconv.js'; + +/** + * What {@link armHttpRequestCounter} did: + * + * - `'armed'` — the counter-emitting observer was registered on this call. + * - `'already-armed'` — some earlier caller already armed this server; the + * seam counts, and this call registered nothing (first-wins). + * - `'unsupported'` — the transport does not implement the + * `IHttpServer.afterResponse` seam; it reports NO HTTP metrics (#9835: + * zero there means "not instrumented", never "no traffic"), and the + * caller must decide how to degrade — the runtime dispatcher falls back + * to counting its own routes. + */ +export type ArmHttpRequestCounterResult = 'armed' | 'already-armed' | 'unsupported'; + +/** + * The per-server latch behind the contract's ownership rule. A registered + * global-registry symbol (`Symbol.for`), not a module-level WeakSet, so the + * latch cannot fork if two copies of this module ever coexist in one + * process — the whole point is that there is exactly ONE latch per server + * object, whoever asks. + */ +const HTTP_REQUEST_COUNTER_ARMED = Symbol.for( + 'objectstack.observability.httpRequestCounterArmed', +); + +/** + * Arm `http_requests_total{method,route,status}` on a transport through the + * `IHttpServer.afterResponse` observation seam (#9835) — AT MOST ONCE per + * server, whoever calls first. + * + * ## Why arming is centralized here + * + * The contract's ownership rule says a request must never be double-counted, + * and two composition layers legitimately hold both a server and a metrics + * registry: the transport's own hosting plugin (`HonoServerPlugin`, which + * the 2026-08-18 ruling on #9650 made the counter's home) and the runtime + * dispatcher (whose `observability.metrics` config is the wiring the docs + * demonstrate). When a host hands ONE registry to both — the ordinary case — + * two independently-registered observers would land every request on the + * same series twice: exactly the #9833 distortion, rebuilt one seam over. + * Routing every arming through this function makes "exactly one + * counter-emitting observer per server" structural: the first caller arms + * (in the shipped composition that is the transport plugin, in Phase 1), + * every later caller is told the seam already counts. + * + * The label shape is pinned by the contract: `route` is the transport's + * `routePattern` — the registered PATTERN, never the concrete path — and + * `status` is stringified for the label set. Emission goes through the + * transport's observer-isolation guarantee, so a throwing registry cannot + * break a response. + * + * @param server - The transport. Pass the RAW registered `http.server` + * instance, not a wrapper: the latch is per object identity, and a wrapper + * would both fork the latch and (per #5122) risk erasing the optional + * member this function feature-detects. + * @param metrics - The registry the counter lands in. First caller wins; a + * second registry offered later is NOT added (the contract's one-owner + * rule), and the result says so. + */ +export function armHttpRequestCounter( + server: IHttpServer, + metrics: MetricsRegistry, +): ArmHttpRequestCounterResult { + if (typeof server.afterResponse !== 'function') return 'unsupported'; + const latched = server as IHttpServer & { [HTTP_REQUEST_COUNTER_ARMED]?: boolean }; + if (latched[HTTP_REQUEST_COUNTER_ARMED]) return 'already-armed'; + latched[HTTP_REQUEST_COUNTER_ARMED] = true; + server.afterResponse((observation) => { + metrics.counter(RUNTIME_METRICS.httpRequestsTotal, { + method: observation.method, + route: observation.routePattern, + status: String(observation.status), + }); + }); + return 'armed'; +} diff --git a/packages/observability/src/index.ts b/packages/observability/src/index.ts index b266cd084d..2811cba510 100644 --- a/packages/observability/src/index.ts +++ b/packages/observability/src/index.ts @@ -16,6 +16,13 @@ export { OBSERVABILITY_METRICS_SERVICE, OBSERVABILITY_ERRORS_SERVICE } from './s // Semantic conventions export { SEMCONV, RUNTIME_METRICS } from './semconv.js'; +// Transport-agnostic HTTP request counter, armed at most once per server via +// the `IHttpServer.afterResponse` observation seam (#9835) +export { + armHttpRequestCounter, + type ArmHttpRequestCounterResult, +} from './http-transport-metrics.js'; + // Metric exporters export { NoopMetricsRegistry, diff --git a/packages/plugins/plugin-hono-server/src/adapter.ts b/packages/plugins/plugin-hono-server/src/adapter.ts index b12934b024..3fb9c65149 100644 --- a/packages/plugins/plugin-hono-server/src/adapter.ts +++ b/packages/plugins/plugin-hono-server/src/adapter.ts @@ -7,11 +7,16 @@ import { IHttpServer, RouteHandler, Middleware, + UNMATCHED_ROUTE_PATTERN, createLogger, } from '@objectstack/core'; +import type { + HttpResponseObserver, + HttpResponseObservation, +} from '@objectstack/core'; import type { Logger } from '@objectstack/spec/contracts'; import type { Context } from 'hono'; -import { currentPerfTiming, RUNTIME_METRICS, type MetricsRegistry } from '@objectstack/observability'; +import { currentPerfTiming, armHttpRequestCounter, type MetricsRegistry } from '@objectstack/observability'; import { Hono } from 'hono'; import { routePath } from 'hono/route'; import { serve } from '@hono/node-server'; @@ -228,8 +233,34 @@ export class HonoHttpServer implements IHttpServer { private middlewares: Array<{ path?: string; handler: Middleware }> = []; /** Whether the Hono middleware that runs {@link middlewares} is mounted. */ private middlewareSeamInstalled = false; - /** Whether the `http_requests_total` middleware is mounted. See {@link installHttpMetricsSeam}. */ + /** + * Local idempotence for {@link installHttpMetricsSeam}. The cross-caller + * "at most one counter per server" latch lives in + * `armHttpRequestCounter` (per-server, whoever arms); this flag only + * short-circuits repeat calls on this adapter instance. + */ private httpMetricsSeamInstalled = false; + /** + * Registered {@link HttpResponseObserver}s, in registration order — read + * per request by the observation seam, so a consumer may register at ANY + * moment after boot (same design as {@link middlewares}). See + * {@link afterResponse}. + */ + private responseObservers: HttpResponseObserver[] = []; + /** Whether the Hono middleware that delivers to {@link responseObservers} is mounted. */ + private responseObservationSeamInstalled = false; + /** + * Requests that reached the `notFound` sink — i.e. matched NO registered + * route (Hono routes method mismatches there too). Marked in + * {@link installNotFoundSeam}'s hook, read by the observation seam so + * `routePattern` can carry the contract's reserved unmatched label: + * `routePath(c)` cannot answer this itself — after `next()` it reports + * the deepest EXECUTED handler, which for an unrouted request is one of + * this adapter's own `use('*')` seams (measured: `/*`), a spelling + * indistinguishable from a real static catch-all route. Keyed on the + * fetch `Request` object (per request, GC-safe). + */ + private unmatchedMarks = new WeakSet(); /** * The LAST-RESORT handler installed by {@link setFallbackHandler}, or * `undefined` when no consumer installed one. Exactly one — installing @@ -707,6 +738,12 @@ export class HonoHttpServer implements IHttpServer { this.notFoundSeamInstalled = true; app.notFound(async (c: any) => { + // Reaching this hook IS the definition of "no registered route + // matched" — mark it for the response-observation seam (#9835), + // which labels these with the contract's reserved unmatched + // pattern. Marked before the fallback runs: a fallback-served + // request is still an unrouted one. + if (c?.req?.raw) this.unmatchedMarks.add(c.req.raw); const handler = this.fallbackHandler; if (handler) { const { response, failed } = await this.runHandler(c, handler); @@ -948,9 +985,9 @@ export class HonoHttpServer implements IHttpServer { } /** - * Mount the single Hono middleware that emits - * `http_requests_total{method,route,status}` for EVERY inbound request on - * this transport. Idempotent. + * Arm `http_requests_total{method,route,status}` for EVERY inbound + * request on this transport, by registering the counter-emitting observer + * on the {@link afterResponse} seam. Idempotent. * * ## Why the counter lives here and not one layer up (#9650) * @@ -1004,7 +1041,79 @@ export class HonoHttpServer implements IHttpServer { if (this.httpMetricsSeamInstalled) return; this.httpMetricsSeamInstalled = true; + // Since #9835 the counter is ONE OBSERVER on the contract-level + // response-observation seam rather than a private middleware: the + // delivery path (and therefore the reach measured for #9650) is + // exactly the one every `afterResponse` consumer gets, so the two can + // never drift — and a request can never be double-counted between + // "the adapter middleware" and "the hook path", because they are the + // same path. De-dup with OTHER arming sites (the runtime dispatcher's + // `observability.metrics` wiring) is `armHttpRequestCounter`'s + // per-server latch: every counter-arming call in the repo routes + // through it, first caller wins — in the shipped composition that is + // this plugin, in Phase 1, which is what makes the transport the + // counter's owner in fact and not only in documentation. + armHttpRequestCounter(this, metrics); + } + + /** + * Register a RESPONSE OBSERVER — the `IHttpServer.afterResponse` CONTRACT + * (#9835); see `@objectstack/spec/contracts` for the full text. Honoured + * here as follows: + * + * - **Delivery** rides ONE Hono middleware ({@link + * installResponseObservationSeam}) that reads {@link responseObservers} + * per request, so registration works at any moment after boot — same + * design as {@link use}. `HonoServerPlugin` mounts the seam at the end + * of `init()`; a bare `HonoHttpServer` gets it mounted on first + * registration, and only that path carries the register-before-routes + * requirement. + * - **`routePattern` is the matched PATTERN** — `routePath(c)` after + * `next()`, i.e. the route that actually answered; the reserved + * `UNMATCHED_ROUTE_PATTERN` when nothing matched. + * - **Reach**: raw-app mounts included (the seam is a raw-app + * middleware), `use()` short-circuits included (mounted before the + * middleware seam). The documented boundary: a CORS preflight the + * transport's own built-in answers is outside the seam. + * - **Isolation**: each observer runs in its own try/catch — a throwing + * observer affects neither the response nor sibling observers. + */ + afterResponse(observer: HttpResponseObserver): void { + this.responseObservers.push(observer); + this.installResponseObservationSeam(); + } + + /** + * Mount the single Hono middleware that delivers + * {@link HttpResponseObservation}s to every registered observer. + * Idempotent. + * + * WHERE this is called decides what can be observed, exactly like + * {@link installMiddlewareSeam} (the two callers are the same pair): + * + * - **`HonoServerPlugin.init()`, immediately BEFORE + * `installMiddlewareSeam()`** — after the transport's own built-ins + * (Server-Timing, CORS), so a preflight CORS answers itself is not + * observed; before the middleware seam, so a request a `use()` + * middleware refuses (the inbound rate limiter's 429) IS observed — a + * refused request is exactly the one an operator alerts on; and before + * any route exists (every route mounts in some plugin's `start()`), so + * the observation wraps every handler however late the observer + * registers. + * - **the first {@link afterResponse}** — for a bare `HonoHttpServer` + * composed without the plugin, so the seam is never silently absent. + * + * A request pays one array-length check when no observers are registered + * — the disarmed cost, same shape as the middleware seam's empty-chain + * fast path. + */ + installResponseObservationSeam(): void { + if (this.responseObservationSeamInstalled) return; + this.responseObservationSeamInstalled = true; + this.app.use('*', async (c, next) => { + if (this.responseObservers.length === 0) return next(); + const startedAt = Date.now(); // Default 500: if `next()` rejects, Hono's error path renders the // 500 and `c.res` is not yet set — reading it would synthesize a // response and change what the caller receives. @@ -1013,20 +1122,38 @@ export class HonoHttpServer implements IHttpServer { await next(); status = c.res.status; } finally { - try { - metrics.counter(RUNTIME_METRICS.httpRequestsTotal, { - method: c.req.method, - // `routePath(c)` is the non-deprecated spelling of - // `c.req.routePath` and returns the same matched - // PATTERN — read after `next()`, so it is the route - // that actually answered rather than this middleware's - // own `*`. Empty only when nothing matched at all. - route: routePath(c) || 'unmatched', - status: String(status), - }); - } catch { - // A metrics backend must never be able to break a - // response. Same discipline as the error reporter. + // An unrouted request executes only this adapter's own + // `use('*')` seams, so after `next()` `routePath(c)` reports + // `/*` — OUR registration, indistinguishable from a real + // static catch-all. The `notFound` sink is the authority on + // "nothing matched" and marks the request (see + // {@link unmatchedMarks}); on a bare server that never + // mounted the notFound seam the mark cannot exist and the + // label degrades to what `routePath` reports. + const unrouted = c.req?.raw ? this.unmatchedMarks.has(c.req.raw) : false; + const observation: HttpResponseObservation = { + method: c.req.method, + // `routePath(c)` is the non-deprecated spelling of + // `c.req.routePath` and returns the same matched + // PATTERN — read after `next()`, so it is the route + // that actually answered rather than this middleware's + // own `*`. + routePattern: unrouted + ? UNMATCHED_ROUTE_PATTERN + : routePath(c) || UNMATCHED_ROUTE_PATTERN, + status, + elapsedMs: Date.now() - startedAt, + }; + // Snapshot so an observer registering from inside an observer + // cannot mutate the list mid-delivery. + for (const observer of [...this.responseObservers]) { + try { + observer(observation); + } catch { + // An observer must never be able to break a response + // or a sibling observer. Same discipline as a metrics + // backend / the error reporter. + } } } }); diff --git a/packages/plugins/plugin-hono-server/src/hono-plugin.test.ts b/packages/plugins/plugin-hono-server/src/hono-plugin.test.ts index ad516cf71a..4cb54155c5 100644 --- a/packages/plugins/plugin-hono-server/src/hono-plugin.test.ts +++ b/packages/plugins/plugin-hono-server/src/hono-plugin.test.ts @@ -35,6 +35,15 @@ vi.mock('./adapter', async (importOriginal) => ({ // end of init(). Real behaviour is covered against the REAL adapter // in `middleware-seam.test.ts`; here it only has to exist. installMiddlewareSeam: vi.fn(), + // [#9835] init() mounts the `afterResponse` delivery seam + // unconditionally (observers may register at any later moment) + // and arms the counter only when a registry resolved. Real + // behaviour is covered against the REAL adapter in + // `response-observation-seam.test.ts` and cross-adapter in + // `@objectstack/http-conformance`; here they only have to exist. + installResponseObservationSeam: vi.fn(), + installHttpMetricsSeam: vi.fn(), + afterResponse: vi.fn(), // [#5090] Same deal for the unmatched-request seam: `start()` mounts // it through the adapter now (one owner for `app.notFound`, which is // last-call-wins). The real 404/405/fallback composition is covered diff --git a/packages/plugins/plugin-hono-server/src/hono-plugin.ts b/packages/plugins/plugin-hono-server/src/hono-plugin.ts index 21d6add1b9..73b5a4e7a7 100644 --- a/packages/plugins/plugin-hono-server/src/hono-plugin.ts +++ b/packages/plugins/plugin-hono-server/src/hono-plugin.ts @@ -106,9 +106,11 @@ export interface HonoPluginOptions { * 1. this option — explicit wiring, and the escape hatch tests use; * 2. the `observability:metrics` service, when the host registered one * BEFORE this plugin; - * 3. neither — then **no middleware is installed at all**, so an - * unconfigured deployment pays no per-request cost. Not a disabled - * counter; no counter. + * 3. neither — then **no counter-emitting observer is registered**, so + * an unconfigured deployment pays no per-request metrics cost (the + * `afterResponse` delivery seam is still mounted, disarmed at one + * array-length check per request — #9835). Not a disabled counter; + * no counter. */ observability?: { metrics?: MetricsRegistry; @@ -441,6 +443,14 @@ export class HonoServerPlugin implements Plugin { // order. A middleware Hono learns about after a route runs after // that route's handler — which is why installing this from a later // phase (or from `kernel:bootstrapped`) observes nothing at all. + // The DELIVERY middleware is mounted unconditionally (#9835): the + // `IHttpServer.afterResponse` contract lets any consumer register an + // observer at any later moment (the seam reads the observer list per + // request, like the middleware seam reads its chain), and Phase 1 is + // the last moment a middleware can still precede every route. With no + // observers registered the per-request cost is one array-length + // check. The COUNTER below stays conditional: no backend, no counter. + this.server.installResponseObservationSeam(); const metrics = this.resolveMetrics(ctx); if (metrics) { this.server.installHttpMetricsSeam(metrics); diff --git a/packages/plugins/plugin-hono-server/src/response-observation-seam.test.ts b/packages/plugins/plugin-hono-server/src/response-observation-seam.test.ts new file mode 100644 index 0000000000..c1de3e4914 --- /dev/null +++ b/packages/plugins/plugin-hono-server/src/response-observation-seam.test.ts @@ -0,0 +1,122 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `HonoHttpServer.afterResponse` — the adapter-local edges of the #9835 + * response-observation seam. Cross-adapter semantics (pattern-never-path, + * unmatched label, append registration, observer isolation) are locked in + * `@objectstack/http-conformance`; the runtime's integration file pins the + * composed-kernel behavior. What is pinned HERE is what only this adapter + * can promise: + * + * - a route mounted on the RAW Hono app (`getRawApp()` — the #9650 blind + * spot) is observed, labelled by its registered pattern; + * - a request the `use()` middleware chain SHORT-CIRCUITS (429) is + * observed — the seam sits outside the middleware seam; + * - a request answered by the `setFallbackHandler` seam carries the + * reserved unmatched label: the fallback is by definition serving a + * request no registered route matched, and `routePath(c)` alone cannot + * say so (after `next()` it reports the adapter's own `use('*')` seam, + * `/*` — the notFound marker is the authority). + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { UNMATCHED_ROUTE_PATTERN } from '@objectstack/core'; +import type { HttpResponseObservation } from '@objectstack/core'; +import { HonoHttpServer } from './adapter'; + +describe('HonoHttpServer afterResponse seam (#9835)', () => { + const opened: HonoHttpServer[] = []; + + const boot = async (setup: (server: HonoHttpServer) => void) => { + const server = new HonoHttpServer(0); + server.installNotFoundSeam(); + setup(server); + await server.listen(0); + opened.push(server); + return `http://127.0.0.1:${server.getPort()}`; + }; + + afterEach(async () => { + while (opened.length > 0) await opened.pop()!.close(); + }); + + it('observes a route mounted on the RAW app, labelled by its registered pattern', async () => { + const seen: HttpResponseObservation[] = []; + const baseUrl = await boot((server) => { + server.afterResponse((o) => seen.push(o)); + // The #9650 blind spot: a plugin mounting through getRawApp() + // bypasses IHttpServer entirely (auth's `rawApp.all` is the + // production line this mirrors). + server.getRawApp().all('/api/v1/auth/*', (c: any) => c.json({ ok: true }, 200)); + }); + + const res = await fetch(`${baseUrl}/api/v1/auth/sign-in/email`, { method: 'POST' }); + expect(res.status).toBe(200); + + expect(seen).toHaveLength(1); + expect(seen[0].routePattern).toBe('/api/v1/auth/*'); + expect(seen[0].routePattern).not.toContain('sign-in'); + expect(seen[0].status).toBe(200); + }); + + it('observes a request the use() chain REFUSES — the seam sits outside the middleware seam', async () => { + const seen: HttpResponseObservation[] = []; + const baseUrl = await boot((server) => { + // Plugin order, reproduced: observation seam first, middleware + // seam second — exactly what `HonoServerPlugin.init()` does. + server.installResponseObservationSeam(); + server.afterResponse((o) => seen.push(o)); + server.installMiddlewareSeam(); + server.use(async (req, res, _next) => { + // Stands in for the inbound rate limiter: refuse everything. + void req; + res.status(429).json({ error: 'too many requests' }); + }); + server.get('/never-reached', async (_req, res) => res.json({ ok: true })); + }); + + const res = await fetch(`${baseUrl}/never-reached`); + expect(res.status).toBe(429); + + // A refused request is exactly the one an operator alerts on. + expect(seen).toHaveLength(1); + expect(seen[0].status).toBe(429); + }); + + it('labels a fallback-served request with the reserved unmatched pattern', async () => { + const seen: HttpResponseObservation[] = []; + const baseUrl = await boot((server) => { + server.afterResponse((o) => seen.push(o)); + server.get('/registered', async (_req, res) => res.json({ ok: true })); + server.setFallbackHandler(async (_req, res) => { + res.status(200).json({ served: 'by-fallback' }); + }); + }); + + const res = await fetch(`${baseUrl}/declarative/endpoint`); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ served: 'by-fallback' }); + + // The fallback answered, but no REGISTERED route matched — the + // contract's unmatched label, delivered via the notFound marker. + expect(seen).toHaveLength(1); + expect(seen[0].routePattern).toBe(UNMATCHED_ROUTE_PATTERN); + expect(seen[0].status).toBe(200); + }); + + it('a method mismatch (405 + Allow) is unmatched too — Hono routes it to the same sink', async () => { + const seen: HttpResponseObservation[] = []; + const baseUrl = await boot((server) => { + server.afterResponse((o) => seen.push(o)); + server.put('/only-put', async (_req, res) => res.json({ ok: true })); + }); + + const res = await fetch(`${baseUrl}/only-put`, { method: 'POST' }); + expect(res.status).toBe(405); + + expect(seen).toHaveLength(1); + expect(seen[0].routePattern).toBe(UNMATCHED_ROUTE_PATTERN); + expect(seen[0].status).toBe(405); + expect(seen[0].method).toBe('POST'); + }); +}); diff --git a/packages/qa/http-conformance/src/adapter.ts b/packages/qa/http-conformance/src/adapter.ts index 9f162ddc49..8d64bd9872 100644 --- a/packages/qa/http-conformance/src/adapter.ts +++ b/packages/qa/http-conformance/src/adapter.ts @@ -7,7 +7,9 @@ import { IHttpResponse, RouteHandler, Middleware, + UNMATCHED_ROUTE_PATTERN, } from '@objectstack/core'; +import type { HttpResponseObserver } from '@objectstack/core'; /** * NodeHttpServer — a thin `IHttpServer` implementation on raw `node:http`, @@ -34,6 +36,15 @@ import { * conformance suite can assert them CROSS-adapter. Asserting them against a * single implementor would prove nothing about the port. * + * Since #9835 it implements the optional + * {@link NodeHttpServer.afterResponse} response-observation seam for the same + * reason — the conformance suite locks the seam's semantics (pattern-never- + * path, refused-request reach, observer isolation) across BOTH adapters. It + * also retires, for this transport, the "zero means not instrumented" hole + * the #9650 ruling documented as an expectation: a composition that arms a + * metrics observer through the seam gets `http_requests_total` here too, + * instead of a counter that stays flat while traffic flows. + * * Deliberately NOT implemented (each one is a known escape hatch whose * consumers feature-detect and degrade): * - `getRawApp()` — Hono-specific; metadata HMR, cloud-connection routes and @@ -118,6 +129,12 @@ export class NodeHttpServer implements IHttpServer { * semantics and the zero registration-order dependency structural here. */ private fallbackHandler: RouteHandler | undefined; + /** + * Registered {@link HttpResponseObserver}s, in registration order — the + * `IHttpServer.afterResponse` seam (#9835). Read per request in + * {@link handleRequest}, so registration works at any moment. + */ + private responseObservers: HttpResponseObserver[] = []; constructor( private port: number = 3000, @@ -204,6 +221,33 @@ export class NodeHttpServer implements IHttpServer { this.fallbackHandler = handler; } + /** + * Register a RESPONSE OBSERVER — the `IHttpServer.afterResponse` CONTRACT + * (#9835); see `@objectstack/spec/contracts` for the full text. Honoured + * here as follows: + * + * - **Delivery** hangs off the native response's `finish` event, armed in + * {@link handleRequest} once the route table has been consulted — so + * the observation fires exactly once, after the response was written + * (buffered, streamed, fallback and error paths alike), with the + * status as sent. A connection that dies before `finish` (client + * abort) goes unobserved, as the contract allows. + * - **`routePattern` is the registered PATTERN** the router matched + * (`route.pattern`, the very string the consumer registered), never + * the concrete path; the reserved `UNMATCHED_ROUTE_PATTERN` when no + * route matched (404/405/fallback answers). + * - **Reach**: everything {@link handleRequest} serves — there is no + * raw-app escape hatch on this adapter (`getRawApp` deliberately + * absent), so route, fallback and unmatched answers are the whole + * surface, and all three are observed. This adapter has no transport + * built-ins that answer before the seam, so its boundary is empty. + * - **Isolation**: each observer runs in its own try/catch — a throwing + * observer affects neither the response nor sibling observers. + */ + afterResponse(observer: HttpResponseObserver): void { + this.responseObservers.push(observer); + } + /** * This adapter's standard answer for a request that matched no route — the * `IHttpServer` unmatched-request CONTRACT (#3607 / ADR-0076 OQ#10): 405 + @@ -263,11 +307,42 @@ export class NodeHttpServer implements IHttpServer { } private async handleRequest(nodeReq: IncomingMessage, nodeRes: ServerResponse) { + const startedAt = Date.now(); const method = (nodeReq.method || 'GET').toUpperCase(); const url = new URL(nodeReq.url || '/', 'http://internal'); const path = url.pathname; const matched = this.match(method, path); + + // ── The `afterResponse` observation seam (#9835) ──────────────────── + // Armed as soon as the router's verdict is known — BEFORE any branch + // below writes, so the unmatched 404/405, the fallback and the route + // paths are all observed. `finish` fires once, when the response has + // been fully written, which is what "after the response exists" means + // on node:http. Zero cost when no observer is registered. + if (this.responseObservers.length > 0) { + nodeRes.once('finish', () => { + const observation = { + method, + // The registered PATTERN, never the concrete path — the + // contract's hard cardinality requirement. Unrouted + // requests carry the contract's reserved label. + routePattern: matched?.route.pattern ?? UNMATCHED_ROUTE_PATTERN, + status: nodeRes.statusCode, + elapsedMs: Date.now() - startedAt, + }; + // Snapshot so an observer registering mid-delivery cannot + // mutate the list under the loop. + for (const observer of [...this.responseObservers]) { + try { + observer(observation); + } catch { + // An observer must never break a response or a + // sibling observer — the contract's isolation rule. + } + } + }); + } // The LAST-RESORT seam (#6143): consulted ONLY here, i.e. only once // every explicitly registered route has missed — see the CONTRACT on // {@link setFallbackHandler}. Resolved BEFORE the request body is read diff --git a/packages/qa/http-conformance/src/response-observation.conformance.test.ts b/packages/qa/http-conformance/src/response-observation.conformance.test.ts new file mode 100644 index 0000000000..b960030733 --- /dev/null +++ b/packages/qa/http-conformance/src/response-observation.conformance.test.ts @@ -0,0 +1,225 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `IHttpServer.afterResponse` — cross-adapter conformance (#9835). + * + * The contract (`packages/spec/src/contracts/http-server.ts`) makes the + * response-observation seam the transport-agnostic home for HTTP metrics: + * an observation point that runs AFTER the response exists, carrying the + * status the framework-agnostic `use()` middleware chain can never see + * (it runs to completion BEFORE dispatch — measured in + * `packages/runtime/src/http-metrics-inbound-coverage.hono.integration.test.ts`). + * Its testable promises, each locked here against BOTH adapters over a real + * socket: + * + * 1. each registered observer is invoked EXACTLY ONCE per answered request, + * with `{ method, routePattern, status, elapsedMs }`; + * 2. `routePattern` is the registered PATTERN, NEVER the concrete path — + * the hard cardinality requirement no adapter may re-decide — and a + * request no route matched carries the reserved + * `UNMATCHED_ROUTE_PATTERN`; + * 3. registration APPENDS (several observers coexist, registration order), + * unlike `setFallbackHandler`'s replace semantics; + * 4. a throwing observer affects neither the response nor sibling + * observers; + * 5. optionality is feature-detected runtime-real: + * `typeof server.afterResponse === 'function'`. + * + * A Hono-only case would prove nothing about cross-adapter agreement, which + * is the entire point of this package: the #9650 ruling's documented + * expectation — "a transport that does not implement the seam reports no + * HTTP metrics" — stops applying to a transport exactly when this suite + * passes for it. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import type { RouteHandler, HttpResponseObservation, HttpResponseObserver } from '@objectstack/core'; +import { UNMATCHED_ROUTE_PATTERN } from '@objectstack/core'; +import { HonoHttpServer } from '@objectstack/plugin-hono-server'; + +import { NodeHttpServer } from './adapter.js'; + +/** + * The structural surface this suite drives — `afterResponse` REQUIRED. The + * member is optional on the contract; an adapter in this list claims it, and + * from that moment the promises above are load-bearing, not aspirational. + */ +interface ObservableServer { + get(path: string, handler: RouteHandler): void; + post(path: string, handler: RouteHandler): void; + afterResponse(observer: HttpResponseObserver): void; + listen(port: number): Promise; + close?(): Promise; + getPort(): number; +} + +type AdapterCase = { + label: 'node' | 'hono'; + make: () => ObservableServer; +}; + +const ADAPTERS: AdapterCase[] = [ + { label: 'node', make: () => new NodeHttpServer(0) }, + { + label: 'hono', + make: () => { + const server = new HonoHttpServer(0); + // The standard composed state: `HonoServerPlugin.start()` mounts + // the unmatched-request seam (404 / 405 + Allow) exactly like + // this — without it a bare HonoHttpServer answers Hono's own + // 404 text page, which no deployment serves. + server.installNotFoundSeam(); + return server; + }, + }, +]; + +describe.each(ADAPTERS)('IHttpServer.afterResponse conformance on $label adapter', ({ make }) => { + const opened: ObservableServer[] = []; + + /** + * Register observers FIRST, then routes, then listen. Registration-time + * flexibility beyond this is adapter-composition territory (the Hono + * PLUGIN mounts the delivery seam in Phase 1 so observers may register + * whenever; a BARE HonoHttpServer mounts it on first registration, which + * therefore must precede routes — the same caveat its `use()` carries). + */ + const boot = async ( + observers: HttpResponseObserver[], + register: (server: ObservableServer) => void, + ) => { + const server = make(); + for (const observer of observers) server.afterResponse(observer); + register(server); + await server.listen(0); + opened.push(server); + return `http://127.0.0.1:${server.getPort()}`; + }; + + afterEach(async () => { + while (opened.length > 0) await opened.pop()!.close?.(); + }); + + it('is feature-detected runtime-real: typeof afterResponse === "function"', () => { + const server = make(); + expect(typeof (server as { afterResponse?: unknown }).afterResponse === 'function').toBe(true); + }); + + it('fires EXACTLY ONCE per answered request, with the registered PATTERN — never the concrete path', async () => { + const seen: HttpResponseObservation[] = []; + const baseUrl = await boot([(o) => seen.push(o)], (server) => { + server.get('/api/v1/data/:id', async (req, res) => { + res.status(200).json({ id: req.params.id }); + }); + }); + + const res = await fetch(`${baseUrl}/api/v1/data/rec_42`); + expect(res.status).toBe(200); + // Delivery may trail the response by a tick (node's `finish` event) — + // settle the event loop before counting. + await new Promise((resolve) => setTimeout(resolve, 20)); + + expect(seen).toHaveLength(1); + expect(seen[0].method).toBe('GET'); + // The hard requirement: the PATTERN (one series per mount), not the + // path (one series per record id). + expect(seen[0].routePattern).toBe('/api/v1/data/:id'); + expect(seen[0].routePattern).not.toContain('rec_42'); + expect(seen[0].status).toBe(200); + expect(seen[0].elapsedMs).toBeGreaterThanOrEqual(0); + }); + + it('carries the status AS SENT — a 503 route observes 503, and elapsedMs covers the handler', async () => { + const seen: HttpResponseObservation[] = []; + const baseUrl = await boot([(o) => seen.push(o)], (server) => { + server.get('/unhealthy', async (_req, res) => { + await new Promise((resolve) => setTimeout(resolve, 25)); + res.status(503).json({ ok: false }); + }); + }); + + const res = await fetch(`${baseUrl}/unhealthy`); + expect(res.status).toBe(503); + await new Promise((resolve) => setTimeout(resolve, 20)); + + expect(seen).toHaveLength(1); + expect(seen[0].status).toBe(503); + // The handler slept 25ms; allow generous slack downward (timer + // coarseness) while still proving the window brackets the handler. + expect(seen[0].elapsedMs).toBeGreaterThanOrEqual(10); + }); + + it('reports a request NO route matched with the reserved UNMATCHED_ROUTE_PATTERN and the 404 as sent', async () => { + const seen: HttpResponseObservation[] = []; + const baseUrl = await boot([(o) => seen.push(o)], (server) => { + server.get('/exists', async (_req, res) => res.json({ ok: true })); + }); + + const res = await fetch(`${baseUrl}/no/such/route`); + expect(res.status).toBe(404); + await new Promise((resolve) => setTimeout(resolve, 20)); + + expect(seen).toHaveLength(1); + expect(seen[0].routePattern).toBe(UNMATCHED_ROUTE_PATTERN); + expect(seen[0].routePattern).not.toContain('/no/such/route'); + expect(seen[0].status).toBe(404); + }); + + it('registration APPENDS — two observers both see the same request, in registration order', async () => { + const order: string[] = []; + const baseUrl = await boot( + [ + () => order.push('metrics'), + () => order.push('access-log'), + ], + (server) => { + server.get('/both', async (_req, res) => res.json({ ok: true })); + }, + ); + + await fetch(`${baseUrl}/both`); + await new Promise((resolve) => setTimeout(resolve, 20)); + + expect(order).toEqual(['metrics', 'access-log']); + }); + + it('a THROWING observer affects neither the response nor its sibling', async () => { + const delivered: HttpResponseObservation[] = []; + const baseUrl = await boot( + [ + () => { + throw new Error('broken metrics backend'); + }, + (o) => delivered.push(o), + ], + (server) => { + server.get('/guarded', async (_req, res) => res.status(200).json({ ok: true })); + }, + ); + + const res = await fetch(`${baseUrl}/guarded`); + // The response is untouched — an observer can never break a request. + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ ok: true }); + await new Promise((resolve) => setTimeout(resolve, 20)); + + // The sibling registered AFTER the thrower still observed. + expect(delivered).toHaveLength(1); + expect(delivered[0].routePattern).toBe('/guarded'); + }); + + it('N requests, N observations — no double delivery on a busy route', async () => { + const seen: HttpResponseObservation[] = []; + const baseUrl = await boot([(o) => seen.push(o)], (server) => { + server.get('/counted', async (_req, res) => res.json({ ok: true })); + }); + + await fetch(`${baseUrl}/counted`); + await fetch(`${baseUrl}/counted`); + await fetch(`${baseUrl}/counted`); + await new Promise((resolve) => setTimeout(resolve, 20)); + + expect(seen).toHaveLength(3); + expect(new Set(seen.map((o) => o.routePattern))).toEqual(new Set(['/counted'])); + }); +}); diff --git a/packages/runtime/src/dispatcher-plugin.ts b/packages/runtime/src/dispatcher-plugin.ts index 8fd4bcc979..e551931b92 100644 --- a/packages/runtime/src/dispatcher-plugin.ts +++ b/packages/runtime/src/dispatcher-plugin.ts @@ -24,6 +24,7 @@ import { NoopMetricsRegistry, NoopErrorReporter, instrumentRouteHandler, + armHttpRequestCounter, type MetricsRegistry, type ErrorReporter, } from './observability/index.js'; @@ -697,6 +698,38 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu * through unchanged. */ const rawServer = server; + // ── `http_requests_total` on the transport seam (#9835) ──── + // A transport implementing `IHttpServer.afterResponse` OWNS the + // request counter for every inbound request on it (the 2026-08-18 + // ruling on #9650 put the counter at the transport; the contract + // JSDoc records the ownership rule). Two consequences here, both + // feature-detected runtime-real per the contract: + // + // 1. When the host wired a registry to THIS plugin, offer it to + // the transport seam. `armHttpRequestCounter` latches + // per-server, first caller wins — a transport plugin that + // already armed its own registry in Phase 1 keeps ownership + // and this call is a no-op, so one registry handed to both + // layers (the ordinary wiring) can never double-count. A host + // that wired ONLY the dispatcher — the wiring the docs + // demonstrate — now gets every inbound surface counted, not + // just the dispatcher's own routes. + // 2. The per-route wrapper below stops emitting its copy of the + // counter (`emitHttpRequestsTotal: false`): the seam already + // counts these routes, and the duplicate would land on the + // same series under the same labels — the measured #9833 + // distortion, counting ONLY the dispatcher's routes twice. + // + // On a transport WITHOUT the seam both revert to the legacy + // behavior: the wrapper counts the dispatcher's own routes, and + // (documented expectation, #9650 ruling) every other surface on + // that transport reports no HTTP metrics — zero there is "not + // instrumented", never "no traffic". + const transportCountsRequests = + typeof (rawServer as IHttpServer).afterResponse === 'function'; + if (transportCountsRequests && config.observability?.metrics) { + armHttpRequestCounter(rawServer as IHttpServer, config.observability.metrics); + } server = new Proxy(rawServer, { get(target, prop, receiver) { if (prop === 'get' || prop === 'post' || prop === 'delete') { @@ -712,6 +745,7 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu errorReporter, generateRequestId, requestIdHeader, + emitHttpRequestsTotal: !transportCountsRequests, }), ); }; diff --git a/packages/runtime/src/http-metrics-inbound-coverage.hono.integration.test.ts b/packages/runtime/src/http-metrics-inbound-coverage.hono.integration.test.ts index af27ba289b..babb0b1cbc 100644 --- a/packages/runtime/src/http-metrics-inbound-coverage.hono.integration.test.ts +++ b/packages/runtime/src/http-metrics-inbound-coverage.hono.integration.test.ts @@ -34,7 +34,10 @@ import { createDispatcherPlugin } from './dispatcher-plugin.js'; * MEASUREMENTS that disqualified the other candidate seams, so the ruling's * evidence stays executable rather than becoming a claim in a comment. §4 * pins the transport seam's own edges (label shape, refused requests, - * resolution chain, and the double count it leaves behind). + * resolution chain, and the #9833 double count — retired by #9835, whose + * `toBe(1)` now pins the retirement). §5 pins the `IHttpServer.afterResponse` + * contract seam (#9835) on the same booted composition: the ruled successor + * that makes the observation point transport-agnostic instead of Hono-only. * * ## Why the composition is shaped this way * @@ -178,7 +181,8 @@ describe('#9650 §1 — inbound coverage of http_requests_total', () => { // Positive control. If this is 0 the harness is wrong, not the repo — // every "not counted" below would then be measuring a broken injection - // rather than the defect. + // rather than the defect. (Since #9835 the emitter behind this row is the + // transport seam, not the dispatcher's per-route proxy — see §5.) it('CONTROL: counts the dispatcher\'s own route (the local instrumented proxy works)', () => { expect(metrics.totalCounter(HTTP_REQUESTS_TOTAL, { route: DISPATCHER_PROBE })).toBeGreaterThan(0); }); @@ -647,7 +651,7 @@ describe('#9650 §4 — the transport seam that was ruled, and its edges', () => expect(metrics.totalCounter(HTTP_REQUESTS_TOTAL, { route: `${AUTH_BASE}/*` })).toBe(1); }, 30_000); - it('installs NO middleware when the host configured no metrics backend', async () => { + it('arms NO counter when the host configured no metrics backend', async () => { const kernel = new LiteKernel(); kernel.use(new HonoServerPlugin({ port: 0, cors: false })); kernel.use(authLikePlugin()); @@ -657,38 +661,163 @@ describe('#9650 §4 — the transport seam that was ruled, and its edges', () => const res = await fetch(`${baseUrl}${AUTH_PROBE}`, { method: 'POST' }); await kernel.shutdown(); - // Not a disabled counter — no counter. An unconfigured deployment - // pays no per-request cost, and the server is unaffected either way. + // Not a disabled counter — no counter. Since #9835 the `afterResponse` + // DELIVERY seam is mounted regardless (so consumers may register + // observers at any time), disarmed to one array-length check per + // request when nobody observes; the counter itself exists only when a + // registry was resolved. The server is unaffected either way. expect(res.status).toBe(200); }, 30_000); /** - * ⚠️ KNOWN, FILED CONSEQUENCE — not an endorsement. + * The #9833 double count, RETIRED by #9835 (this expectation was `toBe(2)` + * while the dispatcher's Proxy still emitted its own copy — the file's + * history keeps the measured defect). Two mechanisms compose here, both + * of them the `IHttpServer.afterResponse` contract's ownership rule: * - * The runtime dispatcher still wraps its OWN routes with - * `instrumentRouteHandler` (`dispatcher-plugin.ts` Proxy), which emits the - * same counter under the same labels. A host that hands one registry to - * both the transport and the dispatcher therefore counts the dispatcher's - * routes twice — and only the dispatcher's, so the ratio between surfaces - * is wrong, not just the scale. - * - * Removing that emission is a `packages/runtime` change and it is NOT - * separable from the rest of `instrumentRouteHandler` (request-id header, - * `http_request_duration_ms`, `http_request_errors_total`, the error - * reporter), so it is filed rather than folded in here. When it lands, - * this expectation becomes `toBe(1)`. + * - the dispatcher feature-detects the seam on the transport and passes + * `emitHttpRequestsTotal: false` to `instrumentRouteHandler`, so its + * per-route wrapper no longer emits the counter; + * - both counter-arming sites (`HonoServerPlugin.init()` and the + * dispatcher's own `observability.metrics` offer) route through + * `armHttpRequestCounter`, whose per-server latch makes "exactly one + * counter-emitting observer per server" structural — one registry + * handed to both layers arms ONCE (first caller wins; here the + * transport plugin, in Phase 1). */ - it('MEASURED: the dispatcher route is counted TWICE while its own Proxy still emits', async () => { + it('the dispatcher route is counted ONCE — the #9833 duplicate is retired (#9835)', async () => { const metrics = new InMemoryMetricsRegistry(); const { kernel, baseUrl } = await bootMeasurementKernel(metrics); await fetch(`${baseUrl}${DISPATCHER_PROBE}`); await fetch(`${baseUrl}${AUTH_PROBE}`, { method: 'POST' }); await kernel.shutdown(); - expect(metrics.totalCounter(HTTP_REQUESTS_TOTAL, { route: DISPATCHER_PROBE })).toBe(2); - // One request each. The surface only the transport seam covers is - // counted ONCE, which is what makes the row above a duplicate rather - // than a uniform scale factor an operator could divide out. + expect(metrics.totalCounter(HTTP_REQUESTS_TOTAL, { route: DISPATCHER_PROBE })).toBe(1); + // One request each, one count each: the ratio between surfaces — + // which is what the 5xx-rate and traffic-share guidance reads — is + // now true, not merely proportional. expect(metrics.totalCounter(HTTP_REQUESTS_TOTAL, { route: `${AUTH_BASE}/*` })).toBe(1); }, 30_000); }); + +describe('#9835 §5 — the `afterResponse` contract seam, on the real booted composition', () => { + it('feature-detects runtime-real on the registered `http.server` — the ask a consumer must make', async () => { + const metrics = new InMemoryMetricsRegistry(); + const { kernel } = await bootMeasurementKernel(metrics); + const httpServer = kernel.getService('http.server'); + await kernel.shutdown(); + + // `typeof === 'function'`, the contract's detection idiom (#5122: + // must be runtime-real — a wrapper that erases the member makes an + // instrumented transport read as "not instrumented"). + expect(typeof httpServer.afterResponse === 'function').toBe(true); + }, 30_000); + + it('fires once per request with STATUS + PATTERN — after boot, with zero Phase-1 choreography', async () => { + const metrics = new InMemoryMetricsRegistry(); + const { kernel, baseUrl } = await bootMeasurementKernel(metrics); + const httpServer = kernel.getService('http.server'); + + // Registered AFTER bootstrap — the plugin mounted the delivery seam + // in Phase 1, so an observer may join at any moment and still see + // every surface. This is what the raw-app middleware measurements in + // §3 could not offer (register-late saw nothing). + const seen: Array<{ method: string; routePattern: string; status: number; elapsedMs: number }> = []; + httpServer.afterResponse!((observation) => seen.push({ ...observation })); + + await fetch(`${baseUrl}/api/v1/data/abc123`); + await fetch(`${baseUrl}${AUTH_PROBE}`, { method: 'POST' }); + await kernel.shutdown(); + + const paramRow = seen.find((o) => o.routePattern === REST_PARAM_ROUTE); + expect(paramRow).toBeDefined(); + // The PATTERN, never the concrete path — the contract's hard + // cardinality requirement, now asserted on the contract seam itself. + expect(seen.map((o) => o.routePattern)).not.toContain('/api/v1/data/abc123'); + expect(paramRow!.status).toBe(200); + expect(paramRow!.method).toBe('GET'); + expect(paramRow!.elapsedMs).toBeGreaterThanOrEqual(0); + // The raw-app auth mount is inside the seam's reach, labelled by its + // registered wildcard — the transport-agnostic seam keeps the reach + // the ruled Hono middleware measured (#9650: do not regress). + const authRow = seen.find((o) => o.routePattern === `${AUTH_BASE}/*`); + expect(authRow).toBeDefined(); + expect(authRow!.status).toBe(200); + }, 30_000); + + it('a host that wires metrics ONLY on the dispatcher now counts EVERY surface through the seam', async () => { + // The wiring the production-readiness docs demonstrate. Before #9835 + // it reached only the dispatcher's own routes (the pre-#9650 blind + // spot); the dispatcher now offers its registry to the transport seam + // via `armHttpRequestCounter`, so the auth raw-app mount and the REST + // RouteManager mount are counted too. + const metrics = new InMemoryMetricsRegistry(); + const kernel = new LiteKernel(); + kernel.use(new HonoServerPlugin({ port: 0, cors: false })); // no registry here + kernel.use(authLikePlugin()); + kernel.use(restLikePlugin()); + kernel.use( + createDispatcherPlugin({ + prefix: '/api/v1', + securityHeaders: false, + observability: { metrics }, + } as any), + ); + await kernel.bootstrap(); + const httpServer = kernel.getService('http.server'); + const baseUrl = `http://127.0.0.1:${httpServer.getPort!()}`; + + await fetch(`${baseUrl}${DISPATCHER_PROBE}`); + await fetch(`${baseUrl}${AUTH_PROBE}`, { method: 'POST' }); + await fetch(`${baseUrl}${REST_PROBE}`); + await kernel.shutdown(); + + expect(metrics.totalCounter(HTTP_REQUESTS_TOTAL, { route: DISPATCHER_PROBE })).toBe(1); + expect(metrics.totalCounter(HTTP_REQUESTS_TOTAL, { route: `${AUTH_BASE}/*` })).toBe(1); + expect(metrics.totalCounter(HTTP_REQUESTS_TOTAL, { route: REST_PROBE })).toBe(1); + }, 30_000); + + it('NO-DOUBLE-COUNT: an extra observer changes nothing, and every surface stays at exactly 1', async () => { + // The card's de-duplication acceptance, pinned across all three + // potential emitters at once: the transport's armed counter (Phase + // 1), the dispatcher's registry offer (Phase 2, latched away), and + // the dispatcher's per-route wrapper (suppressed by feature + // detection). A second afterResponse OBSERVER is registered too — + // observation is many-consumer by contract, while the COUNTER stays + // single-owner. + const metrics = new InMemoryMetricsRegistry(); + const { kernel, baseUrl } = await bootMeasurementKernel(metrics); + const httpServer = kernel.getService('http.server'); + const observed: string[] = []; + httpServer.afterResponse!((o) => observed.push(o.routePattern)); + + await fetch(`${baseUrl}${DISPATCHER_PROBE}`); + await fetch(`${baseUrl}${AUTH_PROBE}`, { method: 'POST' }); + await fetch(`${baseUrl}${REST_PROBE}`); + await kernel.shutdown(); + + expect(metrics.totalCounter(HTTP_REQUESTS_TOTAL, { route: DISPATCHER_PROBE })).toBe(1); + expect(metrics.totalCounter(HTTP_REQUESTS_TOTAL, { route: `${AUTH_BASE}/*` })).toBe(1); + expect(metrics.totalCounter(HTTP_REQUESTS_TOTAL, { route: REST_PROBE })).toBe(1); + // The extra observer really was live the whole time — it observed all + // three requests without adding a single count. + expect(observed).toHaveLength(3); + }, 30_000); + + it('keeps the dispatcher-side signals the transport does not emit: duration histogram + X-Request-Id', async () => { + // Only the COUNTER moved to the transport seam. The per-route wrapper + // still owns request-id echo and `http_request_duration_ms` — gating + // those on the seam would have silently dropped them (#9833 named + // them as the non-separable remainder). + const metrics = new InMemoryMetricsRegistry(); + const { kernel, baseUrl } = await bootMeasurementKernel(metrics); + const res = await fetch(`${baseUrl}${DISPATCHER_PROBE}`); + await kernel.shutdown(); + + expect(res.headers.get('X-Request-Id')).toBeTruthy(); + const durations = metrics.samples.filter( + (s) => s.name === RUNTIME_METRICS.httpRequestDurationMs && s.labels.route === DISPATCHER_PROBE, + ); + expect(durations.length).toBe(1); + }, 30_000); +}); diff --git a/packages/runtime/src/observability/index.ts b/packages/runtime/src/observability/index.ts index e4ee16db14..cddbac6437 100644 --- a/packages/runtime/src/observability/index.ts +++ b/packages/runtime/src/observability/index.ts @@ -13,6 +13,8 @@ export { NoopMetricsRegistry, InMemoryMetricsRegistry, RUNTIME_METRICS, + armHttpRequestCounter, + type ArmHttpRequestCounterResult, type MetricsRegistry, type MetricSample, } from './metrics.js'; diff --git a/packages/runtime/src/observability/instrument.test.ts b/packages/runtime/src/observability/instrument.test.ts index f553342d6e..11f67f1653 100644 --- a/packages/runtime/src/observability/instrument.test.ts +++ b/packages/runtime/src/observability/instrument.test.ts @@ -146,6 +146,64 @@ describe('instrumentRouteHandler', () => { ).toEqual([15]); }); + it('emitHttpRequestsTotal: false suppresses ONLY the request counter — histogram, error counter, reporter and request-id stay on (#9835)', async () => { + // The dispatcher passes this when the transport implements the + // `IHttpServer.afterResponse` seam, which then owns the counter + // (#9833's duplicate). Everything else the wrapper emits is NOT + // emitted by the transport seam and must survive the gate. + const clock = makeClock(); + const wrapped = instrumentRouteHandler( + 'GET', + '/health', + async () => { + clock.advance(15); + }, + { metrics, now: clock.now, emitHttpRequestsTotal: false }, + ); + const res = makeRes(); + await wrapped({ headers: {} }, res); + expect( + metrics.totalCounter('http_requests_total', { route: '/health' }), + ).toBe(0); + expect( + metrics.histogramValues('http_request_duration_ms', { + method: 'GET', + route: '/health', + }), + ).toEqual([15]); + expect(res.headers['X-Request-Id']).toBeTruthy(); + + // The error counter is likewise ungated. + const throwing = instrumentRouteHandler( + 'POST', + '/boom', + async () => { + throw new Error('kaboom'); + }, + { metrics, emitHttpRequestsTotal: false }, + ); + await expect(throwing({ headers: {} }, makeRes())).rejects.toThrow('kaboom'); + expect( + metrics.totalCounter('http_request_errors_total', { route: '/boom' }), + ).toBe(1); + expect( + metrics.totalCounter('http_requests_total', { route: '/boom' }), + ).toBe(0); + }); + + it('emitHttpRequestsTotal defaults to true — the legacy behavior for hook-less transports', async () => { + const wrapped = instrumentRouteHandler( + 'GET', + '/legacy', + async () => {}, + { metrics }, + ); + await wrapped({ headers: {} }, makeRes()); + expect( + metrics.totalCounter('http_requests_total', { route: '/legacy' }), + ).toBe(1); + }); + it('records the status as set by res.status() (e.g. 404)', async () => { const wrapped = instrumentRouteHandler( 'GET', diff --git a/packages/runtime/src/observability/instrument.ts b/packages/runtime/src/observability/instrument.ts index 791b83a1f4..99a7b76c63 100644 --- a/packages/runtime/src/observability/instrument.ts +++ b/packages/runtime/src/observability/instrument.ts @@ -29,6 +29,21 @@ export interface InstrumentOptions { * called, so any monotonic source works. */ now?: () => number; + /** + * Whether this wrapper emits the `http_requests_total` counter for the + * wrapped route. Default `true` — the legacy behavior for transports + * that offer no better seam. + * + * Pass `false` when the transport under the route implements the + * `IHttpServer.afterResponse` observation seam (#9835): there the + * TRANSPORT owns the counter for every inbound request — this wrapper's + * copy would land on the same series under the same labels and count the + * dispatcher's routes twice (the measured, per-surface distortion of + * #9833). Only the counter is gated: request-id echo, the duration + * histogram, the error counter and the error reporter are not emitted by + * the transport seam and always stay on. + */ + emitHttpRequestsTotal?: boolean; } /** @@ -38,8 +53,9 @@ export interface InstrumentOptions { * 1. Resolve a request id from incoming `X-Request-Id` (or mint one). * 2. Set the request id on `req.requestId` and response header. * 3. Time the handler. - * 4. Emit `http_requests_total{method,route,status}` counter and - * `http_request_duration_ms{method,route}` histogram. + * 4. Emit `http_requests_total{method,route,status}` counter (unless the + * transport owns it — see {@link InstrumentOptions.emitHttpRequestsTotal}) + * and the `http_request_duration_ms{method,route}` histogram. * 5. On thrown errors, emit `http_request_errors_total` and call * `errorReporter.captureException` for 5xx. * 6. When the handler catches its own error and calls @@ -60,6 +76,7 @@ export function instrumentRouteHandler( const generateRequestId = opts.generateRequestId; const requestIdHeader = opts.requestIdHeader ?? 'X-Request-Id'; const now = opts.now ?? Date.now; + const emitHttpRequestsTotal = opts.emitHttpRequestsTotal ?? true; return async (req: any, res: any) => { const requestId = resolveRequestId(req?.headers, generateRequestId); @@ -103,11 +120,16 @@ export function instrumentRouteHandler( throw err; } finally { const elapsed = now() - startedAt; - metrics.counter(RUNTIME_METRICS.httpRequestsTotal, { - method, - route, - status: String(status), - }); + // Gated (#9833/#9835): on a transport implementing the + // `afterResponse` seam the counter is the transport's — see + // `InstrumentOptions.emitHttpRequestsTotal`. + if (emitHttpRequestsTotal) { + metrics.counter(RUNTIME_METRICS.httpRequestsTotal, { + method, + route, + status: String(status), + }); + } metrics.histogram( RUNTIME_METRICS.httpRequestDurationMs, elapsed, diff --git a/packages/runtime/src/observability/metrics.ts b/packages/runtime/src/observability/metrics.ts index 19fe0e810b..e1023871c6 100644 --- a/packages/runtime/src/observability/metrics.ts +++ b/packages/runtime/src/observability/metrics.ts @@ -14,6 +14,8 @@ export { NoopMetricsRegistry, InMemoryMetricsRegistry, RUNTIME_METRICS, + armHttpRequestCounter, + type ArmHttpRequestCounterResult, type MetricsRegistry, type MetricSample, } from '@objectstack/observability'; diff --git a/packages/spec/api-surface/contracts.json b/packages/spec/api-surface/contracts.json index 7600f6c5d5..1189c79d8a 100644 --- a/packages/spec/api-surface/contracts.json +++ b/packages/spec/api-surface/contracts.json @@ -90,6 +90,8 @@ "HealthStatus (type)", "HierarchyScope (type)", "HierarchyScopeContext (interface)", + "HttpResponseObservation (interface)", + "HttpResponseObserver (type)", "IAIConversationService (interface)", "IAIService (interface)", "IAdvancedServiceRegistry (interface)", @@ -294,6 +296,7 @@ "ToolResultPart (interface)", "ToolSet (type)", "TransportSendResult (interface)", + "UNMATCHED_ROUTE_PATTERN (const)", "Unsubscribe (type)", "UploadArtifactInput (interface)", "UploadArtifactResult (interface)", diff --git a/packages/spec/export-origins/contracts.json b/packages/spec/export-origins/contracts.json index ace19b9c16..a780c8f450 100644 --- a/packages/spec/export-origins/contracts.json +++ b/packages/spec/export-origins/contracts.json @@ -90,6 +90,8 @@ "HealthStatus": "src/kernel/startup-orchestrator.zod.ts#HealthStatus (type)", "HierarchyScope": "src/contracts/sharing-service.ts#HierarchyScope (type)", "HierarchyScopeContext": "src/contracts/sharing-service.ts#HierarchyScopeContext (interface)", + "HttpResponseObservation": "src/contracts/http-server.ts#HttpResponseObservation (interface)", + "HttpResponseObserver": "src/contracts/http-server.ts#HttpResponseObserver (type)", "IAIConversationService": "src/contracts/ai-service.ts#IAIConversationService (interface)", "IAIService": "src/contracts/ai-service.ts#IAIService (interface)", "IAdvancedServiceRegistry": "src/contracts/service-registry.ts#IAdvancedServiceRegistry (interface)", @@ -294,6 +296,7 @@ "ToolResultPart": "node_modules/@ai-sdk/provider-utils/dist/index.d.ts#ToolResultPart (interface)", "ToolSet": "node_modules/@ai-sdk/provider-utils/dist/index.d.ts#ToolSet (type)", "TransportSendResult": "src/contracts/email-service.ts#TransportSendResult (interface)", + "UNMATCHED_ROUTE_PATTERN": "src/contracts/http-server.ts#UNMATCHED_ROUTE_PATTERN (const)", "Unsubscribe": "src/contracts/cluster-service.ts#Unsubscribe (type)", "UploadArtifactInput": "src/contracts/package-service.ts#UploadArtifactInput (interface)", "UploadArtifactResult": "src/contracts/package-service.ts#UploadArtifactResult (interface)", diff --git a/packages/spec/src/contracts/http-server.test.ts b/packages/spec/src/contracts/http-server.test.ts index a5f7d87c59..e5a517da8d 100644 --- a/packages/spec/src/contracts/http-server.test.ts +++ b/packages/spec/src/contracts/http-server.test.ts @@ -5,7 +5,10 @@ import type { RouteHandler, Middleware, IHttpServer, + HttpResponseObservation, + HttpResponseObserver, } from './http-server'; +import { UNMATCHED_ROUTE_PATTERN } from './http-server'; describe('HTTP Server Contract', () => { describe('IHttpRequest interface', () => { @@ -310,6 +313,150 @@ describe('HTTP Server Contract', () => { }); }); + describe('optional afterResponse (#9835)', () => { + /** A server with only the REQUIRED members. */ + const baseServer = (): IHttpServer => ({ + get: () => {}, + post: () => {}, + put: () => {}, + delete: () => {}, + patch: () => {}, + use: () => {}, + listen: async () => {}, + }); + + it('is optional — an adapter without it still satisfies the contract', () => { + const server = baseServer(); + + expect(typeof server.afterResponse).toBe('undefined'); + // The documented consequence: this transport reports NO HTTP metrics. + // Zero on `http_requests_total` there means "not instrumented", + // never "no traffic" — a consumer must ask, exactly like this: + expect(typeof server.afterResponse === 'function').toBe(false); + }); + + it('is feature-detected with typeof === "function" when provided (runtime-real, not type-only)', () => { + const server: IHttpServer = { + ...baseServer(), + afterResponse: (_observer) => {}, + }; + + expect(typeof server.afterResponse).toBe('function'); + }); + + it('a delegating wrapper that forwards only required members ERASES detection (#5122 shape)', () => { + const underlying: IHttpServer = { + ...baseServer(), + afterResponse: (_observer) => {}, + }; + // The failure mode the contract warns wrappers against: forward the + // required members, drop the optional ones. + const erasingWrapper: IHttpServer = { + get: underlying.get, + post: underlying.post, + put: underlying.put, + delete: underlying.delete, + patch: underlying.patch, + use: underlying.use, + listen: underlying.listen, + }; + // The underlying adapter implements the hook; the wrapper makes the + // contract's own detection idiom read "not instrumented". + expect(typeof underlying.afterResponse === 'function').toBe(true); + expect(typeof erasingWrapper.afterResponse === 'function').toBe(false); + // The compliant wrapper forwards conditionally — present iff wrapped. + const forwardingWrapper: IHttpServer = { + ...erasingWrapper, + ...(typeof underlying.afterResponse === 'function' + ? { afterResponse: underlying.afterResponse.bind(underlying) } + : {}), + }; + expect(typeof forwardingWrapper.afterResponse === 'function').toBe(true); + }); + + it('registration APPENDS — several observers coexist, unlike setFallbackHandler', () => { + const observers: HttpResponseObserver[] = []; + const server: IHttpServer = { + ...baseServer(), + afterResponse: (observer) => { observers.push(observer); }, + }; + + const metricsObserver: HttpResponseObserver = () => {}; + const accessLogObserver: HttpResponseObserver = () => {}; + server.afterResponse!(metricsObserver); + server.afterResponse!(accessLogObserver); + + // Both registered, registration order kept — nothing replaced. + expect(observers).toEqual([metricsObserver, accessLogObserver]); + }); + + it('carries the observation shape: method, routePattern (the PATTERN), status, elapsedMs', () => { + const seen: HttpResponseObservation[] = []; + const observer: HttpResponseObserver = (observation) => { seen.push(observation); }; + + // What a compliant adapter reports for GET /api/v1/data/rec_42 + // answered by the registered route `/api/v1/data/:id`: the PATTERN, + // never the concrete path — the hard requirement the contract states + // so no adapter re-decides cardinality. + observer({ + method: 'GET', + routePattern: '/api/v1/data/:id', + status: 200, + elapsedMs: 3, + }); + + expect(seen).toHaveLength(1); + expect(seen[0].routePattern).toBe('/api/v1/data/:id'); + expect(seen[0].routePattern).not.toContain('rec_42'); + expect(seen[0].status).toBe(200); + expect(typeof seen[0].elapsedMs).toBe('number'); + }); + + it('reports a request no route matched with the reserved UNMATCHED_ROUTE_PATTERN', () => { + // Reserved in the CONTRACT, not per adapter, so every transport + // reports the same spelling and unrouted traffic is one series. + expect(UNMATCHED_ROUTE_PATTERN).toBe('unmatched'); + + const seen: HttpResponseObservation[] = []; + const observer: HttpResponseObserver = (observation) => { seen.push(observation); }; + observer({ + method: 'GET', + routePattern: UNMATCHED_ROUTE_PATTERN, + status: 404, + elapsedMs: 1, + }); + expect(seen[0].routePattern).toBe('unmatched'); + }); + + it('a throwing observer must not affect the response or sibling observers — the delivery contract, modelled', () => { + const observers: HttpResponseObserver[] = []; + const server: IHttpServer = { + ...baseServer(), + afterResponse: (observer) => { observers.push(observer); }, + }; + + const delivered: string[] = []; + server.afterResponse!(() => { throw new Error('broken metrics backend'); }); + server.afterResponse!(() => { delivered.push('access-log'); }); + + // How a compliant adapter delivers: each observer isolated, failures + // swallowed — a metrics backend must never break a response. + const deliver = (observation: HttpResponseObservation) => { + for (const observer of observers) { + try { + observer(observation); + } catch { + /* observer failures never propagate */ + } + } + }; + expect(() => + deliver({ method: 'GET', routePattern: '/x', status: 200, elapsedMs: 0 }), + ).not.toThrow(); + expect(delivered).toEqual(['access-log']); + }); + }); + it('should listen on a port', async () => { let listenedPort: number | undefined; diff --git a/packages/spec/src/contracts/http-server.ts b/packages/spec/src/contracts/http-server.ts index a7dcd3383c..09bfeb1d50 100644 --- a/packages/spec/src/contracts/http-server.ts +++ b/packages/spec/src/contracts/http-server.ts @@ -137,6 +137,51 @@ export type Middleware = ( next: () => void | Promise ) => void | Promise; +/** + * The reserved route label for a request that matched NO registered route — + * the one non-pattern value {@link HttpResponseObservation.routePattern} may + * carry. Reserved here (not per adapter) so every transport reports the same + * spelling and dashboards can rely on one series for unrouted traffic. + */ +export const UNMATCHED_ROUTE_PATTERN = 'unmatched'; + +/** + * What a transport reports about ONE answered request, after the response + * exists — the payload handed to every {@link HttpResponseObserver}. See the + * CONTRACT on {@link IHttpServer.afterResponse} for the semantics of each + * field; the hard requirement worth restating at the type itself: + * `routePattern` is the registered route PATTERN (`/api/v1/data/:id`), never + * the concrete request path. + */ +export interface HttpResponseObservation { + /** HTTP method of the request, uppercase (`GET`, `POST`, …). */ + method: string; + /** + * The registered route pattern that answered (`/api/v1/data/:id`, + * `/api/v1/auth/*`) — NEVER the concrete path (`/api/v1/data/rec_42`), + * which would mint one metric series per record id. A request no + * registered route matched carries {@link UNMATCHED_ROUTE_PATTERN}. + */ + routePattern: string; + /** Numeric HTTP status of the response as sent (`200`, `429`, `500`, …). */ + status: number; + /** + * Wall-clock milliseconds from the transport first seeing the request to + * the response existing. Precision is adapter-defined — consumers must + * not assume sub-millisecond fidelity. + */ + elapsedMs: number; +} + +/** + * A response observer — registered via {@link IHttpServer.afterResponse}, + * invoked by the transport once per answered request. Observation only: it + * has no channel back into the response, and a throwing observer must never + * affect the response or sibling observers (implementations swallow observer + * failures). + */ +export type HttpResponseObserver = (observation: HttpResponseObservation) => void; + /** * IHttpServer - HTTP Server capability interface * @@ -354,4 +399,85 @@ export interface IHttpServer { * @param handler - The handler to invoke for otherwise-unmatched requests */ setFallbackHandler?(handler: RouteHandler): void; + + /** + * Register a RESPONSE OBSERVER: a callback the transport invokes once per + * inbound request, AFTER the response exists. + * + * ## Why this member exists (#9835; ruled 2026-08-18 on #9650) + * + * The {@link use} middleware chain deliberately runs BEFORE dispatch: the + * adapter runs the whole chain and only then continues into routing, so a + * middleware there sees method, path, query and headers and has NO + * response — it cannot carry the `status` label the operator guidance + * (5xx rate) is keyed on (measured in `packages/runtime/src/ + * http-metrics-inbound-coverage.hono.integration.test.ts`). This member + * is the observation point that contract cannot express: it runs after + * the response exists, status known, elapsed time measurable — which is + * what makes HTTP metrics transport-agnostic instead of Hono-only. + * + * ## Observation contract + * + * - Each registered observer is invoked EXACTLY ONCE per request the + * transport answers, after the response status is known. A connection + * that dies before any response is written may go unobserved. + * - **`routePattern` MUST be the registered route PATTERN that answered + * (`/api/v1/data/:id`, `/api/v1/auth/*`) — NEVER the concrete request + * path.** A hard requirement, stated here precisely so no adapter + * re-decides cardinality: labelling by concrete path mints one metric + * series per record id and silently breaks the alerting the counter + * exists for. A request no registered route matched is reported with + * the reserved {@link UNMATCHED_ROUTE_PATTERN}. + * - **Reach**: implementations MUST deliver an observation for every + * inbound request the transport serves — including routes mounted on + * the framework-native handle behind {@link getRawApp} and requests a + * {@link use} middleware short-circuits (a rate limiter's 429). A + * transport built-in that answers before the observation point (e.g. a + * CORS preflight) may fall outside it; each adapter documents its own + * boundary. + * - Registration APPENDS — several observers may coexist (metrics, + * access log), invoked in registration order; there is no unregister + * (an observer lives as long as the server). Unlike + * {@link setFallbackHandler}, registering again never replaces. + * - An observer that throws MUST NOT affect the response or sibling + * observers — implementations swallow observer failures (the same + * discipline a metrics backend is held to). + * + * ## Optionality is visible — and MUST stay runtime-real + * + * Optional and feature-detected with + * `typeof server.afterResponse === 'function'`, like {@link getRawApp}. + * Detection must be runtime-real, not type-only: a wrapper or Proxy over + * an `IHttpServer` that forwards only required members erases this one + * and makes detection read false against an adapter that implements it — + * the optional-member-erasure shape #5122 records. A wrapper MUST forward + * it conditionally: present iff the wrapped server provides it. + * + * ## Who owns `http_requests_total` emission (de-duplication; #9833) + * + * The 2026-08-18 ruling on #9650 puts the counter at the TRANSPORT: it is + * emitted by exactly ONE counter-emitting observer per server, registered + * through this hook by the transport's own composition layer (its hosting + * plugin) when a metrics backend is wired. A downstream consumer that + * holds both this server and a metrics registry (the runtime dispatcher's + * `instrumentRouteHandler` wrapper) MUST NOT add a second per-request + * counter while this member is implemented: it feature-detects the hook + * and suppresses its own counter — keeping its non-duplicated signals + * (request-id echo, duration histogram, error counter/reporter). A + * request must never be double-counted between the transport seam and a + * consumer-side wrapper. + * + * ## A transport that does not implement this seam reports NO HTTP metrics + * + * Stated plainly, per the ruling, rather than letting absence read as + * coverage: on such a transport `http_requests_total` never increments — + * **zero means "not instrumented", never "no traffic"**. A consumer that + * needs the distinction must ASK (feature-detect this member) and surface + * the answer; it must never infer "no traffic" from an empty counter it + * never confirmed was armed. + * + * @param observer - Invoked once per answered request with the + * {@link HttpResponseObservation} + */ + afterResponse?(observer: HttpResponseObserver): void; }