allow telemetry events to be disabled by name in feature flags #6946
Conversation
dosubot
bot
added
the
size:M
📝 WalkthroughWalkthroughAdds remote-config-driven suppression of telemetry events: RemoteConfig gains an optional Changes
Sequence Diagram(s)sequenceDiagram
participant RC as Remote Config
participant MTP as MixpanelTelemetryProvider
participant MP as Mixpanel
rect rgb(240,248,255)
Note over MTP,RC: Initialization / subscription
MTP->>RC: read telemetry_disabled_events
MTP->>MTP: buildEventSet(...) → disabledEvents
RC-->>MTP: subscribe/watch for updates
end
rect rgb(245,255,240)
Note over MTP,MP: Event flow
MTP->>MTP: trackEvent(event)
alt event ∈ disabledEvents
MTP-->>MTP: skip tracking
else
MTP->>MP: queue/track event
end
end
rect rgb(255,248,240)
Note over RC,MTP: Remote config update
RC->>MTP: notify change
MTP->>RC: read updated telemetry_disabled_events
MTP->>MTP: rebuild disabledEvents
end
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 11/26/2025, 09:06:35 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 11/26/2025, 09:14:33 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
christian-byrne
added
needs-backport
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.18 MB (baseline 3.18 MB) • 🔴 +2.36 kBMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 944 kB (baseline 944 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 298 kB (baseline 298 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 138 kB (baseline 138 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 2 added / 2 removed Utilities & Hooks — 2.94 kB (baseline 2.94 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 8.56 MB (baseline 8.56 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.84 MB (baseline 3.84 MB) • ⚪ 0 BBundles that do not match a named category
Status: 17 added / 17 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.ts (1)
140-152: Queued events bypass the disabled check.Events queued before Mixpanel initializes (or before remote config loads) will be tracked in
flushEventQueuewithout checkingdisabledEvents. If an event becomes disabled via remote config after being queued but before flush, it will still be sent.Consider filtering disabled events during flush:
private flushEventQueue(): void { if (!this.isInitialized || !this.mixpanel) { return } while (this.eventQueue.length > 0) { const event = this.eventQueue.shift()! + if (this.disabledEvents.has(event.eventName)) { + continue + } try { this.mixpanel.track(event.eventName, event.properties || {}) } catch (error) { console.error('Failed to track queued event:', error) } } }
🧹 Nitpick comments (2)
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.ts (2)
189-197: Consider a more idiomatic implementation.The
buildEventSetmethod can be simplified usingfilterfor better readability.private buildEventSet(values: TelemetryEventName[]): Set<TelemetryEventName> { - const normalized = new Set<TelemetryEventName>() - values.forEach((value) => { - if (TELEMETRY_EVENT_SET.has(value)) { - normalized.add(value) - } - }) - return normalized + return new Set(values.filter((value) => TELEMETRY_EVENT_SET.has(value))) }
182-187: Silent filtering of invalid event names may complicate debugging.When
buildEventSetfilters out invalid event names from remote config, there's no logging to indicate that an unrecognized event name was provided. This could make misconfiguration harder to diagnose.Consider adding a dev-only warning:
private buildEventSet(values: TelemetryEventName[]): Set<TelemetryEventName> { - const normalized = new Set<TelemetryEventName>() - values.forEach((value) => { - if (TELEMETRY_EVENT_SET.has(value)) { - normalized.add(value) + return new Set( + values.filter((value) => { + const isValid = TELEMETRY_EVENT_SET.has(value) + if (!isValid && import.meta.env.DEV) { + console.warn(`Unknown telemetry event name in disabled list: ${value}`) + } + return isValid + }) + ) - } - }) - return normalized }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/platform/remoteConfig/types.ts(2 hunks)src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.tssrc/platform/remoteConfig/types.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.tssrc/platform/remoteConfig/types.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.tssrc/platform/remoteConfig/types.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.tssrc/platform/remoteConfig/types.ts
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.tssrc/platform/remoteConfig/types.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.tssrc/platform/remoteConfig/types.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.tssrc/platform/remoteConfig/types.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.tssrc/platform/remoteConfig/types.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.tssrc/platform/remoteConfig/types.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.tssrc/platform/remoteConfig/types.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.tssrc/platform/remoteConfig/types.ts
🧠 Learnings (6)
📚 Learning: 2025-11-24T19:46:52.279Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T19:46:52.279Z
Learning: Applies to **/*.vue : Use watch and watchEffect for side effects in Vue 3
Applied to files:
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.ts
📚 Learning: 2025-11-24T19:46:52.279Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T19:46:52.279Z
Learning: Applies to **/*.{vue,ts,tsx} : Leverage VueUse functions for performance-enhancing utilities
Applied to files:
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Use watch and watchEffect for side effects
Applied to files:
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.{vue,ts,js} : Use useIntersectionObserver for visibility detection instead of custom scroll handlers
Applied to files:
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.{vue,ts,js} : Use existing VueUse composables (such as useElementHover) instead of manually managing event listeners
Applied to files:
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.{vue,ts} : Leverage VueUse functions for performance-enhancing styles
Applied to files:
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.ts
🧬 Code graph analysis (1)
src/platform/remoteConfig/types.ts (1)
src/platform/telemetry/types.ts (1)
TelemetryEventName(404-405)
🪛 ESLint
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.ts
[error] 2-2: Unable to resolve path to module 'vue'.
(import-x/no-unresolved)
[error] 45-45: Unable to resolve path to module '@/platform/remoteConfig/remoteConfig'.
(import-x/no-unresolved)
[error] 47-47: Unable to resolve path to module '../../types'.
(import-x/no-unresolved)
[error] 48-48: Unable to resolve path to module '../../utils/surveyNormalization'.
(import-x/no-unresolved)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: deploy-and-comment
- GitHub Check: test
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: collect
🔇 Additional comments (3)
src/platform/remoteConfig/types.ts (1)
36-36: LGTM!The new optional property correctly types the disabled events list using
TelemetryEventName[], enabling type-safe configuration of disabled telemetry events.src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.ts (2)
50-67: LGTM!Good approach defining defaults and a validation set. The
TELEMETRY_EVENT_SETenables O(1) validation of event names from remote config, preventing invalid event names from being added to the disabled set.
163-165: LGTM!Clean early return pattern for disabled events. This prevents both immediate tracking and queuing of disabled events.
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.ts
Outdated
Show resolved
Hide resolved
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.ts
Outdated
Show resolved
Hide resolved
src/platform/telemetry/providers/cloud/MixpanelTelemetryProvider.ts
Outdated
Show resolved
Hide resolved
## Summary Adds ability to toggle events on/off by name from dynamic feature flags. Also makes some events disabled by default (not being used for any analysis and taking too much of event quota currently). Note: telemetry is only enabled on cloud - this doesn't affect local users in any way. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6946-allow-telemetry-events-to-be-disabled-by-name-in-feature-flags-2b76d73d365081ea8676cfbb8217e640) by [Unito](https://www.unito.io)
|
@christian-byrne Successfully backported to #6964 |
github-actions
bot
removed
the
needs-backport
…n feature flags (#6964) Backport of #6946 to `cloud/1.32` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6964-backport-cloud-1-32-allow-telemetry-events-to-be-disabled-by-name-in-feature-flags-2b76d73d36508116b888ecbf19a8a52c) by [Unito](https://www.unito.io) Co-authored-by: Christian Byrne <[email protected]>
3 participants
Summary
Adds ability to toggle events on/off by name from dynamic feature flags. Also makes some events disabled by default (not being used for any analysis and taking too much of event quota currently).
Note: telemetry is only enabled on cloud - this doesn't affect local users in any way.
┆Issue is synchronized with this Notion page by Unito