chore(cli): mute telemetry without removing instrumentation#2695
Conversation
Prevent the Mixpanel client from being initialized so all telemetry events no-op and no data is sent. The tracking instrumentation (trackSpan/trackCommand/trackCli and call sites) is left intact and can be re-enabled by restoring the constructor body. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR disables telemetry in the CLI by modifying the ChangesTelemetry Disablement
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/cli/src/telemetry.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Claude finished @ymc9's task —— View job Code ReviewReviewing PR: chore(cli): mute telemetry without removing instrumentation Todo List
✅ Review SummaryThis PR effectively achieves its goal of muting telemetry while preserving the instrumentation code. The implementation is clean and follows good practices. Code Quality & Best Practices✅ Excellent approach: The solution elegantly disables telemetry by preventing Mixpanel initialization rather than removing tracking calls throughout the codebase. This makes re-enabling trivial. ✅ Clean implementation:
✅ Maintains code structure: All telemetry methods ( Potential Issues✅ No bugs identified: The implementation correctly leverages the existing guard clause ✅ Consistent behavior: The Performance Considerations✅ Improved performance:
Minor consideration: The telemetry constructor still collects system metadata (OS type, version, Prisma version, etc.) at Security Concerns✅ No security issues:
Test CoverageConsideration needed: While the change is straightforward, consider adding tests to verify:
The change maintains backward compatibility and doesn't break existing functionality, making it low-risk. Overall AssessmentVerdict: ✅ Approved - This is a well-executed solution that achieves the stated goal with minimal code changes and no breaking changes. The approach is reversible and maintainable. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/cli/src/telemetry.ts (1)
46-50: 💤 Low valueConsider documenting the required imports for re-enabling.
The commented code references
initandTELEMETRY_TRACKING_TOKEN, which are not currently imported. To make re-enabling telemetry more straightforward, consider adding a comment indicating that the following import should be restored:// To re-enable, also restore: import { init, TELEMETRY_TRACKING_TOKEN } from 'mixpanel';This will help future developers avoid reference errors when uncommenting this code.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/telemetry.ts` around lines 46 - 50, Add a short comment near the commented telemetry block indicating the missing imports so future devs can re-enable it without errors: mention restoring "import { init, TELEMETRY_TRACKING_TOKEN } from 'mixpanel';" and note that these symbols (init and TELEMETRY_TRACKING_TOKEN) are required by the commented code that initializes this.mixpanel; place the comment adjacent to the existing commented block in telemetry.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/cli/src/telemetry.ts`:
- Around line 46-50: Add a short comment near the commented telemetry block
indicating the missing imports so future devs can re-enable it without errors:
mention restoring "import { init, TELEMETRY_TRACKING_TOKEN } from 'mixpanel';"
and note that these symbols (init and TELEMETRY_TRACKING_TOKEN) are required by
the commented code that initializes this.mixpanel; place the comment adjacent to
the existing commented block in telemetry.ts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0185fdbc-0e51-43af-baf5-9e48b306509d
📒 Files selected for processing (1)
packages/cli/src/telemetry.ts
Summary
Mutes all CLI telemetry without deleting the telemetry code. The Mixpanel client is no longer initialized in the
Telemetryconstructor, so everytrack()call short-circuits on itsif (this.mixpanel)guard and no events are sent.Changes
packages/cli/src/telemetry.ts: early-return in the constructor before Mixpanelinit(); the original init logic is preserved as a comment. Removed now-unusedinit/TELEMETRY_TRACKING_TOKENimports.Why this is sufficient
this.mixpanelstaysundefined, so all events (cli:*,cli:command:*,cli:plugin:*, errors) no-op — no network requests to Mixpanel.isTrackingreturnsfalse, so the exit-flush delay inindex.tsis also skipped.Not removed
The full instrumentation pipeline (
trackSpan,trackCommand,trackCli, payload assembly, all call sites) is intact. Re-enabling is just restoring the constructor body.🤖 Generated with Claude Code
Summary by CodeRabbit