From 7807fb33cc4bf4253bcebab2caae492f79b2ade1 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Fri, 25 Sep 2020 16:26:48 +0200 Subject: [PATCH] API Metrics : send context type, not context name Signed-off-by: Guillaume Tardif --- api/client/client.go | 17 ++++++++++++++--- server/interceptor.go | 5 +---- server/metrics.go | 15 +++++++-------- server/metrics_test.go | 25 ++++++++++++++++++++----- server/proxy/proxy.go | 4 ++-- server/server.go | 2 +- 6 files changed, 45 insertions(+), 23 deletions(-) diff --git a/api/client/client.go b/api/client/client.go index a35c0b74d..9dfb2f627 100644 --- a/api/client/client.go +++ b/api/client/client.go @@ -44,10 +44,16 @@ func New(ctx context.Context) (*Client, error) { return nil, err } - return &Client{ - backendType: cc.Type(), + client := NewClient(cc.Type(), service) + return &client, nil +} + +// NewClient returns new client +func NewClient(backendType string, service backend.Service) Client { + return Client{ + backendType: backendType, bs: service, - }, nil + } } // GetCloudService returns a backend CloudService (typically login, create context) @@ -61,6 +67,11 @@ type Client struct { bs backend.Service } +// ContextType the context type associated with backend +func (c *Client) ContextType() string { + return c.backendType +} + // ContainerService returns the backend service for the current context func (c *Client) ContainerService() containers.Service { if cs := c.bs.ContainerService(); cs != nil { diff --git a/server/interceptor.go b/server/interceptor.go index 41072b344..17165a10e 100644 --- a/server/interceptor.go +++ b/server/interceptor.go @@ -111,10 +111,7 @@ func configureContext(ctx context.Context, currentContext string, method string) return nil, err } - ctx, err = proxy.WithClient(ctx, c) - if err != nil { - return nil, err - } + ctx = proxy.WithClient(ctx, c) } s, err := store.New(configDir) diff --git a/server/metrics.go b/server/metrics.go index 9c5c64f41..83ee48847 100644 --- a/server/metrics.go +++ b/server/metrics.go @@ -22,6 +22,7 @@ import ( "google.golang.org/grpc" "github.com/docker/compose-cli/metrics" + "github.com/docker/compose-cli/server/proxy" ) var ( @@ -41,14 +42,12 @@ var ( } ) -func metricsServerInterceptor(clictx context.Context, client metrics.Client) grpc.UnaryServerInterceptor { +func metricsServerInterceptor(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 { - currentContext, err = getConfigContext(clictx) - if err != nil { - return nil, err - } + backendClient := proxy.Client(ctx) + contextType := "" + if backendClient != nil { + contextType = backendClient.ContextType() } data, err := handler(ctx, req) @@ -61,7 +60,7 @@ func metricsServerInterceptor(clictx context.Context, client metrics.Client) grp if command != "" { client.Send(metrics.Command{ Command: command, - Context: currentContext, + Context: contextType, Source: metrics.APISource, Status: status, }) diff --git a/server/metrics_test.go b/server/metrics_test.go index ad061e15b..9f9e886fd 100644 --- a/server/metrics_test.go +++ b/server/metrics_test.go @@ -26,6 +26,11 @@ import ( "google.golang.org/grpc/metadata" "gotest.tools/v3/assert" + "github.com/docker/compose-cli/api/client" + "github.com/docker/compose-cli/api/compose" + "github.com/docker/compose-cli/api/containers" + "github.com/docker/compose-cli/api/secrets" + "github.com/docker/compose-cli/api/volumes" "github.com/docker/compose-cli/errdefs" "github.com/docker/compose-cli/metrics" containersv1 "github.com/docker/compose-cli/protos/containers/v1" @@ -55,18 +60,21 @@ 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) + newClient := client.NewClient("aci", noopService{}) + interceptor := metricsServerInterceptor(mockMetrics) - _, err := interceptor(incomingContext("aci"), nil, containerMethodRoute("List"), mockHandler(nil)) + ctx := proxy.WithClient(incomingContext("acicontext"), &newClient) + _, err := interceptor(ctx, 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) + newClient := client.NewClient("moby", noopService{}) + interceptor := metricsServerInterceptor(mockMetrics) - _, err := interceptor(incomingContext("default"), nil, containerMethodRoute("Create"), mockHandler(errdefs.ErrLoginRequired)) + ctx := proxy.WithClient(incomingContext("default"), &newClient) + _, err := interceptor(ctx, nil, containerMethodRoute("Create"), mockHandler(errdefs.ErrLoginRequired)) assert.Assert(t, err == errdefs.ErrLoginRequired) } @@ -100,6 +108,13 @@ func setupServer() *grpc.Server { return s } +type noopService struct{} + +func (noopService) ContainerService() containers.Service { return nil } +func (noopService) ComposeService() compose.Service { return nil } +func (noopService) SecretsService() secrets.Service { return nil } +func (noopService) VolumeService() volumes.Service { return nil } + type mockMetricsClient struct { mock.Mock } diff --git a/server/proxy/proxy.go b/server/proxy/proxy.go index 7a9f0f4e0..30733ba93 100644 --- a/server/proxy/proxy.go +++ b/server/proxy/proxy.go @@ -31,8 +31,8 @@ import ( type clientKey struct{} // WithClient adds the client to the context -func WithClient(ctx context.Context, c *client.Client) (context.Context, error) { - return context.WithValue(ctx, clientKey{}, c), nil +func WithClient(ctx context.Context, c *client.Client) context.Context { + return context.WithValue(ctx, clientKey{}, c) } // Client returns the client from the context diff --git a/server/server.go b/server/server.go index 0842afe8f..e5adfa53f 100644 --- a/server/server.go +++ b/server/server.go @@ -33,7 +33,7 @@ func New(ctx context.Context) *grpc.Server { s := grpc.NewServer( grpc.ChainUnaryInterceptor( unaryServerInterceptor(ctx), - metricsServerInterceptor(ctx, metrics.NewClient()), + metricsServerInterceptor(metrics.NewClient()), ), grpc.StreamInterceptor(streamServerInterceptor(ctx)), )