[workers-utils] Replace deprecated promjs with MetricsRegistry#12753
[workers-utils] Replace deprecated promjs with MetricsRegistry#12753petebacondarwin merged 6 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 11a97bd The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
✅ All changesets look good |
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
wrangler
commit: |
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
petebacondarwin
force-pushed
the
pbacondarwin/replace-promjs-with-metrics-registry
branch
from
0c86431 to
b88af36
Compare
…sue cloudflare#12753 to replace a deprecated library with a lightweight, maintained solution.
|
Blocked on #12791 |
petebacondarwin
added
blocked
petebacondarwin
force-pushed
the
pbacondarwin/replace-promjs-with-metrics-registry
branch
2 times, most recently
from
5ebbdc4 to
07f0772
Compare
|
Codeowners approval required for this PR:
Show detailed file reviewers |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 2 new potential issues.
⚠️ 2 issues in files not directly in the diff
⚠️ Incomplete migration: format-errors test file still mocks removed promjs dependency (packages/format-errors/src/__tests__/index.test.ts:5-12)
The PR removed promjs from packages/format-errors/package.json and migrated the source code (packages/format-errors/src/index.ts:1) to use @cloudflare/workers-utils/metrics, but the test file was not updated. It still contains vi.mock("promjs", ...) which mocks a module that no longer exists in the dependency tree. While this dead mock doesn't cause an immediate test failure (since the tested functions don't exercise the default export's metrics path), it's an incomplete mechanical transformation — the analogous cleanup was correctly done for edge-preview-authenticated-proxy (its vitest config was updated) but missed here.
⚠️ Incomplete migration: format-errors vitest.config.mts still has promjs alias pointing to non-existent path (packages/format-errors/vitest.config.mts:11-18)
The PR removed promjs from packages/format-errors/package.json but did not update packages/format-errors/vitest.config.mts, which still contains a resolve alias mapping promjs to node_modules/promjs/index.js (line 16). Since promjs is no longer installed, this path doesn't exist. The analogous cleanup was correctly performed for packages/edge-preview-authenticated-proxy/vitest.config.mts (which had the same alias removed in this PR), but was missed for format-errors. While this dead alias doesn't currently cause a test failure (nothing imports promjs anymore), it's stale configuration that should be cleaned up as part of this migration.
View 7 additional findings in Devin Review.
petebacondarwin
force-pushed
the
pbacondarwin/replace-promjs-with-metrics-registry
branch
from
07f0772 to
68e5f27
Compare
There was a problem hiding this comment.
Devin Review found 2 new potential issues.
⚠️ 2 issues in files not directly in the diff
⚠️ Incomplete migration: format-errors test file still mocks removed promjs dependency (packages/format-errors/src/__tests__/index.test.ts:5-12)
The PR removed promjs from packages/format-errors/package.json and migrated the source code (packages/format-errors/src/index.ts:1) to use @cloudflare/workers-utils/metrics, but the test file was not updated. It still contains vi.mock("promjs", ...) which mocks a module that no longer exists in the dependency tree. While this dead mock doesn't cause an immediate test failure (since the tested functions don't exercise the default export's metrics path), it's an incomplete mechanical transformation — the analogous cleanup was correctly done for edge-preview-authenticated-proxy (its vitest config was updated) but missed here.
⚠️ Incomplete migration: format-errors vitest.config.mts still has promjs alias pointing to non-existent path (packages/format-errors/vitest.config.mts:11-18)
The PR removed promjs from packages/format-errors/package.json but did not update packages/format-errors/vitest.config.mts, which still contains a resolve alias mapping promjs to node_modules/promjs/index.js (line 16). Since promjs is no longer installed, this path doesn't exist. The analogous cleanup was correctly performed for packages/edge-preview-authenticated-proxy/vitest.config.mts (which had the same alias removed in this PR), but was missed for format-errors. While this dead alias doesn't currently cause a test failure (nothing imports promjs anymore), it's stale configuration that should be cleaned up as part of this migration.
View 7 additional findings in Devin Review.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
dario-piotrowicz
left a comment
dario-piotrowicz
left a comment
There was a problem hiding this comment.
Looks good to me, however I'd suggest not to use generic names as /metrics and MetricsRegistry but instead have names that clearly convey that these are Prometheus specific. Such as /prometheus-metrics and PrometheusMetricsRegistry for example
petebacondarwin
force-pushed
the
pbacondarwin/replace-promjs-with-metrics-registry
branch
2 times, most recently
from
de29014 to
e6ad9d4
Compare
Replace the deprecated and unmaintained promjs library (broken package.json, no updates since 2022) with a lightweight MetricsRegistry class in @cloudflare/workers-utils/metrics. - Add MetricsRegistry class that produces byte-identical Prometheus text exposition format for counter metrics - Add comprehensive test suite (35 tests) including cases ported from the original promjs test suite - Update edge-preview-authenticated-proxy, playground-preview-worker, and format-errors to use the new MetricsRegistry - Remove promjs from all package.json files - Remove the vitest alias workaround for promjs broken package.json - Fix format-errors tsconfig moduleResolution to support subpath exports
petebacondarwin
force-pushed
the
pbacondarwin/replace-promjs-with-metrics-registry
branch
from
e6ad9d4 to
11a97bd
Compare
|
Codeowners approval required for this PR:
Show detailed file reviewers |
petebacondarwin
deleted the
pbacondarwin/replace-promjs-with-metrics-registry
branch
4 participants
Replace the deprecated and unmaintained
promjslibrary with a lightweightMetricsRegistryclass in@cloudflare/workers-utils/metrics.promjshas been unmaintained for 3+ years, has a brokenpackage.json(themainfield points to a non-existentlib/directory, requiring a vitest alias workaround), and is used in the most trivial way possible: just counters with no labels, always value 1, pushed per-request to a Prometheus gateway.What changed:
MetricsRegistryclass to@cloudflare/workers-utils/metricsthat produces byte-identical Prometheus text exposition format for counter metrics (~80 lines including docs)edge-preview-authenticated-proxy,playground-preview-worker, andformat-errorsto use the newMetricsRegistrypromjsfrom allpackage.jsonfilespackage.jsonformat-errorstsconfigmoduleResolutionfrom"node"to"bundler"to support subpath exportsThe metrics utility is exposed as a separate subpath export (
@cloudflare/workers-utils/metrics) which produces a tiny 160-byte standalone module, ensuring compatibility withvitest-pool-workers/workerd's SSR module resolution.