[WEBXP-315] refactor and expand telemetry for the CLI (#1183)

Author: jackieg017

.github/CODEOWNERS

@@ -1,5 +1,5 @@
-* @CircleCI-Public/lifecycle
-*orb*.go @CircleCI-Public/orb-publishers @CircleCI-Public/lifecycle
+* @circleci/webex
+*orb*.go @circleci/webex
 
 /api/runner @CircleCI-Public/on-prem
 /cmd/runner @CircleCI-Public/on-prem

api/api.go

@@ -17,6 +17,7 @@ import (
 
 	"github.com/CircleCI-Public/circleci-cli/api/graphql"
 	"github.com/CircleCI-Public/circleci-cli/api/rest"
+	"github.com/CircleCI-Public/circleci-cli/errs"
 	"github.com/CircleCI-Public/circleci-cli/references"
 	"github.com/CircleCI-Public/circleci-cli/settings"
 )
@@ -54,6 +55,10 @@ func (errs GQLErrorsCollection) Error() string {
 	return strings.Join(messages, "\n")
 }
 
+func wrapGQLErrors(gqlErrors GQLErrorsCollection) error {
+	return errs.CheckAuthRequired(gqlErrors)
+}
+
 // GQLResponseError is a mapping of the data returned by the GraphQL server of key-value pairs.
 // Typically used with the structure "Message: string", but other response errors provide additional fields.
 type GQLResponseError struct {
@@ -543,7 +548,7 @@ func OrbImportVersion(cl *graphql.Client, orbSrc string, orbID string, orbVersio
 	}
 
 	if len(response.ImportOrbVersion.Errors) > 0 {
-		return nil, response.ImportOrbVersion.Errors
+		return nil, wrapGQLErrors(response.ImportOrbVersion.Errors)
 	}
 
 	return &response.ImportOrbVersion.Orb, nil
@@ -590,7 +595,7 @@ func OrbPublishByName(cl *graphql.Client, configPath, orbName, namespaceName, or
 	}
 
 	if len(response.PublishOrb.Errors) > 0 {
-		return nil, response.PublishOrb.Errors
+		return nil, wrapGQLErrors(response.PublishOrb.Errors)
 	}
 
 	return &response.PublishOrb.Orb, nil
@@ -663,7 +668,7 @@ func OrbID(cl *graphql.Client, namespace string, orb string) (*OrbIDResponse, er
 		return nil, namespaceNotFound(namespace)
 	}
 
-	return nil, fmt.Errorf("the '%s' orb does not exist in the '%s' namespace. Did you misspell the namespace or the orb name?", orb, namespace)
+	return nil, errs.NotFoundf("the '%s' orb does not exist in the '%s' namespace. Did you misspell the namespace or the orb name?", orb, namespace)
 }
 
 // CreateImportedNamespace creates an imported namespace with the provided name. An imported namespace
@@ -697,7 +702,7 @@ func CreateImportedNamespace(cl *graphql.Client, name string) (*ImportNamespaceR
 	}
 
 	if len(response.ImportNamespace.Errors) > 0 {
-		return nil, response.ImportNamespace.Errors
+		return nil, wrapGQLErrors(response.ImportNamespace.Errors)
 	}
 
 	return &response, nil
@@ -731,7 +736,7 @@ func CreateNamespaceWithOwnerID(cl *graphql.Client, name string, ownerID string)
 	err := cl.Run(request, &response)
 
 	if len(response.CreateNamespace.Errors) > 0 {
-		return nil, response.CreateNamespace.Errors
+		return nil, wrapGQLErrors(response.CreateNamespace.Errors)
 	}
 
 	if err != nil {
@@ -828,7 +833,7 @@ mutation($name: String!) {
 	}
 
 	if len(response.DeleteNamespaceAlias.Errors) > 0 {
-		return response.DeleteNamespaceAlias.Errors
+		return wrapGQLErrors(response.DeleteNamespaceAlias.Errors)
 	}
 
 	if !response.DeleteNamespaceAlias.Deleted {
@@ -866,7 +871,7 @@ mutation($id: UUID!) {
 	}
 
 	if len(response.DeleteNamespace.Errors) > 0 {
-		return response.DeleteNamespace.Errors
+		return wrapGQLErrors(response.DeleteNamespace.Errors)
 	}
 
 	if !response.DeleteNamespace.Deleted {
@@ -977,7 +982,7 @@ func renameNamespaceWithNsID(cl *graphql.Client, id, newName string) (*RenameNam
 	err := cl.Run(request, &response)
 
 	if len(response.RenameNamespace.Errors) > 0 {
-		return nil, response.RenameNamespace.Errors
+		return nil, wrapGQLErrors(response.RenameNamespace.Errors)
 	}
 
 	if err != nil {
@@ -1024,7 +1029,7 @@ func createOrbWithNsID(cl *graphql.Client, name string, namespaceID string, isPr
 	err := cl.Run(request, &response)
 
 	if len(response.CreateOrb.Errors) > 0 {
-		return nil, response.CreateOrb.Errors
+		return nil, wrapGQLErrors(response.CreateOrb.Errors)
 	}
 
 	if err != nil {
@@ -1080,7 +1085,7 @@ func CreateImportedOrb(cl *graphql.Client, namespace string, name string) (*Impo
 	}
 
 	if len(response.ImportOrb.Errors) > 0 {
-		return nil, response.ImportOrb.Errors
+		return nil, wrapGQLErrors(response.ImportOrb.Errors)
 	}
 
 	return &response, nil
@@ -1201,7 +1206,7 @@ func OrbPromoteByName(cl *graphql.Client, namespaceName, orbName, label, segment
 	err = cl.Run(request, &response)
 
 	if len(response.PromoteOrb.Errors) > 0 {
-		return nil, response.PromoteOrb.Errors
+		return nil, wrapGQLErrors(response.PromoteOrb.Errors)
 	}
 
 	if err != nil {
@@ -1244,7 +1249,7 @@ mutation($orbId: UUID!, $list: Boolean!) {
 	err = cl.Run(request, &response)
 
 	if len(response.SetOrbListStatus.Errors) > 0 {
-		return nil, response.SetOrbListStatus.Errors
+		return nil, wrapGQLErrors(response.SetOrbListStatus.Errors)
 	}
 
 	if err != nil {
@@ -1298,7 +1303,7 @@ func OrbSource(cl *graphql.Client, orbRef string) (string, error) {
 	}
 
 	if response.OrbVersion.ID == "" {
-		return "", fmt.Errorf("no Orb '%s' was found; please check that the Orb reference is correct", orbRef)
+		return "", &ErrOrbVersionNotExists{OrbRef: orbRef}
 	}
 
 	return response.OrbVersion.Source, nil
@@ -1315,6 +1320,8 @@ func (e *ErrOrbVersionNotExists) Error() string {
 	return fmt.Sprintf("no Orb '%s' was found; please check that the Orb reference is correct", e.OrbRef)
 }
 
+func (e *ErrOrbVersionNotExists) Is(target error) bool { return target == errs.ErrNotFound }
+
 // OrbInfo gets the meta-data of an orb
 func OrbInfo(cl *graphql.Client, orbRef string) (*OrbVersion, error) {
 	if err := references.IsOrbRefWithOptionalVersion(orbRef); err != nil {
@@ -1714,7 +1721,7 @@ func AddOrRemoveOrbCategorization(cl *graphql.Client, namespace string, orb stri
 	responseData := response[mutationName]
 
 	if len(responseData.Errors) > 0 {
-		return &responseData.Errors
+		return wrapGQLErrors(responseData.Errors)
 	}
 
 	if err != nil {

api/context/gql_client.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/CircleCI-Public/circleci-cli/api"
 	"github.com/CircleCI-Public/circleci-cli/api/graphql"
+	"github.com/CircleCI-Public/circleci-cli/errs"
 )
 
 type gqlClient struct {
@@ -53,7 +54,7 @@ func (c *gqlClient) ContextByName(name string) (Context, error) {
 			return context, nil
 		}
 	}
-	return Context{}, errors.New("No context found with that name")
+	return Context{}, errs.NotFoundf("No context found with that name")
 }
 
 func (c *gqlClient) CreateContext(name string) error {

api/context/rest_client.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	"github.com/CircleCI-Public/circleci-cli/api/rest"
+	"github.com/CircleCI-Public/circleci-cli/errs"
 )
 
 type restClient struct {
@@ -64,7 +65,7 @@ func (c restClient) ContextByName(name string) (Context, error) {
 
 		params.PageToken = resp.NextPageToken
 	}
-	return Context{}, fmt.Errorf("context with name %s not found", name)
+	return Context{}, errs.NotFoundf("context with name %s not found", name)
 }
 
 func (c restClient) CreateContext(name string) error {

api/graphql/client.go

@@ -13,6 +13,7 @@ import (
 	"strings"
 
 	"github.com/CircleCI-Public/circleci-cli/api/header"
+	"github.com/CircleCI-Public/circleci-cli/errs"
 	"github.com/CircleCI-Public/circleci-cli/version"
 	"github.com/pkg/errors"
 )
@@ -275,7 +276,7 @@ func (cl *Client) Run(request *Request, resp interface{}) error {
 	}
 
 	if len(wrappedResponse.Errors) > 0 {
-		return wrappedResponse.Errors
+		return errs.CheckAuthRequired(wrappedResponse.Errors)
 	}
 
 	return nil

api/runner/runner.go

@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"github.com/CircleCI-Public/circleci-cli/api/rest"
+	"github.com/CircleCI-Public/circleci-cli/errs"
 )
 
 type Runner struct {
@@ -56,7 +57,7 @@ func (r *Runner) GetResourceClassByName(resourceClass string) (rc *ResourceClass
 		}
 	}
 
-	return nil, fmt.Errorf("resource class %q not found", resourceClass)
+	return nil, errs.NotFoundf("resource class %q not found", resourceClass)
 }
 
 func (r *Runner) GetNamespaceByResourceClass(resourceClass string) (ns string, err error) {

clitest/telemetry.go

@@ -8,6 +8,8 @@ import (
 	"github.com/onsi/gomega"
 )
 
+// CompareTelemetryEvent asserts that the recorded telemetry events exactly
+// match `expected`. Use this when you need strict ordering and field equality.
 func CompareTelemetryEvent(settings *TempSettings, expected []telemetry.Event) {
 	content, err := os.ReadFile(settings.TelemetryDestPath)
 	gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
@@ -17,3 +19,28 @@ func CompareTelemetryEvent(settings *TempSettings, expected []telemetry.Event) {
 	gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
 	gomega.Expect(result).To(gomega.Equal(expected))
 }
+
+// ReadTelemetryEvents reads and parses all telemetry events from the mock file.
+func ReadTelemetryEvents(settings *TempSettings) []telemetry.Event {
+	content, err := os.ReadFile(settings.TelemetryDestPath)
+	gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
+
+	var result []telemetry.Event
+	err = json.Unmarshal(content, &result)
+	gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
+	return result
+}
+
+// CompareTelemetryEventSubset asserts that every event in `expected` appears
+// somewhere in the recorded events. Use this when the middleware also emits
+// cli_command_started / cli_command_finished events alongside legacy events
+// and you only want to verify the legacy event payload.
+func CompareTelemetryEventSubset(settings *TempSettings, expected []telemetry.Event) {
+	content, err := os.ReadFile(settings.TelemetryDestPath)
+	gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
+
+	result := []telemetry.Event{}
+	err = json.Unmarshal(content, &result)
+	gomega.Expect(err).ShouldNot(gomega.HaveOccurred())
+	gomega.Expect(result).To(gomega.ContainElements(expected))
+}

cmd/config_test.go

@@ -50,10 +50,10 @@ var _ = Describe("Config", func() {
 				Expect(err).ShouldNot(HaveOccurred())
 
 				Eventually(session).Should(gexec.Exit(0))
-				clitest.CompareTelemetryEvent(tempSettings, []telemetry.Event{
+				clitest.CompareTelemetryEventSubset(tempSettings, []telemetry.Event{
 					telemetry.CreateConfigEvent(telemetry.CommandInfo{
 						Name:      "pack",
-						LocalArgs: map[string]string{"help": "false"},
+						LocalArgs: map[string]string{},
 					}, nil),
 				})
 			})

cmd/create_telemetry.go

@@ -13,7 +13,6 @@ import (
 	"github.com/CircleCI-Public/circleci-cli/version"
 	"github.com/google/uuid"
 	"github.com/spf13/cobra"
-	"github.com/spf13/pflag"
 	"golang.org/x/term"
 )
 
@@ -180,27 +179,9 @@ func loadTelemetrySettings(settings *settings.TelemetrySettings, user *telemetry
 	}
 }
 
-// Utility function used when creating telemetry events.
-// It takes a cobra Command and creates a telemetry.CommandInfo of it
-// If getParent is true, puts both the command's args in `LocalArgs` and the parent's args
-// Else only put the command's args
-// Note: child flags overwrite parent flags with same name
+// GetCommandInformation takes a cobra Command and creates a telemetry.CommandInfo.
+// If getParent is true, it also includes explicitly-set flags from the parent command.
+// Only flags explicitly set by the user are included; sensitive flags are redacted.
 func GetCommandInformation(cmd *cobra.Command, getParent bool) telemetry.CommandInfo {
-	localArgs := map[string]string{}
-
-	parent := cmd.Parent()
-	if getParent && parent != nil {
-		parent.LocalFlags().VisitAll(func(flag *pflag.Flag) {
-			localArgs[flag.Name] = flag.Value.String()
-		})
-	}
-
-	cmd.LocalFlags().VisitAll(func(flag *pflag.Flag) {
-		localArgs[flag.Name] = flag.Value.String()
-	})
-
-	return telemetry.CommandInfo{
-		Name:      cmd.Name(),
-		LocalArgs: localArgs,
-	}
+	return telemetry.GetCommandInformation(cmd, getParent)
 }

cmd/diagnostic_test.go

@@ -59,7 +59,7 @@ var _ = Describe("Diagnostic", func() {
 			Expect(err).ShouldNot(HaveOccurred())
 
 			Eventually(session).Should(gexec.Exit(0))
-			clitest.CompareTelemetryEvent(tempSettings, []telemetry.Event{
+			clitest.CompareTelemetryEventSubset(tempSettings, []telemetry.Event{
 				telemetry.CreateDiagnosticEvent(nil),
 			})
 		})