From 2570ebec867f99ba65ab76a8154ed32106430a75 Mon Sep 17 00:00:00 2001 From: Djordje Lukic Date: Thu, 27 Aug 2020 11:27:00 +0200 Subject: [PATCH 1/3] Add interceptor for API metrics, ensure registered methods have a corresponding method set for metrics Signed-off-by: Djordje Lukic --- metrics/client.go | 8 +++++ metrics/metrics.go | 1 + server/metrics.go | 67 ++++++++++++++++++++++++++++++++++++++++++ server/metrics_test.go | 60 +++++++++++++++++++++++++++++++++++++ server/server.go | 5 +++- 5 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 server/metrics.go create mode 100644 server/metrics_test.go diff --git a/metrics/client.go b/metrics/client.go index fa62070d..3a603a0f 100644 --- a/metrics/client.go +++ b/metrics/client.go @@ -32,8 +32,16 @@ type client struct { type Command struct { Command string `json:"command"` Context string `json:"context"` + Source string `json:"source"` } +const ( + // CLISource is sent for cli metrics + CLISource = "cli" + // APISource is sent for API metrics + APISource = "api" +) + // Client sends metrics to Docker Desktopn type Client interface { // Send sends the command to Docker Desktop. Note that the function doesn't diff --git a/metrics/metrics.go b/metrics/metrics.go index 2389541f..4a0dd140 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -95,6 +95,7 @@ func Track(context string, args []string, flags *flag.FlagSet) { c.Send(Command{ Command: command, Context: context, + Source: CLISource, }) } }() diff --git a/server/metrics.go b/server/metrics.go new file mode 100644 index 00000000..98dd97e9 --- /dev/null +++ b/server/metrics.go @@ -0,0 +1,67 @@ +/* + Copyright 2020 Docker, Inc. + + 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 server + +import ( + "context" + + "google.golang.org/grpc" + + "github.com/docker/compose-cli/metrics" +) + +var ( + methodMapping = map[string]string{ + "/com.docker.api.protos.containers.v1.Containers/List": "ps", + "/com.docker.api.protos.containers.v1.Containers/Start": "start", + "/com.docker.api.protos.containers.v1.Containers/Stop": "stop", + "/com.docker.api.protos.containers.v1.Containers/Run": "run", + "/com.docker.api.protos.containers.v1.Containers/Exec": "exec", + "/com.docker.api.protos.containers.v1.Containers/Delete": "rm", + "/com.docker.api.protos.containers.v1.Containers/Kill": "kill", + "/com.docker.api.protos.containers.v1.Containers/Inspect": "inspect", + "/com.docker.api.protos.containers.v1.Containers/Logs": "logs", + "/com.docker.api.protos.streams.v1.Streaming/NewStream": "", + "/com.docker.api.protos.context.v1.Contexts/List": "context ls", + "/com.docker.api.protos.context.v1.Contexts/SetCurrent": "context use", + } +) + +func metricsServerInterceptor(clictx context.Context) grpc.UnaryServerInterceptor { + client := metrics.NewClient() + + return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) { + currentContext, err := getIncomingContext(ctx) + if err != nil { + currentContext, err = getConfigContext(clictx) + if err != nil { + return nil, err + } + } + + command := methodMapping[info.FullMethod] + if command != "" { + client.Send(metrics.Command{ + Command: command, + Context: currentContext, + Source: metrics.APISource, + }) + } + + return handler(ctx, req) + } +} diff --git a/server/metrics_test.go b/server/metrics_test.go new file mode 100644 index 00000000..2a7ffdc3 --- /dev/null +++ b/server/metrics_test.go @@ -0,0 +1,60 @@ +/* + Copyright 2020 Docker, Inc. + + 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 server + +import ( + "context" + "strings" + "testing" + + "gotest.tools/v3/assert" + + "google.golang.org/grpc" + + 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" + "github.com/docker/compose-cli/server/proxy" +) + +func TestAllMethodsHaveCorrespondingCliCommand(t *testing.T) { + s := setupServer() + i := s.GetServiceInfo() + for k, v := range i { + if k == "grpc.health.v1.Health" { + continue + } + var errs []string + for _, m := range v.Methods { + name := "/" + k + "/" + m.Name + if _, keyExists := methodMapping[name]; !keyExists { + errs = append(errs, name+" not mapped to a corresponding cli command") + } + } + assert.Equal(t, "", strings.Join(errs, "\n")) + } +} + +func setupServer() *grpc.Server { + ctx := context.TODO() + s := New(ctx) + p := proxy.New(ctx) + containersv1.RegisterContainersServer(s, p) + streamsv1.RegisterStreamingServer(s, p) + contextsv1.RegisterContextsServer(s, p.ContextsProxy()) + return s +} diff --git a/server/server.go b/server/server.go index 63a736ae..a8c7d9c9 100644 --- a/server/server.go +++ b/server/server.go @@ -29,7 +29,10 @@ import ( // New returns a new GRPC server. func New(ctx context.Context) *grpc.Server { s := grpc.NewServer( - grpc.UnaryInterceptor(unaryServerInterceptor(ctx)), + grpc.ChainUnaryInterceptor( + unaryServerInterceptor(ctx), + metricsServerInterceptor(ctx), + ), grpc.StreamInterceptor(streamServerInterceptor(ctx)), ) hs := health.NewServer() From e56061d27c3dcf4e5cc662cf52e5a276cc6a287b Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Tue, 15 Sep 2020 17:28:23 +0200 Subject: [PATCH 2/3] Move fire and forget code from metrics.Track() (used only by CLI) to metrics.Send (used by both CLI and API) Signed-off-by: Guillaume Tardif --- metrics/client.go | 24 +++++++++++++++++++----- metrics/metrics.go | 31 +++++++++---------------------- 2 files changed, 28 insertions(+), 27 deletions(-) diff --git a/metrics/client.go b/metrics/client.go index 3a603a0f..a6947e0f 100644 --- a/metrics/client.go +++ b/metrics/client.go @@ -64,10 +64,24 @@ func NewClient() Client { } func (c *client) Send(command Command) { - req, err := json.Marshal(command) - if err != nil { - return - } + 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. + 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)) + }() + <-wasIn - _, _ = c.httpClient.Post("http://localhost/usage", "application/json", bytes.NewBuffer(req)) } diff --git a/metrics/metrics.go b/metrics/metrics.go index 4a0dd140..333b0f0d 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -78,28 +78,15 @@ const ( // Track sends the tracking analytics to Docker Desktop func Track(context string, args []string, flags *flag.FlagSet) { - 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. - go func() { - defer func() { - _ = recover() - }() - - wasIn <- true - - command := getCommand(args, flags) - if command != "" { - c := NewClient() - c.Send(Command{ - Command: command, - Context: context, - Source: CLISource, - }) - } - }() - <-wasIn + command := getCommand(args, flags) + if command != "" { + c := NewClient() + c.Send(Command{ + Command: command, + Context: context, + Source: CLISource, + }) + } } func getCommand(args []string, flags *flag.FlagSet) string { From 3bf6a00a61cda8700b63430d06f170321b77322c Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Thu, 17 Sep 2020 10:58:01 +0200 Subject: [PATCH 3/3] goimports Signed-off-by: Guillaume Tardif --- aci/convert/container.go | 1 + cli/main.go | 7 ++++--- ecs/backend.go | 1 + ecs/local/backend.go | 3 ++- server/metrics_test.go | 3 +-- 5 files changed, 9 insertions(+), 6 deletions(-) diff --git a/aci/convert/container.go b/aci/convert/container.go index 131994ca..3cff17c4 100644 --- a/aci/convert/container.go +++ b/aci/convert/container.go @@ -21,6 +21,7 @@ import ( "strings" "github.com/compose-spec/compose-go/types" + "github.com/docker/compose-cli/api/containers" ) diff --git a/cli/main.go b/cli/main.go index a4eadd1b..a495d53e 100644 --- a/cli/main.go +++ b/cli/main.go @@ -27,13 +27,14 @@ import ( "syscall" "time" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" + "github.com/spf13/cobra" + "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" "github.com/docker/compose-cli/errdefs" - "github.com/pkg/errors" - "github.com/sirupsen/logrus" - "github.com/spf13/cobra" // Backend registrations _ "github.com/docker/compose-cli/aci" diff --git a/ecs/backend.go b/ecs/backend.go index 8cc0a392..7f7f269f 100644 --- a/ecs/backend.go +++ b/ecs/backend.go @@ -21,6 +21,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/session" + "github.com/docker/compose-cli/api/compose" "github.com/docker/compose-cli/api/containers" "github.com/docker/compose-cli/api/secrets" diff --git a/ecs/local/backend.go b/ecs/local/backend.go index c62ed640..2f793588 100644 --- a/ecs/local/backend.go +++ b/ecs/local/backend.go @@ -19,6 +19,8 @@ package local import ( "context" + "github.com/docker/docker/client" + "github.com/docker/compose-cli/api/compose" "github.com/docker/compose-cli/api/containers" "github.com/docker/compose-cli/api/secrets" @@ -26,7 +28,6 @@ import ( "github.com/docker/compose-cli/backend" "github.com/docker/compose-cli/context/cloud" "github.com/docker/compose-cli/context/store" - "github.com/docker/docker/client" ) const backendType = store.EcsLocalSimulationContextType diff --git a/server/metrics_test.go b/server/metrics_test.go index 2a7ffdc3..0eb725e2 100644 --- a/server/metrics_test.go +++ b/server/metrics_test.go @@ -21,9 +21,8 @@ import ( "strings" "testing" - "gotest.tools/v3/assert" - "google.golang.org/grpc" + "gotest.tools/v3/assert" containersv1 "github.com/docker/compose-cli/protos/containers/v1" contextsv1 "github.com/docker/compose-cli/protos/contexts/v1"