Aggregate prom-client metrics across cluster workers for /metrics#667
Open
silvadenisaraujo wants to merge 5 commits into
Open
Aggregate prom-client metrics across cluster workers for /metrics#667silvadenisaraujo wants to merge 5 commits into
silvadenisaraujo wants to merge 5 commits into
Conversation
Specification only (no implementation) for fixing per-worker Prometheus registry interleaving on /metrics via prom-client AggregatorRegistry. Internal task id: task-58ee1acb (bean)
Multi-worker runtimes kept a per-worker prom-client registry, so /metrics answered from whichever worker the shared-port round-robin picked. Because keep-alive (5s) is shorter than the scrape interval, Prometheus stored an interleaved braid of independent counters under one instance label, treating every downward sample as a counter reset and inflating rate()/increase() on runtime_* counters by orders of magnitude. Aggregate across workers with prom-client's AggregatorRegistry: the master builds a single merged, monotonic view over the existing cluster IPC and the worker answering the scrape serves that aggregate. Single-worker / LINKED mode keeps serving the local default registry unchanged. - src/service/metrics/clusterMetricsAggregator.ts: IPC request/response protocol, master aggregator, worker responder, timeout fallback to local registry. - master.ts / worker/index.ts: wire the tagged IPC messages and set up the aggregator only when workers > 1. - builtIn/middlewares.ts: prometheusLoggerMiddleware serves the aggregate in multi-worker mode, local registry otherwise; /metrics still excluded from request counters. Includes unit tests proving cross-worker sum, monotonicity, single-worker behavior, and default-metric presence. Co-authored-by: Cursor <cursoragent@cursor.com>
This analysis artifact belongs to a separate investigation and was included in this PR by mistake. Co-authored-by: Cursor <cursoragent@cursor.com>
Review comments: - spec: drop redundant task-id/branch metadata lines; mark Status accepted now that the implementation ships in this PR - clusterMetricsAggregator: use optional chaining in the message type guards - worker/index: pass an explicit callback and initial value to .reduce() instead of the bare mergeDeepRight function SonarQube quality gate: - raise new-code coverage: clusterMetricsAggregator.ts to 100% (registry lifecycle + error/fallback paths) and cover the new master/worker IPC onMessage branches via exported handlers Co-authored-by: Cursor <cursoragent@cursor.com>
silvadenisaraujo
marked this pull request as ready for review
July 23, 2026 19:21
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
VTEX IO runtimes run as a Node.js cluster of
min(cpus, 4)workers, each keeping its own prom-client registry, and/metricsis answered by whichever worker the shared-port round-robin hands the scrape to. Because keep-alive (5s) is shorter than the scrape interval, nearly every scrape hits a different worker, so Prometheus stores an interleaved braid of 4 independent counters under one instance label — treating every downward sample as a counter reset and inflatingincrease()/rate()onruntime_*counters by orders of magnitude (~×40 measured live). This PR fixes that by aggregating across workers with prom-client'sAggregatorRegistry(pinned^14.2.0): the master builds a single merged, monotonic view over the existing cluster IPC and the worker answering the scrape serves that aggregate. Single-worker / LINKED mode stays unchanged.Spec
Accepted. See
specs/aggregate-prom-client-metrics-across-cluster-wor.md.Implementation
src/service/metrics/clusterMetricsAggregator.ts(new): the master-sideAggregatorRegistryplus a small tagged-IPC protocol. A worker answering a/metricsscrape asks the master over the existing cluster IPC (AGG_METRICS_REQ), the master builds the merged, monotonic cluster view viaclusterMetrics()and replies (AGG_METRICS_RES). prom-client's ownprom-client:*cluster messages are recognised and ignored by our handlers. On timeout/error the worker falls back to its localregister.metrics()so/metricsnever hangs.src/service/master.ts: initialises the master aggregator whenworkers > 1and routes worker aggregate requests / prom-client messages inonMessage.src/service/worker/index.ts: installs the worker-side cluster responder whenworkers > 1and routes the master's aggregate response inonMessage.src/service/worker/runtime/builtIn/middlewares.ts:prometheusLoggerMiddleware(workers)serves the cluster aggregate in multi-worker mode and the local default registry in single-worker mode. Scrapes are still excluded from request counters (early return preserved) and metric names/labels are unchanged.Covered by tests proving monotonicity across whichever worker answers, sum-across-N-workers, single-worker default-registry behaviour, and presence of default process metrics.
Testing
Validation
Validated live on cluster
dv-dj-iotest-eks-use1c-ju2, deploymentvtex-search-resolver-1-106-1-023ca1f28e29b7a(imageservice-node:7.7.13-beta.0,2 workers).
Method: port-forward one pod and scrape
GET /metrics:5050repeatedly. Eachcurlopens a new connection (round-robined across workers — the condition thatexposed the original bug), asserting no
runtime_http_requests_totalseriesdecreases between consecutive scrapes.
Results (fixed build):
runtime_http_requests_totalfirst → lastThe aggregate is stable and strictly non-decreasing regardless of which worker
answers — no braiding, no counter resets. No fallback/timeout warnings in logs.
Baseline contrast (
service-node:6.42.0, pre-fix): the identical scrape methodproduced braided per-worker output (e.g.
catalog,200cycling 285 → 104 → 167 → …),the behavior that inflated Prometheus
rate()/increase().Live coverage: AC1 (monotonic) ✅, AC2 (sum across workers) ✅, AC4 (default metrics
present) ✅. AC3 (single-worker path) covered by unit tests.
Internal task id: task-58ee1acb (bean)