Merge pull request #262 from xmidt-org/denopink/refactor/replace-logg…

…ing-util-with-sallust

Denopink/refactor/replace logging util with sallust

chrysom/basicClient.go

@@ -14,6 +14,7 @@ import (
 
 	"github.com/xmidt-org/ancla/model"
 	"github.com/xmidt-org/bascule/acquire"
+	"github.com/xmidt-org/sallust"
 	"go.uber.org/zap"
 )
 
@@ -66,7 +67,6 @@ type BasicClient struct {
 	auth         acquire.Acquirer
 	storeBaseURL string
 	bucket       string
-	getLogger    func(context.Context) *zap.Logger
 }
 
 // Auth contains authorization data for requests to Argus.
@@ -93,8 +93,7 @@ type Items []model.Item
 
 // NewBasicClient creates a new BasicClient that can be used to
 // make requests to Argus.
-func NewBasicClient(config BasicClientConfig,
-	getLogger func(context.Context) *zap.Logger) (*BasicClient, error) {
+func NewBasicClient(config BasicClientConfig) (*BasicClient, error) {
 	err := validateBasicConfig(&config)
 	if err != nil {
 		return nil, err
@@ -109,7 +108,6 @@ func NewBasicClient(config BasicClientConfig,
 		auth:         tokenAcquirer,
 		bucket:       config.Bucket,
 		storeBaseURL: config.Address + storeAPIPath,
-		getLogger:    getLogger,
 	}
 
 	return clientStore, nil
@@ -123,7 +121,7 @@ func (c *BasicClient) GetItems(ctx context.Context, owner string) (Items, error)
 	}
 
 	if response.Code != http.StatusOK {
-		c.getLogger(ctx).Error("Argus responded with non-200 response for GetItems request",
+		sallust.Get(ctx).Error("Argus responded with non-200 response for GetItems request",
 			zap.Int("code", response.Code), zap.String(errorHeaderKey, response.ArgusErrorHeader))
 		return nil, fmt.Errorf(errStatusCodeFmt, translateNonSuccessStatusCode(response.Code), response.Code)
 	}
@@ -164,7 +162,7 @@ func (c *BasicClient) PushItem(ctx context.Context, owner string, item model.Ite
 		return UpdatedPushResult, nil
 	}
 
-	c.getLogger(ctx).Error("Argus responded with a non-successful status code for a PushItem request",
+	sallust.Get(ctx).Error("Argus responded with a non-successful status code for a PushItem request",
 		zap.Int("code", response.Code), zap.String(errorHeaderKey, response.ArgusErrorHeader))
 
 	return NilPushResult, fmt.Errorf(errStatusCodeFmt, translateNonSuccessStatusCode(response.Code), response.Code)
@@ -182,7 +180,7 @@ func (c *BasicClient) RemoveItem(ctx context.Context, id, owner string) (model.I
 	}
 
 	if resp.Code != http.StatusOK {
-		c.getLogger(ctx).Error("Argus responded with a non-successful status code for a RemoveItem request",
+		sallust.Get(ctx).Error("Argus responded with a non-successful status code for a RemoveItem request",
 			zap.Int("code", resp.Code), zap.String(errorHeaderKey, resp.ArgusErrorHeader))
 		return model.Item{}, fmt.Errorf(errStatusCodeFmt, translateNonSuccessStatusCode(resp.Code), resp.Code)
 	}

chrysom/basicClient_test.go

@@ -19,7 +19,6 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/xmidt-org/ancla/model"
-	"github.com/xmidt-org/sallust"
 )
 
 const failingURL = "nowhere://"
@@ -161,7 +160,7 @@ func TestSendRequest(t *testing.T) {
 				HTTPClient: server.Client(),
 				Address:    "http://argus-hostname.io",
 				Bucket:     "bucket-name",
-			}, sallust.Get)
+			})
 
 			if tc.AcquirerFails {
 				client.auth = acquirerFunc(failAcquirer)
@@ -259,7 +258,7 @@ func TestGetItems(t *testing.T) {
 				HTTPClient: server.Client(),
 				Address:    server.URL,
 				Bucket:     bucket,
-			}, sallust.Get)
+			})
 
 			require.Nil(err)
 
