mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-07-08 08:08:04 +00:00
feat(telemetry): register client + deployment platform axes on PostHog (#13469)
## Problem Filtering PostHog for the three product surfaces — **desktop local**, **desktop cloud**, **web cloud** — currently requires a different hack per pipe. For this repo's cloud build, the desktop-embedded frontend and a plain browser are indistinguishable except by sniffing `Electron` in `$raw_user_agent` (~124K desktop-cloud vs ~844K web-cloud execution events/week get separated that way today). ## Change Register the two standardized platform axes as PostHog super properties at SDK init in `PostHogTelemetryProvider`: - **`client`** — which surface emitted the event: `'desktop'` when the desktop preload bridge (`window.__comfyDesktop2`) is present, else `'web'`. The bridge is injected by Electron before any page script runs, so detection is deterministic — unlike the existing utm-based `source_app` attribution, which only covers sessions that *entered* via a desktop link. - **`deployment`** — which backend runs the work: pinned to `'cloud'`. The register happens before the pre-init event queue flushes, so events captured during the posthog-js dynamic-import window carry the axes too. ## Why pinning `deployment: 'cloud'` is safe (including embedded-in-desktop) The cloud bundle also runs **embedded in Comfy Desktop** — a cloud install loads this same bundle in Electron, where `isCloud` and the host bridge are both true. Two things happen there: 1. `main.ts` runs `initHostTelemetry()` *after* `initTelemetry()`, and (when remote config `enable_telemetry` is on) it **replaces** the registry with `HostTelemetrySink` — so tracked events (`execution_start`, …) route through the desktop main process, bypassing this provider. Those are tagged the same `client`/`deployment` values main-side from the install's source category ([Comfy-Desktop#1229](https://github.com/Comfy-Org/Comfy-Desktop/pull/1229)). 2. posthog-js keeps capturing independently of the registry (pageviews, web vitals, identify) — those are what these super properties cover in the embedded case, and `deployment: 'cloud'` is correct for them because the cloud bundle always talks to the cloud backend regardless of embedding; the embedding itself is what `client: 'desktop'` captures. The only way a cloud build runs against a non-cloud backend is a dev setup, where `window.__CONFIG__.posthog_project_token` is absent (injected by the cloud server) and the provider disables itself before registering anything. The locally-served frontend (desktop/localhost builds) never runs this provider: `__DISTRIBUTION__` is a compile-time define, so the `initTelemetry()` call folds away, with a runtime `IS_CLOUD_BUILD` guard as backstop. With both PRs, the three platforms become clean property filters: | Surface | Filter | |---|---| | Desktop local | `client=desktop, deployment=local` | | Desktop cloud | `client=desktop, deployment=cloud` | | Web cloud | `client=web, deployment=cloud` | ## Testing - `vitest run` on `PostHogTelemetryProvider.test.ts` — 45 passing, including new coverage: web default, bridge-present → `client=desktop`, and register-before-queue-flush ordering. The two desktop-entry tests that asserted `register` is never called were narrowed to assert no `source_app` register call. - `pnpm typecheck` + eslint/oxlint on touched files — clean. Ref [MAR-51](https://linear.app/comfyorg/issue/MAR-51/foundation-desktop-sdk-dual-send-to-posthog-alongside-mixpanel) --------- Co-authored-by: AustinMroz <austin@comfy.org>
This commit is contained in:
@@ -195,6 +195,46 @@ describe('PostHogTelemetryProvider', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('platform axes (client / deployment)', () => {
|
||||
afterEach(() => {
|
||||
delete window.__comfyDesktop2
|
||||
})
|
||||
|
||||
it('registers client=web and deployment=cloud in a plain browser', async () => {
|
||||
createProvider()
|
||||
await vi.dynamicImportSettled()
|
||||
|
||||
expect(hoisted.mockRegister).toHaveBeenCalledWith({
|
||||
client: 'web',
|
||||
deployment: 'cloud'
|
||||
})
|
||||
})
|
||||
|
||||
it('registers client=desktop when the desktop preload bridge is present', async () => {
|
||||
window.__comfyDesktop2 = {
|
||||
isRemote: () => false,
|
||||
Telemetry: { capture: vi.fn() }
|
||||
}
|
||||
createProvider()
|
||||
await vi.dynamicImportSettled()
|
||||
|
||||
expect(hoisted.mockRegister).toHaveBeenCalledWith({
|
||||
client: 'desktop',
|
||||
deployment: 'cloud'
|
||||
})
|
||||
})
|
||||
|
||||
it('registers platform axes before flushing pre-init queued events', async () => {
|
||||
const provider = createProvider()
|
||||
provider.trackSignupOpened()
|
||||
await vi.dynamicImportSettled()
|
||||
|
||||
const registerOrder = hoisted.mockRegister.mock.invocationCallOrder[0]
|
||||
const captureOrder = hoisted.mockCapture.mock.invocationCallOrder[0]
|
||||
expect(registerOrder).toBeLessThan(captureOrder)
|
||||
})
|
||||
})
|
||||
|
||||
describe('desktop entry capture', () => {
|
||||
function setLocation(search: string): void {
|
||||
Object.defineProperty(window.location, 'search', {
|
||||
@@ -208,12 +248,20 @@ describe('PostHogTelemetryProvider', () => {
|
||||
setLocation('')
|
||||
})
|
||||
|
||||
// The platform-axes register (client/deployment) always fires, so these
|
||||
// assert no register call carrying desktop-entry attribution props.
|
||||
function desktopEntryRegisterCalls(): unknown[][] {
|
||||
return hoisted.mockRegister.mock.calls.filter(
|
||||
([props]) => props && 'source_app' in (props as Record<string, unknown>)
|
||||
)
|
||||
}
|
||||
|
||||
it('does not register desktop props when utm_source is absent', async () => {
|
||||
setLocation('')
|
||||
createProvider()
|
||||
await vi.dynamicImportSettled()
|
||||
|
||||
expect(hoisted.mockRegister).not.toHaveBeenCalled()
|
||||
expect(desktopEntryRegisterCalls()).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('does not register desktop props when utm_source is not comfy.desktop', async () => {
|
||||
@@ -221,7 +269,7 @@ describe('PostHogTelemetryProvider', () => {
|
||||
createProvider()
|
||||
await vi.dynamicImportSettled()
|
||||
|
||||
expect(hoisted.mockRegister).not.toHaveBeenCalled()
|
||||
expect(desktopEntryRegisterCalls()).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('registers source_app and desktop_device_id when arriving from desktop', async () => {
|
||||
|
||||
@@ -143,6 +143,9 @@ export class PostHogTelemetryProvider implements TelemetryProvider {
|
||||
before_send: createPostHogBeforeSend()
|
||||
})
|
||||
this.isInitialized = true
|
||||
// Before flushEventQueue so pre-init events also carry the
|
||||
// platform super properties.
|
||||
this.registerPlatformProps()
|
||||
this.flushEventQueue()
|
||||
this.registerDesktopEntryProps()
|
||||
|
||||
@@ -285,6 +288,18 @@ export class PostHogTelemetryProvider implements TelemetryProvider {
|
||||
)
|
||||
}
|
||||
|
||||
private registerPlatformProps(): void {
|
||||
if (!this.posthog) return
|
||||
try {
|
||||
this.posthog.register({
|
||||
client: window.__comfyDesktop2 ? 'desktop' : 'web',
|
||||
deployment: 'cloud'
|
||||
})
|
||||
} catch (error) {
|
||||
console.error('Failed to register platform props:', error)
|
||||
}
|
||||
}
|
||||
|
||||
private registerDesktopEntryProps(): void {
|
||||
if (!this.posthog) return
|
||||
const props = readDesktopEntryProps()
|
||||
|
||||
Reference in New Issue
Block a user