feat: track cli command run events #189

sunnyguduru · 2024-04-18T21:30:36Z

Here's what the properties look like:

cmd/environments/get.go

cmd/utils/utils.go

dbolson

Can you add tests?

cmd/utils/utils.go

sunnyguduru · 2024-04-19T23:49:52Z

cmd/environments/get_test.go

+	t.Run("will track analytics for 'CLI Command Run' event", func(t *testing.T) {
+		id := "test-id"
+		mockedTrackingArgs := []interface{}{
+			"testAccessToken",
+			"http://test.com",
+			"CLI Command Run",
+			map[string]interface{}{
+				"name":    "environments",
+				"action":  "get",
+				"baseURI": "http://test.com",
+				"id":      id,
+				"flags":   []string{"access-token", "base-uri", "environment", "project"},
+			},
+		}
+
+		client := environments.MockClient{}
+		client.
+			On("Get", mockArgs...).
+			Return([]byte(cmd.ValidResponse), nil)
+		clients := cmd.APIClients{
+			EnvironmentsClient: &client,
+		}
+		tracker := analytics.MockTracker{ID: id}
+		tracker.On("SendEvent", mockedTrackingArgs...)
+
+		args := []string{
+			"environments", "get",
+			"--access-token", "testAccessToken",
+			"--base-uri", "http://test.com",
+			"--environment", "test-env",
+			"--project", "test-proj",
+		}
+
+		_, err := cmd.CallCmd(t, clients, &tracker, args)
+		tracker.AssertCalled(t, "SendEvent", mockedTrackingArgs...)
+		require.NoError(t, err)
+	})


Added tests for commands

dbolson · 2024-04-22T15:30:05Z

internal/analytics/event_utils.go

+
+func CmdRunEventProperties(cmd *cobra.Command, name string) map[string]interface{} {
+	id := uuid.New()
+	baseURI := viper.GetString(cliflags.BaseURIFlag)


Can you pass this value in instead of using viper? It removes the dependency.

dbolson · 2024-04-22T15:32:47Z

cmd/projects/list_test.go


 		assert.EqualError(t, err, "base-uri is invalid"+errorHelp)
 	})
