Skip to content

Commit b5c58b6

Browse files
mrubensmrubens
authored
Track when telemetry settings change (#8339)
1 parent 28a7e4c commit b5c58b6

File tree

4 files changed

+200
-3
lines changed

4 files changed

+200
-3
lines changed

‎packages/telemetry/src/TelemetryService.ts‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import { ZodError } from "zod"
22

3-
import { type TelemetryClient, type TelemetryPropertiesProvider, TelemetryEventName } from "@roo-code/types"
3+
import {
4+
type TelemetryClient,
5+
type TelemetryPropertiesProvider,
6+
TelemetryEventName,
7+
type TelemetrySetting,
8+
} from "@roo-code/types"
49

510
/**
611
* TelemetryService wrapper class that defers initialization.
@@ -226,6 +231,18 @@ export class TelemetryService {
226231
this.captureEvent(TelemetryEventName.TITLE_BUTTON_CLICKED, { button })
227232
}
228233

234+
/**
235+
* Captures when telemetry settings are changed
236+
* @param previousSetting The previous telemetry setting
237+
* @param newSetting The new telemetry setting
238+
*/
239+
public captureTelemetrySettingsChanged(previousSetting: TelemetrySetting, newSetting: TelemetrySetting): void {
240+
this.captureEvent(TelemetryEventName.TELEMETRY_SETTINGS_CHANGED, {
241+
previousSetting,
242+
newSetting,
243+
})
244+
}
245+
229246
/**
230247
* Checks if telemetry is currently enabled
231248
* @returns Whether telemetry is enabled

‎packages/types/src/telemetry.ts‎

Lines changed: 9 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
/**
@@ -202,6 +203,14 @@ export const rooCodeTelemetryEventSchema = z.discriminatedUnion("type", [
202203
]),
203204
properties: telemetryPropertiesSchema,
204205
}),
206+
z.object({
207+
type: z.literal(TelemetryEventName.TELEMETRY_SETTINGS_CHANGED),
208+
properties: z.object({
209+
...telemetryPropertiesSchema.shape,
210+
previousSetting: telemetrySettingsSchema,
211+
newSetting: telemetrySettingsSchema,
212+
}),
213+
}),
205214
z.object({
206215
type: z.literal(TelemetryEventName.TASK_MESSAGE),
207216
properties: z.object({

‎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: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2296,9 +2296,25 @@ 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-
TelemetryService.instance.updateTelemetryState(isOptedIn)
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+
// Update the telemetry state
2308+
await updateGlobalState("telemetrySetting", telemetrySetting)
2309+
if (TelemetryService.hasInstance()) {
2310+
TelemetryService.instance.updateTelemetryState(isOptedIn)
2311+
}
2312+
2313+
// If turning telemetry ON, fire event AFTER enabling
2314+
if (!wasPreviouslyOptedIn && isOptedIn && TelemetryService.hasInstance()) {
2315+
TelemetryService.instance.captureTelemetrySettingsChanged(previousSetting, telemetrySetting)
2316+
}
2317+
23022318
await provider.postStateToWebview()
23032319
break
23042320
}

0 commit comments

Comments
 (0)