feat: Add PostHog telemetry provider#9409
Merged
christian-byrne merged 1 commit intomainfrom Mar 6, 2026
Merged
Conversation
dosubot
bot
added
the
size:L
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new cloud PostHog telemetry provider, registers it in telemetry initialization, exposes PostHog config in types and remote config, adds the Changes
Sequence Diagram(s)sequenceDiagram
participant Registry as TelemetryRegistry
participant Remote as RemoteConfig
participant User as UserService
participant PH as PostHog SDK
Registry->>Remote: read posthog_project_token / posthog_api_host
alt token missing
Registry->>Registry: mark PostHog provider disabled
else token present
Registry->>PH: dynamic import & init(token, api_host)
PH-->>Registry: initialized
Registry->>User: subscribe onUserResolved
User-->>Registry: user resolved (id, props)
Registry->>PH: identify(user.id, user.props)
end
Registry->>Registry: track(event) (queues if PH not ready)
alt PH ready
Registry->>PH: capture event
else PH not ready
Registry->>Registry: enqueue event
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ✅ 549 passed, 0 failed · 5 flaky📊 Browser Reports
|
📦 Bundle: 4.49 MB gzip ⚪ 0 BDetailsSummary
Category Glance App Entry Points — 17.8 kB (baseline 17.8 kB) • ⚪ 0 BMain entry bundles and manifests
Graph Workspace — 920 kB (baseline 920 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Views & Navigation — 72.4 kB (baseline 72.4 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Panels & Settings — 436 kB (baseline 436 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
User & Accounts — 16.1 kB (baseline 16.1 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Editors & Dialogs — 779 B (baseline 779 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
UI Components — 47 kB (baseline 47 kB) • ⚪ 0 BReusable component library chunks
Data & Services — 2.73 MB (baseline 2.73 MB) • ⚪ 0 BStores, services, APIs, and repositories
Utilities & Hooks — 56.6 kB (baseline 56.6 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 8.86 MB (baseline 8.86 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.92 MB (baseline 7.92 MB) • ⚪ 0 BBundles that do not match a named category
|
robinjhuang
added
backport
⚡ Performance Report
Raw data{
"timestamp": "2026-03-05T06:30:29.526Z",
"gitSha": "3f9771160f6a2ddd991ef7a43ba905ce15b7ea08",
"branch": "feat/posthog-telemetry-provider",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2036.5400000000022,
"styleRecalcs": 125,
"styleRecalcDurationMs": 22.601,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 448.59700000000004,
"heapDeltaBytes": -1980544
},
{
"name": "canvas-idle",
"durationMs": 2049.24699999998,
"styleRecalcs": 126,
"styleRecalcDurationMs": 24.176,
"layouts": 1,
"layoutDurationMs": 0.26499999999999996,
"taskDurationMs": 407.47,
"heapDeltaBytes": -2223200
},
{
"name": "canvas-idle",
"durationMs": 2026.7590000000268,
"styleRecalcs": 123,
"styleRecalcDurationMs": 24.641,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 422.642,
"heapDeltaBytes": -2690176
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2041.5720000000022,
"styleRecalcs": 184,
"styleRecalcDurationMs": 53.248000000000005,
"layouts": 12,
"layoutDurationMs": 3.3950000000000005,
"taskDurationMs": 1004.405,
"heapDeltaBytes": -1743180
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2050.6559999999754,
"styleRecalcs": 181,
"styleRecalcDurationMs": 53.75600000000001,
"layouts": 12,
"layoutDurationMs": 3.1040000000000005,
"taskDurationMs": 1041.1709999999998,
"heapDeltaBytes": -3249792
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1859.6620000000144,
"styleRecalcs": 169,
"styleRecalcDurationMs": 51.467999999999996,
"layouts": 12,
"layoutDurationMs": 3.4040000000000004,
"taskDurationMs": 847.958,
"heapDeltaBytes": -2380452
},
{
"name": "dom-widget-clipping",
"durationMs": 585.5530000000044,
"styleRecalcs": 43,
"styleRecalcDurationMs": 12.623000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 358.627,
"heapDeltaBytes": 7563348
},
{
"name": "dom-widget-clipping",
"durationMs": 562.0710000000031,
"styleRecalcs": 42,
"styleRecalcDurationMs": 12.649000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 347.802,
"heapDeltaBytes": 7484896
},
{
"name": "dom-widget-clipping",
"durationMs": 613.9149999999631,
"styleRecalcs": 47,
"styleRecalcDurationMs": 17.403,
"layouts": 1,
"layoutDurationMs": 0.2850000000000001,
"taskDurationMs": 391.776,
"heapDeltaBytes": 8787960
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 597.6509999999848,
"styleRecalcs": 75,
"styleRecalcDurationMs": 19.446,
"layouts": 1,
"layoutDurationMs": 0.26699999999999996,
"taskDurationMs": 430.628,
"heapDeltaBytes": -8626128
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 580.2999999999656,
"styleRecalcs": 72,
"styleRecalcDurationMs": 15.466000000000001,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 417.79299999999995,
"heapDeltaBytes": -7724456
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 597.5799999999936,
"styleRecalcs": 73,
"styleRecalcDurationMs": 16.779000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 433.90400000000005,
"heapDeltaBytes": -7408840
},
{
"name": "subgraph-idle",
"durationMs": 2012.943000000007,
"styleRecalcs": 123,
"styleRecalcDurationMs": 24.332,
"layouts": 1,
"layoutDurationMs": 0.17199999999999996,
"taskDurationMs": 413.447,
"heapDeltaBytes": -1082888
},
{
"name": "subgraph-idle",
"durationMs": 1992.1370000000138,
"styleRecalcs": 120,
"styleRecalcDurationMs": 20.819999999999997,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 381.76500000000004,
"heapDeltaBytes": -1846816
},
{
"name": "subgraph-idle",
"durationMs": 2010.0040000000376,
"styleRecalcs": 122,
"styleRecalcDurationMs": 24.495000000000005,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 411.59599999999995,
"heapDeltaBytes": -1919120
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1677.1770000000004,
"styleRecalcs": 154,
"styleRecalcDurationMs": 44.598,
"layouts": 16,
"layoutDurationMs": 3.826,
"taskDurationMs": 712.765,
"heapDeltaBytes": -5741424
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1683.1830000000423,
"styleRecalcs": 154,
"styleRecalcDurationMs": 44.937000000000005,
"layouts": 16,
"layoutDurationMs": 4.1579999999999995,
"taskDurationMs": 721.743,
"heapDeltaBytes": -3988856
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1732.0359999999937,
"styleRecalcs": 156,
"styleRecalcDurationMs": 47.800000000000004,
"layouts": 16,
"layoutDurationMs": 4.039,
"taskDurationMs": 751.4119999999999,
"heapDeltaBytes": -3855896
}
]
} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
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/initTelemetry.ts (1)
23-35:⚠️ Potential issue | 🟠 MajorAvoid all-or-nothing provider initialization.
Line 29 uses
Promise.all(...); if the new PostHog import at Line 34 fails,
init rejects and Mixpanel/GTM/Impact are never registered. Isolate PostHog load
so existing providers still initialize.💡 Proposed fix
const [ { TelemetryRegistry }, { MixpanelTelemetryProvider }, { GtmTelemetryProvider }, - { ImpactTelemetryProvider }, - { PostHogTelemetryProvider } + { ImpactTelemetryProvider } ] = await Promise.all([ import('./TelemetryRegistry'), import('./providers/cloud/MixpanelTelemetryProvider'), import('./providers/cloud/GtmTelemetryProvider'), - import('./providers/cloud/ImpactTelemetryProvider'), - import('./providers/cloud/PostHogTelemetryProvider') + import('./providers/cloud/ImpactTelemetryProvider') ]) @@ registry.registerProvider(new MixpanelTelemetryProvider()) registry.registerProvider(new GtmTelemetryProvider()) registry.registerProvider(new ImpactTelemetryProvider()) - registry.registerProvider(new PostHogTelemetryProvider()) + try { + const { PostHogTelemetryProvider } = await import( + './providers/cloud/PostHogTelemetryProvider' + ) + registry.registerProvider(new PostHogTelemetryProvider()) + } catch (error) { + console.error('[Telemetry] Failed to load PostHog provider', error) + }As per coding guidelines, "Implement proper error propagation."
Also applies to: 41-41
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/telemetry/initTelemetry.ts` around lines 23 - 35, The current Promise.all import of TelemetryRegistry and providers (TelemetryRegistry, MixpanelTelemetryProvider, GtmTelemetryProvider, ImpactTelemetryProvider, PostHogTelemetryProvider) makes the whole init fail if one import (e.g., PostHogTelemetryProvider) throws; change to import TelemetryRegistry first then load provider modules individually using Promise.allSettled or try/catch per import so successful providers (MixpanelTelemetryProvider, GtmTelemetryProvider, ImpactTelemetryProvider) still get registered even if PostHog fails, and ensure errors from failed imports are logged via the existing telemetry init logging path without rethrowing to block initialization (follow "proper error propagation" guideline).
🧹 Nitpick comments (1)
src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts (1)
73-75: Bound the pending event queue to prevent unbounded growth.Lines 155 and 173 always enqueue when not initialized, and Line 73 has no cap.
A prolonged init/load issue can grow memory indefinitely.♻️ Proposed refactor
+const MAX_EVENT_QUEUE_SIZE = 500 + export class PostHogTelemetryProvider implements TelemetryProvider { private isEnabled = true private posthog: PostHog | null = null private eventQueue: QueuedEvent[] = [] @@ if (this.isInitialized && this.posthog) { try { this.posthog.capture(eventName, properties || {}) } catch (error) { console.error('Failed to track PostHog event:', error) } } else { + if (this.eventQueue.length >= MAX_EVENT_QUEUE_SIZE) { + this.eventQueue.shift() + } this.eventQueue.push(event) } } @@ if (this.isInitialized && this.posthog) { try { this.posthog.capture(eventName, properties || {}) } catch (error) { console.error('Failed to track PostHog event:', error) } } else { + if (this.eventQueue.length >= MAX_EVENT_QUEUE_SIZE) { + this.eventQueue.shift() + } this.eventQueue.push({ eventName, properties: properties as TelemetryEventProperties }) } }Also applies to: 154-156, 173-176
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts` around lines 73 - 75, The pending event queue eventQueue can grow unbounded while isInitialized is false; add a fixed cap (e.g. const MAX_PENDING_EVENTS = 1000) and centralize enqueue logic into a helper (e.g. enqueuePendingEvent(event: QueuedEvent)) to be used instead of direct pushes (replace places that push when !isInitialized). The helper should enforce the cap by dropping oldest entries or rejecting new ones (and log via the provider logger when a drop occurs), and ensure eventQueue remains a bounded array of QueuedEvent; update any code paths that directly push to eventQueue (the locations that currently enqueue when !isInitialized) to call this helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts`:
- Around line 269-275: The submitted-survey branch calls
this.posthog.people.set(...) without checking the telemetry disabled-events
config; modify the guard at the top of the block (the code handling stage ===
'submitted' with normalizedResponses && this.posthog) to first verify the
USER_SURVEY_SUBMITTED event is enabled (e.g. check this.config.disabledEvents
does not include 'USER_SURVEY_SUBMITTED' or call the provider
helper/isEventEnabled method) and only call
this.posthog.people.set(normalizedResponses) when the event is allowed; if
disabled, skip the people.set call entirely to respect opt-out.
---
Outside diff comments:
In `@src/platform/telemetry/initTelemetry.ts`:
- Around line 23-35: The current Promise.all import of TelemetryRegistry and
providers (TelemetryRegistry, MixpanelTelemetryProvider, GtmTelemetryProvider,
ImpactTelemetryProvider, PostHogTelemetryProvider) makes the whole init fail if
one import (e.g., PostHogTelemetryProvider) throws; change to import
TelemetryRegistry first then load provider modules individually using
Promise.allSettled or try/catch per import so successful providers
(MixpanelTelemetryProvider, GtmTelemetryProvider, ImpactTelemetryProvider) still
get registered even if PostHog fails, and ensure errors from failed imports are
logged via the existing telemetry init logging path without rethrowing to block
initialization (follow "proper error propagation" guideline).
---
Nitpick comments:
In `@src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts`:
- Around line 73-75: The pending event queue eventQueue can grow unbounded while
isInitialized is false; add a fixed cap (e.g. const MAX_PENDING_EVENTS = 1000)
and centralize enqueue logic into a helper (e.g. enqueuePendingEvent(event:
QueuedEvent)) to be used instead of direct pushes (replace places that push when
!isInitialized). The helper should enforce the cap by dropping oldest entries or
rejecting new ones (and log via the provider logger when a drop occurs), and
ensure eventQueue remains a bounded array of QueuedEvent; update any code paths
that directly push to eventQueue (the locations that currently enqueue when
!isInitialized) to call this helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4a1d7a47-bbac-4b7c-b636-0da8440d0ad6
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
global.d.tspackage.jsonpnpm-workspace.yamlscripts/vite-define-shim.tssrc/platform/telemetry/initTelemetry.tssrc/platform/telemetry/providers/cloud/PostHogTelemetryProvider.tsvite.config.mtsvitest.setup.ts
src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts
Outdated
Show resolved
Hide resolved
robinjhuang
force-pushed
the
feat/posthog-telemetry-provider
branch
from
2536397 to
5644b56
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts (1)
269-275:⚠️ Potential issue | 🟠 MajorRespect disabled-event config before
people.seton survey submit.Line 269 can still send survey-derived user properties even when
USER_SURVEY_SUBMITTEDis disabled. This bypasses the provider’s opt-out
control.💡 Proposed fix
- if (stage === 'submitted' && normalizedResponses && this.posthog) { + if ( + stage === 'submitted' && + normalizedResponses && + this.posthog && + this.isEnabled && + !this.disabledEvents.has(TelemetryEvents.USER_SURVEY_SUBMITTED) + ) { try { this.posthog.people.set(normalizedResponses) } catch (error) { console.error('Failed to set PostHog user properties:', error) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts` around lines 269 - 275, The posthog.people.set call in PostHogTelemetryProvider (inside the survey submit block) bypasses the provider's opt-out for USER_SURVEY_SUBMITTED; update the conditional to also check the provider's disabled-event config (the same check used for captures) before calling this.posthog.people.set so that people.set runs only when the USER_SURVEY_SUBMITTED event is enabled (e.g., use the existing disabled-events check or an isEventEnabled('USER_SURVEY_SUBMITTED') helper in the PostHogTelemetryProvider class to gate the call); keep the existing normalizedResponses and this.posthog null checks and preserve the try/catch around people.set.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts`:
- Around line 376-377: trackPageView currently drops caller-provided metadata
and therefore violates the TelemetryProvider interface; update the
PostHogTelemetryProvider.trackPageView signature to accept the optional
properties?: PageViewMetadata parameter, pass those properties through to
this.captureRaw when emitting TelemetryEvents.PAGE_VIEW, and ensure the method
signature and any related type imports match the TelemetryProvider contract so
page view metadata (e.g., path) is preserved the same way as in
GtmTelemetryProvider and ImpactTelemetryProvider.
---
Duplicate comments:
In `@src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts`:
- Around line 269-275: The posthog.people.set call in PostHogTelemetryProvider
(inside the survey submit block) bypasses the provider's opt-out for
USER_SURVEY_SUBMITTED; update the conditional to also check the provider's
disabled-event config (the same check used for captures) before calling
this.posthog.people.set so that people.set runs only when the
USER_SURVEY_SUBMITTED event is enabled (e.g., use the existing disabled-events
check or an isEventEnabled('USER_SURVEY_SUBMITTED') helper in the
PostHogTelemetryProvider class to gate the call); keep the existing
normalizedResponses and this.posthog null checks and preserve the try/catch
around people.set.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 54318a55-5fd5-4e86-ba83-261a572a64a7
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
global.d.tspackage.jsonpnpm-workspace.yamlsrc/platform/remoteConfig/types.tssrc/platform/telemetry/initTelemetry.tssrc/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/platform/telemetry/initTelemetry.ts
src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts
Outdated
Show resolved
Hide resolved
robinjhuang
force-pushed
the
feat/posthog-telemetry-provider
branch
from
5644b56 to
8ebc719
Compare
dosubot
bot
added
size:XL
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts (2)
376-377:⚠️ Potential issue | 🟠 MajorPreserve page-view metadata in
trackPageView.
At Line 376, the method drops optional metadata passed by callers and diverges
from the broader telemetry contract behavior.💡 Proposed fix
import type { AuthMetadata, EnterLinearMetadata, ExecutionErrorMetadata, ExecutionSuccessMetadata, HelpCenterClosedMetadata, HelpCenterOpenedMetadata, HelpResourceClickedMetadata, NodeSearchMetadata, NodeSearchResultMetadata, + PageViewMetadata, PageVisibilityMetadata, SettingChangedMetadata, SubscriptionMetadata, SurveyResponses, @@ - trackPageView(pageName: string): void { - this.captureRaw(TelemetryEvents.PAGE_VIEW, { page_name: pageName }) + trackPageView(pageName: string, properties?: PageViewMetadata): void { + this.captureRaw(TelemetryEvents.PAGE_VIEW, { + page_name: pageName, + ...(properties ?? {}) + }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts` around lines 376 - 377, The trackPageView method currently only accepts pageName and calls this.captureRaw(TelemetryEvents.PAGE_VIEW, { page_name: pageName }), dropping optional metadata; update trackPageView to accept an optional metadata parameter (e.g., metadata?: Record<string, any>) and merge it with the page_name field before calling this.captureRaw so that additional caller-provided page-view properties are preserved; ensure the signature change and the merge happen in PostHogTelemetryProvider.trackPageView and that the merged object is passed to this.captureRaw with TelemetryEvents.PAGE_VIEW.
269-275:⚠️ Potential issue | 🟠 MajorRespect disabled-event opt-out before calling
people.set.
At Line 269, survey profile updates are still sent even when
USER_SURVEY_SUBMITTEDis disabled, which bypasses telemetry opt-out.💡 Proposed fix
- if (stage === 'submitted' && normalizedResponses && this.posthog) { + if ( + stage === 'submitted' && + normalizedResponses && + this.posthog && + this.isEnabled && + !this.disabledEvents.has(TelemetryEvents.USER_SURVEY_SUBMITTED) + ) { try { this.posthog.people.set(normalizedResponses) } catch (error) { console.error('Failed to set PostHog user properties:', error) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts` around lines 269 - 275, The survey user-profile update block currently calls this.posthog.people.set(...) regardless of the USER_SURVEY_SUBMITTED opt-out; change it to first check the telemetry opt-out setting and only call people.set when the event is enabled (e.g. verify a flag or method like isEventEnabled('USER_SURVEY_SUBMITTED') on your telemetry/config object), and skip the set (and any related work) when the event is disabled; ensure you still guard on this.posthog, stage === 'submitted', and normalizedResponses before invoking people.set and keep the existing try/catch error logging around the call.
🧹 Nitpick comments (1)
src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.test.ts (1)
219-230: Add a regression case for page-view metadata passthrough.
Current test only assertspage_name; it won’t catch dropped optional metadata
(e.g.path) in provider implementations.💡 Suggested test addition
describe('page view', () => { it('captures page view with page_name property', async () => { const provider = createProvider() await vi.dynamicImportSettled() provider.trackPageView('workflow_editor') expect(hoisted.mockCapture).toHaveBeenCalledWith( TelemetryEvents.PAGE_VIEW, { page_name: 'workflow_editor' } ) }) + + it('captures page view with additional metadata', async () => { + const provider = createProvider() + await vi.dynamicImportSettled() + + provider.trackPageView('workflow_editor', { path: 'https://app.test/wf' }) + + expect(hoisted.mockCapture).toHaveBeenCalledWith( + TelemetryEvents.PAGE_VIEW, + { page_name: 'workflow_editor', path: 'https://app.test/wf' } + ) + }) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.test.ts` around lines 219 - 230, The test in PostHogTelemetryProvider.test.ts only asserts page_name and misses optional metadata; update the 'page view' spec that calls provider.trackPageView to include optional metadata (e.g., pass { path: '/some/path' } or combine with page_name) and add an expectation that hoisted.mockCapture was called with TelemetryEvents.PAGE_VIEW and the full metadata object (ensuring that provider.trackPageView and the provider implementation pass through optional fields like path). Target the test that invokes provider.trackPageView and the assertion using hoisted.mockCapture/TelmetryEvents.PAGE_VIEW to verify metadata passthrough.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts`:
- Around line 376-377: The trackPageView method currently only accepts pageName
and calls this.captureRaw(TelemetryEvents.PAGE_VIEW, { page_name: pageName }),
dropping optional metadata; update trackPageView to accept an optional metadata
parameter (e.g., metadata?: Record<string, any>) and merge it with the page_name
field before calling this.captureRaw so that additional caller-provided
page-view properties are preserved; ensure the signature change and the merge
happen in PostHogTelemetryProvider.trackPageView and that the merged object is
passed to this.captureRaw with TelemetryEvents.PAGE_VIEW.
- Around line 269-275: The survey user-profile update block currently calls
this.posthog.people.set(...) regardless of the USER_SURVEY_SUBMITTED opt-out;
change it to first check the telemetry opt-out setting and only call people.set
when the event is enabled (e.g. verify a flag or method like
isEventEnabled('USER_SURVEY_SUBMITTED') on your telemetry/config object), and
skip the set (and any related work) when the event is disabled; ensure you still
guard on this.posthog, stage === 'submitted', and normalizedResponses before
invoking people.set and keep the existing try/catch error logging around the
call.
---
Nitpick comments:
In `@src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.test.ts`:
- Around line 219-230: The test in PostHogTelemetryProvider.test.ts only asserts
page_name and misses optional metadata; update the 'page view' spec that calls
provider.trackPageView to include optional metadata (e.g., pass { path:
'/some/path' } or combine with page_name) and add an expectation that
hoisted.mockCapture was called with TelemetryEvents.PAGE_VIEW and the full
metadata object (ensuring that provider.trackPageView and the provider
implementation pass through optional fields like path). Target the test that
invokes provider.trackPageView and the assertion using
hoisted.mockCapture/TelmetryEvents.PAGE_VIEW to verify metadata passthrough.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 92f96981-f4b3-4add-9b6c-7b05ec9d6100
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
global.d.tspackage.jsonpnpm-workspace.yamlsrc/platform/remoteConfig/types.tssrc/platform/telemetry/initTelemetry.tssrc/platform/telemetry/providers/cloud/PostHogTelemetryProvider.test.tssrc/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- pnpm-workspace.yaml
- src/platform/remoteConfig/types.ts
- global.d.ts
robinjhuang
force-pushed
the
feat/posthog-telemetry-provider
branch
from
8ebc719 to
a6b1b3e
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts (1)
77-87: Minor: RedundantconfigureDisabledEventscall on initialization.
configureDisabledEventsis called explicitly on lines 78-80, then the watch with{ immediate: true }calls it again immediately on line 84. Consider removing the explicit call since the immediate watch handles it.Proposed fix
constructor() { - this.configureDisabledEvents( - (window.__CONFIG__ as Partial<RemoteConfig> | undefined) ?? null - ) watch( remoteConfig, (config) => { this.configureDisabledEvents(config) }, { immediate: true } )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts` around lines 77 - 87, In the constructor of PostHogTelemetryProvider remove the redundant explicit call to configureDisabledEvents((window.__CONFIG__ as Partial<RemoteConfig> | undefined) ?? null) and rely on the existing watch(remoteConfig, (config) => { this.configureDisabledEvents(config) }, { immediate: true }) to invoke configureDisabledEvents on init; leave the watch and its immediate option unchanged so initial and subsequent config updates still call configureDisabledEvents.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts`:
- Around line 77-87: In the constructor of PostHogTelemetryProvider remove the
redundant explicit call to configureDisabledEvents((window.__CONFIG__ as
Partial<RemoteConfig> | undefined) ?? null) and rely on the existing
watch(remoteConfig, (config) => { this.configureDisabledEvents(config) }, {
immediate: true }) to invoke configureDisabledEvents on init; leave the watch
and its immediate option unchanged so initial and subsequent config updates
still call configureDisabledEvents.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a97c9d6f-4b33-41b6-a45e-7a1b6120f6d4
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
.github/workflows/ci-oss-assets-validation.yamlglobal.d.tspackage.jsonpnpm-workspace.yamlsrc/platform/remoteConfig/types.tssrc/platform/telemetry/initTelemetry.tssrc/platform/telemetry/providers/cloud/PostHogTelemetryProvider.test.tssrc/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- global.d.ts
- package.json
- src/platform/telemetry/initTelemetry.ts
- pnpm-workspace.yaml
- src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.test.ts
- src/platform/remoteConfig/types.ts
Member
Author
|
@benceruleanlu is this a flaky test? Or a bug I introduced 
|
Member
Flaky. Snapshot tests in FE have been the biggest piece of tech debt for months. |
Contributor
christian-byrne
left a comment
christian-byrne
left a comment
There was a problem hiding this comment.
Looks good, can you add a query term to ensure it never gets included in the OSS distribution? Just 1-2 lines in this file https://github.com/Comfy-Org/ComfyUI_frontend/blob/main/.github/workflows/ci-dist-telemetry-scan.yaml.
robinjhuang
force-pushed
the
feat/posthog-telemetry-provider
branch
from
a6b1b3e to
30bfa65
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts (2)
269-275:⚠️ Potential issue | 🟠 MajorGuard survey profile updates behind the disabled-event check.
At Line 269,
people.set(...)still runs for submitted surveys even whenUSER_SURVEY_SUBMITTEDis disabled, which bypasses telemetry opt-out for that event.💡 Proposed fix
- if (stage === 'submitted' && normalizedResponses && this.posthog) { + if ( + stage === 'submitted' && + normalizedResponses && + this.posthog && + this.isEnabled && + !this.disabledEvents.has(TelemetryEvents.USER_SURVEY_SUBMITTED) + ) { try { this.posthog.people.set(normalizedResponses) } catch (error) { console.error('Failed to set PostHog user properties:', error) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts` around lines 269 - 275, When handling survey submission in PostHogTelemetryProvider, ensure you only call this.posthog.people.set(normalizedResponses) for stage === 'submitted' if the USER_SURVEY_SUBMITTED event is enabled; add the same disabled-event check used elsewhere (the feature/telemetry event enabling guard) before invoking this.posthog.people.set to honor telemetry opt-out for submitted surveys and leave normalizedResponses and this.posthog null-checks intact.
376-377:⚠️ Potential issue | 🟠 MajorPreserve caller-provided page-view metadata.
At Line 376,
trackPageViewdrops optional metadata passed by callers and only forwardspage_name, so fields likepathare lost.💡 Proposed fix
import type { AuthMetadata, EnterLinearMetadata, ExecutionErrorMetadata, ExecutionSuccessMetadata, HelpCenterClosedMetadata, HelpCenterOpenedMetadata, HelpResourceClickedMetadata, NodeSearchMetadata, NodeSearchResultMetadata, + PageViewMetadata, PageVisibilityMetadata, SettingChangedMetadata, SubscriptionMetadata, SurveyResponses, TabCountMetadata, @@ - trackPageView(pageName: string): void { - this.captureRaw(TelemetryEvents.PAGE_VIEW, { page_name: pageName }) + trackPageView(pageName: string, properties?: PageViewMetadata): void { + this.captureRaw(TelemetryEvents.PAGE_VIEW, { + page_name: pageName, + ...(properties ?? {}) + }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts` around lines 376 - 377, trackPageView currently overwrites any caller-provided metadata by only sending { page_name: pageName } to captureRaw; update the trackPageView method to accept an optional metadata object parameter and merge the caller metadata with { page_name: pageName } before calling this.captureRaw(TelemetryEvents.PAGE_VIEW,...), so fields like path are preserved; locate the trackPageView function in PostHogTelemetryProvider and ensure the merged payload is passed to captureRaw rather than replacing caller metadata.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts`:
- Around line 269-275: When handling survey submission in
PostHogTelemetryProvider, ensure you only call
this.posthog.people.set(normalizedResponses) for stage === 'submitted' if the
USER_SURVEY_SUBMITTED event is enabled; add the same disabled-event check used
elsewhere (the feature/telemetry event enabling guard) before invoking
this.posthog.people.set to honor telemetry opt-out for submitted surveys and
leave normalizedResponses and this.posthog null-checks intact.
- Around line 376-377: trackPageView currently overwrites any caller-provided
metadata by only sending { page_name: pageName } to captureRaw; update the
trackPageView method to accept an optional metadata object parameter and merge
the caller metadata with { page_name: pageName } before calling
this.captureRaw(TelemetryEvents.PAGE_VIEW,...), so fields like path are
preserved; locate the trackPageView function in PostHogTelemetryProvider and
ensure the merged payload is passed to captureRaw rather than replacing caller
metadata.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 54f5a361-e8ef-40f6-9296-c3081edfcf67
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.github/workflows/ci-dist-telemetry-scan.yaml.github/workflows/ci-oss-assets-validation.yamlglobal.d.tspackage.jsonpnpm-workspace.yamlsrc/platform/remoteConfig/types.tssrc/platform/telemetry/initTelemetry.tssrc/platform/telemetry/providers/cloud/PostHogTelemetryProvider.test.tssrc/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/platform/remoteConfig/types.ts
- pnpm-workspace.yaml
- package.json
- global.d.ts
- .github/workflows/ci-oss-assets-validation.yaml
src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts
Outdated
Show resolved
Hide resolved
src/platform/telemetry/providers/cloud/PostHogTelemetryProvider.ts
Outdated
Show resolved
Hide resolved
robinjhuang
force-pushed
the
feat/posthog-telemetry-provider
branch
from
30bfa65 to
6b2d886
Compare
- Add posthog-js dependency - Create PostHogTelemetryProvider following MixpanelTelemetryProvider pattern - Register provider in initTelemetry.ts - Read posthog_project_token and posthog_api_host from window.__CONFIG__ (runtime config from /features endpoint) - Extract getExecutionContext to shared utility used by both Mixpanel and PostHog - Add PostHog to dist telemetry scan and license clarifications - Tree-shaken away in OSS builds (verified)
robinjhuang
force-pushed
the
feat/posthog-telemetry-provider
branch
from
6b2d886 to
7aff5f3
Compare
robinjhuang
added
the
needs-backport
|
robinjhuang
added a commit
that referenced
this pull request
Mar 6, 2026
Backport of #9409 to cloud/1.40.
christian-byrne
pushed a commit
that referenced
this pull request
Mar 6, 2026
Backport of #9409 to `cloud/1.40`. Automatically created by manual backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9457-backport-cloud-1-40-feat-Add-PostHog-telemetry-provider-31b6d73d365081ac9eb4d596daafddf3) by [Unito](https://www.unito.io)
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
3 participants
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.
Add PostHog as a telemetry provider for cloud builds so custom events can be correlated with session recordings. Follows the same pattern as MixpanelTelemetryProvider with dynamic import, event queuing, and disabled events from remote config. Tree-shaken away in OSS builds.
The posthog-js package uses Apache-2.0 (verified from its LICENSE file) but declares it as "SEE LICENSE IN LICENSE" in package.json, which
the license checker can't parse.
┆Issue is synchronized with this Notion page by Unito