[CI-3190] Improve parameter logging (#1000)

* Remove old logging

* Add new logging

* Add to commands

* Remove comment

* Change flag retrieval order

* Unify logging functions

* Remove parameter console logging

cli/cli.go

@@ -18,8 +18,6 @@ import (
 	"github.com/urfave/cli"
 )
 
-var globalTracker analytics.Tracker
-
 // Run ...
 func Run() {
 	// In the case of `--output-format=json` flag is set for the run command, all the logs are expected in JSON format.
@@ -89,12 +87,11 @@ func Run() {
 		globalTracker.Wait()
 	}()
 
-	command, subcommand, flags := commandExecutionInfo(os.Args[1:])
-	globalTracker.SendCommandInfo(command, subcommand, flags)
-
 	app.Action = func(c *cli.Context) error {
 		pluginName, pluginArgs, isPlugin := plugins.ParseArgs(c.Args())
 		if isPlugin {
+			logPluginCommandParameters(pluginName, pluginArgs)
+
 			plugin, found, err := plugins.LoadPlugin(pluginName)
 			if err != nil {
 				return fmt.Errorf("Failed to get plugin (%s), error: %s", pluginName, err)
@@ -125,46 +122,6 @@ func Run() {
 	}
 }
 
-func commandExecutionInfo(args []string) (string, string, []string) {
-	if len(args) == 0 {
-		return "", "", nil
-	}
-
-	command := args[0]
-	commandFlags := collectFlags(args)
-	isPluginCommand := strings.HasPrefix(command, ":")
-
-	if isPluginCommand && len(args) > 1 {
-		return command, args[1], commandFlags
-	}
-
-	return command, "", commandFlags
-}
-
-func collectFlags(args []string) []string {
-	var commandFlags []string
-	for _, arg := range args {
-		if !strings.HasPrefix(arg, "-") && !strings.HasPrefix(arg, "--") {
-			continue
-		}
-
-		components := strings.Split(arg, "=")
-		if len(components) < 1 {
-			continue
-		}
-
-		components = strings.Split(components[0], " ")
-		if len(components) < 1 {
-			continue
-		}
-
-		trimmedFlag := strings.TrimPrefix(strings.TrimPrefix(components[0], "--"), "-")
-		commandFlags = append(commandFlags, trimmedFlag)
-	}
-
-	return commandFlags
-}
-
 func loggerParameters(arguments []string) (isRunCommand bool, outputFormat log.LoggerType, isDebug bool) {
 	for i, argument := range arguments {
 		if argument == "run" {

cli/cli_test.go

@@ -7,54 +7,6 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestCommandInfo(t *testing.T) {
-	tests := []struct {
-		name           string
-		args           []string
-		wantCommand    string
-		wantSubcommand string
-		wantFlags      []string
-	}{
-		{
-			name:           "Empty command",
-			args:           []string{},
-			wantCommand:    "",
-			wantSubcommand: "",
-			wantFlags:      nil,
-		},
-		{
-			name:           "CLI command",
-			args:           []string{"run", "e2e"},
-			wantCommand:    "run",
-			wantSubcommand: "",
-			wantFlags:      nil,
-		},
-		{
-			name:           "Plugin command",
-			args:           []string{":plugin", "do", "something"},
-			wantCommand:    ":plugin",
-			wantSubcommand: "do",
-			wantFlags:      nil,
-		},
-		{
-			name:           "Flags",
-			args:           []string{"run", "--A", "-a", "--B=true", "-b false", "--C /path/to/something"},
-			wantCommand:    "run",
-			wantSubcommand: "",
-			wantFlags:      []string{"A", "a", "B", "b", "C"},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			command, subcommand, flags := commandExecutionInfo(tt.args)
-			assert.Equalf(t, tt.wantCommand, command, "commandExecutionInfo(%v)", tt.args)
-			assert.Equalf(t, tt.wantSubcommand, subcommand, "commandExecutionInfo(%v)", tt.args)
-			assert.Equalf(t, tt.wantFlags, flags, "commandExecutionInfo(%v)", tt.args)
-		})
-	}
-}
-
 func Test_loggerParameters(t *testing.T) {
 	tests := []struct {
 		name             string

cli/command_analytics.go

@@ -0,0 +1,60 @@
+package cli
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/bitrise-io/bitrise/analytics"
+	"github.com/urfave/cli"
+)
+
+var globalTracker analytics.Tracker
+
+func logPluginCommandParameters(name string, _ []string) {
+	// Plugin command parameters are routed into the function but are not processed yet because it is complex to correctly
+	// parse the arguments without knowing the structure. If we notice that our users do use plugins, then we can add
+	// plugin specific argument parsers.
+	sendCommandInfo(fmt.Sprintf(":%s", name), "", []string{})
+}
+
+func logCommandParameters(c *cli.Context) {
+	if c == nil {
+		return
+	}
+
+	commandName := "unknown"
+	subcommandName := ""
+
+	if names := strings.Split(c.Command.FullName(), " "); 0 < len(names) {
+		commandName = names[0]
+		if 1 < len(names) {
+			subcommandName = names[1]
+		}
+	}
+
+	flags := collectFlags(c)
+
+	sendCommandInfo(commandName, subcommandName, flags)
+}
+
+func collectFlags(c *cli.Context) []string {
+	var flags []string
+
+	for _, flag := range c.GlobalFlagNames() {
+		if isSet := c.GlobalIsSet(flag); isSet {
+			flags = append(flags, flag)
+		}
+	}
+
+	for _, flag := range c.FlagNames() {
+		if isSet := c.IsSet(flag); isSet {
+			flags = append(flags, flag)
+		}
+	}
+
+	return flags
+}
+
+func sendCommandInfo(command, subcommand string, flags []string) {
+	globalTracker.SendCommandInfo(command, subcommand, flags)
+}

cli/init.go

@@ -15,6 +15,8 @@ var initCmd = cli.Command{
 	Aliases: []string{"i"},
 	Usage:   "Init bitrise config.",
 	Action: func(c *cli.Context) error {
+		logCommandParameters(c)
+
 		logger := log.NewLogger(log.GetGlobalLoggerOpts())
 		if err := initConfig(c); err != nil {
 

cli/merge.go

@@ -27,6 +27,8 @@ var mergeConfigCommand = cli.Command{
 }
 
 func mergeConfig(c *cli.Context) error {
+	logCommandParameters(c)
+
 	var configPth string
 	if c.Args().Present() {
 		configPth = c.Args().First()

cli/plugin_delete.go

@@ -14,6 +14,8 @@ var pluginDeleteCommand = cli.Command{
 	Name:  "delete",
 	Usage: "Delete bitrise plugin.",
 	Action: func(c *cli.Context) error {
+		logCommandParameters(c)
+
 		if err := pluginDelete(c); err != nil {
 			log.Errorf("Plugin delete failed, error: %s", err)
 			os.Exit(1)

cli/plugin_info.go

@@ -15,6 +15,8 @@ var pluginInfoCommand = cli.Command{
 	Name:  "info",
 	Usage: "Installed bitrise plugin's info",
 	Action: func(c *cli.Context) error {
+		logCommandParameters(c)
+
 		if err := pluginInfo(c); err != nil {
 			log.Errorf("Plugin info failed, error: %s", err)
 			os.Exit(1)

cli/plugin_install.go

@@ -13,6 +13,8 @@ var pluginInstallCommand = cli.Command{
 	Name:  "install",
 	Usage: "Install bitrise plugin.",
 	Action: func(c *cli.Context) error {
+		logCommandParameters(c)
+
 		if err := pluginInstall(c); err != nil {
 			log.Errorf("Plugin install failed, error: %s", err)
 			os.Exit(1)

cli/plugin_list.go

@@ -14,6 +14,8 @@ var pluginListCommand = cli.Command{
 	Name:  "list",
 	Usage: "List installed bitrise plugins.",
 	Action: func(c *cli.Context) error {
+		logCommandParameters(c)
+
 		if err := pluginList(c); err != nil {
 			log.Errorf("Plugin list failed, error: %s", err)
 			os.Exit(1)

cli/plugin_update.go

@@ -14,6 +14,8 @@ var pluginUpdateCommand = cli.Command{
 	Name:  "update",
 	Usage: "Update bitrise plugin. If <plugin_name> not specified, every plugin will be updated.",
 	Action: func(c *cli.Context) error {
+		logCommandParameters(c)
+
 		if err := pluginUpdate(c); err != nil {
 			log.Errorf("Plugin update failed, error: %s", err)
 			os.Exit(1)

cli/preload_steps.go

@@ -24,6 +24,8 @@ var stepsCommand = cli.Command{
 			Name:  "list-cached",
 			Usage: "List all the cached steps",
 			Action: func(c *cli.Context) error {
+				logCommandParameters(c)
+
 				return listCachedSteps(c)
 			},
 			Flags: []cli.Flag{
@@ -44,6 +46,8 @@ var stepsCommand = cli.Command{
 			Usage:     "Makes sure that Bitrise CLI can be used in offline mode by preloading Bitrise maintaned Steps.",
 			UsageText: fmt.Sprintf("Use the %s env var to test after preloading steps.", configs.IsSteplibOfflineModeEnvKey),
 			Action: func(c *cli.Context) error {
+				logCommandParameters(c)
+
 				if err := preloadSteps(c); err != nil {
 					log.Errorf("Preload failed: %s", err)
 					os.Exit(1)

cli/run.go

@@ -73,6 +73,8 @@ var runCommand = cli.Command{
 }
 
 func run(c *cli.Context) error {
+	logCommandParameters(c)
+
 	signalInterruptChan := make(chan os.Signal, 1)
 	signal.Notify(signalInterruptChan, syscall.SIGINT, syscall.SIGTERM)
 

cli/setup.go

@@ -12,6 +12,8 @@ var setupCommand = cli.Command{
 	Name:  "setup",
 	Usage: "Setup the current host. Install every required tool to run Workflows.",
 	Action: func(c *cli.Context) error {
+		logCommandParameters(c)
+
 		if err := setup(c); err != nil {
 			log.Errorf("Setup failed, error: %s", err)
 			os.Exit(1)

cli/share.go

@@ -6,6 +6,8 @@ import (
 )
 
 func share(c *cli.Context) error {
+	logCommandParameters(c)
+
 	if err := tools.StepmanShare(); err != nil {
 		failf("Bitrise share failed, error: %s", err)
 	}

cli/share_audit.go

@@ -5,7 +5,9 @@ import (
 	"github.com/urfave/cli"
 )
 
-func shareAudit(_ *cli.Context) error {
+func shareAudit(c *cli.Context) error {
+	logCommandParameters(c)
+
 	if err := tools.StepmanShareAudit(); err != nil {
 		failf("Bitrise share audit failed, error: %s", err)
 	}

cli/share_create.go

@@ -6,6 +6,8 @@ import (
 )
 
 func create(c *cli.Context) error {
+	logCommandParameters(c)
+
 	// Input validation
 	tag := c.String(TagKey)
 	if tag == "" {

cli/share_finish.go

@@ -5,7 +5,9 @@ import (
 	"github.com/urfave/cli"
 )
 
-func finish(_ *cli.Context) error {
+func finish(c *cli.Context) error {
+	logCommandParameters(c)
+
 	if err := tools.StepmanShareFinish(); err != nil {
 		failf("Bitrise share finish failed, error: %s", err)
 	}

cli/share_start.go

@@ -6,6 +6,8 @@ import (
 )
 
 func start(c *cli.Context) error {
+	logCommandParameters(c)
+
 	// Input validation
 	collectionURI := c.String(CollectionKey)
 	if collectionURI == "" {

cli/tools.go

@@ -12,6 +12,8 @@ var envmanCommand = cli.Command{
 	Usage:           "Runs an envman command.",
 	SkipFlagParsing: true,
 	Action: func(c *cli.Context) error {
+		logCommandParameters(c)
+
 		if err := runCommandWith("envman", c); err != nil {
 			failf("Command failed, error: %s", err)
 		}

cli/trigger.go

@@ -65,6 +65,8 @@ func printAvailableTriggerFilters(triggerMap []models.TriggerMapItemModel) {
 }
 
 func trigger(c *cli.Context) error {
+	logCommandParameters(c)
+
 	// Expand cli.Context
 	var prGlobalFlagPtr *bool
 	if c.GlobalIsSet(PRKey) {

cli/trigger_check.go

@@ -73,6 +73,8 @@ func getPipelineAndWorkflowIDByParamsInCompatibleMode(triggerMap models.TriggerM
 // --------------------
 
 func triggerCheck(c *cli.Context) error {
+	logCommandParameters(c)
+
 	warnings := []string{}
 
 	//

cli/update.go

@@ -31,6 +31,8 @@ var updateCommand = cli.Command{
 	Name:  "update",
 	Usage: "Updates the Bitrise CLI.",
 	Action: func(c *cli.Context) error {
+		logCommandParameters(c)
+
 		if err := update(c); err != nil {
 			log.Errorf("Update Bitrise CLI failed, error: %s", err)
 			os.Exit(1)

cli/validate.go

@@ -218,6 +218,8 @@ func runValidate(bitriseConfigPath string, bitriseConfigBase64Data string, inven
 }
 
 func validate(c *cli.Context) error {
+	logCommandParameters(c)
+
 	// Expand cli.Context
 	bitriseConfigBase64Data := c.String(ConfigBase64Key)
 	bitriseConfigPath := c.String(ConfigKey)

cli/version.go

@@ -22,6 +22,8 @@ type VersionOutputModel struct {
 }
 
 func printVersionCmd(c *cli.Context) error {
+	logCommandParameters(c)
+
 	fullVersion := c.Bool("full")
 
 	if err := output.ConfigureOutputFormat(c); err != nil {

cli/workflow_list.go

@@ -17,6 +17,8 @@ var workflowListCommand = cli.Command{
 	Name:  "workflows",
 	Usage: "List of available workflows in config.",
 	Action: func(c *cli.Context) error {
+		logCommandParameters(c)
+
 		if err := workflowList(c); err != nil {
 			log.Errorf("List of available workflows in config failed, error: %s", err)
 			os.Exit(1)