Merge pull request #1503 from docker/metrics-failures

Author: ndeloof

cli/cmd/compose/compose.go

@@ -30,6 +30,7 @@ import (
 
 	"github.com/docker/compose-cli/api/context/store"
 	"github.com/docker/compose-cli/cli/formatter"
+	"github.com/docker/compose-cli/cli/metrics"
 )
 
 // Warning is a global warning to be displayed to user on command failure
@@ -74,7 +75,7 @@ func (o *projectOptions) toProject(services []string, po ...cli.ProjectOptionsFn
 
 	project, err := cli.ProjectFromOptions(options)
 	if err != nil {
-		return nil, err
+		return nil, metrics.WrapComposeError(err)
 	}
 
 	s, err := project.GetServices(services...)
@@ -112,6 +113,14 @@ func Command(contextType string) *cobra.Command {
 		Short:            "Docker Compose",
 		Use:              "compose",
 		TraverseChildren: true,
+		// By default (no Run/RunE in parent command) for typos in subcommands, cobra displays the help of parent command but exit(0) !
+		RunE: func(cmd *cobra.Command, args []string) error {
+			if len(args) == 0 {
+				return cmd.Help()
+			}
+			_ = cmd.Help()
+			return fmt.Errorf("unknown docker command: %q", "compose "+args[0])
+		},
 		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
 			if noAnsi {
 				if ansi != "auto" {

cli/main.go

@@ -70,7 +70,7 @@ var (
 		"version":          {},
 		"backend-metadata": {},
 	}
-	unknownCommandRegexp = regexp.MustCompile(`unknown command "([^"]*)"`)
+	unknownCommandRegexp = regexp.MustCompile(`unknown docker command: "([^"]*)"`)
 )
 
 func init() {
@@ -122,7 +122,7 @@ func main() {
 			if len(args) == 0 {
 				return cmd.Help()
 			}
-			return fmt.Errorf("unknown command %q", args[0])
+			return fmt.Errorf("unknown docker command: %q", args[0])
 		},
 	}
 
@@ -292,7 +292,18 @@ func exit(ctx string, err error, ctype string) {
 		os.Exit(exit.StatusCode)
 	}
 
-	metrics.Track(ctype, os.Args[1:], metrics.FailureStatus)
+	var composeErr metrics.ComposeError
+	metricsStatus := metrics.FailureStatus
+	exitCode := 1
+	if errors.As(err, &composeErr) {
+		metricsStatus = composeErr.GetMetricsFailureCategory().MetricsStatus
+		exitCode = composeErr.GetMetricsFailureCategory().ExitCode
+	}
+	if strings.HasPrefix(err.Error(), "unknown shorthand flag:") || strings.HasPrefix(err.Error(), "unknown flag:") || strings.HasPrefix(err.Error(), "unknown docker command:") {
+		metricsStatus = metrics.CommandSyntaxFailure.MetricsStatus
+		exitCode = metrics.CommandSyntaxFailure.ExitCode
+	}
+	metrics.Track(ctype, os.Args[1:], metricsStatus)
 
 	if errors.Is(err, errdefs.ErrLoginRequired) {
 		fmt.Fprintln(os.Stderr, err)
@@ -310,7 +321,8 @@ func exit(ctx string, err error, ctype string) {
 		os.Exit(1)
 	}
 
-	fatal(err)
+	fmt.Fprintln(os.Stderr, err)
+	os.Exit(exitCode)
 }
 
 func fatal(err error) {

cli/metrics/client.go

@@ -47,17 +47,6 @@ func init() {
 	}
 }
 
-const (
-	// APISource is sent for API metrics
-	APISource = "api"
-	// SuccessStatus is sent for API metrics
-	SuccessStatus = "success"
-	// FailureStatus is sent for API metrics
-	FailureStatus = "failure"
-	// CanceledStatus is sent for API metrics
-	CanceledStatus = "canceled"
-)
-
 // Client sends metrics to Docker Desktopn
 type Client interface {
 	// Send sends the command to Docker Desktop. Note that the function doesn't

cli/metrics/compose_errors.go

@@ -0,0 +1,72 @@
+/*
+   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
+
+import (
+	"io/fs"
+
+	"github.com/pkg/errors"
+
+	composeerrdefs "github.com/compose-spec/compose-go/errdefs"
+)
+
+// ComposeError error to categorize failures and extract metrics info
+type ComposeError struct {
+	Err      error
+	Category *FailureCategory
+}
+
+// WrapComposeError wraps the error if not nil, otherwise returns nil
+func WrapComposeError(err error) error {
+	if err == nil {
+		return nil
+	}
+	return ComposeError{
+		Err: err,
+	}
+}
+
+// WrapCategorisedComposeError wraps the error if not nil, otherwise returns nil
+func WrapCategorisedComposeError(err error, failure FailureCategory) error {
+	if err == nil {
+		return nil
+	}
+	return ComposeError{
+		Err:      err,
+		Category: &failure,
+	}
+}
+
+// Unwrap get underlying error
+func (e ComposeError) Unwrap() error { return e.Err }
+
+func (e ComposeError) Error() string { return e.Err.Error() }
+
+// GetMetricsFailureCategory get metrics status and error code corresponding to this error
+func (e ComposeError) GetMetricsFailureCategory() FailureCategory {
+	if e.Category != nil {
+		return *e.Category
+	}
+	var pathError *fs.PathError
+	if errors.As(e.Err, &pathError) {
+		return FileNotFoundFailure
+	}
+	if composeerrdefs.IsNotFoundError(e.Err) {
+		return FileNotFoundFailure
+	}
+	return ComposeParseFailure
+}

cli/metrics/definitions.go

@@ -0,0 +1,57 @@
+/*
+   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
+
+// FailureCategory sruct regrouping metrics failure status and specific exit code
+type FailureCategory struct {
+	MetricsStatus string
+	ExitCode      int
+}
+
+const (
+	// APISource is sent for API metrics
+	APISource = "api"
+	// SuccessStatus command success
+	SuccessStatus = "success"
+	// FailureStatus command failure
+	FailureStatus = "failure"
+	// ComposeParseFailureStatus failure while parsing compose file
+	ComposeParseFailureStatus = "failure-compose-parse"
+	// FileNotFoundFailureStatus failure getting compose file
+	FileNotFoundFailureStatus = "failure-file-not-found"
+	// CommandSyntaxFailureStatus failure reading command
+	CommandSyntaxFailureStatus = "failure-cmd-syntax"
+	// BuildFailureStatus failure building imge
+	BuildFailureStatus = "failure-build"
+	// PullFailureStatus failure pulling imge
+	PullFailureStatus = "failure-pull"
+	// CanceledStatus command canceled
+	CanceledStatus = "canceled"
+)
+
+var (
+	// FileNotFoundFailure failure for compose file not found
+	FileNotFoundFailure = FailureCategory{MetricsStatus: FileNotFoundFailureStatus, ExitCode: 14}
+	// ComposeParseFailure failure for composefile parse error
+	ComposeParseFailure = FailureCategory{MetricsStatus: ComposeParseFailureStatus, ExitCode: 15}
+	// CommandSyntaxFailure failure for command line syntax
+	CommandSyntaxFailure = FailureCategory{MetricsStatus: CommandSyntaxFailureStatus, ExitCode: 16}
+	//BuildFailure failure while building images.
+	BuildFailure = FailureCategory{MetricsStatus: BuildFailureStatus, ExitCode: 17}
+	// PullFailure failure while pulling image
+	PullFailure = FailureCategory{MetricsStatus: PullFailureStatus, ExitCode: 18}
+)

local/compose/build.go

@@ -24,21 +24,21 @@ import (
 	"path"
 	"strings"
 
-	moby "github.com/docker/docker/api/types"
-
-	"github.com/docker/compose-cli/api/compose"
-	composeprogress "github.com/docker/compose-cli/api/progress"
-	"github.com/docker/compose-cli/utils"
-
 	"github.com/compose-spec/compose-go/types"
 	"github.com/containerd/containerd/platforms"
 	"github.com/docker/buildx/build"
 	"github.com/docker/buildx/driver"
 	_ "github.com/docker/buildx/driver/docker" // required to get default driver registered
 	"github.com/docker/buildx/util/progress"
+	moby "github.com/docker/docker/api/types"
 	"github.com/docker/docker/errdefs"
 	bclient "github.com/moby/buildkit/client"
 	specs "github.com/opencontainers/image-spec/specs-go/v1"
+
+	"github.com/docker/compose-cli/api/compose"
+	composeprogress "github.com/docker/compose-cli/api/progress"
+	"github.com/docker/compose-cli/cli/metrics"
+	"github.com/docker/compose-cli/utils"
 )
 
 func (s *composeService) Build(ctx context.Context, project *types.Project, options compose.BuildOptions) error {
@@ -160,7 +160,8 @@ func (s *composeService) build(ctx context.Context, project *types.Project, opts
 	if info.OSType == "windows" {
 		// no support yet for Windows container builds in Buildkit
 		// https://docs.docker.com/develop/develop-images/build_enhancements/#limitations
-		return s.windowsBuild(opts, mode)
+		err := s.windowsBuild(opts, mode)
+		return metrics.WrapCategorisedComposeError(err, metrics.BuildFailure)
 	}
 	if len(opts) == 0 {
 		return nil
@@ -190,6 +191,9 @@ func (s *composeService) build(ctx context.Context, project *types.Project, opts
 	if err == nil {
 		err = errW
 	}
+	if err != nil {
+		return metrics.WrapCategorisedComposeError(err, metrics.BuildFailure)
+	}
 
 	cw := composeprogress.ContextWriter(ctx)
 	for _, c := range observedState {

local/compose/pull.go

@@ -36,6 +36,7 @@ import (
 	"github.com/docker/compose-cli/api/compose"
 	"github.com/docker/compose-cli/api/config"
 	"github.com/docker/compose-cli/api/progress"
+	"github.com/docker/compose-cli/cli/metrics"
 )
 
 func (s *composeService) Pull(ctx context.Context, project *types.Project, opts compose.PullOptions) error {
@@ -121,7 +122,7 @@ func (s *composeService) pullServiceImage(ctx context.Context, service types.Ser
 			Status: progress.Error,
 			Text:   "Error",
 		})
-		return err
+		return metrics.WrapCategorisedComposeError(err, metrics.PullFailure)
 	}
 
 	dec := json.NewDecoder(stream)
@@ -131,10 +132,10 @@ func (s *composeService) pullServiceImage(ctx context.Context, service types.Ser
 			if err == io.EOF {
 				break
 			}
-			return err
+			return metrics.WrapCategorisedComposeError(err, metrics.PullFailure)
 		}
 		if jm.Error != nil {
-			return errors.New(jm.Error.Message)
+			return metrics.WrapCategorisedComposeError(errors.New(jm.Error.Message), metrics.PullFailure)
 		}
 		toPullProgressEvent(service.Name, jm, w)
 	}

local/e2e/cli-only/e2e_test.go

@@ -182,7 +182,7 @@ func TestContextMetrics(t *testing.T) {
 		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"}`,
+			`{"command":"version","context":"moby","source":"cli","status":"failure-cmd-syntax"}`,
 		}, usage)
 	})
 
@@ -224,7 +224,6 @@ func TestContextDuplicateACI(t *testing.T) {
 }
 
 func TestContextRemove(t *testing.T) {
-
 	t.Run("remove current", func(t *testing.T) {
 		c := NewParallelE2eCLI(t, binDir)
 

local/e2e/compose/compose_run_test.go

@@ -95,9 +95,9 @@ func TestLocalComposeRun(t *testing.T) {
 	})
 
 	t.Run("compose run --publish", func(t *testing.T) {
-		c.RunDockerCmd("compose", "-f", "./fixtures/run-test/compose.yml", "run", "--publish", "8080:80", "-d", "back", "/bin/sh", "-c", "sleep 1")
+		c.RunDockerCmd("compose", "-f", "./fixtures/run-test/compose.yml", "run", "--publish", "8081:80", "-d", "back", "/bin/sh", "-c", "sleep 1")
 		res := c.RunDockerCmd("ps")
-		assert.Assert(t, strings.Contains(res.Stdout(), "8080->80/tcp"), res.Stdout())
+		assert.Assert(t, strings.Contains(res.Stdout(), "8081->80/tcp"), res.Stdout())
 	})
 
 	t.Run("down", func(t *testing.T) {

local/e2e/compose/fixtures/wrong-composefile/build-error.yml

@@ -0,0 +1,3 @@
+services:
+  simple:
+    build:  service1