+
+	t.Run("will track analytics for 'CLI Command Run' event", func(t *testing.T) {


Can you remove the quotes for these tests? It messes up the test name when running with -v.

dbolson · 2024-04-22T15:36:47Z

cmd/environments/environments.go

 	cmd.AddCommand(getCmd)

+	cmd.PersistentPreRun = func(cmd *cobra.Command, args []string) {


Can you set this when we create cmd above? Otherwise it seems like it's doing something special that wouldn't work if it were set at the same time as the other fields.

Could you update this for all the commands?

dbolson · 2024-04-22T15:37:44Z

internal/analytics/event_utils.go

@@ -0,0 +1,30 @@
+package analytics


Can you rename this file to be events.go since "utils" doesn't add any meaning?

dbolson · 2024-04-22T15:49:12Z

cmd/cliflags/baseuri.go

@@ -0,0 +1,3 @@
+package cliflags
+
+const BaseURIDefault = "https://app.launchdarkly.com"


I don't think this needs to be its own file.

dbolson · 2024-04-22T15:51:10Z

cmd/environments/get_test.go

+
+	t.Run("will track analytics for 'CLI Command Run' event", func(t *testing.T) {
+		id := "test-id"
+		mockedTrackingArgs := []interface{}{


You could remove some duplication by extracting this to a helper function.

// analytics/client.go func MockedTracker(name string, action string, flags []string) *MockTracker { id := "test-id" mockedTrackingArgs := []interface{}{ "testAccessToken", "http://test.com", "CLI Command Run", map[string]interface{}{ "action": action, "baseURI": "http://test.com", "flags": flags, "id": id, "name": name, }, } tracker := MockTracker{ID: id} tracker.On("SendEvent", mockedTrackingArgs...) return &tracker }

and call it in the tests like

tracker := analytics.MockedTracker( "flags", "get", []string{"access-token", "base-uri", "environment", "flag", "project"}, // these need to be alphabetized )

dbolson · 2024-04-22T18:06:48Z

cmd/cliflags/flags.go

@@ -10,3 +11,5 @@ const (
 	ProjectFlag     = "project"
 	RoleFlag        = "role"
 )
+
+const BaseURIDefault = "https://app.launchdarkly.com"


Can you define these with the other constants? It could be after a linebreak but in the same const() declaration.

dbolson · 2024-04-22T18:07:25Z

cmd/environments/environments.go

 	cmd.AddCommand(getCmd)

+	cmd.PersistentPreRun = func(cmd *cobra.Command, args []string) {


Could you update this for all the commands?

dbolson · 2024-04-22T18:09:12Z

cmd/flags/update_test.go

@@ -285,7 +280,7 @@ func TestToggle(t *testing.T) {
 			"--environment", "test-env-key",
 		}

-		_, err := cmd.CallCmd(t, clients, &tracker, args)
+		_, err := cmd.CallCmd(t, clients, tracker, args)
 		tracker.AssertCalled(t, "SendEvent", mockedTrackingArgs...)


You can get rid of this assertion since the call in MockedTracker will fail if it doesn't get the right input. You can test that out by changing the input in the production code to see the test fail without this one. Then you wouldn't need to return the mocked args.

Ok, that makes sense. I'll also get rid of mocked args

dbolson · 2024-04-22T22:29:19Z

cmd/config/config.go

 	cmd := &cobra.Command{
 		Long:  "View and modify specific configuration values",
 		RunE:  run(),
 		Short: "View and modify specific configuration values",
 		Use:   "config",
+		PreRun: func(cmd *cobra.Command, args []string) {


Why is this PreRun while the others are PersistentPreRun?

I considered leaving a comment. PersistentPreRun from my testing only ran on child cmds attached to the parent cmd. The parent cmd is where PersistentPreRun is defined.

On the other hand PreRun will run on the parent command only. In this case config doesn't have any sub commands similar to the others (flags, environments). Hence why its a PreRun and not PersistentPreRun

Sunny Guduru added 3 commits April 18, 2024 14:24

added uuid

526b618

added track environments get

b137312

add tracking and properties for environment get

b4fe3e4

sunnyguduru commented Apr 18, 2024

View reviewed changes

cmd/environments/get.go Outdated Show resolved Hide resolved

Sunny Guduru added 4 commits April 18, 2024 14:36

put helper func in own package

64397b6

added tracking for flags commands

90e7d4d

added tracking for members commands

55504e0

added tracking to projects commands

17aa87c

sunnyguduru force-pushed the sunny/sc-240591/track-cli-command-run-events branch from cb73d5c to 17aa87c Compare April 18, 2024 21:51

use preRun

e930ac7

sunnyguduru marked this pull request as ready for review April 18, 2024 23:56

sunnyguduru requested review from dbolson and k3llymariee April 18, 2024 23:56

sunnyguduru commented Apr 18, 2024

View reviewed changes

cmd/utils/utils.go Outdated Show resolved Hide resolved

dbolson requested changes Apr 19, 2024

View reviewed changes

cmd/utils/utils.go Outdated Show resolved Hide resolved

cmd/utils/utils.go Outdated Show resolved Hide resolved

cmd/utils/utils.go Outdated Show resolved Hide resolved

cmd/utils/utils.go Outdated Show resolved Hide resolved

cmd/utils/utils.go Outdated Show resolved Hide resolved

Sunny Guduru added 7 commits April 19, 2024 12:22

added baseurl constant, changed some code to one liner

d9f7714

move env get tracking to env file

c023912

converted flags to use persistent prerun

36a347e

converted members

32e5ed3

converted projects

8517471

added tracking to config

9e1da14

remove repeated code

859a723

sunnyguduru force-pushed the sunny/sc-240591/track-cli-command-run-events branch from a07077a to 859a723 Compare April 19, 2024 21:41

Sunny Guduru added 4 commits April 19, 2024 15:13

added mock tracker. Use mock tracker

4de3013

added envs tracking test

85937c8

did flags and members

a742193

added tests for projects

05e9e6f

sunnyguduru commented Apr 19, 2024

View reviewed changes

sunnyguduru requested a review from dbolson April 19, 2024 23:52

dbolson requested changes Apr 22, 2024

View reviewed changes

Sunny Guduru added 9 commits April 22, 2024 09:51

move baseURI const

b934839

rename event_utils to events

658d0cd

updated get test

07abc2c

update flags

138d46a

updated members

b2abe94

updated projects

a79ee39

update members again

4830091

update flags again

099c2c4

remove quotes:

fd0b12c

sunnyguduru requested a review from dbolson April 22, 2024 17:49

dbolson requested changes Apr 22, 2024

View reviewed changes

Sunny Guduru added 2 commits April 22, 2024 13:08

moved base url const

365c546

remove assert and extra return

7323eba

sunnyguduru requested a review from dbolson April 22, 2024 20:14

Sunny Guduru added 2 commits April 22, 2024 13:21

updated test

a6ce217

moved pre runs to definition:

d60d1a3

dbolson approved these changes Apr 22, 2024

View reviewed changes

sunnyguduru merged commit fd98b42 into main Apr 22, 2024
2 checks passed

sunnyguduru deleted the sunny/sc-240591/track-cli-command-run-events branch April 22, 2024 22:47

github-actions bot mentioned this pull request Apr 22, 2024

chore(main): release 0.9.0 #198

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: track cli command run events #189

feat: track cli command run events #189

sunnyguduru commented Apr 18, 2024 •

edited

Loading

dbolson left a comment

sunnyguduru Apr 19, 2024 •

edited

Loading

dbolson Apr 22, 2024

dbolson Apr 22, 2024

dbolson Apr 22, 2024

dbolson Apr 22, 2024

dbolson Apr 22, 2024

dbolson Apr 22, 2024

dbolson Apr 22, 2024

dbolson Apr 22, 2024

dbolson Apr 22, 2024

dbolson Apr 22, 2024

sunnyguduru Apr 22, 2024

dbolson Apr 22, 2024

sunnyguduru Apr 22, 2024

		cmd.AddCommand(getCmd)

		cmd.PersistentPreRun = func(cmd *cobra.Command, args []string) {

		@@ -0,0 +1,3 @@
		package cliflags

		const BaseURIDefault = "https://app.launchdarkly.com"

feat: track cli command run events #189

feat: track cli command run events #189

Conversation

sunnyguduru commented Apr 18, 2024 • edited Loading

dbolson left a comment

Choose a reason for hiding this comment

sunnyguduru Apr 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunnyguduru commented Apr 18, 2024 •

edited

Loading

sunnyguduru Apr 19, 2024 •

edited

Loading