Merge pull request #254 from deploymenttheory/dev-jl-logging

Logging Refactor - Moved to zap.SugaredLogger

Author: thejoeker12

concurrency/handler.go

@@ -5,13 +5,13 @@ import (
 	"sync"
 	"time"
 
-	"github.com/deploymenttheory/go-api-http-client/logger"
+	"go.uber.org/zap"
 )
 
 // ConcurrencyHandler controls the number of concurrent HTTP requests.
 type ConcurrencyHandler struct {
 	sem                      chan struct{}
-	logger                   logger.Logger
+	logger                   *zap.SugaredLogger
 	AcquisitionTimes         []time.Duration
 	lock                     sync.Mutex
 	lastTokenAcquisitionTime time.Time
@@ -55,7 +55,7 @@ type ConcurrencyMetrics struct {
 // concurrency limit, logger, and concurrency metrics. The ConcurrencyHandler ensures
 // no more than a certain number of concurrent requests are made.
 // It uses a semaphore to control concurrency.
-func NewConcurrencyHandler(limit int, logger logger.Logger, metrics *ConcurrencyMetrics) *ConcurrencyHandler {
+func NewConcurrencyHandler(limit int, logger *zap.SugaredLogger, metrics *ConcurrencyMetrics) *ConcurrencyHandler {
 	return &ConcurrencyHandler{
 		sem:              make(chan struct{}, limit),
 		logger:           logger,

httpclient/client.go

@@ -13,31 +13,44 @@ import (
 	"time"
 
 	"github.com/deploymenttheory/go-api-http-client/concurrency"
+	"go.uber.org/zap"
 
-	"github.com/deploymenttheory/go-api-http-client/logger"
 	"github.com/deploymenttheory/go-api-http-client/redirecthandler"
-	"go.uber.org/zap"
 )
 
+const ()
+
 // TODO all struct comments
 
 // Master struct/object
 type Client struct {
-	config ClientConfig
-	http   *http.Client
-
-	AuthToken       string
-	AuthTokenExpiry time.Time
-	Logger          logger.Logger
-	Concurrency     *concurrency.ConcurrencyHandler
-	Integration     *APIIntegration
+	// Config
+	config *ClientConfig
+
+	// Integration
+	Integration *APIIntegration
+
+	// Executor
+	http *http.Client
+
+	// Logger
+	Sugar *zap.SugaredLogger
+
+	// Concurrency Mananger
+	Concurrency *concurrency.ConcurrencyHandler
 }
 
 // Options/Variables for Client
 type ClientConfig struct {
 	// Interface which implements the APIIntegration patterns. Integration handles all server/endpoint specific configuration, auth and vars.
 	Integration APIIntegration
 
+	// TODO
+	Sugar *zap.SugaredLogger
+
+	// Wether or not empty values will be set or an error thrown for missing items.
+	PopulateDefaultValues bool
+
 	// HideSenitiveData controls if sensitive data will be visible in logs. Debug option which should be True in production use.
 	HideSensitiveData bool `json:"hide_sensitive_data"`
 
@@ -81,60 +94,61 @@ type ClientConfig struct {
 }
 
 // BuildClient creates a new HTTP client with the provided configuration.
-func BuildClient(config ClientConfig, populateDefaultValues bool, log logger.Logger) (*Client, error) {
-	err := validateClientConfig(config, populateDefaultValues)
+func (c *ClientConfig) Build() (*Client, error) {
+	if c.Sugar == nil {
+		zapLogger, err := zap.NewProduction()
+		if err != nil {
+			return nil, err
+		}
+
+		c.Sugar = zapLogger.Sugar()
+		c.Sugar.Info("No logger provided. Defaulting to Sugared Zap Production Logger")
+	}
+
+	c.Sugar.Debug("validating configuration")
+
+	err := c.validateClientConfig()
 	if err != nil {
 		return nil, fmt.Errorf("invalid configuration: %v", err)
 	}
-
-	log.Info(fmt.Sprintf("initializing new http client, auth: %s", config.Integration.GetFQDN()))
+	c.Sugar.Debug("configuration valid")
 
 	httpClient := &http.Client{
-		Timeout: config.CustomTimeout,
+		Timeout: c.CustomTimeout,
 	}
 
 	// TODO refactor redirects
-	if err := redirecthandler.SetupRedirectHandler(httpClient, config.FollowRedirects, config.MaxRedirects, log); err != nil {
+	if err := redirecthandler.SetupRedirectHandler(httpClient, c.FollowRedirects, c.MaxRedirects, c.Sugar); err != nil {
 		return nil, fmt.Errorf("Failed to set up redirect handler: %v", err)
 	}
 
+	// TODO refactor concurrency
 	var concurrencyHandler *concurrency.ConcurrencyHandler
-	if config.EnableConcurrencyManagement {
+	if c.EnableConcurrencyManagement {
 		concurrencyMetrics := &concurrency.ConcurrencyMetrics{}
 		concurrencyHandler = concurrency.NewConcurrencyHandler(
-			config.MaxConcurrentRequests,
-			log,
+			c.MaxConcurrentRequests,
+			c.Sugar,
 			concurrencyMetrics,
 		)
 	} else {
 		concurrencyHandler = nil
 	}
 
 	client := &Client{
-		Integration: &config.Integration,
+		Integration: &c.Integration,
 		http:        httpClient,
-		config:      config,
-		Logger:      log,
+		config:      c,
+		Sugar:       c.Sugar,
 		Concurrency: concurrencyHandler,
 	}
 
 	if len(client.config.CustomCookies) > 0 {
-		client.loadCustomCookies(config.CustomCookies)
+		client.Sugar.Debug("setting custom cookies")
+		client.loadCustomCookies()
 	}
 
-	log.Debug("New API client initialized",
-		zap.String("Authentication Method", (*client.Integration).GetAuthMethodDescriptor()),
-		zap.Bool("Hide Sensitive Data In Logs", config.HideSensitiveData),
-		zap.Int("Max Retry Attempts", config.MaxRetryAttempts),
-		zap.Bool("Enable Dynamic Rate Limiting", config.EnableDynamicRateLimiting),
-		zap.Int("Max Concurrent Requests", config.MaxConcurrentRequests),
-		zap.Bool("Follow Redirects", config.FollowRedirects),
-		zap.Int("Max Redirects", config.MaxRedirects),
-		zap.Duration("Token Refresh Buffer Period", config.TokenRefreshBufferPeriod),
-		zap.Duration("Total Retry Duration", config.TotalRetryDuration),
-		zap.Duration("Custom Timeout", config.CustomTimeout),
-		zap.Bool("Enable Concurrency Management", config.EnableConcurrencyManagement),
-	)
+	client.Sugar.Infof("client init complete: %+v", client)
 
 	return client, nil
 

httpclient/config_validation.go

@@ -55,7 +55,7 @@ func LoadConfigFromFile(filepath string) (*ClientConfig, error) {
 		return nil, fmt.Errorf("could not unmarshal JSON: %v", err)
 	}
 
-	SetDefaultValuesClientConfig(&config)
+	config.SetDefaultValuesClientConfig()
 
 	return &config, nil
 }
@@ -96,43 +96,43 @@ func LoadConfigFromEnv() (*ClientConfig, error) {
 }
 
 // TODO Review validateClientConfig
-func validateClientConfig(config ClientConfig, populateDefaults bool) error {
+func (c ClientConfig) validateClientConfig() error {
 
-	if populateDefaults {
-		SetDefaultValuesClientConfig(&config)
+	if c.PopulateDefaultValues {
+		c.SetDefaultValuesClientConfig()
 	}
 
 	// TODO adjust these strings to have links to documentation & centralise them
-	if config.Integration == nil {
+	if c.Integration == nil {
 		return errors.New("no http client api integration supplied, please see repo documentation for this client and go-api-http-client-integration and provide an implementation")
 	}
 
-	if config.EnableConcurrencyManagement {
-		if config.MaxConcurrentRequests < 1 {
+	if c.EnableConcurrencyManagement {
+		if c.MaxConcurrentRequests < 1 {
 			return errors.New("maximum concurrent requests cannot be less than 1")
 		}
 	}
 
-	if config.CustomTimeout.Seconds() < 0 {
+	if c.CustomTimeout.Seconds() < 0 {
 		return errors.New("timeout cannot be less than 0 seconds")
 	}
 
-	if config.TokenRefreshBufferPeriod.Seconds() < 0 {
+	if c.TokenRefreshBufferPeriod.Seconds() < 0 {
 		return errors.New("refresh buffer period cannot be less than 0 seconds")
 	}
 
-	if config.RetryEligiableRequests {
-		if config.TotalRetryDuration.Seconds() < 0 {
+	if c.RetryEligiableRequests {
+		if c.TotalRetryDuration.Seconds() < 0 {
 			return errors.New("total retry duration cannot be less than 0 seconds")
 		}
 
-		if config.MaxRetryAttempts < 0 {
+		if c.MaxRetryAttempts < 0 {
 			return errors.New("max retry cannot be less than 0")
 		}
 
 	}
 
-	if config.FollowRedirects {
+	if c.FollowRedirects {
 		if DefaultMaxRedirects < 1 {
 			return errors.New("max redirects cannot be less than 1")
 		}
@@ -142,15 +142,15 @@ func validateClientConfig(config ClientConfig, populateDefaults bool) error {
 }
 
 // SetDefaultValuesClientConfig sets default values for the client configuration. Ensuring that all fields have a valid or minimum value.
-func SetDefaultValuesClientConfig(config *ClientConfig) {
-	setDefaultBool(&config.HideSensitiveData, DefaultHideSensitiveData)
-	setDefaultInt(&config.MaxRetryAttempts, DefaultMaxRetryAttempts, 1)
-	setDefaultInt(&config.MaxConcurrentRequests, DefaultMaxConcurrentRequests, 1)
-	setDefaultBool(&config.EnableDynamicRateLimiting, DefaultEnableDynamicRateLimiting)
-	setDefaultDuration(&config.CustomTimeout, DefaultCustomTimeout)
-	setDefaultDuration(&config.TokenRefreshBufferPeriod, DefaultTokenRefreshBufferPeriod)
-	setDefaultDuration(&config.TotalRetryDuration, DefaultTotalRetryDuration)
-	setDefaultBool(&config.FollowRedirects, DefaultFollowRedirects)
-	setDefaultInt(&config.MaxRedirects, DefaultMaxRedirects, 0)
-	setDefaultBool(&config.EnableConcurrencyManagement, DefaultEnableConcurrencyManagement)
+func (c *ClientConfig) SetDefaultValuesClientConfig() {
+	setDefaultBool(&c.HideSensitiveData, DefaultHideSensitiveData)
+	setDefaultInt(&c.MaxRetryAttempts, DefaultMaxRetryAttempts, 1)
+	setDefaultInt(&c.MaxConcurrentRequests, DefaultMaxConcurrentRequests, 1)
+	setDefaultBool(&c.EnableDynamicRateLimiting, DefaultEnableDynamicRateLimiting)
+	setDefaultDuration(&c.CustomTimeout, DefaultCustomTimeout)
+	setDefaultDuration(&c.TokenRefreshBufferPeriod, DefaultTokenRefreshBufferPeriod)
+	setDefaultDuration(&c.TotalRetryDuration, DefaultTotalRetryDuration)
+	setDefaultBool(&c.FollowRedirects, DefaultFollowRedirects)
+	setDefaultInt(&c.MaxRedirects, DefaultMaxRedirects, 0)
+	setDefaultBool(&c.EnableConcurrencyManagement, DefaultEnableConcurrencyManagement)
 }

httpclient/cookies.go

@@ -1,27 +1,24 @@
 package httpclient
 
 import (
-	"fmt"
-	"net/http"
 	"net/http/cookiejar"
 	"net/url"
 )
 
-func (c *Client) loadCustomCookies(cookiesList []*http.Cookie) error {
+func (c *Client) loadCustomCookies() error {
 	cookieJar, err := cookiejar.New(nil)
 	if err != nil {
 		return err
 	}
+	c.http.Jar = cookieJar
 
 	cookieUrl, err := url.Parse((*c.Integration).GetFQDN())
-
 	if err != nil {
 		return err
 	}
 
-	c.http.Jar = cookieJar
-	c.http.Jar.SetCookies(cookieUrl, cookiesList)
-	c.Logger.Debug(fmt.Sprintf("%+v", c.http.Jar))
+	c.http.Jar.SetCookies(cookieUrl, c.config.CustomCookies)
+	c.Sugar.Debug("custom cookies set: %v", c.http.Jar.Cookies(cookieUrl))
 
 	return nil
 }

httpclient/headers.go

@@ -3,19 +3,12 @@ package httpclient
 
 import (
 	"net/http"
-
-	"github.com/deploymenttheory/go-api-http-client/logger"
-	"go.uber.org/zap"
 )
 
 // CheckDeprecationHeader checks the response headers for the Deprecation header and logs a warning if present.
-func CheckDeprecationHeader(resp *http.Response, log logger.Logger) {
+func (c *Client) CheckDeprecationHeader(resp *http.Response) {
 	deprecationHeader := resp.Header.Get("Deprecation")
 	if deprecationHeader != "" {
-
-		log.Warn("API endpoint is deprecated",
-			zap.String("Date", deprecationHeader),
-			zap.String("Endpoint", resp.Request.URL.String()),
-		)
+		c.Sugar.Warn("API endpoint is deprecated", deprecationHeader, resp.Request.URL.String())
 	}
 }