From a4d66707286a89cd7777d77bbca654f2d5559c70 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Thu, 4 Jun 2026 12:00:53 -0700 Subject: [PATCH 1/7] chore(events): add cli-emitted events --- api/dashboard/client.go | 21 +- api/dashboard/types.go | 11 + pkg/auth/authenticate.go | 7 +- pkg/auth/oauth_flow.go | 52 +++- pkg/cmd/application/create/create.go | 20 +- pkg/cmd/application/create/create_test.go | 33 +-- pkg/cmd/application/downgrade/downgrade.go | 2 +- pkg/cmd/application/planchange/planchange.go | 21 +- .../application/planchange/planchange_test.go | 35 +-- pkg/cmd/application/upgrade/upgrade.go | 2 +- pkg/cmd/auth/login/login.go | 57 +++- pkg/cmd/root/root.go | 83 +++++- pkg/cmd/shared/apputil/create.go | 53 +++- pkg/cmdutil/classify.go | 90 +++++++ pkg/cmdutil/classify_test.go | 91 +++++++ pkg/telemetry/event.go | 16 ++ pkg/telemetry/events.go | 250 ++++++++++++++++++ pkg/telemetry/telemetry.go | 125 ++++++++- pkg/telemetry/telemetry_test.go | 156 ++++++++++- 19 files changed, 1028 insertions(+), 97 deletions(-) create mode 100644 pkg/cmdutil/classify.go create mode 100644 pkg/cmdutil/classify_test.go create mode 100644 pkg/telemetry/event.go create mode 100644 pkg/telemetry/events.go diff --git a/api/dashboard/client.go b/api/dashboard/client.go index 46d77a5e..0d1be53b 100644 --- a/api/dashboard/client.go +++ b/api/dashboard/client.go @@ -335,11 +335,14 @@ func (c *Client) CreateApplication(accessToken, region, name string) (*Applicati } } - return nil, fmt.Errorf( - "create application failed with status %d: %s", - resp.StatusCode, - respStr, - ) + return nil, &APIError{ + StatusCode: resp.StatusCode, + Message: fmt.Sprintf( + "create application failed with status %d: %s", + resp.StatusCode, + respStr, + ), + } } var singleResp SingleApplicationResponse @@ -493,7 +496,13 @@ func (c *Client) ChangeApplicationPlan(accessToken, appID, plan string) (*Applic return nil, ErrSessionExpired } if resp.StatusCode < 200 || resp.StatusCode >= 300 { - return nil, fmt.Errorf("Couldn't change your application's plan: %d", resp.StatusCode) + return nil, &APIError{ + StatusCode: resp.StatusCode, + Message: fmt.Sprintf( + "Couldn't change your application's plan: %d", + resp.StatusCode, + ), + } } respBody, _ := io.ReadAll(resp.Body) diff --git a/api/dashboard/types.go b/api/dashboard/types.go index da309f4d..edf9f285 100644 --- a/api/dashboard/types.go +++ b/api/dashboard/types.go @@ -106,6 +106,17 @@ type RegionsResponse struct { // ErrSessionExpired is returned when an API call gets a 401 Unauthorized. var ErrSessionExpired = errors.New("session expired") +// APIError is returned for non-2xx dashboard responses. It carries the HTTP +// status so callers (and telemetry) can branch on it, keeping the message. +type APIError struct { + StatusCode int + Message string +} + +func (e *APIError) Error() string { return e.Message } + +func (e *APIError) HTTPStatusCode() int { return e.StatusCode } + // ErrClusterUnavailable is returned when a region has no available cluster. type ErrClusterUnavailable struct { Region string diff --git a/pkg/auth/authenticate.go b/pkg/auth/authenticate.go index af0df076..a8698d3d 100644 --- a/pkg/auth/authenticate.go +++ b/pkg/auth/authenticate.go @@ -1,6 +1,7 @@ package auth import ( + "context" "errors" "fmt" @@ -23,7 +24,9 @@ func EnsureAuthenticated( cs := io.ColorScheme() fmt.Fprintf(io.Out, "%s %s\n", cs.WarningIcon(), err) - return RunOAuth(io, client, false, true) + // Lazy login from another command: no request-scoped telemetry context here, + // so OAuth flow events are emitted only by the dedicated auth login/signup commands. + return RunOAuth(context.Background(), io, client, false, true) } // ReauthenticateIfExpired checks if err is a session-expired error from the API. @@ -41,5 +44,5 @@ func ReauthenticateIfExpired( ClearToken() fmt.Fprintf(io.Out, "%s Session expired.\n", cs.WarningIcon()) - return RunOAuth(io, client, false, true) + return RunOAuth(context.Background(), io, client, false, true) } diff --git a/pkg/auth/oauth_flow.go b/pkg/auth/oauth_flow.go index a4334f43..e51d8f65 100644 --- a/pkg/auth/oauth_flow.go +++ b/pkg/auth/oauth_flow.go @@ -1,13 +1,17 @@ package auth import ( + "context" "fmt" "os" "os/exec" "runtime" + "time" "github.com/algolia/cli/api/dashboard" + "github.com/algolia/cli/pkg/cmdutil" "github.com/algolia/cli/pkg/iostreams" + "github.com/algolia/cli/pkg/telemetry" ) // DefaultOAuthClientID is a public OAuth client ID (PKCE flow, not a secret). @@ -20,7 +24,10 @@ func OAuthClientID() string { return v } if DefaultOAuthClientID == "" { - fmt.Fprintln(os.Stderr, "fatal: ALGOLIA_OAUTH_CLIENT_ID is not set and no default was compiled in") + fmt.Fprintln( + os.Stderr, + "fatal: ALGOLIA_OAUTH_CLIENT_ID is not set and no default was compiled in", + ) os.Exit(1) } return DefaultOAuthClientID @@ -35,11 +42,27 @@ 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) { +func RunOAuth( + ctx context.Context, + io *iostreams.IOStreams, + client *dashboard.Client, + signup, openBrowser bool, +) (string, error) { cs := io.ColorScheme() + flow := telemetry.FlowLogin + if signup { + flow = telemetry.FlowSignup + } + start := time.Now() + telemetry.Track(ctx, telemetry.AuthStarted(flow, !openBrowser)) + redirectURI, resultCh, err := StartCallbackServer() if err != nil { + telemetry.Track( + ctx, + telemetry.AuthFailed(flow, telemetry.AuthStepCallback, cmdutil.ErrorClass(err)), + ) return "", err } @@ -63,25 +86,44 @@ func RunOAuth(io *iostreams.IOStreams, client *dashboard.Client, signup, openBro fmt.Fprintf(io.Out, "Opening browser to sign in...\n") } fmt.Fprintf(io.Out, "If the browser doesn't open, visit:\n %s\n\n", cs.Bold(authorizeURL)) - _ = OpenBrowser(authorizeURL) + if browserErr := OpenBrowser(authorizeURL); browserErr != nil { + telemetry.Track(ctx, telemetry.AuthBrowserFailed(flow, cmdutil.ErrorClass(browserErr))) + } else { + telemetry.Track(ctx, telemetry.AuthBrowserOpened(flow)) + } } else { fmt.Fprintf(io.Out, "Open this URL in your browser to authenticate:\n\n %s\n\n", cs.Bold(authorizeURL)) } fmt.Fprintf(io.Out, "Waiting for authentication...\n") cbResult := <-resultCh + telemetry.Track(ctx, telemetry.AuthCallbackReceived(flow, time.Since(start))) if cbResult.Error != "" { - return "", fmt.Errorf("authorization failed: %s", cbResult.Error) + err := fmt.Errorf("authorization failed: %s", cbResult.Error) + telemetry.Track( + ctx, + telemetry.AuthFailed(flow, telemetry.AuthStepCallback, cmdutil.ErrorClass(err)), + ) + return "", err } if cbResult.Code == "" { - return "", fmt.Errorf("no authorization code received") + err := fmt.Errorf("no authorization code received") + telemetry.Track( + ctx, + telemetry.AuthFailed(flow, telemetry.AuthStepCallback, cmdutil.ErrorClass(err)), + ) + return "", err } io.StartProgressIndicatorWithLabel("Exchanging code for tokens") tokenResp, err := client.AuthorizationCodeGrant(cbResult.Code, codeVerifier, redirectURI) io.StopProgressIndicator() if err != nil { + telemetry.Track( + ctx, + telemetry.AuthFailed(flow, telemetry.AuthStepExchange, cmdutil.ErrorClass(err)), + ) return "", err } diff --git a/pkg/cmd/application/create/create.go b/pkg/cmd/application/create/create.go index ed623900..14911394 100644 --- a/pkg/cmd/application/create/create.go +++ b/pkg/cmd/application/create/create.go @@ -1,6 +1,7 @@ package create import ( + "context" "fmt" "strings" @@ -16,6 +17,7 @@ import ( "github.com/algolia/cli/pkg/iostreams" pkgopen "github.com/algolia/cli/pkg/open" "github.com/algolia/cli/pkg/prompt" + "github.com/algolia/cli/pkg/telemetry" "github.com/algolia/cli/pkg/validators" ) @@ -78,7 +80,7 @@ func NewCreateCmd(f *cmdutil.Factory) *cobra.Command { }, RunE: func(cmd *cobra.Command, args []string) error { opts.nameProvided = cmd.Flags().Changed("name") - return runCreateCmd(opts) + return runCreateCmd(cmd.Context(), opts) }, } @@ -99,7 +101,7 @@ func NewCreateCmd(f *cmdutil.Factory) *cobra.Command { return cmd } -func runCreateCmd(opts *CreateOptions) error { +func runCreateCmd(ctx context.Context, opts *CreateOptions) error { cs := opts.IO.ColorScheme() name, err := resolveName(opts) @@ -180,11 +182,23 @@ func runCreateCmd(opts *CreateOptions) error { return err } if !accepted { + telemetry.Track( + ctx, + telemetry.ApplicationCreateAborted(telemetry.TriggeredFromExplicitCommand), + ) fmt.Fprintf(opts.IO.Out, "%s Aborted; no application was created.\n", cs.WarningIcon()) return nil } - appDetails, err := apputil.CreateAndFetchApplication(opts.IO, client, token, opts.Region, name) + appDetails, err := apputil.CreateAndFetchApplication( + ctx, + opts.IO, + client, + token, + opts.Region, + name, + telemetry.TriggeredFromExplicitCommand, + ) if err != nil { return err } diff --git a/pkg/cmd/application/create/create_test.go b/pkg/cmd/application/create/create_test.go index 097c2adb..c8cd6048 100644 --- a/pkg/cmd/application/create/create_test.go +++ b/pkg/cmd/application/create/create_test.go @@ -1,6 +1,7 @@ package create import ( + "context" "encoding/json" "net/http" "net/http/httptest" @@ -212,7 +213,7 @@ func TestRun_FreeNonInteractive(t *testing.T) { opts, out, _ := newOpts(t, srv, false) opts.AcceptTerms = true - require.NoError(t, runCreateCmd(opts)) + require.NoError(t, runCreateCmd(context.Background(), opts)) assert.Equal(t, 1, srv.createCalls) assert.Equal(t, 0, srv.patchCalls) assert.Contains(t, out.String(), "APP1") @@ -224,7 +225,7 @@ func TestRun_NonInteractiveRequiresAcceptTerms(t *testing.T) { opts, _, _ := newOpts(t, srv, false) - err := runCreateCmd(opts) + err := runCreateCmd(context.Background(), opts) require.Error(t, err) assert.Contains(t, err.Error(), "must be accepted") assert.Equal(t, 0, srv.createCalls) @@ -238,7 +239,7 @@ func TestRun_PaidWithBillingNonInteractive(t *testing.T) { opts.Plan = "grow" opts.AcceptTerms = true - require.NoError(t, runCreateCmd(opts)) + require.NoError(t, runCreateCmd(context.Background(), opts)) assert.Equal(t, 1, srv.createCalls) assert.Equal(t, 1, srv.patchCalls) assert.Equal(t, "grow", srv.lastPlan) @@ -252,7 +253,7 @@ func TestRun_PaidWithBillingRequiresAcceptTerms(t *testing.T) { opts, _, _ := newOpts(t, srv, false) opts.Plan = "grow" - err := runCreateCmd(opts) + err := runCreateCmd(context.Background(), opts) require.Error(t, err) assert.Contains(t, err.Error(), "must be accepted") assert.Equal(t, 0, srv.createCalls) @@ -267,7 +268,7 @@ func TestRun_PaidNoBillingNonInteractive(t *testing.T) { opts.Plan = "grow" opts.AcceptTerms = true - err := runCreateCmd(opts) + err := runCreateCmd(context.Background(), opts) require.Error(t, err) assert.Contains(t, err.Error(), "payment method") assert.Equal(t, 0, srv.createCalls) @@ -284,7 +285,7 @@ func TestRun_PaidNoBillingInteractiveOpensBilling(t *testing.T) { opts, _, opened := newOpts(t, srv, true) opts.Plan = "grow" - require.NoError(t, runCreateCmd(opts)) + require.NoError(t, runCreateCmd(context.Background(), opts)) assert.Equal(t, 0, srv.createCalls) assert.Equal( t, @@ -302,7 +303,7 @@ func TestRun_PaidNoBillingInteractiveDeclineOpen(t *testing.T) { opts, _, opened := newOpts(t, srv, true) opts.Plan = "grow" - require.NoError(t, runCreateCmd(opts)) + require.NoError(t, runCreateCmd(context.Background(), opts)) assert.Equal(t, 0, srv.createCalls) assert.Empty(t, *opened) } @@ -316,7 +317,7 @@ func TestRun_ToSDeclineAborts(t *testing.T) { opts, out, _ := newOpts(t, srv, true) opts.Plan = "free" - require.NoError(t, runCreateCmd(opts)) + require.NoError(t, runCreateCmd(context.Background(), opts)) assert.Equal(t, 0, srv.createCalls) assert.Contains(t, out.String(), "Aborted") } @@ -332,7 +333,7 @@ func TestRun_AcceptTermsSkipsPromptInteractive(t *testing.T) { opts.Plan = "free" opts.AcceptTerms = true - require.NoError(t, runCreateCmd(opts)) + require.NoError(t, runCreateCmd(context.Background(), opts)) assert.Equal(t, 1, srv.createCalls) assert.Contains(t, out.String(), "Terms accepted via --accept-terms") } @@ -346,7 +347,7 @@ func TestRun_PaidPlanHiddenByServerNonInteractive(t *testing.T) { opts.Plan = "grow" opts.AcceptTerms = true - err := runCreateCmd(opts) + err := runCreateCmd(context.Background(), opts) require.Error(t, err) assert.Contains(t, err.Error(), "payment method") assert.NotContains(t, err.Error(), "invalid plan") @@ -366,7 +367,7 @@ func TestRun_PaidPlanHiddenByServerInteractiveOpensBilling(t *testing.T) { opts, _, opened := newOpts(t, srv, true) opts.Plan = "grow" - require.NoError(t, runCreateCmd(opts)) + require.NoError(t, runCreateCmd(context.Background(), opts)) assert.Equal(t, 0, srv.createCalls) assert.Equal( t, @@ -383,7 +384,7 @@ func TestRun_InvalidPlanErrors(t *testing.T) { opts.Plan = "bogus" opts.AcceptTerms = true - err := runCreateCmd(opts) + err := runCreateCmd(context.Background(), opts) require.Error(t, err) assert.Contains(t, err.Error(), "invalid plan") assert.Equal(t, 0, srv.createCalls) @@ -397,7 +398,7 @@ func TestRun_InteractivePickerHidesPaidWithoutBilling(t *testing.T) { opts, out, _ := newOpts(t, srv, true) - require.NoError(t, runCreateCmd(opts)) + require.NoError(t, runCreateCmd(context.Background(), opts)) assert.Equal(t, 1, srv.createCalls) assert.Equal(t, 0, srv.patchCalls) assert.Contains(t, out.String(), "only the Free plan is available") @@ -418,7 +419,7 @@ func TestRun_InteractivePickerSelectsPaid(t *testing.T) { opts, out, _ := newOpts(t, srv, true) - require.NoError(t, runCreateCmd(opts)) + require.NoError(t, runCreateCmd(context.Background(), opts)) assert.Equal(t, 1, srv.createCalls) assert.Equal(t, 1, srv.patchCalls) assert.Equal(t, "grow", srv.lastPlan) @@ -434,7 +435,7 @@ func TestRun_DryRunDoesNotCallAPI(t *testing.T) { opts.DryRun = true opts.PrintFlags = newPrintFlags("") - require.NoError(t, runCreateCmd(opts)) + require.NoError(t, runCreateCmd(context.Background(), opts)) assert.Equal(t, 0, srv.createCalls) assert.Equal(t, 0, srv.patchCalls) assert.Contains(t, out.String(), "Dry run") @@ -450,7 +451,7 @@ func TestRun_PlanChangeFailureKeepsFreeApp(t *testing.T) { opts.Plan = "grow" opts.AcceptTerms = true - err := runCreateCmd(opts) + err := runCreateCmd(context.Background(), opts) require.Error(t, err) assert.Contains(t, err.Error(), "failed to apply") assert.Equal(t, 1, srv.createCalls) diff --git a/pkg/cmd/application/downgrade/downgrade.go b/pkg/cmd/application/downgrade/downgrade.go index 27013e51..69be2623 100644 --- a/pkg/cmd/application/downgrade/downgrade.go +++ b/pkg/cmd/application/downgrade/downgrade.go @@ -49,7 +49,7 @@ func NewDowngradeCmd(f *cmdutil.Factory) *cobra.Command { "skipAuthCheck": "true", }, RunE: func(cmd *cobra.Command, args []string) error { - return planchange.Run(opts) + return planchange.Run(cmd.Context(), opts) }, } diff --git a/pkg/cmd/application/planchange/planchange.go b/pkg/cmd/application/planchange/planchange.go index 9c604164..60647608 100644 --- a/pkg/cmd/application/planchange/planchange.go +++ b/pkg/cmd/application/planchange/planchange.go @@ -5,6 +5,7 @@ package planchange import ( + "context" "fmt" "strings" @@ -17,6 +18,7 @@ import ( "github.com/algolia/cli/pkg/iostreams" pkgopen "github.com/algolia/cli/pkg/open" "github.com/algolia/cli/pkg/prompt" + "github.com/algolia/cli/pkg/telemetry" ) // Direction selects whether the flow offers higher-tier (upgrade) or @@ -54,9 +56,11 @@ type changeResult struct { } // Run executes the shared plan-change flow. -func Run(opts *Options) error { +func Run(ctx context.Context, opts *Options) error { cs := opts.IO.ColorScheme() + opts.trackUpgrade(ctx, telemetry.ApplicationUpgradeStarted(opts.Plan)) + appID, err := opts.Config.Profile().GetApplicationID() if err != nil { return fmt.Errorf( @@ -146,6 +150,7 @@ func Run(opts *Options) error { return err } if !accepted { + opts.trackUpgrade(ctx, telemetry.ApplicationUpgradeDeclinedTerms(target.ID)) fmt.Fprintf( opts.IO.Out, "%s Plan change aborted; no changes were made.\n", @@ -153,13 +158,19 @@ func Run(opts *Options) error { ) return nil } + opts.trackUpgrade(ctx, telemetry.ApplicationUpgradeAcceptedTerms(target.ID)) if err := callWithReauth(opts.IO, client, &token, "Changing plan", func(t string) error { _, e := client.ChangeApplicationPlan(t, appID, target.ID) return e }); err != nil { + if opts.Direction == DirectionUpgrade { + class, _, status := cmdutil.ClassifyError(err) + telemetry.Track(ctx, telemetry.ApplicationUpgradeFailed(target.ID, class, status)) + } return err } + opts.trackUpgrade(ctx, telemetry.ApplicationUpgradeCompleted(target.ID)) if opts.PrintFlags.OutputFlagSpecified() && opts.PrintFlags.OutputFormat != nil { p, err := opts.PrintFlags.ToPrinter() @@ -189,6 +200,14 @@ func Run(opts *Options) error { return offerCostManagementBudget(opts, client.DashboardURL, appID) } +// trackUpgrade emits event only for upgrades; downgrades share this path but +// the analytics spec defines no downgrade events. +func (opts *Options) trackUpgrade(ctx context.Context, event telemetry.Event) { + if opts.Direction == DirectionUpgrade { + telemetry.Track(ctx, event) + } +} + // offerCostManagementBudget tells the user they can create a budget and, when // confirmed, opens the cost management page in the browser. func offerCostManagementBudget(opts *Options, dashboardURL, appID string) error { diff --git a/pkg/cmd/application/planchange/planchange_test.go b/pkg/cmd/application/planchange/planchange_test.go index bd637118..1a180d85 100644 --- a/pkg/cmd/application/planchange/planchange_test.go +++ b/pkg/cmd/application/planchange/planchange_test.go @@ -1,6 +1,7 @@ package planchange import ( + "context" "encoding/json" "net/http" "net/http/httptest" @@ -196,7 +197,7 @@ func TestRun_WithPlanFlag(t *testing.T) { opts.Plan = "grow" opts.AcceptTerms = true - require.NoError(t, Run(opts)) + require.NoError(t, Run(context.Background(), opts)) assert.Equal(t, 1, srv.patchCalls) assert.Equal(t, "grow", srv.lastPlan) assert.Contains(t, out.String(), "Grow") @@ -211,7 +212,7 @@ func TestRun_FreeTargetNotBilled(t *testing.T) { opts.Plan = "free" opts.AcceptTerms = true - require.NoError(t, Run(opts)) + require.NoError(t, Run(context.Background(), opts)) assert.Equal(t, 1, srv.patchCalls) // "free" maps to the free-type template, whose id is "build". assert.Equal(t, "build", srv.lastPlan) @@ -226,7 +227,7 @@ func TestRun_BillingBlock(t *testing.T) { opts.Plan = "grow" opts.AcceptTerms = true - err := Run(opts) + err := Run(context.Background(), opts) require.Error(t, err) assert.Contains(t, err.Error(), "payment method") assert.Equal(t, 0, srv.patchCalls) @@ -241,7 +242,7 @@ func TestRun_ToSDeclineAborts(t *testing.T) { opts, out, _ := newOpts(t, srv, true) opts.Plan = "grow" - require.NoError(t, Run(opts)) + require.NoError(t, Run(context.Background(), opts)) assert.Equal(t, 0, srv.patchCalls) assert.Contains(t, out.String(), "aborted") } @@ -253,7 +254,7 @@ func TestRun_NonInteractiveRequiresPlan(t *testing.T) { opts, _, _ := newOpts(t, srv, false) // No --plan and no TTY. - err := Run(opts) + err := Run(context.Background(), opts) require.Error(t, err) assert.Contains(t, err.Error(), "--plan is required") assert.Equal(t, 0, srv.patchCalls) @@ -274,7 +275,7 @@ func TestRun_InteractivePicker(t *testing.T) { opts, out, _ := newOpts(t, srv, true) - require.NoError(t, Run(opts)) + require.NoError(t, Run(context.Background(), opts)) assert.Equal(t, 1, srv.patchCalls) assert.Equal(t, "grow", srv.lastPlan) assert.Contains(t, out.String(), "Current application: APP1 (My App)") @@ -288,7 +289,7 @@ func TestRun_DryRunDoesNotCallAPI(t *testing.T) { opts.Plan = "grow" opts.DryRun = true - require.NoError(t, Run(opts)) + require.NoError(t, Run(context.Background(), opts)) assert.Equal(t, 0, srv.patchCalls) assert.Contains(t, out.String(), "Dry run") assert.Contains(t, out.String(), "Grow") @@ -303,7 +304,7 @@ func TestRun_OfferCostManagementBudget(t *testing.T) { opts, out, opened := newOpts(t, srv, true) opts.Plan = "grow" - require.NoError(t, Run(opts)) + require.NoError(t, Run(context.Background(), opts)) assert.Equal(t, 1, srv.patchCalls) assert.Contains(t, out.String(), "create a budget") assert.Equal( @@ -323,7 +324,7 @@ func TestRun_FreePlanSkipsCostManagementBudget(t *testing.T) { opts.Plan = "free" opts.AcceptTerms = true - require.NoError(t, Run(opts)) + require.NoError(t, Run(context.Background(), opts)) assert.Equal(t, 1, srv.patchCalls) assert.NotContains(t, out.String(), "create a budget") assert.Empty(t, *opened) @@ -338,7 +339,7 @@ func TestRun_OutputJSON(t *testing.T) { opts.AcceptTerms = true opts.PrintFlags = newPrintFlags("json") - require.NoError(t, Run(opts)) + require.NoError(t, Run(context.Background(), opts)) assert.Equal(t, 1, srv.patchCalls) assert.Contains(t, out.String(), `"plan":"grow"`) assert.Contains(t, out.String(), `"application_id":"APP1"`) @@ -355,7 +356,7 @@ func TestRun_UpgradeFiltersToHigherPlans(t *testing.T) { opts, out, _ := newOpts(t, srv, true) opts.Direction = DirectionUpgrade - require.NoError(t, Run(opts)) + require.NoError(t, Run(context.Background(), opts)) assert.Equal(t, 1, srv.patchCalls) assert.Equal(t, "grow-plus", srv.lastPlan) assert.Contains(t, out.String(), "current plan: Grow") @@ -372,7 +373,7 @@ func TestRun_DowngradeFiltersToLowerPlans(t *testing.T) { opts, _, _ := newOpts(t, srv, true) opts.Direction = DirectionDowngrade - require.NoError(t, Run(opts)) + require.NoError(t, Run(context.Background(), opts)) assert.Equal(t, 1, srv.patchCalls) assert.Equal(t, "build", srv.lastPlan) } @@ -385,7 +386,7 @@ func TestRun_UpgradeAtHighestPlanIsNoOp(t *testing.T) { opts, out, _ := newOpts(t, srv, true) opts.Direction = DirectionUpgrade - require.NoError(t, Run(opts)) + require.NoError(t, Run(context.Background(), opts)) assert.Equal(t, 0, srv.patchCalls) assert.Contains(t, out.String(), "already on the highest") assert.Contains(t, out.String(), "nothing to upgrade") @@ -399,7 +400,7 @@ func TestRun_DowngradeAtLowestPlanIsNoOp(t *testing.T) { opts, out, _ := newOpts(t, srv, true) opts.Direction = DirectionDowngrade - require.NoError(t, Run(opts)) + require.NoError(t, Run(context.Background(), opts)) assert.Equal(t, 0, srv.patchCalls) assert.Contains(t, out.String(), "already on the lowest") assert.Contains(t, out.String(), "nothing to downgrade") @@ -417,7 +418,7 @@ func TestRun_PlanFlagOverridesDirection(t *testing.T) { opts.Plan = "free" opts.AcceptTerms = true - require.NoError(t, Run(opts)) + require.NoError(t, Run(context.Background(), opts)) assert.Equal(t, 1, srv.patchCalls) assert.Equal(t, "build", srv.lastPlan) } @@ -431,7 +432,7 @@ func TestRun_SamePlanIsNoOp(t *testing.T) { opts.Plan = "grow" opts.AcceptTerms = true - require.NoError(t, Run(opts)) + require.NoError(t, Run(context.Background(), opts)) assert.Equal(t, 0, srv.patchCalls) assert.Contains(t, out.String(), "already on the Grow plan") assert.Contains(t, out.String(), "no change needed") @@ -448,7 +449,7 @@ func TestRun_UnknownCurrentPlanShowsAllPlans(t *testing.T) { opts, _, _ := newOpts(t, srv, true) opts.Direction = DirectionUpgrade - require.NoError(t, Run(opts)) + require.NoError(t, Run(context.Background(), opts)) assert.Equal(t, 1, srv.patchCalls) assert.Equal(t, "build", srv.lastPlan) } diff --git a/pkg/cmd/application/upgrade/upgrade.go b/pkg/cmd/application/upgrade/upgrade.go index ed1cd632..1eee1c7e 100644 --- a/pkg/cmd/application/upgrade/upgrade.go +++ b/pkg/cmd/application/upgrade/upgrade.go @@ -50,7 +50,7 @@ func NewUpgradeCmd(f *cmdutil.Factory) *cobra.Command { "skipAuthCheck": "true", }, RunE: func(cmd *cobra.Command, args []string) error { - return planchange.Run(opts) + return planchange.Run(cmd.Context(), opts) }, } diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 7fdb1461..5b2a1ee7 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -3,6 +3,7 @@ package login import ( "context" "fmt" + "time" "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" @@ -78,9 +79,11 @@ func NewLoginCmd(f *cmdutil.Factory) *cobra.Command { } cmd.Flags().StringVar(&opts.AppName, "app-name", "", "Auto-select application by name") - cmd.Flags().StringVar(&opts.ProfileName, "profile-name", "", "Name for the CLI profile (defaults to application name)") + cmd.Flags(). + StringVar(&opts.ProfileName, "profile-name", "", "Name for the CLI profile (defaults to application name)") cmd.Flags().BoolVar(&opts.Default, "default", true, "Set the profile as the default") - cmd.Flags().BoolVar(&opts.NoBrowser, "no-browser", false, "Print the authorize URL instead of opening the browser") + cmd.Flags(). + BoolVar(&opts.NoBrowser, "no-browser", false, "Print the authorize URL instead of opening the browser") return cmd } @@ -95,8 +98,14 @@ func RunOAuthFlow(ctx context.Context, opts *LoginOptions, signup bool) error { cs := opts.IO.ColorScheme() client := opts.NewDashboardClient(auth.OAuthClientID()) + flow := telemetry.FlowLogin + if signup { + flow = telemetry.FlowSignup + } + start := time.Now() + openBrowser := !opts.NoBrowser - accessToken, err := auth.RunOAuth(opts.IO, client, signup, openBrowser) + accessToken, err := auth.RunOAuth(ctx, opts.IO, client, signup, openBrowser) if err != nil { return err } @@ -107,18 +116,38 @@ func RunOAuthFlow(ctx context.Context, opts *LoginOptions, signup bool) error { apps, err := client.ListApplications(accessToken) opts.IO.StopProgressIndicator() if err != nil { + telemetry.Track( + ctx, + telemetry.AuthFailed(flow, telemetry.AuthStepAppsFetch, cmdutil.ErrorClass(err)), + ) return err } + hadExistingApps := len(apps) > 0 + createdAppDuringFlow := false + var appDetails *dashboard.Application if len(apps) == 0 { - fmt.Fprintf(opts.IO.Out, "\n%s No applications found. Let's create one.\n", cs.WarningIcon()) - - appDetails, err = apputil.CreateAndFetchApplication(opts.IO, client, accessToken, "", opts.AppName) + fmt.Fprintf( + opts.IO.Out, + "\n%s No applications found. Let's create one.\n", + cs.WarningIcon(), + ) + + appDetails, err = apputil.CreateAndFetchApplication( + ctx, + opts.IO, + client, + accessToken, + "", + opts.AppName, + telemetry.TriggeredFromAuthFlow, + ) if err != nil { return err } + createdAppDuringFlow = true } else { interactive := opts.IO.CanPrompt() app, err := selectApplication(opts, apps, interactive) @@ -139,7 +168,15 @@ func RunOAuthFlow(ctx context.Context, opts *LoginOptions, signup bool) error { profileName = appDetails.Name } - return apputil.ConfigureProfile(opts.IO, opts.Config, appDetails, profileName, opts.Default) + if err := apputil.ConfigureProfile(opts.IO, opts.Config, appDetails, profileName, opts.Default); err != nil { + return err + } + + telemetry.Track( + ctx, + telemetry.AuthCompleted(flow, time.Since(start), hadExistingApps, createdAppDuringFlow), + ) + return nil } // identifyAuthenticatedUser emits a telemetry Identify for the user that just @@ -180,7 +217,11 @@ func reuseExistingAPIKey(cfg config.IConfig, app *dashboard.Application) bool { return false } -func selectApplication(opts *LoginOptions, apps []dashboard.Application, interactive bool) (*dashboard.Application, error) { +func selectApplication( + opts *LoginOptions, + apps []dashboard.Application, + interactive bool, +) (*dashboard.Application, error) { if opts.AppName != "" { for i := range apps { if apps[i].Name == opts.AppName { diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 399535e1..1a273032 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" @@ -191,14 +192,12 @@ func Execute() exitCode { return err } - // Send telemetry. - err = telemetryClient.Track(ctx, "Command Invoked") + // Command Invoked; flushed at the end of Execute with the command's other events. + err = telemetryClient.Track(ctx, telemetry.EventCommandInvoked, nil) if err != nil && hasDebug { fmt.Fprintf(stderr, "Error tracking telemetry: %s\n", err) } - go telemetryClient.Close() // flush telemetry events - return nil } @@ -210,23 +209,41 @@ func Execute() exitCode { } // Run the command. + start := time.Now() cmd, err := rootCmd.ExecuteContextC(ctx) - // Handle eventual errors. + + // Exit code; reused below as the Command Completed exit_code property. + code := exitCodeForError(err, authError) + + // Command Failed precedes Command Completed so Command Completed is always last. + if cmd != nil && cmdutil.ShouldTrackUsage(cmd) { + if err != nil { + class, source, status := cmdutil.ClassifyError(err) + telemetry.Track(ctx, telemetry.CommandFailed(class, source, status)) + } + telemetry.Track( + ctx, + telemetry.CommandCompleted(time.Since(start), err == nil, int(code)), + ) + } + flushTelemetry(ctx) + + // Print the error (exit code already resolved above). if err != nil { - if err == cmdutil.ErrSilent { - return exitError - } else if cmdutil.IsUserCancellation(err) { + switch { + case err == cmdutil.ErrSilent: + // Intentionally silent. + case cmdutil.IsUserCancellation(err): if errors.Is(err, terminal.InterruptErr) { // ensure the next shell prompt will start on its own line fmt.Fprint(stderr, "\n") } - return exitCancel - } else if errors.Is(err, authError) { - return exitAuth + case errors.Is(err, authError): + // Already reported by PersistentPreRunE. + default: + printError(stderr, err, cmd, hasDebug) } - - printError(stderr, err, cmd, hasDebug) - return exitError + return code } // If there is an update available, notify the user. @@ -248,6 +265,44 @@ func Execute() exitCode { return exitOK } +// exitCodeForError maps a command error to its process exit code. +func exitCodeForError(err error, authError error) exitCode { + switch { + case err == nil: + return exitOK + case err == cmdutil.ErrSilent: + return exitError + case cmdutil.IsUserCancellation(err): + return exitCancel + case errors.Is(err, authError): + return exitAuth + default: + return exitError + } +} + +// flushTelemetry sends this command's queued events as one batch and waits for +// it, so events aren't dropped at exit. The client bounds its own HTTP request; +// the cap here is a last-resort ceiling, kept above that timeout so a real +// in-flight flush is never abandoned. +func flushTelemetry(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(5 * time.Second): + } +} + // createContext creates a context with telemetry. func createContext( cmd *cobra.Command, diff --git a/pkg/cmd/shared/apputil/create.go b/pkg/cmd/shared/apputil/create.go index a5d217ee..7e7e8576 100644 --- a/pkg/cmd/shared/apputil/create.go +++ b/pkg/cmd/shared/apputil/create.go @@ -1,29 +1,39 @@ package apputil import ( + "context" "errors" "fmt" "strings" + "time" "github.com/AlecAivazis/survey/v2" "github.com/algolia/cli/api/dashboard" + "github.com/algolia/cli/pkg/cmdutil" "github.com/algolia/cli/pkg/config" "github.com/algolia/cli/pkg/iostreams" "github.com/algolia/cli/pkg/prompt" + "github.com/algolia/cli/pkg/telemetry" ) // CreateApplicationWithRetry creates an application, retrying with a different -// region if the selected one has no available cluster. +// region if the selected one has no available cluster. triggeredFrom tags the +// emitted telemetry events with their origin (telemetry.TriggeredFrom*). func CreateApplicationWithRetry( + ctx context.Context, io *iostreams.IOStreams, client *dashboard.Client, accessToken string, region string, appName string, + triggeredFrom string, ) (*dashboard.Application, string, error) { cs := io.ColorScheme() + telemetry.Track(ctx, telemetry.ApplicationCreateStarted(triggeredFrom)) + start := time.Now() + for { if region == "" { var err error @@ -38,6 +48,10 @@ func CreateApplicationWithRetry( io.StopProgressIndicator() if err == nil { + telemetry.Track( + ctx, + telemetry.ApplicationCreateCompleted(triggeredFrom, time.Since(start)), + ) fmt.Fprintf(io.Out, "%s Application %s created in region %q\n", cs.SuccessIcon(), cs.Bold(app.ID), region) return app, region, nil @@ -45,20 +59,35 @@ func CreateApplicationWithRetry( var clusterErr *dashboard.ErrClusterUnavailable if errors.As(err, &clusterErr) { - fmt.Fprintf(io.Out, "%s No cluster available in region %q. Please select another region.\n", - cs.WarningIcon(), region) + fmt.Fprintf( + io.Out, + "%s No cluster available in region %q. Please select another region.\n", + cs.WarningIcon(), + region, + ) region = "" if !io.CanPrompt() { - return nil, "", fmt.Errorf("no cluster available in region %q — try a different --region", clusterErr.Region) + trackCreateFailed(ctx, triggeredFrom, err) + return nil, "", fmt.Errorf( + "no cluster available in region %q — try a different --region", + clusterErr.Region, + ) } continue } + trackCreateFailed(ctx, triggeredFrom, err) return nil, "", fmt.Errorf("application creation failed: %w", err) } } +// trackCreateFailed emits CLI Application Create Failed with the classified error. +func trackCreateFailed(ctx context.Context, triggeredFrom string, err error) { + class, _, status := cmdutil.ClassifyError(err) + telemetry.Track(ctx, telemetry.ApplicationCreateFailed(triggeredFrom, class, status)) +} + // PromptRegion fetches regions from the API and prompts the user to select one. func PromptRegion( io *iostreams.IOStreams, @@ -103,11 +132,20 @@ func PromptRegion( // CreateAndFetchApplication creates an application (with region retry) and // generates an API key for it. func CreateAndFetchApplication( + ctx context.Context, io *iostreams.IOStreams, client *dashboard.Client, - accessToken, region, appName string, + accessToken, region, appName, triggeredFrom string, ) (*dashboard.Application, error) { - app, _, err := CreateApplicationWithRetry(io, client, accessToken, region, appName) + app, _, err := CreateApplicationWithRetry( + ctx, + io, + client, + accessToken, + region, + appName, + triggeredFrom, + ) if err != nil { return nil, err } @@ -160,7 +198,8 @@ func ConfigureProfile( } profileName = strings.ToLower(profileName) - if exists, existingAppID := cfg.ApplicationIDForProfile(profileName); exists && existingAppID != appDetails.ID { + if exists, existingAppID := cfg.ApplicationIDForProfile(profileName); exists && + existingAppID != appDetails.ID { profileName = strings.ToLower(appDetails.Name + "-" + appDetails.ID) } diff --git a/pkg/cmdutil/classify.go b/pkg/cmdutil/classify.go new file mode 100644 index 00000000..7c5bae52 --- /dev/null +++ b/pkg/cmdutil/classify.go @@ -0,0 +1,90 @@ +package cmdutil + +import ( + "context" + "errors" + "fmt" + "net" + "net/http" + + "github.com/algolia/cli/api/dashboard" +) + +// error_source buckets for failure telemetry: where an error came from, so +// failures can be triaged without parsing the (high-cardinality) message. +const ( + ErrorSourceNetwork = "network" + ErrorSourceAPI = "api" + ErrorSourceValidation = "validation" + ErrorSourceLocal = "local" +) + +// httpStatusError is satisfied by errors carrying an HTTP status (e.g. +// *dashboard.APIError), declared here to keep the classifier decoupled from it. +type httpStatusError interface{ HTTPStatusCode() int } + +// ClassifyError returns a stable, low-cardinality error class, a source bucket +// (ErrorSource*), and the HTTP status when present (0 otherwise), so every +// "*Failed" event classifies errors the same way. Most specific checks first. +func ClassifyError(err error) (class, source string, httpStatus int) { + if err == nil { + return "", "", 0 + } + + var flagErr *FlagError + if errors.As(err, &flagErr) { + return "validation_error", ErrorSourceValidation, 0 + } + + if errors.Is(err, dashboard.ErrSessionExpired) { + return "session_expired", ErrorSourceAPI, http.StatusUnauthorized + } + var clusterErr *dashboard.ErrClusterUnavailable + if errors.As(err, &clusterErr) { + return "cluster_unavailable", ErrorSourceAPI, 0 + } + var statusErr httpStatusError + if errors.As(err, &statusErr) { + status := statusErr.HTTPStatusCode() + return fmt.Sprintf("http_%d", status), ErrorSourceAPI, status + } + + if errors.Is(err, context.Canceled) { + return "canceled", ErrorSourceLocal, 0 + } + if errors.Is(err, context.DeadlineExceeded) { + return "timeout", ErrorSourceNetwork, 0 + } + + var dnsErr *net.DNSError + if errors.As(err, &dnsErr) { + return "dns_error", ErrorSourceNetwork, 0 + } + var netErr net.Error + if errors.As(err, &netErr) { + if netErr.Timeout() { + return "timeout", ErrorSourceNetwork, 0 + } + return "network_error", ErrorSourceNetwork, 0 + } + + // Fall back to the error's type name: stable/low-cardinality for typed errors. + return rootCauseType(err), ErrorSourceLocal, 0 +} + +// ErrorClass returns only the class component of ClassifyError. +func ErrorClass(err error) string { + class, _, _ := ClassifyError(err) + return class +} + +// rootCauseType returns the type name of the deepest wrapped error. +func rootCauseType(err error) string { + for { + next := errors.Unwrap(err) + if next == nil { + return fmt.Sprintf("%T", err) + } + err = next + } +} diff --git a/pkg/cmdutil/classify_test.go b/pkg/cmdutil/classify_test.go new file mode 100644 index 00000000..38a2b8b8 --- /dev/null +++ b/pkg/cmdutil/classify_test.go @@ -0,0 +1,91 @@ +package cmdutil + +import ( + "context" + "errors" + "fmt" + "net" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/algolia/cli/api/dashboard" +) + +func TestClassifyError(t *testing.T) { + tests := []struct { + name string + err error + wantClass string + wantSource string + wantStatus int + }{ + { + name: "nil", + }, + { + name: "flag error", + err: FlagErrorf("bad flag"), + wantClass: "validation_error", + wantSource: ErrorSourceValidation, + }, + { + name: "wrapped api error carries status", + err: fmt.Errorf( + "create failed: %w", + &dashboard.APIError{StatusCode: 500, Message: "boom"}, + ), + wantClass: "http_500", + wantSource: ErrorSourceAPI, + wantStatus: 500, + }, + { + name: "session expired", + err: fmt.Errorf("call failed: %w", dashboard.ErrSessionExpired), + wantClass: "session_expired", + wantSource: ErrorSourceAPI, + wantStatus: 401, + }, + { + name: "cluster unavailable", + err: &dashboard.ErrClusterUnavailable{Region: "us", Message: "no cluster"}, + wantClass: "cluster_unavailable", + wantSource: ErrorSourceAPI, + }, + { + name: "context canceled", + err: context.Canceled, + wantClass: "canceled", + wantSource: ErrorSourceLocal, + }, + { + name: "deadline exceeded", + err: context.DeadlineExceeded, + wantClass: "timeout", + wantSource: ErrorSourceNetwork, + }, + { + name: "dns error", + err: &net.DNSError{Err: "no such host", Name: "example.invalid"}, + wantClass: "dns_error", + wantSource: ErrorSourceNetwork, + }, + { + name: "generic error falls back to local", + err: errors.New("something went wrong"), + wantClass: "*errors.errorString", + wantSource: ErrorSourceLocal, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + class, source, status := ClassifyError(tt.err) + assert.Equal(t, tt.wantClass, class) + assert.Equal(t, tt.wantSource, source) + assert.Equal(t, tt.wantStatus, status) + + assert.Equal(t, tt.wantClass, ErrorClass(tt.err)) + }) + } +} diff --git a/pkg/telemetry/event.go b/pkg/telemetry/event.go new file mode 100644 index 00000000..5924fafd --- /dev/null +++ b/pkg/telemetry/event.go @@ -0,0 +1,16 @@ +package telemetry + +import "context" + +type Event struct { + Name string + Properties map[string]any +} + +func Track(ctx context.Context, event Event) { + client := GetTelemetryClient(ctx) + if client == nil { + return + } + _ = client.Track(ctx, event.Name, event.Properties) +} diff --git a/pkg/telemetry/events.go b/pkg/telemetry/events.go new file mode 100644 index 00000000..e86cab37 --- /dev/null +++ b/pkg/telemetry/events.go @@ -0,0 +1,250 @@ +package telemetry + +import "time" + +// Catalog of the analytics events the CLI emits, mirroring the analytics spec. +// Keeping names and property schemas here (not inline at call sites) keeps them +// auditable in one place and prevents names/keys from drifting. +// +// To add an event: add a name constant, add a constructor, then call +// telemetry.Track(ctx, telemetry.YourEvent(...)). + +// Event names. Flow events follow the "CLI " convention. +const ( + EventCommandInvoked = "Command Invoked" + EventCommandCompleted = "Command Completed" + EventCommandFailed = "Command Failed" + + EventAuthStarted = "CLI Auth Started" + EventAuthBrowserOpened = "CLI Auth Browser Opened" + EventAuthBrowserFailed = "CLI Auth Browser Failed" + EventAuthCallbackReceived = "CLI Auth Callback Received" + EventAuthCompleted = "CLI Auth Completed" + EventAuthFailed = "CLI Auth Failed" + + EventApplicationCreateStarted = "CLI Application Create Started" + EventApplicationCreateCompleted = "CLI Application Create Completed" + EventApplicationCreateFailed = "CLI Application Create Failed" + EventApplicationCreateAborted = "CLI Application Create Aborted" + + EventApplicationUpgradeStarted = "CLI Application Upgrade Started" + EventApplicationUpgradeAcceptedTerms = "CLI Application Upgrade Accepted Terms" + EventApplicationUpgradeDeclinedTerms = "CLI Application Upgrade Declined Terms" + EventApplicationUpgradeFailed = "CLI Application Upgrade Failed" + EventApplicationUpgradeCompleted = "CLI Application Upgrade Completed" +) + +// Property values, so call sites reference a constant instead of a literal. +const ( + FlowLogin = "login" + FlowSignup = "signup" + + // triggered_from: auth flow vs explicit "application create" command. + TriggeredFromAuthFlow = "auth_flow" + TriggeredFromExplicitCommand = "explicit_command" + + // step: where the auth flow failed (CLI Auth Failed). + AuthStepBrowser = "browser" + AuthStepCallback = "callback" + AuthStepExchange = "exchange" + AuthStepAppsFetch = "apps_fetch" +) + +// --- Root command events --------------------------------------------------- + +// CommandInvoked is emitted when a tracked command starts. +func CommandInvoked() Event { + return Event{Name: EventCommandInvoked} +} + +// CommandCompleted is emitted (deferred) when the root command returns. +func CommandCompleted(duration time.Duration, succeeded bool, exitCode int) Event { + return Event{ + Name: EventCommandCompleted, + Properties: map[string]any{ + "duration_ms": duration.Milliseconds(), + "succeeded": succeeded, + "exit_code": exitCode, + }, + } +} + +// CommandFailed is emitted when the root command returns an error. httpStatus +// is omitted when zero (i.e. the error carried no HTTP status). +func CommandFailed(errorClass, errorSource string, httpStatus int) Event { + props := map[string]any{ + "error_class": errorClass, + "error_source": errorSource, + } + if httpStatus != 0 { + props["http_status"] = httpStatus + } + return Event{Name: EventCommandFailed, Properties: props} +} + +// --- Auth flow events ------------------------------------------------------ + +// AuthStarted is emitted when the OAuth flow begins. +func AuthStarted(flow string, noBrowser bool) Event { + return Event{ + Name: EventAuthStarted, + Properties: map[string]any{ + "flow": flow, + "no_browser": noBrowser, + }, + } +} + +// AuthBrowserOpened is emitted when the default browser is launched successfully. +func AuthBrowserOpened(flow string) Event { + return Event{ + Name: EventAuthBrowserOpened, + Properties: map[string]any{"flow": flow}, + } +} + +// AuthBrowserFailed is emitted when launching the browser fails and the +// authorize URL is printed instead. +func AuthBrowserFailed(flow, errorClass string) Event { + return Event{ + Name: EventAuthBrowserFailed, + Properties: map[string]any{ + "flow": flow, + "error_class": errorClass, + }, + } +} + +// AuthCallbackReceived is emitted when the OAuth redirect hits the local +// callback server. +func AuthCallbackReceived(flow string, duration time.Duration) Event { + return Event{ + Name: EventAuthCallbackReceived, + Properties: map[string]any{ + "flow": flow, + "duration_ms": duration.Milliseconds(), + }, + } +} + +// AuthCompleted is emitted once the profile is fully configured at the end of +// the flow. +func AuthCompleted( + flow string, + duration time.Duration, + hadExistingApps, createdAppDuringFlow bool, +) Event { + return Event{ + Name: EventAuthCompleted, + Properties: map[string]any{ + "flow": flow, + "duration_ms": duration.Milliseconds(), + "had_existing_apps": hadExistingApps, + "created_app_during_flow": createdAppDuringFlow, + }, + } +} + +// AuthFailed is emitted on a timeout, token exchange error, or app fetch error. +// step is one of the AuthStep* constants. +func AuthFailed(flow, step, errorClass string) Event { + return Event{ + Name: EventAuthFailed, + Properties: map[string]any{ + "flow": flow, + "step": step, + "error_class": errorClass, + }, + } +} + +// --- Application create events --------------------------------------------- + +// ApplicationCreateStarted is emitted once validation passes, before the +// Dashboard API call. triggeredFrom is one of the TriggeredFrom* constants. +func ApplicationCreateStarted(triggeredFrom string) Event { + return Event{ + Name: EventApplicationCreateStarted, + Properties: map[string]any{"triggered_from": triggeredFrom}, + } +} + +// ApplicationCreateCompleted is emitted when the Dashboard API returns 2xx. +func ApplicationCreateCompleted(triggeredFrom string, duration time.Duration) Event { + return Event{ + Name: EventApplicationCreateCompleted, + Properties: map[string]any{ + "triggered_from": triggeredFrom, + "duration_ms": duration.Milliseconds(), + }, + } +} + +// ApplicationCreateFailed is emitted when the Dashboard API returns an error. +// httpStatus is omitted when zero. +func ApplicationCreateFailed(triggeredFrom, errorClass string, httpStatus int) Event { + props := map[string]any{ + "triggered_from": triggeredFrom, + "error_class": errorClass, + } + if httpStatus != 0 { + props["http_status"] = httpStatus + } + return Event{Name: EventApplicationCreateFailed, Properties: props} +} + +// ApplicationCreateAborted is emitted when the user declines the confirmation +// prompt. +func ApplicationCreateAborted(triggeredFrom string) Event { + return Event{ + Name: EventApplicationCreateAborted, + Properties: map[string]any{"triggered_from": triggeredFrom}, + } +} + +// --- Application upgrade events --------------------------------------------- + +// ApplicationUpgradeStarted is emitted when the user starts the upgrade flow. +func ApplicationUpgradeStarted(plan string) Event { + return Event{ + Name: EventApplicationUpgradeStarted, + Properties: map[string]any{"plan": plan}, + } +} + +// ApplicationUpgradeAcceptedTerms is emitted when the user accepts the T&C. +func ApplicationUpgradeAcceptedTerms(plan string) Event { + return Event{ + Name: EventApplicationUpgradeAcceptedTerms, + Properties: map[string]any{"plan": plan}, + } +} + +// ApplicationUpgradeDeclinedTerms is emitted when the user declines the T&C. +func ApplicationUpgradeDeclinedTerms(plan string) Event { + return Event{ + Name: EventApplicationUpgradeDeclinedTerms, + Properties: map[string]any{"plan": plan}, + } +} + +// ApplicationUpgradeFailed is emitted when the Dashboard API returns an error. +// httpStatus is omitted when zero. +func ApplicationUpgradeFailed(plan, errorClass string, httpStatus int) Event { + props := map[string]any{ + "plan": plan, + "error_class": errorClass, + } + if httpStatus != 0 { + props["http_status"] = httpStatus + } + return Event{Name: EventApplicationUpgradeFailed, Properties: props} +} + +// ApplicationUpgradeCompleted is emitted when the upgrade flow succeeds. +func ApplicationUpgradeCompleted(plan string) Event { + return Event{ + Name: EventApplicationUpgradeCompleted, + Properties: map[string]any{"plan": plan}, + } +} diff --git a/pkg/telemetry/telemetry.go b/pkg/telemetry/telemetry.go index f81abe75..c3b2b61f 100644 --- a/pkg/telemetry/telemetry.go +++ b/pkg/telemetry/telemetry.go @@ -4,10 +4,14 @@ import ( "context" "crypto/md5" // nolint:gosec "fmt" + "io" "log" "net" + "net/http" "os" "runtime" + "sync" + "time" "github.com/segmentio/analytics-go/v3" "github.com/spf13/cobra" @@ -21,6 +25,11 @@ import ( const ( AppName = "cli" telemetryAnalyticsURL = "https://telemetry-proxy.algolia.com/" + + // telemetryHTTPTimeout bounds the total duration of the telemetry flush + // HTTP request (connect, TLS, and response), so closing the client at the + // end of a command can never hang the CLI on a slow or unreachable endpoint. + telemetryHTTPTimeout = 3 * time.Second ) type telemetryMetadataKey struct{} @@ -29,12 +38,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 + debug bool + + mu sync.Mutex + lastTS time.Time } type AnalyticsTelemetryLogger struct { @@ -60,14 +73,61 @@ func newTelemetryLogger(debug bool) AnalyticsTelemetryLogger { } func NewAnalyticsTelemetryClient(debug bool) (TelemetryClient, error) { + return newAnalyticsTelemetryClient(telemetryAnalyticsURL, debug) +} + +func newAnalyticsTelemetryClient(endpoint string, debug bool) (TelemetryClient, error) { client, err := analytics.NewWithConfig("", analytics.Config{ - Endpoint: telemetryAnalyticsURL, + Endpoint: endpoint, Logger: newTelemetryLogger(debug), + // In debug mode, surface the library's own batch/flush logs. + Verbose: debug, + // Buffer every event into one batch flushed at Close. The default 5s + // interval would split a long command (e.g. interactive login) across + // requests, reordering events downstream and risking a dropped batch. + Interval: 24 * time.Hour, + BatchSize: 250, + // Bound the flush request so Close() at exit can't hang the CLI. + Transport: boundedRoundTripper{ + base: http.DefaultTransport, + timeout: telemetryHTTPTimeout, + }, }) if err != nil { return nil, err } - return &AnalyticsTelemetryClient{client: client}, nil + return &AnalyticsTelemetryClient{client: client, debug: debug}, nil +} + +// boundedRoundTripper applies a total per-request timeout, like +// http.Client.Timeout (analytics.Config only accepts a RoundTripper). +type boundedRoundTripper struct { + base http.RoundTripper + timeout time.Duration +} + +func (t boundedRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + ctx, cancel := context.WithTimeout(req.Context(), t.timeout) + resp, err := t.base.RoundTrip(req.WithContext(ctx)) + if err != nil { + cancel() + return nil, err + } + // Cancel when the body is closed, not now, or the response would be truncated. + resp.Body = &cancelOnCloseBody{ReadCloser: resp.Body, cancel: cancel} + return resp, nil +} + +// cancelOnCloseBody cancels the request context when the body is closed. +type cancelOnCloseBody struct { + io.ReadCloser + cancel context.CancelFunc +} + +func (b *cancelOnCloseBody) Close() error { + err := b.ReadCloser.Close() + b.cancel() + return err } // IdentifyOnce sends a single Identify event through a short-lived client and @@ -236,19 +296,34 @@ 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 merges custom properties over the base properties (custom wins on collisions). +func (a *AnalyticsTelemetryClient) Track( + ctx context.Context, + event string, + properties map[string]any, +) error { metadata := GetEventMetadata(ctx) + props := map[string]interface{}{ + "invocation_id": metadata.InvocationID, + "app_id": metadata.AppID, + "command": metadata.CommandPath, + "flags": metadata.CommandFlags, + } + for k, v := range properties { + props[k] = v + } + + // In debug mode, echo each event to stderr for local observability. + if a.debug { + fmt.Fprintf(os.Stderr, "[telemetry] %s %v\n", event, properties) + } + 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, + Timestamp: a.nextTimestamp(), Context: &analytics.Context{ Device: analytics.DeviceInfo{ Id: metadata.AnonymousID, @@ -263,11 +338,33 @@ func (a *AnalyticsTelemetryClient) Track(ctx context.Context, event string) erro return a.client.Enqueue(track) } +// nextTimestamp returns strictly increasing timestamps at least 1ms apart. +// Amplitude truncates event_time to milliseconds, so same-millisecond events +// are ordered non-deterministically; spacing by 1ms preserves emit order. +func (a *AnalyticsTelemetryClient) nextTimestamp() time.Time { + a.mu.Lock() + defer a.mu.Unlock() + + ts := time.Now() + if !ts.After(a.lastTS) { + ts = a.lastTS.Add(time.Millisecond) + } + a.lastTS = ts + return ts +} + // Close closes the client, waiting for all pending events to be sent. 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..f7617b24 100644 --- a/pkg/telemetry/telemetry_test.go +++ b/pkg/telemetry/telemetry_test.go @@ -2,7 +2,12 @@ package telemetry import ( "context" + "encoding/json" + "net/http" + "net/http/httptest" + "sync" "testing" + "time" "github.com/segmentio/analytics-go/v3" "github.com/spf13/cobra" @@ -130,7 +135,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 +151,157 @@ 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_TimestampsAreStrictlyIncreasing(t *testing.T) { + fake := &fakeAnalyticsClient{} + client := &AnalyticsTelemetryClient{client: fake} + + metadata := NewEventMetadata() + ctx := WithEventMetadata(context.Background(), metadata) + + // Back-to-back events land in the same millisecond but must still get + // strictly increasing timestamps so Amplitude preserves emit order. + const n = 5 + for i := 0; i < n; i++ { + require.NoError(t, client.Track(ctx, "Event", nil)) + } + require.Len(t, fake.messages, n) + + var prev time.Time + for i, m := range fake.messages { + ts := m.(analytics.Track).Timestamp + if i > 0 { + assert.True( + t, + ts.After(prev), + "timestamp %d (%v) must be strictly after previous (%v)", + i, ts, prev, + ) + } + prev = ts + } +} + +// collectBatches starts a server recording each batch POST's event names in order. +func collectBatches(t *testing.T) (url string, batches *[][]string) { + t.Helper() + + var ( + mu sync.Mutex + got [][]string + ) + srv := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var payload struct { + Batch []struct { + Event string `json:"event"` + } `json:"batch"` + } + require.NoError(t, json.NewDecoder(r.Body).Decode(&payload)) + + names := make([]string, 0, len(payload.Batch)) + for _, m := range payload.Batch { + names = append(names, m.Event) + } + + mu.Lock() + got = append(got, names) + mu.Unlock() + + w.WriteHeader(http.StatusOK) + }), + ) + t.Cleanup(srv.Close) + + return srv.URL, &got +} + +// TestAnalyticsClient_SendsAllEventsInOneOrderedBatch is the regression test for +// the "events out of order / sometimes missing" bug: every event must reach the +// backend in one ordered batch, not split across the library's periodic flushes. +func TestAnalyticsClient_SendsAllEventsInOneOrderedBatch(t *testing.T) { + url, batches := collectBatches(t) + + client, err := newAnalyticsTelemetryClient(url, false) + require.NoError(t, err) + + metadata := NewEventMetadata() + metadata.AnonymousID = "anon-test" // ensure messages validate without a MAC + ctx := WithEventMetadata(context.Background(), metadata) + + want := []string{ + EventCommandInvoked, + EventAuthStarted, + EventAuthBrowserOpened, + EventAuthCallbackReceived, + EventAuthCompleted, + EventCommandCompleted, + } + for _, name := range want { + require.NoError(t, client.Track(ctx, name, nil)) + // A real command emits these over time; the gap must not trigger a flush. + time.Sleep(2 * time.Millisecond) + } + + // Close is the single flush point and must block until the batch is sent. + client.Close() + + require.Len(t, *batches, 1, "all events must be delivered in exactly one batch") + assert.Equal(t, want, (*batches)[0], "events must arrive in emit order") +} + +// TestBoundedRoundTripper_TimesOut verifies the flush request is bounded, so +// closing the client at exit can never hang the CLI on a stalled endpoint. +func TestBoundedRoundTripper_TimesOut(t *testing.T) { + blocked := make(chan struct{}) + srv := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + <-blocked // never respond until the test tears the server down + }), + ) + t.Cleanup(func() { + close(blocked) + srv.Close() + }) + + rt := boundedRoundTripper{base: http.DefaultTransport, timeout: 50 * time.Millisecond} + req, err := http.NewRequest(http.MethodGet, srv.URL, nil) + require.NoError(t, err) + + start := time.Now() + resp, err := rt.RoundTrip(req) + elapsed := time.Since(start) + + require.Error(t, err, "a stalled endpoint must surface as an error, not a hang") + if resp != nil { + _ = resp.Body.Close() + } + assert.Less(t, elapsed, time.Second, "request must be abandoned near the configured timeout") +} + +func TestTrack_MergesCustomProperties(t *testing.T) { + fake := &fakeAnalyticsClient{} + client := &AnalyticsTelemetryClient{client: fake} + + metadata := NewEventMetadata() + ctx := WithEventMetadata(context.Background(), metadata) + + props := map[string]any{"flow": "signup", "duration_ms": int64(1200)} + require.NoError(t, client.Track(ctx, EventAuthCompleted, props)) + require.Len(t, fake.messages, 1) + + track, ok := fake.messages[0].(analytics.Track) + require.True(t, ok) + assert.Equal(t, EventAuthCompleted, track.Event) + assert.Equal(t, "signup", track.Properties["flow"]) + assert.Equal(t, int64(1200), track.Properties["duration_ms"]) + assert.Contains(t, track.Properties, "invocation_id") + assert.Contains(t, track.Properties, "command") +} From cc32e79e0d73acb2bf7da3a434a64a15258960f1 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Thu, 4 Jun 2026 12:19:28 -0700 Subject: [PATCH 2/7] chore(events): add application downgrade events --- pkg/cmd/application/planchange/planchange.go | 47 +++++++--- .../application/planchange/planchange_test.go | 86 +++++++++++++++++++ pkg/telemetry/events.go | 55 +++++++++++- 3 files changed, 173 insertions(+), 15 deletions(-) diff --git a/pkg/cmd/application/planchange/planchange.go b/pkg/cmd/application/planchange/planchange.go index 60647608..6bab1b19 100644 --- a/pkg/cmd/application/planchange/planchange.go +++ b/pkg/cmd/application/planchange/planchange.go @@ -58,8 +58,9 @@ type changeResult struct { // Run executes the shared plan-change flow. func Run(ctx context.Context, opts *Options) error { cs := opts.IO.ColorScheme() + ev := planChangeEventsFor(opts.Direction) - opts.trackUpgrade(ctx, telemetry.ApplicationUpgradeStarted(opts.Plan)) + telemetry.Track(ctx, ev.started(opts.Plan)) appID, err := opts.Config.Profile().GetApplicationID() if err != nil { @@ -150,7 +151,7 @@ func Run(ctx context.Context, opts *Options) error { return err } if !accepted { - opts.trackUpgrade(ctx, telemetry.ApplicationUpgradeDeclinedTerms(target.ID)) + telemetry.Track(ctx, ev.declinedTerms(target.ID)) fmt.Fprintf( opts.IO.Out, "%s Plan change aborted; no changes were made.\n", @@ -158,19 +159,17 @@ func Run(ctx context.Context, opts *Options) error { ) return nil } - opts.trackUpgrade(ctx, telemetry.ApplicationUpgradeAcceptedTerms(target.ID)) + telemetry.Track(ctx, ev.acceptedTerms(target.ID)) if err := callWithReauth(opts.IO, client, &token, "Changing plan", func(t string) error { _, e := client.ChangeApplicationPlan(t, appID, target.ID) return e }); err != nil { - if opts.Direction == DirectionUpgrade { - class, _, status := cmdutil.ClassifyError(err) - telemetry.Track(ctx, telemetry.ApplicationUpgradeFailed(target.ID, class, status)) - } + class, _, status := cmdutil.ClassifyError(err) + telemetry.Track(ctx, ev.failed(target.ID, class, status)) return err } - opts.trackUpgrade(ctx, telemetry.ApplicationUpgradeCompleted(target.ID)) + telemetry.Track(ctx, ev.completed(target.ID)) if opts.PrintFlags.OutputFlagSpecified() && opts.PrintFlags.OutputFormat != nil { p, err := opts.PrintFlags.ToPrinter() @@ -200,11 +199,33 @@ func Run(ctx context.Context, opts *Options) error { return offerCostManagementBudget(opts, client.DashboardURL, appID) } -// trackUpgrade emits event only for upgrades; downgrades share this path but -// the analytics spec defines no downgrade events. -func (opts *Options) trackUpgrade(ctx context.Context, event telemetry.Event) { - if opts.Direction == DirectionUpgrade { - telemetry.Track(ctx, event) +// planChangeEvents bundles the telemetry constructors for one direction, so the +// shared flow emits upgrade- or downgrade-named events without branching at +// each call site. +type planChangeEvents struct { + started func(plan string) telemetry.Event + acceptedTerms func(plan string) telemetry.Event + declinedTerms func(plan string) telemetry.Event + failed func(plan, errorClass string, httpStatus int) telemetry.Event + completed func(plan string) telemetry.Event +} + +func planChangeEventsFor(dir Direction) planChangeEvents { + if dir == DirectionDowngrade { + return planChangeEvents{ + started: telemetry.ApplicationDowngradeStarted, + acceptedTerms: telemetry.ApplicationDowngradeAcceptedTerms, + declinedTerms: telemetry.ApplicationDowngradeDeclinedTerms, + failed: telemetry.ApplicationDowngradeFailed, + completed: telemetry.ApplicationDowngradeCompleted, + } + } + return planChangeEvents{ + started: telemetry.ApplicationUpgradeStarted, + acceptedTerms: telemetry.ApplicationUpgradeAcceptedTerms, + declinedTerms: telemetry.ApplicationUpgradeDeclinedTerms, + failed: telemetry.ApplicationUpgradeFailed, + completed: telemetry.ApplicationUpgradeCompleted, } } diff --git a/pkg/cmd/application/planchange/planchange_test.go b/pkg/cmd/application/planchange/planchange_test.go index 1a180d85..1f7d45e5 100644 --- a/pkg/cmd/application/planchange/planchange_test.go +++ b/pkg/cmd/application/planchange/planchange_test.go @@ -17,6 +17,7 @@ import ( "github.com/algolia/cli/pkg/auth" "github.com/algolia/cli/pkg/cmdutil" "github.com/algolia/cli/pkg/prompt" + "github.com/algolia/cli/pkg/telemetry" "github.com/algolia/cli/test" ) @@ -438,6 +439,91 @@ func TestRun_SamePlanIsNoOp(t *testing.T) { assert.Contains(t, out.String(), "no change needed") } +// recordingTelemetry records the names of the events emitted during a run. +type recordingTelemetry struct{ events []string } + +func (r *recordingTelemetry) Identify(context.Context) error { return nil } + +func (r *recordingTelemetry) Track(_ context.Context, event string, _ map[string]any) error { + r.events = append(r.events, event) + return nil +} + +func (r *recordingTelemetry) Close() {} + +// TestRun_EmitsPlanChangeEventsByDirection verifies the shared flow emits the +// upgrade- or downgrade-named events depending on the direction. +func TestRun_EmitsPlanChangeEventsByDirection(t *testing.T) { + tests := []struct { + name string + direction Direction + want []string + }{ + { + name: "upgrade", + direction: DirectionUpgrade, + want: []string{ + telemetry.EventApplicationUpgradeStarted, + telemetry.EventApplicationUpgradeAcceptedTerms, + telemetry.EventApplicationUpgradeCompleted, + }, + }, + { + name: "downgrade", + direction: DirectionDowngrade, + want: []string{ + telemetry.EventApplicationDowngradeStarted, + telemetry.EventApplicationDowngradeAcceptedTerms, + telemetry.EventApplicationDowngradeCompleted, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + srv := newServer(t, `{"has_payment_method": true}`) + srv.currentPlanLabel = "Grow" + defer srv.Close() + + stubPicker(t, 0) + defer prompt.StubConfirm(true)() + + opts, _, _ := newOpts(t, srv, true) + opts.Direction = tt.direction + + rec := &recordingTelemetry{} + ctx := telemetry.WithTelemetryClient(context.Background(), rec) + + require.NoError(t, Run(ctx, opts)) + assert.Equal(t, tt.want, rec.events) + }) + } +} + +// TestRun_EmitsDeclinedTermsEvent verifies declining the terms emits the +// direction-specific declined-terms event and nothing after it. +func TestRun_EmitsDeclinedTermsEvent(t *testing.T) { + srv := newServer(t, `{"has_payment_method": true}`) + srv.currentPlanLabel = "Grow" + defer srv.Close() + + stubPicker(t, 0) + defer prompt.StubConfirm(false)() + + opts, _, _ := newOpts(t, srv, true) + opts.Direction = DirectionDowngrade + + rec := &recordingTelemetry{} + ctx := telemetry.WithTelemetryClient(context.Background(), rec) + + require.NoError(t, Run(ctx, opts)) + assert.Equal(t, []string{ + telemetry.EventApplicationDowngradeStarted, + telemetry.EventApplicationDowngradeDeclinedTerms, + }, rec.events) + assert.Equal(t, 0, srv.patchCalls) +} + func TestRun_UnknownCurrentPlanShowsAllPlans(t *testing.T) { srv := newServer(t, `{"has_payment_method": true}`) srv.currentPlanLabel = "Enterprise" diff --git a/pkg/telemetry/events.go b/pkg/telemetry/events.go index e86cab37..4aaa6fbe 100644 --- a/pkg/telemetry/events.go +++ b/pkg/telemetry/events.go @@ -32,6 +32,12 @@ const ( EventApplicationUpgradeDeclinedTerms = "CLI Application Upgrade Declined Terms" EventApplicationUpgradeFailed = "CLI Application Upgrade Failed" EventApplicationUpgradeCompleted = "CLI Application Upgrade Completed" + + EventApplicationDowngradeStarted = "CLI Application Downgrade Started" + EventApplicationDowngradeAcceptedTerms = "CLI Application Downgrade Accepted Terms" + EventApplicationDowngradeDeclinedTerms = "CLI Application Downgrade Declined Terms" + EventApplicationDowngradeFailed = "CLI Application Downgrade Failed" + EventApplicationDowngradeCompleted = "CLI Application Downgrade Completed" ) // Property values, so call sites reference a constant instead of a literal. @@ -207,8 +213,7 @@ func ApplicationCreateAborted(triggeredFrom string) Event { // ApplicationUpgradeStarted is emitted when the user starts the upgrade flow. func ApplicationUpgradeStarted(plan string) Event { return Event{ - Name: EventApplicationUpgradeStarted, - Properties: map[string]any{"plan": plan}, + Name: EventApplicationUpgradeStarted, } } @@ -248,3 +253,49 @@ func ApplicationUpgradeCompleted(plan string) Event { Properties: map[string]any{"plan": plan}, } } + +// --- Application downgrade events ------------------------------------------- + +// ApplicationDowngradeStarted is emitted when the user starts the downgrade flow. +func ApplicationDowngradeStarted(plan string) Event { + return Event{ + Name: EventApplicationDowngradeStarted, + } +} + +// ApplicationDowngradeAcceptedTerms is emitted when the user accepts the T&C. +func ApplicationDowngradeAcceptedTerms(plan string) Event { + return Event{ + Name: EventApplicationDowngradeAcceptedTerms, + Properties: map[string]any{"plan": plan}, + } +} + +// ApplicationDowngradeDeclinedTerms is emitted when the user declines the T&C. +func ApplicationDowngradeDeclinedTerms(plan string) Event { + return Event{ + Name: EventApplicationDowngradeDeclinedTerms, + Properties: map[string]any{"plan": plan}, + } +} + +// ApplicationDowngradeFailed is emitted when the Dashboard API returns an error. +// httpStatus is omitted when zero. +func ApplicationDowngradeFailed(plan, errorClass string, httpStatus int) Event { + props := map[string]any{ + "plan": plan, + "error_class": errorClass, + } + if httpStatus != 0 { + props["http_status"] = httpStatus + } + return Event{Name: EventApplicationDowngradeFailed, Properties: props} +} + +// ApplicationDowngradeCompleted is emitted when the downgrade flow succeeds. +func ApplicationDowngradeCompleted(plan string) Event { + return Event{ + Name: EventApplicationDowngradeCompleted, + Properties: map[string]any{"plan": plan}, + } +} From 53d5813718406f6002b9ba0b7e644523231d758b Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Tue, 9 Jun 2026 16:33:46 -0700 Subject: [PATCH 3/7] style: revert unrelated golines churn CI only enforces gofumpt via golangci-lint; golines (run by 'task format') rewrapped untouched code and added noise to the diff. AGENTS.md: avoid unrelated formatting churn. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/auth/oauth_flow.go | 5 +---- pkg/cmd/auth/login/login.go | 18 ++++-------------- pkg/cmd/shared/apputil/create.go | 16 ++++------------ 3 files changed, 9 insertions(+), 30 deletions(-) diff --git a/pkg/auth/oauth_flow.go b/pkg/auth/oauth_flow.go index e51d8f65..e9a8f571 100644 --- a/pkg/auth/oauth_flow.go +++ b/pkg/auth/oauth_flow.go @@ -24,10 +24,7 @@ func OAuthClientID() string { return v } if DefaultOAuthClientID == "" { - fmt.Fprintln( - os.Stderr, - "fatal: ALGOLIA_OAUTH_CLIENT_ID is not set and no default was compiled in", - ) + fmt.Fprintln(os.Stderr, "fatal: ALGOLIA_OAUTH_CLIENT_ID is not set and no default was compiled in") os.Exit(1) } return DefaultOAuthClientID diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 5b2a1ee7..57ec8a7e 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -79,11 +79,9 @@ func NewLoginCmd(f *cmdutil.Factory) *cobra.Command { } cmd.Flags().StringVar(&opts.AppName, "app-name", "", "Auto-select application by name") - cmd.Flags(). - StringVar(&opts.ProfileName, "profile-name", "", "Name for the CLI profile (defaults to application name)") + cmd.Flags().StringVar(&opts.ProfileName, "profile-name", "", "Name for the CLI profile (defaults to application name)") cmd.Flags().BoolVar(&opts.Default, "default", true, "Set the profile as the default") - cmd.Flags(). - BoolVar(&opts.NoBrowser, "no-browser", false, "Print the authorize URL instead of opening the browser") + cmd.Flags().BoolVar(&opts.NoBrowser, "no-browser", false, "Print the authorize URL instead of opening the browser") return cmd } @@ -129,11 +127,7 @@ func RunOAuthFlow(ctx context.Context, opts *LoginOptions, signup bool) error { var appDetails *dashboard.Application if len(apps) == 0 { - fmt.Fprintf( - opts.IO.Out, - "\n%s No applications found. Let's create one.\n", - cs.WarningIcon(), - ) + fmt.Fprintf(opts.IO.Out, "\n%s No applications found. Let's create one.\n", cs.WarningIcon()) appDetails, err = apputil.CreateAndFetchApplication( ctx, @@ -217,11 +211,7 @@ func reuseExistingAPIKey(cfg config.IConfig, app *dashboard.Application) bool { return false } -func selectApplication( - opts *LoginOptions, - apps []dashboard.Application, - interactive bool, -) (*dashboard.Application, error) { +func selectApplication(opts *LoginOptions, apps []dashboard.Application, interactive bool) (*dashboard.Application, error) { if opts.AppName != "" { for i := range apps { if apps[i].Name == opts.AppName { diff --git a/pkg/cmd/shared/apputil/create.go b/pkg/cmd/shared/apputil/create.go index 7e7e8576..318e4bc8 100644 --- a/pkg/cmd/shared/apputil/create.go +++ b/pkg/cmd/shared/apputil/create.go @@ -59,20 +59,13 @@ func CreateApplicationWithRetry( var clusterErr *dashboard.ErrClusterUnavailable if errors.As(err, &clusterErr) { - fmt.Fprintf( - io.Out, - "%s No cluster available in region %q. Please select another region.\n", - cs.WarningIcon(), - region, - ) + fmt.Fprintf(io.Out, "%s No cluster available in region %q. Please select another region.\n", + cs.WarningIcon(), region) region = "" if !io.CanPrompt() { trackCreateFailed(ctx, triggeredFrom, err) - return nil, "", fmt.Errorf( - "no cluster available in region %q — try a different --region", - clusterErr.Region, - ) + return nil, "", fmt.Errorf("no cluster available in region %q — try a different --region", clusterErr.Region) } continue } @@ -198,8 +191,7 @@ func ConfigureProfile( } profileName = strings.ToLower(profileName) - if exists, existingAppID := cfg.ApplicationIDForProfile(profileName); exists && - existingAppID != appDetails.ID { + if exists, existingAppID := cfg.ApplicationIDForProfile(profileName); exists && existingAppID != appDetails.ID { profileName = strings.ToLower(appDetails.Name + "-" + appDetails.ID) } From 40c76935d431c82679bef3879e9fb3ead9780685 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Tue, 9 Jun 2026 16:37:23 -0700 Subject: [PATCH 4/7] feat(events): give plan-change events a consistent plan vocabulary Started carried an ignored plan param; the other events mixed the raw --plan input with the resolved template id under one 'plan' property. Now: requested_plan (raw --plan, omitted when picked interactively) on Started, and a from_plan/to_plan pair (plan ids, 'unknown' fallback) on the accepted/declined/failed/completed events. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/cmd/application/planchange/planchange.go | 32 +++++--- .../application/planchange/planchange_test.go | 64 ++++++++++++++-- pkg/telemetry/events.go | 75 ++++++++++++------- 3 files changed, 129 insertions(+), 42 deletions(-) diff --git a/pkg/cmd/application/planchange/planchange.go b/pkg/cmd/application/planchange/planchange.go index 6bab1b19..3ad554a0 100644 --- a/pkg/cmd/application/planchange/planchange.go +++ b/pkg/cmd/application/planchange/planchange.go @@ -110,6 +110,10 @@ func Run(ctx context.Context, opts *Options) error { return nil } + // The application's current plan, resolved against the self-serve list so + // from_plan and to_plan share the plan-id vocabulary in telemetry. + fromPlan := currentPlanID(plans, app) + if isCurrentPlan(app, *target) { fmt.Fprintf( opts.IO.Out, @@ -151,7 +155,7 @@ func Run(ctx context.Context, opts *Options) error { return err } if !accepted { - telemetry.Track(ctx, ev.declinedTerms(target.ID)) + telemetry.Track(ctx, ev.declinedTerms(fromPlan, target.ID)) fmt.Fprintf( opts.IO.Out, "%s Plan change aborted; no changes were made.\n", @@ -159,17 +163,17 @@ func Run(ctx context.Context, opts *Options) error { ) return nil } - telemetry.Track(ctx, ev.acceptedTerms(target.ID)) + telemetry.Track(ctx, ev.acceptedTerms(fromPlan, target.ID)) if err := callWithReauth(opts.IO, client, &token, "Changing plan", func(t string) error { _, e := client.ChangeApplicationPlan(t, appID, target.ID) return e }); err != nil { class, _, status := cmdutil.ClassifyError(err) - telemetry.Track(ctx, ev.failed(target.ID, class, status)) + telemetry.Track(ctx, ev.failed(fromPlan, target.ID, class, status)) return err } - telemetry.Track(ctx, ev.completed(target.ID)) + telemetry.Track(ctx, ev.completed(fromPlan, target.ID)) if opts.PrintFlags.OutputFlagSpecified() && opts.PrintFlags.OutputFormat != nil { p, err := opts.PrintFlags.ToPrinter() @@ -203,11 +207,11 @@ func Run(ctx context.Context, opts *Options) error { // shared flow emits upgrade- or downgrade-named events without branching at // each call site. type planChangeEvents struct { - started func(plan string) telemetry.Event - acceptedTerms func(plan string) telemetry.Event - declinedTerms func(plan string) telemetry.Event - failed func(plan, errorClass string, httpStatus int) telemetry.Event - completed func(plan string) telemetry.Event + started func(requestedPlan string) telemetry.Event + acceptedTerms func(fromPlan, toPlan string) telemetry.Event + declinedTerms func(fromPlan, toPlan string) telemetry.Event + failed func(fromPlan, toPlan, errorClass string, httpStatus int) telemetry.Event + completed func(fromPlan, toPlan string) telemetry.Event } func planChangeEventsFor(dir Direction) planChangeEvents { @@ -351,6 +355,16 @@ func currentPlanIndex(plans []dashboard.Plan, app *dashboard.Application) int { return -1 } +// currentPlanID returns the id of the plan the application is currently on, +// or "unknown" when the application or its plan can't be resolved (explicit +// value so the telemetry dimension groups cleanly). +func currentPlanID(plans []dashboard.Plan, app *dashboard.Application) string { + if idx := currentPlanIndex(plans, app); idx >= 0 { + return plans[idx].ID + } + return "unknown" +} + // isCurrentPlan reports whether target is the plan the application is already on. func isCurrentPlan(app *dashboard.Application, target dashboard.Plan) bool { if app == nil { diff --git a/pkg/cmd/application/planchange/planchange_test.go b/pkg/cmd/application/planchange/planchange_test.go index 1f7d45e5..d7446752 100644 --- a/pkg/cmd/application/planchange/planchange_test.go +++ b/pkg/cmd/application/planchange/planchange_test.go @@ -439,25 +439,41 @@ func TestRun_SamePlanIsNoOp(t *testing.T) { assert.Contains(t, out.String(), "no change needed") } -// recordingTelemetry records the names of the events emitted during a run. -type recordingTelemetry struct{ events []string } +// recordedEvent is one Track call seen by recordingTelemetry. +type recordedEvent struct { + name string + props map[string]any +} + +// recordingTelemetry records the events emitted during a run. +type recordingTelemetry struct{ events []recordedEvent } func (r *recordingTelemetry) Identify(context.Context) error { return nil } -func (r *recordingTelemetry) Track(_ context.Context, event string, _ map[string]any) error { - r.events = append(r.events, event) +func (r *recordingTelemetry) Track(_ context.Context, event string, props map[string]any) error { + r.events = append(r.events, recordedEvent{name: event, props: props}) return nil } func (r *recordingTelemetry) Close() {} +func (r *recordingTelemetry) names() []string { + names := make([]string, 0, len(r.events)) + for _, e := range r.events { + names = append(names, e.name) + } + return names +} + // TestRun_EmitsPlanChangeEventsByDirection verifies the shared flow emits the -// upgrade- or downgrade-named events depending on the direction. +// upgrade- or downgrade-named events depending on the direction, with the +// from_plan/to_plan pair resolved from the current plan and the picked target. func TestRun_EmitsPlanChangeEventsByDirection(t *testing.T) { tests := []struct { name string direction Direction want []string + wantTo string }{ { name: "upgrade", @@ -467,6 +483,7 @@ func TestRun_EmitsPlanChangeEventsByDirection(t *testing.T) { telemetry.EventApplicationUpgradeAcceptedTerms, telemetry.EventApplicationUpgradeCompleted, }, + wantTo: "grow-plus", }, { name: "downgrade", @@ -476,6 +493,7 @@ func TestRun_EmitsPlanChangeEventsByDirection(t *testing.T) { telemetry.EventApplicationDowngradeAcceptedTerms, telemetry.EventApplicationDowngradeCompleted, }, + wantTo: "build", }, } @@ -495,7 +513,14 @@ func TestRun_EmitsPlanChangeEventsByDirection(t *testing.T) { ctx := telemetry.WithTelemetryClient(context.Background(), rec) require.NoError(t, Run(ctx, opts)) - assert.Equal(t, tt.want, rec.events) + assert.Equal(t, tt.want, rec.names()) + + // Started comes from the interactive picker: no requested_plan. + assert.Empty(t, rec.events[0].props) + + wantPlans := map[string]any{"from_plan": "grow", "to_plan": tt.wantTo} + assert.Equal(t, wantPlans, rec.events[1].props) + assert.Equal(t, wantPlans, rec.events[2].props) }) } } @@ -520,10 +545,35 @@ func TestRun_EmitsDeclinedTermsEvent(t *testing.T) { assert.Equal(t, []string{ telemetry.EventApplicationDowngradeStarted, telemetry.EventApplicationDowngradeDeclinedTerms, - }, rec.events) + }, rec.names()) + assert.Equal( + t, + map[string]any{"from_plan": "grow", "to_plan": "build"}, + rec.events[1].props, + ) assert.Equal(t, 0, srv.patchCalls) } +// TestRun_StartedCarriesRequestedPlan verifies an explicit --plan value lands +// on the Started event as requested_plan. +func TestRun_StartedCarriesRequestedPlan(t *testing.T) { + srv := newServer(t, `{"has_payment_method": true}`) + srv.currentPlanLabel = "Grow" + defer srv.Close() + + opts, _, _ := newOpts(t, srv, false) + opts.Plan = "grow-plus" + opts.AcceptTerms = true + + rec := &recordingTelemetry{} + ctx := telemetry.WithTelemetryClient(context.Background(), rec) + + require.NoError(t, Run(ctx, opts)) + require.NotEmpty(t, rec.events) + assert.Equal(t, telemetry.EventApplicationUpgradeStarted, rec.events[0].name) + assert.Equal(t, map[string]any{"requested_plan": "grow-plus"}, rec.events[0].props) +} + func TestRun_UnknownCurrentPlanShowsAllPlans(t *testing.T) { srv := newServer(t, `{"has_payment_method": true}`) srv.currentPlanLabel = "Enterprise" diff --git a/pkg/telemetry/events.go b/pkg/telemetry/events.go index 4aaa6fbe..ea733a7d 100644 --- a/pkg/telemetry/events.go +++ b/pkg/telemetry/events.go @@ -209,37 +209,43 @@ func ApplicationCreateAborted(triggeredFrom string) Event { } // --- Application upgrade events --------------------------------------------- +// +// Plan properties share one vocabulary across the upgrade/downgrade events: +// requested_plan is the raw --plan input (omitted when the user picked +// interactively), from_plan is the plan the application is on when the change +// is attempted ("unknown" when it can't be resolved), and to_plan is the +// resolved target plan id. // ApplicationUpgradeStarted is emitted when the user starts the upgrade flow. -func ApplicationUpgradeStarted(plan string) Event { +// requestedPlan is the raw --plan value; empty (interactive picker) is omitted. +func ApplicationUpgradeStarted(requestedPlan string) Event { return Event{ - Name: EventApplicationUpgradeStarted, + Name: EventApplicationUpgradeStarted, + Properties: requestedPlanProps(requestedPlan), } } // ApplicationUpgradeAcceptedTerms is emitted when the user accepts the T&C. -func ApplicationUpgradeAcceptedTerms(plan string) Event { +func ApplicationUpgradeAcceptedTerms(fromPlan, toPlan string) Event { return Event{ Name: EventApplicationUpgradeAcceptedTerms, - Properties: map[string]any{"plan": plan}, + Properties: planChangeProps(fromPlan, toPlan), } } // ApplicationUpgradeDeclinedTerms is emitted when the user declines the T&C. -func ApplicationUpgradeDeclinedTerms(plan string) Event { +func ApplicationUpgradeDeclinedTerms(fromPlan, toPlan string) Event { return Event{ Name: EventApplicationUpgradeDeclinedTerms, - Properties: map[string]any{"plan": plan}, + Properties: planChangeProps(fromPlan, toPlan), } } // ApplicationUpgradeFailed is emitted when the Dashboard API returns an error. // httpStatus is omitted when zero. -func ApplicationUpgradeFailed(plan, errorClass string, httpStatus int) Event { - props := map[string]any{ - "plan": plan, - "error_class": errorClass, - } +func ApplicationUpgradeFailed(fromPlan, toPlan, errorClass string, httpStatus int) Event { + props := planChangeProps(fromPlan, toPlan) + props["error_class"] = errorClass if httpStatus != 0 { props["http_status"] = httpStatus } @@ -247,45 +253,45 @@ func ApplicationUpgradeFailed(plan, errorClass string, httpStatus int) Event { } // ApplicationUpgradeCompleted is emitted when the upgrade flow succeeds. -func ApplicationUpgradeCompleted(plan string) Event { +func ApplicationUpgradeCompleted(fromPlan, toPlan string) Event { return Event{ Name: EventApplicationUpgradeCompleted, - Properties: map[string]any{"plan": plan}, + Properties: planChangeProps(fromPlan, toPlan), } } // --- Application downgrade events ------------------------------------------- // ApplicationDowngradeStarted is emitted when the user starts the downgrade flow. -func ApplicationDowngradeStarted(plan string) Event { +// requestedPlan is the raw --plan value; empty (interactive picker) is omitted. +func ApplicationDowngradeStarted(requestedPlan string) Event { return Event{ - Name: EventApplicationDowngradeStarted, + Name: EventApplicationDowngradeStarted, + Properties: requestedPlanProps(requestedPlan), } } // ApplicationDowngradeAcceptedTerms is emitted when the user accepts the T&C. -func ApplicationDowngradeAcceptedTerms(plan string) Event { +func ApplicationDowngradeAcceptedTerms(fromPlan, toPlan string) Event { return Event{ Name: EventApplicationDowngradeAcceptedTerms, - Properties: map[string]any{"plan": plan}, + Properties: planChangeProps(fromPlan, toPlan), } } // ApplicationDowngradeDeclinedTerms is emitted when the user declines the T&C. -func ApplicationDowngradeDeclinedTerms(plan string) Event { +func ApplicationDowngradeDeclinedTerms(fromPlan, toPlan string) Event { return Event{ Name: EventApplicationDowngradeDeclinedTerms, - Properties: map[string]any{"plan": plan}, + Properties: planChangeProps(fromPlan, toPlan), } } // ApplicationDowngradeFailed is emitted when the Dashboard API returns an error. // httpStatus is omitted when zero. -func ApplicationDowngradeFailed(plan, errorClass string, httpStatus int) Event { - props := map[string]any{ - "plan": plan, - "error_class": errorClass, - } +func ApplicationDowngradeFailed(fromPlan, toPlan, errorClass string, httpStatus int) Event { + props := planChangeProps(fromPlan, toPlan) + props["error_class"] = errorClass if httpStatus != 0 { props["http_status"] = httpStatus } @@ -293,9 +299,26 @@ func ApplicationDowngradeFailed(plan, errorClass string, httpStatus int) Event { } // ApplicationDowngradeCompleted is emitted when the downgrade flow succeeds. -func ApplicationDowngradeCompleted(plan string) Event { +func ApplicationDowngradeCompleted(fromPlan, toPlan string) Event { return Event{ Name: EventApplicationDowngradeCompleted, - Properties: map[string]any{"plan": plan}, + Properties: planChangeProps(fromPlan, toPlan), + } +} + +// requestedPlanProps returns the Started properties; nil when the plan wasn't +// given explicitly (interactive picker). +func requestedPlanProps(requestedPlan string) map[string]any { + if requestedPlan == "" { + return nil + } + return map[string]any{"requested_plan": requestedPlan} +} + +// planChangeProps is the shared from/to property pair of plan-change events. +func planChangeProps(fromPlan, toPlan string) map[string]any { + return map[string]any{ + "from_plan": fromPlan, + "to_plan": toPlan, } } From 1861cc2f5d7d4a6501512cd436759e88289b749c Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Tue, 9 Jun 2026 16:38:43 -0700 Subject: [PATCH 5/7] feat(telemetry): add version and event_index to every tracked event version (semver for releases, 'main' for source builds) lets Segment destinations route dev vs prod traffic to the right Amplitude project. event_index is the per-invocation emit order: a deterministic ordering key that doesn't depend on Amplitude's undocumented same-millisecond tie-breaking, complementing the spaced timestamps. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/telemetry/telemetry.go | 20 ++++++++++++++++++-- pkg/telemetry/telemetry_test.go | 22 ++++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/pkg/telemetry/telemetry.go b/pkg/telemetry/telemetry.go index c3b2b61f..279b79a2 100644 --- a/pkg/telemetry/telemetry.go +++ b/pkg/telemetry/telemetry.go @@ -46,8 +46,9 @@ type AnalyticsTelemetryClient struct { client analytics.Client debug bool - mu sync.Mutex - lastTS time.Time + mu sync.Mutex + lastTS time.Time + eventIndex int64 } type AnalyticsTelemetryLogger struct { @@ -309,6 +310,13 @@ func (a *AnalyticsTelemetryClient) Track( "app_id": metadata.AppID, "command": metadata.CommandPath, "flags": metadata.CommandFlags, + // version lets downstream destinations split released binaries + // (semver) from source builds ("main"). + "version": metadata.CLIVersion, + // event_index is the emit order within one invocation: the + // vendor-independent ground truth for sequencing, since Amplitude's + // tie-breaking on identical timestamps is undocumented. + "event_index": a.nextEventIndex(), } for k, v := range properties { props[k] = v @@ -338,6 +346,14 @@ func (a *AnalyticsTelemetryClient) Track( return a.client.Enqueue(track) } +// nextEventIndex returns 1, 2, 3... in emit order for this command run. +func (a *AnalyticsTelemetryClient) nextEventIndex() int64 { + a.mu.Lock() + defer a.mu.Unlock() + a.eventIndex++ + return a.eventIndex +} + // nextTimestamp returns strictly increasing timestamps at least 1ms apart. // Amplitude truncates event_time to milliseconds, so same-millisecond events // are ordered non-deterministically; spacing by 1ms preserves emit order. diff --git a/pkg/telemetry/telemetry_test.go b/pkg/telemetry/telemetry_test.go index f7617b24..77698a09 100644 --- a/pkg/telemetry/telemetry_test.go +++ b/pkg/telemetry/telemetry_test.go @@ -305,3 +305,25 @@ func TestTrack_MergesCustomProperties(t *testing.T) { assert.Contains(t, track.Properties, "invocation_id") assert.Contains(t, track.Properties, "command") } + +// TestTrack_AddsVersionAndEventIndex verifies every event carries the CLI +// version (to split dev/prod traffic downstream) and a 1-based emit-order +// index (the deterministic ordering ground truth). +func TestTrack_AddsVersionAndEventIndex(t *testing.T) { + fake := &fakeAnalyticsClient{} + client := &AnalyticsTelemetryClient{client: fake} + + metadata := NewEventMetadata() + ctx := WithEventMetadata(context.Background(), metadata) + + require.NoError(t, client.Track(ctx, "A", nil)) + require.NoError(t, client.Track(ctx, "B", nil)) + require.Len(t, fake.messages, 2) + + for i, m := range fake.messages { + track, ok := m.(analytics.Track) + require.True(t, ok) + assert.Equal(t, metadata.CLIVersion, track.Properties["version"]) + assert.Equal(t, int64(i+1), track.Properties["event_index"]) + } +} From fabc19f30a15f1f372896bc54c6cc3cfcce476bf Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Tue, 9 Jun 2026 16:40:16 -0700 Subject: [PATCH 6/7] chore(events): drop dead code, classify generic errors as unknown AuthStepBrowser was never emitted (browser failures have their own non-fatal event); root.go now goes through the CommandInvoked constructor like every other catalog event; and the error_class fallback maps Go's generic error types to 'unknown' instead of letting *errors.errorString dominate the dimension with no signal. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/cmd/root/root.go | 7 +++++-- pkg/cmdutil/classify.go | 16 +++++++++++++--- pkg/cmdutil/classify_test.go | 10 ++++++++-- pkg/telemetry/events.go | 5 +++-- 4 files changed, 29 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 1a273032..645080ea 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -192,8 +192,11 @@ func Execute() exitCode { return err } - // Command Invoked; flushed at the end of Execute with the command's other events. - err = telemetryClient.Track(ctx, telemetry.EventCommandInvoked, nil) + // Command Invoked; flushed at the end of Execute with the command's other + // events. The client is called directly (not telemetry.Track) so debug + // runs surface the enqueue error. + invoked := telemetry.CommandInvoked() + err = telemetryClient.Track(ctx, invoked.Name, invoked.Properties) if err != nil && hasDebug { fmt.Fprintf(stderr, "Error tracking telemetry: %s\n", err) } diff --git a/pkg/cmdutil/classify.go b/pkg/cmdutil/classify.go index 7c5bae52..f25a9c89 100644 --- a/pkg/cmdutil/classify.go +++ b/pkg/cmdutil/classify.go @@ -68,7 +68,8 @@ func ClassifyError(err error) (class, source string, httpStatus int) { return "network_error", ErrorSourceNetwork, 0 } - // Fall back to the error's type name: stable/low-cardinality for typed errors. + // Fall back to the root cause's type name: stable and low-cardinality for + // typed errors, "unknown" for generic ones. return rootCauseType(err), ErrorSourceLocal, 0 } @@ -78,13 +79,22 @@ func ErrorClass(err error) string { return class } -// rootCauseType returns the type name of the deepest wrapped error. +// rootCauseType returns the type name of the deepest wrapped error, or +// "unknown" for the generic types every errors.New/fmt.Errorf produces — +// otherwise that one type name would dominate the error_class dimension +// while carrying no signal. func rootCauseType(err error) string { for { next := errors.Unwrap(err) if next == nil { - return fmt.Sprintf("%T", err) + break } err = next } + switch name := fmt.Sprintf("%T", err); name { + case "*errors.errorString", "*errors.joinError", "*fmt.wrapError", "*fmt.wrapErrors": + return "unknown" + default: + return name + } } diff --git a/pkg/cmdutil/classify_test.go b/pkg/cmdutil/classify_test.go index 38a2b8b8..614235e1 100644 --- a/pkg/cmdutil/classify_test.go +++ b/pkg/cmdutil/classify_test.go @@ -71,9 +71,15 @@ func TestClassifyError(t *testing.T) { wantSource: ErrorSourceNetwork, }, { - name: "generic error falls back to local", + name: "generic error falls back to unknown", err: errors.New("something went wrong"), - wantClass: "*errors.errorString", + wantClass: "unknown", + wantSource: ErrorSourceLocal, + }, + { + name: "wrapped generic error stays unknown", + err: fmt.Errorf("outer: %w", errors.New("inner")), + wantClass: "unknown", wantSource: ErrorSourceLocal, }, } diff --git a/pkg/telemetry/events.go b/pkg/telemetry/events.go index ea733a7d..225de5ef 100644 --- a/pkg/telemetry/events.go +++ b/pkg/telemetry/events.go @@ -49,8 +49,9 @@ const ( TriggeredFromAuthFlow = "auth_flow" TriggeredFromExplicitCommand = "explicit_command" - // step: where the auth flow failed (CLI Auth Failed). - AuthStepBrowser = "browser" + // step: where the auth flow failed (CLI Auth Failed). Browser launch + // failures are not a step here: they have their own non-fatal event, + // CLI Auth Browser Failed. AuthStepCallback = "callback" AuthStepExchange = "exchange" AuthStepAppsFetch = "apps_fetch" From b8076b78327b0e62e08a8234c8339e4d4def3c80 Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Tue, 9 Jun 2026 16:45:35 -0700 Subject: [PATCH 7/7] feat(events): emit auth flow events on lazy logins too EnsureAuthenticated and ReauthenticateIfExpired ran the OAuth flow with context.Background(), silently dropping every CLI Auth * event on the most common auth path (running any command while logged out, or with an expired session). Thread the command context down so lazy logins emit the same flow events, attributed to the command that triggered them. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/auth/authenticate.go | 12 ++++--- pkg/cmd/application/create/create.go | 11 +++--- pkg/cmd/application/list/list.go | 9 ++--- pkg/cmd/application/planchange/planchange.go | 16 +++++---- pkg/cmd/application/plans/plans.go | 9 ++--- pkg/cmd/application/plans/plans_test.go | 5 +-- pkg/cmd/application/selectapp/select.go | 13 +++---- pkg/cmd/application/update/update.go | 9 ++--- pkg/cmd/application/update/update_test.go | 5 +-- pkg/cmd/open/open.go | 33 +++++++++-------- pkg/cmd/open/open_test.go | 37 ++++++++++---------- 11 files changed, 87 insertions(+), 72 deletions(-) diff --git a/pkg/auth/authenticate.go b/pkg/auth/authenticate.go index a8698d3d..a04a72ef 100644 --- a/pkg/auth/authenticate.go +++ b/pkg/auth/authenticate.go @@ -11,8 +11,11 @@ import ( // EnsureAuthenticated returns a valid access token from the stored session. // If no valid session exists and the terminal is interactive, it triggers the -// browser-based OAuth login flow automatically. +// browser-based OAuth login flow automatically. ctx carries the command's +// telemetry context so lazy logins emit the same auth flow events as the +// dedicated auth login/signup commands. func EnsureAuthenticated( + ctx context.Context, io *iostreams.IOStreams, client *dashboard.Client, ) (string, error) { @@ -24,14 +27,13 @@ func EnsureAuthenticated( cs := io.ColorScheme() fmt.Fprintf(io.Out, "%s %s\n", cs.WarningIcon(), err) - // Lazy login from another command: no request-scoped telemetry context here, - // so OAuth flow events are emitted only by the dedicated auth login/signup commands. - return RunOAuth(context.Background(), io, client, false, true) + return RunOAuth(ctx, io, client, false, true) } // ReauthenticateIfExpired checks if err is a session-expired error from the API. // If so, it clears the invalid token and triggers the login flow. func ReauthenticateIfExpired( + ctx context.Context, io *iostreams.IOStreams, client *dashboard.Client, err error, @@ -44,5 +46,5 @@ func ReauthenticateIfExpired( ClearToken() fmt.Fprintf(io.Out, "%s Session expired.\n", cs.WarningIcon()) - return RunOAuth(context.Background(), io, client, false, true) + return RunOAuth(ctx, io, client, false, true) } diff --git a/pkg/cmd/application/create/create.go b/pkg/cmd/application/create/create.go index 14911394..dfda0dfe 100644 --- a/pkg/cmd/application/create/create.go +++ b/pkg/cmd/application/create/create.go @@ -137,13 +137,13 @@ func runCreateCmd(ctx context.Context, opts *CreateOptions) error { client := opts.NewDashboardClient(auth.OAuthClientID()) - token, err := auth.EnsureAuthenticated(opts.IO, client) + token, err := auth.EnsureAuthenticated(ctx, opts.IO, client) if err != nil { return err } var plans []dashboard.Plan - if err := callWithReauth(opts.IO, client, &token, "Fetching plans", func(t string) error { + if err := callWithReauth(ctx, opts.IO, client, &token, "Fetching plans", func(t string) error { var e error plans, e = client.GetSelfServePlans(t) return e @@ -156,7 +156,7 @@ func runCreateCmd(ctx context.Context, opts *CreateOptions) error { // Best-effort: continue without billing status if /1/user fails. var user *dashboard.DashboardUser - if err := callWithReauth(opts.IO, client, &token, "Checking account", func(t string) error { + if err := callWithReauth(ctx, opts.IO, client, &token, "Checking account", func(t string) error { var e error user, e = client.GetUser(t) return e @@ -204,7 +204,7 @@ func runCreateCmd(ctx context.Context, opts *CreateOptions) error { } if !target.IsFree() { - if err := callWithReauth(opts.IO, client, &token, "Applying plan", func(t string) error { + if err := callWithReauth(ctx, opts.IO, client, &token, "Applying plan", func(t string) error { _, e := client.ChangeApplicationPlan(t, appDetails.ID, target.ID) return e }); err != nil { @@ -400,6 +400,7 @@ func offerBilling(opts *CreateOptions, client *dashboard.Client, plan dashboard. // callWithReauth runs fn, re-authenticating once and retrying on an expired session. func callWithReauth( + ctx context.Context, io *iostreams.IOStreams, client *dashboard.Client, token *string, @@ -413,7 +414,7 @@ func callWithReauth( return nil } - newToken, reAuthErr := auth.ReauthenticateIfExpired(io, client, err) + newToken, reAuthErr := auth.ReauthenticateIfExpired(ctx, io, client, err) if reAuthErr != nil { return reAuthErr } diff --git a/pkg/cmd/application/list/list.go b/pkg/cmd/application/list/list.go index 7dc1f3b3..e02bb97f 100644 --- a/pkg/cmd/application/list/list.go +++ b/pkg/cmd/application/list/list.go @@ -1,6 +1,7 @@ package list import ( + "context" "fmt" "github.com/AlecAivazis/survey/v2" @@ -55,7 +56,7 @@ func NewListCmd(f *cmdutil.Factory) *cobra.Command { "skipAuthCheck": "true", }, RunE: func(cmd *cobra.Command, args []string) error { - return runListCmd(opts) + return runListCmd(cmd.Context(), opts) }, } @@ -64,11 +65,11 @@ func NewListCmd(f *cmdutil.Factory) *cobra.Command { return cmd } -func runListCmd(opts *ListOptions) error { +func runListCmd(ctx context.Context, opts *ListOptions) error { cs := opts.IO.ColorScheme() client := opts.NewDashboardClient(auth.OAuthClientID()) - accessToken, err := auth.EnsureAuthenticated(opts.IO, client) + accessToken, err := auth.EnsureAuthenticated(ctx, opts.IO, client) if err != nil { return err } @@ -77,7 +78,7 @@ func runListCmd(opts *ListOptions) error { apps, err := client.ListApplications(accessToken) opts.IO.StopProgressIndicator() if err != nil { - newToken, reAuthErr := auth.ReauthenticateIfExpired(opts.IO, client, err) + newToken, reAuthErr := auth.ReauthenticateIfExpired(ctx, opts.IO, client, err) if reAuthErr != nil { return reAuthErr } diff --git a/pkg/cmd/application/planchange/planchange.go b/pkg/cmd/application/planchange/planchange.go index 3ad554a0..d954044c 100644 --- a/pkg/cmd/application/planchange/planchange.go +++ b/pkg/cmd/application/planchange/planchange.go @@ -72,13 +72,13 @@ func Run(ctx context.Context, opts *Options) error { client := opts.NewDashboardClient(auth.OAuthClientID()) - token, err := auth.EnsureAuthenticated(opts.IO, client) + token, err := auth.EnsureAuthenticated(ctx, opts.IO, client) if err != nil { return err } var plans []dashboard.Plan - if err := callWithReauth(opts.IO, client, &token, "Fetching plans", func(t string) error { + if err := callWithReauth(ctx, opts.IO, client, &token, "Fetching plans", func(t string) error { var e error plans, e = client.GetSelfServePlans(t) return e @@ -92,7 +92,7 @@ func Run(ctx context.Context, opts *Options) error { // Billing status is best-effort. If /1/user is unavailable we continue // without it and let the server enforce billing validity. var user *dashboard.DashboardUser - if err := callWithReauth(opts.IO, client, &token, "Checking account", func(t string) error { + if err := callWithReauth(ctx, opts.IO, client, &token, "Checking account", func(t string) error { var e error user, e = client.GetUser(t) return e @@ -100,7 +100,7 @@ func Run(ctx context.Context, opts *Options) error { user = nil } - app := fetchApplication(opts, client, &token, appID) + app := fetchApplication(ctx, opts, client, &token, appID) target, err := resolveTarget(opts, appID, app, plans) if err != nil { @@ -165,7 +165,7 @@ func Run(ctx context.Context, opts *Options) error { } telemetry.Track(ctx, ev.acceptedTerms(fromPlan, target.ID)) - if err := callWithReauth(opts.IO, client, &token, "Changing plan", func(t string) error { + if err := callWithReauth(ctx, opts.IO, client, &token, "Changing plan", func(t string) error { _, e := client.ChangeApplicationPlan(t, appID, target.ID) return e }); err != nil { @@ -299,13 +299,14 @@ func resolveTarget( // fetchApplication returns the current application, or nil if it can't be fetched. func fetchApplication( + ctx context.Context, opts *Options, client *dashboard.Client, token *string, appID string, ) *dashboard.Application { var app *dashboard.Application - if err := callWithReauth(opts.IO, client, token, "Fetching application", func(t string) error { + if err := callWithReauth(ctx, opts.IO, client, token, "Fetching application", func(t string) error { var e error app, e = client.GetApplication(t, appID) return e @@ -512,6 +513,7 @@ func planChoices(plans []dashboard.Plan) []string { // session-expired error it re-authenticates once and retries, mirroring the // retry pattern used by the other application commands. func callWithReauth( + ctx context.Context, io *iostreams.IOStreams, client *dashboard.Client, token *string, @@ -525,7 +527,7 @@ func callWithReauth( return nil } - newToken, reAuthErr := auth.ReauthenticateIfExpired(io, client, err) + newToken, reAuthErr := auth.ReauthenticateIfExpired(ctx, io, client, err) if reAuthErr != nil { return reAuthErr } diff --git a/pkg/cmd/application/plans/plans.go b/pkg/cmd/application/plans/plans.go index e07413c8..f384967d 100644 --- a/pkg/cmd/application/plans/plans.go +++ b/pkg/cmd/application/plans/plans.go @@ -1,6 +1,7 @@ package plans import ( + "context" "fmt" "github.com/MakeNowJust/heredoc" @@ -52,7 +53,7 @@ func NewPlansCmd(f *cmdutil.Factory) *cobra.Command { "skipAuthCheck": "true", }, RunE: func(cmd *cobra.Command, args []string) error { - return runPlansCmd(opts) + return runPlansCmd(cmd.Context(), opts) }, } @@ -61,12 +62,12 @@ func NewPlansCmd(f *cmdutil.Factory) *cobra.Command { return cmd } -func runPlansCmd(opts *PlansOptions) error { +func runPlansCmd(ctx context.Context, opts *PlansOptions) error { cs := opts.IO.ColorScheme() client := opts.NewDashboardClient(auth.OAuthClientID()) - accessToken, err := auth.EnsureAuthenticated(opts.IO, client) + accessToken, err := auth.EnsureAuthenticated(ctx, opts.IO, client) if err != nil { return err } @@ -75,7 +76,7 @@ func runPlansCmd(opts *PlansOptions) error { plans, err := client.GetSelfServePlans(accessToken) opts.IO.StopProgressIndicator() if err != nil { - newToken, reAuthErr := auth.ReauthenticateIfExpired(opts.IO, client, err) + newToken, reAuthErr := auth.ReauthenticateIfExpired(ctx, opts.IO, client, err) if reAuthErr != nil { return reAuthErr } diff --git a/pkg/cmd/application/plans/plans_test.go b/pkg/cmd/application/plans/plans_test.go index 05bc16c3..889b3a63 100644 --- a/pkg/cmd/application/plans/plans_test.go +++ b/pkg/cmd/application/plans/plans_test.go @@ -1,6 +1,7 @@ package plans import ( + "context" "encoding/json" "net/http" "net/http/httptest" @@ -95,7 +96,7 @@ func Test_runPlansCmd(t *testing.T) { defer srv.Close() opts, out := newOpts(t, srv, true, "") - require.NoError(t, runPlansCmd(opts)) + require.NoError(t, runPlansCmd(context.Background(), opts)) got := out.String() assert.Contains(t, got, "Build") @@ -109,7 +110,7 @@ func Test_runPlansCmd_outputJSON(t *testing.T) { defer srv.Close() opts, out := newOpts(t, srv, false, "json") - require.NoError(t, runPlansCmd(opts)) + require.NoError(t, runPlansCmd(context.Background(), opts)) got := out.String() assert.Contains(t, got, `"name":"Build"`) diff --git a/pkg/cmd/application/selectapp/select.go b/pkg/cmd/application/selectapp/select.go index ad69d089..cdc2af12 100644 --- a/pkg/cmd/application/selectapp/select.go +++ b/pkg/cmd/application/selectapp/select.go @@ -1,6 +1,7 @@ package selectapp import ( + "context" "fmt" "github.com/AlecAivazis/survey/v2" @@ -58,7 +59,7 @@ func NewSelectCmd(f *cmdutil.Factory) *cobra.Command { "skipAuthCheck": "true", }, RunE: func(cmd *cobra.Command, args []string) error { - _, err := runSelectCmd(opts) + _, err := runSelectCmd(cmd.Context(), opts) return err }, } @@ -73,7 +74,7 @@ func NewSelectCmd(f *cmdutil.Factory) *cobra.Command { // chosen application. Other commands (e.g. open) use it to ensure an // application is selected before proceeding. A nil application is returned // when the account has no applications. -func Run(f *cmdutil.Factory) (*dashboard.Application, error) { +func Run(ctx context.Context, f *cmdutil.Factory) (*dashboard.Application, error) { opts := &SelectOptions{ IO: f.IOStreams, Config: f.Config, @@ -82,14 +83,14 @@ func Run(f *cmdutil.Factory) (*dashboard.Application, error) { }, } - return runSelectCmd(opts) + return runSelectCmd(ctx, opts) } -func runSelectCmd(opts *SelectOptions) (*dashboard.Application, error) { +func runSelectCmd(ctx context.Context, opts *SelectOptions) (*dashboard.Application, error) { cs := opts.IO.ColorScheme() client := opts.NewDashboardClient(auth.OAuthClientID()) - accessToken, err := auth.EnsureAuthenticated(opts.IO, client) + accessToken, err := auth.EnsureAuthenticated(ctx, opts.IO, client) if err != nil { return nil, err } @@ -98,7 +99,7 @@ func runSelectCmd(opts *SelectOptions) (*dashboard.Application, error) { apps, err := client.ListApplications(accessToken) opts.IO.StopProgressIndicator() if err != nil { - newToken, reAuthErr := auth.ReauthenticateIfExpired(opts.IO, client, err) + newToken, reAuthErr := auth.ReauthenticateIfExpired(ctx, opts.IO, client, err) if reAuthErr != nil { return nil, reAuthErr } diff --git a/pkg/cmd/application/update/update.go b/pkg/cmd/application/update/update.go index 7a0cbab5..aaa892ba 100644 --- a/pkg/cmd/application/update/update.go +++ b/pkg/cmd/application/update/update.go @@ -1,6 +1,7 @@ package update import ( + "context" "fmt" "github.com/MakeNowJust/heredoc" @@ -51,7 +52,7 @@ func NewUpdateCmd(f *cmdutil.Factory) *cobra.Command { "skipAuthCheck": "true", }, RunE: func(cmd *cobra.Command, args []string) error { - return runUpdateCmd(opts) + return runUpdateCmd(cmd.Context(), opts) }, } @@ -63,7 +64,7 @@ func NewUpdateCmd(f *cmdutil.Factory) *cobra.Command { return cmd } -func runUpdateCmd(opts *UpdateOptions) error { +func runUpdateCmd(ctx context.Context, opts *UpdateOptions) error { cs := opts.IO.ColorScheme() appID, err := opts.Config.Profile().GetApplicationID() @@ -76,7 +77,7 @@ func runUpdateCmd(opts *UpdateOptions) error { client := opts.NewDashboardClient(auth.OAuthClientID()) - accessToken, err := auth.EnsureAuthenticated(opts.IO, client) + accessToken, err := auth.EnsureAuthenticated(ctx, opts.IO, client) if err != nil { return err } @@ -85,7 +86,7 @@ func runUpdateCmd(opts *UpdateOptions) error { app, err := client.UpdateApplication(accessToken, appID, opts.Name) opts.IO.StopProgressIndicator() if err != nil { - newToken, reAuthErr := auth.ReauthenticateIfExpired(opts.IO, client, err) + newToken, reAuthErr := auth.ReauthenticateIfExpired(ctx, opts.IO, client, err) if reAuthErr != nil { return reAuthErr } diff --git a/pkg/cmd/application/update/update_test.go b/pkg/cmd/application/update/update_test.go index 9c3dd403..3efb2973 100644 --- a/pkg/cmd/application/update/update_test.go +++ b/pkg/cmd/application/update/update_test.go @@ -1,6 +1,7 @@ package update import ( + "context" "encoding/json" "net/http" "net/http/httptest" @@ -84,7 +85,7 @@ func Test_runUpdateCmd(t *testing.T) { defer srv.Close() opts, out := newOpts(t, srv, true, "") - require.NoError(t, runUpdateCmd(opts)) + require.NoError(t, runUpdateCmd(context.Background(), opts)) got := out.String() assert.Contains(t, got, "APP1") @@ -96,7 +97,7 @@ func Test_runUpdateCmd_outputJSON(t *testing.T) { defer srv.Close() opts, out := newOpts(t, srv, false, "json") - require.NoError(t, runUpdateCmd(opts)) + require.NoError(t, runUpdateCmd(context.Background(), opts)) got := out.String() assert.Contains(t, got, `"id":"APP1"`) diff --git a/pkg/cmd/open/open.go b/pkg/cmd/open/open.go index 0f46db8a..076d7caf 100644 --- a/pkg/cmd/open/open.go +++ b/pkg/cmd/open/open.go @@ -1,6 +1,7 @@ package open import ( + "context" "fmt" "sort" "strings" @@ -106,8 +107,8 @@ type OpenOptions struct { PrintFlags *cmdutil.PrintFlags - Authenticate func(*iostreams.IOStreams, *dashboard.Client) (string, error) - SelectApplication func() (*dashboard.Application, error) + Authenticate func(context.Context, *iostreams.IOStreams, *dashboard.Client) (string, error) + SelectApplication func(context.Context) (*dashboard.Application, error) ListApplications func(*dashboard.Client, string) ([]dashboard.Application, error) NewDashboardClient func(clientID string) *dashboard.Client Browser func(string) error @@ -119,8 +120,8 @@ func NewOpenCmd(f *cmdutil.Factory) *cobra.Command { config: f.Config, PrintFlags: cmdutil.NewPrintFlags(), Authenticate: auth.EnsureAuthenticated, - SelectApplication: func() (*dashboard.Application, error) { - return selectapp.Run(f) + SelectApplication: func(ctx context.Context) (*dashboard.Application, error) { + return selectapp.Run(ctx, f) }, NewDashboardClient: func(clientID string) *dashboard.Client { return dashboard.NewClient(clientID) @@ -174,7 +175,7 @@ func NewOpenCmd(f *cmdutil.Factory) *cobra.Command { if len(args) > 0 { opts.Shortcut = args[0] } - return runOpenCmd(opts) + return runOpenCmd(cmd.Context(), opts) }, } @@ -186,7 +187,7 @@ func NewOpenCmd(f *cmdutil.Factory) *cobra.Command { return cmd } -func runOpenCmd(opts *OpenOptions) error { +func runOpenCmd(ctx context.Context, opts *OpenOptions) error { listing := opts.List || opts.Shortcut == "" // With an output format, emit page metadata instead of opening a browser. @@ -206,11 +207,11 @@ func runOpenCmd(opts *OpenOptions) error { if resource.AppPath != "" { if _, err := opts.config.Profile().GetApplicationID(); err == nil { client := opts.NewDashboardClient(auth.OAuthClientID()) - accessToken, err := opts.Authenticate(opts.IO, client) + accessToken, err := opts.Authenticate(ctx, opts.IO, client) if err != nil { return err } - appID, err := opts.resolveApplicationForAccount(client, accessToken) + appID, err := opts.resolveApplicationForAccount(ctx, client, accessToken) if err != nil { return err } @@ -224,7 +225,7 @@ func runOpenCmd(opts *OpenOptions) error { // Application pages require sign-in and an application scope. if target, ok := dashboardTargets[opts.Shortcut]; ok { - return openDashboardTarget(opts, target) + return openDashboardTarget(ctx, opts, target) } return unsupportedShortcutError(opts.Shortcut) @@ -310,14 +311,14 @@ func (opts *OpenOptions) allEntries() []pageEntry { // openDashboardTarget signs the user in, resolves the current application // (selecting one if needed), then opens the dashboard page. -func openDashboardTarget(opts *OpenOptions, target dashboardTarget) error { +func openDashboardTarget(ctx context.Context, opts *OpenOptions, target dashboardTarget) error { client := opts.NewDashboardClient(auth.OAuthClientID()) - accessToken, err := opts.Authenticate(opts.IO, client) + accessToken, err := opts.Authenticate(ctx, opts.IO, client) if err != nil { return err } - appID, err := opts.resolveApplicationForAccount(client, accessToken) + appID, err := opts.resolveApplicationForAccount(ctx, client, accessToken) if err != nil { return err } @@ -347,6 +348,7 @@ func applicationInAccount(apps []dashboard.Application, appID string) bool { } func (opts *OpenOptions) fetchAccountApplications( + ctx context.Context, client *dashboard.Client, accessToken string, ) ([]dashboard.Application, error) { @@ -361,7 +363,7 @@ func (opts *OpenOptions) fetchAccountApplications( apps, err := listFn(client, accessToken) opts.IO.StopProgressIndicator() if err != nil { - newToken, reAuthErr := auth.ReauthenticateIfExpired(opts.IO, client, err) + newToken, reAuthErr := auth.ReauthenticateIfExpired(ctx, opts.IO, client, err) if reAuthErr != nil { return nil, reAuthErr } @@ -381,10 +383,11 @@ func (opts *OpenOptions) fetchAccountApplications( // appears in the account's application list; otherwise the user is prompted to // select one. func (opts *OpenOptions) resolveApplicationForAccount( + ctx context.Context, client *dashboard.Client, accessToken string, ) (string, error) { - apps, err := opts.fetchAccountApplications(client, accessToken) + apps, err := opts.fetchAccountApplications(ctx, client, accessToken) if err != nil { return "", err } @@ -394,7 +397,7 @@ func (opts *OpenOptions) resolveApplicationForAccount( return appID, nil } - app, selErr := opts.SelectApplication() + app, selErr := opts.SelectApplication(ctx) if selErr != nil { return "", selErr } diff --git a/pkg/cmd/open/open_test.go b/pkg/cmd/open/open_test.go index be83cd52..f4e72432 100644 --- a/pkg/cmd/open/open_test.go +++ b/pkg/cmd/open/open_test.go @@ -1,6 +1,7 @@ package open import ( + "context" "encoding/json" "errors" "testing" @@ -59,11 +60,11 @@ func newTestOptions( IO: io, config: cfg, PrintFlags: cmdutil.NewPrintFlags(), - Authenticate: func(_ *iostreams.IOStreams, _ *dashboard.Client) (string, error) { + Authenticate: func(_ context.Context, _ *iostreams.IOStreams, _ *dashboard.Client) (string, error) { *authed = true return "test-token", nil }, - SelectApplication: func() (*dashboard.Application, error) { + SelectApplication: func(context.Context) (*dashboard.Application, error) { return nil, errors.New("SelectApplication should not be called") }, NewDashboardClient: func(string) *dashboard.Client { @@ -94,7 +95,7 @@ func TestRunOpenCmd_ResourceShortcutNoAuth(t *testing.T) { opts, opened, authed := newTestOptions(io, test.NewDefaultConfigStub()) opts.Shortcut = "docs" - err := runOpenCmd(opts) + err := runOpenCmd(context.Background(), opts) require.NoError(t, err) assert.False(t, *authed, "resource shortcuts must not require sign-in") assert.Equal(t, "https://algolia.com/doc/", *opened) @@ -113,7 +114,7 @@ func TestRunOpenCmd_ResourceShortcutWithAppID(t *testing.T) { return &dashboard.Client{DashboardURL: "https://staging.algolia.test"} } - err := runOpenCmd(opts) + err := runOpenCmd(context.Background(), opts) require.NoError(t, err) assert.True(t, *authed, "app-scoped status validates the profile application against the signed-in account") assert.Equal(t, "https://staging.algolia.test/apps/APP123/monitoring/status", *opened) @@ -131,7 +132,7 @@ func TestRunOpenCmd_ResourceShortcutNoAppUsesDefault(t *testing.T) { opts, opened, _ := newTestOptions(io, cfg) opts.Shortcut = "status" - err := runOpenCmd(opts) + err := runOpenCmd(context.Background(), opts) require.NoError(t, err) assert.Equal(t, "https://status.algolia.com/", *opened) } @@ -148,7 +149,7 @@ func TestRunOpenCmd_DashboardTargetConfiguredApp(t *testing.T) { opts, opened, authed := newTestOptions(io, cfg) opts.Shortcut = "billing" - err := runOpenCmd(opts) + err := runOpenCmd(context.Background(), opts) require.NoError(t, err) assert.True(t, *authed, "application pages require sign-in") assert.Equal( @@ -170,7 +171,7 @@ func TestRunOpenCmd_DashboardTargetAppScoped(t *testing.T) { opts, opened, _ := newTestOptions(io, cfg) opts.Shortcut = "dashboard" - err := runOpenCmd(opts) + err := runOpenCmd(context.Background(), opts) require.NoError(t, err) assert.Equal(t, "https://dashboard.algolia.com/apps/APP123/dashboard", *opened) } @@ -190,7 +191,7 @@ func TestRunOpenCmd_DashboardTargetUsesConfiguredDashboardURL(t *testing.T) { return &dashboard.Client{DashboardURL: "https://staging.algolia.test"} } - err := runOpenCmd(opts) + err := runOpenCmd(context.Background(), opts) require.NoError(t, err) assert.Equal( t, @@ -216,12 +217,12 @@ func TestRunOpenCmd_DashboardTargetIgnoresStaleProfileApp(t *testing.T) { opts.ListApplications = func(_ *dashboard.Client, _ string) ([]dashboard.Application, error) { return []dashboard.Application{{ID: "USER_B_APP", Name: "User B App"}}, nil } - opts.SelectApplication = func() (*dashboard.Application, error) { + opts.SelectApplication = func(context.Context) (*dashboard.Application, error) { selectCalled = true return &dashboard.Application{ID: "USER_B_APP", Name: "User B App"}, nil } - err := runOpenCmd(opts) + err := runOpenCmd(context.Background(), opts) require.NoError(t, err) assert.True(t, selectCalled, "stale profile application must trigger selection") assert.Equal( @@ -244,11 +245,11 @@ func TestRunOpenCmd_DashboardTargetSelectsAppWhenNoneConfigured(t *testing.T) { opts, opened, _ := newTestOptions(io, cfg) opts.Shortcut = "dashboard" - opts.SelectApplication = func() (*dashboard.Application, error) { + opts.SelectApplication = func(context.Context) (*dashboard.Application, error) { return &dashboard.Application{ID: "SELECTED", Name: "Picked"}, nil } - err := runOpenCmd(opts) + err := runOpenCmd(context.Background(), opts) require.NoError(t, err) assert.Equal(t, "https://dashboard.algolia.com/apps/SELECTED/dashboard", *opened) } @@ -259,7 +260,7 @@ func TestRunOpenCmd_Unsupported(t *testing.T) { opts, opened, authed := newTestOptions(io, test.NewDefaultConfigStub()) opts.Shortcut = "bogus" - err := runOpenCmd(opts) + err := runOpenCmd(context.Background(), opts) require.Error(t, err) assert.Contains(t, err.Error(), "unsupported open command, given: bogus") assert.Contains(t, err.Error(), "Available shortcuts:") @@ -275,7 +276,7 @@ func TestRunOpenCmd_ListIncludesBothKinds(t *testing.T) { opts, _, _ := newTestOptions(io, test.NewDefaultConfigStub()) opts.List = true - err := runOpenCmd(opts) + err := runOpenCmd(context.Background(), opts) require.NoError(t, err) out := stdout.String() @@ -296,7 +297,7 @@ func TestRunOpenCmd_ListJSONOutput(t *testing.T) { opts.List = true withOutputFormat(opts, "json") - err := runOpenCmd(opts) + err := runOpenCmd(context.Background(), opts) require.NoError(t, err) var entries []pageEntry @@ -339,7 +340,7 @@ func TestRunOpenCmd_SingleShortcutJSONOutput(t *testing.T) { opts.Shortcut = "billing" withOutputFormat(opts, "json") - err := runOpenCmd(opts) + err := runOpenCmd(context.Background(), opts) require.NoError(t, err) var entry pageEntry @@ -366,7 +367,7 @@ func TestRunOpenCmd_UnsupportedWithJSONOutput(t *testing.T) { opts.Shortcut = "bogus" withOutputFormat(opts, "json") - err := runOpenCmd(opts) + err := runOpenCmd(context.Background(), opts) require.Error(t, err) assert.Contains(t, err.Error(), "unsupported open command, given: bogus") assert.Contains(t, err.Error(), "Available shortcuts:") @@ -379,7 +380,7 @@ func TestRunOpenCmd_InvalidOutputFormat(t *testing.T) { opts.List = true withOutputFormat(opts, "yaml") - err := runOpenCmd(opts) + err := runOpenCmd(context.Background(), opts) require.Error(t, err) assert.Contains(t, err.Error(), "unable to match a printer") }