Cluster fixes by jdmarshall · Pull Request #789 · prometheus/client_js

jdmarshall · 2026-07-28T01:58:02Z

Rework of worker and cluster lifecycle management. Also allows the primary cluster thread to report stats, adds debug information to common possible failure paths.

Fixes #155, #181, #183, #280, #501, #563, #788

jdmarshall · 2026-07-28T01:59:10Z

This changes unreleased API modifications for AggregatorRegistry, so this will need to be part of v0.16.

Also fixes a memory leak in trunk for broadcastchannels, and improves lifecycle management for dead worker threads (both kinds)

cjihrig · 2026-07-28T02:08:59Z

+					return { threadId, channel };
 				});

+				await delay(5); // Let announcements arrive


Just speaking from past experience, this seems like it could become a flaky test in the future (on slow CI machines).

I'll check if there's callback.

This test already includes delay(), btw.

No callback. Might be stuck with this one.

cjihrig · 2026-07-28T02:11:07Z

-				.then(metrics => Registry.aggregate(metrics.flat()).metrics())
-				.then(result => done(undefined, result), done);
+			const myMetrics = Promise.all(
+				registries.map(async r => r.getMetricsAsJSON()),


It looks like you do something similar in a couple other places in this PR, and those places don't add the extra async to the map callback.

In particular, cluster now uses a similar announcement system to filter workers. This fixes prometheus#181.

Caught by jest warnings.

Fixes prometheus#183

Signed-off-by: Jason Marshall <jdmarshall@users.noreply.github.com>

Signed-off-by: alencristen <299997878+alencristen@users.noreply.github.com>

99.9% of the time process.send() is going to work. We don't need to guard it when it's already inside of a try block. Just guard the retry send. Also reduces the amount of excessive mocking going on in the tests by using jest more instead of creating our own mocks. Signed-off-by: Jason Marshall <jdmarshall@users.noreply.github.com>

cjihrig

LGTM, but there are a couple linter issues in lib/worker.js.

krajorama

I'm not a frontend developer , but trying to help with review using LLM.

I think in general smaller PRs are easier to review and argue about.

Findings:

index.d.ts

index.d.ts still declares the removed addWorker — index.d.ts:209

addWorker(worker: Worker): void; no longer exists at runtime. TypeScript users compile clean and get a TypeError. Delete the declaration.

Primary metric inclusion is breaking

But filed under ### Changed — CHANGELOG.md:44

clusterMetrics() now always aggregates the primary's registries — confirmed: with zero workers it returns the primary's process_cpu_seconds_total etc. Calling collectDefaultMetrics() unconditionally in both primary and workers is a very common pattern, and those users will silently start seeing the primary's process metrics summed into every series. This is released API (v15.x), so it belongs under ### Breaking with a migration note.

It also deserves an opt-out. WorkerRegistry takes a primary flag; ClusterRegistry has no equivalent knob. Two smaller consequences: setRegistries's doc comment (lib/cluster.js:154 still says "Call from workers to…", and the empty-cluster return value changed from '' to '\n' — which is what forced the .trim() at test/clusterTest.js:76 and is worth its own changelog line.

Unreachable branch

lib/cluster.js:119

allMetrics = [myMetrics, ...workerMetrics] always has ≥1 element, so allMetrics.length === 0, the debug('No workers found…') call, and the '' fast path are dead. Test workerMetrics.length or drop the branch.

The worker-threads half of "lifecycle for dead worker threads (both kinds)" isn't there

lib/cluster.js gained a disconnect handler; lib/worker.js gained nothing. The 'close' listener at lib/worker.js:247only fires when the local channel object is closed, never when the remote thread dies. Verified: after worker.terminate(), every subsequent workerMetrics() rejects after 5 s, permanently. This is pre-existing (identical on main) so not a regression — but given the PR's stated scope, it's the obvious gap.

The two new "listeners don't accumulate" tests cannot fail

test/clusterTest.js:79, test/workerTest.js:140

The require sits outside the loop, so jest.resetModules() inside it never produces a fresh module instance; ar is assigned and never read (also a lint smell); and there are no assertions. Measured:

test-style loop -> cluster message listeners: 1
fresh module each time -> cluster message listeners: 6
So the property under test isn't guaranteed by listenersAdded at all — it's guaranteed by the require cache. Re-require inside the loop and assert cluster.listenerCount('message').

Test hygiene in test/clusterTest.js line 101

the originalWorkers restore was dropped, so the global cluster.workers stays mocked for the rest of the file (later announce() calls then send to jest mocks).
line 89: jest.resetModules() followed by new Registry(regType) from the top-of-file require is a no-op.
line 147: gauge.remove() isn't in a finally — a failed assertion leaves primary_gauge_test in the global registry, polluting later tests.
Nothing covers disconnect pruning workers, duplicate-announcement dedup, or collector-error propagation.

krajorama · 2026-08-04T13:09:58Z

+ */
+async function workerListener(event) {
+	const name = `@prometheus/client:worker:${threadId}`;
+	const channel = new BroadcastChannel(name).unref();


Wouldn't this create and leak a BroadcastChannel on every event?

krajorama · 2026-08-04T13:11:33Z

+	if (message.type === ANNOUNCEMENT) {
+		process.send({ type: ANNOUNCEMENT });
+	} else if (message.type === GET_METRICS_REQ) {
+		const metrics = await Promise.all(


What happens if await throws an exception, this is outside the try so possibly unhandled error?

krajorama · 2026-08-04T13:13:15Z

+		announce();
+	} else {
+		process.on('message', workerListener);
+		process.send({ type: ANNOUNCEMENT });


The non-primary branch calls process.send unconditionally. With cluster.isPrimary === false but no IPC channel (NODE_UNIQUE_ID leaked into a grandchild — the case some process managers hit), this throws where main constructs fine:

PR: THREW: TypeError: process.send is not a function
main: OK: constructed without throwing
main never called send at construction time, so this is a new failure mode. Guard with typeof process.send === 'function' && process.connected.

krajorama · 2026-08-04T13:16:18Z

 			};
 			requests.set(requestId, request);
-			const responsePromises = [...this.channels.keys()].map(
+			const responsePromises = [...channels.keys()].sort().map(


[...channels.keys()].sort() sorts channel names, so with ≥10 threads you get 1, 10, 11, 2, 20, 3. lib/cluster.js sorts numerically on worker.id; this undoes the numeric intent of 0162ab5 ("aggregate metrics in a deterministic order") on the worker-threads side. Still deterministic, but no longer matching cluster.js. The threadId now carried in the response payload is also unused for ordering — keep a name→threadId map and sort on that.

jdmarshall modified the milestones: v1, v0.16 Jul 28, 2026

jdmarshall force-pushed the clusterFixes branch from 30fd482 to 81ee11d Compare July 28, 2026 02:05

cjihrig reviewed Jul 28, 2026

View reviewed changes

jdmarshall force-pushed the clusterFixes branch from 81ee11d to da41795 Compare July 28, 2026 15:27

jdmarshall added 4 commits July 28, 2026 08:41

Rework cluster and worker lifecycle logic to be more similar.

8a6941a

In particular, cluster now uses a similar announcement system to filter workers. This fixes prometheus#181.

Fixing missing unref for BroadcastChannel.

0bd94ae

Caught by jest warnings.

Export stats from the primary thread.

3c366f7

Fixes prometheus#183

Rework state tracking to support bot prometheus#155 and prometheus#788

c0b8140

Signed-off-by: Jason Marshall <jdmarshall@users.noreply.github.com>

jdmarshall force-pushed the clusterFixes branch from da41795 to c0b8140 Compare July 28, 2026 15:42

fix(cluster): skip responses after IPC disconnect

efa7691

Signed-off-by: alencristen <299997878+alencristen@users.noreply.github.com>

jdmarshall mentioned this pull request Jul 28, 2026

fix(cluster): skip responses after IPC disconnect #787

Closed

jdmarshall force-pushed the clusterFixes branch from d07286c to 3bc15bb Compare July 28, 2026 16:37

jdmarshall requested a review from cjihrig July 28, 2026 16:37

cjihrig approved these changes Jul 28, 2026

View reviewed changes

milcho0604 mentioned this pull request Aug 2, 2026

fix: normalize label values once, when a label combination is first stored #798

Open

krajorama reviewed Aug 4, 2026

View reviewed changes

jdmarshall mentioned this pull request Aug 4, 2026

Roadmap to 1.0 #794

Open

Navigation Menu

Uh oh!

Cluster fixes - #789

Cluster fixes#789
jdmarshall wants to merge 6 commits into
prometheus:mainfrom
jdmarshall:clusterFixes

jdmarshall commented Jul 28, 2026 •

edited

Loading

Uh oh!

jdmarshall commented Jul 28, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

cjihrig Jul 28, 2026

Uh oh!

jdmarshall Jul 28, 2026

Uh oh!

jdmarshall Jul 28, 2026

Uh oh!

jdmarshall Jul 28, 2026

Uh oh!

cjihrig Jul 28, 2026

Uh oh!

cjihrig left a comment

Uh oh!

krajorama left a comment •

edited

Loading

Uh oh!

krajorama Aug 4, 2026

Uh oh!

krajorama Aug 4, 2026

Uh oh!

krajorama Aug 4, 2026

Uh oh!

krajorama Aug 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

jdmarshall commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jdmarshall commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjihrig left a comment

Choose a reason for hiding this comment

Uh oh!

krajorama left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

index.d.ts

Primary metric inclusion is breaking

Unreachable branch

The worker-threads half of "lifecycle for dead worker threads (both kinds)" isn't there

The two new "listeners don't accumulate" tests cannot fail

Test hygiene in test/clusterTest.js line 101

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jdmarshall commented Jul 28, 2026 •

edited

Loading

jdmarshall commented Jul 28, 2026 •

edited

Loading

krajorama left a comment •

edited

Loading