Skip to content
Open
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ This release marks our first release under the Prometheus umbrella.
runtime object. Value-style uses such as `MetricType.Counter` (which threw at runtime)
no longer compile; compare against the string literals instead. Under
`verbatimModuleSyntax`, import it with `import type`.
- The cluster primary now reports metrics

### Changed

Expand All @@ -46,6 +47,8 @@ This release marks our first release under the Prometheus umbrella.
- chore: Add copyright license headers and test
- Make cluster and worker-thread metric aggregation order deterministic
- Export `MetricObject`, `MetricObjectWithValues`, `MetricValue` and `MetricValueWithName` from the TypeScript definitions
- Improve cluster support to allow workers to opt out
- Abort cluster metric responses during process termination

### Added

Expand Down
5 changes: 5 additions & 0 deletions example/cluster.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ const metricsServer = express();
const clusterRegistry = new ClusterRegistry();

if (cluster.isPrimary) {
require('../').collectDefaultMetrics({
gcDurationBuckets: [0.001, 0.01, 0.1, 1, 2, 5], // These are the default buckets.
});

for (let i = 1; i <= 4; i++) {
cluster.fork({ ...process.env, PORT: 3000 + i });
}
Expand All @@ -32,6 +36,7 @@ if (cluster.isPrimary) {
res.set('Content-Type', clusterRegistry.contentType);
res.send(metrics);
} catch (ex) {
console.log(ex);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit:

Suggested change
console.log(ex);
console.error(ex);

res.statusCode = 500;
res.send(ex.message);
}
Expand Down
1 change: 0 additions & 1 deletion index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,6 @@ export class WorkerRegistry<T extends RegistryContentType> extends Registry<T> {
*/
workerMetrics(): Promise<string>;

addWorker(worker: Worker): void;
/**
* Sets the registry or registries to be aggregated. Call from workers to
* use a registry/registries other than the default global registry.
Expand Down
227 changes: 167 additions & 60 deletions lib/cluster.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
* cluster master.
*/

const { debuglog } = require('node:util');
const Registry = require('./registry');
// We need to lazy-load the 'cluster' module as some application servers -
// namely Passenger - crash when it is imported.
Expand All @@ -31,17 +32,25 @@ let cluster = () => {
return data;
};

const debug = debuglog('prom:metrics:cluster');
const ANNOUNCEMENT = '@prometheus-io/client:announcement';
const GET_METRICS_REQ = '@prometheus-io/client:getMetricsReq';
const GET_METRICS_RES = '@prometheus-io/client:getMetricsRes';

let registries = [Registry.globalRegistry];
let requestCtr = 0; // Concurrency control
let listenersAdded = false;
const requests = new Map(); // Pending requests for workers' local metrics.
const workers = new Map();

class AggregatorRegistry extends Registry {
/**
* Create a Registry.
* @param regContentType
*/
constructor(regContentType = Registry.PROMETHEUS_CONTENT_TYPE) {
super(regContentType);

addListeners();
}

Expand All @@ -53,9 +62,9 @@ class AggregatorRegistry extends Registry {
*/
clusterMetrics() {
const requestId = requestCtr++;
const workers = Object.values(cluster().workers)
.filter(worker => worker.isConnected())
.sort((left, right) => left.id - right.id);
const orderedWorkers = [...workers.values()].sort(
(left, right) => left.id - right.id,
);

return new Promise((resolve, reject) => {
let settled = false;
Expand All @@ -78,36 +87,43 @@ class AggregatorRegistry extends Registry {
responseHandlers,
done,
errorTimeout: setTimeout(() => {
const err = new Error('Operation timed out.');
const err = new Error(
`Operation timed out. ${request.responseHandlers.size} outstanding responses.`,
);
request.done(err);
}, 5000),
}, 5_000),
};
requests.set(requestId, request);

const message = {
type: GET_METRICS_REQ,
requestId,
};

if (workers.length === 0) {
// No workers were up
process.nextTick(() => done(undefined, ''));
return;
}

const responsePromises = workers.map(
const workerMetrics = orderedWorkers.map(
worker =>
new Promise((resolveResponse, rejectResponse) => {
responseHandlers.set(worker.id, {
resolve: resolveResponse,
reject: rejectResponse,
});
worker.send(message);

worker.send({
type: GET_METRICS_REQ,
requestId,
});
}),
);

Promise.all(responsePromises)
.then(metrics => Registry.aggregate(metrics.flat()).metrics())
const myMetrics = Promise.all(
registries.map(r => r.getMetricsAsJSON()),
).then(metrics => {
return { metrics };
});

if (workerMetrics.length === 0) {
debug('No workers found for requestId', requestId);
}

const allMetrics = [myMetrics, ...workerMetrics];

Promise.all(allMetrics)
.then(responses => responses.flatMap(response => response.metrics))
.then(metrics => Registry.aggregate(metrics).metrics())
.then(result => done(undefined, result), done);
});
}
Expand Down Expand Up @@ -158,54 +174,145 @@ class AggregatorRegistry extends Registry {
* @returns {void}
*/
function addListeners() {
if (listenersAdded) return;
if (listenersAdded) {
return;
}

listenersAdded = true;

if (cluster().isPrimary) {
// Listen for worker responses to requests for local metrics
cluster().on('message', (worker, message) => {
if (message.type === GET_METRICS_RES) {
const request = requests.get(message.requestId);
replaceListener('message', cluster(), primaryListener);
replaceListener('disconnect', cluster(), disconnect);

if (request === undefined) {
return;
}
announce();
} else {
replaceListener('message', process, workerListener);
Comment on lines +184 to +189

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've been trying to understand an issue flagged by LLM. "replaceListener" was removing listeners from the "old" registry, but that means you can lose metrics as they won't be called anymore. So reverting to simply doing on() without replaceListener fixes that, see diff and regression test. Is this what you also flagged here ?

It does reintroduce #155 warning in "listeners don't accumulate" in test/clusterTest.js, but there is a way to fix it apparently (in a separate PR):

Removing replaceListener does re-expose #155's original symptom — 11+ reloads of the module in one process will emit MaxListenersExceededWarning again. That's the honest trade, and it's the right one: a warning in a test harness that clears require.cache is strictly better than silent metric loss in production. If the author wants both, the fix is a shared registration marker (a symbol on the emitter, or a globalThis key) that skips installing a second listener rather than removing the first — skipping leaves the earlier instance's workers map intact, so no instance goes blind.

Suggested change
replaceListener('message', cluster(), primaryListener);
replaceListener('disconnect', cluster(), disconnect);
if (request === undefined) {
return;
}
announce();
} else {
replaceListener('message', process, workerListener);
cluster().on('message', primaryListener);
cluster().on('disconnect', disconnect);
announce();
} else {
process.on('message', workerListener);

Regression test for this:

		it('keeps polling workers after a duplicate module instance is loaded', async () => {
			const originalWorkers = cluster.workers;

			jest.resetModules();
			const FirstInstance = require('../lib/cluster');
			const registry = new FirstInstance(regType);

			// A duplicated copy of the package - a nested dependency, say - loads
			// its own module instance in the same primary process.
			jest.resetModules();
			const SecondInstance = require('../lib/cluster');
			new SecondInstance(regType);

			const worker = { id: 1, isConnected: () => true, send: jest.fn() };
			cluster.workers = { 1: worker };
			cluster.emit('message', worker, { type: ANNOUNCEMENT });

			let result;
			try {
				result = registry.clusterMetrics();

				const requests = worker.send.mock.calls
					.map(([message]) => message)
					.filter(message => message.type === GET_METRICS_REQ);

				// The first instance must still know about the worker. Otherwise it
				// silently reports primary-only metrics, with no timeout or error.
				expect(requests).toHaveLength(1);
			} finally {
				cluster.emit('message', worker, {
					type: GET_METRICS_RES,
					requestId: 0,
					metrics: [[metric(1)]],
				});
				await result?.catch(() => {});
				cluster.emit('disconnect', worker);
				cluster.workers = originalWorkers;
			}
		});

@jdmarshall jdmarshall Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Major edit:

Because the responses are being aggregated through a promise, only the first result was ever being seen anyway. In fact what you were probably always seeing before was the oldest or second oldest metrics per process, based on when the event was delivered and processing time to gather the metrics. Which is exactly the wrong data for functional and integration tests.

To the best of my knowledge prom-client has never worked with hot reload. Let alone well. And anyone would see that it doesn't within a few minutes of trying, especially if they used older versions that were especially crabby about this.

We have a bigger problem with what to do about dead workers. Because their metrics disappear when they do, and since we are gathering them, we are getting the wrong answers for counts and gauges. #803 which is a problem since the general wisdom is 'let the process crash' when unhandledException or unhandledRejection fires.

What I think that suggests is an update to the README, suggesting you let a Prometheus sidecar handle the aggregation in Serious Projects rather than using cluster.js or worker.js


const response = request.responseHandlers.get(worker.id);
if (response === undefined) {
return;
}
request.responseHandlers.delete(worker.id);
if (typeof process.send !== 'function') {
debug('worker has no process.send()');
} else if (!process.connected) {
debug('worker is not connected to parent process');
} else {
process.send({ type: ANNOUNCEMENT });
}
}
}

if (message.error) {
response.reject(new Error(message.error));
} else {
response.resolve(message.metrics);
}
/**
* Watch for metrics events and aggregator announcements
*
* Whereas clusters are a top-level activity, multiple modules may start their
* own workers and require telemetry collection.
* @param message {MessageEvent}
*/
async function workerListener(message) {
if (message.type === ANNOUNCEMENT) {
process.send({ type: ANNOUNCEMENT });
} else if (message.type === GET_METRICS_REQ) {
try {
const metrics = await Promise.all(
registries.map(r => r.getMetricsAsJSON()),
);

if (!process.connected) {
debug('Connection to primary lost.');
} else {
process.send({
type: GET_METRICS_RES,
requestId: message.requestId,
metrics,
});
}
});
} else {
// Respond to master's requests for worker's local metrics.
process.on('message', message => {
if (message.type === GET_METRICS_REQ) {
Promise.all(registries.map(r => r.getMetricsAsJSON()))
.then(metrics => {
process.send({
type: GET_METRICS_RES,
requestId: message.requestId,
metrics,
});
})
.catch(error => {
process.send({
type: GET_METRICS_RES,
requestId: message.requestId,
error: error.message,
});
});
} catch (error) {
debug('Error sending to primary', error);
if (!process.connected) {
debug('Connection to primary lost.');
} else {
process.send({
type: GET_METRICS_RES,
requestId: message.requestId,
error: error.message,
});
}
});
}
}
}

/**
* Add workers to the aggregation list when they are announced.
*
* Whereas clusters are a top-level activity, multiple modules may start their
* own workers and require telemetry collection.
* @param event {MessageEvent}
*/

async function primaryListener(worker, event) {
if (event.type === ANNOUNCEMENT) {
if (workers.has(worker.id)) {
debug('duplicate worker announcement', worker.id);
return;
}

workers.set(worker.id, worker);
} else if (event.type === GET_METRICS_RES) {
const request = requests.get(event.requestId);

if (request === undefined) {
debug('unexpected results from worker', worker.id);
return;
}

const response = request.responseHandlers.get(worker.id);
if (response === undefined) {
return;
}
request.responseHandlers.delete(worker.id);

if (event.error) {
response.reject(new Error(event.error));
} else {
response.resolve({
threadId: worker.id,
metrics: event.metrics,
});
}
}
}

function disconnect(event) {
debug('worker disconnected', event.id);
workers.delete(event.id);
}

function announce() {
for (const worker of Object.values(cluster().workers)) {
if (worker.isConnected()) {
worker.send({ type: ANNOUNCEMENT });
}
}
}

/**
* Replace any listeners with new ones.
*
* @param messageType
* @param emitter {EventEmitter}
* @param fn
*/
function replaceListener(messageType, emitter, fn) {
// Reloading a module creates a unique instance of each function, so the
// identity checks is cluster.off() will fail.
const functionString = fn.toString();

for (const listener of emitter.listeners(messageType)) {
// eslint-disable-next-line eqeqeq
if (functionString == listener) {
debug('removing duplicate listener', messageType);
emitter.off(messageType, listener);
}
}

emitter.on(messageType, fn);
}

Comment on lines +295 to 317

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For above.

Suggested change
/**
* Replace any listeners with new ones.
*
* @param messageType
* @param emitter {EventEmitter}
* @param fn
*/
function replaceListener(messageType, emitter, fn) {
// Reloading a module creates a unique instance of each function, so the
// identity checks is cluster.off() will fail.
const functionString = fn.toString();
for (const listener of emitter.listeners(messageType)) {
// eslint-disable-next-line eqeqeq
if (functionString == listener) {
debug('removing duplicate listener', messageType);
emitter.off(messageType, listener);
}
}
emitter.on(messageType, fn);
}

module.exports = AggregatorRegistry;
Loading
Loading