feat: use email as main tracking field - #2700
Merged
thisisnithin merged 24 commits intoApr 15, 2026
Merged
thisisnithin merged 24 commits into
thisisnithin merged 24 commits into
Conversation
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:
WalkthroughCLI telemetry now returns a structured TelemetryIdentity from apiClient.platform.whoAmI() (falling back to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
This comment has been minimized.
This comment has been minimized.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2700 +/- ##
==========================================
+ Coverage 1.59% 40.69% +39.10%
==========================================
Files 296 999 +703
Lines 41501 124268 +82767
Branches 432 5589 +5157
==========================================
+ Hits 662 50573 +49911
- Misses 40553 71956 +31403
- Partials 286 1739 +1453
🚀 New features to boost your workflow:
|
![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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/src/core/telemetry.ts (1)
5-5:⚠️ Potential issue | 🟡 MinorRemove the unused
getLoginDetailsimport.The
getLoginDetailsfunction is imported on line 5 but is not used anywhere in this file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/core/telemetry.ts` at line 5, Remove the unused import getLoginDetails from the import statement that currently reads "import { config, getBaseHeaders, getLoginDetails } ..." — edit the import to only include the used symbols (config and getBaseHeaders) and run the linter/TypeScript check to ensure no other references to getLoginDetails remain in this module.
🧹 Nitpick comments (4)
cli/src/core/telemetry.ts (2)
145-148:console.debugoutput may be unintended in production.Unlike the other error handlers that check
process.env.DEBUG, thisconsole.debugcall will always output to stderr. Consider gating it behind the DEBUG check for consistency.♻️ Proposed fix
} catch (err) { // skip catch, returning anonymous identity if any error occurs (e.g. network issues, not logged in, etc.) - console.debug('Failed to get identity for telemetry, using anonymous.', err) + if (process.env.DEBUG) { + console.debug('Failed to get identity for telemetry, using anonymous.', err); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/core/telemetry.ts` around lines 145 - 148, The catch block in telemetry.ts currently uses console.debug(...) unconditionally; change it to only log when DEBUG is enabled (same pattern used elsewhere) by guarding the console.debug call with a check like if (process.env.DEBUG) before logging, so the "Failed to get identity for telemetry, using anonymous." message is only printed in debug mode; update the catch in the function that retrieves identity for telemetry (the catch that currently calls console.debug with the err) to follow this conditional logging pattern.
33-36: Consider usinginterfaceinstead oftypefor object shapes.Per coding guidelines, interfaces are preferred over type aliases for object shapes.
♻️ Proposed fix
-type TelemetryIdentity = { +interface TelemetryIdentity { userEmail?: string; organizationSlug: string; -} +}As per coding guidelines: "Prefer interfaces over type aliases for object shapes in TypeScript".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/core/telemetry.ts` around lines 33 - 36, Replace the type alias TelemetryIdentity with an equivalent interface declaration to follow the project's TypeScript guideline preferring interfaces for object shapes; locate the TelemetryIdentity type and change it to an interface (keeping the same property names and optional marker userEmail?: and organizationSlug) and update any imports/uses if necessary to reference the interface name unchanged.studio/src/lib/track.ts (2)
51-52: Stale comment no longer reflects the implementation.The comment states "We use the id posthog sets to identify the user" but the code now uses
📝 Proposed fix
// Identify with PostHog - // We use the id posthog sets to identify the user. This way we do not lose cross domain tracking. + // We use email as the primary identifier and alias old session IDs to maintain cross-domain tracking. const posthog = PostHogClient();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@studio/src/lib/track.ts` around lines 51 - 52, Update the stale comment in studio/src/lib/track.ts to reflect the current implementation: replace "We use the id posthog sets to identify the user" with a description stating that the code uses the user's email as the primary identifier when calling PostHog (e.g., in the identify/track logic that references email and PostHog client), and remove any mention of relying on PostHog's generated distinct_id so the comment matches the actual behavior.
7-11: Remove deadwindow.kodeclaration and avoidanytypes.Per retrieved learnings,
window.ko(Koala tracking) is dead code and should be removed. Additionally, usinganytype violates TypeScript guidelines.♻️ Proposed fix
declare global { interface Window { - ko: any; - Reo: any; + Reo: { + identify: (params: { username: string; type: string }) => void; + }; } }Based on learnings: "In studio/src/lib/track.ts, remove all references to window.ko (Koala tracking) as it is no longer used."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@studio/src/lib/track.ts` around lines 7 - 11, Remove the dead Koala declaration by deleting the window.ko entry from the global Window interface in studio/src/lib/track.ts, and replace the use of the any type for window.Reo with a safer type: either declare Reo?: unknown on Window or create a minimal ReoClient interface (e.g., methods you call) and use Reo?: ReoClient; also scan this file for any other uses of any related to tracking and replace them with proper types or unknown to avoid using any.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/src/core/telemetry.ts`:
- Around line 140-143: The WhoAmIResponse currently returns userEmail as an
empty string which causes identity.userEmail ?? identity.organizationSlug in
capture() to treat it as a valid value; update the function that constructs the
identity object to normalize empty userEmail to null/undefined (e.g., set
userEmail to resp.userEmail?.trim() || undefined) so that capture() will fall
back to organizationSlug; locate references to WhoAmIResponse and the identity
return (userEmail and organizationSlug) in telemetry.ts and ensure the
normalized value is used by capture().
In `@studio/src/lib/track.ts`:
- Around line 64-67: Replace the loose comparison and add a null/undefined
guard: retrieve distinctId via posthog.get_distinct_id() and only call
posthog.alias(email, distinctId) when distinctId is defined (not undefined/null)
and not equal to email using strict inequality (distinctId !== email); update
the condition around the alias call that currently references distinctId and
email to perform these checks before invoking posthog.alias.
- Line 63: Add the missing semicolon at the end of the posthog.group('orgslug',
organizationSlug) statement in track.ts so the expression terminates correctly;
locate the usage of posthog.group in the file and append a semicolon to the
statement.
---
Outside diff comments:
In `@cli/src/core/telemetry.ts`:
- Line 5: Remove the unused import getLoginDetails from the import statement
that currently reads "import { config, getBaseHeaders, getLoginDetails } ..." —
edit the import to only include the used symbols (config and getBaseHeaders) and
run the linter/TypeScript check to ensure no other references to getLoginDetails
remain in this module.
---
Nitpick comments:
In `@cli/src/core/telemetry.ts`:
- Around line 145-148: The catch block in telemetry.ts currently uses
console.debug(...) unconditionally; change it to only log when DEBUG is enabled
(same pattern used elsewhere) by guarding the console.debug call with a check
like if (process.env.DEBUG) before logging, so the "Failed to get identity for
telemetry, using anonymous." message is only printed in debug mode; update the
catch in the function that retrieves identity for telemetry (the catch that
currently calls console.debug with the err) to follow this conditional logging
pattern.
- Around line 33-36: Replace the type alias TelemetryIdentity with an equivalent
interface declaration to follow the project's TypeScript guideline preferring
interfaces for object shapes; locate the TelemetryIdentity type and change it to
an interface (keeping the same property names and optional marker userEmail?:
and organizationSlug) and update any imports/uses if necessary to reference the
interface name unchanged.
In `@studio/src/lib/track.ts`:
- Around line 51-52: Update the stale comment in studio/src/lib/track.ts to
reflect the current implementation: replace "We use the id posthog sets to
identify the user" with a description stating that the code uses the user's
email as the primary identifier when calling PostHog (e.g., in the
identify/track logic that references email and PostHog client), and remove any
mention of relying on PostHog's generated distinct_id so the comment matches the
actual behavior.
- Around line 7-11: Remove the dead Koala declaration by deleting the window.ko
entry from the global Window interface in studio/src/lib/track.ts, and replace
the use of the any type for window.Reo with a safer type: either declare Reo?:
unknown on Window or create a minimal ReoClient interface (e.g., methods you
call) and use Reo?: ReoClient; also scan this file for any other uses of any
related to tracking and replace them with proper types or unknown to avoid using
any.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 95eaad94-affc-4b89-9505-b3857a3e4c83
📒 Files selected for processing (2)
cli/src/core/telemetry.tsstudio/src/lib/track.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…r-custom-events-in-posthog
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
cli/src/core/telemetry.ts (1)
140-143:⚠️ Potential issue | 🟡 MinorNormalize empty
userEmailbefore returning identity.At Line 142, an empty string is preserved, so
identity.userEmail ?? identity.organizationSlugcan still produce an emptydistinctId.🔧 Proposed fix
if (resp.response?.code === EnumStatusCode.OK) { return { organizationSlug: resp.organizationSlug, - userEmail: resp.userEmail, + userEmail: resp.userEmail?.trim() || undefined, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/core/telemetry.ts` around lines 140 - 143, The returned identity currently preserves an empty string in userEmail which causes downstream `identity.userEmail ?? identity.organizationSlug` to still pick an empty value; in the return object (the block that sets organizationSlug: resp.organizationSlug, userEmail: resp.userEmail) normalize userEmail by treating empty or whitespace-only strings as undefined (e.g., trim and set to undefined/null) so that downstream logic using `identity.userEmail ?? identity.organizationSlug` will correctly fall back to organizationSlug.
🧹 Nitpick comments (1)
cli/src/core/telemetry.ts (1)
33-36: Use aninterfaceforTelemetryIdentityobject shape.The TypeScript guideline requires interfaces over type aliases for object shapes.
♻️ Proposed change
-type TelemetryIdentity = { +interface TelemetryIdentity { userEmail?: string; organizationSlug: string; -}; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/src/core/telemetry.ts` around lines 33 - 36, Replace the type alias TelemetryIdentity with an interface declaration (interface TelemetryIdentity { userEmail?: string; organizationSlug: string; }) so the object shape uses an interface per project TypeScript guidelines; update any imports/usages that reference TelemetryIdentity if necessary to reflect the new declaration but keep the same property names and optionality.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/src/core/telemetry.ts`:
- Around line 145-148: The catch block that currently calls
console.debug('Failed to get identity for telemetry, using anonymous.', err)
should not log unconditionally; update the catch in the telemetry identity
retrieval function (the try/catch that gets identity for telemetry) to only emit
that debug output when debugging is enabled (e.g. guard with process.env.DEBUG
or the module's debug flag) or use the existing debug logger API instead of
unconditional console.debug so no error details are printed in normal CLI runs.
---
Duplicate comments:
In `@cli/src/core/telemetry.ts`:
- Around line 140-143: The returned identity currently preserves an empty string
in userEmail which causes downstream `identity.userEmail ??
identity.organizationSlug` to still pick an empty value; in the return object
(the block that sets organizationSlug: resp.organizationSlug, userEmail:
resp.userEmail) normalize userEmail by treating empty or whitespace-only strings
as undefined (e.g., trim and set to undefined/null) so that downstream logic
using `identity.userEmail ?? identity.organizationSlug` will correctly fall back
to organizationSlug.
---
Nitpick comments:
In `@cli/src/core/telemetry.ts`:
- Around line 33-36: Replace the type alias TelemetryIdentity with an interface
declaration (interface TelemetryIdentity { userEmail?: string; organizationSlug:
string; }) so the object shape uses an interface per project TypeScript
guidelines; update any imports/usages that reference TelemetryIdentity if
necessary to reflect the new declaration but keep the same property names and
optionality.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f91a2449-e5e7-4690-be06-05254ac11314
📒 Files selected for processing (2)
cli/src/core/telemetry.tsstudio/src/lib/track.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- studio/src/lib/track.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
studio/src/lib/track.ts (1)
53-54:⚠️ Potential issue | 🟡 MinorUse
constand strict equality fordistinctIdcheck.
distinctIdis never reassigned, and the loose comparison can coerce types unexpectedly.As per coding guidelines: "Prefer `const` over `let` and `let` over `var`" and "Always use strict equality (`===` and `!==`) instead of loose equality (`==` and `!=`)".🔧 Proposed fix
- let distinctId = posthog.get_distinct_id(); - if (distinctId == organizationSlug) { + const distinctId = posthog.get_distinct_id(); + if (distinctId === organizationSlug) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@studio/src/lib/track.ts` around lines 53 - 54, Replace the mutable declaration and loose comparison: change the declaration of distinctId (from posthog.get_distinct_id()) to use const instead of let, and update the equality check to use strict equality (===) when comparing distinctId to organizationSlug; locate this in the block that calls posthog.get_distinct_id() and the subsequent if (distinctId == organizationSlug) check and update the variable declaration and comparison accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@studio/src/lib/track.ts`:
- Around line 61-64: The early return when distinctId === email in track.ts
prevents executing posthog.identify(...) and posthog.group('orgslug',
organizationSlug'), leaving org grouping and profile properties stale; remove or
alter that early return so that even when distinctId === email you still call
posthog.identify(...) (or at minimum call posthog.group('orgslug',
organizationSlug') and trigger the profile refresh logic) after the
duplicate-check, ensuring the org grouping and user profile update paths run
(refer to distinctId, email, posthog.identify, posthog.group, and the profile
refresh code in track.ts).
---
Duplicate comments:
In `@studio/src/lib/track.ts`:
- Around line 53-54: Replace the mutable declaration and loose comparison:
change the declaration of distinctId (from posthog.get_distinct_id()) to use
const instead of let, and update the equality check to use strict equality (===)
when comparing distinctId to organizationSlug; locate this in the block that
calls posthog.get_distinct_id() and the subsequent if (distinctId ==
organizationSlug) check and update the variable declaration and comparison
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 98eb7c95-1fc6-4642-a80b-6737604dfe35
📒 Files selected for processing (1)
studio/src/lib/track.ts
alepane21
marked this pull request as ready for review
April 3, 2026 09:44
alepane21
requested review from
JivusAyrus,
StarpTech,
endigma,
jensneuse,
thisisnithin and
wilsonrivera
as code owners
April 3, 2026 09:44
…r-custom-events-in-posthog
thisisnithin
approved these changes
Apr 7, 2026
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Router image scan passed✅ No security vulnerabilities found in image: |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
thisisnithin
approved these changes
Apr 8, 2026
…r-custom-events-in-posthog
…r-custom-events-in-posthog
…r-custom-events-in-posthog
…r-custom-events-in-posthog
Being fixed in #2714 which upgrades opentelemetry-go dependencies.
JivusAyrus
approved these changes
Apr 13, 2026
…r-custom-events-in-posthog
thisisnithin
deleted the
ale/eng-8365-enable-email-level-breakdown-for-custom-events-in-posthog
branch
April 15, 2026 09:06
5 tasks
6 tasks
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
4 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.
Currently we track by organization slug.
To get more sense from the data, we should separate data by email and group them by organization id. This will help us to have the best use of PostHog.
Be careful that:
Summary by CodeRabbit
Checklist
Open Source AI Manifesto
This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.