From ed198055c9632dde9473c3ddb162c17c6ece04dd Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Wed, 10 Jun 2026 11:27:34 -0700 Subject: [PATCH 1/6] fix(telemetry): flush events after the command instead of before - go telemetryClient.Close() ran in PersistentPreRunE, closing the client before the command executed and racing with process exit - close the client in Execute() via defer, bounded by a 3s timeout so an unreachable telemetry endpoint never delays exit - fall back to NoOpTelemetryClient when the analytics client cannot be created instead of storing a nil interface in the context - enable analytics-go verbose logging in debug mode so successful flushes are visible with DEBUG=1 Co-Authored-By: Claude Fable 5 --- pkg/cmd/root/root.go | 50 ++++++++++++++++++++++++-------------- pkg/telemetry/telemetry.go | 1 + 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 399535e1..4564deda 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -13,6 +13,7 @@ import ( "os/exec" "path/filepath" "strings" + "time" "github.com/AlecAivazis/survey/v2/terminal" "github.com/MakeNowJust/heredoc" @@ -197,17 +198,12 @@ func Execute() exitCode { fmt.Fprintf(stderr, "Error tracking telemetry: %s\n", err) } - go telemetryClient.Close() // flush telemetry events - return nil } // Command context is used to pass information to the telemetry client. - ctx, err := createContext(rootCmd, stderr, hasDebug, hasTelemetry) - if err != nil { - printError(stderr, err, rootCmd, hasDebug) - return exitError - } + ctx := createContext(rootCmd, stderr, hasDebug, hasTelemetry) + defer closeTelemetry(ctx) // Run the command. cmd, err := rootCmd.ExecuteContextC(ctx) @@ -248,31 +244,49 @@ func Execute() exitCode { return exitOK } +// closeTelemetry flushes the pending telemetry events, giving up after a +// short timeout so an unreachable telemetry endpoint never delays exit. +func closeTelemetry(ctx context.Context) { + client := telemetry.GetTelemetryClient(ctx) + if client == nil { + return + } + done := make(chan struct{}) + go func() { + client.Close() + close(done) + }() + select { + case <-done: + case <-time.After(3 * time.Second): + } +} + // createContext creates a context with telemetry. func createContext( cmd *cobra.Command, stderr io.Writer, hasDebug bool, hasTelemetry bool, -) (context.Context, error) { +) context.Context { ctx := context.Background() telemetryMetadata := telemetry.NewEventMetadata() updatedCtx := telemetry.WithEventMetadata(ctx, telemetryMetadata) - var telemetryClient telemetry.TelemetryClient - var err error + var telemetryClient telemetry.TelemetryClient = &telemetry.NoOpTelemetryClient{} if hasTelemetry { - telemetryClient, err = telemetry.NewAnalyticsTelemetryClient(hasDebug) - // Fail silently if telemetry is not available unless in debug mode. - if err != nil && hasDebug { - fmt.Fprintf(stderr, "Error creating telemetry client: %s\n", err) - return nil, err + client, err := telemetry.NewAnalyticsTelemetryClient(hasDebug) + if err != nil { + // Fail silently (fall back to no-op telemetry) unless in debug mode. + if hasDebug { + fmt.Fprintf(stderr, "Error creating telemetry client: %s\n", err) + } + } else { + telemetryClient = client } - } else { - telemetryClient = &telemetry.NoOpTelemetryClient{} } contextWithTelemetry := telemetry.WithTelemetryClient(updatedCtx, telemetryClient) - return contextWithTelemetry, nil + return contextWithTelemetry } // printError prints an error to the stderr, with additional information if applicable. diff --git a/pkg/telemetry/telemetry.go b/pkg/telemetry/telemetry.go index f81abe75..04f9e878 100644 --- a/pkg/telemetry/telemetry.go +++ b/pkg/telemetry/telemetry.go @@ -63,6 +63,7 @@ func NewAnalyticsTelemetryClient(debug bool) (TelemetryClient, error) { client, err := analytics.NewWithConfig("", analytics.Config{ Endpoint: telemetryAnalyticsURL, Logger: newTelemetryLogger(debug), + Verbose: debug, }) if err != nil { return nil, err From c8e3b3d7027e0227e0f217890bd24bbbd7a463bf Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Wed, 10 Jun 2026 12:39:45 -0700 Subject: [PATCH 2/6] feat(telemetry): track events with custom properties and a sequence number - Track now takes a properties map, merged with the base properties; base properties are written last so call sites cannot override them - each Track event carries a monotonic sequence number (atomic counter) so the exact order of one invocation can be reconstructed downstream: Segment stores timestamps with millisecond precision, so back-to-back events can tie Co-Authored-By: Claude Fable 5 --- pkg/cmd/root/root.go | 2 +- pkg/telemetry/telemetry.go | 48 +++++++++++++++++++------- pkg/telemetry/telemetry_test.go | 60 +++++++++++++++++++++++++++++++-- 3 files changed, 95 insertions(+), 15 deletions(-) diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 4564deda..2feb4164 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -193,7 +193,7 @@ func Execute() exitCode { } // Send telemetry. - err = telemetryClient.Track(ctx, "Command Invoked") + err = telemetryClient.Track(ctx, "Command Invoked", nil) if err != nil && hasDebug { fmt.Fprintf(stderr, "Error tracking telemetry: %s\n", err) } diff --git a/pkg/telemetry/telemetry.go b/pkg/telemetry/telemetry.go index 04f9e878..9330d055 100644 --- a/pkg/telemetry/telemetry.go +++ b/pkg/telemetry/telemetry.go @@ -8,6 +8,7 @@ import ( "net" "os" "runtime" + "sync/atomic" "github.com/segmentio/analytics-go/v3" "github.com/spf13/cobra" @@ -29,12 +30,16 @@ type telemetryClientKey struct{} type TelemetryClient interface { Identify(ctx context.Context) error - Track(ctx context.Context, event string) error + Track(ctx context.Context, event string, properties map[string]any) error Close() } type AnalyticsTelemetryClient struct { client analytics.Client + // sequence numbers the Track events of one invocation so their order can + // be reconstructed downstream: Segment stores timestamps with millisecond + // precision, so back-to-back events can tie. + sequence atomic.Int64 } type AnalyticsTelemetryLogger struct { @@ -237,19 +242,30 @@ func (a *AnalyticsTelemetryClient) Identify(ctx context.Context) error { return a.client.Enqueue(identify) } -// Track tracks the event with the provided properties -func (a *AnalyticsTelemetryClient) Track(ctx context.Context, event string) error { +// Track tracks the event with the provided custom properties, merged with the +// base properties of the invocation +func (a *AnalyticsTelemetryClient) Track( + ctx context.Context, + event string, + properties map[string]any, +) error { metadata := GetEventMetadata(ctx) + props := make(map[string]any, len(properties)+5) + for k, v := range properties { + props[k] = v + } + // Base properties are set last so custom ones can never override them. + props["invocation_id"] = metadata.InvocationID + props["app_id"] = metadata.AppID + props["command"] = metadata.CommandPath + props["flags"] = metadata.CommandFlags + props["sequence"] = a.sequence.Add(1) + track := analytics.Track{ Event: event, AnonymousId: metadata.AnonymousID, - Properties: map[string]interface{}{ - "invocation_id": metadata.InvocationID, - "app_id": metadata.AppID, - "command": metadata.CommandPath, - "flags": metadata.CommandFlags, - }, + Properties: props, Context: &analytics.Context{ Device: analytics.DeviceInfo{ Id: metadata.AnonymousID, @@ -269,6 +285,14 @@ func (a *AnalyticsTelemetryClient) Close() { _ = a.client.Close() } -func (a *NoOpTelemetryClient) Identify(ctx context.Context) error { return nil } -func (a *NoOpTelemetryClient) Track(ctx context.Context, event string) error { return nil } -func (a *NoOpTelemetryClient) Close() {} +func (a *NoOpTelemetryClient) Identify(ctx context.Context) error { return nil } + +func (a *NoOpTelemetryClient) Track( + ctx context.Context, + event string, + properties map[string]any, +) error { + return nil +} + +func (a *NoOpTelemetryClient) Close() {} diff --git a/pkg/telemetry/telemetry_test.go b/pkg/telemetry/telemetry_test.go index 8e381464..3ad0bd96 100644 --- a/pkg/telemetry/telemetry_test.go +++ b/pkg/telemetry/telemetry_test.go @@ -130,7 +130,7 @@ func TestTrack_IncludesUserWhenAuthenticated(t *testing.T) { metadata.SetUser("user-42", "user@test.com", "Test User") ctx := WithEventMetadata(context.Background(), metadata) - require.NoError(t, client.Track(ctx, "Command Invoked")) + require.NoError(t, client.Track(ctx, "Command Invoked", nil)) require.Len(t, fake.messages, 1) track, ok := fake.messages[0].(analytics.Track) @@ -146,10 +146,66 @@ func TestTrack_OmitsUserWhenAnonymous(t *testing.T) { metadata := NewEventMetadata() ctx := WithEventMetadata(context.Background(), metadata) - require.NoError(t, client.Track(ctx, "Command Invoked")) + 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.Empty(t, track.UserId) } + +func TestTrack_MergesCustomProperties(t *testing.T) { + fake := &fakeAnalyticsClient{} + client := &AnalyticsTelemetryClient{client: fake} + + metadata := NewEventMetadata() + metadata.SetAppID("app-id") + ctx := WithEventMetadata(context.Background(), metadata) + + require.NoError(t, client.Track(ctx, "CLI Auth Started", map[string]any{"flow": "login"})) + require.Len(t, fake.messages, 1) + + track, ok := fake.messages[0].(analytics.Track) + require.True(t, ok) + assert.Equal(t, "login", track.Properties["flow"]) + assert.Equal(t, metadata.InvocationID, track.Properties["invocation_id"]) + assert.Equal(t, "app-id", track.Properties["app_id"]) +} + +func TestTrack_SequenceIsMonotonic(t *testing.T) { + fake := &fakeAnalyticsClient{} + client := &AnalyticsTelemetryClient{client: fake} + + metadata := NewEventMetadata() + ctx := WithEventMetadata(context.Background(), metadata) + + for i := 0; i < 3; i++ { + require.NoError(t, client.Track(ctx, "Command Invoked", nil)) + } + require.Len(t, fake.messages, 3) + + for i, msg := range fake.messages { + track, ok := msg.(analytics.Track) + require.True(t, ok) + assert.Equal(t, int64(i+1), track.Properties["sequence"]) + } +} + +func TestTrack_CustomPropertiesCannotOverrideBase(t *testing.T) { + fake := &fakeAnalyticsClient{} + client := &AnalyticsTelemetryClient{client: fake} + + metadata := NewEventMetadata() + ctx := WithEventMetadata(context.Background(), metadata) + + require.NoError(t, client.Track(ctx, "Command Invoked", map[string]any{ + "invocation_id": "spoofed", + "sequence": int64(999), + })) + require.Len(t, fake.messages, 1) + + track, ok := fake.messages[0].(analytics.Track) + require.True(t, ok) + assert.Equal(t, metadata.InvocationID, track.Properties["invocation_id"]) + assert.Equal(t, int64(1), track.Properties["sequence"]) +} From babf4775bae92150737850779f1af13f45ccccea Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Wed, 10 Jun 2026 13:37:46 -0700 Subject: [PATCH 3/6] feat(telemetry): add event catalog and Command Completed event - events.go: typed catalog for the flow events (event name constants, Flow/Step/Direction enums), FlowTracker helper (current step and flow duration) and ErrorClass (root cause type only, never the message, which could contain user data) - Execute() reports a Command Completed event with succeeded, exit_code, duration_ms, and error_class/user_cancelled on failure, through a deferred read of the named return value Co-Authored-By: Claude Fable 5 --- pkg/cmd/root/root.go | 44 +++++++++++- pkg/telemetry/events.go | 125 +++++++++++++++++++++++++++++++++++ pkg/telemetry/events_test.go | 35 ++++++++++ 3 files changed, 202 insertions(+), 2 deletions(-) create mode 100644 pkg/telemetry/events.go create mode 100644 pkg/telemetry/events_test.go diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 2feb4164..c6b207c9 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -122,7 +122,8 @@ func NewRootCmd(f *cmdutil.Factory) *cobra.Command { return cmd } -func Execute() exitCode { +func Execute() (code exitCode) { + start := time.Now() hasDebug := os.Getenv("DEBUG") != "" hasTelemetry := os.Getenv("ALGOLIA_CLI_TELEMETRY") != "0" @@ -193,7 +194,7 @@ func Execute() exitCode { } // Send telemetry. - err = telemetryClient.Track(ctx, "Command Invoked", nil) + err = telemetryClient.Track(ctx, telemetry.EventCommandInvoked, nil) if err != nil && hasDebug { fmt.Fprintf(stderr, "Error tracking telemetry: %s\n", err) } @@ -205,8 +206,17 @@ func Execute() exitCode { ctx := createContext(rootCmd, stderr, hasDebug, hasTelemetry) defer closeTelemetry(ctx) + // Report how the command ended just before the final flush (deferred + // functions run last-in-first-out). + var executedCmd *cobra.Command + var executeErr error + defer func() { + trackCommandCompleted(ctx, executedCmd, code, executeErr, time.Since(start)) + }() + // Run the command. cmd, err := rootCmd.ExecuteContextC(ctx) + executedCmd, executeErr = cmd, err // Handle eventual errors. if err != nil { if err == cmdutil.ErrSilent { @@ -244,6 +254,36 @@ func Execute() exitCode { return exitOK } +// trackCommandCompleted reports how the command ended: success, failure (with +// the class of the error) or user cancellation. +func trackCommandCompleted( + ctx context.Context, + cmd *cobra.Command, + code exitCode, + err error, + elapsed time.Duration, +) { + if cmd == nil || !cmdutil.ShouldTrackUsage(cmd) { + return + } + client := telemetry.GetTelemetryClient(ctx) + if client == nil { + return + } + + props := map[string]any{ + "succeeded": code == exitOK, + "exit_code": int(code), + "duration_ms": elapsed.Milliseconds(), + } + if err != nil { + props["error_class"] = telemetry.ErrorClass(err) + props["user_cancelled"] = cmdutil.IsUserCancellation(err) + } + + _ = client.Track(ctx, telemetry.EventCommandCompleted, props) +} + // closeTelemetry flushes the pending telemetry events, giving up after a // short timeout so an unreachable telemetry endpoint never delays exit. func closeTelemetry(ctx context.Context) { diff --git a/pkg/telemetry/events.go b/pkg/telemetry/events.go new file mode 100644 index 00000000..8c37dcd9 --- /dev/null +++ b/pkg/telemetry/events.go @@ -0,0 +1,125 @@ +package telemetry + +import ( + "errors" + "fmt" + "time" +) + +// Event names. New flow events follow the `CLI ` convention; +// the command lifecycle events stay unprefixed for consistency with the +// historical "Command Invoked". +const ( + EventCommandInvoked = "Command Invoked" + EventCommandCompleted = "Command Completed" + + EventAuthStarted = "CLI Auth Started" + EventAuthCompleted = "CLI Auth Completed" + EventAuthFailed = "CLI Auth Failed" + EventAuthAborted = "CLI Auth Aborted" + + EventApplicationCreateStarted = "CLI Application Create Started" + EventApplicationCreateAcceptedTerms = "CLI Application Create Accepted Terms" + EventApplicationCreateDeclinedTerms = "CLI Application Create Declined Terms" + EventApplicationCreateCompleted = "CLI Application Create Completed" + EventApplicationCreateFailed = "CLI Application Create Failed" + EventApplicationCreateAborted = "CLI Application Create Aborted" + + EventApplicationPlanChangeStarted = "CLI Application Plan Change Started" + EventApplicationPlanChangeAcceptedTerms = "CLI Application Plan Change Accepted Terms" + EventApplicationPlanChangeDeclinedTerms = "CLI Application Plan Change Declined Terms" + EventApplicationPlanChangeCompleted = "CLI Application Plan Change Completed" + EventApplicationPlanChangeFailed = "CLI Application Plan Change Failed" + EventApplicationPlanChangeAborted = "CLI Application Plan Change Aborted" +) + +// Flow is the kind of auth flow the user is going through. +type Flow string + +const ( + FlowLogin Flow = "login" + FlowSignup Flow = "signup" +) + +// Step locates where the user is inside an interactive flow, so aborts and +// failures can tell where the user stopped. +type Step string + +const ( + // Auth flow steps. + StepBrowserWait Step = "browser_wait" + StepCodeExchange Step = "code_exchange" + StepAppsFetch Step = "apps_fetch" + StepAppSelect Step = "app_select" + StepAppCreate Step = "app_create" + StepProfileConfigure Step = "profile_configure" + + // Application create and plan change flow steps. + StepName Step = "name" + StepPlan Step = "plan" + StepTerms Step = "terms" + StepRegion Step = "region" + StepAPICall Step = "api_call" + StepApplyPlan Step = "apply_plan" +) + +// Direction is the direction of a plan change. +type Direction string + +const ( + DirectionUpgrade Direction = "upgrade" + DirectionDowngrade Direction = "downgrade" +) + +// FlowTracker carries the state of one interactive flow: the step the user is +// currently in and the flow start time, to compute durations. All its methods +// are safe on a nil tracker, so helpers shared by several flows can take an +// optional tracker. +type FlowTracker struct { + start time.Time + step Step +} + +func NewFlowTracker() *FlowTracker { + return &FlowTracker{start: time.Now()} +} + +// SetStep records the step the flow is entering. +func (f *FlowTracker) SetStep(step Step) { + if f == nil { + return + } + f.step = step +} + +// Step returns the step the flow is currently in. +func (f *FlowTracker) Step() Step { + if f == nil { + return "" + } + return f.step +} + +// DurationMS returns the time elapsed since the flow started, in milliseconds. +func (f *FlowTracker) DurationMS() int64 { + if f == nil { + return 0 + } + return time.Since(f.start).Milliseconds() +} + +// ErrorClass returns the type of the root cause of an error, never its +// message, which could contain user data. +func ErrorClass(err error) string { + if err == nil { + return "" + } + for { + unwrapped := errors.Unwrap(err) + if unwrapped == nil { + break + } + err = unwrapped + } + return fmt.Sprintf("%T", err) +} diff --git a/pkg/telemetry/events_test.go b/pkg/telemetry/events_test.go new file mode 100644 index 00000000..a697db4e --- /dev/null +++ b/pkg/telemetry/events_test.go @@ -0,0 +1,35 @@ +package telemetry + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +type testRootError struct{} + +func (testRootError) Error() string { return "boom" } + +func TestErrorClass_ReturnsRootCauseType(t *testing.T) { + wrapped := fmt.Errorf("outer: %w", fmt.Errorf("inner: %w", testRootError{})) + assert.Equal(t, "telemetry.testRootError", ErrorClass(wrapped)) +} + +func TestErrorClass_NilError(t *testing.T) { + assert.Equal(t, "", ErrorClass(nil)) +} + +func TestFlowTracker_NilTrackerIsSafe(t *testing.T) { + var tracker *FlowTracker + tracker.SetStep(StepTerms) + assert.Equal(t, Step(""), tracker.Step()) + assert.Equal(t, int64(0), tracker.DurationMS()) +} + +func TestFlowTracker_TracksStepAndDuration(t *testing.T) { + tracker := NewFlowTracker() + tracker.SetStep(StepPlan) + assert.Equal(t, StepPlan, tracker.Step()) + assert.GreaterOrEqual(t, tracker.DurationMS(), int64(0)) +} From 1f8b139a52a88ac9314b18a612f0bbf04376ce60 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Wed, 10 Jun 2026 13:53:03 -0700 Subject: [PATCH 4/6] fix(telemetry): address independent review findings - don't emit an orphan Command Completed when PersistentPreRunE never ran (--help, --version, unknown flag/command, failed auth check): no Command Invoked was sent, and the command property would be empty - measure the command duration right after execution, so duration_ms never includes the update-notifier wait (which only happens on the success path and would bias the metric) - ErrorClass keeps the first informative type of the error chain instead of unwrapping to the root, which collapsed every CLI error into *errors.errorString - tests: trackCommandCompleted (skip/success/failure), sequence uniqueness under concurrency, informative wrapper type Co-Authored-By: Claude Fable 5 --- pkg/cmd/root/root.go | 15 ++++- pkg/cmd/root/root_test.go | 97 +++++++++++++++++++++++++++++++++ pkg/telemetry/events.go | 24 ++++---- pkg/telemetry/events_test.go | 12 +++- pkg/telemetry/telemetry_test.go | 34 ++++++++++++ 5 files changed, 167 insertions(+), 15 deletions(-) diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index c6b207c9..92b7e5b6 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -210,13 +210,15 @@ func Execute() (code exitCode) { // functions run last-in-first-out). var executedCmd *cobra.Command var executeErr error + var elapsed time.Duration defer func() { - trackCommandCompleted(ctx, executedCmd, code, executeErr, time.Since(start)) + trackCommandCompleted(ctx, executedCmd, code, executeErr, elapsed) }() - // Run the command. + // Run the command. The duration is measured right away so it never + // includes the update-notifier wait below. cmd, err := rootCmd.ExecuteContextC(ctx) - executedCmd, executeErr = cmd, err + executedCmd, executeErr, elapsed = cmd, err, time.Since(start) // Handle eventual errors. if err != nil { if err == cmdutil.ErrSilent { @@ -266,6 +268,13 @@ func trackCommandCompleted( if cmd == nil || !cmdutil.ShouldTrackUsage(cmd) { return } + // An empty command path means PersistentPreRunE never ran (--help, + // --version, unknown flag or command, failed auth check): no Command + // Invoked was sent, so don't send an orphan Command Completed either. + metadata := telemetry.GetEventMetadata(ctx) + if metadata == nil || metadata.CommandPath == "" { + return + } client := telemetry.GetTelemetryClient(ctx) if client == nil { return diff --git a/pkg/cmd/root/root_test.go b/pkg/cmd/root/root_test.go index 5e38dca9..da949661 100644 --- a/pkg/cmd/root/root_test.go +++ b/pkg/cmd/root/root_test.go @@ -2,16 +2,113 @@ package root import ( "bytes" + "context" "errors" "fmt" "net" "testing" + "time" "github.com/spf13/cobra" "github.com/algolia/cli/pkg/cmdutil" + "github.com/algolia/cli/pkg/telemetry" ) +// recordingTelemetryClient captures the tracked events so tests can assert on +// them without hitting the network. +type recordingTelemetryClient struct { + events []recordedEvent +} + +type recordedEvent struct { + name string + props map[string]any +} + +func (r *recordingTelemetryClient) Identify(ctx context.Context) error { return nil } + +func (r *recordingTelemetryClient) Track( + ctx context.Context, + event string, + properties map[string]any, +) error { + r.events = append(r.events, recordedEvent{event, properties}) + return nil +} + +func (r *recordingTelemetryClient) Close() {} + +func newTelemetryContext(client telemetry.TelemetryClient, commandPath string) context.Context { + metadata := telemetry.NewEventMetadata() + metadata.SetCommandPath(commandPath) + ctx := telemetry.WithEventMetadata(context.Background(), metadata) + return telemetry.WithTelemetryClient(ctx, client) +} + +func TestTrackCommandCompleted_SkipsWhenPreRunNeverRan(t *testing.T) { + client := &recordingTelemetryClient{} + // An empty command path means PersistentPreRunE never ran. + ctx := newTelemetryContext(client, "") + + trackCommandCompleted(ctx, &cobra.Command{Use: "algolia"}, exitOK, nil, time.Second) + + if len(client.events) != 0 { + t.Errorf("expected no event, got %d", len(client.events)) + } +} + +func TestTrackCommandCompleted_ReportsSuccess(t *testing.T) { + client := &recordingTelemetryClient{} + ctx := newTelemetryContext(client, "algolia indices list") + + trackCommandCompleted(ctx, &cobra.Command{Use: "list"}, exitOK, nil, 1500*time.Millisecond) + + if len(client.events) != 1 { + t.Fatalf("expected 1 event, got %d", len(client.events)) + } + event := client.events[0] + if event.name != telemetry.EventCommandCompleted { + t.Errorf("event = %q, want %q", event.name, telemetry.EventCommandCompleted) + } + if event.props["succeeded"] != true { + t.Errorf("succeeded = %v, want true", event.props["succeeded"]) + } + if event.props["exit_code"] != 0 { + t.Errorf("exit_code = %v, want 0", event.props["exit_code"]) + } + if event.props["duration_ms"] != int64(1500) { + t.Errorf("duration_ms = %v, want 1500", event.props["duration_ms"]) + } + if _, ok := event.props["error_class"]; ok { + t.Error("unexpected error_class on success") + } +} + +func TestTrackCommandCompleted_ReportsFailure(t *testing.T) { + client := &recordingTelemetryClient{} + ctx := newTelemetryContext(client, "algolia indices list") + + trackCommandCompleted(ctx, &cobra.Command{Use: "list"}, exitError, errors.New("boom"), time.Second) + + if len(client.events) != 1 { + t.Fatalf("expected 1 event, got %d", len(client.events)) + } + props := client.events[0].props + if props["succeeded"] != false { + t.Errorf("succeeded = %v, want false", props["succeeded"]) + } + if props["exit_code"] != 1 { + t.Errorf("exit_code = %v, want 1", props["exit_code"]) + } + if props["error_class"] != "*errors.errorString" { + t.Errorf("error_class = %v, want *errors.errorString", props["error_class"]) + } + if props["user_cancelled"] != false { + t.Errorf("user_cancelled = %v, want false", props["user_cancelled"]) + } +} + func TestPrintError(t *testing.T) { cmd := &cobra.Command{} diff --git a/pkg/telemetry/events.go b/pkg/telemetry/events.go index 8c37dcd9..6d18201f 100644 --- a/pkg/telemetry/events.go +++ b/pkg/telemetry/events.go @@ -108,18 +108,20 @@ func (f *FlowTracker) DurationMS() int64 { return time.Since(f.start).Milliseconds() } -// ErrorClass returns the type of the root cause of an error, never its -// message, which could contain user data. +// ErrorClass returns the type of the first informative error of the chain, +// skipping the anonymous wrappers created by fmt.Errorf. It never returns an +// error message, which could contain user data. func ErrorClass(err error) string { - if err == nil { - return "" - } - for { - unwrapped := errors.Unwrap(err) - if unwrapped == nil { - break + for err != nil { + class := fmt.Sprintf("%T", err) + switch class { + case "*fmt.wrapError", "*fmt.wrapErrors", "*errors.joinError": + if unwrapped := errors.Unwrap(err); unwrapped != nil { + err = unwrapped + continue + } } - err = unwrapped + return class } - return fmt.Sprintf("%T", err) + return "" } diff --git a/pkg/telemetry/events_test.go b/pkg/telemetry/events_test.go index a697db4e..61c5a9ee 100644 --- a/pkg/telemetry/events_test.go +++ b/pkg/telemetry/events_test.go @@ -11,11 +11,21 @@ type testRootError struct{} func (testRootError) Error() string { return "boom" } -func TestErrorClass_ReturnsRootCauseType(t *testing.T) { +type testWrapperError struct{ inner error } + +func (e testWrapperError) Error() string { return "wrap" } +func (e testWrapperError) Unwrap() error { return e.inner } + +func TestErrorClass_SkipsFmtWrappers(t *testing.T) { wrapped := fmt.Errorf("outer: %w", fmt.Errorf("inner: %w", testRootError{})) assert.Equal(t, "telemetry.testRootError", ErrorClass(wrapped)) } +func TestErrorClass_KeepsInformativeWrapperType(t *testing.T) { + wrapped := fmt.Errorf("outer: %w", testWrapperError{inner: testRootError{}}) + assert.Equal(t, "telemetry.testWrapperError", ErrorClass(wrapped)) +} + func TestErrorClass_NilError(t *testing.T) { assert.Equal(t, "", ErrorClass(nil)) } diff --git a/pkg/telemetry/telemetry_test.go b/pkg/telemetry/telemetry_test.go index 3ad0bd96..d9445ce2 100644 --- a/pkg/telemetry/telemetry_test.go +++ b/pkg/telemetry/telemetry_test.go @@ -2,6 +2,7 @@ package telemetry import ( "context" + "sync" "testing" "github.com/segmentio/analytics-go/v3" @@ -76,10 +77,13 @@ func TestSetUser(t *testing.T) { // fakeAnalyticsClient captures the messages enqueued by the telemetry client so // tests can assert on the payload without hitting the network. type fakeAnalyticsClient struct { + mu sync.Mutex messages []analytics.Message } func (f *fakeAnalyticsClient) Enqueue(msg analytics.Message) error { + f.mu.Lock() + defer f.mu.Unlock() f.messages = append(f.messages, msg) return nil } @@ -191,6 +195,36 @@ func TestTrack_SequenceIsMonotonic(t *testing.T) { } } +func TestTrack_SequenceIsUniqueUnderConcurrency(t *testing.T) { + fake := &fakeAnalyticsClient{} + client := &AnalyticsTelemetryClient{client: fake} + + metadata := NewEventMetadata() + ctx := WithEventMetadata(context.Background(), metadata) + + const n = 100 + var wg sync.WaitGroup + for i := 0; i < n; i++ { + wg.Add(1) + go func() { + defer wg.Done() + _ = client.Track(ctx, "Command Invoked", nil) + }() + } + wg.Wait() + + require.Len(t, fake.messages, n) + seen := make(map[int64]bool, n) + for _, msg := range fake.messages { + track, ok := msg.(analytics.Track) + require.True(t, ok) + seq, ok := track.Properties["sequence"].(int64) + require.True(t, ok) + assert.False(t, seen[seq], "duplicate sequence %d", seq) + seen[seq] = true + } +} + func TestTrack_CustomPropertiesCannotOverrideBase(t *testing.T) { fake := &fakeAnalyticsClient{} client := &AnalyticsTelemetryClient{client: fake} From 6666c42873b70939c1b3e79649f99a943180cc80 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Wed, 10 Jun 2026 14:39:48 -0700 Subject: [PATCH 5/6] feat(telemetry): instrument the auth flow with funnel events - RunOAuthFlow emits CLI Auth Started at entry, then exactly one terminal event: Completed, Aborted (user cancellation) or Failed, with the step the user stopped at - RunOAuth takes an optional nil-safe FlowTracker and records the browser_wait and code_exchange steps; automatic re-authentications (EnsureAuthenticated, ReauthenticateIfExpired) pass nil so they are not counted as auth funnels - new Event struct, typed constructors and TrackEvent helper: one-line call sites, compile-time consistent properties - identifyAuthenticatedUser uses the context client; IdentifyOnce is removed (it only existed because the client used to be closed before the command ran) - telemetrytest.RecordingClient: shared recording fake for flow tests Co-Authored-By: Claude Fable 5 --- pkg/auth/authenticate.go | 8 +++- pkg/auth/oauth_flow.go | 13 +++++- pkg/cmd/auth/login/login.go | 51 +++++++++++++++++++++-- pkg/cmd/auth/login/login_test.go | 48 +++++++++++++++++++++ pkg/telemetry/events.go | 53 ++++++++++++++++++++++++ pkg/telemetry/events_test.go | 41 ++++++++++++++++++ pkg/telemetry/telemetry.go | 21 ---------- pkg/telemetry/telemetrytest/recording.go | 45 ++++++++++++++++++++ 8 files changed, 253 insertions(+), 27 deletions(-) create mode 100644 pkg/telemetry/telemetrytest/recording.go diff --git a/pkg/auth/authenticate.go b/pkg/auth/authenticate.go index af0df076..3ca2f711 100644 --- a/pkg/auth/authenticate.go +++ b/pkg/auth/authenticate.go @@ -23,7 +23,9 @@ func EnsureAuthenticated( cs := io.ColorScheme() fmt.Fprintf(io.Out, "%s %s\n", cs.WarningIcon(), err) - return RunOAuth(io, client, false, true) + // No flow tracker: this re-authentication belongs to the calling flow, + // not to an `auth login` funnel. + return RunOAuth(io, client, false, true, nil) } // ReauthenticateIfExpired checks if err is a session-expired error from the API. @@ -41,5 +43,7 @@ func ReauthenticateIfExpired( ClearToken() fmt.Fprintf(io.Out, "%s Session expired.\n", cs.WarningIcon()) - return RunOAuth(io, client, false, true) + // No flow tracker: this re-authentication belongs to the calling flow, + // not to an `auth login` funnel. + return RunOAuth(io, client, false, true, nil) } diff --git a/pkg/auth/oauth_flow.go b/pkg/auth/oauth_flow.go index a4334f43..31d1ac29 100644 --- a/pkg/auth/oauth_flow.go +++ b/pkg/auth/oauth_flow.go @@ -8,6 +8,7 @@ import ( "github.com/algolia/cli/api/dashboard" "github.com/algolia/cli/pkg/iostreams" + "github.com/algolia/cli/pkg/telemetry" ) // DefaultOAuthClientID is a public OAuth client ID (PKCE flow, not a secret). @@ -35,7 +36,15 @@ func OAuthClientID() string { // launched, e.g. SSH / containers). // // If signup is true the browser opens to the sign-up page. -func RunOAuth(io *iostreams.IOStreams, client *dashboard.Client, signup, openBrowser bool) (string, error) { +// +// The optional tracker (nil-safe) records which step the flow is in, so the +// telemetry of the calling flow can tell where the user stopped. +func RunOAuth( + io *iostreams.IOStreams, + client *dashboard.Client, + signup, openBrowser bool, + tracker *telemetry.FlowTracker, +) (string, error) { cs := io.ColorScheme() redirectURI, resultCh, err := StartCallbackServer() @@ -69,6 +78,7 @@ func RunOAuth(io *iostreams.IOStreams, client *dashboard.Client, signup, openBro } fmt.Fprintf(io.Out, "Waiting for authentication...\n") + tracker.SetStep(telemetry.StepBrowserWait) cbResult := <-resultCh if cbResult.Error != "" { @@ -78,6 +88,7 @@ func RunOAuth(io *iostreams.IOStreams, client *dashboard.Client, signup, openBro return "", fmt.Errorf("no authorization code received") } + tracker.SetStep(telemetry.StepCodeExchange) io.StartProgressIndicatorWithLabel("Exchanging code for tokens") tokenResp, err := client.AuthorizationCodeGrant(cbResult.Code, codeVerifier, redirectURI) io.StopProgressIndicator() diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 7fdb1461..e3c844ac 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -92,17 +92,54 @@ func runLoginCmd(ctx context.Context, opts *LoginOptions) error { // RunOAuthFlow runs the full browser-based OAuth + profile setup flow. // If signup is true, the browser opens to the sign-up page instead of sign-in. func RunOAuthFlow(ctx context.Context, opts *LoginOptions, signup bool) error { + flow := telemetry.FlowLogin + if signup { + flow = telemetry.FlowSignup + } + tracker := telemetry.NewFlowTracker() + telemetry.TrackEvent(ctx, telemetry.AuthStarted(flow, opts.NoBrowser)) + + err := runOAuthFlowSteps(ctx, opts, signup, tracker) + trackOAuthFlowOutcome(ctx, flow, tracker, err) + return err +} + +// trackOAuthFlowOutcome reports how the auth flow ended: completed, aborted by +// the user, or failed. +func trackOAuthFlowOutcome( + ctx context.Context, + flow telemetry.Flow, + tracker *telemetry.FlowTracker, + err error, +) { + switch { + case err == nil: + telemetry.TrackEvent(ctx, telemetry.AuthCompleted(flow, tracker)) + case cmdutil.IsUserCancellation(err): + telemetry.TrackEvent(ctx, telemetry.AuthAborted(flow, tracker)) + default: + telemetry.TrackEvent(ctx, telemetry.AuthFailed(flow, tracker, err)) + } +} + +func runOAuthFlowSteps( + ctx context.Context, + opts *LoginOptions, + signup bool, + tracker *telemetry.FlowTracker, +) error { cs := opts.IO.ColorScheme() client := opts.NewDashboardClient(auth.OAuthClientID()) openBrowser := !opts.NoBrowser - accessToken, err := auth.RunOAuth(opts.IO, client, signup, openBrowser) + accessToken, err := auth.RunOAuth(opts.IO, client, signup, openBrowser, tracker) if err != nil { return err } identifyAuthenticatedUser(ctx) + tracker.SetStep(telemetry.StepAppsFetch) opts.IO.StartProgressIndicatorWithLabel("Fetching applications") apps, err := client.ListApplications(accessToken) opts.IO.StopProgressIndicator() @@ -115,11 +152,13 @@ func RunOAuthFlow(ctx context.Context, opts *LoginOptions, signup bool) error { if len(apps) == 0 { fmt.Fprintf(opts.IO.Out, "\n%s No applications found. Let's create one.\n", cs.WarningIcon()) + tracker.SetStep(telemetry.StepAppCreate) appDetails, err = apputil.CreateAndFetchApplication(opts.IO, client, accessToken, "", opts.AppName) if err != nil { return err } } else { + tracker.SetStep(telemetry.StepAppSelect) interactive := opts.IO.CanPrompt() app, err := selectApplication(opts, apps, interactive) if err != nil { @@ -139,15 +178,21 @@ func RunOAuthFlow(ctx context.Context, opts *LoginOptions, signup bool) error { profileName = appDetails.Name } + tracker.SetStep(telemetry.StepProfileConfigure) 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) { - telemetry.IdentifyOnce(ctx) + 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 diff --git a/pkg/cmd/auth/login/login_test.go b/pkg/cmd/auth/login/login_test.go index ed818558..a1a45d70 100644 --- a/pkg/cmd/auth/login/login_test.go +++ b/pkg/cmd/auth/login/login_test.go @@ -2,6 +2,7 @@ package login import ( "context" + "errors" "testing" "github.com/stretchr/testify/assert" @@ -13,6 +14,7 @@ import ( "github.com/algolia/cli/pkg/cmdutil" "github.com/algolia/cli/pkg/iostreams" "github.com/algolia/cli/pkg/telemetry" + "github.com/algolia/cli/pkg/telemetry/telemetrytest" "github.com/algolia/cli/test" ) @@ -130,6 +132,52 @@ func TestApplyStoredIdentity_TokenWithoutIdentityReturnsFalse(t *testing.T) { assert.Empty(t, metadata.UserID) } +func TestTrackOAuthFlowOutcome(t *testing.T) { + tests := []struct { + name string + err error + wantEvent string + wantStep bool + }{ + { + name: "success", + err: nil, + wantEvent: telemetry.EventAuthCompleted, + }, + { + name: "user cancellation", + err: cmdutil.ErrCancel, + wantEvent: telemetry.EventAuthAborted, + wantStep: true, + }, + { + name: "failure", + err: errors.New("boom"), + wantEvent: telemetry.EventAuthFailed, + wantStep: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := &telemetrytest.RecordingClient{} + ctx := telemetry.WithTelemetryClient(context.Background(), client) + tracker := telemetry.NewFlowTracker() + tracker.SetStep(telemetry.StepAppsFetch) + + trackOAuthFlowOutcome(ctx, telemetry.FlowLogin, tracker, tt.err) + + require.Len(t, client.Events, 1) + event := client.Events[0] + assert.Equal(t, tt.wantEvent, event.Name) + assert.Equal(t, telemetry.FlowLogin, event.Properties["flow"]) + if tt.wantStep { + assert.Equal(t, telemetry.StepAppsFetch, event.Properties["step"]) + } + }) + } +} + func TestSelectApplication_MultipleApps_NonInteractive_NoAppName(t *testing.T) { io, _, _, _ := iostreams.Test() opts := &LoginOptions{IO: io} diff --git a/pkg/telemetry/events.go b/pkg/telemetry/events.go index 6d18201f..ea391b59 100644 --- a/pkg/telemetry/events.go +++ b/pkg/telemetry/events.go @@ -1,6 +1,7 @@ package telemetry import ( + "context" "errors" "fmt" "time" @@ -108,6 +109,58 @@ func (f *FlowTracker) DurationMS() int64 { return time.Since(f.start).Milliseconds() } +// Event is a fully assembled telemetry event, ready to be tracked. +type Event struct { + Name string + Properties map[string]any +} + +// TrackEvent sends the event through the context's telemetry client, silently +// doing nothing when no client is present. +func TrackEvent(ctx context.Context, event Event) { + client := GetTelemetryClient(ctx) + if client == nil { + return + } + _ = client.Track(ctx, event.Name, event.Properties) +} + +// AuthStarted is emitted when the browser-based OAuth flow begins. +func AuthStarted(flow Flow, noBrowser bool) Event { + return Event{EventAuthStarted, map[string]any{ + "flow": flow, + "no_browser": noBrowser, + }} +} + +// AuthCompleted is emitted when the profile is fully configured at the end of +// the auth flow. +func AuthCompleted(flow Flow, tracker *FlowTracker) Event { + return Event{EventAuthCompleted, map[string]any{ + "flow": flow, + "duration_ms": tracker.DurationMS(), + }} +} + +// AuthAborted is emitted when the user cancelled the auth flow, with the step +// they stopped at. +func AuthAborted(flow Flow, tracker *FlowTracker) Event { + return Event{EventAuthAborted, map[string]any{ + "flow": flow, + "step": tracker.Step(), + }} +} + +// AuthFailed is emitted when the auth flow failed, with the step it failed at. +func AuthFailed(flow Flow, tracker *FlowTracker, err error) Event { + return Event{EventAuthFailed, map[string]any{ + "flow": flow, + "step": tracker.Step(), + "duration_ms": tracker.DurationMS(), + "error_class": ErrorClass(err), + }} +} + // ErrorClass returns the type of the first informative error of the chain, // skipping the anonymous wrappers created by fmt.Errorf. It never returns an // error message, which could contain user data. diff --git a/pkg/telemetry/events_test.go b/pkg/telemetry/events_test.go index 61c5a9ee..7127d30e 100644 --- a/pkg/telemetry/events_test.go +++ b/pkg/telemetry/events_test.go @@ -1,6 +1,8 @@ package telemetry import ( + "context" + "errors" "fmt" "testing" @@ -30,6 +32,45 @@ func TestErrorClass_NilError(t *testing.T) { assert.Equal(t, "", ErrorClass(nil)) } +func TestTrackEvent_NoClientInContextIsSafe(t *testing.T) { + // Must not panic when the context carries no telemetry client. + TrackEvent(context.Background(), AuthStarted(FlowLogin, false)) +} + +func TestAuthStarted(t *testing.T) { + event := AuthStarted(FlowSignup, true) + assert.Equal(t, EventAuthStarted, event.Name) + assert.Equal(t, FlowSignup, event.Properties["flow"]) + assert.Equal(t, true, event.Properties["no_browser"]) +} + +func TestAuthCompleted(t *testing.T) { + event := AuthCompleted(FlowLogin, NewFlowTracker()) + assert.Equal(t, EventAuthCompleted, event.Name) + assert.Equal(t, FlowLogin, event.Properties["flow"]) + assert.Contains(t, event.Properties, "duration_ms") +} + +func TestAuthAborted(t *testing.T) { + tracker := NewFlowTracker() + tracker.SetStep(StepAppSelect) + + event := AuthAborted(FlowLogin, tracker) + assert.Equal(t, EventAuthAborted, event.Name) + assert.Equal(t, StepAppSelect, event.Properties["step"]) +} + +func TestAuthFailed(t *testing.T) { + tracker := NewFlowTracker() + tracker.SetStep(StepAppsFetch) + + event := AuthFailed(FlowLogin, tracker, errors.New("boom")) + assert.Equal(t, EventAuthFailed, event.Name) + assert.Equal(t, StepAppsFetch, event.Properties["step"]) + assert.Equal(t, "*errors.errorString", event.Properties["error_class"]) + assert.Contains(t, event.Properties, "duration_ms") +} + func TestFlowTracker_NilTrackerIsSafe(t *testing.T) { var tracker *FlowTracker tracker.SetStep(StepTerms) diff --git a/pkg/telemetry/telemetry.go b/pkg/telemetry/telemetry.go index 9330d055..d0a4c918 100644 --- a/pkg/telemetry/telemetry.go +++ b/pkg/telemetry/telemetry.go @@ -6,7 +6,6 @@ import ( "fmt" "log" "net" - "os" "runtime" "sync/atomic" @@ -76,26 +75,6 @@ func NewAnalyticsTelemetryClient(debug bool) (TelemetryClient, error) { return &AnalyticsTelemetryClient{client: client}, nil } -// IdentifyOnce sends a single Identify event through a short-lived client and -// flushes it before returning. It is meant for one-shot identification (for -// example, right after authentication fills the token) where the command's -// request-scoped client may already have been closed. It honors the same -// ALGOLIA_CLI_TELEMETRY and DEBUG environment variables as the root command and -// fails silently so telemetry never blocks the user. -func IdentifyOnce(ctx context.Context) { - if os.Getenv("ALGOLIA_CLI_TELEMETRY") == "0" { - return - } - - client, err := NewAnalyticsTelemetryClient(os.Getenv("DEBUG") != "") - if err != nil { - return - } - defer client.Close() - - _ = client.Identify(ctx) -} - // anonymousID is a unique identifier for an anonymous user of the CLI (basically the hash of the mac address) func anonymousID() string { addrs, err := net.Interfaces() diff --git a/pkg/telemetry/telemetrytest/recording.go b/pkg/telemetry/telemetrytest/recording.go new file mode 100644 index 00000000..bea7ae86 --- /dev/null +++ b/pkg/telemetry/telemetrytest/recording.go @@ -0,0 +1,45 @@ +// Package telemetrytest provides test doubles for the telemetry package. +package telemetrytest + +import ( + "context" + "sync" + + "github.com/algolia/cli/pkg/telemetry" +) + +var _ telemetry.TelemetryClient = (*RecordingClient)(nil) + +// RecordedEvent is one event captured by a RecordingClient. +type RecordedEvent struct { + Name string + Properties map[string]any +} + +// RecordingClient is a telemetry.TelemetryClient that records the tracked +// events so tests can assert on their names, properties and order. +type RecordingClient struct { + mu sync.Mutex + Events []RecordedEvent + Identifies int +} + +func (r *RecordingClient) Identify(ctx context.Context) error { + r.mu.Lock() + defer r.mu.Unlock() + r.Identifies++ + return nil +} + +func (r *RecordingClient) Track( + ctx context.Context, + event string, + properties map[string]any, +) error { + r.mu.Lock() + defer r.mu.Unlock() + r.Events = append(r.Events, RecordedEvent{event, properties}) + return nil +} + +func (r *RecordingClient) Close() {} From ca71a88ec663d58e59e7fea0efbf2a9403a5d8a1 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Thu, 11 Jun 2026 10:07:24 -0700 Subject: [PATCH 6/6] feat(telemetry): logout flow event (#242) Co-authored-by: Claude Fable 5 --- pkg/cmd/auth/logout/logout.go | 10 +++- pkg/cmd/auth/logout/logout_test.go | 73 ++++++++++++++++++++++++++++++ pkg/telemetry/events.go | 12 +++++ 3 files changed, 93 insertions(+), 2 deletions(-) create mode 100644 pkg/cmd/auth/logout/logout_test.go diff --git a/pkg/cmd/auth/logout/logout.go b/pkg/cmd/auth/logout/logout.go index 6205dc3e..1b0c7b7b 100644 --- a/pkg/cmd/auth/logout/logout.go +++ b/pkg/cmd/auth/logout/logout.go @@ -1,6 +1,7 @@ package logout import ( + "context" "fmt" "github.com/MakeNowJust/heredoc" @@ -10,6 +11,7 @@ import ( "github.com/algolia/cli/pkg/auth" "github.com/algolia/cli/pkg/cmdutil" "github.com/algolia/cli/pkg/iostreams" + "github.com/algolia/cli/pkg/telemetry" "github.com/algolia/cli/pkg/validators" ) @@ -38,14 +40,14 @@ func NewLogoutCmd(f *cmdutil.Factory) *cobra.Command { `), Args: validators.NoArgs(), RunE: func(cmd *cobra.Command, args []string) error { - return runLogoutCmd(opts) + return runLogoutCmd(cmd.Context(), opts) }, } return cmd } -func runLogoutCmd(opts *LogoutOptions) error { +func runLogoutCmd(ctx context.Context, opts *LogoutOptions) error { cs := opts.IO.ColorScheme() stored := auth.LoadToken() @@ -68,6 +70,10 @@ func runLogoutCmd(opts *LogoutOptions) error { } } + // Tracked before clearing the local state, while the user identifier is + // still attached to the telemetry metadata. + telemetry.TrackEvent(ctx, telemetry.AuthLogout()) + auth.ClearToken() fmt.Fprintf(opts.IO.Out, "%s Signed out successfully.\n", cs.SuccessIcon()) diff --git a/pkg/cmd/auth/logout/logout_test.go b/pkg/cmd/auth/logout/logout_test.go new file mode 100644 index 00000000..dc75bd27 --- /dev/null +++ b/pkg/cmd/auth/logout/logout_test.go @@ -0,0 +1,73 @@ +package logout + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/zalando/go-keyring" + + "github.com/algolia/cli/api/dashboard" + "github.com/algolia/cli/pkg/auth" + "github.com/algolia/cli/pkg/iostreams" + "github.com/algolia/cli/pkg/telemetry" + "github.com/algolia/cli/pkg/telemetry/telemetrytest" +) + +func newLogoutOpts(srv *httptest.Server) *LogoutOptions { + io, _, _, _ := iostreams.Test() + return &LogoutOptions{ + IO: io, + NewDashboardClient: func(string) *dashboard.Client { + c := dashboard.NewClientWithHTTPClient("test", srv.Client()) + c.DashboardURL = srv.URL + return c + }, + } +} + +func TestLogout_EmitsAuthLogoutBeforeClearingToken(t *testing.T) { + keyring.MockInit() + t.Cleanup(auth.ClearToken) + require.NoError(t, auth.SaveToken(&dashboard.OAuthTokenResponse{ + AccessToken: "access", + RefreshToken: "refresh", + ExpiresIn: 3600, + })) + + mux := http.NewServeMux() + mux.HandleFunc("/2/oauth/revoke", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + client := &telemetrytest.RecordingClient{} + ctx := telemetry.WithTelemetryClient(context.Background(), client) + + require.NoError(t, runLogoutCmd(ctx, newLogoutOpts(srv))) + + require.Len(t, client.Events, 1) + event := client.Events[0] + assert.Equal(t, telemetry.EventAuthLogout, event.Name) + assert.Equal(t, telemetry.FlowLogout, event.Properties["flow"]) + // The token is cleared after the event was tracked. + assert.Nil(t, auth.LoadToken()) + // No Identify at logout time: Segment cannot un-identify. + assert.Zero(t, client.Identifies) +} + +func TestLogout_AlreadySignedOutEmitsNothing(t *testing.T) { + keyring.MockInit() + auth.ClearToken() + + client := &telemetrytest.RecordingClient{} + ctx := telemetry.WithTelemetryClient(context.Background(), client) + + require.NoError(t, runLogoutCmd(ctx, newLogoutOpts(nil))) + + assert.Empty(t, client.Events) +} diff --git a/pkg/telemetry/events.go b/pkg/telemetry/events.go index ea391b59..82fccbd6 100644 --- a/pkg/telemetry/events.go +++ b/pkg/telemetry/events.go @@ -18,6 +18,7 @@ const ( EventAuthCompleted = "CLI Auth Completed" EventAuthFailed = "CLI Auth Failed" EventAuthAborted = "CLI Auth Aborted" + EventAuthLogout = "CLI Auth Logout" EventApplicationCreateStarted = "CLI Application Create Started" EventApplicationCreateAcceptedTerms = "CLI Application Create Accepted Terms" @@ -40,6 +41,7 @@ type Flow string const ( FlowLogin Flow = "login" FlowSignup Flow = "signup" + FlowLogout Flow = "logout" ) // Step locates where the user is inside an interactive flow, so aborts and @@ -151,6 +153,16 @@ func AuthAborted(flow Flow, tracker *FlowTracker) Event { }} } +// AuthLogout is emitted when the user signs out. It must be tracked before +// the local state is cleared, while the user identifier is still attached to +// the telemetry metadata; no Identify follows, since Segment's identity graph +// has no concept of un-identifying. +func AuthLogout() Event { + return Event{EventAuthLogout, map[string]any{ + "flow": FlowLogout, + }} +} + // AuthFailed is emitted when the auth flow failed, with the step it failed at. func AuthFailed(flow Flow, tracker *FlowTracker, err error) Event { return Event{EventAuthFailed, map[string]any{