From 203bf56dfd0a10cc976c85ca3cc4a6d33982d2de Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 28 Oct 2026 13:58:17 -0700 Subject: [PATCH] feat(wsl-mover): report how a move's undo ends (#35322) * feat(wsl-mover): report how a move's undo ends (wsl_move_undo) * fix(wsl-mover): tell a thrown undo apart from a failed one; clamp its duration --- src/main/ipc/telemetry.ts | 1 + src/main/ipc/wsl-move-handlers.test.ts | 10 ++ src/main/ipc/wsl-move-handlers.ts | 12 +- src/main/telemetry/wsl-move-telemetry.ts | 2 +- .../telemetry/wsl-move-undo-telemetry.test.ts | 110 ++++++++++++++++++ src/main/telemetry/wsl-move-undo-telemetry.ts | 57 +++++++++ src/shared/telemetry-event-registry.ts | 7 +- .../telemetry-wsl-move-event-schemas.ts | 23 ++++ 8 files changed, 215 insertions(+), 7 deletions(-) create mode 100644 src/main/telemetry/wsl-move-undo-telemetry.test.ts create mode 100644 src/main/telemetry/wsl-move-undo-telemetry.ts diff --git a/src/main/ipc/telemetry.ts b/src/main/ipc/telemetry.ts index 27ad906e32e..cacb8bb38f3 100644 --- a/src/main/ipc/telemetry.ts +++ b/src/main/ipc/telemetry.ts @@ -37,6 +37,7 @@ const MAIN_OWNED_TELEMETRY_EVENTS = new Set([ 'ssh_host_server_legacy_relay_bridge', 'wsl_move', 'wsl_move_orchestrator', + 'wsl_move_undo', 'wsl_usage_snapshot' ]) diff --git a/src/main/ipc/wsl-move-handlers.test.ts b/src/main/ipc/wsl-move-handlers.test.ts index 7bf7ecd9efd..9a6e8dbc22f 100644 --- a/src/main/ipc/wsl-move-handlers.test.ts +++ b/src/main/ipc/wsl-move-handlers.test.ts @@ -30,6 +30,9 @@ vi.mock('electron', () => ({ } })) +const track = vi.hoisted(() => vi.fn()) +vi.mock('../telemetry/client', () => ({ track })) + import { registerWslMoveHandlers } from './wsl-move-handlers' let userDataPath: string @@ -39,6 +42,7 @@ beforeEach(() => { registerWslMoveDesktopRewriteStore({ id: 'renderer-a', side: 'renderer' }) seedWslMoveCutover(userDataPath, cutover('move-1', 'destination-committed')) flushStorageData.mockReset() + track.mockReset() registerWslMoveHandlers( () => userDataPath, (senderId) => senderId === MAIN_WINDOW_ID @@ -242,6 +246,8 @@ describe('WSL move IPC', () => { reason: 'gated-off' }) expect(listWslMoveCutovers(userDataPath)).toHaveLength(1) + // Requests the gate or the window check turned away never reach undo, so they are not counted. + expect(track).not.toHaveBeenCalled() }) // The host's copy of the agent kept running beside the one resumed here. @@ -265,6 +271,10 @@ describe('WSL move IPC', () => { expect(stopHostCopies).toHaveBeenCalledOnce() expect(listWslMoveCutovers(userDataPath)).toHaveLength(1) expect(isWslMoveWindowWriteHeld()).toBe(false) + expect(track).toHaveBeenCalledWith('wsl_move_undo', { + outcome: reason.replaceAll('-', '_'), + duration_bucket: expect.any(String) + }) } ) }) diff --git a/src/main/ipc/wsl-move-handlers.ts b/src/main/ipc/wsl-move-handlers.ts index e5b6af68098..222be3b0aa0 100644 --- a/src/main/ipc/wsl-move-handlers.ts +++ b/src/main/ipc/wsl-move-handlers.ts @@ -28,6 +28,7 @@ import { import type { WslMoveDriverDeps } from '../persistence/moving-wsl-projects/wsl-move-driver-ports' import { listUnfinishedWslMoves } from '../persistence/moving-wsl-projects/wsl-move-host-automations-rewrite' import { runTrackedWslMove } from '../telemetry/wsl-move-telemetry' +import { runTrackedWslMoveUndo } from '../telemetry/wsl-move-undo-telemetry' import { isAnyWslMoveRunning } from '../persistence/moving-wsl-projects/wsl-move-running-moves' import { wslHostsGateOpen, wslHostsServing } from '../wsl/wsl-host-gate' import type { WslMoveUndoPorts } from './wsl-move-undo-wiring' @@ -171,13 +172,14 @@ function registerWslMoveUseHereHandler( } const ref = request.kind === 'repo' ? { repoId: request.id } : { folderWorkspaceId: request.id } - return releaseWslMovedProjectHere( - getUserDataPath(), - ref, + const unsavedWorkspaceIds = 'unsavedWorkspaceIds' in request && Array.isArray(request.unsavedWorkspaceIds) ? request.unsavedWorkspaceIds.filter((id): id is string => typeof id === 'string') - : [], - createUndoPorts + : [] + return runTrackedWslMoveUndo( + () => performance.now(), + () => + releaseWslMovedProjectHere(getUserDataPath(), ref, unsavedWorkspaceIds, createUndoPorts) ) } ) diff --git a/src/main/telemetry/wsl-move-telemetry.ts b/src/main/telemetry/wsl-move-telemetry.ts index b8827b0d5e6..3691c2331f4 100644 --- a/src/main/telemetry/wsl-move-telemetry.ts +++ b/src/main/telemetry/wsl-move-telemetry.ts @@ -25,7 +25,7 @@ function pick(values: readonly T[], value: string, fallback: T return values.find((candidate) => candidate === value) ?? fallback } -function durationBucket(durationMs: number): Props['duration_bucket'] { +export function durationBucket(durationMs: number): Props['duration_bucket'] { if (durationMs < 5_000) { return 'lt_5s' } diff --git a/src/main/telemetry/wsl-move-undo-telemetry.test.ts b/src/main/telemetry/wsl-move-undo-telemetry.test.ts new file mode 100644 index 00000000000..23427b234a7 --- /dev/null +++ b/src/main/telemetry/wsl-move-undo-telemetry.test.ts @@ -0,0 +1,110 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { eventSchemas } from '../../shared/telemetry-events' +import type { WslMoveUseLocallyResult } from '../../shared/wsl-move/wsl-moved-project' +import { runTrackedWslMoveUndo, wslMoveUndoOutcome } from './wsl-move-undo-telemetry' + +const track = vi.hoisted(() => vi.fn()) +vi.mock('./client', () => ({ track })) + +beforeEach(() => { + track.mockReset() +}) + +const released: WslMoveUseLocallyResult = { + released: true, + distro: 'Ubuntu', + repoIds: ['repo-1'], + folderWorkspaceIds: [] +} + +async function tracked(result: WslMoveUseLocallyResult, durationMs = 1_000): Promise { + let ms = 0 + const returned = await runTrackedWslMoveUndo( + () => ms, + async () => { + ms += durationMs + return result + } + ) + expect(returned).toBe(result) + expect(track).toHaveBeenCalledTimes(1) + const [name, props] = track.mock.calls[0] ?? [] + expect(name).toBe('wsl_move_undo') + expect(eventSchemas.wsl_move_undo.safeParse(props).success).toBe(true) + return props +} + +describe('wsl_move_undo telemetry', () => { + it('reports a released project with its duration bucket and nothing it named', async () => { + const props = await tracked(released, 20_000) + expect(props).toEqual({ outcome: 'released', duration_bucket: '15s_60s' }) + expect(JSON.stringify(props)).not.toMatch(/Ubuntu|repo-1/u) + }) + + it.each([ + ['not-moved', 'not_moved'], + ['superseded', 'superseded'], + ['move-running', 'move_running'], + ['host-still-running', 'host_still_running'], + ['host-unreachable', 'host_unreachable'], + ['unsaved-files', 'unsaved_files'], + ['failed', 'failed'] + ] as const)('reports a %s refusal as %s', async (reason, outcome) => { + const props = await tracked({ released: false, reason, detail: 'C:\\Users\\someone' }) + expect(props).toEqual({ outcome, duration_bucket: 'lt_5s' }) + }) + + it('reports an undo that threw as threw, apart from a failed refusal, and rethrows it', async () => { + const error = new Error('boom') + await expect( + runTrackedWslMoveUndo( + () => 0, + async () => { + throw error + } + ) + ).rejects.toBe(error) + expect(track).toHaveBeenCalledWith('wsl_move_undo', { + outcome: 'threw', + duration_bucket: 'lt_5s' + }) + }) + + it('buckets a clock that stepped back as the shortest duration', async () => { + let ms = 10_000 + await expect( + runTrackedWslMoveUndo( + () => ms, + async () => { + ms -= 5_000 + throw new Error('boom') + } + ) + ).rejects.toThrow('boom') + expect(track).toHaveBeenCalledWith('wsl_move_undo', { + outcome: 'threw', + duration_bucket: 'lt_5s' + }) + }) + + it('never lets telemetry cost the undo', async () => { + track.mockImplementation(() => { + throw new Error('telemetry down') + }) + await expect( + runTrackedWslMoveUndo( + () => 0, + async () => released + ) + ).resolves.toBe(released) + }) + + it('maps an unknown reason to other', () => { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: simulates a reason a newer main could return. + const unknown = { + released: false, + reason: 'something-new' + } as unknown as WslMoveUseLocallyResult + expect(wslMoveUndoOutcome(unknown)).toBe('other') + }) +}) diff --git a/src/main/telemetry/wsl-move-undo-telemetry.ts b/src/main/telemetry/wsl-move-undo-telemetry.ts new file mode 100644 index 00000000000..d8039f8f14f --- /dev/null +++ b/src/main/telemetry/wsl-move-undo-telemetry.ts @@ -0,0 +1,57 @@ +/** + * Undo telemetry for a WSL move ("use this project here again"): one enum-only row per request + * that got past the gate, with how it ended and a duration bucket. Nothing a user or host named leaves. + */ +import type { EventProps } from '../../shared/telemetry-events' +import { WSL_MOVE_UNDO_OUTCOME_VALUES } from '../../shared/telemetry-wsl-move-event-schemas' +import type { WslMoveUseLocallyResult } from '../../shared/wsl-move/wsl-moved-project' +import { track } from './client' +import { durationBucket } from './wsl-move-telemetry' + +type Props = EventProps<'wsl_move_undo'> + +// Why: a new refusal reason must get its own telemetry value, not fall into `other` unnoticed. +type Reason = Exclude['reason'], 'gated-off'> +type Snake = T extends `${infer Head}-${infer Tail}` + ? `${Head}_${Snake}` + : T +type Unlisted = Exclude, (typeof WSL_MOVE_UNDO_OUTCOME_VALUES)[number]> +const everyReasonListed: [Unlisted] extends [never] ? true : never = true +void everyReasonListed + +export function wslMoveUndoOutcome(result: WslMoveUseLocallyResult): Props['outcome'] { + if (result.released) { + return 'released' + } + const value = result.reason.replaceAll('-', '_') + return WSL_MOVE_UNDO_OUTCOME_VALUES.find((candidate) => candidate === value) ?? 'other' +} + +/** Runs one undo with its outcome reported; the undo's own result is returned as is. */ +export async function runTrackedWslMoveUndo( + now: () => number, + run: () => Promise +): Promise { + const startedAt = now() + let result: WslMoveUseLocallyResult + try { + result = await run() + } catch (error) { + safeTrack({ outcome: 'threw', duration_bucket: durationBucket(Math.max(0, now() - startedAt)) }) + throw error + } + safeTrack({ + outcome: wslMoveUndoOutcome(result), + duration_bucket: durationBucket(Math.max(0, now() - startedAt)) + }) + return result +} + +function safeTrack(props: Props): void { + try { + // track() applies the consent gate and validates against the strict schema. + track('wsl_move_undo', props) + } catch { + // Telemetry never costs an undo. + } +} diff --git a/src/shared/telemetry-event-registry.ts b/src/shared/telemetry-event-registry.ts index 3edc0b3109c..7d45ea6f94a 100644 --- a/src/shared/telemetry-event-registry.ts +++ b/src/shared/telemetry-event-registry.ts @@ -112,7 +112,11 @@ import { sshHostServerMoveSchema, sshHostServerLegacyRelayBridgeSchema } from './telemetry-ssh-host-server-event-schemas' -import { wslMoveOrchestratorSchema, wslMoveSchema } from './telemetry-wsl-move-event-schemas' +import { + wslMoveOrchestratorSchema, + wslMoveUndoSchema, + wslMoveSchema +} from './telemetry-wsl-move-event-schemas' // Breaking payload changes require a new event name to keep historical rows distinguishable; optional additions are safe. export const eventSchemas = { @@ -228,6 +232,7 @@ export const eventSchemas = { wsl_move: wslMoveSchema, wsl_move_orchestrator: wslMoveOrchestratorSchema, + wsl_move_undo: wslMoveUndoSchema, wsl_usage_snapshot: wslUsageSnapshotSchema } as const diff --git a/src/shared/telemetry-wsl-move-event-schemas.ts b/src/shared/telemetry-wsl-move-event-schemas.ts index 5294cc9dbff..b60bff59508 100644 --- a/src/shared/telemetry-wsl-move-event-schemas.ts +++ b/src/shared/telemetry-wsl-move-event-schemas.ts @@ -117,3 +117,26 @@ export const wslMoveOrchestratorSchema = z attempt_bucket: z.enum(WSL_MOVE_ORCHESTRATOR_ATTEMPT_BUCKETS) }) .strict() + +/** How "use this project here again" ended; refusals keep the result's own reason. */ +export const WSL_MOVE_UNDO_OUTCOME_VALUES = [ + 'released', + 'not_moved', + 'superseded', + 'move_running', + 'host_still_running', + 'host_unreachable', + 'unsaved_files', + 'failed', + // The undo threw instead of returning a refusal. + 'threw', + 'other' +] as const + +// Same enum-only rule as `wsl_move`: no distro, project or migration id leaves. +export const wslMoveUndoSchema = z + .object({ + outcome: z.enum(WSL_MOVE_UNDO_OUTCOME_VALUES), + duration_bucket: z.enum(SSH_RUNTIME_DURATION_BUCKETS) + }) + .strict()