Repository navigation
[Fix] Stop code index telemetry storms and hash the machine ID - #2329
zoomote2[bot] wants to merge 1 commit into
Conversation
…stinct IDs (#830) - Add a per-workspace circuit breaker to the code-index file watcher: after 3 consecutive failed batches it drops further file-system events until the watcher is recreated (embedder/vector-store config change), stopping the retry loop behind the 254k-event Code Index Error spike. - Remove the two per-call captureToolUsage sites re-added by #1073 so tool usage reports only as the aggregated toolsUsed breakdown on Task Completed; drop the dead captureToolUsage/captureConversationMessage methods from TelemetryService. - Salt-hash the PostHog distinct ID (SHA-256 + fixed salt) in PostHogTelemetryClient and before posting machineId to the webview, so the raw VS Code machine ID never leaves the extension host. - Update PRIVACY.md: hashed machine-ID wording and a clarification that the Cloud sync pipeline is separate from PostHog telemetry (issue #830).
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one): 1
Please check the settings in the CodeRabbit UI or the
 |
|
1 issue outstanding. See session
Reviewed fd1399f |
Review statusThis PR was opened by an automated account. A human maintainer must verify the change intent, provenance, and validation before merging. Current step: Mark the PR ready. Required CI must pass before CodeRabbit starts. Review-state labels are managed by this workflow; do not edit them manually. |
![zoomote2[bot]](https://avatars.githubusercontent.com/u/278826416?s=60&v=4){kind=small}
| if (!this.circuitBreakerOpen && this.consecutiveFailedBatches >= CIRCUIT_BREAKER_BATCH_THRESHOLD) { | ||
| this.circuitBreakerOpen = true | ||
| this.accumulatedEvents.clear() | ||
| console.error( | ||
| `[FileWatcher] Circuit breaker opened after ${this.consecutiveFailedBatches} consecutive failed batches; ` + | ||
| `dropping further file-system events for this workspace until the embedder or vector store configuration changes.`, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Circuit breaker opens with no user-visible state change.
When the breaker opens here, the only signal is a console.error. On this same trip batch, the orchestrator's onBatchProgressUpdate handler still transitions the state to Indexed ("File changes processed. Index up-to-date.") because the final progress event fires with processedInBatch === totalInBatch, and the orchestrator's onDidFinishBatchProcessing handler only logs summary.batchError — nothing reaches the state manager.
From this point on, every file-system event in the workspace is dropped silently (deletions included), so the index goes permanently stale for the session while the UI keeps claiming it is up-to-date. A user who fixed a transient outage (e.g. an Ollama restart) gets no signal and no recovery hint until they happen to change config or re-run indexing.
Consider surfacing the open breaker to the code-index state — e.g. a dedicated event or an circuitBreakerOpen flag on BatchProcessingSummary that the orchestrator converts into an Error/Standby state message — so the UI stops claiming "up-to-date" once the watcher has stopped consuming events.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Related GitHub Issue
Closes: #830
Description
Issue #830 covers two goals: lower PostHog event volume and fix consent and privacy gaps. Earlier PRs already shipped most of the issue: task completion aggregation (#1071), model cache empty-response throttling, the consent banner with explicit Accept and Decline (#1069), the
vscode.env.isTelemetryEnabledgate with a live toggle listener, and the awaited shutdown (#1070). This PR closes the remaining gaps:Code Index Errorevents in the June spike.FileWatchernow counts consecutive failed batches. A batch fails when the batch pipeline breaks (deletion or upsert errors) or when every attempted file in the batch fails. After 3 consecutive failures, the watcher drops further events and stops the telemetry until it is recreated. A config change recreates the watcher throughCodeIndexManager._recreateServices, so the breaker opens again only when the embedder config actually changes. Isolated per-file failures never trip the breaker. ATelemetryService-level breaker (fix(telemetry): add circuit breaker and shutdown draining to TelemetryService #1070) already caps event volume; this breaker stops the retry loop itself.Tool Usedevents (regression fix). feat(telemetry): aggregate task completion telemetry with delta installments #1071 moved tool usage into the aggregatedtoolsUsedbreakdown onTask Completed, but fix(telemetry): record tool usage once centrally, sanitize raw tool names #1073 re-added two per-callcaptureToolUsagecalls. Each tool call then sent two events. This PR removes both call sites and the now-deadcaptureToolUsageandcaptureConversationMessagemethods.recordToolUsagestill feeds the per-task breakdown, and a call-site reintroduction now fails typecheck.vscode.env.machineIdas the distinct ID. A newhashDistinctIdhelper (SHA-256 with a fixed application salt) now produces the ID inPostHogTelemetryClient.ClineProvideralso hashes the value before it posts state to the webview, so the raw machine ID never leaves the extension host. The webview keeps using the posted value as its own PostHog distinct ID, so analytics identity stays stable per device.PRIVACY.mdnow says the machine ID is salted and hashed before it leaves the device, and it states that the Cloud sync pipeline, which transmits task messages when Cloud sync is on, is separate from PostHog telemetry.The issue references
TELEMETRY_GAP_ANALYSIS.mdin the repo root. That file does not exist in the repository, so this PR follows the issue body alone.Test Procedure
file-watcher.spec.ts: breaker opens after 3 consecutive failed batches and later events are dropped; a successful batch resets the counter; a batch whose upsert exhausts retries counts as a failure; isolated per-file failures never trip the breaker.PostHogTelemetryClient.test.tsnow asserts the PostHog distinct ID equals the salted hash and never contains the raw machine ID.presentAssistantMessage-tool-usage-attribution.spec.tsasserts onerecordToolUsageper validated call and no per-call telemetry event.pnpm --dir src exec vitest run services/code-index core/assistant-message,pnpm --filter @roo-code/telemetry test, thenpnpm run check-typesandpnpm test.pnpm run check-typespasses (11/11 tasks) andpnpm test --forcepasses (14/14 tasks; 12,572 extension tests, 2,487 webview tests). ESLint suppression counts did not increase.Pre-Submission Checklist
src/test-utils/,@/utils/test-utils) are used only for mechanical duplication; test intent and scenario-specific mocks remain visible.*.visual.tsxsnapshot inwebview-ui/. Seewebview-ui/AGENTS.md→ "When a UI change needs a snapshot". (No UI change in this PR.)Visual Snapshots
No UI change. No snapshot required.
Videos (interaction / animation only)
No interaction change. No video required.
Documentation Updates
Zoo Code Docs needs no paired update. This PR has no user-visible UI or behavior change: the circuit breaker, the event aggregation fix, and the distinct-ID hash are all internal telemetry behavior. The policy text changes live in
PRIVACY.mdin this repository.Additional Notes
One operational note for reviewers: PostHog will see a new distinct-ID format after deployment. Existing event history keeps the old IDs, so before-and-after user counts will not join across the deploy boundary.
Get in Touch
0x7777777_