Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added
- Added a manually triggered cloud image release workflow for isolated internal deployments. [#1566](https://github.com/sourcebot-dev/sourcebot/pull/1566)
- Added Prometheus metrics for the web process, served on `WEB_METRICS_PORT` (default `3070`). [#1570](https://github.com/sourcebot-dev/sourcebot/pull/1570)

### Fixed
- 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)
Expand Down
3 changes: 3 additions & 0 deletions packages/shared/src/env.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,9 @@ const options = {

WORKER_API_URL: z.string().url().default("http://localhost:3060"),

// Port the web process serves its Prometheus metrics on.
WEB_METRICS_PORT: numberSchema.default(3070),

// Auth
AUTH_SECRET: z.string(),
AUTH_URL: z.string().url(),
Expand Down
1 change: 1 addition & 0 deletions packages/web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@
"posthog-js": "^1.369.0",
"posthog-node": "^5.24.15",
"pretty-bytes": "^6.1.1",
"prom-client": "^15.1.3",
"psl": "^1.15.0",
"react": "19.2.4",
"react-day-picker": "^9.14.0",
Expand Down
5 changes: 5 additions & 0 deletions packages/web/src/instrumentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ export async function register() {
await import('./sentry.edge.config');
}

if (process.env.NEXT_RUNTIME === 'nodejs') {
const { startMetricsServer } = await import('./metricsServer');
startMetricsServer();
}

if (process.env.NEXT_RUNTIME === 'nodejs') {
const { initialize } = await import('./initialize');
await initialize();
Expand Down
50 changes: 50 additions & 0 deletions packages/web/src/metricsServer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { createLogger, env } from '@sourcebot/shared';
import { createServer, Server } from 'node:http';
import { registry } from './promClient';

const logger = createLogger('web-metrics-server');

/**
* Serves the web process' Prometheus metrics on its own port, rather than as a
* Next.js route, so that scraping doesn't pass through the app's middleware or
* get exposed publicly through the ingress.
*/
export const startMetricsServer = (): Server | undefined => {
// Guard against a missing port: `listen(undefined)` binds a random one, which
// would leave the scrape target silently broken instead of loudly absent.
const port = Number(env.WEB_METRICS_PORT);
if (!Number.isInteger(port) || port <= 0) {
logger.error(`Invalid WEB_METRICS_PORT '${env.WEB_METRICS_PORT}'; metrics server not started.`);
return undefined;
Comment thread
brendan-kellam marked this conversation as resolved.
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incomplete metrics port validation

Medium Severity

The port guard only rejects non-integers and values <= 0, so ports above 65535 still reach server.listen. Node throws ERR_SOCKET_BAD_PORT synchronously there, which the 'error' listener does not catch. That exception escapes startMetricsServer and aborts register before initialize runs, so a bad WEB_METRICS_PORT can take down web startup instead of only skipping metrics.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit cf8db22. Configure here.


const server = createServer(async (req, res) => {
if (req.url !== '/metrics') {
res.writeHead(404);
res.end();
return;
}

try {
const metrics = await registry.metrics();
res.writeHead(200, { 'Content-Type': registry.contentType });
res.end(metrics);
} catch (error) {
logger.error(`Failed to collect metrics: ${error}`);
res.writeHead(500);
res.end();
}
});

// Metrics must never take down the web server, so swallow listen failures
// (a port collision, most likely) instead of letting the 'error' event throw.
server.on('error', (error) => {
logger.error(`Metrics server error: ${error}`);
});

server.listen(port, () => {
logger.info(`Web metrics server listening on port ${port}`);
});

return server;
};
47 changes: 47 additions & 0 deletions packages/web/src/promClient.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { describe, expect, it } from 'vitest';
import { registry } from './promClient';

const metricNames = (output: string): Set<string> => {
return new Set(
output
.split('\n')
.filter(line => line.length > 0 && !line.startsWith('#'))
.map(line => line.split(/[ {]/)[0])
);
};

describe('web promClient', () => {
it('exposes the metrics needed to diagnose heap pressure', async () => {
const names = metricNames(await registry.metrics());

expect(names).toContain('nodejs_heap_size_limit_bytes');
expect(names).toContain('nodejs_heap_size_used_bytes');
expect(names).toContain('nodejs_eventloop_lag_p99_seconds');
});

it('registers the gc duration histogram', () => {
// Asserted via the registry rather than the rendered output: the histogram
// emits no series until a garbage collection has actually been observed.
expect(registry.getSingleMetric('nodejs_gc_duration_seconds')).toBeDefined();
});

it('reports a plausible heap size limit', async () => {
const output = await registry.metrics();
const line = output.split('\n').find(l => l.startsWith('nodejs_heap_size_limit_bytes '));

expect(line).toBeDefined();

const limit = Number(line!.split(' ')[1]);
expect(Number.isFinite(limit)).toBe(true);
// Any real V8 heap limit is well above 100MB and well below 100GB.
expect(limit).toBeGreaterThan(100 * 1024 * 1024);
expect(limit).toBeLessThan(100 * 1024 * 1024 * 1024);
});

it('can be collected repeatedly', async () => {
const first = await registry.metrics();
const second = await registry.metrics();

expect(metricNames(first)).toEqual(metricNames(second));
});
});
21 changes: 21 additions & 0 deletions packages/web/src/promClient.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import client, { Gauge, Registry } from 'prom-client';
import { getHeapStatistics } from 'node:v8';

export const registry = new Registry();

// `collectDefaultMetrics` reports heap usage but not the ceiling it's measured
// against, and usage alone can't distinguish "busy" from "out of room". Without
// the limit there's no way to tell whether V8 is doing cheap incremental
// collections or is pinned at its ceiling running full mark-compacts.
const heapSizeLimit = new Gauge({
name: 'nodejs_heap_size_limit_bytes',
help: 'V8 heap size limit in bytes',
collect() {
this.set(getHeapStatistics().heap_size_limit);
},
});
registry.registerMetric(heapSizeLimit);

client.collectDefaultMetrics({
register: registry,
});
1 change: 1 addition & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9291,6 +9291,7 @@ __metadata:
posthog-js: "npm:^1.369.0"
posthog-node: "npm:^5.24.15"
pretty-bytes: "npm:^6.1.1"
prom-client: "npm:^15.1.3"
psl: "npm:^1.15.0"
raw-loader: "npm:^4.0.2"
react: "npm:19.2.4"
Expand Down
Loading