feat(telemetry): make IdentifyUser idempotent and lazy via persisted LastIdentifiedEmail#196
Open
devin-ai-integration[bot] wants to merge 2 commits intomainfrom
Open
feat(telemetry): make IdentifyUser idempotent and lazy via persisted LastIdentifiedEmail#196devin-ai-integration[bot] wants to merge 2 commits intomainfrom
devin-ai-integration[bot] wants to merge 2 commits intomainfrom
Conversation
…LastIdentifiedEmail PostHog person records for users who logged in on a CLI version older than v0.43.59 (which introduced IdentifyUser) were never enriched with the `email` property, because IdentifyUser only fired inside the user auth branch of `infisical login`. Once those users upgraded the CLI, their persisted `LoggedInUserEmail` was used as the PostHog distinctId on every event, but no Identify/Alias was ever sent for them. The same gap existed for `infisical user switch`, which mutates the active email in the config without going through the login flow. Replace IdentifyUser(email) with IdentifyUserIfNeeded() (no args). It reads LoggedInUserEmail from the config, compares against a new LastIdentifiedEmail field, and only enqueues Identify+Alias when the two differ. After enqueueing, it persists LastIdentifiedEmail so the call is a no-op on subsequent invocations. Call IdentifyUserIfNeeded() from CaptureEvent before each Capture so that the very first telemetry event after a CLI upgrade (or a profile switch) lazily enriches the PostHog person record. Also call it from the user-auth login path for clarity, which is now redundant-but-safe because of the persistence guard. Preserve LastIdentifiedEmail in WriteInitalConfig so a fresh login on the same email does not re-fire Identify, while a fresh login on a different email does.
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0de21605ed
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
6 tasks
…ueue Addresses Codex review on #196. If posthogClient.Enqueue returns an error (closed client, malformed message, etc.) and we still write LastIdentifiedEmail = email, the guard at the top of IdentifyUserIfNeeded would skip Identify/Alias forever for that email — locking the user out of person enrichment. Capture the error from each Enqueue call. On Identify failure, bail out early without persisting. On Alias failure, also bail out (the anonymous-to-email link is part of the Identify story; we want the next invocation to retry). Only persist LastIdentifiedEmail when both enqueues succeed, so transient failures self-heal on the next CLI invocation.
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
1 participant
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.
Description 📣
Follow-up to #146. PostHog person records for users who were already logged in before they upgraded to a CLI version with
IdentifyUser(v0.43.59+) are never enriched with theemailproperty. Their persistedLoggedInUserEmailis used as the PostHog distinctId on everycli-command:*event, so PostHog auto-creates a person — butIdentifyUseronly fires inside the user-auth branch ofinfisical login, which they don't re-enter on subsequent CLI invocations. The same gap exists forinfisical user switch, which rewritesLoggedInUserEmailin the config without going through the login flow.Concrete symptom seen in PostHog: a person like
automation@beauty-crm.localshows up keyed by email distinctId, withcli-command:secretsevents flowing in, but noemailproperty on the person record (only PostHog auto-captured GeoIP/location).This PR fixes that by making the Identify/Alias call idempotent and lazy:
Telemetry.IdentifyUser(email)withTelemetry.IdentifyUserIfNeeded()(no args). It readsLoggedInUserEmailfrom the config and compares it against a newLastIdentifiedEmailfield. Identify+Alias are only enqueued when the two differ; on success it persistsLastIdentifiedEmail = LoggedInUserEmailso the next call is a no-op.IdentifyUserIfNeeded()fromCaptureEventbefore every Capture, so the first telemetry event after a CLI upgrade (or a profile switch) lazily enriches the PostHog person record. The cost is one extra config read per CLI command and, in the steady state, zero PostHog enqueues.LastIdentifiedEmailthroughWriteInitalConfigso a re-login on the same email doesn't re-Identify, while a fresh login on a different email correctly does (becauseLoggedInUserEmail != LastIdentifiedEmailafter the write).What this PR does not address (intentionally — separate design call worth its own PR):
--method=universal-auth|kubernetes|aws-iam|gcp-iam|gcp-id-token|azure|oidc-auth|jwt-auth|ldap-auth) still fire zero PostHog events from theinfisical logincommand itself, and subsequent commands run with a machine-identity token still resolve toanonymous_cli_<machineId>because the token is never persisted toLoggedInUserEmail. That is the dominant source of residualanonymous_cli_*persons in PostHog and is best addressed with$process_person_profile: false+ groups, not by extendingIdentifyUser.IdentifyUserpath at all and will only stop generating anonymous events as users upgrade organically.Type ✨
Tests 🛠️
Manual reasoning-through of the state transitions (no automated tests for telemetry exist today; the package is mock-free and tightly coupled to PostHog + filesystem).
LoggedInUserEmailLastIdentifiedEmailCaptureEvent""""anonymous_cli_<machineId>. Unchanged.a@x.com(new)""(new)LastIdentifiedEmail = a@x.compersisted. Unchanged from #146.a@x.com""LastIdentifiedEmail = a@x.compersisted.a@x.coma@x.cominfisical user switchtob@x.comb@x.coma@x.comb@x.com,LastIdentifiedEmail = b@x.compersisted.a@x.coma@x.comBuild verified locally:
go build ./... go vet ./packages/telemetry/... ./packages/cmd/login.go ./packages/util/... ./packages/models/... # (Pre-existing run.go vet warnings are unrelated and present on main)Link to Devin session: https://app.devin.ai/sessions/6363c6181d1641f8a564bc8161e4270f
Requested by: @0xArshdeep