Skip to content

Commit a97a7df

Browse files
mrubensmrubens
committed
Track when telemetry settings change
1 parent 28a7e4c commit a97a7df

File tree

4 files changed

+185
-1
lines changed

4 files changed

+185
-1
lines changed

‎packages/telemetry/src/TelemetryService.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,18 @@ export class TelemetryService {
226226
this.captureEvent(TelemetryEventName.TITLE_BUTTON_CLICKED, { button })
227227
}
228228

229+
/**
230+
* Captures when telemetry settings are changed
231+
* @param previousSetting The previous telemetry setting
232+
* @param newSetting The new telemetry setting
233+
*/
234+
public captureTelemetrySettingsChanged(previousSetting: string, newSetting: string): void {
235+
this.captureEvent(TelemetryEventName.TELEMETRY_SETTINGS_CHANGED, {
236+
previousSetting,
237+
newSetting,
238+
})
239+
}
240+
229241
/**
230242
* Checks if telemetry is currently enabled
231243
* @returns Whether telemetry is enabled

‎packages/types/src/telemetry.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ export enum TelemetryEventName {
7171
SHELL_INTEGRATION_ERROR = "Shell Integration Error",
7272
CONSECUTIVE_MISTAKE_ERROR = "Consecutive Mistake Error",
7373
CODE_INDEX_ERROR = "Code Index Error",
74+
TELEMETRY_SETTINGS_CHANGED = "Telemetry Settings Changed",
7475
}
7576

7677
/**
@@ -199,6 +200,7 @@ export const rooCodeTelemetryEventSchema = z.discriminatedUnion("type", [
199200
TelemetryEventName.TAB_SHOWN,
200201
TelemetryEventName.MODE_SETTINGS_CHANGED,
201202
TelemetryEventName.CUSTOM_MODE_CREATED,
203+
TelemetryEventName.TELEMETRY_SETTINGS_CHANGED,
202204
]),
203205
properties: telemetryPropertiesSchema,
204206
}),

‎src/core/webview/__tests__/telemetrySettingsTracking.spec.ts‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
import { describe, it, expect, vi, beforeEach } from "vitest"
2+
import { TelemetryService } from "@roo-code/telemetry"
3+
import { TelemetryEventName, type TelemetrySetting } from "@roo-code/types"
4+
5+
describe("Telemetry Settings Tracking", () => {
6+
let mockTelemetryService: {
7+
captureTelemetrySettingsChanged: ReturnType<typeof vi.fn>
8+
updateTelemetryState: ReturnType<typeof vi.fn>
9+
hasInstance: ReturnType<typeof vi.fn>
10+
}
11+
12+
beforeEach(() => {
13+
// Reset mocks
14+
vi.clearAllMocks()
15+
16+
// Create mock service
17+
mockTelemetryService = {
18+
captureTelemetrySettingsChanged: vi.fn(),
19+
updateTelemetryState: vi.fn(),
20+
hasInstance: vi.fn().mockReturnValue(true),
21+
}
22+
23+
// Mock the TelemetryService
24+
vi.spyOn(TelemetryService, "hasInstance").mockReturnValue(true)
25+
vi.spyOn(TelemetryService, "instance", "get").mockReturnValue(mockTelemetryService as any)
26+
})
27+
28+
describe("when telemetry is turned OFF", () => {
29+
it("should fire event BEFORE disabling telemetry", () => {
30+
const previousSetting = "enabled" as TelemetrySetting
31+
const newSetting = "disabled" as TelemetrySetting
32+
33+
// Simulate the logic from webviewMessageHandler
34+
const isOptedIn = newSetting !== "disabled"
35+
const wasPreviouslyOptedIn = previousSetting !== "disabled"
36+
37+
// If turning telemetry OFF, fire event BEFORE disabling
38+
if (wasPreviouslyOptedIn && !isOptedIn && TelemetryService.hasInstance()) {
39+
TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, newSetting)
40+
}
41+
42+
// Update the telemetry state
43+
TelemetryService.instance.updateTelemetryState(isOptedIn)
44+
45+
// Verify the event was captured before updateTelemetryState
46+
expect(mockTelemetryService.captureTelemetrySettingsChanged).toHaveBeenCalledWith("enabled", "disabled")
47+
expect(mockTelemetryService.captureTelemetrySettingsChanged).toHaveBeenCalledBefore(
48+
mockTelemetryService.updateTelemetryState as any,
49+
)
50+
expect(mockTelemetryService.updateTelemetryState).toHaveBeenCalledWith(false)
51+
})
52+
53+
it("should fire event when going from unset to disabled", () => {
54+
const previousSetting = "unset" as TelemetrySetting
55+
const newSetting = "disabled" as TelemetrySetting
56+
57+
const isOptedIn = newSetting !== "disabled"
58+
const wasPreviouslyOptedIn = previousSetting !== "disabled"
59+
60+
if (wasPreviouslyOptedIn && !isOptedIn && TelemetryService.hasInstance()) {
61+
TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, newSetting)
62+
}
63+
64+
TelemetryService.instance.updateTelemetryState(isOptedIn)
65+
66+
expect(mockTelemetryService.captureTelemetrySettingsChanged).toHaveBeenCalledWith("unset", "disabled")
67+
})
68+
})
69+
70+
describe("when telemetry is turned ON", () => {
71+
it("should fire event AFTER enabling telemetry", () => {
72+
const previousSetting = "disabled" as TelemetrySetting
73+
const newSetting = "enabled" as TelemetrySetting
74+
75+
const isOptedIn = newSetting !== "disabled"
76+
const wasPreviouslyOptedIn = previousSetting !== "disabled"
77+
78+
// Update the telemetry state first
79+
TelemetryService.instance.updateTelemetryState(isOptedIn)
80+
81+
// If turning telemetry ON, fire event AFTER enabling
82+
if (!wasPreviouslyOptedIn && isOptedIn && TelemetryService.hasInstance()) {
83+
TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, newSetting)
84+
}
85+
86+
// Verify the event was captured after updateTelemetryState
87+
expect(mockTelemetryService.updateTelemetryState).toHaveBeenCalledWith(true)
88+
expect(mockTelemetryService.captureTelemetrySettingsChanged).toHaveBeenCalledWith("disabled", "enabled")
89+
expect(mockTelemetryService.updateTelemetryState).toHaveBeenCalledBefore(
90+
mockTelemetryService.captureTelemetrySettingsChanged as any,
91+
)
92+
})
93+
94+
it("should not fire event when going from enabled to enabled", () => {
95+
const previousSetting = "enabled" as TelemetrySetting
96+
const newSetting = "enabled" as TelemetrySetting
97+
98+
const isOptedIn = newSetting !== "disabled"
99+
const wasPreviouslyOptedIn = previousSetting !== "disabled"
100+
101+
// Neither condition should be met
102+
if (wasPreviouslyOptedIn && !isOptedIn && TelemetryService.hasInstance()) {
103+
TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, newSetting)
104+
}
105+
106+
TelemetryService.instance.updateTelemetryState(isOptedIn)
107+
108+
if (!wasPreviouslyOptedIn && isOptedIn && TelemetryService.hasInstance()) {
109+
TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, newSetting)
110+
}
111+
112+
// Should not fire any telemetry events
113+
expect(mockTelemetryService.captureTelemetrySettingsChanged).not.toHaveBeenCalled()
114+
expect(mockTelemetryService.updateTelemetryState).toHaveBeenCalledWith(true)
115+
})
116+
117+
it("should fire event when going from unset to enabled (telemetry banner close)", () => {
118+
const previousSetting = "unset" as TelemetrySetting
119+
const newSetting = "enabled" as TelemetrySetting
120+
121+
const isOptedIn = newSetting !== "disabled"
122+
const wasPreviouslyOptedIn = previousSetting !== "disabled"
123+
124+
// For unset -> enabled, both are opted in, so no event should fire
125+
if (wasPreviouslyOptedIn && !isOptedIn && TelemetryService.hasInstance()) {
126+
TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, newSetting)
127+
}
128+
129+
TelemetryService.instance.updateTelemetryState(isOptedIn)
130+
131+
if (!wasPreviouslyOptedIn && isOptedIn && TelemetryService.hasInstance()) {
132+
TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, newSetting)
133+
}
134+
135+
// unset is treated as opted-in, so no event should fire
136+
expect(mockTelemetryService.captureTelemetrySettingsChanged).not.toHaveBeenCalled()
137+
})
138+
})
139+
140+
describe("TelemetryService.captureTelemetrySettingsChanged", () => {
141+
it("should call captureEvent with correct parameters", () => {
142+
// Create a real instance to test the method
143+
const mockCaptureEvent = vi.fn()
144+
const service = new (TelemetryService as any)([])
145+
service.captureEvent = mockCaptureEvent
146+
147+
service.captureTelemetrySettingsChanged("enabled", "disabled")
148+
149+
expect(mockCaptureEvent).toHaveBeenCalledWith(TelemetryEventName.TELEMETRY_SETTINGS_CHANGED, {
150+
previousSetting: "enabled",
151+
newSetting: "disabled",
152+
})
153+
})
154+
})
155+
})

‎src/core/webview/webviewMessageHandler.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2296,9 +2296,24 @@ export const webviewMessageHandler = async (
22962296

22972297
case "telemetrySetting": {
22982298
const telemetrySetting = message.text as TelemetrySetting
2299-
await updateGlobalState("telemetrySetting", telemetrySetting)
2299+
const previousSetting = getGlobalState("telemetrySetting") || "unset"
23002300
const isOptedIn = telemetrySetting !== "disabled"
2301+
const wasPreviouslyOptedIn = previousSetting !== "disabled"
2302+
2303+
// If turning telemetry OFF, fire event BEFORE disabling
2304+
if (wasPreviouslyOptedIn && !isOptedIn && TelemetryService.hasInstance()) {
2305+
TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, telemetrySetting)
2306+
}
2307+
2308+
// Update the telemetry state
2309+
await updateGlobalState("telemetrySetting", telemetrySetting)
23012310
TelemetryService.instance.updateTelemetryState(isOptedIn)
2311+
2312+
// If turning telemetry ON, fire event AFTER enabling
2313+
if (!wasPreviouslyOptedIn && isOptedIn && TelemetryService.hasInstance()) {
2314+
TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, telemetrySetting)
2315+
}
2316+
23022317
await provider.postStateToWebview()
23032318
break
23042319
}

0 commit comments

Comments
 (0)