From 3ccc6034617a05e95b1ca045c81ac434bf07d567 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Wed, 16 Sep 2020 17:22:21 +0200 Subject: [PATCH 1/3] Add status field in CLI metrics : success, failure, cancelled Signed-off-by: Guillaume Tardif --- aci/context.go | 4 +++- cli/cmd/context/create.go | 2 +- cli/cmd/context/inspect.go | 2 +- cli/cmd/context/ls.go | 2 +- cli/cmd/login/login.go | 2 +- cli/cmd/logout/logout.go | 2 +- cli/cmd/version.go | 2 +- cli/main.go | 35 ++++++++++++++++++++--------------- cli/mobycli/exec.go | 16 ++++++++++++---- ecs/context.go | 4 +++- metrics/client.go | 8 +++++++- metrics/metrics.go | 3 ++- 12 files changed, 53 insertions(+), 29 deletions(-) diff --git a/aci/context.go b/aci/context.go index 512cd848a..340abe0fb 100644 --- a/aci/context.go +++ b/aci/context.go @@ -21,6 +21,8 @@ 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" @@ -138,7 +140,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 { - os.Exit(0) + return resources.Group{}, errdefs.Cancelled(err) } return resources.Group{}, err diff --git a/cli/cmd/context/create.go b/cli/cmd/context/create.go index 5723e16be..e1ebdba1f 100644 --- a/cli/cmd/context/create.go +++ b/cli/cmd/context/create.go @@ -70,7 +70,7 @@ $ docker context create my-context --description "some description" --docker "ho Use: "create CONTEXT", Short: "Create new context", RunE: func(cmd *cobra.Command, args []string) error { - mobycli.Exec() + mobycli.Exec(cmd.Root()) return nil }, Long: longHelp, diff --git a/cli/cmd/context/inspect.go b/cli/cmd/context/inspect.go index 0a939c654..4e7ce4856 100644 --- a/cli/cmd/context/inspect.go +++ b/cli/cmd/context/inspect.go @@ -27,7 +27,7 @@ func inspectCommand() *cobra.Command { Use: "inspect", Short: "Display detailed information on one or more contexts", RunE: func(cmd *cobra.Command, args []string) error { - mobycli.Exec() + mobycli.Exec(cmd.Root()) return nil }, } diff --git a/cli/cmd/context/ls.go b/cli/cmd/context/ls.go index 754957ea8..0c7d43665 100644 --- a/cli/cmd/context/ls.go +++ b/cli/cmd/context/ls.go @@ -69,7 +69,7 @@ func runList(cmd *cobra.Command, opts lsOpts) error { return err } if opts.format != "" { - mobycli.Exec() + mobycli.Exec(cmd.Root()) return nil } diff --git a/cli/cmd/login/login.go b/cli/cmd/login/login.go index 11ca6eeb2..f9a6247d8 100644 --- a/cli/cmd/login/login.go +++ b/cli/cmd/login/login.go @@ -56,7 +56,7 @@ func runLogin(cmd *cobra.Command, args []string) error { backend := args[0] return errors.New("unknown backend type for cloud login: " + backend) } - mobycli.Exec() + mobycli.Exec(cmd.Root()) return nil } diff --git a/cli/cmd/logout/logout.go b/cli/cmd/logout/logout.go index 1f235a77c..8a92e1b51 100644 --- a/cli/cmd/logout/logout.go +++ b/cli/cmd/logout/logout.go @@ -37,6 +37,6 @@ func Command() *cobra.Command { } func runLogout(cmd *cobra.Command, args []string) error { - mobycli.Exec() + mobycli.Exec(cmd.Root()) return nil } diff --git a/cli/cmd/version.go b/cli/cmd/version.go index 6b0facedb..3075970c2 100644 --- a/cli/cmd/version.go +++ b/cli/cmd/version.go @@ -51,7 +51,7 @@ func runVersion(cmd *cobra.Command, version string) error { // we don't want to fail on error, there is an error if the engine is not available but it displays client version info // Still, technically the [] byte versionResult could be nil, just let the original command display what it has to display if versionResult == nil { - mobycli.Exec() + mobycli.Exec(cmd.Root()) return nil } var s string = string(versionResult) diff --git a/cli/main.go b/cli/main.go index a7a2bcd7b..18fded249 100644 --- a/cli/main.go +++ b/cli/main.go @@ -31,6 +31,8 @@ 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" @@ -102,7 +104,7 @@ func main() { SilenceUsage: true, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { if !isContextAgnosticCommand(cmd) { - mobycli.ExecIfDefaultCtxType(cmd.Context()) + mobycli.ExecIfDefaultCtxType(cmd.Context(), cmd.Root()) } return nil }, @@ -136,7 +138,7 @@ func main() { helpFunc := root.HelpFunc() root.SetHelpFunc(func(cmd *cobra.Command, args []string) { if !isContextAgnosticCommand(cmd) { - mobycli.ExecIfDefaultCtxType(cmd.Context()) + mobycli.ExecIfDefaultCtxType(cmd.Context(), cmd.Root()) } helpFunc(cmd, args) }) @@ -158,7 +160,7 @@ func main() { // --host and --version should immediately be forwarded to the original cli if opts.Host != "" || opts.Version { - mobycli.Exec() + mobycli.Exec(root) } if opts.Config == "" { @@ -171,7 +173,7 @@ func main() { s, err := store.New(configDir) if err != nil { - mobycli.Exec() + mobycli.Exec(root) } ctype := store.DefaultContextType @@ -185,41 +187,43 @@ func main() { root.AddCommand(volume.ACICommand()) } - metrics.Track(ctype, os.Args[1:], root.PersistentFlags()) - ctx = apicontext.WithCurrentContext(ctx, currentContext) ctx = store.WithContextStore(ctx, s) if err = root.ExecuteContext(ctx); err != nil { // if user canceled request, simply exit without any error message - if errors.Is(ctx.Err(), context.Canceled) { + if dockererrdef.IsCancelled(err) || errors.Is(ctx.Err(), context.Canceled) { + metrics.Track(ctype, os.Args[1:], root.PersistentFlags(), metrics.CancelledStatus) os.Exit(130) } if ctype == store.AwsContextType { exit(root, currentContext, errors.Errorf(`%q context type has been renamed. Recreate the context by running: -$ docker context create %s `, cc.Type(), store.EcsContextType)) +$ 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) + exit(root, currentContext, err, ctype) } - mobycli.ExecIfDefaultCtxType(ctx) + mobycli.ExecIfDefaultCtxType(ctx, root) - checkIfUnknownCommandExistInDefaultContext(err, currentContext) + checkIfUnknownCommandExistInDefaultContext(err, currentContext, root) - exit(root, currentContext, err) + exit(root, currentContext, err, ctype) } + metrics.Track(ctype, os.Args[1:], root.PersistentFlags(), metrics.SuccessStatus) } -func exit(cmd *cobra.Command, ctx string, err error) { +func exit(root *cobra.Command, ctx string, err error, ctype string) { + metrics.Track(ctype, os.Args[1:], root.PersistentFlags(), metrics.FailureStatus) + if errors.Is(err, errdefs.ErrLoginRequired) { fmt.Fprintln(os.Stderr, err) os.Exit(errdefs.ExitCodeLoginRequired) } if errors.Is(err, errdefs.ErrNotImplemented) { - cmd, _, _ := cmd.Traverse(os.Args[1:]) + cmd, _, _ := root.Traverse(os.Args[1:]) name := cmd.Name() parent := cmd.Parent() if parent != nil && parent.Parent() != nil { @@ -237,13 +241,14 @@ func fatal(err error) { os.Exit(1) } -func checkIfUnknownCommandExistInDefaultContext(err error, currentContext string) { +func checkIfUnknownCommandExistInDefaultContext(err error, currentContext string, root *cobra.Command) { 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(currentContext, os.Args[1:], root.PersistentFlags(), metrics.FailureStatus) os.Exit(1) } } diff --git a/cli/mobycli/exec.go b/cli/mobycli/exec.go index 8e6128076..8b22165ba 100644 --- a/cli/mobycli/exec.go +++ b/cli/mobycli/exec.go @@ -24,6 +24,10 @@ import ( "os/signal" "strings" + "github.com/spf13/cobra" + + "github.com/docker/compose-cli/metrics" + apicontext "github.com/docker/compose-cli/context" "github.com/docker/compose-cli/context/store" ) @@ -33,8 +37,8 @@ var delegatedContextTypes = []string{store.DefaultContextType} // ComDockerCli name of the classic cli binary const ComDockerCli = "com.docker.cli" -// ExecIfDefaultCtxType delegates to com.docker.cli if on moby or AWS context (until there is an AWS backend) -func ExecIfDefaultCtxType(ctx context.Context) { +// ExecIfDefaultCtxType delegates to com.docker.cli if on moby context +func ExecIfDefaultCtxType(ctx context.Context, root *cobra.Command) { currentContext := apicontext.CurrentContext(ctx) s := store.ContextStore(ctx) @@ -42,7 +46,7 @@ func ExecIfDefaultCtxType(ctx context.Context) { currentCtx, err := s.Get(currentContext) // Only run original docker command if the current context is not ours. if err != nil || mustDelegateToMoby(currentCtx.Type()) { - Exec() + Exec(root) } } @@ -56,7 +60,7 @@ func mustDelegateToMoby(ctxType string) bool { } // Exec delegates to com.docker.cli if on moby context -func Exec() { +func Exec(root *cobra.Command) { cmd := exec.Command(ComDockerCli, os.Args[1:]...) cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout @@ -83,12 +87,16 @@ func Exec() { err := cmd.Run() childExit <- true if err != nil { + metrics.Track(store.DefaultContextName, os.Args[1:], root.PersistentFlags(), metrics.FailureStatus) + if exiterr, ok := err.(*exec.ExitError); ok { os.Exit(exiterr.ExitCode()) } fmt.Fprintln(os.Stderr, err) os.Exit(1) } + metrics.Track(store.DefaultContextName, os.Args[1:], root.PersistentFlags(), metrics.SuccessStatus) + os.Exit(0) } diff --git a/ecs/context.go b/ecs/context.go index 13e39014b..39448d983 100644 --- a/ecs/context.go +++ b/ecs/context.go @@ -23,6 +23,8 @@ 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" @@ -155,7 +157,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 { - os.Exit(-1) + return "", errdefs.Cancelled(err) } return "", err } diff --git a/metrics/client.go b/metrics/client.go index a6947e0f9..470317c3c 100644 --- a/metrics/client.go +++ b/metrics/client.go @@ -33,6 +33,7 @@ type Command struct { Command string `json:"command"` Context string `json:"context"` Source string `json:"source"` + Status string `json:"status"` } const ( @@ -40,6 +41,12 @@ const ( CLISource = "cli" // APISource is sent for API metrics APISource = "api" + // SuccessStatus is sent for API metrics + SuccessStatus = "success" + // FailureStatus is sent for API metrics + FailureStatus = "failure" + // CancelledStatus is sent for API metrics + CancelledStatus = "cancelled" ) // Client sends metrics to Docker Desktopn @@ -83,5 +90,4 @@ func (c *client) Send(command Command) { _, _ = c.httpClient.Post("http://localhost/usage", "application/json", bytes.NewBuffer(req)) }() <-wasIn - } diff --git a/metrics/metrics.go b/metrics/metrics.go index 333b0f0d4..9ab3d1db3 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -77,7 +77,7 @@ const ( ) // Track sends the tracking analytics to Docker Desktop -func Track(context string, args []string, flags *flag.FlagSet) { +func Track(context string, args []string, flags *flag.FlagSet, status string) { command := getCommand(args, flags) if command != "" { c := NewClient() @@ -85,6 +85,7 @@ func Track(context string, args []string, flags *flag.FlagSet) { Command: command, Context: context, Source: CLISource, + Status: status, }) } } From a71b2a39bda3b92083f30e4ca4118360cec898c6 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Fri, 18 Sep 2020 10:56:24 +0200 Subject: [PATCH 2/3] 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 340abe0fb..3ca254697 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 18fded249..83961575b 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 8b22165ba..99a832a9c 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 39448d983..b82f96ff6 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 db5141cc5..159be0bcf 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 470317c3c..93e440735 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 98dd97e9b..cf8edc891 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 0eb725e2a..002aa2c49 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 a8c7d9c93..7d7380299 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)), ) From 6f19bbfd5ec4f8ab50e85b2908475830201f71e9 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Fri, 18 Sep 2020 15:57:02 +0200 Subject: [PATCH 3/3] =?UTF-8?q?We=20can=E2=80=99t=20anymore=20=E2=80=9Cfir?= =?UTF-8?q?e=20and=20forget=E2=80=9D,=20now=20that=20metrics=20get=20poste?= =?UTF-8?q?d=20right=20at=20the=20end,=20most=20of=20the=20time=20we?= =?UTF-8?q?=E2=80=99d=20loose=20them.=20Give=20it=20max=2050=20ms=20to=20p?= =?UTF-8?q?ost=20metrics,=20that=E2=80=99s=20plenty,=20post=20call=20ends?= =?UTF-8?q?=20in=20~2=20ms=20or=20less=20when=20desktop=20is=20up,=20less?= =?UTF-8?q?=20than=20one=20ms=20to=20fail=20the=20post=20when=20DD=20is=20?= =?UTF-8?q?not=20listening.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Guillaume Tardif --- metrics/client.go | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/metrics/client.go b/metrics/client.go index 93e440735..509599a16 100644 --- a/metrics/client.go +++ b/metrics/client.go @@ -22,6 +22,7 @@ import ( "encoding/json" "net" "net/http" + "time" ) type client struct { @@ -71,23 +72,23 @@ func NewClient() Client { } func (c *client) Send(command Command) { - wasIn := make(chan bool) - - // Fire and forget, we don't want to slow down the user waiting for DD - // metrics endpoint to respond. We could lose some events but that's ok. + result := make(chan bool, 1) go func() { - defer func() { - _ = recover() - }() - - wasIn <- true - - req, err := json.Marshal(command) - if err != nil { - return - } - - _, _ = c.httpClient.Post("http://localhost/usage", "application/json", bytes.NewBuffer(req)) + postMetrics(command, c) + result <- true }() - <-wasIn + + // wait for the post finished, or timeout in case anything freezes. + // Posting metrics without Desktop listening returns in less than a ms, and a handful of ms (often <2ms) when Desktop is listening + select { + case <-result: + case <-time.After(50 * time.Millisecond): + } +} + +func postMetrics(command Command, c *client) { + req, err := json.Marshal(command) + if err == nil { + _, _ = c.httpClient.Post("http://localhost/usage", "application/json", bytes.NewBuffer(req)) + } }