CLOUDP-297205: Fix duplicated telemetry and early uuid collection (#3584)

blva · web-flow · commit ce5c04cd84d5 · 2025-01-28T16:43:35.000Z
diff --git a/internal/cli/deployments/connect.go b/internal/cli/deployments/connect.go
@@ -50,6 +50,7 @@ func Run(ctx context.Context, opts *ConnectOpts) error {
 
 func PostRun(opts *ConnectOpts) {
 	opts.DeploymentTelemetry.AppendDeploymentType()
+	opts.DeploymentTelemetry.AppendDeploymentUUID()
 }
 
 // atlas deployments connect [clusterName].
diff --git a/internal/cli/deployments/connect_test.go b/internal/cli/deployments/connect_test.go
@@ -157,5 +157,11 @@ func TestPostRun(t *testing.T) {
 		AppendDeploymentType().
 		Times(1)
 
+	deploymentsTest.
+		MockDeploymentTelemetry.
+		EXPECT().
+		AppendDeploymentUUID().
+		Times(1)
+
 	PostRun(opts)
 }
diff --git a/internal/cli/deployments/delete.go b/internal/cli/deployments/delete.go
@@ -69,6 +69,7 @@ func (opts *DeleteOpts) Run(ctx context.Context) error {
 	if opts.IsAtlasDeploymentType() {
 		return opts.runAtlas()
 	}
+
 	return opts.runLocal(ctx)
 }
 
@@ -77,9 +78,13 @@ func (opts *DeleteOpts) runAtlas() error {
 }
 
 func (opts *DeleteOpts) runLocal(ctx context.Context) error {
+	// Collect UUID before deletion
+	opts.DeploymentTelemetry.AppendDeploymentUUID()
+
 	return opts.Delete(func() error {
 		_, _ = log.Warningln("deleting deployment...")
 		opts.StartSpinner()
+
 		defer opts.StopSpinner()
 		return opts.DeploymentOpts.RemoveLocal(ctx)
 	})
@@ -141,6 +146,7 @@ Deleting a Local deployment also deletes any local data volumes.
 			if len(args) == 1 {
 				opts.DeploymentName = args[0]
 			}
+
 			return opts.PreRunE(
 				opts.initAtlasStore(cmd.Context()),
 				opts.InitOutput(cmd.OutOrStdout(), ""),
diff --git a/internal/cli/deployments/delete_test.go b/internal/cli/deployments/delete_test.go
@@ -83,6 +83,11 @@ func TestDelete_Run_Local(t *testing.T) {
 	opts.Confirm = true
 
 	deploymentsTest.LocalMockFlow(ctx)
+	deploymentsTest.
+		MockDeploymentTelemetry.
+		EXPECT().
+		AppendDeploymentUUID().
+		Times(1)
 
 	deploymentsTest.
 		MockContainerEngine.
diff --git a/internal/cli/deployments/logs.go b/internal/cli/deployments/logs.go
@@ -229,6 +229,7 @@ func (opts *DownloadOpts) validateAtlasFlags() error {
 
 func (opts *DownloadOpts) PostRun() {
 	opts.DeploymentTelemetry.AppendDeploymentType()
+	opts.DeploymentTelemetry.AppendDeploymentUUID()
 }
 
 // atlas deployments logs.
diff --git a/internal/cli/deployments/logs_test.go b/internal/cli/deployments/logs_test.go
@@ -115,5 +115,11 @@ func TestDownloadOpts_PostRun(t *testing.T) {
 		AppendDeploymentType().
 		Times(1)
 
+	deploymentTest.
+		MockDeploymentTelemetry.
+		EXPECT().
+		AppendDeploymentUUID().
+		Times(1)
+
 	opts.PostRun()
 }
diff --git a/internal/cli/deployments/options/deployment_opts_telemetry.go b/internal/cli/deployments/options/deployment_opts_telemetry.go
@@ -38,6 +38,7 @@ var (
 //go:generate mockgen -destination=../../../mocks/mock_deployment_opts_telemetry.go -package=mocks github.com/mongodb/mongodb-atlas-cli/atlascli/internal/cli/deployments/options DeploymentTelemetry
 type DeploymentTelemetry interface {
 	AppendDeploymentType()
+	AppendDeploymentUUID()
 }
 
 func NewDeploymentTypeTelemetry(opts *DeploymentOpts) DeploymentTelemetry {
@@ -91,13 +92,20 @@ func (opts *DeploymentOpts) AppendDeploymentType() {
 	var deploymentType string
 	if opts.IsLocalDeploymentType() {
 		deploymentType = LocalCluster
-		if err := opts.collectUUID(context.TODO()); err != nil {
-			_, _ = log.Debugf("error collecting deployment uuid: %v", err)
-		}
 	} else if opts.IsAtlasDeploymentType() {
 		deploymentType = AtlasCluster
 	}
 	if deploymentType != "" {
 		telemetry.AppendOption(telemetry.WithDeploymentType(deploymentType))
 	}
 }
+
+func (opts *DeploymentOpts) AppendDeploymentUUID() {
+	if opts.IsAtlasDeploymentType() || opts.DeploymentType == "" || opts.DeploymentUUID != "" {
+		return
+	}
+
+	if err := opts.collectUUID(context.TODO()); err != nil {
+		_, _ = log.Debugf("error collecting deployment uuid: %v", err)
+	}
+}
diff --git a/internal/cli/deployments/pause.go b/internal/cli/deployments/pause.go
@@ -70,6 +70,9 @@ func (opts *PauseOpts) Run(ctx context.Context) error {
 }
 
 func (opts *PauseOpts) RunLocal(ctx context.Context, deployment options.Deployment) error {
+	// Collect UUID before pausing
+	opts.DeploymentTelemetry.AppendDeploymentUUID()
+
 	if err := opts.stopContainer(ctx, deployment); err != nil {
 		return err
 	}
diff --git a/internal/cli/deployments/pause_test.go b/internal/cli/deployments/pause_test.go
@@ -58,6 +58,12 @@ func TestPause_RunLocal(t *testing.T) {
 		Return(nil).
 		Times(1)
 
+	deploymentTest.
+		MockDeploymentTelemetry.
+		EXPECT().
+		AppendDeploymentUUID().
+		Times(1)
+
 	if err := pauseOpts.Run(ctx); err != nil {
 		t.Fatalf("Run() unexpected error: %v", err)
 	}
diff --git a/internal/cli/deployments/search/indexes/create.go b/internal/cli/deployments/search/indexes/create.go
@@ -304,7 +304,9 @@ func (opts *CreateOpts) watchAtlas(_ context.Context) (any, bool, error) {
 }
 
 func (opts *CreateOpts) PostRun(ctx context.Context) error {
-	opts.AppendDeploymentType()
+	opts.DeploymentTelemetry.AppendDeploymentType()
+	opts.DeploymentTelemetry.AppendDeploymentUUID()
+
 	if !opts.EnableWatch {
 		return opts.Print(opts.indexID.Index)
 	}
diff --git a/internal/cli/deployments/search/indexes/create_test.go b/internal/cli/deployments/search/indexes/create_test.go
@@ -143,6 +143,18 @@ func TestCreate_RunLocal(t *testing.T) {
 		}, nil).
 		Times(1)
 
+	testDeployments.
+		MockDeploymentTelemetry.
+		EXPECT().
+		AppendDeploymentType().
+		Times(1)
+
+	testDeployments.
+		MockDeploymentTelemetry.
+		EXPECT().
+		AppendDeploymentUUID().
+		Times(1)
+
 	if err := opts.Run(ctx); err != nil {
 		t.Fatalf("Run() unexpected error: %v", err)
 	}
@@ -294,6 +306,18 @@ func TestCreate_RunAtlas(t *testing.T) {
 		Times(1).
 		Return(indexWithID, nil)
 
+	deploymentTest.
+		MockDeploymentTelemetry.
+		EXPECT().
+		AppendDeploymentType().
+		Times(1)
+
+	deploymentTest.
+		MockDeploymentTelemetry.
+		EXPECT().
+		AppendDeploymentUUID().
+		Times(1)
+
 	if err := opts.Run(ctx); err != nil {
 		t.Fatalf("Run() unexpected error: %v", err)
 	}
diff --git a/internal/cli/deployments/search/indexes/delete.go b/internal/cli/deployments/search/indexes/delete.go
@@ -108,6 +108,7 @@ func (opts *DeleteOpts) initMongoDBClient() error {
 
 func (opts *DeleteOpts) PostRun() {
 	opts.DeploymentTelemetry.AppendDeploymentType()
+	opts.DeploymentTelemetry.AppendDeploymentUUID()
 }
 
 func DeleteBuilder() *cobra.Command {
diff --git a/internal/cli/deployments/search/indexes/delete_test.go b/internal/cli/deployments/search/indexes/delete_test.go
@@ -179,5 +179,11 @@ func TestDeleteOpts_PostRun(t *testing.T) {
 		AppendDeploymentType().
 		Times(1)
 
+	deploymentTest.
+		MockDeploymentTelemetry.
+		EXPECT().
+		AppendDeploymentUUID().
+		Times(1)
+
 	opts.PostRun()
 }
diff --git a/internal/cli/deployments/search/indexes/describe.go b/internal/cli/deployments/search/indexes/describe.go
@@ -112,6 +112,7 @@ func (opts *DescribeOpts) initStore(ctx context.Context) func() error {
 
 func (opts *DescribeOpts) PostRun() {
 	opts.DeploymentTelemetry.AppendDeploymentType()
+	opts.DeploymentTelemetry.AppendDeploymentUUID()
 }
 
 func DescribeBuilder() *cobra.Command {
diff --git a/internal/cli/deployments/search/indexes/describe_test.go b/internal/cli/deployments/search/indexes/describe_test.go
@@ -194,5 +194,11 @@ func TestDescribeOpts_PostRun(t *testing.T) {
 		AppendDeploymentType().
 		Times(1)
 
+	deploymentTest.
+		MockDeploymentTelemetry.
+		EXPECT().
+		AppendDeploymentUUID().
+		Times(1)
+
 	opts.PostRun()
 }
diff --git a/internal/cli/deployments/search/indexes/list.go b/internal/cli/deployments/search/indexes/list.go
@@ -116,6 +116,7 @@ func (opts *ListOpts) validateAndPrompt() error {
 
 func (opts *ListOpts) PostRun() {
 	opts.DeploymentTelemetry.AppendDeploymentType()
+	opts.DeploymentTelemetry.AppendDeploymentUUID()
 }
 
 func ListBuilder() *cobra.Command {
diff --git a/internal/cli/deployments/search/indexes/list_test.go b/internal/cli/deployments/search/indexes/list_test.go
@@ -227,5 +227,11 @@ func TestList_PostRun(t *testing.T) {
 		AppendDeploymentType().
 		Times(1)
 
+	deploymentTest.
+		MockDeploymentTelemetry.
+		EXPECT().
+		AppendDeploymentUUID().
+		Times(1)
+
 	opts.PostRun()
 }
diff --git a/internal/cli/deployments/setup.go b/internal/cli/deployments/setup.go
@@ -667,7 +667,6 @@ func (opts *SetupOpts) Run(ctx context.Context) error {
 		return err
 	}
 
-	opts.AppendDeploymentType()
 	if strings.EqualFold(options.LocalCluster, opts.DeploymentType) {
 		return opts.runLocal(ctx)
 	}
@@ -677,6 +676,7 @@ func (opts *SetupOpts) Run(ctx context.Context) error {
 
 func (opts *SetupOpts) PostRun() {
 	opts.DeploymentTelemetry.AppendDeploymentType()
+	opts.DeploymentTelemetry.AppendDeploymentUUID()
 }
 
 func (opts *SetupOpts) validateTier() error {
diff --git a/internal/cli/deployments/setup_test.go b/internal/cli/deployments/setup_test.go
@@ -44,6 +44,10 @@ func TestSetupOpts_PostRun(t *testing.T) {
 		EXPECT().
 		AppendDeploymentType().
 		Times(1)
+	deploymentTest.MockDeploymentTelemetry.
+		EXPECT().
+		AppendDeploymentUUID().
+		Times(1)
 
 	opts.PostRun()
 }
diff --git a/internal/cli/deployments/start.go b/internal/cli/deployments/start.go
@@ -87,6 +87,7 @@ func (opts *StartOpts) RunAtlas() error {
 
 func (opts *StartOpts) PostRun() error {
 	opts.DeploymentTelemetry.AppendDeploymentType()
+	opts.DeploymentTelemetry.AppendDeploymentUUID()
 	return opts.PostRunMessages()
 }
 
diff --git a/internal/cli/deployments/start_test.go b/internal/cli/deployments/start_test.go
@@ -151,6 +151,12 @@ func TestStartOpts_PostRun(t *testing.T) {
 		AppendDeploymentType().
 		Times(1)
 
+	deploymentsTest.
+		MockDeploymentTelemetry.
+		EXPECT().
+		AppendDeploymentUUID().
+		Times(1)
+
 	deploymentsTest.MockContainerEngine.EXPECT().Ready().Return(nil).Times(1)
 
 	require.NoError(t, opts.PostRun())
diff --git a/internal/mocks/mock_deployment_opts_telemetry.go b/internal/mocks/mock_deployment_opts_telemetry.go

Original file line number	Diff line number	Diff line change
`@@ -50,6 +50,7 @@ func Run(ctx context.Context, opts *ConnectOpts) error {`
`50`	`50`
`51`	`51`	`func PostRun(opts *ConnectOpts) {`
`52`	`52`	`opts.DeploymentTelemetry.AppendDeploymentType()`
	`53`	`+ opts.DeploymentTelemetry.AppendDeploymentUUID()`
`53`	`54`	`}`
`54`	`55`
`55`	`56`	`// atlas deployments connect [clusterName].`
Original file line number	Diff line number	Diff line change
`@@ -229,6 +229,7 @@ func (opts *DownloadOpts) validateAtlasFlags() error {`
`229`	`229`
`230`	`230`	`func (opts *DownloadOpts) PostRun() {`
`231`	`231`	`opts.DeploymentTelemetry.AppendDeploymentType()`
	`232`	`+ opts.DeploymentTelemetry.AppendDeploymentUUID()`
`232`	`233`	`}`
`233`	`234`
`234`	`235`	`// atlas deployments logs.`
Original file line number	Diff line number	Diff line change
`@@ -70,6 +70,9 @@ func (opts *PauseOpts) Run(ctx context.Context) error {`
`70`	`70`	`}`
`71`	`71`
`72`	`72`	`func (opts *PauseOpts) RunLocal(ctx context.Context, deployment options.Deployment) error {`
	`73`	`+ // Collect UUID before pausing`
	`74`	`+ opts.DeploymentTelemetry.AppendDeploymentUUID()`
	`75`	`+`
`73`	`76`	`if err := opts.stopContainer(ctx, deployment); err != nil {`
`74`	`77`	`return err`
`75`	`78`	`}`
Original file line number	Diff line number	Diff line change
`@@ -304,7 +304,9 @@ func (opts *CreateOpts) watchAtlas(_ context.Context) (any, bool, error) {`
`304`	`304`	`}`
`305`	`305`
`306`	`306`	`func (opts *CreateOpts) PostRun(ctx context.Context) error {`
`307`		`- opts.AppendDeploymentType()`
	`307`	`+ opts.DeploymentTelemetry.AppendDeploymentType()`
	`308`	`+ opts.DeploymentTelemetry.AppendDeploymentUUID()`
	`309`	`+`
`308`	`310`	`if !opts.EnableWatch {`
`309`	`311`	`return opts.Print(opts.indexID.Index)`
`310`	`312`	`}`