Skip to content

Commit 5e13dbb

Browse files
feat(web): record HTTP request duration metrics (#1571)
* feat(web): record HTTP request duration metrics The web process now reports runtime metrics, but nothing about how long requests actually take, so per-endpoint latency is still invisible. That is the signal needed to see a stall from the outside: /api/health does almost no work, so its duration is essentially event loop queueing delay. Add an http_request_duration_seconds histogram labelled by method, route, and status, populated by subscribing to Node's built-in http.server.request.start and http.server.response.finish diagnostics channels. Next.js owns the server instance in a standalone build, so there is no request pipeline to wrap; the channels observe every request without patching anything. Paths are collapsed to a bounded route label. Repository and file paths are unbounded, so labelling by full path would mint a time series per file viewed. Requests to the metrics port are skipped, since the channels are process-wide and every scrape would otherwise record itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: add changelog entry for HTTP request duration metrics Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(web): bound route label cardinality with an allowlist Truncating path depth bounded depth, not breadth. The first path segment is client-supplied and /api/[...slug] is a catch-all, so /wp-admin, /.env, and /api/<anything> each minted a new time series. Scanner traffic could grow the series count without limit, which is exactly what the normalization was supposed to prevent. Match the truncated path against a known-route set and report anything else as `other`, bounding distinct route labels to that set plus one regardless of what is requested. A route missing from the set loses granularity rather than breaking, so it fails closed. Also strengthens the metrics-port exclusion assertion. It checked for the absence of a `/metrics` label, which became vacuous once unknown paths collapse to `other` — it now asserts the total observation count, and fails if the port filter is removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(web): derive route labels from routes-manifest instead of a hardcoded list The hardcoded KNOWN_ROUTES set bounded cardinality but rots: new routes silently degrade to `other` until someone updates the file, and two-segment truncation collapses distinct routes (everything under /api/ee/* became one label). Next's build already emits .next/routes-manifest.json with every defined route and a matching regex, ordered by the router's own resolution priority. Load that at startup: exact-match static routes, then first dynamic regex wins, mirroring how the server actually routes the request. Labels become the route pattern itself (/browse/[...path], /settings/connections/[id]), so cardinality is bounded by the number of defined routes plus /_next and `other`, and the label set tracks the app automatically at build time. Scanner traffic now lands on the catch-all routes that genuinely serve it (/[...slug], /api/[...slug]) rather than a synthetic bucket. If the manifest is missing or unreadable, everything is labelled `other` — granularity lost, bound kept. Verified against the production build's manifest (78 static + 18 dynamic routes) and the running pod: the standalone server chdirs to the app dir, so the cwd-relative manifest path resolves. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(web): extend HTTP latency histogram buckets * fix(web): label rewritten routes in HTTP metrics * fix(web): clear stale HTTP metrics route table --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 3aa8711 commit 5e13dbb

6 files changed

Lines changed: 519 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
### Added
1111
- Added a manually triggered cloud image release workflow for isolated internal deployments. [#1566](https://github.com/sourcebot-dev/sourcebot/pull/1566)
1212
- Added Prometheus metrics for the web process, served on `WEB_METRICS_PORT` (default `3070`). [#1570](https://github.com/sourcebot-dev/sourcebot/pull/1570)
13+
- Added an `http_request_duration_seconds` metric recording web request latency by route, method, and status. [#1571](https://github.com/sourcebot-dev/sourcebot/pull/1571)
1314

1415
### Fixed
1516
- Fixed the web process being capped at a ~4GiB heap regardless of how much memory the container has, which caused multi-second garbage collection pauses on larger deployments. [#1569](https://github.com/sourcebot-dev/sourcebot/pull/1569)
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
import { afterAll, describe, expect, it } from 'vitest';
2+
import { createServer, Server } from 'node:http';
3+
import { initRouteTable, startHttpMetrics } from './httpMetrics';
4+
import { registry } from './promClient';
5+
6+
const app = createServer((_req, res) => {
7+
res.writeHead(200);
8+
res.end('ok');
9+
});
10+
11+
// Stands in for the real metrics server: requests to it must not be recorded,
12+
// since the diagnostics channels are process-wide and would otherwise make
13+
// every scrape observe itself.
14+
const metricsServer = createServer((_req, res) => {
15+
res.writeHead(200);
16+
res.end('# metrics');
17+
});
18+
19+
const listen = (server: Server): Promise<number> => {
20+
return new Promise(resolve => {
21+
server.listen(0, () => resolve((server.address() as { port: number }).port));
22+
});
23+
};
24+
25+
afterAll(() => {
26+
app.close();
27+
metricsServer.close();
28+
});
29+
30+
const countLines = (output: string): string[] => {
31+
return output.split('\n').filter(line => line.startsWith('http_request_duration_seconds_count'));
32+
};
33+
34+
describe('httpMetrics', () => {
35+
it('records durations per route pattern and ignores the metrics port', async () => {
36+
// The table is injected rather than read from disk so the test doesn't
37+
// depend on a prior `next build` having produced routes-manifest.json.
38+
initRouteTable({
39+
staticRoutes: [{ page: '/api/health' }],
40+
dynamicRoutes: [{ page: '/browse/[...path]', regex: '^/browse/(.+?)(?:/)?$' }],
41+
rewrites: {
42+
afterFiles: [{ source: '/api/mcp', regex: '^/api/mcp(?:/)?$' }],
43+
},
44+
});
45+
46+
// Both servers take an ephemeral port, then the metrics port is published
47+
// to env before subscribing, so the test never depends on a fixed port.
48+
const metricsPort = await listen(metricsServer);
49+
const appPort = await listen(app);
50+
process.env.WEB_METRICS_PORT = String(metricsPort);
51+
52+
startHttpMetrics();
53+
54+
await fetch(`http://127.0.0.1:${appPort}/api/health`);
55+
await fetch(`http://127.0.0.1:${appPort}/browse/github.com/a/b/-/blob/x.ts`);
56+
await fetch(`http://127.0.0.1:${appPort}/browse/github.com/c/d/-/blob/y.ts`);
57+
await fetch(`http://127.0.0.1:${appPort}/api/mcp`);
58+
await fetch(`http://127.0.0.1:${metricsPort}/metrics`);
59+
60+
// The finish channel fires after the response is flushed to the client.
61+
await new Promise(resolve => setTimeout(resolve, 100));
62+
63+
const metrics = await registry.metrics();
64+
const counts = countLines(metrics);
65+
66+
expect(counts.some(line => line.includes('route="/api/health"'))).toBe(true);
67+
expect(counts.some(line => line.includes('route="/api/mcp"'))).toBe(true);
68+
expect(counts.some(line => line.includes('status="200"'))).toBe(true);
69+
70+
// Two distinct file paths must collapse to the single route-pattern series.
71+
const browse = counts.filter(line => line.includes('route="/browse/[...path]"'));
72+
expect(browse).toHaveLength(1);
73+
expect(browse[0].trim().endsWith('2')).toBe(true);
74+
75+
// Keep enough resolution to distinguish the long-tail stalls this
76+
// metric is intended to expose rather than collapsing them into +Inf.
77+
for (const upperBound of [15, 20, 30, 60]) {
78+
expect(metrics).toContain(`le="${upperBound}"`);
79+
}
80+
81+
// The scrape of the metrics port must not be recorded. Asserted on the
82+
// total observation count rather than on the absence of a `/metrics`
83+
// label: `/metrics` is not a known route, so it would land in `other`
84+
// and an absent-label check would pass even with the filter removed.
85+
const total = counts.reduce((sum, line) => sum + Number(line.trim().split(' ').pop()), 0);
86+
expect(total).toBe(4);
87+
expect(counts.some(line => line.includes('route="other"'))).toBe(false);
88+
});
89+
});
Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
import { describe, expect, it } from 'vitest';
2+
import { buildRouteTable, initRouteTable, normalizeRoute } from './httpMetrics';
3+
4+
// Mirrors the shape and ordering of the real .next/routes-manifest.json:
5+
// dynamic routes are listed in Next's resolution priority, with catch-alls
6+
// after specific routes and the root catch-all last.
7+
const table = buildRouteTable({
8+
staticRoutes: [
9+
{ page: '/' },
10+
{ page: '/search' },
11+
{ page: '/repos' },
12+
{ page: '/api/health' },
13+
{ page: '/api/commits' },
14+
],
15+
dynamicRoutes: [
16+
{ page: '/api/auth/[...nextauth]', regex: '^/api/auth/(.+?)(?:/)?$' },
17+
{ page: '/api/repos/[repoId]/image', regex: '^/api/repos/([^/]+?)/image(?:/)?$' },
18+
{ page: '/api/[...slug]', regex: '^/api/(.+?)(?:/)?$' },
19+
{ page: '/browse/[...path]', regex: '^/browse/(.+?)(?:/)?$' },
20+
{ page: '/settings/[...slug]', regex: '^/settings/(.+?)(?:/)?$' },
21+
{ page: '/[...slug]', regex: '^/(.+?)(?:/)?$' },
22+
],
23+
rewrites: {
24+
afterFiles: [
25+
{ source: '/ingest/:path*', regex: '^/ingest(?:/(.+?))?(?:/)?$' },
26+
{ source: '/.well-known/oauth-authorization-server', regex: '^/\\.well-known/oauth-authorization-server(?:/)?$' },
27+
{ source: '/.well-known/oauth-protected-resource/:path*', regex: '^/\\.well-known/oauth-protected-resource(?:/(.+?))?(?:/)?$' },
28+
{ source: '/register', regex: '^/register(?:/)?$' },
29+
{ source: '/api/mcp', regex: '^/api/mcp(?:/)?$' },
30+
{ source: '/scim/v2/:path*', regex: '^/scim/v2(?:/(.+?))?(?:/)?$' },
31+
],
32+
},
33+
});
34+
35+
const maxLabels = table.staticPages.size
36+
+ table.dynamicRoutes.length
37+
+ table.beforeFilesRewrites.length
38+
+ table.afterFilesRewrites.length
39+
+ table.fallbackRewrites.length
40+
+ 2; // + '/_next', 'other'
41+
42+
describe('normalizeRoute', () => {
43+
it('discards a previously loaded table when initialization fails', () => {
44+
expect(initRouteTable({
45+
staticRoutes: [{ page: '/api/health' }],
46+
dynamicRoutes: [],
47+
})).toBe(true);
48+
expect(normalizeRoute('/api/health')).toBe('/api/health');
49+
50+
expect(initRouteTable({
51+
staticRoutes: [],
52+
dynamicRoutes: [{ page: '/broken', regex: '[' }],
53+
})).toBe(false);
54+
expect(normalizeRoute('/api/health')).toBe('other');
55+
});
56+
57+
it('maps the root path', () => {
58+
expect(normalizeRoute('/', table)).toBe('/');
59+
expect(normalizeRoute('', table)).toBe('/');
60+
});
61+
62+
it('matches static routes exactly', () => {
63+
expect(normalizeRoute('/api/health', table)).toBe('/api/health');
64+
expect(normalizeRoute('/search', table)).toBe('/search');
65+
});
66+
67+
it('labels dynamic routes with their route pattern', () => {
68+
expect(normalizeRoute('/api/auth/callback/github', table)).toBe('/api/auth/[...nextauth]');
69+
expect(normalizeRoute('/api/repos/42/image', table)).toBe('/api/repos/[repoId]/image');
70+
expect(normalizeRoute('/settings/connections/42', table)).toBe('/settings/[...slug]');
71+
});
72+
73+
it('respects manifest ordering: specific routes win over catch-alls', () => {
74+
// /api/auth/... must hit [...nextauth], not the /api/[...slug] catch-all.
75+
expect(normalizeRoute('/api/auth/session', table)).toBe('/api/auth/[...nextauth]');
76+
// Unknown API paths fall through to the catch-all that actually serves them.
77+
expect(normalizeRoute('/api/not-a-real-route', table)).toBe('/api/[...slug]');
78+
});
79+
80+
it('labels rewritten paths with their public source pattern', () => {
81+
expect(normalizeRoute('/api/mcp', table)).toBe('/api/mcp');
82+
expect(normalizeRoute('/scim/v2/Users/42', table)).toBe('/scim/v2/:path*');
83+
expect(normalizeRoute('/.well-known/oauth-authorization-server', table))
84+
.toBe('/.well-known/oauth-authorization-server');
85+
expect(normalizeRoute('/.well-known/oauth-protected-resource/api/mcp', table))
86+
.toBe('/.well-known/oauth-protected-resource/:path*');
87+
expect(normalizeRoute('/register', table)).toBe('/register');
88+
expect(normalizeRoute('/ingest/events', table)).toBe('/ingest/:path*');
89+
});
90+
91+
it('matches rewrites in Next routing order', () => {
92+
const precedenceTable = buildRouteTable({
93+
staticRoutes: [
94+
{ page: '/docs' },
95+
{ page: '/api/health' },
96+
],
97+
dynamicRoutes: [
98+
{ page: '/api/[...slug]', regex: '^/api/(.+?)(?:/)?$' },
99+
{ page: '/browse/[...path]', regex: '^/browse/(.+?)(?:/)?$' },
100+
],
101+
rewrites: {
102+
beforeFiles: [{ source: '/docs/:path*', regex: '^/docs(?:/(.+?))?(?:/)?$' }],
103+
afterFiles: [{ source: '/api/:path*', regex: '^/api/(.+?)(?:/)?$' }],
104+
fallback: [{ source: '/:path*', regex: '^/(.+?)(?:/)?$' }],
105+
},
106+
});
107+
108+
expect(normalizeRoute('/docs', precedenceTable)).toBe('/docs/:path*');
109+
expect(normalizeRoute('/api/health', precedenceTable)).toBe('/api/health');
110+
expect(normalizeRoute('/api/mcp', precedenceTable)).toBe('/api/:path*');
111+
expect(normalizeRoute('/browse/org/repo', precedenceTable)).toBe('/browse/[...path]');
112+
expect(normalizeRoute('/unmatched', precedenceTable)).toBe('/:path*');
113+
});
114+
115+
it('collapses unbounded repository and file paths to one label', () => {
116+
const a = normalizeRoute('/browse/github.com/org/repo/-/blob/src/index.ts', table);
117+
const b = normalizeRoute('/browse/github.com/other/repo/-/blob/lib/other.ts', table);
118+
119+
expect(a).toBe('/browse/[...path]');
120+
expect(b).toBe(a);
121+
});
122+
123+
it('is unaffected by trailing or duplicate slashes', () => {
124+
expect(normalizeRoute('/search/', table)).toBe('/search');
125+
expect(normalizeRoute('//search//', table)).toBe('/search');
126+
expect(normalizeRoute('/api/health/', table)).toBe('/api/health');
127+
});
128+
129+
it('labels asset requests /_next without consulting the table', () => {
130+
expect(normalizeRoute('/_next/static/chunks/main.js', table)).toBe('/_next');
131+
expect(normalizeRoute('/_next/image', undefined)).toBe('/_next');
132+
});
133+
134+
it('reports everything as other when no table is loaded', () => {
135+
expect(normalizeRoute('/api/health', undefined)).toBe('other');
136+
expect(normalizeRoute('/search', undefined)).toBe('other');
137+
});
138+
139+
describe('cardinality bounding', () => {
140+
it('routes scanner traffic to catch-alls, not new labels', () => {
141+
expect(normalizeRoute('/wp-admin', table)).toBe('/[...slug]');
142+
expect(normalizeRoute('/.env', table)).toBe('/[...slug]');
143+
expect(normalizeRoute('/api/12345', table)).toBe('/api/[...slug]');
144+
});
145+
146+
it('reports unmatched paths as other when there is no root catch-all', () => {
147+
const noCatchAll = buildRouteTable({
148+
staticRoutes: [{ page: '/search' }],
149+
dynamicRoutes: [{ page: '/api/[...slug]', regex: '^/api/(.+?)(?:/)?$' }],
150+
});
151+
152+
expect(normalizeRoute('/wp-admin', noCatchAll)).toBe('other');
153+
expect(normalizeRoute('/api/anything', noCatchAll)).toBe('/api/[...slug]');
154+
});
155+
156+
it('stays bounded under scanner traffic', () => {
157+
const hostile: string[] = [];
158+
for (let i = 0; i < 1000; i++) {
159+
hostile.push(`/scan-${i}`);
160+
hostile.push(`/api/scan-${i}`);
161+
hostile.push(`/${i}/${i}/${i}`);
162+
}
163+
164+
const labels = new Set(hostile.map(p => normalizeRoute(p, table)));
165+
166+
// 3,000 distinct hostile paths produce exactly the two catch-all labels.
167+
expect(labels).toEqual(new Set(['/[...slug]', '/api/[...slug]']));
168+
});
169+
170+
it('never exceeds the table-derived bound for any input', () => {
171+
const paths = [
172+
'/', '/search', '/repos', '/browse/a/b/c', '/api/health',
173+
'/api/commits', '/api/auth/session', '/_next/static/x.js',
174+
'/api/mcp', '/scim/v2/Users/42', '/ingest/events',
175+
'/wp-admin', '/api/bogus', '/random', '/..%2f', '/a/b/c/d/e',
176+
];
177+
for (let i = 0; i < 500; i++) {
178+
paths.push(`/junk${i}`, `/api/junk${i}`);
179+
}
180+
181+
const labels = new Set(paths.map(p => normalizeRoute(p, table)));
182+
183+
expect(labels.size).toBeLessThanOrEqual(maxLabels);
184+
expect(labels).toContain('/api/health');
185+
});
186+
});
187+
});

0 commit comments

Comments
 (0)