diff --git a/docs/contributing/canary-rollouts.mdx b/docs/contributing/canary-rollouts.mdx index a6fedbfb3..4e59fda7f 100644 --- a/docs/contributing/canary-rollouts.mdx +++ b/docs/contributing/canary-rollouts.mdx @@ -57,14 +57,32 @@ 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: +Every telemetry event carries the assignment as a PostHog flag property: + +``` +$feature/canary-my-feature: "true" | "false" +``` + +PostHog treats `$feature/` as a first-class flag, so breakdowns, funnels +split by cohort and the experiment surfaces work on a canary with **nothing +configured server-side** — the decision still happens locally and offline, +which the render path requires. ```sql --- enrolled vs everyone else -countIf(properties.canaries LIKE '%my-feature%') AS in_canary +SELECT properties['$feature/canary-my-feature'] AS cohort, count() +FROM events WHERE event = 'render_complete' GROUP BY cohort ``` +Two details worth knowing: + +- **Both arms are emitted.** A non-enrolled install reports `"false"`, not a + missing property. Absent means *this build predates the canary*, which is a + different fact from *this install is control* — collapsing them makes a ramp + unreadable. +- **Keys are namespaced with `canary-`.** A real PostHog flag namespace + already exists in this project, owned by the web app. The infix guarantees a + canary can never alias a real flag and fight it for the same property. + ## Behaviour worth knowing **Ramping is inclusive.** Widening `10 → 25` keeps everyone who was already diff --git a/packages/cli/src/telemetry/canary.test.ts b/packages/cli/src/telemetry/canary.test.ts index 8912a47d5..0d5043170 100644 --- a/packages/cli/src/telemetry/canary.test.ts +++ b/packages/cli/src/telemetry/canary.test.ts @@ -54,7 +54,7 @@ vi.mock("@hyperframes/core/canary-registry", async () => { }; }); -const { isCanaryEnabled, resolveCanary, activeCanaryNames, __resetCanaryCacheForTests } = +const { isCanaryEnabled, resolveCanary, canaryEventProperties, __resetCanaryCacheForTests } = await import("./canary.js"); beforeEach(() => { @@ -106,10 +106,14 @@ describe("CLI canary binding", () => { expect(isCanaryEnabled("test-beta")).toBe(true); }); - it("reports enrolled canaries for telemetry, undefined when none", () => { - expect(activeCanaryNames()).toBe("test-alpha"); + it("emits PostHog flag-shaped properties for every registered canary", () => { + expect(canaryEventProperties()).toEqual({ + "$feature/canary-test-alpha": "true", + "$feature/canary-test-beta": "false", + }); + __resetCanaryCacheForTests(); process.env.HF_CANARY_TEST_ALPHA = "off"; - expect(activeCanaryNames()).toBeUndefined(); + expect(canaryEventProperties()["$feature/canary-test-alpha"]).toBe("false"); }); }); diff --git a/packages/cli/src/telemetry/canary.ts b/packages/cli/src/telemetry/canary.ts index 53eca75ae..56d41ae7e 100644 --- a/packages/cli/src/telemetry/canary.ts +++ b/packages/cli/src/telemetry/canary.ts @@ -20,7 +20,12 @@ // Leaf subpath imports, not the "@hyperframes/core" barrel: this resolves on // the CLI startup path, and the barrel pulls the whole core surface. Same // reason the producer is lazily loaded. -import { evaluateCanary, parseCanaryOverride, type CanaryDecision } from "@hyperframes/core/canary"; +import { + canaryFeatureProperties, + evaluateCanary, + parseCanaryOverride, + type CanaryDecision, +} from "@hyperframes/core/canary"; import { CANARIES, canaryEnvVar, findCanary } from "@hyperframes/core/canary-registry"; import { readConfig } from "./config.js"; import { getSystemMeta } from "./system.js"; @@ -74,12 +79,14 @@ export function isCanaryEnabled(name: string): boolean { } /** - * Comma-joined names of the canaries this install is enrolled in, or - * undefined when none — attach to telemetry so every event can be segmented - * by cohort. One low-cardinality property beats a dynamic property per - * canary, and `contains` filtering works fine in PostHog. + * Canary assignments as PostHog flag properties — `$feature/canary-` + * set to `"true"` / `"false"` for every registered canary. Spread onto every + * event so any metric can be broken down by cohort using PostHog's native + * flag tooling, with nothing configured server-side. See + * `canaryFeatureProperties` for why non-enrolled canaries are emitted too. */ -export function activeCanaryNames(): string | undefined { - const active = CANARIES.filter((c) => resolveCanary(c.name).enabled).map((c) => c.name); - return active.length > 0 ? active.join(",") : undefined; +export function canaryEventProperties(): Record { + return canaryFeatureProperties( + CANARIES.map((c) => ({ name: c.name, enabled: resolveCanary(c.name).enabled })), + ); } diff --git a/packages/cli/src/telemetry/client.test.ts b/packages/cli/src/telemetry/client.test.ts index e9782d8f6..149bf1484 100644 --- a/packages/cli/src/telemetry/client.test.ts +++ b/packages/cli/src/telemetry/client.test.ts @@ -20,9 +20,12 @@ vi.mock("../utils/env.js", () => ({ // 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"); +const canaryProps = vi.fn<() => Record>(() => ({ + "$feature/canary-feat-x": "true", + "$feature/canary-feat-y": "false", +})); vi.mock("./canary.js", () => ({ - activeCanaryNames: () => canaryNames(), + canaryEventProperties: () => canaryProps(), })); // Intercept the exit-time child process so flushSync delivery is assertable. @@ -152,27 +155,34 @@ describe("telemetry queue delivery", () => { }); describe("canary cohort on every event", () => { - it("attaches enrolled canaries to the event properties", async () => { - canaryNames.mockReturnValue("feat-x,feat-y"); + it("attaches canary assignments as PostHog flag properties", async () => { + canaryProps.mockReturnValue({ + "$feature/canary-feat-x": "true", + "$feature/canary-feat-y": "false", + }); 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"); + const props = eventProps(fetchMock); + // Enrolled AND control are both emitted — absent would mean "this build + // predates the canary", a different fact from "not enrolled". + expect(props["$feature/canary-feat-x"]).toBe("true"); + expect(props["$feature/canary-feat-y"]).toBe("false"); }); - 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); + it("adds no canary properties when the registry is empty", async () => { + canaryProps.mockReturnValue({}); 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); + expect(Object.keys(eventProps(fetchMock)).some((k) => k.startsWith("$feature/canary-"))).toBe( + false, + ); }); }); diff --git a/packages/cli/src/telemetry/client.ts b/packages/cli/src/telemetry/client.ts index 1df81c34d..a1af000ea 100644 --- a/packages/cli/src/telemetry/client.ts +++ b/packages/cli/src/telemetry/client.ts @@ -3,7 +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 { canaryEventProperties } from "./canary.js"; import { enqueue, type EventProperties } from "./transport.js"; import { telemetryRuntimeOverride } from "./policy.js"; @@ -80,12 +80,13 @@ export function trackEvent( // 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(), + // Canary assignments as `$feature/canary-` — PostHog's native flag + // property shape, so breakdowns and experiment analysis work on a canary + // with nothing configured server-side. On 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, so opted-out installs + // never pay for it. See telemetry/canary.ts. + ...canaryEventProperties(), 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 66a3a6a50..78b038c32 100644 --- a/packages/core/src/canary.test.ts +++ b/packages/core/src/canary.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { randomUUID } from "node:crypto"; import { canaryBucket, evaluateCanary, parseCanaryOverride, type CanaryInput } from "./canary.js"; import { CANARIES, canaryEnvVar, findCanary, overdueCanaries } from "./canaryRegistry.js"; +import { CANARY_FEATURE_PREFIX, canaryFeatureKey, canaryFeatureProperties } from "./canary.js"; const base = (over: Partial = {}): CanaryInput => ({ feature: "test-feature", @@ -249,3 +250,36 @@ describe("registry", () => { expect(overdueCanaries()).toEqual([]); }); }); + +describe("PostHog flag-shaped properties", () => { + it("namespaces keys so a canary can never alias a real PostHog flag", () => { + // A real flag namespace already exists in this project, owned by the web + // app (e.g. `enable-chat-tab`). Without the `canary-` infix a canary named + // after a real flag would fight it for the same property. + expect(canaryFeatureKey("de-parallel-router")).toBe("$feature/canary-de-parallel-router"); + expect(CANARY_FEATURE_PREFIX.startsWith("$feature/")).toBe(true); + }); + + it("emits every canary, not just enrolled ones", () => { + // Absent vs "false" are different facts: absent = this build predates the + // canary, "false" = this build has it and this install is control. + // Collapsing them makes a ramp unreadable. + const props = canaryFeatureProperties([ + { name: "a", enabled: true }, + { name: "b", enabled: false }, + ]); + expect(props).toEqual({ + "$feature/canary-a": "true", + "$feature/canary-b": "false", + }); + }); + + it("uses string values, matching how PostHog records boolean flags", () => { + const props = canaryFeatureProperties([{ name: "a", enabled: true }]); + expect(typeof props["$feature/canary-a"]).toBe("string"); + }); + + it("is empty when nothing is registered", () => { + expect(canaryFeatureProperties([])).toEqual({}); + }); +}); diff --git a/packages/core/src/canary.ts b/packages/core/src/canary.ts index 0b00e0641..e96642338 100644 --- a/packages/core/src/canary.ts +++ b/packages/core/src/canary.ts @@ -134,3 +134,47 @@ export function parseCanaryOverride(raw: string | undefined): boolean | undefine if (v === "0" || v === "false" || v === "off" || v === "no") return false; return undefined; } + +/** + * Property-name prefix for canary assignments on telemetry events. + * + * PostHog treats `$feature/` as a first-class flag property: breakdowns, + * funnels split by cohort and the experiment surfaces all key on it. Emitting + * assignments in that shape means the analysis tooling works on a canary with + * nothing configured server-side — the decision still happens locally and + * offline, which the render path requires (no render-time network calls, and + * behaviour must not depend on analytics being reachable). + * + * The `canary-` infix is deliberate. A real PostHog flag namespace already + * exists in this project, owned by the web app (e.g. `enable-chat-tab`, set by + * posthog-js). Namespacing guarantees a canary key can never alias a real flag + * key and have the two fight over the same property. + */ +export const CANARY_FEATURE_PREFIX = "$feature/canary-"; + +/** `de-parallel-router` → `$feature/canary-de-parallel-router`. */ +export function canaryFeatureKey(name: string): string { + return `${CANARY_FEATURE_PREFIX}${name}`; +} + +/** + * Build the telemetry properties for a set of resolved canaries. + * + * Emits EVERY registered canary, not just the enrolled ones, because absent + * and `"false"` mean different things: absent is "this build predates the + * canary", `"false"` is "this build has it and this install is not enrolled". + * Collapsing those makes a ramp unreadable — you cannot tell a control group + * from an old version. + * + * Values are the strings `"true"` / `"false"` to match how PostHog records + * boolean flag values, so the property is directly comparable to a real flag. + */ +export function canaryFeatureProperties( + entries: ReadonlyArray<{ name: string; enabled: boolean }>, +): Record { + const props: Record = {}; + for (const entry of entries) { + props[canaryFeatureKey(entry.name)] = entry.enabled ? "true" : "false"; + } + return props; +} diff --git a/packages/studio/src/telemetry/canary.test.ts b/packages/studio/src/telemetry/canary.test.ts index ee91abba0..5ae407721 100644 --- a/packages/studio/src/telemetry/canary.test.ts +++ b/packages/studio/src/telemetry/canary.test.ts @@ -32,7 +32,7 @@ vi.mock("@hyperframes/core/canary-registry", async () => { const { isCanaryEnabled, resolveCanary, - activeCanaryNames, + canaryEventProperties, canaryParamName, __resetStudioCanaryCacheForTests, } = await import("./canary"); @@ -164,11 +164,14 @@ describe("cohort identity", () => { }); describe("telemetry", () => { - it("reports enrolled canaries, undefined when none", () => { - expect(activeCanaryNames()).toBe("on-everywhere"); + it("emits the same PostHog flag-shaped properties as the CLI", () => { + expect(canaryEventProperties()).toEqual({ + "$feature/canary-on-everywhere": "true", + "$feature/canary-off-everywhere": "false", + }); __resetStudioCanaryCacheForTests(); setSearch("?hf_canary_on_everywhere=off"); - expect(activeCanaryNames()).toBeUndefined(); + expect(canaryEventProperties()["$feature/canary-on-everywhere"]).toBe("false"); }); }); diff --git a/packages/studio/src/telemetry/canary.ts b/packages/studio/src/telemetry/canary.ts index f1494390e..bf8b13905 100644 --- a/packages/studio/src/telemetry/canary.ts +++ b/packages/studio/src/telemetry/canary.ts @@ -33,7 +33,12 @@ // browser bundle, and the barrel re-exports the whole core surface (parsers, // lint, studio-server); pulling that in here drags a Node-oriented dependency // graph into the bundle. These two modules are pure and leaf. -import { evaluateCanary, parseCanaryOverride, type CanaryDecision } from "@hyperframes/core/canary"; +import { + canaryFeatureProperties, + evaluateCanary, + parseCanaryOverride, + type CanaryDecision, +} from "@hyperframes/core/canary"; import { CANARIES, findCanary } from "@hyperframes/core/canary-registry"; import { resolveStudioDistinctId } from "./distinctId"; import { safeSessionStorage } from "../utils/safeStorage"; @@ -142,11 +147,12 @@ export function isCanaryEnabled(name: string): boolean { } /** - * Comma-joined names of the canaries this install is enrolled in, or undefined - * when none — attached to every Studio event so any metric can be split by - * cohort, exactly as the CLI does. + * Canary assignments as PostHog flag properties (`$feature/canary-`), + * attached to every Studio event so any metric can be split by cohort — + * identical shape to the CLI, so a rollout spanning both reads as one flag. */ -export function activeCanaryNames(): string | undefined { - const active = CANARIES.filter((c) => resolveCanary(c.name).enabled).map((c) => c.name); - return active.length > 0 ? active.join(",") : undefined; +export function canaryEventProperties(): Record { + return canaryFeatureProperties( + CANARIES.map((c) => ({ name: c.name, enabled: resolveCanary(c.name).enabled })), + ); } diff --git a/packages/studio/src/telemetry/client.ts b/packages/studio/src/telemetry/client.ts index b872ceb7a..f43aa3486 100644 --- a/packages/studio/src/telemetry/client.ts +++ b/packages/studio/src/telemetry/client.ts @@ -6,7 +6,7 @@ import { getAnonymousId, hasShownNotice, isOptedOut, markNoticeShown } from "./config"; import { getBrowserSystemMeta } from "./system"; -import { activeCanaryNames } from "./canary"; +import { canaryEventProperties } from "./canary"; // Write-only PostHog project key, safe to embed in client code. const POSTHOG_API_KEY = "phc_zjjbX0PnWxERXrMHhkEJWj9A9BhGVLRReICgsfTMmpx"; @@ -74,10 +74,10 @@ export function trackEvent(event: string, properties: EventProperties = {}): voi const sys = getBrowserSystemMeta(); eventQueue.push({ event, - // `canaries` mirrors the CLI: the cohorts this install is enrolled in, on - // EVERY event so any metric can be split by cohort. Resolved after the - // shouldTrack guard, so opted-out users never pay for it. - properties: { ...properties, ...sys, canaries: activeCanaryNames() }, + // Canary assignments as `$feature/canary-`, mirroring the CLI so a + // rollout spanning both surfaces reads as one flag in PostHog. Resolved + // after the shouldTrack guard, so opted-out users never pay for it. + properties: { ...properties, ...sys, ...canaryEventProperties() }, timestamp: new Date().toISOString(), });