From c1fe338ac789df18cb920fbe07e41dcee8f810b8 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Fri, 13 Nov 2020 12:44:14 +0100 Subject: [PATCH 1/3] Collect metrics --help flag Signed-off-by: Guillaume Tardif --- metrics/commands.go | 2 +- metrics/metrics.go | 5 +---- metrics/metrics_test.go | 22 +++++++++++----------- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/metrics/commands.go b/metrics/commands.go index bcf9ee91..10adbc94 100644 --- a/metrics/commands.go +++ b/metrics/commands.go @@ -18,7 +18,7 @@ package metrics var commandFlags = []string{ //added to catch scan details - "--version", "--login", + "--version", "--login", "--help", } // Generated with generatecommands/main.go diff --git a/metrics/metrics.go b/metrics/metrics.go index 3f6c3931..5b6454d4 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -51,9 +51,6 @@ func GetCommand(args []string) string { result := "" onlyFlags := false for _, arg := range args { - if arg == "--help" { - return "" - } if arg == "--" { break } @@ -63,7 +60,7 @@ func GetCommand(args []string) string { } else { result = result + " " + arg } - if !isManagementCommand(arg) { + if isCommand(arg) && !isManagementCommand(arg) { onlyFlags = true } } diff --git a/metrics/metrics_test.go b/metrics/metrics_test.go index cd66e18a..fe4e604b 100644 --- a/metrics/metrics_test.go +++ b/metrics/metrics_test.go @@ -153,26 +153,26 @@ func TestGetCommand(t *testing.T) { } } -func TestIgnoreHelpCommands(t *testing.T) { +func TestKeepHelpCommands(t *testing.T) { testCases := []struct { name string args []string expected string }{ { - name: "help", - args: []string{"--help"}, - expected: "", - }, - { - name: "help on run", + name: "run with help flag", args: []string{"run", "--help"}, - expected: "", + expected: "run --help", }, { - name: "help on compose up", - args: []string{"compose", "up", "--help"}, - expected: "", + name: "with help flag before-after commands", + args: []string{"compose", "--help", "up"}, + expected: "compose --help up", + }, + { + name: "help flag", + args: []string{"--help"}, + expected: "--help", }, } From b238232a773be5dea76339b6c40fc5cc2b9bf955 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Fri, 13 Nov 2020 13:37:50 +0100 Subject: [PATCH 2/3] Reorder `--help` flag always first in metrics Signed-off-by: Guillaume Tardif --- metrics/commands.go | 2 +- metrics/metrics.go | 12 +++++++----- metrics/metrics_test.go | 4 ++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/metrics/commands.go b/metrics/commands.go index 10adbc94..bcf9ee91 100644 --- a/metrics/commands.go +++ b/metrics/commands.go @@ -18,7 +18,7 @@ package metrics var commandFlags = []string{ //added to catch scan details - "--version", "--login", "--help", + "--version", "--login", } // Generated with generatecommands/main.go diff --git a/metrics/metrics.go b/metrics/metrics.go index 5b6454d4..801c25bb 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -17,6 +17,8 @@ package metrics import ( + "strings" + "github.com/docker/compose-cli/utils" ) @@ -51,15 +53,15 @@ func GetCommand(args []string) string { result := "" onlyFlags := false for _, arg := range args { + if arg == "--help" { + result = strings.TrimSpace(arg + " " + result) + continue + } if arg == "--" { break } if isCommandFlag(arg) || (!onlyFlags && isCommand(arg)) { - if result == "" { - result = arg - } else { - result = result + " " + arg - } + result = strings.TrimSpace(result + " " + arg) if isCommand(arg) && !isManagementCommand(arg) { onlyFlags = true } diff --git a/metrics/metrics_test.go b/metrics/metrics_test.go index fe4e604b..52f35b2f 100644 --- a/metrics/metrics_test.go +++ b/metrics/metrics_test.go @@ -162,12 +162,12 @@ func TestKeepHelpCommands(t *testing.T) { { name: "run with help flag", args: []string{"run", "--help"}, - expected: "run --help", + expected: "--help run", }, { name: "with help flag before-after commands", args: []string{"compose", "--help", "up"}, - expected: "compose --help up", + expected: "--help compose up", }, { name: "help flag", From 486e7b8f877fed8a5fe5f8a2982baebe8da58024 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Fri, 13 Nov 2020 14:08:12 +0100 Subject: [PATCH 3/3] Also log help commands (no flag) Signed-off-by: Guillaume Tardif --- metrics/commands.go | 1 + metrics/generatecommands/main.go | 1 + metrics/metrics_test.go | 5 +++++ tests/e2e/e2e_test.go | 36 +++++++++++++++++++------------- 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/metrics/commands.go b/metrics/commands.go index bcf9ee91..af491a89 100644 --- a/metrics/commands.go +++ b/metrics/commands.go @@ -23,6 +23,7 @@ var commandFlags = []string{ // Generated with generatecommands/main.go var managementCommands = []string{ + "help", "ecs", "scan", "app", diff --git a/metrics/generatecommands/main.go b/metrics/generatecommands/main.go index 9491c4e8..c24c6305 100644 --- a/metrics/generatecommands/main.go +++ b/metrics/generatecommands/main.go @@ -35,6 +35,7 @@ func main() { fmt.Printf(` var managementCommands = []string{ + "help", "%s", } diff --git a/metrics/metrics_test.go b/metrics/metrics_test.go index 52f35b2f..f9c24d87 100644 --- a/metrics/metrics_test.go +++ b/metrics/metrics_test.go @@ -174,6 +174,11 @@ func TestKeepHelpCommands(t *testing.T) { args: []string{"--help"}, expected: "--help", }, + { + name: "help commands", + args: []string{"help", "run"}, + expected: "help run", + }, } for _, testCase := range testCases { diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index f404bded..d0615490 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -161,15 +161,19 @@ func TestContextMetrics(t *testing.T) { s.Start() defer s.Stop() - t.Run("do not send metrics on help commands", func(t *testing.T) { + t.Run("send metrics on help commands", func(t *testing.T) { s.ResetUsage() + c.RunDockerCmd("help", "run") c.RunDockerCmd("--help") - c.RunDockerCmd("ps", "--help") c.RunDockerCmd("run", "--help") usage := s.GetUsage() - assert.Equal(t, 0, len(usage)) + assert.DeepEqual(t, []string{ + `{"command":"help run","context":"moby","source":"cli","status":"success"}`, + `{"command":"--help","context":"moby","source":"cli","status":"success"}`, + `{"command":"--help run","context":"moby","source":"cli","status":"success"}`, + }, usage) }) t.Run("metrics on default context", func(t *testing.T) { @@ -180,10 +184,11 @@ func TestContextMetrics(t *testing.T) { c.RunDockerOrExitError("version", "--xxx") usage := s.GetUsage() - assert.Equal(t, 3, len(usage)) - assert.Equal(t, `{"command":"ps","context":"moby","source":"cli","status":"success"}`, usage[0]) - assert.Equal(t, `{"command":"version","context":"moby","source":"cli","status":"success"}`, usage[1]) - assert.Equal(t, `{"command":"version","context":"moby","source":"cli","status":"failure"}`, usage[2]) + assert.DeepEqual(t, []string{ + `{"command":"ps","context":"moby","source":"cli","status":"success"}`, + `{"command":"version","context":"moby","source":"cli","status":"success"}`, + `{"command":"version","context":"moby","source":"cli","status":"failure"}`, + }, usage) }) t.Run("metrics on other context type", func(t *testing.T) { @@ -198,14 +203,15 @@ func TestContextMetrics(t *testing.T) { c.RunDockerCmd("--context", "test-example", "ps") usage := s.GetUsage() - assert.Equal(t, 7, len(usage)) - assert.Equal(t, `{"command":"context create","context":"moby","source":"cli","status":"success"}`, usage[0]) - 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":"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]) + assert.DeepEqual(t, []string{ + `{"command":"context create","context":"moby","source":"cli","status":"success"}`, + `{"command":"ps","context":"moby","source":"cli","status":"success"}`, + `{"command":"context use","context":"moby","source":"cli","status":"success"}`, + `{"command":"ps","context":"example","source":"cli","status":"success"}`, + `{"command":"stop","context":"example","source":"cli","status":"failure"}`, + `{"command":"context use","context":"example","source":"cli","status":"success"}`, + `{"command":"ps","context":"example","source":"cli","status":"success"}`, + }, usage) }) }