From a71b2a39bda3b92083f30e4ca4118360cec898c6 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Fri, 18 Sep 2020 10:56:24 +0200 Subject: [PATCH] Add status field in API metrics Signed-off-by: Guillaume Tardif --- aci/context.go | 7 +++--- cli/main.go | 6 ++--- cli/mobycli/exec.go | 3 +-- ecs/context.go | 5 ++--- errdefs/errors.go | 7 ++++++ metrics/client.go | 4 ++-- server/metrics.go | 14 +++++++----- server/metrics_test.go | 50 ++++++++++++++++++++++++++++++++++++++++++ server/server.go | 4 +++- 9 files changed, 79 insertions(+), 21 deletions(-) diff --git a/aci/context.go b/aci/context.go index 340abe0f..3ca25469 100644 --- a/aci/context.go +++ b/aci/context.go @@ -21,8 +21,6 @@ import ( "fmt" "os" - "github.com/docker/docker/errdefs" - "github.com/AlecAivazis/survey/v2/terminal" "github.com/Azure/azure-sdk-for-go/profiles/preview/preview/subscription/mgmt/subscription" "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources" @@ -30,6 +28,7 @@ import ( "github.com/pkg/errors" "github.com/docker/compose-cli/context/store" + "github.com/docker/compose-cli/errdefs" "github.com/docker/compose-cli/prompt" ) @@ -42,7 +41,7 @@ type ContextParams struct { } // ErrSubscriptionNotFound is returned when a required subscription is not found -var ErrSubscriptionNotFound = errors.New("subscription not found") +var ErrSubscriptionNotFound = errors.Wrapf(errdefs.ErrNotFound, "subscription") // IsSubscriptionNotFoundError returns true if the unwrapped error is IsSubscriptionNotFoundError func IsSubscriptionNotFoundError(err error) bool { @@ -140,7 +139,7 @@ func (helper contextCreateACIHelper) chooseGroup(ctx context.Context, subscripti group, err := helper.selector.Select("Select a resource group", groupNames) if err != nil { if err == terminal.InterruptErr { - return resources.Group{}, errdefs.Cancelled(err) + return resources.Group{}, errdefs.ErrCanceled } return resources.Group{}, err diff --git a/cli/main.go b/cli/main.go index 18fded24..83961575 100644 --- a/cli/main.go +++ b/cli/main.go @@ -31,8 +31,6 @@ import ( "github.com/sirupsen/logrus" "github.com/spf13/cobra" - dockererrdef "github.com/docker/docker/errdefs" - "github.com/docker/compose-cli/cli/cmd/compose" "github.com/docker/compose-cli/cli/cmd/logout" volume "github.com/docker/compose-cli/cli/cmd/volume" @@ -192,8 +190,8 @@ func main() { if err = root.ExecuteContext(ctx); err != nil { // if user canceled request, simply exit without any error message - if dockererrdef.IsCancelled(err) || errors.Is(ctx.Err(), context.Canceled) { - metrics.Track(ctype, os.Args[1:], root.PersistentFlags(), metrics.CancelledStatus) + if errdefs.IsErrCanceled(err) || errors.Is(ctx.Err(), context.Canceled) { + metrics.Track(ctype, os.Args[1:], root.PersistentFlags(), metrics.CanceledStatus) os.Exit(130) } if ctype == store.AwsContextType { diff --git a/cli/mobycli/exec.go b/cli/mobycli/exec.go index 8b22165b..99a832a9 100644 --- a/cli/mobycli/exec.go +++ b/cli/mobycli/exec.go @@ -26,10 +26,9 @@ import ( "github.com/spf13/cobra" - "github.com/docker/compose-cli/metrics" - apicontext "github.com/docker/compose-cli/context" "github.com/docker/compose-cli/context/store" + "github.com/docker/compose-cli/metrics" ) var delegatedContextTypes = []string{store.DefaultContextType} diff --git a/ecs/context.go b/ecs/context.go index 39448d98..b82f96ff 100644 --- a/ecs/context.go +++ b/ecs/context.go @@ -23,8 +23,6 @@ import ( "reflect" "strings" - "github.com/docker/docker/errdefs" - "github.com/AlecAivazis/survey/v2/terminal" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/aws/credentials" @@ -32,6 +30,7 @@ import ( "gopkg.in/ini.v1" "github.com/docker/compose-cli/context/store" + "github.com/docker/compose-cli/errdefs" "github.com/docker/compose-cli/prompt" ) @@ -157,7 +156,7 @@ func (h contextCreateAWSHelper) chooseProfile(section map[string]ini.Section) (s selected, err := h.user.Select("Select AWS Profile", profiles) if err != nil { if err == terminal.InterruptErr { - return "", errdefs.Cancelled(err) + return "", errdefs.ErrCanceled } return "", err } diff --git a/errdefs/errors.go b/errdefs/errors.go index db5141cc..159be0bc 100644 --- a/errdefs/errors.go +++ b/errdefs/errors.go @@ -42,6 +42,8 @@ var ( // ErrNotImplemented is returned when a backend doesn't implement // an action ErrNotImplemented = errors.New("not implemented") + // ErrCanceled is returned when the command was canceled by user + ErrCanceled = errors.New("canceled") // ErrParsingFailed is returned when a string cannot be parsed ErrParsingFailed = errors.New("parsing failed") // ErrWrongContextType is returned when the caller tries to get a context @@ -78,3 +80,8 @@ func IsErrNotImplemented(err error) bool { func IsErrParsingFailed(err error) bool { return errors.Is(err, ErrParsingFailed) } + +// IsErrCanceled returns true if the unwrapped error is ErrCanceled +func IsErrCanceled(err error) bool { + return errors.Is(err, ErrCanceled) +} diff --git a/metrics/client.go b/metrics/client.go index 470317c3..93e44073 100644 --- a/metrics/client.go +++ b/metrics/client.go @@ -45,8 +45,8 @@ const ( SuccessStatus = "success" // FailureStatus is sent for API metrics FailureStatus = "failure" - // CancelledStatus is sent for API metrics - CancelledStatus = "cancelled" + // CanceledStatus is sent for API metrics + CanceledStatus = "canceled" ) // Client sends metrics to Docker Desktopn diff --git a/server/metrics.go b/server/metrics.go index 98dd97e9..cf8edc89 100644 --- a/server/metrics.go +++ b/server/metrics.go @@ -41,9 +41,7 @@ var ( } ) -func metricsServerInterceptor(clictx context.Context) grpc.UnaryServerInterceptor { - client := metrics.NewClient() - +func metricsServerInterceptor(clictx context.Context, client metrics.Client) grpc.UnaryServerInterceptor { return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) { currentContext, err := getIncomingContext(ctx) if err != nil { @@ -53,15 +51,21 @@ func metricsServerInterceptor(clictx context.Context) grpc.UnaryServerIntercepto } } + data, err := handler(ctx, req) + + status := metrics.SuccessStatus + if err != nil { + status = metrics.FailureStatus + } command := methodMapping[info.FullMethod] if command != "" { client.Send(metrics.Command{ Command: command, Context: currentContext, Source: metrics.APISource, + Status: status, }) } - - return handler(ctx, req) + return data, err } } diff --git a/server/metrics_test.go b/server/metrics_test.go index 0eb725e2..002aa2c4 100644 --- a/server/metrics_test.go +++ b/server/metrics_test.go @@ -21,9 +21,13 @@ import ( "strings" "testing" + "github.com/stretchr/testify/mock" "google.golang.org/grpc" + "google.golang.org/grpc/metadata" "gotest.tools/v3/assert" + "github.com/docker/compose-cli/errdefs" + "github.com/docker/compose-cli/metrics" containersv1 "github.com/docker/compose-cli/protos/containers/v1" contextsv1 "github.com/docker/compose-cli/protos/contexts/v1" streamsv1 "github.com/docker/compose-cli/protos/streams/v1" @@ -48,6 +52,44 @@ func TestAllMethodsHaveCorrespondingCliCommand(t *testing.T) { } } +func TestTrackSuccess(t *testing.T) { + var mockMetrics = &mockMetricsClient{} + mockMetrics.On("Send", metrics.Command{Command: "ps", Context: "aci", Status: "success", Source: "api"}).Return() + interceptor := metricsServerInterceptor(context.TODO(), mockMetrics) + + _, err := interceptor(incomingContext("aci"), nil, containerMethodRoute("List"), mockHandler(nil)) + assert.NilError(t, err) +} + +func TestTrackSFailures(t *testing.T) { + var mockMetrics = &mockMetricsClient{} + mockMetrics.On("Send", metrics.Command{Command: "ps", Context: "default", Status: "failure", Source: "api"}).Return() + interceptor := metricsServerInterceptor(context.TODO(), mockMetrics) + + _, err := interceptor(incomingContext("default"), nil, containerMethodRoute("Create"), mockHandler(errdefs.ErrLoginRequired)) + assert.Assert(t, err == errdefs.ErrLoginRequired) +} + +func containerMethodRoute(action string) *grpc.UnaryServerInfo { + var info = &grpc.UnaryServerInfo{ + FullMethod: "/com.docker.api.protos.containers.v1.Containers/" + action, + } + return info +} + +func mockHandler(err error) func(ctx context.Context, req interface{}) (interface{}, error) { + return func(ctx context.Context, req interface{}) (interface{}, error) { + return nil, err + } +} + +func incomingContext(status string) context.Context { + ctx := metadata.NewIncomingContext(context.TODO(), metadata.MD{ + (key): []string{status}, + }) + return ctx +} + func setupServer() *grpc.Server { ctx := context.TODO() s := New(ctx) @@ -57,3 +99,11 @@ func setupServer() *grpc.Server { contextsv1.RegisterContextsServer(s, p.ContextsProxy()) return s } + +type mockMetricsClient struct { + mock.Mock +} + +func (s *mockMetricsClient) Send(command metrics.Command) { + s.Called(command) +} diff --git a/server/server.go b/server/server.go index a8c7d9c9..7d738029 100644 --- a/server/server.go +++ b/server/server.go @@ -24,6 +24,8 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/health" "google.golang.org/grpc/health/grpc_health_v1" + + "github.com/docker/compose-cli/metrics" ) // New returns a new GRPC server. @@ -31,7 +33,7 @@ func New(ctx context.Context) *grpc.Server { s := grpc.NewServer( grpc.ChainUnaryInterceptor( unaryServerInterceptor(ctx), - metricsServerInterceptor(ctx), + metricsServerInterceptor(ctx, metrics.NewClient()), ), grpc.StreamInterceptor(streamServerInterceptor(ctx)), )