fix(journal): bound Prometheus route label cardinality
The httpRequestDuration histogram labeled every request with the raw URL
path (`url.split("?")[0]`), so every distinct path — `/activities/<uuid>`,
`/routes/<uuid>`, probed usernames, crawler junk — became a permanent
label-set. prom-client never evicts label-sets, so RSS grew linearly for
the life of the process and reset only on restart. On the flagship this
showed as ~25 MiB/day of unbounded journal memory growth; a live
`/api/metrics` scrape held 2,655 histogram series across 291 distinct
`route` values (344 KB body), dominated by per-UUID activity paths.
Add `normalizeRoute()` in metrics.server.ts: it collapses dynamic path
segments back to the `:param` templates declared in app/routes.ts (UUID
and numeric segments -> `:id`; `:username`/`:provider` slots keyed by
their preceding static segment), strips query/fragment, and enforces a
hard cap (MAX_ROUTES) that funnels anything beyond the known route table
into a single `/other` bucket as an absolute backstop. Cardinality is now
proportional to the route table, not to traffic.
Logs still record the raw path (`logger.info`) — only the metric label is
bounded.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
9266233614
commit
cd73093beb
3 changed files with 153 additions and 2 deletions
89
apps/journal/app/lib/metrics.server.test.ts
Normal file
89
apps/journal/app/lib/metrics.server.test.ts
Normal file
|
|
@ -0,0 +1,89 @@
|
||||||
|
import { describe, it, expect, beforeEach } from "vitest";
|
||||||
|
import { normalizeRoute, _resetRouteNormalizationForTest } from "./metrics.server.ts";
|
||||||
|
|
||||||
|
describe("normalizeRoute", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
_resetRouteNormalizationForTest();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("passes static routes through unchanged", () => {
|
||||||
|
expect(normalizeRoute("/")).toBe("/");
|
||||||
|
expect(normalizeRoute("/feed")).toBe("/feed");
|
||||||
|
expect(normalizeRoute("/explore")).toBe("/explore");
|
||||||
|
expect(normalizeRoute("/settings/security")).toBe("/settings/security");
|
||||||
|
expect(normalizeRoute("/api/v1/routes")).toBe("/api/v1/routes");
|
||||||
|
expect(normalizeRoute("/legal/terms")).toBe("/legal/terms");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("collapses UUID segments to :id", () => {
|
||||||
|
expect(
|
||||||
|
normalizeRoute("/activities/0257f241-d880-4ca6-b073-c8706f07a293"),
|
||||||
|
).toBe("/activities/:id");
|
||||||
|
expect(
|
||||||
|
normalizeRoute("/routes/0257f241-d880-4ca6-b073-c8706f07a293/edit"),
|
||||||
|
).toBe("/routes/:id/edit");
|
||||||
|
expect(
|
||||||
|
normalizeRoute("/api/v1/activities/0257F241-D880-4CA6-B073-C8706F07A293"),
|
||||||
|
).toBe("/api/v1/activities/:id");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("collapses purely-numeric segments to :id", () => {
|
||||||
|
expect(normalizeRoute("/api/notifications/12345/read")).toBe(
|
||||||
|
"/api/notifications/:id/read",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("collapses usernames to :username after a `users` segment", () => {
|
||||||
|
expect(normalizeRoute("/users/alice")).toBe("/users/:username");
|
||||||
|
expect(normalizeRoute("/users/bob/followers")).toBe(
|
||||||
|
"/users/:username/followers",
|
||||||
|
);
|
||||||
|
expect(normalizeRoute("/api/users/carol/follow")).toBe(
|
||||||
|
"/api/users/:username/follow",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("collapses sync providers to :provider", () => {
|
||||||
|
expect(normalizeRoute("/api/sync/connect/wahoo")).toBe(
|
||||||
|
"/api/sync/connect/:provider",
|
||||||
|
);
|
||||||
|
expect(normalizeRoute("/api/sync/callback/garmin")).toBe(
|
||||||
|
"/api/sync/callback/:provider",
|
||||||
|
);
|
||||||
|
expect(
|
||||||
|
normalizeRoute(
|
||||||
|
"/api/sync/push/komoot/0257f241-d880-4ca6-b073-c8706f07a293",
|
||||||
|
),
|
||||||
|
).toBe("/api/sync/push/:provider/:id");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("strips query strings and fragments before normalizing", () => {
|
||||||
|
expect(
|
||||||
|
normalizeRoute("/activities/0257f241-d880-4ca6-b073-c8706f07a293?foo=bar"),
|
||||||
|
).toBe("/activities/:id");
|
||||||
|
expect(normalizeRoute("/feed#section")).toBe("/feed");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("preserves a dotted version segment (does not treat 2.1 as numeric)", () => {
|
||||||
|
expect(normalizeRoute("/nodeinfo/2.1")).toBe("/nodeinfo/2.1");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns the same template for two different ids (bounded cardinality)", () => {
|
||||||
|
const a = normalizeRoute("/activities/0257f241-d880-4ca6-b073-c8706f07a293");
|
||||||
|
const b = normalizeRoute("/activities/ffffffff-d880-4ca6-b073-c8706f07a293");
|
||||||
|
expect(a).toBe(b);
|
||||||
|
expect(a).toBe("/activities/:id");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("collapses unbounded junk to /other once the cap is exceeded", () => {
|
||||||
|
// Fill the tracking set with distinct unmatched paths up to the cap.
|
||||||
|
for (let i = 0; i < 200; i++) {
|
||||||
|
expect(normalizeRoute(`/scan-${i}`)).toBe(`/scan-${i}`);
|
||||||
|
}
|
||||||
|
// The next novel path can't be tracked, so it collapses to /other.
|
||||||
|
expect(normalizeRoute("/scan-200")).toBe("/other");
|
||||||
|
expect(normalizeRoute("/another-novel-path")).toBe("/other");
|
||||||
|
// Already-tracked paths are still returned verbatim.
|
||||||
|
expect(normalizeRoute("/scan-0")).toBe("/scan-0");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
@ -49,3 +49,65 @@ export const demoBotSyntheticActivitiesTotal = getOrCreate(
|
||||||
);
|
);
|
||||||
|
|
||||||
export const registry = client.register;
|
export const registry = client.register;
|
||||||
|
|
||||||
|
// --- Route label normalization -------------------------------------------
|
||||||
|
//
|
||||||
|
// The `route` label on httpRequestDuration MUST be bounded. prom-client
|
||||||
|
// stores one label-set per distinct combination and NEVER evicts, so
|
||||||
|
// feeding it raw request paths (`/activities/<uuid>`, probed usernames,
|
||||||
|
// crawler junk) leaks memory linearly for the life of the process — it
|
||||||
|
// only resets on restart. `normalizeRoute` collapses dynamic path
|
||||||
|
// segments back to the `:param` templates declared in app/routes.ts so
|
||||||
|
// cardinality stays proportional to the route table, not to traffic.
|
||||||
|
|
||||||
|
const UUID_RE =
|
||||||
|
/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
|
||||||
|
|
||||||
|
// A dynamic segment is keyed by the static segment that precedes it,
|
||||||
|
// mirroring the non-id `:param` slots in app/routes.ts. (`:id` params are
|
||||||
|
// all UUIDs and get caught by UUID_RE on their own.)
|
||||||
|
const DYNAMIC_AFTER = new Map<string, string>([
|
||||||
|
["users", ":username"], // /users/:username, /api/users/:username/...
|
||||||
|
["connect", ":provider"], // /api/sync/connect/:provider
|
||||||
|
["callback", ":provider"], // /api/sync/callback/:provider
|
||||||
|
["disconnect", ":provider"], // /api/sync/disconnect/:provider
|
||||||
|
["webhook", ":provider"], // /api/sync/webhook/:provider
|
||||||
|
["push", ":provider"], // /api/sync/push/:provider/:routeId
|
||||||
|
]);
|
||||||
|
|
||||||
|
// Absolute backstop. Real route templates number well under this; once we
|
||||||
|
// have seen this many distinct normalized routes (e.g. an aggressive
|
||||||
|
// scanner hitting paths that match no `:param` rule), everything new
|
||||||
|
// collapses to a single `/other` bucket so cardinality can never run away.
|
||||||
|
const MAX_ROUTES = 200;
|
||||||
|
const seenRoutes = new Set<string>();
|
||||||
|
|
||||||
|
/** Collapse a request path's dynamic segments to a bounded route template. */
|
||||||
|
export function normalizeRoute(pathname: string): string {
|
||||||
|
const path = (pathname.split("?")[0] ?? "/").split("#")[0] || "/";
|
||||||
|
if (path === "/") return "/";
|
||||||
|
|
||||||
|
const out: string[] = [];
|
||||||
|
for (const seg of path.split("/")) {
|
||||||
|
if (seg === "") continue;
|
||||||
|
const prev = out.length > 0 ? out[out.length - 1]! : "";
|
||||||
|
if (UUID_RE.test(seg) || /^\d+$/.test(seg)) {
|
||||||
|
out.push(":id");
|
||||||
|
} else if (DYNAMIC_AFTER.has(prev)) {
|
||||||
|
out.push(DYNAMIC_AFTER.get(prev)!);
|
||||||
|
} else {
|
||||||
|
out.push(seg);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
const normalized = "/" + out.join("/");
|
||||||
|
|
||||||
|
if (seenRoutes.has(normalized)) return normalized;
|
||||||
|
if (seenRoutes.size >= MAX_ROUTES) return "/other";
|
||||||
|
seenRoutes.add(normalized);
|
||||||
|
return normalized;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Test-only: clear the distinct-route tracking set. */
|
||||||
|
export function _resetRouteNormalizationForTest(): void {
|
||||||
|
seenRoutes.clear();
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -6,7 +6,7 @@ import { createReadStream, statSync } from "node:fs";
|
||||||
import { join, extname, resolve } from "node:path";
|
import { join, extname, resolve } from "node:path";
|
||||||
import { logger, requestContext } from "./app/lib/logger.server.ts";
|
import { logger, requestContext } from "./app/lib/logger.server.ts";
|
||||||
import { randomUUID } from "node:crypto";
|
import { randomUUID } from "node:crypto";
|
||||||
import { httpRequestDuration, registry } from "./app/lib/metrics.server.ts";
|
import { httpRequestDuration, normalizeRoute, registry } from "./app/lib/metrics.server.ts";
|
||||||
import { createBoss, startWorker } from "@trails-cool/jobs";
|
import { createBoss, startWorker } from "@trails-cool/jobs";
|
||||||
import { getDatabaseUrl } from "@trails-cool/db";
|
import { getDatabaseUrl } from "@trails-cool/db";
|
||||||
import postgres, { type Sql } from "postgres";
|
import postgres, { type Sql } from "postgres";
|
||||||
|
|
@ -117,7 +117,7 @@ const server = createServer((req, res) => {
|
||||||
const duration = Date.now() - start;
|
const duration = Date.now() - start;
|
||||||
logger.info({ method: req.method, path: url, status: res.statusCode, duration }, "request");
|
logger.info({ method: req.method, path: url, status: res.statusCode, duration }, "request");
|
||||||
httpRequestDuration.observe(
|
httpRequestDuration.observe(
|
||||||
{ method: req.method ?? "GET", route: url.split("?")[0]!, status: String(res.statusCode) },
|
{ method: req.method ?? "GET", route: normalizeRoute(url), status: String(res.statusCode) },
|
||||||
duration / 1000,
|
duration / 1000,
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue