diff --git a/cli/main.go b/cli/main.go index 316479921..ea6d3e4cd 100644 --- a/cli/main.go +++ b/cli/main.go @@ -198,37 +198,37 @@ func main() { if err = root.ExecuteContext(ctx); err != nil { // if user canceled request, simply exit without any error message if errdefs.IsErrCanceled(err) || errors.Is(ctx.Err(), context.Canceled) { - metrics.Track(ctype, os.Args[1:], root.PersistentFlags(), metrics.CanceledStatus) + metrics.Track(ctype, os.Args[1:], metrics.CanceledStatus) os.Exit(130) } if ctype == store.AwsContextType { - exit(root, currentContext, errors.Errorf(`%q context type has been renamed. Recreate the context by running: + exit(currentContext, errors.Errorf(`%q context type has been renamed. Recreate the context by running: $ docker context create %s `, cc.Type(), store.EcsContextType), ctype) } // Context should always be handled by new CLI requiredCmd, _, _ := root.Find(os.Args[1:]) if requiredCmd != nil && isContextAgnosticCommand(requiredCmd) { - exit(root, currentContext, err, ctype) + exit(currentContext, err, ctype) } mobycli.ExecIfDefaultCtxType(ctx, root) - checkIfUnknownCommandExistInDefaultContext(err, currentContext, root, ctype) + checkIfUnknownCommandExistInDefaultContext(err, currentContext, ctype) - exit(root, currentContext, err, ctype) + exit(currentContext, err, ctype) } - metrics.Track(ctype, os.Args[1:], root.PersistentFlags(), metrics.SuccessStatus) + metrics.Track(ctype, os.Args[1:], metrics.SuccessStatus) } -func exit(root *cobra.Command, ctx string, err error, ctype string) { - metrics.Track(ctype, os.Args[1:], root.PersistentFlags(), metrics.FailureStatus) +func exit(ctx string, err error, ctype string) { + metrics.Track(ctype, os.Args[1:], metrics.FailureStatus) if errors.Is(err, errdefs.ErrLoginRequired) { fmt.Fprintln(os.Stderr, err) os.Exit(errdefs.ExitCodeLoginRequired) } if errors.Is(err, errdefs.ErrNotImplemented) { - name := metrics.GetCommand(os.Args[1:], root.PersistentFlags()) + name := metrics.GetCommand(os.Args[1:]) fmt.Fprintf(os.Stderr, "Command %q not available in current context (%s)\n", name, ctx) os.Exit(1) @@ -242,14 +242,14 @@ func fatal(err error) { os.Exit(1) } -func checkIfUnknownCommandExistInDefaultContext(err error, currentContext string, root *cobra.Command, contextType string) { +func checkIfUnknownCommandExistInDefaultContext(err error, currentContext string, contextType string) { submatch := unknownCommandRegexp.FindSubmatch([]byte(err.Error())) if len(submatch) == 2 { dockerCommand := string(submatch[1]) if mobycli.IsDefaultContextCommand(dockerCommand) { fmt.Fprintf(os.Stderr, "Command %q not available in current context (%s), you can use the \"default\" context to run this command\n", dockerCommand, currentContext) - metrics.Track(contextType, os.Args[1:], root.PersistentFlags(), metrics.FailureStatus) + metrics.Track(contextType, os.Args[1:], metrics.FailureStatus) os.Exit(1) } } diff --git a/cli/mobycli/exec.go b/cli/mobycli/exec.go index 5b89cd1c3..8c4500050 100644 --- a/cli/mobycli/exec.go +++ b/cli/mobycli/exec.go @@ -86,7 +86,7 @@ func Exec(root *cobra.Command) { err := cmd.Run() childExit <- true if err != nil { - metrics.Track(store.DefaultContextType, os.Args[1:], root.PersistentFlags(), metrics.FailureStatus) + metrics.Track(store.DefaultContextType, os.Args[1:], metrics.FailureStatus) if exiterr, ok := err.(*exec.ExitError); ok { os.Exit(exiterr.ExitCode()) @@ -94,7 +94,7 @@ func Exec(root *cobra.Command) { fmt.Fprintln(os.Stderr, err) os.Exit(1) } - metrics.Track(store.DefaultContextType, os.Args[1:], root.PersistentFlags(), metrics.SuccessStatus) + metrics.Track(store.DefaultContextType, os.Args[1:], metrics.SuccessStatus) os.Exit(0) } diff --git a/metrics/commands.go b/metrics/commands.go new file mode 100644 index 000000000..bcf9ee91f --- /dev/null +++ b/metrics/commands.go @@ -0,0 +1,147 @@ +/* + Copyright 2020 Docker Compose CLI authors + + 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 metrics + +var commandFlags = []string{ + //added to catch scan details + "--version", "--login", +} + +// Generated with generatecommands/main.go +var managementCommands = []string{ + "ecs", + "scan", + "app", + "builder", + "imagetools", + "buildx", + "checkpoint", + "config", + "container", + "context", + "create", + "image", + "manifest", + "network", + "node", + "plugin", + "secret", + "service", + "stack", + "swarm", + "system", + "key", + "signer", + "trust", + "volume", + "login", + "logout", + "compose", +} + +var commands = []string{ + "bundle", + "completion", + "init", + "inspect", + "install", + "deploy", + "list", + "ls", + "merge", + "pull", + "push", + "render", + "split", + "status", + "uninstall", + "upgrade", + "validate", + "version", + "build", + "prune", + "create", + "bake", + "f", + "b", + "du", + "rm", + "stop", + "use", + "remove", + "attach", + "commit", + "cp", + "diff", + "exec", + "export", + "kill", + "logs", + "ps", + "pause", + "port", + "rename", + "restart", + "run", + "start", + "stats", + "top", + "unpause", + "update", + "wait", + "aci", + "ecs", + "show", + "history", + "import", + "load", + "images", + "rmi", + "save", + "tag", + "annotate", + "connect", + "disconnect", + "demote", + "promote", + "disable", + "enable", + "set", + "rollback", + "scale", + "up", + "down", + "services", + "ca", + "join", + "join-token", + "leave", + "unlock", + "unlock-key", + "df", + "events", + "info", + "generate", + "add", + "revoke", + "sign", + "login", + "azure", + "logout", + "search", + "convert", +} diff --git a/metrics/generatecommands/main.go b/metrics/generatecommands/main.go new file mode 100644 index 000000000..9491c4e8a --- /dev/null +++ b/metrics/generatecommands/main.go @@ -0,0 +1,106 @@ +/* + Copyright 2020 Docker Compose CLI authors + + 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 main + +import ( + "fmt" + "os/exec" + "strings" + + "github.com/docker/compose-cli/utils" +) + +var managementCommands = []string{"ecs", "scan"} + +var commands = []string{} + +func main() { + fmt.Println("Walking through docker help to list commands...") + getCommands() + getCommands("compose") + + fmt.Printf(` +var managementCommands = []string{ + "%s", +} + +var commands = []string{ + "%s", +} +`, strings.Join(managementCommands, "\", \n\t\""), strings.Join(commands, "\", \n\t\"")) +} + +const ( + mgtCommandsSection = "Management Commands:" + commandsSection = "Commands:" + aliasesSection = "Aliases:" +) + +func getCommands(execCommands ...string) { + withHelp := append(execCommands, "--help") + cmd := exec.Command("docker", withHelp...) + output, err := cmd.Output() + if err != nil { + return + } + text := string(output) + lines := strings.Split(text, "\n") + section := "" + for _, line := range lines { + trimmedLine := strings.TrimSpace(line) + if strings.HasPrefix(trimmedLine, mgtCommandsSection) { + section = mgtCommandsSection + continue + } + if strings.HasPrefix(trimmedLine, commandsSection) || strings.HasPrefix(trimmedLine, "Available Commands:") { + section = commandsSection + if len(execCommands) > 0 { + command := execCommands[len(execCommands)-1] + managementCommands = append(managementCommands, command) + } + continue + } + if strings.HasPrefix(trimmedLine, aliasesSection) { + section = aliasesSection + continue + } + if trimmedLine == "" { + section = "" + continue + } + + tokens := strings.Split(trimmedLine, " ") + command := strings.Replace(tokens[0], "*", "", 1) + switch section { + case mgtCommandsSection: + getCommands(append(execCommands, command)...) + case commandsSection: + if !utils.StringContains(commands, command) { + commands = append(commands, command) + } + getCommands(append(execCommands, command)...) + case aliasesSection: + aliases := strings.Split(trimmedLine, ",") + for _, alias := range aliases { + trimmedAlias := strings.TrimSpace(alias) + if !utils.StringContains(commands, trimmedAlias) { + commands = append(commands, trimmedAlias) + } + } + } + } +} diff --git a/metrics/metrics.go b/metrics/metrics.go index 05892cd4a..3f6c3931d 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -17,68 +17,12 @@ package metrics import ( - "strings" - - flag "github.com/spf13/pflag" - "github.com/docker/compose-cli/utils" ) -var managementCommands = []string{ - "app", - "assemble", - "builder", - "buildx", - "ecs", - "ecs compose", - "cluster", - "compose", - "config", - "container", - "context", - // We add "context create" as a management command to be able to catch - // calls to "context create aci" - "context create", - "help", - "image", - // Adding "login" as a management command so that the system can catch - // commands like `docker login azure` - "login", - "manifest", - "network", - "node", - "plugin", - "registry", - "secret", - "service", - "stack", - "swarm", - "system", - "template", - "trust", - "volume", -} - -// managementSubCommands holds a list of allowed subcommands of a management -// command. For example we want to send an event for "docker login azure" but -// we don't wat to send the name of the registry when the user does a -// "docker login my-registry", we only want to send "login" -var managementSubCommands = map[string][]string{ - "login": { - "azure", - }, - "context create": { - "aci", - }, -} - -const ( - scanCommand = "scan" -) - // Track sends the tracking analytics to Docker Desktop -func Track(context string, args []string, flags *flag.FlagSet, status string) { - command := GetCommand(args, flags) +func Track(context string, args []string, status string) { + command := GetCommand(args) if command != "" { c := NewClient() c.Send(Command{ @@ -90,115 +34,39 @@ func Track(context string, args []string, flags *flag.FlagSet, status string) { } } +func isCommand(word string) bool { + return utils.StringContains(commands, word) || isManagementCommand(word) +} + +func isManagementCommand(word string) bool { + return utils.StringContains(managementCommands, word) +} + +func isCommandFlag(word string) bool { + return utils.StringContains(commandFlags, word) +} + // GetCommand get the invoked command -func GetCommand(args []string, flags *flag.FlagSet) string { - command := "" - strippedArgs := stripFlags(args, flags) - - if len(strippedArgs) != 0 { - command = strippedArgs[0] - - if command == scanCommand { - return getScanCommand(args) +func GetCommand(args []string) string { + result := "" + onlyFlags := false + for _, arg := range args { + if arg == "--help" { + return "" } - - for { - if utils.StringContains(managementCommands, command) { - if sub := getSubCommand(command, strippedArgs[1:]); sub != "" { - command += " " + sub - strippedArgs = strippedArgs[1:] - continue - } - } + if arg == "--" { break } - } - - return command -} - -func getScanCommand(args []string) string { - command := args[0] - - if utils.StringContains(args, "--auth") { - return command + " auth" - } - - if utils.StringContains(args, "--version") { - return command + " version" - } - - return command -} - -func getSubCommand(command string, args []string) string { - if len(args) == 0 { - return "" - } - - if val, ok := managementSubCommands[command]; ok { - if utils.StringContains(val, args[0]) { - return args[0] - } - return "" - } - - if isArg(args[0]) { - return args[0] - } - - return "" -} - -func stripFlags(args []string, flags *flag.FlagSet) []string { - commands := []string{} - - for len(args) > 0 { - s := args[0] - args = args[1:] - - if s == "--" { - return commands - } - - if flagArg(s, flags) { - if len(args) <= 1 { - return commands + if isCommandFlag(arg) || (!onlyFlags && isCommand(arg)) { + if result == "" { + result = arg + } else { + result = result + " " + arg + } + if !isManagementCommand(arg) { + onlyFlags = true } - args = args[1:] - } - - if isArg(s) { - commands = append(commands, s) } } - - return commands -} - -func flagArg(s string, flags *flag.FlagSet) bool { - return strings.HasPrefix(s, "--") && !strings.Contains(s, "=") && !hasNoOptDefVal(s[2:], flags) || - strings.HasPrefix(s, "-") && !strings.Contains(s, "=") && len(s) == 2 && !shortHasNoOptDefVal(s[1:], flags) -} - -func isArg(s string) bool { - return s != "" && !strings.HasPrefix(s, "-") -} - -func hasNoOptDefVal(name string, fs *flag.FlagSet) bool { - flag := fs.Lookup(name) - if flag == nil { - return false - } - - return flag.NoOptDefVal != "" -} - -func shortHasNoOptDefVal(name string, fs *flag.FlagSet) bool { - flag := fs.ShorthandLookup(name[:1]) - if flag == nil { - return false - } - - return flag.NoOptDefVal != "" + return result } diff --git a/metrics/metrics_test.go b/metrics/metrics_test.go index cb79f9a57..cd66e18a4 100644 --- a/metrics/metrics_test.go +++ b/metrics/metrics_test.go @@ -19,15 +19,10 @@ package metrics import ( "testing" - "github.com/spf13/cobra" "gotest.tools/v3/assert" ) -func TestFlag(t *testing.T) { - root := &cobra.Command{} - root.PersistentFlags().BoolP("debug", "D", false, "debug") - root.PersistentFlags().String("str", "str", "str") - +func TestGetCommand(t *testing.T) { testCases := []struct { name string args []string @@ -58,16 +53,6 @@ func TestFlag(t *testing.T) { args: []string{"--debug", "--str", "str-value"}, expected: "", }, - { - name: "with unknown short flag", - args: []string{"-f", "run"}, - expected: "", - }, - { - name: "with unknown long flag", - args: []string{"--unknown-flag", "run"}, - expected: "", - }, { name: "management command", args: []string{"image", "ls"}, @@ -76,7 +61,7 @@ func TestFlag(t *testing.T) { { name: "management command with flag", args: []string{"image", "--test", "ls"}, - expected: "image", + expected: "image ls", }, { name: "management subcommand with flag", @@ -88,26 +73,36 @@ func TestFlag(t *testing.T) { args: []string{"login", "azure"}, expected: "login azure", }, + { + name: "azure logout", + args: []string{"logout", "azure"}, + expected: "logout azure", + }, { name: "azure login with flags", args: []string{"login", "-u", "test", "azure"}, expected: "login azure", }, { - name: "azure login with azure user", - args: []string{"login", "-u", "azure"}, + name: "login to a registry", + args: []string{"login", "myregistry"}, expected: "login", }, { - name: "login to a registry", - args: []string{"login", "registry"}, - expected: "login", + name: "logout from a registry", + args: []string{"logout", "myregistry"}, + expected: "logout", }, { name: "context create aci", args: []string{"context", "create", "aci"}, expected: "context create aci", }, + { + name: "context create ecs", + args: []string{"context", "create", "ecs"}, + expected: "context create ecs", + }, { name: "create a context from another context", args: []string{"context", "create", "test-context", "--from=default"}, @@ -119,9 +114,9 @@ func TestFlag(t *testing.T) { expected: "create", }, { - name: "create a container named aci", - args: []string{"create", "aci"}, - expected: "create", + name: "start a container named aci", + args: []string{"start", "aci"}, + expected: "start", }, { name: "create a container named test-container", @@ -152,15 +147,44 @@ func TestFlag(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - result := GetCommand(testCase.args, root.PersistentFlags()) + result := GetCommand(testCase.args) + assert.Equal(t, testCase.expected, result) + }) + } +} + +func TestIgnoreHelpCommands(t *testing.T) { + testCases := []struct { + name string + args []string + expected string + }{ + { + name: "help", + args: []string{"--help"}, + expected: "", + }, + { + name: "help on run", + args: []string{"run", "--help"}, + expected: "", + }, + { + name: "help on compose up", + args: []string{"compose", "up", "--help"}, + expected: "", + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + result := GetCommand(testCase.args) assert.Equal(t, testCase.expected, result) }) } } func TestEcs(t *testing.T) { - root := &cobra.Command{} - testCases := []struct { name string args []string @@ -217,23 +241,21 @@ func TestEcs(t *testing.T) { expected: "ecs compose logs", }, { - name: "setup", - args: []string{"ecs", "setup"}, - expected: "ecs setup", + name: "ecs", + args: []string{"ecs", "anything"}, + expected: "ecs", }, } for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - result := GetCommand(testCase.args, root.PersistentFlags()) + result := GetCommand(testCase.args) assert.Equal(t, testCase.expected, result) }) } } func TestScan(t *testing.T) { - root := &cobra.Command{} - testCases := []struct { name string args []string @@ -246,34 +268,34 @@ func TestScan(t *testing.T) { }, { name: "scan image with long flags", - args: []string{"scan", "--file", "file", "image"}, + args: []string{"scan", "--file", "file", "myimage"}, expected: "scan", }, { name: "scan image with short flags", - args: []string{"scan", "-f", "file", "image"}, + args: []string{"scan", "-f", "file", "myimage"}, expected: "scan", }, { name: "scan with long flag", - args: []string{"scan", "--dependency-tree", "image"}, + args: []string{"scan", "--dependency-tree", "myimage"}, expected: "scan", }, { name: "auth", - args: []string{"scan", "--auth"}, - expected: "scan auth", + args: []string{"scan", "--login"}, + expected: "scan --login", }, { name: "version", args: []string{"scan", "--version"}, - expected: "scan version", + expected: "scan --version", }, } for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - result := GetCommand(testCase.args, root.PersistentFlags()) + result := GetCommand(testCase.args) assert.Equal(t, testCase.expected, result) }) } diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index 18dc00e3f..97cdee5eb 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -164,6 +164,17 @@ func TestContextMetrics(t *testing.T) { s.Start() defer s.Stop() + t.Run("do not send metrics on help commands", func(t *testing.T) { + s.ResetUsage() + + c.RunDockerCmd("--help") + c.RunDockerCmd("ps", "--help") + c.RunDockerCmd("run", "--help") + + usage := s.GetUsage() + assert.Equal(t, 0, len(usage)) + }) + t.Run("metrics on default context", func(t *testing.T) { s.ResetUsage() @@ -185,7 +196,7 @@ func TestContextMetrics(t *testing.T) { c.RunDockerCmd("ps") c.RunDockerCmd("context", "use", "test-example") c.RunDockerCmd("ps") - c.RunDockerOrExitError("error") + c.RunDockerOrExitError("stop", "unknown") c.RunDockerCmd("context", "use", "default") c.RunDockerCmd("--context", "test-example", "ps") @@ -195,7 +206,7 @@ func TestContextMetrics(t *testing.T) { assert.Equal(t, `{"command":"ps","context":"moby","source":"cli","status":"success"}`, usage[1]) assert.Equal(t, `{"command":"context use","context":"moby","source":"cli","status":"success"}`, usage[2]) assert.Equal(t, `{"command":"ps","context":"example","source":"cli","status":"success"}`, usage[3]) - assert.Equal(t, `{"command":"error","context":"example","source":"cli","status":"failure"}`, usage[4]) + assert.Equal(t, `{"command":"stop","context":"example","source":"cli","status":"failure"}`, usage[4]) assert.Equal(t, `{"command":"context use","context":"example","source":"cli","status":"success"}`, usage[5]) assert.Equal(t, `{"command":"ps","context":"example","source":"cli","status":"success"}`, usage[6]) })