diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 28a8351b..a0c1c15b 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -140,7 +140,7 @@ func runOAuthFlowSteps( return err } - identifyAuthenticatedUser(ctx) + applyStoredIdentity(ctx) tracker.SetStep(telemetry.StepAppsFetch) opts.IO.StartProgressIndicatorWithLabel("Fetching applications") @@ -200,22 +200,10 @@ func runOAuthFlowSteps( return apputil.ConfigureProfile(opts.IO, opts.Config, appDetails, profileName, opts.Default) } -// identifyAuthenticatedUser emits a telemetry Identify for the user that just -// authenticated. It is a no-op when no identified token is present. -func identifyAuthenticatedUser(ctx context.Context) { - if !applyStoredIdentity(ctx) { - return - } - client := telemetry.GetTelemetryClient(ctx) - if client == nil { - return - } - _ = client.Identify(ctx) -} - // applyStoredIdentity copies the persisted user identity from the stored token -// onto the request's telemetry metadata. It reports whether an identity was -// applied. +// onto the request's telemetry metadata so the flow's own events carry the user. +// The Identify itself is sent once at command completion. It reports whether an +// identity was applied. func applyStoredIdentity(ctx context.Context) bool { token := auth.LoadToken() if token == nil || token.UserID == "" { diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index f8042333..a53e242c 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -162,6 +162,7 @@ func Execute() (code exitCode) { // Pre-command auth check and telemetry setup. authError := errors.New("authError") + var preRunAccessToken string rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { if auth.IsAuthCheckEnabled(cmd) { if err := auth.CheckAuth(&cfg); err != nil { @@ -194,18 +195,12 @@ func Execute() (code exitCode) { if token := auth.LoadToken(); token != nil { telemetryMetadata.SetUser(token.UserID, token.Email, token.Name) + preRunAccessToken = token.AccessToken } ctx := cmd.Context() telemetryClient := telemetry.GetTelemetryClient(ctx) - // Identify the user. - err = telemetryClient.Identify(ctx) - if err != nil && hasDebug { - fmt.Fprintf(stderr, "Failed to identify user: %s\n", err) - return err - } - // Send telemetry. err = telemetryClient.Track(ctx, telemetry.EventCommandInvoked, nil) if err != nil && hasDebug { @@ -232,7 +227,7 @@ func Execute() (code exitCode) { // includes the update-notifier wait below. cmd, err := rootCmd.ExecuteContextC(ctx) executedCmd, executeErr, elapsed = cmd, err, time.Since(start) - identifyNewlyAuthenticatedUser(ctx, cmd) + identifyNewSession(ctx, cmd, preRunAccessToken) // Handle eventual errors. if err != nil { if err == cmdutil.ErrSilent { @@ -270,23 +265,24 @@ func Execute() (code exitCode) { return exitOK } -// identifyNewlyAuthenticatedUser re-sends an Identify when the user signed in -// during the command (e.g. `application create` while logged out): the -// Identify from PersistentPreRunE went out anonymous, so the identity would -// otherwise only ship on the next invocation. Runs before the deferred -// Command Completed so that event carries the user too. -func identifyNewlyAuthenticatedUser(ctx context.Context, cmd *cobra.Command) { +// identifyNewSession sends an Identify when the run established or renewed the +// session: the stored access token differs from the one known at +// PersistentPreRunE (login, signup, account switch, re-authentication or silent +// token refresh), so the identity and its traits do not have to wait for the +// next invocation. Runs before the deferred Command Completed so that event +// carries the user too. +func identifyNewSession(ctx context.Context, cmd *cobra.Command, preRunAccessToken string) { if cmd == nil || !cmdutil.ShouldTrackUsage(cmd) { return } // Same gating as trackCommandCompleted: an empty command path means // PersistentPreRunE never ran, so no login could have happened either. metadata := telemetry.GetEventMetadata(ctx) - if metadata == nil || metadata.CommandPath == "" || metadata.UserID != "" { + if metadata == nil || metadata.CommandPath == "" { return } token := auth.LoadToken() - if token == nil || token.UserID == "" { + if token == nil || token.UserID == "" || token.AccessToken == preRunAccessToken { return } metadata.SetUser(token.UserID, token.Email, token.Name) diff --git a/pkg/cmd/root/root_test.go b/pkg/cmd/root/root_test.go index da949661..b4ebfd5b 100644 --- a/pkg/cmd/root/root_test.go +++ b/pkg/cmd/root/root_test.go @@ -10,7 +10,10 @@ import ( "time" "github.com/spf13/cobra" + "github.com/zalando/go-keyring" + "github.com/algolia/cli/api/dashboard" + "github.com/algolia/cli/pkg/auth" "github.com/algolia/cli/pkg/cmdutil" "github.com/algolia/cli/pkg/telemetry" ) @@ -18,7 +21,8 @@ import ( // recordingTelemetryClient captures the tracked events so tests can assert on // them without hitting the network. type recordingTelemetryClient struct { - events []recordedEvent + events []recordedEvent + identifies int } type recordedEvent struct { @@ -26,7 +30,10 @@ type recordedEvent struct { props map[string]any } -func (r *recordingTelemetryClient) Identify(ctx context.Context) error { return nil } +func (r *recordingTelemetryClient) Identify(ctx context.Context) error { + r.identifies++ + return nil +} func (r *recordingTelemetryClient) Track( ctx context.Context, @@ -109,6 +116,114 @@ func TestTrackCommandCompleted_ReportsFailure(t *testing.T) { } } +func storeMockToken(t *testing.T, userID int, accessToken string) { + t.Helper() + keyring.MockInit() + t.Cleanup(auth.ClearToken) + + err := auth.SaveToken(&dashboard.OAuthTokenResponse{ + AccessToken: accessToken, + RefreshToken: "refresh", + ExpiresIn: 3600, + User: &dashboard.User{ + ID: userID, + Email: "user@test.com", + Name: "Test User", + }, + }) + if err != nil { + t.Fatalf("SaveToken() error = %v", err) + } +} + +func TestIdentifyNewSession_SkipsUnchangedSession(t *testing.T) { + storeMockToken(t, 42, "token-1") + + client := &recordingTelemetryClient{} + ctx := newTelemetryContext(client, "algolia indices list") + telemetry.GetEventMetadata(ctx).SetUser("42", "user@test.com", "Test User") + + identifyNewSession(ctx, &cobra.Command{Use: "list"}, "token-1") + + if client.identifies != 0 { + t.Errorf("expected no Identify, got %d", client.identifies) + } +} + +func TestIdentifyNewSession_IdentifiesRenewedSessionForSameUser(t *testing.T) { + storeMockToken(t, 42, "token-2") + + client := &recordingTelemetryClient{} + ctx := newTelemetryContext(client, "algolia indices list") + telemetry.GetEventMetadata(ctx).SetUser("42", "user@test.com", "Test User") + + identifyNewSession(ctx, &cobra.Command{Use: "list"}, "token-1") + + if client.identifies != 1 { + t.Errorf("expected 1 Identify, got %d", client.identifies) + } +} + +func TestIdentifyNewSession_IdentifiesAccountSwitch(t *testing.T) { + storeMockToken(t, 43, "token-2") + + client := &recordingTelemetryClient{} + ctx := newTelemetryContext(client, "algolia indices list") + telemetry.GetEventMetadata(ctx).SetUser("42", "other@test.com", "Other User") + + identifyNewSession(ctx, &cobra.Command{Use: "list"}, "token-1") + + if client.identifies != 1 { + t.Errorf("expected 1 Identify, got %d", client.identifies) + } + if got := telemetry.GetEventMetadata(ctx).UserID; got != "43" { + t.Errorf("metadata UserID = %q, want %q", got, "43") + } +} + +func TestIdentifyNewSession_IdentifiesUserAuthenticatedMidRun(t *testing.T) { + storeMockToken(t, 42, "token-1") + + client := &recordingTelemetryClient{} + ctx := newTelemetryContext(client, "algolia indices list") + + identifyNewSession(ctx, &cobra.Command{Use: "list"}, "") + + if client.identifies != 1 { + t.Errorf("expected 1 Identify, got %d", client.identifies) + } + if got := telemetry.GetEventMetadata(ctx).UserID; got != "42" { + t.Errorf("metadata UserID = %q, want %q", got, "42") + } +} + +func TestIdentifyNewSession_SkipsWhenPreRunNeverRan(t *testing.T) { + storeMockToken(t, 42, "token-1") + + client := &recordingTelemetryClient{} + ctx := newTelemetryContext(client, "") + + identifyNewSession(ctx, &cobra.Command{Use: "list"}, "") + + if client.identifies != 0 { + t.Errorf("expected no Identify, got %d", client.identifies) + } +} + +func TestIdentifyNewSession_SkipsWhenNoTokenStored(t *testing.T) { + keyring.MockInit() + auth.ClearToken() + + client := &recordingTelemetryClient{} + ctx := newTelemetryContext(client, "algolia indices list") + + identifyNewSession(ctx, &cobra.Command{Use: "list"}, "") + + if client.identifies != 0 { + t.Errorf("expected no Identify, got %d", client.identifies) + } +} + func TestPrintError(t *testing.T) { cmd := &cobra.Command{} diff --git a/pkg/telemetry/telemetry.go b/pkg/telemetry/telemetry.go index 2cdd108d..29776ad8 100644 --- a/pkg/telemetry/telemetry.go +++ b/pkg/telemetry/telemetry.go @@ -265,7 +265,12 @@ func (a *AnalyticsTelemetryClient) Track( ) error { metadata := GetEventMetadata(ctx) - props := make(map[string]any, len(properties)+6) + var isCI int8 + if utils.IsCI() { + isCI = 1 + } + + props := make(map[string]any, len(properties)+10) for k, v := range properties { props[k] = v } @@ -276,6 +281,10 @@ func (a *AnalyticsTelemetryClient) Track( props["flags"] = metadata.CommandFlags props["sequence"] = a.sequence.Add(1) props["cli_context"] = metadata.CLIContext + props["version"] = metadata.CLIVersion + props["operating_system"] = metadata.OS + props["configured_applications"] = metadata.ConfiguredApplicationsNb + props["is_ci"] = isCI track := analytics.Track{ Event: event, diff --git a/pkg/telemetry/telemetry_test.go b/pkg/telemetry/telemetry_test.go index 0592efec..025cf411 100644 --- a/pkg/telemetry/telemetry_test.go +++ b/pkg/telemetry/telemetry_test.go @@ -232,6 +232,41 @@ func TestTrack_IncludesCLIContext(t *testing.T) { assert.Equal(t, "agent:claude-code", track.Properties["cli_context"]) } +func TestTrack_IncludesVersionAndOperatingSystem(t *testing.T) { + fake := &fakeAnalyticsClient{} + client := &AnalyticsTelemetryClient{client: fake} + + metadata := NewEventMetadata() + ctx := WithEventMetadata(context.Background(), metadata) + + require.NoError(t, client.Track(ctx, "Command Invoked", nil)) + require.Len(t, fake.messages, 1) + + track, ok := fake.messages[0].(analytics.Track) + require.True(t, ok) + assert.Equal(t, metadata.CLIVersion, track.Properties["version"]) + assert.Equal(t, metadata.OS, track.Properties["operating_system"]) +} + +func TestTrack_IncludesConfiguredApplicationsAndIsCI(t *testing.T) { + t.Setenv("CI", "1") + + fake := &fakeAnalyticsClient{} + client := &AnalyticsTelemetryClient{client: fake} + + metadata := NewEventMetadata() + metadata.SetConfiguredApplicationsNb(3) + ctx := WithEventMetadata(context.Background(), metadata) + + require.NoError(t, client.Track(ctx, "Command Invoked", nil)) + require.Len(t, fake.messages, 1) + + track, ok := fake.messages[0].(analytics.Track) + require.True(t, ok) + assert.Equal(t, 3, track.Properties["configured_applications"]) + assert.Equal(t, int8(1), track.Properties["is_ci"]) +} + func TestNewEventMetadata_DetectsCLIContext(t *testing.T) { metadata := NewEventMetadata() assert.NotEmpty(t, metadata.CLIContext) @@ -291,15 +326,22 @@ func TestTrack_SequenceIsUniqueUnderConcurrency(t *testing.T) { } func TestTrack_CustomPropertiesCannotOverrideBase(t *testing.T) { + t.Setenv("CI", "1") + fake := &fakeAnalyticsClient{} client := &AnalyticsTelemetryClient{client: fake} metadata := NewEventMetadata() + metadata.SetConfiguredApplicationsNb(3) ctx := WithEventMetadata(context.Background(), metadata) require.NoError(t, client.Track(ctx, "Command Invoked", map[string]any{ - "invocation_id": "spoofed", - "sequence": int64(999), + "invocation_id": "spoofed", + "sequence": int64(999), + "version": "spoofed", + "operating_system": "spoofed", + "configured_applications": 999, + "is_ci": int8(0), })) require.Len(t, fake.messages, 1) @@ -307,4 +349,8 @@ func TestTrack_CustomPropertiesCannotOverrideBase(t *testing.T) { require.True(t, ok) assert.Equal(t, metadata.InvocationID, track.Properties["invocation_id"]) assert.Equal(t, int64(1), track.Properties["sequence"]) + assert.Equal(t, metadata.CLIVersion, track.Properties["version"]) + assert.Equal(t, metadata.OS, track.Properties["operating_system"]) + assert.Equal(t, 3, track.Properties["configured_applications"]) + assert.Equal(t, int8(1), track.Properties["is_ci"]) }