@@ -394,7 +393,7 @@ func TestPushItem(t *testing.T) {
 				HTTPClient: server.Client(),
 				Address:    server.URL,
 				Bucket:     bucket,
-			}, sallust.Get)
+			})
 
 			if tc.ShouldMakeRequestFail {
 				client.auth = acquirerFunc(failAcquirer)
@@ -493,7 +492,7 @@ func TestRemoveItem(t *testing.T) {
 				HTTPClient: server.Client(),
 				Address:    server.URL,
 				Bucket:     bucket,
-			}, sallust.Get)
+			})
 
 			if tc.ShouldMakeRequestFail {
 				client.auth = acquirerFunc(failAcquirer)

handler.go

@@ -4,13 +4,11 @@
 package ancla
 
 import (
-	"context"
 	"net/http"
 	"time"
 
 	kithttp "github.com/go-kit/kit/transport/http"
 	webhook "github.com/xmidt-org/webhook-schema"
-	"go.uber.org/zap"
 )
 
 // NewAddWebhookHandler returns an HTTP handler for adding
@@ -20,7 +18,7 @@ func NewAddWebhookHandler(s Service, config HandlerConfig) http.Handler {
 		newAddWebhookEndpoint(s),
 		addWebhookRequestDecoder(newTransportConfig(config)),
 		encodeAddWebhookResponse,
-		kithttp.ServerErrorEncoder(errorEncoder(config.GetLogger)),
+		kithttp.ServerErrorEncoder(errorEncoder()),
 	)
 }
 
@@ -31,7 +29,7 @@ func NewGetAllWebhooksHandler(s Service, config HandlerConfig) http.Handler {
 		newGetAllWebhooksEndpoint(s),
 		kithttp.NopRequestDecoder,
 		encodeGetAllWebhooksResponse,
-		kithttp.ServerErrorEncoder(errorEncoder(config.GetLogger)),
+		kithttp.ServerErrorEncoder(errorEncoder()),
 	)
 }
 
