Add telemetry sampling for non-Pro users#2642
Conversation
- Store Dyad Pro status in localStorage for telemetry access - Sample 10% of events for non-Pro users (always send errors) - Pro users continue to send all events Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
wwwillchen-bot
temporarily deployed
to
ai-bots
GitHub Actions
Inactive
wwwillchen-bot
temporarily deployed
to
ai-bots
GitHub Actions
Inactive
|
@BugBot run |
Summary of ChangesHello @wwwillchen-bot, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new telemetry sampling strategy to optimize data collection. It differentiates between Pro and non-Pro users, significantly reducing the volume of telemetry data sent by non-Pro users while ensuring that all critical error events are still captured. This change aims to manage data ingestion more efficiently without compromising the ability to identify and address issues. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces telemetry sampling for non-Pro users, sending only 10% of events while ensuring all error events are still reported. The implementation correctly uses localStorage to make the user's Pro status available to the telemetry module for sampling decisions.
My review found one significant issue with the sampling logic. It would incorrectly sample critical PostHog events like $identify and $pageview, which would corrupt analytics data for non-Pro users. I've provided a suggestion to ensure these events are always sent, similar to how error events are handled, to maintain data integrity.
| const isErrorEvent = | ||
| event?.event === "$exception" || | ||
| event?.event?.toLowerCase().includes("error") || | ||
| event?.properties?.$exception_type || | ||
| event?.properties?.error; | ||
|
|
||
| if (!isErrorEvent && Math.random() > 0.1) { | ||
| console.debug("Non-Pro user: sampling out event", event?.event); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
The current sampling logic for non-Pro users will also sample crucial PostHog events like $identify and $pageview, which can lead to data integrity issues.
$identifyevents: Sampling these will cause user identification to fail for 90% of non-Pro user sessions. This makes it impossible to reliably associate other telemetry events with a specific user, skewing all user-centric analytics.$pageviewevents: Sampling these will make user flow analysis (like funnels and paths) unreliable for this user segment, as most navigation actions won't be recorded.
These special events should always be sent, just like error events, to ensure the collected data is accurate and useful. The suggested change updates the logic to prevent these events from being sampled.
const shouldAlwaysSend =
event?.event === "$exception" ||
event?.event === "$identify" ||
event?.event === "$pageview" ||
event?.event?.toLowerCase().includes("error") ||
event?.properties?.$exception_type ||
event?.properties?.error;
if (!shouldAlwaysSend && Math.random() > 0.1) {
console.debug("Non-Pro user: sampling out event", event?.event);
return null;
}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/renderer.tsx">
<violation number="1" location="src/renderer.tsx:98">
P2: The optional chaining stops before `.includes`, so `event?.event?.toLowerCase()` can return `undefined` and then `.includes()` throws. Guard the string check (or optional-chain the result) before calling `.includes()` to avoid runtime errors in telemetry sampling.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if (!isDyadProUser()) { | ||
| const isErrorEvent = | ||
| event?.event === "$exception" || | ||
| event?.event?.toLowerCase().includes("error") || |
There was a problem hiding this comment.
P2: The optional chaining stops before .includes, so event?.event?.toLowerCase() can return undefined and then .includes() throws. Guard the string check (or optional-chain the result) before calling .includes() to avoid runtime errors in telemetry sampling.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/renderer.tsx, line 98:
<comment>The optional chaining stops before `.includes`, so `event?.event?.toLowerCase()` can return `undefined` and then `.includes()` throws. Guard the string check (or optional-chain the result) before calling `.includes()` to avoid runtime errors in telemetry sampling.</comment>
<file context>
@@ -87,6 +91,20 @@ const posthogClient = posthog.init(
+ if (!isDyadProUser()) {
+ const isErrorEvent =
+ event?.event === "$exception" ||
+ event?.event?.toLowerCase().includes("error") ||
+ event?.properties?.$exception_type ||
+ event?.properties?.error;
</file context>
| event?.event?.toLowerCase().includes("error") || | |
| (typeof event?.event === "string" && | |
| event.event.toLowerCase().includes("error")) || |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| if (!isErrorEvent && Math.random() > 0.1) { | ||
| console.debug("Non-Pro user: sampling out event", event?.event); | ||
| return null; |
There was a problem hiding this comment.
🟡 MEDIUM | race-condition | Found by: Correctness, Endorsed by: Code Health, UX
Pro status in localStorage may be stale at PostHog init time
isDyadProUser() reads from localStorage, but processSettingsForTelemetry() (which writes the Pro status) is called from a React hook. PostHog is initialized at module level before React renders. On first app launch (or after clearing storage), the Pro status won't be in localStorage yet, so all users will be treated as non-Pro until settings are fetched and processed. This means some early startup events could be incorrectly sampled for Pro users.
💡 Suggestion: Add a comment documenting this known timing gap, e.g. // Note: on first launch, Pro status may not be in localStorage yet. This is acceptable since it only affects telemetry sampling during the brief startup window.
| event?.event?.toLowerCase().includes("error") || | ||
| event?.properties?.$exception_type || | ||
| event?.properties?.error; | ||
|
|
There was a problem hiding this comment.
🟡 MEDIUM | complexity | Found by: Code Health, Endorsed by: Correctness, UX
Magic number for sampling rate
The sampling rate 0.1 (10%) is a magic number embedded in the callback. If this rate needs tuning, someone has to hunt through PostHog init config to find it.
💡 Suggestion: Extract to a named constant, e.g. const NON_PRO_TELEMETRY_SAMPLE_RATE = 0.1; defined near the top of the file.
🔍 Dyadbot Code Review SummaryVerdict: ✅ YES - Ready to merge Reviewed by 3 specialized agents: Correctness Expert, Code Health Expert, UX Wizard. Issues Summary
🟢 Low Priority Notes (3 items)
🚫 Dropped Issues (0 items)No issues were dropped during discussion. Generated by Dyadbot swarm code review |
Greptile OverviewGreptile SummaryThis PR implements telemetry sampling for non-Pro users to reduce PostHog event volume. Pro status is now persisted to Key changes:
Notes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant App as App Startup
participant Settings as useSettings Hook
participant LocalStorage as localStorage
participant PostHog as PostHog before_send
App->>Settings: Load user settings
Settings->>Settings: Call processSettingsForTelemetry()
Settings->>LocalStorage: Store Pro status (hasDyadProKey)
Note over PostHog: Event triggered
PostHog->>LocalStorage: isDyadProUser() reads Pro status
alt Non-Pro User
PostHog->>PostHog: Check if error event
alt Error Event
PostHog->>PostHog: Always send
else Non-Error Event
PostHog->>PostHog: Sample 10% (Math.random)
alt Sampled Out
PostHog->>PostHog: Return null (drop event)
else Sampled In
PostHog->>PostHog: Send event
end
end
else Pro User
PostHog->>PostHog: Send all events
end
|
github-actions
Bot
added
the
needs-human:review-issue
🎭 Playwright Test Results
|
| OS | Passed | Failed | Flaky | Skipped |
|---|---|---|---|---|
| 🍎 macOS | 222 | 4 | 12 | 6 |
Summary: 222 passed, 4 failed, 12 flaky, 6 skipped
Failed Tests
🍎 macOS
capacitor.spec.ts > capacitor upgrade and sync works- TimeoutError: locator.click: Timeout 30000ms exceeded.
security_review.spec.ts > security review - edit and use knowledge- Error: expect(string).toMatchSnapshot(expected) failed
select_component.spec.ts > select component next.js- Error: expect(locator).toMatchAriaSnapshot(expected) failed
template-create-nextjs.spec.ts > create next.js app- Error: expect(locator).toMatchAriaSnapshot(expected) failed
📋 Re-run Failing Tests (macOS)
Copy and paste to re-run all failing spec files locally:
npm run e2e \
e2e-tests/capacitor.spec.ts \
e2e-tests/security_review.spec.ts \
e2e-tests/select_component.spec.ts \
e2e-tests/template-create-nextjs.spec.ts⚠️ Flaky Tests
🍎 macOS
annotator.spec.ts > annotator - capture and submit screenshot(passed after 1 retry)approve.spec.ts > write to index, approve, check preview(passed after 1 retry)context_manage.spec.ts > manage context - exclude paths with smart context(passed after 1 retry)debugging_logs.spec.ts > console logs should appear in the console(passed after 1 retry)import.spec.ts > import app(passed after 1 retry)local_agent_advanced.spec.ts > local-agent - security review fix(passed after 1 retry)local_agent_ask.spec.ts > local-agent ask mode(passed after 1 retry)refresh.spec.ts > refresh preserves current route(passed after 1 retry)setup_flow.spec.ts > Setup Flow > setup banner shows correct state when node.js is installed(passed after 1 retry)setup.spec.ts > setup ai provider(passed after 1 retry)- ... and 2 more
## Summary - Store Dyad Pro status in localStorage for telemetry access - Sample 10% of events for non-Pro users (error events always sent) - Pro users continue to send all events ## Test plan - Verify telemetry events are sampled correctly for non-Pro users - Confirm error events are always sent regardless of Pro status - Test Pro users still send all events 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/dyad-sh/dyad/pull/2642" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a> <!-- devin-review-badge-end --> <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Adds telemetry sampling for non‑Pro users to reduce event volume while keeping error visibility. Stores Dyad Pro status in localStorage; Pro users keep sending all events. - **New Features** - Save Dyad Pro status in localStorage (dyadProStatus) and expose isDyadProUser(). - Sample PostHog events for non‑Pro: send 10% of non‑error events; always send errors. - No change for Pro users; all events continue to be sent. <sup>Written for commit 685922c. Summary will update on new commits.</sup> <!-- End of auto-generated description by cubic. --> Co-authored-by: Will Chen <willchen90@gmail.com> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
## Summary - Store Dyad Pro status in localStorage for telemetry access - Sample 10% of events for non-Pro users (error events always sent) - Pro users continue to send all events ## Test plan - Verify telemetry events are sampled correctly for non-Pro users - Confirm error events are always sent regardless of Pro status - Test Pro users still send all events 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/dyad-sh/dyad/pull/2642" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a> <!-- devin-review-badge-end --> <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Adds telemetry sampling for non‑Pro users to reduce event volume while keeping error visibility. Stores Dyad Pro status in localStorage; Pro users keep sending all events. - **New Features** - Save Dyad Pro status in localStorage (dyadProStatus) and expose isDyadProUser(). - Sample PostHog events for non‑Pro: send 10% of non‑error events; always send errors. - No change for Pro users; all events continue to be sent. <sup>Written for commit 685922c. Summary will update on new commits.</sup> <!-- End of auto-generated description by cubic. --> Co-authored-by: Will Chen <willchen90@gmail.com> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
2 participants
Summary
Test plan
🤖 Generated with Claude Code
Summary by cubic
Adds telemetry sampling for non‑Pro users to reduce event volume while keeping error visibility. Stores Dyad Pro status in localStorage; Pro users keep sending all events.
Written for commit 685922c. Summary will update on new commits.