feat: add telemetry queue persistence system #7133
Conversation
- Implement TelemetryQueueManager for persistent event storage - Add QueuedTelemetryClient base class with retry logic - Update PostHogTelemetryClient to use queuing system - Store queue per-workspace to avoid conflicts - Add exponential backoff retry (1s to 60s max) - Events persist for 24 hours before expiring - Queue limited to 100 events to manage file size - Add comprehensive tests for queue functionality - Disable PostHog's internal queue for better control This ensures telemetry events are not lost during network outages or server downtime, with events persisted to disk and retried automatically when connectivity is restored.
dosubot
bot
added
size:XXL
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thank you for your contribution! I've reviewed the telemetry queue persistence implementation and found several issues that need attention. The overall architecture is solid, but there are some critical issues around production logging and potential race conditions that should be addressed.
|
|
||
| // Force immediate flush to detect network errors | ||
| // This will throw if there's a network issue | ||
| await this.client.flush() |
There was a problem hiding this comment.
The flush() call could fail for reasons other than network issues. Could we add more specific error handling here to distinguish between different failure types? This would help with debugging and potentially allow for different retry strategies based on the error type.
| /** | ||
| * Persist queue to disk | ||
| */ | ||
| private async persistQueue(): Promise<void> { |
There was a problem hiding this comment.
The persistQueue() method writes to disk asynchronously without awaiting. Could this cause race conditions if multiple rapid enqueue operations occur? Consider using a write queue or debouncing mechanism to prevent potential data corruption.
| public updateTelemetryState(didUserOptIn: boolean): void { | ||
| this.telemetryEnabled = didUserOptIn | ||
| } | ||
| } |
There was a problem hiding this comment.
Could we define a proper interface for the test client instead of using 'any' type? This would improve type safety and make the tests more maintainable.
| }) | ||
|
|
||
| describe("getRetryDelay", () => { | ||
| it("should calculate exponential backoff correctly", () => { |
There was a problem hiding this comment.
I noticed there's no test for the scenario where the queue file is corrupted (invalid JSON). The loadQueue method handles this case, but it would be good to have explicit test coverage.
- Make console.log statements conditional based on debug flag - Add singleton reset method for TelemetryQueueManager - Fix race condition in persistQueue with debouncing and promise tracking - Improve error handling in PostHogTelemetryClient to differentiate error types - Make retry interval configurable in QueuedTelemetryClient - Update tests to reflect 24-hour event expiration instead of retry-based removal - Add test for handling corrupted JSON queue files - Fix test expectations for retry count and event filtering
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
- Move pendingPersist flag clearing inside persistQueue() method - Implement loop-based draining to handle concurrent persist requests - Add comprehensive tests for concurrent operations - Ensures no telemetry events are lost during rapid enqueue operations The previous implementation had a lost-notification bug where the pendingPersist flag was cleared in the setImmediate callback before calling persistQueue(). This could cause events enqueued during an in-flight persist to remain unpersisted. The fix implements Option A: clearing the flag inside persistQueue() and using a while loop to drain all pending requests, ensuring any enqueue that happens during a persist operation triggers another persist pass immediately after.
- Remove unused _isNetworkError variable in PostHogTelemetryClient - Add TelemetryQueueManager.resetInstance() call in extension deactivation to prevent memory leaks and stale data when switching workspaces
- Removed all console.info and console.log statements that were used for debugging - Kept console.error statements for actual error reporting - Fixed ESLint warnings for unused error variables - Telemetry debug logs are now completely removed to prevent console spam
github-project-automation
bot
moved this from PR [Needs Review]
to Done
in Roo Code Roadmap
Summary
This PR implements a telemetry queuing system to ensure telemetry events are not lost during network outages or server downtime.
Problem
Currently, the Roo Code Extension sends telemetry using a "fire and forget" approach. When telemetry events fail to send (due to network issues, server downtime, or other connectivity problems), they are simply lost with no retry mechanism.
Solution
Key Components:
TelemetryQueueManager - Manages persistent event storage
QueuedTelemetryClient - Base class for telemetry clients
Updated PostHogTelemetryClient
Features:
Testing:
Changes
How to Test
The queue file is stored at:
Important
Introduces a telemetry queuing system with persistent storage and retry logic to handle failed telemetry events due to network issues.
TelemetryQueueManagerfor persistent event storage and retry logic with exponential backoff inTelemetryQueueManager.ts.QueuedTelemetryClientas a base class for telemetry clients to handle automatic queuing and retry inQueuedTelemetryClient.ts.PostHogTelemetryClientto extendQueuedTelemetryClient, disabling PostHog's internal queue and handling retries inPostHogTelemetryClient.ts.TelemetryQueueManagerinTelemetryQueueManager.test.ts.QueuedTelemetryClientretry logic inQueuedTelemetryClient.test.ts.PostHogTelemetryClienttests inPostHogTelemetryClient.test.ts.index.ts.extension.tsto register the new telemetry client with debug mode support.This description was created by
for a897849. You can customize this summary. It will automatically update as commits are pushed.