Prevent Metric attribute serialization key collisions - #6901
Conversation
🦋 Changeset detectedLatest commit: 76359a0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
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 |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — a single reproduction test for a Metric attribute series key collision:
- New test
"keeps distinct attribute sets in separate series"— creates two counters sharing a metric ID but with attribute sets{ a: "b,c=d" }and{ a: "b", c: "d" }that naively serialize to the same string, then asserts each counter maintains an independent count.
The test is well-formed: it follows the existing patterns in the file (it.effect, Effect.gen, nextId() for isolation, assert.strictEqual), uses the correct Metric APIs, and the assertions unambiguously capture the expected contract — that distinct attribute sets must be tracked as separate series.
DeepSeek Pro (free via Pullfrog for OSS) (Kimi K2 not used — the program covers this model; add its provider key to run your pick) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — replaced the buggy manual attribute serialization with JSON.stringify to prevent key collisions:
- Inlined
serializeAttributesto useJSON.stringify— removed the two-functionserializeAttributes→serializeEntrieschain that concatenated unescapedkey=valuepairs with commas.JSON.stringifyofObject.entries(or the array path directly) produces deterministic, collision-free strings because arrays of string tuples never produce ambiguous serialization. - Added a patch changeset — correctly scoped as
"effect": patch. - Retained the reproduction test from the prior review — the test
"keeps distinct attribute sets in separate series"now serves as the regression test for the fix.
DeepSeek Pro (free via Pullfrog for OSS) (Kimi K2 not used — the program covers this model; add its provider key to run your pick) | 𝕏
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|

Summary
Different valid metric attribute sets can serialize to the same registry key, causing logically separate series to share a hook and contaminate each other's values.
Important
This PR starts with focused failing reproduction tests. Add the implementation fix to this same branch; CI is expected to fail until that fix is included.
Distinct metric attribute sets can share registry state
Module:
MetricAudit ID:
core-g-r-metric-attribute-series-key-collisionSeverity / confidence: high / high
What happens
Different valid metric attribute sets can serialize to the same registry key, causing logically separate series to share a hook and contaminate each other's values.
Why it happens
makeKey concatenates unescaped key=value pairs with commas. The valid sets { a: "b,c=d" } and { a: "b", c: "d" } both serialize as a=b,c=d and occupy one registry entry.
Expected behavior
Metric attributes identify dimensions: referentially equivalent definitions share a series, but each distinct attribute combination represents a separate series.
Relevant implementation
These links and excerpts are pinned to audit base
c9b56ab507f224426ee8388dc450da447ec4715f.packages/effect/src/Metric.ts:2823-2829packages/effect/src/Metric.ts:3480-3508View problematic code at
packages/effect/src/Metric.ts:2823-2829View exact lines on GitHub
View problematic code at
packages/effect/src/Metric.ts:3480-3508View exact lines on GitHub
Reproduction
pnpm vitest run packages/effect/test/Metric.test.ts -t "keeps distinct attribute sets in separate series"Observed failure: The intended failure was reproduced: the first counter was 11 instead of 1.
Implementation handoff
The initial reproduction tests on this branch are the regression specification for the implementation fix that should follow in this PR.
pnpm vitest run packages/effect/test/Metric.test.ts -t "keeps distinct attribute sets in separate series"Audit provenance
c9b56ab507f224426ee8388dc450da447ec4715fc9b56ab507f224426ee8388dc450da447ec4715fcore-g-r-metric-attribute-series-key-collisionCloses EFF-348