@@ -40,7 +38,6 @@ func NewGetAllWebhooksHandler(s Service, config HandlerConfig) http.Handler {
 type HandlerConfig struct {
 	V                 webhook.Validators
 	DisablePartnerIDs bool
-	GetLogger         func(context.Context) *zap.Logger
 }
 
 func newTransportConfig(hConfig HandlerConfig) transportConfig {

service.go

@@ -74,12 +74,12 @@ type ClientService struct {
 }
 
 // NewService builds the Argus client service from the given configuration.
-func NewService(cfg Config, getLogger func(context.Context) *zap.Logger) (*ClientService, error) {
+func NewService(cfg Config) (*ClientService, error) {
 	if cfg.Logger == nil {
 		cfg.Logger = sallust.Default()
 	}
 	prepArgusBasicClientConfig(&cfg)
-	basic, err := chrysom.NewBasicClient(cfg.BasicClientConfig, getLogger)
+	basic, err := chrysom.NewBasicClient(cfg.BasicClientConfig)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create chrysom basic client: %v", err)
 	}
@@ -180,7 +180,7 @@ type ServiceIn struct {
 func ProvideService() fx.Option {
 	return fx.Provide(
 		func(in ServiceIn) (*ClientService, error) {
-			svc, err := NewService(in.Config, getLogger)
+			svc, err := NewService(in.Config)
 			if err != nil {
 				return nil, errors.Join(errFailedConfig, err)
 			}
@@ -206,7 +206,7 @@ func ProvideListener() fx.Option {
 	return fx.Options(
 		fx.Provide(
 			func(in ListenerIn) (err error) {
-				stopWatches, err := in.Svc.StartListener(in.listenerConfig, setLoggerInContext(), in.Measures, in.Watcher)
+				stopWatches, err := in.Svc.StartListener(in.listenerConfig, sallust.With, in.Measures, in.Watcher)
 				if err != nil {
 					return fmt.Errorf("webhook service start listener error: %v", err)
 				}
@@ -217,14 +217,3 @@ func ProvideListener() fx.Option {
 		),
 	)
 }
-
-func setLoggerInContext() func(context.Context, *zap.Logger) context.Context {
-	return func(parent context.Context, logger *zap.Logger) context.Context {
-		return sallust.With(parent, logger)
-	}
-}
-
-func getLogger(ctx context.Context) *zap.Logger {
-	logger := sallust.Get(ctx).With(zap.Time("ts", time.Now().UTC()), zap.Any("caller", zap.WithCaller(true)))
-	return logger
-}

service_test.go

@@ -14,14 +14,12 @@ import (
 	"github.com/xmidt-org/ancla/chrysom"
 	"github.com/xmidt-org/ancla/model"
 	"github.com/xmidt-org/sallust"
-	"go.uber.org/zap"
 )
 
 func TestNewService(t *testing.T) {
 	tcs := []struct {
 		desc        string
 		config      Config
-		getLogger   func(context.Context) *zap.Logger
 		expectedErr bool
 	}{
 		{
@@ -41,7 +39,7 @@ func TestNewService(t *testing.T) {
 	for _, tc := range tcs {
 		t.Run(tc.desc, func(t *testing.T) {
 			assert := assert.New(t)
-			_, err := NewService(tc.config, tc.getLogger)
+			_, err := NewService(tc.config)
 			if tc.expectedErr {
 				assert.NotNil(err)
 				return
@@ -58,7 +56,7 @@ func TestStartListener(t *testing.T) {
 			Bucket:  "test",
 		},
 	}
-	mockService, _ := NewService(mockServiceConfig, nil)
+	mockService, _ := NewService(mockServiceConfig)
 	tcs := []struct {
 		desc           string
 		serviceConfig  Config

transport.go

@@ -17,6 +17,7 @@ import (
 	"github.com/spf13/cast"
 	"github.com/xmidt-org/bascule/basculechecks"
 	"github.com/xmidt-org/httpaux/erraux"
+	"github.com/xmidt-org/sallust"
 	webhook "github.com/xmidt-org/webhook-schema"
 	"go.uber.org/zap"
 
@@ -236,7 +237,7 @@ func (wv webhookValidator) setV2Defaults(r *webhook.RegistrationV2) {
 	//TODO: need to get registrationV2 defaults
 }
 
-func errorEncoder(getLogger func(context.Context) *zap.Logger) kithttp.ErrorEncoder {
+func errorEncoder() kithttp.ErrorEncoder {
 	return func(ctx context.Context, err error, w http.ResponseWriter) {
 		w.Header().Set(contentTypeHeader, jsonContentType)
 		code := http.StatusInternalServerError
@@ -245,7 +246,7 @@ func errorEncoder(getLogger func(context.Context) *zap.Logger) kithttp.ErrorEnco
 			code = sc.StatusCode()
 		}
 
-		logger := getLogger(ctx)
+		logger := sallust.Get(ctx)
 		if logger != nil && code != http.StatusNotFound {
 			logger.Error("sending non-200, non-404 response", zap.Int("code", code), zap.Error(err))
 		}

transport_test.go

@@ -17,12 +17,11 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/xmidt-org/bascule"
-	"github.com/xmidt-org/sallust"
 	"github.com/xmidt-org/webhook-schema"
 )
 
 func TestErrorEncoder(t *testing.T) {
-	mockHandlerConfig := HandlerConfig{GetLogger: sallust.Get}
+	mockHandlerConfig := HandlerConfig{}
 
 	type testCase struct {
 		Description  string
@@ -48,7 +47,7 @@ func TestErrorEncoder(t *testing.T) {
 		t.Run(tc.Description, func(t *testing.T) {
 			assert := assert.New(t)
 			recorder := httptest.NewRecorder()
-			e := errorEncoder(tc.HConfig.GetLogger)
+			e := errorEncoder()
 			e(context.Background(), tc.InputErr, recorder)
 			assert.Equal(tc.ExpectedCode, recorder.Code)
 			assert.JSONEq(fmt.Sprintf(`{"message": "%s"}`, tc.InputErr.Error()), recorder.Body.String())