Telemetry: Centralize disable logic with module-level flag#34441
Telemetry: Centralize disable logic with module-level flag#34441valentinpalkovic wants to merge 2 commits into
Conversation
Instead of requiring every telemetry call site to check `disableTelemetry`, introduce a module-level flag in the telemetry module that gates all calls. The `telemetry()` function now early-returns when disabled, making all callers safe by default. This fixes #34431 where `checklist.ts` was missing the guard, causing telemetry to be sent despite `disableTelemetry: true`. The flag is set at startup from CLI options (via `withTelemetry`) and from config (via preset loading in `buildDevStandalone`, `buildStaticStandalone`, and `experimental_serverChannel`). All ~20 scattered `if (!disableTelemetry)` guards have been removed. Closes #34431 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
View your CI Pipeline Execution ↗ for commit 7f4084b
☁️ Nx Cloud last updated this comment at |
|
Closing to recreate with proper PR template. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (32)
📝 WalkthroughWalkthroughCentralized telemetry disablement was introduced via module-level controls ( Changes
Sequence Diagram(s)sequenceDiagram
participant Presets
participant Server
participant Channel as "Server Channel"
participant Telemetry as "Telemetry Module"
participant External as "External Telemetry Service"
Presets->>Telemetry: call setTelemetryEnabled(false)? / default enabled
Presets->>Server: start server (withTelemetry)
Server->>Channel: register handlers (emit events)
Channel->>Telemetry: telemetry('event', payload)
alt telemetry module enabled
Telemetry->>External: sendTelemetry(payload)
else telemetry module disabled
Telemetry-->>Channel: no-op (return early)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
Comment |
Closes #34431
What I did
Telemetry was sent despite
disableTelemetry: truein the main config because telemetry disable guards (if (!options.disableTelemetry)) were scattered across ~40 files and some locations (likechecklist.ts, reported in #34431) were missing the guard entirely.This PR introduces a module-level disable flag in the telemetry module. The
telemetry()function now early-returns when disabled, making all callers safe by default — no guards needed at call sites.How it works
setTelemetryEnabled(enabled)andisTelemetryModuleEnabled()tocode/core/src/telemetry/index.tstelemetry()function checks the module flag and early-returns when disabled (skipping bothnotify()andsendTelemetry())withTelemetry()— from CLI--disable-telemetryflagbuildDevStandalone()— fromcore.disableTelemetryconfigbuildStaticStandalone()— fromcore.disableTelemetryconfigexperimental_serverChannel(common-preset) — fromcore.disableTelemetryconfigcore.disableTelemetryorSTORYBOOK_DISABLE_TELEMETRYenvif (!disableTelemetry)guards across the codebasecoreOptions/coreConfigparameters from channel init functionsFiles changed (32 files)
telemetry/index.ts— module flag +setTelemetryEnabled+isTelemetryModuleEnabledwithTelemetry.ts,build-dev.ts,build-static.ts,common-preset.ts,vitest-plugin/index.tsdoTelemetry.ts,telemetry-channel.ts,create-new-story-channel.ts,file-search-channel.ts,ghost-stories-channel.ts,open-in-editor-channel.ts,whats-new.ts,save-story.ts,dev-server.ts,cli-storybook/bin/run.ts,upgrade.ts,TelemetryService.ts,initiate.ts,UserPreferencesCommand.ts,AddonConfigurationCommand.ts,ProjectDetectionCommand.ts,scaffold-new-project.ts,onboarding/preset.ts,vitest/preset.ts,mocking-utils/extract.tsChecklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
disableTelemetry: truein.storybook/main.tsconfigstorybook devstorybook.js.org/event-log--disable-telemetryCLI flagDocumentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
Refactor
Chores