fix(telemetry): cut PostHog event volume (flag events, person.set carrier, error caps) - #1362
Draft
benceruleanlu wants to merge 1 commit into
Draft
fix(telemetry): cut PostHog event volume (flag events, person.set carrier, error caps)#1362benceruleanlu wants to merge 1 commit into
benceruleanlu wants to merge 1 commit into
Conversation
…rier, error caps) Four independent volume cuts in the main-process telemetry pipe, part of the PostHog cost-cut program (GTM-394): - $feature_flag_called (3.37M/28d): set client-level sendFeatureFlagEvent: false so no flag read can emit the implicit per-evaluation event. Flag evaluation behavior is unchanged. - comfy.desktop.person.set (15.2M/28d): authenticated person-property updates now ride $set/$set_once on the next captured event instead of a dedicated carrier event. session.ended at shutdown is the guaranteed last carrier; identity boundaries (logout/account switch) still flush eagerly as a dedicated event so updates never land on the wrong person. identify/distinct_id semantics untouched. - comfy.desktop.app_update.error (2.6M/28d): cap identical error messages at 5 emits per app session on top of the existing 1s repeat guard; the 10-min auto-check re-fails forever on a broken install. - $exception (~1.9M/mo, 93.9% four crash-loop messages): cap identical scrubbed exception messages at 5 per session in captureException. First occurrences always ship; distinct messages are unaffected.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of the PostHog cost-cut program (GTM-394): desktop is the largest event producer on the ~$18.6K/mo bill (target ≤$10K). The four cuts here remove roughly 21M+ events/28d of pure carrier/loop volume — ~$700-800/mo combined across product-analytics ingestion, the warehouse event export, and the error-tracking line ($340/mo) — without removing any analytical signal. All four changes are in the main process (
src/main/lib/telemetry.ts,src/main/lib/updater.ts); the renderer has no PostHog SDK, so this covers every desktop emit path.1.
$feature_flag_called— 3.37M/28d, 91.7% fromdesktop-cloud-capacitygetOpsFlagalready opts out per-call (sendFeatureFlagEvents: false, since #1304), but fleet volume shows released builds still emitting one implicit event per flag evaluation. This PR sets the client-level defaultsendFeatureFlagEvent: falseon the PostHog constructor so no current or futuregetFeatureFlag/isFeatureEnabledcall site can silently reintroduce a per-evaluation event.Deliberately NOT changed: flag evaluation is untouched —
getOpsFlag,getAllFlags(which never emitted this event), the ops-flag cache, and experiment variant assignment all behave exactly as before. Experiment exposure remains explicitly recorded viacomfy.desktop.experiment.exposed(experiments.ts), which is what dashboards actually join on.2.
comfy.desktop.person.set— 15.2M/28d from ~750K persons (~20/person/mo)The event's only job was carrying
$setperson properties. Authenticated person-property updates now ride$set/$set_onceon the next captured event (posthog-node applies person properties from any event's properties), so the dedicated carrier event disappears from steady-state traffic.Carrier choice and why person properties keep flowing:
capture()— in practice the very next product event, since updates are registered adjacent to the events that cause them (boot settings snapshot, hardware census, cloud launch, settings changes).comfy.desktop.session.endedat shutdown is the guaranteed last carrier of a clean session, so a queued update is delivered at most one session late even in an otherwise event-quiet session.person.set(logout, account switch) so an update can never land on the wrong person; anything unflushable at that boundary is discarded rather than leaked to the next identity.Deliberately NOT changed:
distinct_idselection, the singleclient.identify()at Firebase-UID bind (pre-auth$set/$set_oncestill applied there), the pending-identity-merge replay, and$process_person_profileenforcement are all untouched — so PostHog person profiles, the warehouse persons export, and ~2.5M desktop MAU metering are unaffected. Property freshness semantics shift from "immediate dedicated event" to "next captured event, worst-case session end"; last-write-wins for$setand first-write-wins for$set_onceare preserved.3.
comfy.desktop.app_update.error— 2.6M/28dA broken install re-fails the 10-minute auto-check forever. Identical error messages are now capped at 5 emits per app session (on top of the existing 1s identical-error repeat guard). Keyed on the message alone so operation/version churn can't defeat the cap; a NEW failure mode mid-session still ships its first occurrences. Since
*.errorevents intentionally bypass the SDK-level sliding-window guard, this call-site cap was the missing bound.4.
$exceptionrate-limiting — ~1.9M/mo, error tracking $340/mo93.9% of volume is four Electron crash-loop messages ("Comfy window renderer process exited (launch-failed)", "Renderer process gone: launch-failed", "Child process Utility exited: crashed", "Child process GPU exited: crashed"), with individual machines emitting thousands each.
captureExceptionnow caps identical scrubbed name+message pairs at 5 per session in the local capture wrapper (every$exceptionfunnels through it: process handlers, IPC handler, window attach). First occurrences always ship — this suppresses crash-loop repeats, not signal — and keying on the scrubbed message means PII variation (usernames in paths) can't split a loop into distinct keys. The Datadog mirror stays in sync because forwarding already gates on the capture being accepted.Out of scope
comfy.desktop.execution.*andcomfy.desktop.comfyui.model_usageare separate workstreams and untouched.Testing
vitest run— 224 files, 3700 passed / 1 skipped (18 new tests: client-level flag-event suppression, person-property carrier lifecycle incl. logout flush + scrub + consent revocation,$exceptionper-message cap incl. SDK-failure slot handling,app_update.errorsession cap under fake timers)pnpm typecheck(all four tsconfigs) andeslint .— clean (ran via pre-commit hook)🤖 Generated with Claude Code