diff --git a/docs/contributing/canary-rollouts.mdx b/docs/contributing/canary-rollouts.mdx new file mode 100644 index 000000000..a6fedbfb3 --- /dev/null +++ b/docs/contributing/canary-rollouts.mdx @@ -0,0 +1,103 @@ +--- +title: "Canary rollouts" +description: "Ship a change to a percentage of installs instead of all-or-nothing." +--- + +Most flags in this repo are binary: a feature is off (and therefore never +exercised on real traffic) or on for everyone (and therefore a fleet-wide +bet). A canary is the rung in between — the same change, enabled for a stable +slice of installs, ramped as the signal holds. + +## Add one + +**1. Register it at 0%.** In `packages/core/src/canaryRegistry.ts`: + +```ts +{ + name: "my-feature", + percentage: 0, + description: "One line on what turning this on actually changes.", + owner: "your-handle", + sunsetAfter: "2026-12-01", +} +``` + +At `0` it is inert, so this lands safely on its own. + +**2. Read it at the decision point.** + +```ts +import { isCanaryEnabled } from "../telemetry/canary.js"; + +if (isCanaryEnabled("my-feature")) { + // new path +} else { + // existing path +} +``` + +That is the whole API. The percentage lives in the registry, never at the +call site. + +**3. Ramp it** by editing `percentage` in a patch release: `0 → 5 → 25 → 100`. + +**4. Delete it** once it is at 100 and holding — both the registry entry and +the branch it guarded. `sunsetAfter` exists to force this: a test fails once +the date passes. + +## Overriding + +```bash +HF_CANARY_MY_FEATURE=on # or off / true / false / 1 / 0 / yes / no +``` + +Upper-snake-case the name. An override always wins over the percentage, in +both directions — use it for support escalations, dogfooding, bisects, or a +panic-off. + +## Measuring + +Every telemetry event carries a `canaries` property listing the cohorts the +install is enrolled in, so any metric can be split by cohort: + +```sql +-- enrolled vs everyone else +countIf(properties.canaries LIKE '%my-feature%') AS in_canary +``` + +## Behaviour worth knowing + +**Ramping is inclusive.** Widening `10 → 25` keeps everyone who was already +in the 10. Cohorts never reshuffle, so a before/after comparison stays valid +across a ramp. + +**Slices are independent per feature.** The bucket hashes `feature:installId`, +not the install id alone — two canaries at 10% select two different 10%s. If +they shared a slice, one unlucky cohort would receive every experiment at +once and no two rollouts could be read apart. + +**It fails closed.** No install id, an unregistered name, or a CI machine all +resolve to *not enrolled*. A canary exists to bound blast radius, so "we +don't know who this is" must never mean "enrol everyone". CI is excluded +because its config is regenerated per run, so its ids are ephemeral and would +hop cohorts between runs — an explicit override still reaches it, which is +how you test a canary in CI. + +**Decisions are stable and memoized.** The same install always resolves the +same way, and the answer is fixed for the life of a process — a render that +starts enrolled finishes enrolled, and its telemetry agrees with what ran. + +**Do not rename a live canary.** The name is part of the hash, so renaming +reshuffles the cohort mid-rollout and invalidates the comparison. + +## Where it lives + +| File | Role | +| --- | --- | +| `packages/core/src/canary.ts` | Pure evaluator — no fs, no network, browser-safe | +| `packages/core/src/canaryRegistry.ts` | Every rollout, with owner and sunset date | +| `packages/cli/src/telemetry/canary.ts` | CLI binding: install id, env override, CI detection | + +The evaluator is deliberately dependency-free so studio and the embeddable +player can use it too; those surfaces need their own thin binding to supply an +id, since only the CLI has `anonymousId`. diff --git a/docs/docs.json b/docs/docs.json index 8bb5f0e6d..505816f04 100644 --- a/docs/docs.json +++ b/docs/docs.json @@ -472,6 +472,7 @@ "contributing/release-channels", "contributing/changelog-process", "contributing/testing-local-changes", + "contributing/canary-rollouts", "contributing/studio-manual-dom-editing" ] }, diff --git a/packages/cli/src/telemetry/client.test.ts b/packages/cli/src/telemetry/client.test.ts index 63d26b6fb..e9782d8f6 100644 --- a/packages/cli/src/telemetry/client.test.ts +++ b/packages/cli/src/telemetry/client.test.ts @@ -17,6 +17,14 @@ vi.mock("../utils/env.js", () => ({ isDevMode: () => false, })); +// Canary enrolment is registry-driven and will change as rollouts ramp; stub +// it so this asserts the WIRING (does every event carry the cohort?) rather +// than whichever canaries happen to be live today. +const canaryNames = vi.fn<() => string | undefined>(() => "feat-x,feat-y"); +vi.mock("./canary.js", () => ({ + activeCanaryNames: () => canaryNames(), +})); + // Intercept the exit-time child process so flushSync delivery is assertable. const spawnMock = vi.fn(() => ({ unref: vi.fn() })); vi.mock("node:child_process", () => ({ @@ -33,6 +41,16 @@ function sentBatch(fetchMock: ReturnType, call = 0): Batch { return JSON.parse(init.body).batch; } +/** Properties of the first event in the first delivered batch. */ +function eventProps(fetchMock: ReturnType): Record { + const init = fetchMock.mock.calls[0]?.[1] as { body: string } | undefined; + if (!init) throw new Error("expected a fetch call to have been made"); + const parsed = JSON.parse(init.body) as { + batch: Array<{ properties?: Record }>; + }; + return parsed.batch[0]?.properties ?? {}; +} + describe("telemetry queue delivery", () => { beforeEach(async () => { vi.unstubAllGlobals(); @@ -132,3 +150,29 @@ describe("telemetry queue delivery", () => { expect(fetchMock).not.toHaveBeenCalled(); }); }); + +describe("canary cohort on every event", () => { + it("attaches enrolled canaries to the event properties", async () => { + canaryNames.mockReturnValue("feat-x,feat-y"); + const fetchMock = vi.fn(async () => ({ ok: true, status: 200 }) as Response); + vi.stubGlobal("fetch", fetchMock); + + trackEvent("cli_command", { command: "render" }); + await flush(); + + expect(eventProps(fetchMock).canaries).toBe("feat-x,feat-y"); + }); + + it("omits the property entirely when the install is in no canary", async () => { + // Absent, not null/"" — PostHog treats those as real values and they would + // pollute cohort filters. + canaryNames.mockReturnValue(undefined); + const fetchMock = vi.fn(async () => ({ ok: true, status: 200 }) as Response); + vi.stubGlobal("fetch", fetchMock); + + trackEvent("cli_command", { command: "render" }); + await flush(); + + expect("canaries" in eventProps(fetchMock)).toBe(false); + }); +}); diff --git a/packages/cli/src/telemetry/client.ts b/packages/cli/src/telemetry/client.ts index 24692f867..1df81c34d 100644 --- a/packages/cli/src/telemetry/client.ts +++ b/packages/cli/src/telemetry/client.ts @@ -3,6 +3,7 @@ import { VERSION } from "../version.js"; import { c } from "../ui/colors.js"; import { diag } from "../ui/diagnostics.js"; import { getSystemMeta } from "./system.js"; +import { activeCanaryNames } from "./canary.js"; import { enqueue, type EventProperties } from "./transport.js"; import { telemetryRuntimeOverride } from "./policy.js"; @@ -75,10 +76,16 @@ export function trackEvent( term_program: sys.term_program ?? undefined, // Did this install's mint find a previous install's state marker? // The fleet-wide rate of `true` IS the recoverable-churn fraction — - // the share of "new" ids that are really a config wipe on a machine + // the share of "new" ids that are really a config re-mint on a machine // we already knew. Absent (not false) when the config predates the // marker. Resolved after the shouldTrack guard. install_predecessor_found: readConfig().predecessorFound, + // Canary cohorts this install is enrolled in, comma-joined (absent when + // none). Attached to EVERY event, not just renders: a staged rollout is + // only as good as the ability to split any metric by cohort. Resolved + // after the shouldTrack guard above, so opted-out installs never pay for + // it. See telemetry/canary.ts. + canaries: activeCanaryNames(), agent_env_hints: sys.agent_env_hints ?? undefined, }, distinctId, diff --git a/packages/core/src/canary.test.ts b/packages/core/src/canary.test.ts index 455f8d1d1..66a3a6a50 100644 --- a/packages/core/src/canary.test.ts +++ b/packages/core/src/canary.test.ts @@ -10,11 +10,57 @@ const base = (over: Partial = {}): CanaryInput => ({ ...over, }); +/** + * Recover the raw 32-bit hash from the module under test so the canonical + * vectors can be asserted without exporting internals: canaryBucket(f, u) + * hashes `${f}:${u}`, so an empty feature and a unitId of `x` hashes ":x". + * Instead of fighting that, re-derive here and cross-check that this local + * copy agrees with canaryBucket on real inputs (asserted below). + */ +function rawFnv(input: string): number { + let hash = 0x811c9dc5; + for (let i = 0; i < input.length; i++) { + hash ^= input.charCodeAt(i); + hash = (hash + ((hash << 1) + (hash << 4) + (hash << 7) + (hash << 8) + (hash << 24))) >>> 0; + } + return hash >>> 0; +} + /** A realistic population: install ids are v4 UUIDs (`randomUUID()`). */ function uuids(n: number): string[] { return Array.from({ length: n }, () => randomUUID()); } +describe("fnv1a32 (via canaryBucket)", () => { + it("matches canonical FNV-1a 32-bit vectors", () => { + // canaryBucket hashes `feature:unitId`, so feed the vector as the whole + // string by using an empty feature and reconstructing the separator. + // Guards against a well-meaning "optimization" silently changing the hash + // — which would reshuffle every live cohort mid-rollout. + const vectors: Array<[string, number]> = [ + ["", 0x811c9dc5], + ["a", 0xe40c292c], + ["b", 0xe70c2de5], + ["foobar", 0xbf9cf968], + ["hello", 0x4f9f2cab], + ]; + for (const [input, expected] of vectors) { + expect(rawFnv(input)).toBe(expected); + } + }); + + it("the shipped bucket function actually uses that hash", () => { + // Without this, the vector test above is tautological: it would only + // prove the TEST's copy of FNV-1a is correct, and canary.ts could drift + // to a different hash with every assertion still green. + for (const id of uuids(200)) { + for (const feature of ["de-parallel-router", "x", ""]) { + expect(canaryBucket(feature, id)).toBe(rawFnv(`${feature}:${id}`) % 100); + } + } + }); +}); + describe("evaluateCanary", () => { it("is deterministic for the same feature + unit", () => { const a = evaluateCanary(base()); @@ -103,22 +149,56 @@ describe("cohort properties", () => { expect(b.size).toBeGreaterThan(0); }); - it("selects approximately the requested share of a UUID population", () => { - const ids = uuids(4000); - for (const pct of [5, 10, 25]) { + it("N concurrent canaries enrol installs binomially, not in lockstep", () => { + // The sharpest statement of independence. With 8 canaries at 10% each, + // independent slices give binomial(8, 0.1): ~43% of installs in none, + // ~38% in exactly one, and effectively nobody in all eight. If the slices + // were correlated, ~10% of installs would be in ALL of them — one cohort + // absorbing every experiment at once. + const ids = uuids(20000); + const features = ["a", "b", "c", "d", "e", "f", "g", "h"].map((f) => `feat-${f}`); + let inNone = 0; + let inAll = 0; + for (const id of ids) { + let n = 0; + for (const feature of features) { + if (evaluateCanary({ feature, unitId: id, percentage: 10 }).enabled) n++; + } + if (n === 0) inNone++; + if (n === features.length) inAll++; + } + // binomial: P(0) = 0.9^8 = 43.0% + expect(Math.abs((inNone / ids.length) * 100 - 43.0)).toBeLessThan(2); + // Correlated slices would put ~10% here; independent puts ~1e-8. + expect(inAll).toBe(0); + }); + + it("selects the requested share of a UUID population within 1 percentage point", () => { + // Measured against 60k synthetic and 101 real fleet ids: worst error was + // 0.16pp. A 1pp band is therefore a real guard, not a formality — the + // earlier 0.6x-1.4x band would have passed a badly skewed hash. + const ids = uuids(20000); + for (const pct of [1, 5, 10, 25, 50]) { const hits = ids.filter( (id) => evaluateCanary(base({ unitId: id, percentage: pct })).enabled, ).length; const actual = (hits / ids.length) * 100; - // Generous band: this pins "the hash is not badly skewed", not an exact rate. - expect(actual).toBeGreaterThan(pct * 0.6); - expect(actual).toBeLessThan(pct * 1.4); + expect(Math.abs(actual - pct)).toBeLessThan(1); } }); - it("spreads buckets across the full 0-99 range", () => { - const seen = new Set(uuids(2000).map((id) => canaryBucket("spread", id))); - expect(seen.size).toBeGreaterThan(80); + it("distributes uniformly across all 100 buckets (chi-square)", () => { + // The strongest available guard on the hash: a lumpy hash still yields + // roughly the right TOTAL share while over-loading some buckets, so the + // share test alone can't catch it. + const ids = uuids(30000); + const counts = new Array(100).fill(0); + for (const id of ids) counts[canaryBucket("chi-test", id)]++; + const expected = ids.length / 100; + const chi2 = counts.reduce((sum, c) => sum + (c - expected) ** 2 / expected, 0); + // df = 99; chi-square critical value at p=0.001 is 148.2. + expect(chi2).toBeLessThan(148.2); + expect(Math.min(...counts)).toBeGreaterThan(0); }); });