From 4f464dc424c648c1210fbfd0675ff774ee0037d4 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Thu, 30 Jul 2026 16:11:34 -0700 Subject: [PATCH] feat(cli,studio): telemetry opt-out is canary opt-out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both reviewers flagged the same gap: seed injection was gated on telemetryShouldTrack(), but canary EVALUATION was not. An install with DO_NOT_TRACK=1 was still bucketed and still had real code paths flipped (e.g. HF_DE_PARALLEL_ROUTER), silently and unmeasurably. A canary is a measured rollout — we enrol a slice precisely so it can be compared against everyone else. An install that sends nothing can't be compared, so enrolling it buys no signal and only changes that user's code path, on an experimental feature, without their knowledge. That is the wrong side of an opt-out. Resolves to a new `telemetry_opt_out` reason BEFORE bucketing, so no cohort is assigned at all. Distinct from `excluded` because "why is my canary off" has a very different answer for CI than for opted-out, and the reason never reaches telemetry by construction. Covers every opt-out route: persisted preference, the runtime env vars and dev/telemetry-disabled builds via policy.ts, and Studio's hyperframes-studio:telemetryDisabled. An explicit HF_CANARY_* / ?hf_canary_*= override still wins — a deliberate local choice, not silent enrolment, and the documented way to exercise a canary with telemetry off. The CLI check mirrors shouldTrack() rather than importing it: client.ts already imports canary.ts for canaryEventProperties, so depending on it would be a cycle. Both read the same two inputs, so they cannot disagree. Tests: 9 new across CLI and Studio (preference off, each runtime override, no bucket assigned, override still honoured, flag properties all-false). Fault injection: removing the CLI gate fails 6, removing the Studio gate fails 3. Co-Authored-By: Claude Opus 5 (1M context) --- docs/contributing/canary-rollouts.mdx | 14 ++++ packages/cli/src/telemetry/canary.test.ts | 61 +++++++++++++++- packages/cli/src/telemetry/canary.ts | 76 ++++++++++++++------ packages/core/src/canary.ts | 8 ++- packages/studio/src/telemetry/canary.test.ts | 33 +++++++++ packages/studio/src/telemetry/canary.ts | 27 ++++--- 6 files changed, 186 insertions(+), 33 deletions(-) diff --git a/docs/contributing/canary-rollouts.mdx b/docs/contributing/canary-rollouts.mdx index 0124f89a1..ea6f15386 100644 --- a/docs/contributing/canary-rollouts.mdx +++ b/docs/contributing/canary-rollouts.mdx @@ -220,6 +220,20 @@ rm -rf ~/.hyperframes # clears telemetry id, canary cohorts, and breaker stat `hyperframes telemetry status` prints both paths if you want to inspect or delete them individually. Nothing canary-related is stored anywhere else. +**Opting out of telemetry opts you out of canaries.** A canary is a *measured* +rollout — we enrol a slice precisely so it can be compared against everyone +else. An install that sends nothing can't be compared, so enrolling it buys no +signal and only changes that user's code path, on an experimental feature, +without their knowledge. Every opt-out route counts: the persisted preference +(`hyperframes telemetry disable`), the runtime env vars +(`HYPERFRAMES_NO_TELEMETRY`, `DO_NOT_TRACK`), dev/telemetry-disabled builds, +and Studio's `hyperframes-studio:telemetryDisabled`. The decision resolves to +`telemetry_opt_out` *before* bucketing, so no cohort is assigned at all. + +An explicit `HF_CANARY_` (or `?hf_canary_=` in Studio) +override still wins — that is a deliberate local choice, and it stays the way +to exercise a canary with telemetry off. + **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 diff --git a/packages/cli/src/telemetry/canary.test.ts b/packages/cli/src/telemetry/canary.test.ts index f1d8b8b10..80582db91 100644 --- a/packages/cli/src/telemetry/canary.test.ts +++ b/packages/cli/src/telemetry/canary.test.ts @@ -1,17 +1,32 @@ import { describe, expect, it, vi, beforeEach } from "vitest"; -const configState: { anonymousId: string; bucketSeed: string | undefined } = { +const configState: { + anonymousId: string; + bucketSeed: string | undefined; + telemetryEnabled: boolean; +} = { anonymousId: "db0c1f4a-b95e-4c35-90c6-1a15bd76f717", bucketSeed: undefined, + telemetryEnabled: true, }; const systemState = { is_ci: false }; +// null = no runtime opt-out in force; a string names the source (env var, +// dev build, ...) exactly as policy.ts reports it. +const policyState: { runtimeOverride: string | null } = { runtimeOverride: null }; vi.mock("./config.js", () => ({ - readConfig: () => ({ anonymousId: configState.anonymousId, bucketSeed: configState.bucketSeed }), + readConfig: () => ({ + anonymousId: configState.anonymousId, + bucketSeed: configState.bucketSeed, + telemetryEnabled: configState.telemetryEnabled, + }), })); vi.mock("./system.js", () => ({ getSystemMeta: () => ({ is_ci: systemState.is_ci }), })); +vi.mock("./policy.js", () => ({ + telemetryRuntimeOverride: () => policyState.runtimeOverride, +})); // The registry is data; pin a known shape so these tests don't move when a // real canary is added or ramped. @@ -64,11 +79,53 @@ beforeEach(() => { __resetCanaryCacheForTests(); configState.anonymousId = "db0c1f4a-b95e-4c35-90c6-1a15bd76f717"; configState.bucketSeed = undefined; + configState.telemetryEnabled = true; systemState.is_ci = false; + policyState.runtimeOverride = null; delete process.env.HF_CANARY_TEST_ALPHA; delete process.env.HF_CANARY_TEST_BETA; }); +describe("telemetry opt-out is canary opt-out", () => { + it("does not enrol when the persisted preference is off", () => { + configState.telemetryEnabled = false; + // test-alpha is at 100% — it would be on for everyone otherwise. + expect(resolveCanary("test-alpha")).toEqual({ + enabled: false, + reason: "telemetry_opt_out", + }); + }); + + it.each(["HYPERFRAMES_NO_TELEMETRY", "DO_NOT_TRACK", "dev_mode"])( + "does not enrol under the %s runtime override, even with the preference on", + (source) => { + configState.telemetryEnabled = true; + policyState.runtimeOverride = source; + expect(resolveCanary("test-alpha").reason).toBe("telemetry_opt_out"); + }, + ); + + it("never buckets an opted-out install — no cohort is assigned at all", () => { + configState.telemetryEnabled = false; + // A bucket number would mean we hashed them into a slice anyway. + expect(resolveCanary("test-alpha").bucket).toBeUndefined(); + }); + + it("still honours an explicit override — the documented way to test with telemetry off", () => { + configState.telemetryEnabled = false; + process.env.HF_CANARY_TEST_BETA = "on"; + expect(resolveCanary("test-beta")).toEqual({ enabled: true, reason: "forced_on" }); + }); + + it("reports every canary as false to PostHog shape when opted out", () => { + configState.telemetryEnabled = false; + expect(canaryEventProperties()).toEqual({ + "$feature/canary-test-alpha": "false", + "$feature/canary-test-beta": "false", + }); + }); +}); + describe("bucketing unit", () => { it("buckets on the bucketSeed when present — the unit that survives config wipes", async () => { const { evaluateCanary } = await import("@hyperframes/core/canary"); diff --git a/packages/cli/src/telemetry/canary.ts b/packages/cli/src/telemetry/canary.ts index f1d0d95c5..d76708d80 100644 --- a/packages/cli/src/telemetry/canary.ts +++ b/packages/cli/src/telemetry/canary.ts @@ -28,8 +28,28 @@ import { } from "@hyperframes/core/canary"; import { CANARIES, canaryEnvVar, findCanary } from "@hyperframes/core/canary-registry"; import { readConfig } from "./config.js"; +import { telemetryRuntimeOverride } from "./policy.js"; import { getSystemMeta } from "./system.js"; +/** + * Opting out of telemetry opts you out of canaries. + * + * A canary is a measured rollout: we enrol a slice precisely so we can compare + * it against everyone else. An install that sends nothing can't be compared, + * so enrolling it buys no signal — it only changes that user's code path, on + * an experimental feature, without their knowledge and with no way for us to + * see the result. That is the wrong side of an opt-out. + * + * Deliberately mirrors `shouldTrack()` rather than importing it: client.ts + * already imports this module for `canaryEventProperties`, so depending on it + * here would be a cycle. Both read the same two inputs (`policy.ts`'s runtime + * override, then the persisted preference), so they cannot disagree. + */ +function telemetryActive(): boolean { + if (telemetryRuntimeOverride() !== null) return false; + return readConfig().telemetryEnabled; +} + /** * Decisions are memoized per process: a `--batch` run asks the same question * once per row, and a canary must not change its mind mid-process — a render @@ -43,6 +63,40 @@ export function __resetCanaryCacheForTests(): void { decisions.clear(); } +/** The uncached decision. Split out so `resolveCanary` is purely the memo. */ +function decideCanary(name: string): CanaryDecision { + const definition = findCanary(name); + if (!definition) return { enabled: false, reason: "out_of_cohort" }; + + const override = parseCanaryOverride(process.env[canaryEnvVar(definition.name)]); + + // Resolved ahead of evaluate so an opted-out install is never bucketed at + // all. An explicit HF_CANARY_* override still wins: that is a deliberate + // local choice (support session, bisect, developer testing), not silent + // enrolment, and it stays the way to exercise a canary with telemetry off. + if (override === undefined && !telemetryActive()) { + return { enabled: false, reason: "telemetry_opt_out" }; + } + + const config = readConfig(); + return evaluateCanary({ + feature: definition.name, + // The bucket seed, NOT the anonymousId: the seed is inherited across + // config.json re-mints via the install-state file, so cohorts hold when + // the telemetry id re-rolls. Both files live in ~/.hyperframes, so + // deleting that directory clears the cohort too — intentional. Fallback + // covers only a failed backfill write on a legacy config. + unitId: config.bucketSeed ?? config.anonymousId, + percentage: definition.percentage, + override, + // CI installs regenerate their config per run, so their ids are + // ephemeral — they would hop cohorts between runs, adding noise to the + // rollout signal while saying nothing about real users. An explicit + // override still gets through, which is how you test a canary in CI. + exclude: getSystemMeta().is_ci, + }); +} + /** * Full decision for a registered canary, including the reason — use this when * you want to record WHY, not just whether. @@ -54,27 +108,7 @@ export function resolveCanary(name: string): CanaryDecision { const cached = decisions.get(name); if (cached) return cached; - const definition = findCanary(name); - const config = readConfig(); - const decision: CanaryDecision = definition - ? evaluateCanary({ - feature: definition.name, - // The bucket seed, NOT the anonymousId: the seed is inherited across - // config.json re-mints via the install-state file, so cohorts hold - // when the telemetry id re-rolls. Both files live in ~/.hyperframes, - // so deleting that directory clears the cohort too — intentional. - // Fallback covers only a failed backfill write on a legacy config. - unitId: config.bucketSeed ?? config.anonymousId, - percentage: definition.percentage, - override: parseCanaryOverride(process.env[canaryEnvVar(definition.name)]), - // CI installs regenerate their config per run, so their ids are - // ephemeral — they would hop cohorts between runs, adding noise to the - // rollout signal while saying nothing about real users. An explicit - // override still gets through, which is how you test a canary in CI. - exclude: getSystemMeta().is_ci, - }) - : { enabled: false, reason: "out_of_cohort" }; - + const decision = decideCanary(name); decisions.set(name, decision); return decision; } diff --git a/packages/core/src/canary.ts b/packages/core/src/canary.ts index e96642338..10dd4fa12 100644 --- a/packages/core/src/canary.ts +++ b/packages/core/src/canary.ts @@ -39,7 +39,13 @@ export type CanaryReason = | "in_cohort" | "out_of_cohort" | "no_unit_id" - | "excluded"; + | "excluded" + // Telemetry is off, so the install is not enrolled. Distinct from + // "excluded" because the caller decides this BEFORE evaluate is reached — + // and because "why is my canary off" has a very different answer in the two + // cases. Never appears in telemetry by construction: an install that + // resolves this way sends nothing. + | "telemetry_opt_out"; export interface CanaryDecision { enabled: boolean; diff --git a/packages/studio/src/telemetry/canary.test.ts b/packages/studio/src/telemetry/canary.test.ts index bd51b998b..ff89f6a97 100644 --- a/packages/studio/src/telemetry/canary.test.ts +++ b/packages/studio/src/telemetry/canary.test.ts @@ -192,3 +192,36 @@ describe("telemetry", () => { expect(canaryEventProperties()["$feature/canary-on-everywhere"]).toBe("false"); }); }); + +describe("telemetry opt-out is canary opt-out", () => { + // The studio opt-out lever, per telemetry/config.ts. + const OPT_OUT_KEY = "hyperframes-studio:telemetryDisabled"; + + it("does not enrol an opted-out browser profile", () => { + localStorage.setItem(OPT_OUT_KEY, "1"); + // on-everywhere is at 100% — it would be on for everyone otherwise. + expect(resolveCanary("on-everywhere")).toEqual({ + enabled: false, + reason: "telemetry_opt_out", + }); + }); + + it("never buckets an opted-out profile — no cohort is assigned at all", () => { + localStorage.setItem(OPT_OUT_KEY, "1"); + expect(resolveCanary("on-everywhere").bucket).toBeUndefined(); + }); + + it("still honours an explicit URL override", () => { + localStorage.setItem(OPT_OUT_KEY, "1"); + setSearch("?hf_canary_off_everywhere=on"); + expect(resolveCanary("off-everywhere")).toEqual({ enabled: true, reason: "forced_on" }); + }); + + it("reports every canary as false when opted out", () => { + localStorage.setItem(OPT_OUT_KEY, "1"); + expect(canaryEventProperties()).toEqual({ + "$feature/canary-on-everywhere": "false", + "$feature/canary-off-everywhere": "false", + }); + }); +}); diff --git a/packages/studio/src/telemetry/canary.ts b/packages/studio/src/telemetry/canary.ts index af95db2b8..943ed720f 100644 --- a/packages/studio/src/telemetry/canary.ts +++ b/packages/studio/src/telemetry/canary.ts @@ -41,6 +41,7 @@ import { } from "@hyperframes/core/canary"; import { CANARIES, findCanary } from "@hyperframes/core/canary-registry"; import { resolveStudioDistinctId } from "./distinctId"; +import { isOptedOut } from "./config"; import { safeSessionStorage } from "../utils/safeStorage"; /** `my-feature` → `hf_canary_my_feature`, the query param and storage key. */ @@ -151,15 +152,23 @@ export function resolveCanary(name: string): CanaryDecision { if (cached) return cached; const definition = findCanary(name); - const decision: CanaryDecision = definition - ? evaluateCanary({ - feature: definition.name, - unitId: resolveBucketUnit(), - percentage: definition.percentage, - override: readOverride(definition.name), - exclude: isAutomatedBrowser(), - }) - : { enabled: false, reason: "out_of_cohort" }; + const override = definition ? readOverride(definition.name) : undefined; + // Opting out of telemetry opts you out of canaries — same rule as the CLI + // (see packages/cli/src/telemetry/canary.ts). An install that sends nothing + // can't be compared against anyone, so enrolling it changes that user's + // code path for no signal. An explicit override still wins: that is a + // deliberate local choice, not silent enrolment. + const decision: CanaryDecision = !definition + ? { enabled: false, reason: "out_of_cohort" } + : override === undefined && isOptedOut() + ? { enabled: false, reason: "telemetry_opt_out" } + : evaluateCanary({ + feature: definition.name, + unitId: resolveBucketUnit(), + percentage: definition.percentage, + override, + exclude: isAutomatedBrowser(), + }); decisions.set(name, decision); return decision;