Skip to content

Record cluster worker scrape failures - #850

Open
nightjard wants to merge 1 commit into
prometheus:mainfrom
nightjard:fix/cluster-worker-failure-histogram-290
Open

Record cluster worker scrape failures#850
nightjard wants to merge 1 commit into
prometheus:mainfrom
nightjard:fix/cluster-worker-failure-histogram-290

Conversation

@nightjard

Copy link
Copy Markdown

Summary

Add an internal histogram named
prom_client_cluster_worker_scrape_failures to record failed cluster metric
collections.

Each observation contains the number of workers that failed to return metrics:

  • For timeouts, it records the number of outstanding worker responses.
  • For worker-reported errors, it records the known worker failure count.
  • Failed collections continue to reject without returning partial metrics.
  • New observations are exposed on the next successful clusterMetrics() call.

A histogram preserves information from consecutive failures instead of only
retaining the most recent value, as a gauge would.

Testing

Added tests covering:

  • Multiple workers timing out.
  • Worker-reported collection errors.
  • Histogram exposure after a successful recovery scrape.
  • Prometheus and OpenMetrics output formats.

Signed-off-by: SirGundir <crstalhopkin@gmail.com>
@nightjard
nightjard force-pushed the fix/cluster-worker-failure-histogram-290 branch from 94a6e40 to 801c14c Compare September 4, 2026 19:31
Comment thread lib/cluster.js
}

if (failedWorkers > 0) {
clusterWorkerScrapeFailures.observe(failedWorkers);

@jdmarshall jdmarshall Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think observing 0's when there are no workers is probably unlikely to happen but is also extra information about the class of failure we may be dealing with. As written this will never catch unclassified errors such as an NPE in the try block.

For instance, regressions in the Node API are not unknown. I work on a project that doesn't work on node 24.7-24.16, for instance, due to breaking changes in child_process.fork()

@jdmarshall jdmarshall left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This same change should be made in worker.js as well, please, to retain feature parity.

Comment thread lib/cluster.js
const metrics = await Promise.all(
registries.map(r => r.getMetricsAsJSON()),
);
metrics.push([await clusterWorkerScrapeFailures.get()]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are you sure you're not double-reporting this? That should be in the default registry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants