Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion packages/telemetry/src/TelemetryService.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions packages/types/src/telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}

/**
Expand Down Expand Up @@ -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({
Expand Down
155 changes: 155 additions & 0 deletions src/core/webview/__tests__/telemetrySettingsTracking.spec.ts
Original file line number Diff line number Diff line change
@@ -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<typeof vi.fn>
updateTelemetryState: ReturnType<typeof vi.fn>
hasInstance: ReturnType<typeof vi.fn>
}

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",
})
})
})
})
20 changes: 18 additions & 2 deletions src/core/webview/webviewMessageHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading