Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLOUDP-237254: Set deploymet_type telemetry value in missing atlas deployments commands #2790

Merged
merged 9 commits into from Mar 22, 2024
1 change: 1 addition & 0 deletions internal/cli/atlas/deployments/delete.go
Expand Up @@ -86,6 +86,7 @@ func (opts *DeleteOpts) runLocal(ctx context.Context) error {
}

func (opts *DeleteOpts) PostRun() error {
opts.UpdateDeploymentTelemetry()
if !opts.EnableWatch || !opts.IsAtlasDeploymentType() {
return nil
}
Expand Down
23 changes: 23 additions & 0 deletions internal/cli/atlas/deployments/delete_test.go
Expand Up @@ -159,3 +159,26 @@ func TestDeleteBuilder(t *testing.T) {
[]string{flag.TypeFlag, flag.Force, flag.EnableWatch, flag.WatchTimeout, flag.ProjectID},
)
}

func TestDelete_PostRun(t *testing.T) {
ctrl := gomock.NewController(t)
deploymentsTest := fixture.NewMockLocalDeploymentOpts(ctrl, "localDeployment")
buf := new(bytes.Buffer)

opts := &DeleteOpts{
DeploymentOpts: *deploymentsTest.Opts,
GlobalOpts: cli.GlobalOpts{
ProjectID: "64f670f0bf789926667dad1a",
},
OutputOpts: cli.OutputOpts{
OutWriter: buf,
},
DeleteOpts: cli.NewDeleteOpts(deleteSuccessMessage, deleteFailMessage),
}

deploymentsTest.MockDeploymentTelemetry.EXPECT().AppendDeploymentType().Times(1)

if err := opts.PostRun(); err != nil {
t.Fatalf("PostRun() unexpected error: %v", err)
}
}
4 changes: 3 additions & 1 deletion internal/cli/atlas/deployments/list.go
Expand Up @@ -71,6 +71,7 @@ func (opts *ListOpts) Run(ctx context.Context) error {
}

func (opts *ListOpts) PostRun() error {
opts.UpdateDeploymentTelemetry()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the expectation for list here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it will only set it if customer provides --type local/atlas, otherwise it will not set the type

return opts.PostRunMessages()
}

Expand All @@ -89,7 +90,8 @@ func ListBuilder() *cobra.Command {
PreRunE: func(cmd *cobra.Command, _ []string) error {
return opts.PreRunE(
opts.InitStore(cmd.Context(), cmd.OutOrStdout()),
opts.InitOutput(cmd.OutOrStdout(), listTemplate))
opts.InitOutput(cmd.OutOrStdout(), listTemplate),
)
},
RunE: func(cmd *cobra.Command, _ []string) error {
return opts.Run(cmd.Context())
Expand Down
41 changes: 41 additions & 0 deletions internal/cli/atlas/deployments/list_test.go
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/golang/mock/gomock"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/cli"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/cli/atlas/deployments/options"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/cli/atlas/deployments/test/fixture"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/config"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/flag"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/mocks"
Expand Down Expand Up @@ -147,3 +148,43 @@ func TestListBuilder(t *testing.T) {
[]string{flag.ProjectID},
)
}

func TestList_PostRun(t *testing.T) {
ctrl := gomock.NewController(t)
buf := new(bytes.Buffer)

mockStore := mocks.NewMockClusterLister(ctrl)
mockCredentialsGetter := mocks.NewMockCredentialsGetter(ctrl)
mockProfileReader := mocks.NewMockProfileReader(ctrl)

deploymentsTest := fixture.NewMockLocalDeploymentOpts(ctrl, "localDeployment")
deploymentsTest.Opts.Config = mockProfileReader
deploymentsTest.Opts.CredStore = mockCredentialsGetter
deploymentsTest.Opts.AtlasClusterListStore = mockStore

listOpts := &ListOpts{
DeploymentOpts: *deploymentsTest.Opts,
GlobalOpts: cli.GlobalOpts{
ProjectID: "64f670f0bf789926667dad1a",
},
OutputOpts: cli.OutputOpts{
Template: listTemplate,
OutWriter: buf,
},
}

mockCredentialsGetter.
EXPECT().
AuthType().
Return(config.OAuth).
Times(1)

deploymentsTest.MockDeploymentTelemetry.
EXPECT().
AppendDeploymentType().
Times(1)

if err := listOpts.PostRun(); err != nil {
t.Fatalf("PostRun() unexpected error: %v", err)
}
}
Expand Up @@ -15,13 +15,9 @@ package options

import (
"context"

"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/telemetry"
)

func (opts *ConnectOpts) connectToLocal(ctx context.Context) error {
telemetry.AppendOption(telemetry.WithDeploymentType(LocalCluster))

connectionString, err := opts.ConnectionString(ctx)
if err != nil {
return err
Expand Down
7 changes: 7 additions & 0 deletions internal/cli/atlas/deployments/options/deployment_opts.go
Expand Up @@ -96,6 +96,7 @@ type DeploymentOpts struct {
DefaultSetter cli.DefaultSetterOpts
AtlasClusterListStore store.ClusterLister
Config setup.ProfileReader
DeploymentTelemetry DeploymentTelemetry
}

type Deployment struct {
Expand All @@ -115,6 +116,8 @@ func (opts *DeploymentOpts) InitStore(ctx context.Context, writer io.Writer) fun
return err
}
opts.DefaultSetter.OutWriter = writer
opts.DeploymentTelemetry = NewDeploymentTypeTelemetry(opts)
opts.UpdateDeploymentTelemetry()
return opts.DefaultSetter.InitStore(ctx)
}
}
Expand Down Expand Up @@ -256,3 +259,7 @@ func (opts *DeploymentOpts) NoDeploymentTypeSet() bool {
func (opts *DeploymentOpts) IsAuthEnabled() bool {
return opts.DBUsername != ""
}

func (opts *DeploymentOpts) UpdateDeploymentTelemetry() {
opts.DeploymentTelemetry.AppendDeploymentType()
}
Expand Up @@ -102,9 +102,9 @@ func (opts *DeploymentOpts) Select(deployments []Deployment) (Deployment, error)

if len(deployments) == 1 {
opts.DeploymentName = deployments[0].Name
opts.DeploymentType = deployments[0].Type
opts.DeploymentType = strings.ToLower(deployments[0].Type)

telemetry.AppendOption(telemetry.WithDeploymentType(opts.DeploymentType))
opts.AppendDeploymentType()
return deployments[0], nil
}

Expand All @@ -130,7 +130,7 @@ func (opts *DeploymentOpts) Select(deployments []Deployment) (Deployment, error)

deployment := deploymentsByDisplayName[displayName]
opts.DeploymentName = deployment.Name
opts.DeploymentType = deployment.Type
telemetry.AppendOption(telemetry.WithDeploymentType(opts.DeploymentType))
opts.DeploymentType = strings.ToLower(deployment.Type)
opts.AppendDeploymentType()
return deployment, nil
}
@@ -0,0 +1,40 @@
// Copyright 2023 MongoDB Inc
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package options

import (
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/telemetry"
)

//go:generate mockgen -destination=../../../../mocks/mock_deployment_opts_telemetry.go -package=mocks github.com/mongodb/mongodb-atlas-cli/atlascli/internal/cli/atlas/deployments/options DeploymentTelemetry
type DeploymentTelemetry interface {
AppendDeploymentType()
}

func NewDeploymentTypeTelemetry(opts *DeploymentOpts) DeploymentTelemetry {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] in go the recommendation is to accept interfaces and return structs, not the other way around

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can do that it may look a bit weird for DeploymentOpts though as there we need to use DeploymentType we'd need to cast with something like this (which works fine):

func NewDeploymentTypeTelemetry(opts DeploymentTelemetry) *DeploymentOpts {
	return opts.(*DeploymentOpts)
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a bit weird that you need to cast it to an interface if it accepts an interface but let's leave this for now

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the problem may be this is not really a "new" which should return a new object but this is more of a "cast as interface" thing

return opts
}

func (opts *DeploymentOpts) AppendDeploymentType() {
var deploymentType string
if opts.IsLocalDeploymentType() {
deploymentType = LocalCluster
} else if opts.IsAtlasDeploymentType() {
deploymentType = AtlasCluster
}
if deploymentType != "" {
telemetry.AppendOption(telemetry.WithDeploymentType(deploymentType))
}
}
2 changes: 1 addition & 1 deletion internal/cli/atlas/deployments/setup.go
Expand Up @@ -815,7 +815,7 @@ func (opts *SetupOpts) Run(ctx context.Context) error {
return err
}

telemetry.AppendOption(telemetry.WithDeploymentType(opts.DeploymentType))
opts.AppendDeploymentType()
if strings.EqualFold(options.LocalCluster, opts.DeploymentType) {
return opts.runLocal(ctx)
}
Expand Down
Expand Up @@ -24,5 +24,6 @@ type MockDeploymentOpts struct {
MockCredentialsGetter *mocks.MockCredentialsGetter
MockAtlasClusterListStore *mocks.MockClusterLister
MockPodman *mocks.MockClient
MockDeploymentTelemetry *mocks.MockDeploymentTelemetry
Opts *options.DeploymentOpts
}
16 changes: 10 additions & 6 deletions internal/cli/atlas/deployments/test/fixture/deployment_local.go
Expand Up @@ -24,15 +24,19 @@ import (

func NewMockLocalDeploymentOpts(ctrl *gomock.Controller, deploymentName string) MockDeploymentOpts {
mockPodman := mocks.NewMockClient(ctrl)
return MockDeploymentOpts{
ctrl: ctrl,
MockPodman: mockPodman,
mockDeploymentTelemetry := mocks.NewMockDeploymentTelemetry(ctrl)
mockOpts := MockDeploymentOpts{
ctrl: ctrl,
MockPodman: mockPodman,
MockDeploymentTelemetry: mockDeploymentTelemetry,
Opts: &options.DeploymentOpts{
PodmanClient: mockPodman,
DeploymentName: deploymentName,
DeploymentType: "local",
PodmanClient: mockPodman,
DeploymentName: deploymentName,
DeploymentType: "local",
DeploymentTelemetry: mockDeploymentTelemetry,
},
}
return mockOpts
}

func (m *MockDeploymentOpts) LocalMockFlowWithMockContainer(ctx context.Context, mockContainer []*podman.Container) {
Expand Down
46 changes: 46 additions & 0 deletions internal/mocks/mock_deployment_opts_telemetry.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.