Files
gitmost/apps/server/src/integrations/metrics/http-metrics.hook.ts
T
agent_coder 43b11d92ab fix(#362 review F1-F3): add /icons/ prefix + honest comment + real boundary test
- F1: added '/icons/' to STATIC_PATH_PREFIXES — public/icons/ is copied verbatim
  to client/dist (a sibling of the already-included brand/vad/locales), and
  index.html references /icons/favicon-*.png on every page load, so those requests
  were getting their own route labels instead of collapsing to `static`.
- F2: corrected the comment — only /assets/ is content-hashed (unbounded per
  deploy); /vad//brand//locales//icons/ have stable names (repetitive, not
  unbounded). Either way none belong in the API-route histogram.
- F3: the negative test now exercises the trailing-slash boundary (the actual
  anti-false-collapse guard): '/assets' (no slash), '/assetsx/foo.js',
  '/iconset/x.png' must NOT collapse to `static` — cases that a buggy
  includes()/slashless-prefix impl would wrongly collapse. Plus '/icons/*' added
  to the positive it.each.

Gate: server tsc 0; metrics.spec passes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-05 02:38:47 +03:00

68 lines
2.7 KiB
TypeScript

import { FastifyReply, FastifyRequest } from 'fastify';
import { isStreamingResponse } from './metrics.constants';
import { observeHttp } from './metrics.registry';
// URL path prefixes served by @fastify/static (client build output under
// client/dist). `/assets/` holds the content-hashed bundle (index-*.js,
// chunk-*.js) — a NEW set of names every deploy, i.e. an UNBOUNDED label set
// (#362); the others (/vad/, /brand/, /locales/, /icons/ — copied verbatim from
// public/) have stable names, so they are merely repetitive per-file labels
// rather than unbounded. Either way none of these belong in the API-route
// histogram: collapse them all to one bounded `static` label. (Edge latency for
// static is already measured by Traefik's traefik_router_request_duration_*.)
const STATIC_PATH_PREFIXES = [
'/assets/',
'/vad/',
'/brand/',
'/locales/',
'/icons/',
];
/**
* Resolve the BOUNDED route label for an HTTP response.
*
* HARD REQUIREMENT (#355): use the ROUTE TEMPLATE (`/pages/:id`), NEVER a raw
* URL (`/pages/abc-123` or `/assets/index-CAbxDtto.js`), so label cardinality
* stays finite. Fastify exposes the matched template on `req.routeOptions.url`,
* BUT @fastify/static serves each file through a route whose matched url is the
* raw (hashed) file path — so for static assets that value is itself unbounded.
* Detect static requests by their path prefix FIRST and collapse to `static`;
* otherwise use the route template; on a 404 (no route matched) → `unknown`.
*/
export function resolveRouteLabel(req: FastifyRequest): string {
const path = (req.url ?? '').split('?', 1)[0];
if (STATIC_PATH_PREFIXES.some((p) => path.startsWith(p))) return 'static';
const url = req.routeOptions?.url;
return typeof url === 'string' && url.length > 0 ? url : 'unknown';
}
/**
* Fastify onResponse handler that records http_request_duration_seconds.
* No-op when metrics are disabled (the hook is only registered when enabled,
* but the observe helpers are also guarded). Never throws into the response
* pipeline — telemetry must not break request handling.
*/
export function recordHttpResponse(
req: FastifyRequest,
reply: FastifyReply,
): void {
try {
const route = resolveRouteLabel(req);
// Exclude SSE/streaming responses: onResponse fires at connection close for
// those, so it would record the stream lifetime and poison p95/p99.
const contentType = reply.getHeader('content-type');
if (isStreamingResponse(contentType, route)) return;
observeHttp(
req.method,
route,
reply.statusCode,
// Fastify measures elapsed time in ms; the metric is in seconds.
reply.elapsedTime / 1000,
);
} catch {
// Swallow: a telemetry failure must never affect the served response.
}
}