diff --git a/packages/telemetry/src/TelemetryService.ts b/packages/telemetry/src/TelemetryService.ts index 3dbab273148..5ea4cef936f 100644 --- a/packages/telemetry/src/TelemetryService.ts +++ b/packages/telemetry/src/TelemetryService.ts @@ -1,6 +1,11 @@ import { ZodError } from "zod" -import { type TelemetryClient, type TelemetryPropertiesProvider, TelemetryEventName } from "@roo-code/types" +import { + type TelemetryClient, + type TelemetryPropertiesProvider, + TelemetryEventName, + type TelemetrySetting, +} from "@roo-code/types" /** * TelemetryService wrapper class that defers initialization. @@ -226,6 +231,18 @@ export class TelemetryService { this.captureEvent(TelemetryEventName.TITLE_BUTTON_CLICKED, { button }) } + /** + * Captures when telemetry settings are changed + * @param previousSetting The previous telemetry setting + * @param newSetting The new telemetry setting + */ + public captureTelemetrySettingsChanged(previousSetting: TelemetrySetting, newSetting: TelemetrySetting): void { + this.captureEvent(TelemetryEventName.TELEMETRY_SETTINGS_CHANGED, { + previousSetting, + newSetting, + }) + } + /** * Checks if telemetry is currently enabled * @returns Whether telemetry is enabled diff --git a/packages/types/src/telemetry.ts b/packages/types/src/telemetry.ts index abaceb9fbb0..f8b3e207380 100644 --- a/packages/types/src/telemetry.ts +++ b/packages/types/src/telemetry.ts @@ -71,6 +71,7 @@ export enum TelemetryEventName { SHELL_INTEGRATION_ERROR = "Shell Integration Error", CONSECUTIVE_MISTAKE_ERROR = "Consecutive Mistake Error", CODE_INDEX_ERROR = "Code Index Error", + TELEMETRY_SETTINGS_CHANGED = "Telemetry Settings Changed", } /** @@ -202,6 +203,14 @@ export const rooCodeTelemetryEventSchema = z.discriminatedUnion("type", [ ]), properties: telemetryPropertiesSchema, }), + z.object({ + type: z.literal(TelemetryEventName.TELEMETRY_SETTINGS_CHANGED), + properties: z.object({ + ...telemetryPropertiesSchema.shape, + previousSetting: telemetrySettingsSchema, + newSetting: telemetrySettingsSchema, + }), + }), z.object({ type: z.literal(TelemetryEventName.TASK_MESSAGE), properties: z.object({ diff --git a/src/core/webview/__tests__/telemetrySettingsTracking.spec.ts b/src/core/webview/__tests__/telemetrySettingsTracking.spec.ts new file mode 100644 index 00000000000..a99c8862a34 --- /dev/null +++ b/src/core/webview/__tests__/telemetrySettingsTracking.spec.ts @@ -0,0 +1,155 @@ +import { describe, it, expect, vi, beforeEach } from "vitest" +import { TelemetryService } from "@roo-code/telemetry" +import { TelemetryEventName, type TelemetrySetting } from "@roo-code/types" + +describe("Telemetry Settings Tracking", () => { + let mockTelemetryService: { + captureTelemetrySettingsChanged: ReturnType + updateTelemetryState: ReturnType + hasInstance: ReturnType + } + + beforeEach(() => { + // Reset mocks + vi.clearAllMocks() + + // Create mock service + mockTelemetryService = { + captureTelemetrySettingsChanged: vi.fn(), + updateTelemetryState: vi.fn(), + hasInstance: vi.fn().mockReturnValue(true), + } + + // Mock the TelemetryService + vi.spyOn(TelemetryService, "hasInstance").mockReturnValue(true) + vi.spyOn(TelemetryService, "instance", "get").mockReturnValue(mockTelemetryService as any) + }) + + describe("when telemetry is turned OFF", () => { + it("should fire event BEFORE disabling telemetry", () => { + const previousSetting = "enabled" as TelemetrySetting + const newSetting = "disabled" as TelemetrySetting + + // Simulate the logic from webviewMessageHandler + const isOptedIn = newSetting !== "disabled" + const wasPreviouslyOptedIn = previousSetting !== "disabled" + + // If turning telemetry OFF, fire event BEFORE disabling + if (wasPreviouslyOptedIn && !isOptedIn && TelemetryService.hasInstance()) { + TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, newSetting) + } + + // Update the telemetry state + TelemetryService.instance.updateTelemetryState(isOptedIn) + + // Verify the event was captured before updateTelemetryState + expect(mockTelemetryService.captureTelemetrySettingsChanged).toHaveBeenCalledWith("enabled", "disabled") + expect(mockTelemetryService.captureTelemetrySettingsChanged).toHaveBeenCalledBefore( + mockTelemetryService.updateTelemetryState as any, + ) + expect(mockTelemetryService.updateTelemetryState).toHaveBeenCalledWith(false) + }) + + it("should fire event when going from unset to disabled", () => { + const previousSetting = "unset" as TelemetrySetting + const newSetting = "disabled" as TelemetrySetting + + const isOptedIn = newSetting !== "disabled" + const wasPreviouslyOptedIn = previousSetting !== "disabled" + + if (wasPreviouslyOptedIn && !isOptedIn && TelemetryService.hasInstance()) { + TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, newSetting) + } + + TelemetryService.instance.updateTelemetryState(isOptedIn) + + expect(mockTelemetryService.captureTelemetrySettingsChanged).toHaveBeenCalledWith("unset", "disabled") + }) + }) + + describe("when telemetry is turned ON", () => { + it("should fire event AFTER enabling telemetry", () => { + const previousSetting = "disabled" as TelemetrySetting + const newSetting = "enabled" as TelemetrySetting + + const isOptedIn = newSetting !== "disabled" + const wasPreviouslyOptedIn = previousSetting !== "disabled" + + // Update the telemetry state first + TelemetryService.instance.updateTelemetryState(isOptedIn) + + // If turning telemetry ON, fire event AFTER enabling + if (!wasPreviouslyOptedIn && isOptedIn && TelemetryService.hasInstance()) { + TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, newSetting) + } + + // Verify the event was captured after updateTelemetryState + expect(mockTelemetryService.updateTelemetryState).toHaveBeenCalledWith(true) + expect(mockTelemetryService.captureTelemetrySettingsChanged).toHaveBeenCalledWith("disabled", "enabled") + expect(mockTelemetryService.updateTelemetryState).toHaveBeenCalledBefore( + mockTelemetryService.captureTelemetrySettingsChanged as any, + ) + }) + + it("should not fire event when going from enabled to enabled", () => { + const previousSetting = "enabled" as TelemetrySetting + const newSetting = "enabled" as TelemetrySetting + + const isOptedIn = newSetting !== "disabled" + const wasPreviouslyOptedIn = previousSetting !== "disabled" + + // Neither condition should be met + if (wasPreviouslyOptedIn && !isOptedIn && TelemetryService.hasInstance()) { + TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, newSetting) + } + + TelemetryService.instance.updateTelemetryState(isOptedIn) + + if (!wasPreviouslyOptedIn && isOptedIn && TelemetryService.hasInstance()) { + TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, newSetting) + } + + // Should not fire any telemetry events + expect(mockTelemetryService.captureTelemetrySettingsChanged).not.toHaveBeenCalled() + expect(mockTelemetryService.updateTelemetryState).toHaveBeenCalledWith(true) + }) + + it("should fire event when going from unset to enabled (telemetry banner close)", () => { + const previousSetting = "unset" as TelemetrySetting + const newSetting = "enabled" as TelemetrySetting + + const isOptedIn = newSetting !== "disabled" + const wasPreviouslyOptedIn = previousSetting !== "disabled" + + // For unset -> enabled, both are opted in, so no event should fire + if (wasPreviouslyOptedIn && !isOptedIn && TelemetryService.hasInstance()) { + TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, newSetting) + } + + TelemetryService.instance.updateTelemetryState(isOptedIn) + + if (!wasPreviouslyOptedIn && isOptedIn && TelemetryService.hasInstance()) { + TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, newSetting) + } + + // unset is treated as opted-in, so no event should fire + expect(mockTelemetryService.captureTelemetrySettingsChanged).not.toHaveBeenCalled() + }) + }) + + describe("TelemetryService.captureTelemetrySettingsChanged", () => { + it("should call captureEvent with correct parameters", () => { + // Create a real instance to test the method + const mockCaptureEvent = vi.fn() + const service = new (TelemetryService as any)([]) + service.captureEvent = mockCaptureEvent + + service.captureTelemetrySettingsChanged("enabled", "disabled") + + expect(mockCaptureEvent).toHaveBeenCalledWith(TelemetryEventName.TELEMETRY_SETTINGS_CHANGED, { + previousSetting: "enabled", + newSetting: "disabled", + }) + }) + }) +}) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index c3e57c67a20..af5f9925c35 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -2296,9 +2296,25 @@ export const webviewMessageHandler = async ( case "telemetrySetting": { const telemetrySetting = message.text as TelemetrySetting - await updateGlobalState("telemetrySetting", telemetrySetting) + const previousSetting = getGlobalState("telemetrySetting") || "unset" const isOptedIn = telemetrySetting !== "disabled" - TelemetryService.instance.updateTelemetryState(isOptedIn) + const wasPreviouslyOptedIn = previousSetting !== "disabled" + + // If turning telemetry OFF, fire event BEFORE disabling + if (wasPreviouslyOptedIn && !isOptedIn && TelemetryService.hasInstance()) { + TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, telemetrySetting) + } + // Update the telemetry state + await updateGlobalState("telemetrySetting", telemetrySetting) + if (TelemetryService.hasInstance()) { + TelemetryService.instance.updateTelemetryState(isOptedIn) + } + + // If turning telemetry ON, fire event AFTER enabling + if (!wasPreviouslyOptedIn && isOptedIn && TelemetryService.hasInstance()) { + TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, telemetrySetting) + } + await provider.postStateToWebview() break }