Skip to content

Commit 474405a

Browse files
committed
fix(webapp): bound the snapshot-store cohort metric to two labels
The cohort metric labelled each member with its organizationId, so the series count grew with the cohort and leaked an unbounded high-cardinality id into metrics. Collapse it to a fixed cohort/other pair; per-org investigation moves to traces and logs filtered by orgId.
1 parent 95ae613 commit 474405a

2 files changed

Lines changed: 13 additions & 8 deletions

File tree

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1-
// Bounds per-org metric cardinality: only cohort members get a distinct label, everything else
2-
// (undefined included) collapses to "other" — at most cohort size plus one series.
1+
// Bounds per-org metric cardinality to exactly two series. A cohort member is labelled "cohort" and
2+
// everything else (undefined included) "other", so the series count never grows with the cohort. Never
3+
// emit the organizationId itself: it is unbounded at rollout and violates the no-high-cardinality-label
4+
// rule. Per-org investigation belongs in traces/logs filtered by orgId, not in a metric label.
35
export function cohortMetricLabel(
46
organizationId: string | undefined,
57
isCohortMember: (organizationId: string) => boolean
68
): string {
7-
return organizationId !== undefined && isCohortMember(organizationId) ? organizationId : "other";
9+
return organizationId !== undefined && isCohortMember(organizationId) ? "cohort" : "other";
810
}

‎apps/webapp/test/cohortMetricLabel.test.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ import { describe, expect, it } from "vitest";
22
import { cohortMetricLabel } from "~/v3/cohortMetricLabel.server";
33

44
describe("cohortMetricLabel", () => {
5-
it("returns the org id when it is a cohort member", () => {
6-
expect(cohortMetricLabel("org_123", (id) => id === "org_123")).toBe("org_123");
5+
it("returns the fixed 'cohort' label for a member, never the org id", () => {
6+
expect(cohortMetricLabel("org_123", (id) => id === "org_123")).toBe("cohort");
77
});
88

99
it("returns 'other' for a non-member org", () => {
@@ -14,9 +14,12 @@ describe("cohortMetricLabel", () => {
1414
expect(cohortMetricLabel(undefined, () => true)).toBe("other");
1515
});
1616

17-
it("collapses every org to 'other' with an always-false predicate", () => {
18-
for (const id of ["org_a", "org_b", "org_c"]) {
19-
expect(cohortMetricLabel(id, () => false)).toBe("other");
17+
it("emits at most two distinct labels regardless of how many orgs are members", () => {
18+
const labels = new Set<string>();
19+
for (const id of ["org_a", "org_b", "org_c", "org_d"]) {
20+
labels.add(cohortMetricLabel(id, () => true));
21+
labels.add(cohortMetricLabel(id, () => false));
2022
}
23+
expect([...labels].sort()).toEqual(["cohort", "other"]);
2124
});
2225
});

0 commit comments

Comments
 (0)