From 6ee2407bfd11f9588b253a9e19851fd5edc6cda5 Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Wed, 18 Mar 2020 16:04:51 -0700 Subject: [PATCH 01/28] refactor to simply include logger via sdk key --- pkg/client/client.go | 77 +++++++++---------- pkg/client/factory.go | 11 ++- pkg/config/datafileprojectconfig/config.go | 6 +- pkg/config/polling_manager.go | 38 ++++----- pkg/config/static_manager.go | 19 +++-- pkg/decision/bucketer/murmurhashbucketer.go | 9 ++- .../bucketer/murmurhashbucketer_test.go | 2 +- pkg/decision/composite_experiment_service.go | 19 ++--- pkg/decision/composite_feature_service.go | 12 +-- pkg/decision/composite_service.go | 18 ++--- pkg/decision/experiment_bucketer_service.go | 13 ++-- pkg/decision/experiment_override_service.go | 8 +- pkg/decision/feature_experiment_service.go | 8 +- pkg/decision/persisting_experiment_service.go | 12 +-- pkg/decision/rollout_service.go | 12 +-- pkg/event/dispatcher.go | 24 +++--- pkg/event/factory.go | 6 +- pkg/event/processor.go | 34 ++++---- pkg/logging/logger.go | 4 +- pkg/notification/center.go | 13 ++-- pkg/notification/manager.go | 11 ++- pkg/utils/execgroup.go | 7 +- pkg/utils/requester.go | 24 +++--- 23 files changed, 196 insertions(+), 191 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index d4621f955..b52ae42b2 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -33,8 +33,6 @@ import ( "github.com/optimizely/go-sdk/pkg/utils" ) -var logger = logging.GetLogger("Client") - // OptimizelyClient is the entry point to the Optimizely SDK type OptimizelyClient struct { ConfigManager config.ProjectConfigManager @@ -42,6 +40,7 @@ type OptimizelyClient struct { EventProcessor event.Processor notificationCenter notification.Center execGroup *utils.ExecGroup + logger logging.OptimizelyLogProducer } // Activate returns the key of the variation the user is bucketed into and queues up an impression event to be sent to @@ -59,14 +58,14 @@ func (o *OptimizelyClient) Activate(experimentKey string, userContext entities.U err = errors.New("unexpected error") } errorMessage := fmt.Sprintf("Activate call, optimizely SDK is panicking with the error:") - logger.Error(errorMessage, err) - logger.Debug(string(debug.Stack())) + o.logger.Error(errorMessage, err) + o.logger.Debug(string(debug.Stack())) } }() decisionContext, experimentDecision, err := o.getExperimentDecision(experimentKey, userContext) if err != nil { - logger.Error("received an error while computing experiment decision", err) + o.logger.Error("received an error while computing experiment decision", err) return result, err } @@ -95,14 +94,14 @@ func (o *OptimizelyClient) IsFeatureEnabled(featureKey string, userContext entit err = errors.New("unexpected error") } errorMessage := fmt.Sprintf("IsFeatureEnabled call, optimizely SDK is panicking with the error:") - logger.Error(errorMessage, err) - logger.Debug(string(debug.Stack())) + o.logger.Error(errorMessage, err) + o.logger.Debug(string(debug.Stack())) } }() decisionContext, featureDecision, err := o.getFeatureDecision(featureKey, "", userContext) if err != nil { - logger.Error("received an error while computing feature decision", err) + o.logger.Error("received an error while computing feature decision", err) return result, err } @@ -113,9 +112,9 @@ func (o *OptimizelyClient) IsFeatureEnabled(featureKey string, userContext entit } if result { - logger.Info(fmt.Sprintf(`Feature "%s" is enabled for user "%s".`, featureKey, userContext.ID)) + o.logger.Info(fmt.Sprintf(`Feature "%s" is enabled for user "%s".`, featureKey, userContext.ID)) } else { - logger.Info(fmt.Sprintf(`Feature "%s" is not enabled for user "%s".`, featureKey, userContext.ID)) + o.logger.Info(fmt.Sprintf(`Feature "%s" is not enabled for user "%s".`, featureKey, userContext.ID)) } if featureDecision.Source == decision.FeatureTest && featureDecision.Variation != nil { @@ -141,14 +140,14 @@ func (o *OptimizelyClient) GetEnabledFeatures(userContext entities.UserContext) err = errors.New("unexpected error") } errorMessage := fmt.Sprintf("GetEnabledFeatures call, optimizely SDK is panicking with the error:") - logger.Error(errorMessage, err) - logger.Debug(string(debug.Stack())) + o.logger.Error(errorMessage, err) + o.logger.Debug(string(debug.Stack())) } }() projectConfig, err := o.getProjectConfig() if err != nil { - logger.Error("Error retrieving ProjectConfig", err) + o.logger.Error("Error retrieving ProjectConfig", err) return enabledFeatures, err } @@ -241,7 +240,7 @@ func (o *OptimizelyClient) GetAllFeatureVariables(featureKey string, userContext variableMap = make(map[string]interface{}) decisionContext, featureDecision, err := o.getFeatureDecision(featureKey, "", userContext) if err != nil { - logger.Error("Optimizely SDK tracking error", err) + o.logger.Error("Optimizely SDK tracking error", err) return enabled, variableMap, err } @@ -251,7 +250,7 @@ func (o *OptimizelyClient) GetAllFeatureVariables(featureKey string, userContext feature := decisionContext.Feature if feature == nil { - logger.Warning(fmt.Sprintf(`feature "%s" does not exist`, featureKey)) + o.logger.Warning(fmt.Sprintf(`feature "%s" does not exist`, featureKey)) return enabled, variableMap, nil } @@ -275,7 +274,7 @@ func (o *OptimizelyClient) GetAllFeatureVariables(featureKey string, userContext out, err = strconv.Atoi(val) case entities.String: default: - logger.Warning(fmt.Sprintf(`type "%s" is unknown, returning string`, varType)) + o.logger.Warning(fmt.Sprintf(`type "%s" is unknown, returning string`, varType)) } variableMap[v.Key] = out @@ -298,14 +297,14 @@ func (o *OptimizelyClient) GetVariation(experimentKey string, userContext entiti err = errors.New("unexpected error") } errorMessage := fmt.Sprintf("GetVariation call, optimizely SDK is panicking with the error:") - logger.Error(errorMessage, err) - logger.Debug(string(debug.Stack())) + o.logger.Error(errorMessage, err) + o.logger.Debug(string(debug.Stack())) } }() _, experimentDecision, err := o.getExperimentDecision(experimentKey, userContext) if err != nil { - logger.Error("received an error while computing experiment decision", err) + o.logger.Error("received an error while computing experiment decision", err) } if experimentDecision.Variation != nil { @@ -330,14 +329,14 @@ func (o *OptimizelyClient) Track(eventKey string, userContext entities.UserConte err = errors.New("unexpected error") } errorMessage := fmt.Sprintf("Track call, optimizely SDK is panicking with the error:") - logger.Error(errorMessage, err) - logger.Debug(string(debug.Stack())) + o.logger.Error(errorMessage, err) + o.logger.Debug(string(debug.Stack())) } }() projectConfig, e := o.getProjectConfig() if e != nil { - logger.Error("Optimizely SDK tracking error", e) + o.logger.Error("Optimizely SDK tracking error", e) return e } @@ -345,7 +344,7 @@ func (o *OptimizelyClient) Track(eventKey string, userContext entities.UserConte if e != nil { errorMessage := fmt.Sprintf(`Unable to get event for key "%s": %s`, eventKey, e) - logger.Warning(errorMessage) + o.logger.Warning(errorMessage) return nil } @@ -353,7 +352,7 @@ func (o *OptimizelyClient) Track(eventKey string, userContext entities.UserConte if o.EventProcessor.ProcessEvent(userEvent) && o.notificationCenter != nil { trackNotification := notification.TrackNotification{EventKey: eventKey, UserContext: userContext, EventTags: eventTags, ConversionEvent: *userEvent.Conversion} if err = o.notificationCenter.Send(notification.Track, trackNotification); err != nil { - logger.Warning("Problem with sending notification") + o.logger.Warning("Problem with sending notification") } } @@ -373,23 +372,23 @@ func (o *OptimizelyClient) getFeatureDecision(featureKey, variableKey string, us err = errors.New("unexpected error") } errorMessage := fmt.Sprintf("getFeatureDecision call, optimizely SDK is panicking with the error:") - logger.Error(errorMessage, err) - logger.Debug(string(debug.Stack())) + o.logger.Error(errorMessage, err) + o.logger.Debug(string(debug.Stack())) } }() userID := userContext.ID - logger.Debug(fmt.Sprintf(`Evaluating feature "%s" for user "%s".`, featureKey, userID)) + o.logger.Debug(fmt.Sprintf(`Evaluating feature "%s" for user "%s".`, featureKey, userID)) projectConfig, e := o.getProjectConfig() if e != nil { - logger.Error("Error calling getFeatureDecision", e) + o.logger.Error("Error calling getFeatureDecision", e) return decisionContext, featureDecision, e } feature, e := projectConfig.GetFeatureByKey(featureKey) if e != nil { - logger.Warning(fmt.Sprintf(`Could not get feature for key "%s": %s`, featureKey, e)) + o.logger.Warning(fmt.Sprintf(`Could not get feature for key "%s": %s`, featureKey, e)) return decisionContext, featureDecision, nil } @@ -397,7 +396,7 @@ func (o *OptimizelyClient) getFeatureDecision(featureKey, variableKey string, us if variableKey != "" { variable, err = projectConfig.GetVariableByKey(feature.Key, variableKey) if err != nil { - logger.Warning(fmt.Sprintf(`Could not get variable for key "%s": %s`, variableKey, err)) + o.logger.Warning(fmt.Sprintf(`Could not get variable for key "%s": %s`, variableKey, err)) return decisionContext, featureDecision, nil } } @@ -410,7 +409,7 @@ func (o *OptimizelyClient) getFeatureDecision(featureKey, variableKey string, us featureDecision, err = o.DecisionService.GetFeatureDecision(decisionContext, userContext) if err != nil { - logger.Warning(fmt.Sprintf(`Received error while making a decision for feature "%s": %s`, featureKey, err)) + o.logger.Warning(fmt.Sprintf(`Received error while making a decision for feature "%s": %s`, featureKey, err)) return decisionContext, featureDecision, nil } @@ -420,7 +419,7 @@ func (o *OptimizelyClient) getFeatureDecision(featureKey, variableKey string, us func (o *OptimizelyClient) getExperimentDecision(experimentKey string, userContext entities.UserContext) (decisionContext decision.ExperimentDecisionContext, experimentDecision decision.ExperimentDecision, err error) { userID := userContext.ID - logger.Debug(fmt.Sprintf(`Evaluating experiment "%s" for user "%s".`, experimentKey, userID)) + o.logger.Debug(fmt.Sprintf(`Evaluating experiment "%s" for user "%s".`, experimentKey, userID)) projectConfig, e := o.getProjectConfig() if e != nil { @@ -429,7 +428,7 @@ func (o *OptimizelyClient) getExperimentDecision(experimentKey string, userConte experiment, e := projectConfig.GetExperimentByKey(experimentKey) if e != nil { - logger.Warning(fmt.Sprintf(`Could not get experiment for key "%s": %s`, experimentKey, e)) + o.logger.Warning(fmt.Sprintf(`Could not get experiment for key "%s": %s`, experimentKey, e)) return decisionContext, experimentDecision, nil } @@ -440,15 +439,15 @@ func (o *OptimizelyClient) getExperimentDecision(experimentKey string, userConte experimentDecision, err = o.DecisionService.GetExperimentDecision(decisionContext, userContext) if err != nil { - logger.Warning(fmt.Sprintf(`Received error while making a decision for experiment "%s": %s`, experimentKey, err)) + o.logger.Warning(fmt.Sprintf(`Received error while making a decision for experiment "%s": %s`, experimentKey, err)) return decisionContext, experimentDecision, nil } if experimentDecision.Variation != nil { result := experimentDecision.Variation.Key - logger.Info(fmt.Sprintf(`User "%s" is bucketed into variation "%s" of experiment "%s".`, userContext.ID, result, experimentKey)) + o.logger.Info(fmt.Sprintf(`User "%s" is bucketed into variation "%s" of experiment "%s".`, userContext.ID, result, experimentKey)) } else { - logger.Info(fmt.Sprintf(`User "%s" is not bucketed into any variation for experiment "%s": %s.`, userContext.ID, experimentKey, experimentDecision.Reason)) + o.logger.Info(fmt.Sprintf(`User "%s" is not bucketed into any variation for experiment "%s": %s.`, userContext.ID, experimentKey, experimentDecision.Reason)) } return decisionContext, experimentDecision, err @@ -469,12 +468,12 @@ func (o *OptimizelyClient) OnTrack(callback func(eventKey string, userContext en } } if !success { - logger.Warning(fmt.Sprintf("Unable to convert notification payload %v into TrackNotification", payload)) + o.logger.Warning(fmt.Sprintf("Unable to convert notification payload %v into TrackNotification", payload)) } } id, err := o.notificationCenter.AddHandler(notification.Track, handler) if err != nil { - logger.Warning("Problem with adding notification handler") + o.logger.Warning("Problem with adding notification handler") return 0, err } return id, nil @@ -486,7 +485,7 @@ func (o *OptimizelyClient) RemoveOnTrack(id int) error { return fmt.Errorf("no notification center found") } if err := o.notificationCenter.RemoveHandler(id, notification.Track); err != nil { - logger.Warning("Problem with removing notification handler") + o.logger.Warning("Problem with removing notification handler") return err } return nil diff --git a/pkg/client/factory.go b/pkg/client/factory.go index 0e23084c0..7022b0ea4 100644 --- a/pkg/client/factory.go +++ b/pkg/client/factory.go @@ -20,6 +20,7 @@ package client import ( "context" "errors" + "github.com/optimizely/go-sdk/pkg/logging" "time" "github.com/optimizely/go-sdk/pkg/config" @@ -73,8 +74,10 @@ func (f OptimizelyFactory) Client(clientOptions ...OptionFunc) (*OptimizelyClien ctx = context.Background() } - eg := utils.NewExecGroup(ctx) - appClient := &OptimizelyClient{execGroup: eg, notificationCenter: registry.GetNotificationCenter(f.SDKKey)} + eg := utils.NewExecGroup(logging.GetLogger(f.SDKKey, "ExecGroup"), ctx) + appClient := &OptimizelyClient{execGroup: eg, + notificationCenter: registry.GetNotificationCenter(f.SDKKey), + logger: logging.GetLogger(f.SDKKey, "OptimizelyClient")} if f.configManager != nil { appClient.ConfigManager = f.configManager @@ -108,7 +111,7 @@ func (f OptimizelyFactory) Client(clientOptions ...OptionFunc) (*OptimizelyClien if f.overrideStore != nil { experimentServiceOptions = append(experimentServiceOptions, decision.WithOverrideStore(f.overrideStore)) } - compositeExperimentService := decision.NewCompositeExperimentService(experimentServiceOptions...) + compositeExperimentService := decision.NewCompositeExperimentService(f.SDKKey, experimentServiceOptions...) compositeService := decision.NewCompositeService(f.SDKKey, decision.WithCompositeExperimentService(compositeExperimentService)) appClient.DecisionService = compositeService } @@ -211,7 +214,7 @@ func (f OptimizelyFactory) StaticClient() (*OptimizelyClient, error) { configManager = staticConfigManager } else if f.Datafile != nil { - staticConfigManager, err := config.NewStaticProjectConfigManagerFromPayload(f.Datafile) + staticConfigManager, err := config.NewStaticProjectConfigManagerFromPayload(logging.GetLogger(f.SDKKey, "StaticProjectConfigManagerFromPayload"), f.Datafile) if err != nil { return nil, err diff --git a/pkg/config/datafileprojectconfig/config.go b/pkg/config/datafileprojectconfig/config.go index 4a787f5d3..b69f49882 100644 --- a/pkg/config/datafileprojectconfig/config.go +++ b/pkg/config/datafileprojectconfig/config.go @@ -20,14 +20,12 @@ package datafileprojectconfig import ( "errors" "fmt" + "github.com/optimizely/go-sdk/pkg/logging" "github.com/optimizely/go-sdk/pkg/config/datafileprojectconfig/mappers" "github.com/optimizely/go-sdk/pkg/entities" - "github.com/optimizely/go-sdk/pkg/logging" ) -var logger = logging.GetLogger("DatafileProjectConfig") - var datafileVersions = map[string]struct{}{ "4": {}, } @@ -175,7 +173,7 @@ func (c DatafileProjectConfig) GetGroupByID(groupID string) (entities.Group, err } // NewDatafileProjectConfig initializes a new datafile from a json byte array using the default JSON datafile parser -func NewDatafileProjectConfig(jsonDatafile []byte) (*DatafileProjectConfig, error) { +func NewDatafileProjectConfig(logger logging.OptimizelyLogProducer, jsonDatafile []byte) (*DatafileProjectConfig, error) { datafile, err := Parse(jsonDatafile) if err != nil { logger.Error("Error parsing datafile", err) diff --git a/pkg/config/polling_manager.go b/pkg/config/polling_manager.go index f26c85139..1c41d32f1 100644 --- a/pkg/config/polling_manager.go +++ b/pkg/config/polling_manager.go @@ -20,12 +20,12 @@ package config import ( "context" "fmt" + "github.com/optimizely/go-sdk/pkg/logging" "net/http" "sync" "time" "github.com/optimizely/go-sdk/pkg/config/datafileprojectconfig" - "github.com/optimizely/go-sdk/pkg/logging" "github.com/optimizely/go-sdk/pkg/notification" "github.com/optimizely/go-sdk/pkg/registry" "github.com/optimizely/go-sdk/pkg/utils" @@ -48,8 +48,6 @@ const DatafileURLTemplate = "https://cdn.optimizely.com/datafiles/%s.json" // Err403Forbidden is 403Forbidden specific error var Err403Forbidden = errors.New("unable to fetch fresh datafile (consider rechecking SDK key), status code: 403 Forbidden") -var cmLogger = logging.GetLogger("PollingConfigManager") - // PollingProjectConfigManager maintains a dynamic copy of the project config by continuously polling for the datafile // from the Optimizely CDN at a given (configurable) interval. type PollingProjectConfigManager struct { @@ -60,6 +58,7 @@ type PollingProjectConfigManager struct { pollingInterval time.Duration requester utils.Requester sdkKey string + logger logging.OptimizelyLogProducer configLock sync.RWMutex err error @@ -120,7 +119,7 @@ func (cm *PollingProjectConfigManager) SyncConfig() { if e != nil { msg := "unable to fetch fresh datafile" - cmLogger.Warning(msg) + cm.logger.Warning(msg) cm.configLock.Lock() if code == http.StatusForbidden { @@ -133,7 +132,7 @@ func (cm *PollingProjectConfigManager) SyncConfig() { } if code == http.StatusNotModified { - cmLogger.Debug("The datafile was not modified and won't be downloaded again") + cm.logger.Debug("The datafile was not modified and won't be downloaded again") return } @@ -144,9 +143,9 @@ func (cm *PollingProjectConfigManager) SyncConfig() { cm.lastModified = lastModified } - projectConfig, err := datafileprojectconfig.NewDatafileProjectConfig(datafile) + projectConfig, err := datafileprojectconfig.NewDatafileProjectConfig(logging.GetLogger(cm.sdkKey, "NewDatafileProjectConfig"), datafile) if err != nil { - cmLogger.Warning("failed to create project config") + cm.logger.Warning("failed to create project config") closeMutex(errors.New("unable to parse datafile")) return } @@ -156,28 +155,28 @@ func (cm *PollingProjectConfigManager) SyncConfig() { previousRevision = cm.projectConfig.GetRevision() } if projectConfig.GetRevision() == previousRevision { - cmLogger.Debug(fmt.Sprintf("No datafile updates. Current revision number: %s", cm.projectConfig.GetRevision())) + cm.logger.Debug(fmt.Sprintf("No datafile updates. Current revision number: %s", cm.projectConfig.GetRevision())) closeMutex(nil) return } err = cm.setConfig(projectConfig) closeMutex(err) if err == nil { - cmLogger.Debug(fmt.Sprintf("New datafile set with revision: %s. Old revision: %s", projectConfig.GetRevision(), previousRevision)) + cm.logger.Debug(fmt.Sprintf("New datafile set with revision: %s. Old revision: %s", projectConfig.GetRevision(), previousRevision)) cm.sendConfigUpdateNotification() } } // Start starts the polling func (cm *PollingProjectConfigManager) Start(ctx context.Context) { - cmLogger.Debug("Polling Config Manager Initiated") + cm.logger.Debug("Polling Config Manager Initiated") t := time.NewTicker(cm.pollingInterval) for { select { case <-t.C: cm.SyncConfig() case <-ctx.Done(): - cmLogger.Debug("Polling Config Manager Stopped") + cm.logger.Debug("Polling Config Manager Stopped") return } } @@ -189,9 +188,10 @@ func NewPollingProjectConfigManager(sdkKey string, pollingMangerOptions ...Optio pollingProjectConfigManager := PollingProjectConfigManager{ notificationCenter: registry.GetNotificationCenter(sdkKey), pollingInterval: DefaultPollingInterval, - requester: utils.NewHTTPRequester(), + requester: utils.NewHTTPRequester(logging.GetLogger(sdkKey, "HTTPRequester")), datafileURLTemplate: DatafileURLTemplate, sdkKey: sdkKey, + logger: logging.GetLogger(sdkKey, "PollingProjectConfigManager"), } for _, opt := range pollingMangerOptions { @@ -212,9 +212,10 @@ func NewAsyncPollingProjectConfigManager(sdkKey string, pollingMangerOptions ... pollingProjectConfigManager := PollingProjectConfigManager{ notificationCenter: registry.GetNotificationCenter(sdkKey), pollingInterval: DefaultPollingInterval, - requester: utils.NewHTTPRequester(), + requester: utils.NewHTTPRequester(logging.GetLogger(sdkKey, "HTTPRequester")), datafileURLTemplate: DatafileURLTemplate, sdkKey: sdkKey, + logger: logging.GetLogger(sdkKey, "PollingProjectConfigManager"), } for _, opt := range pollingMangerOptions { @@ -253,12 +254,12 @@ func (cm *PollingProjectConfigManager) OnProjectConfigUpdate(callback func(notif if projectConfigUpdateNotification, ok := payload.(notification.ProjectConfigUpdateNotification); ok { callback(projectConfigUpdateNotification) } else { - cmLogger.Warning(fmt.Sprintf("Unable to convert notification payload %v into ProjectConfigUpdateNotification", payload)) + cm.logger.Warning(fmt.Sprintf("Unable to convert notification payload %v into ProjectConfigUpdateNotification", payload)) } } id, err := cm.notificationCenter.AddHandler(notification.ProjectConfigUpdate, handler) if err != nil { - cmLogger.Warning("Problem with adding notification handler") + cm.logger.Warning("Problem with adding notification handler") return 0, err } return id, nil @@ -267,7 +268,7 @@ func (cm *PollingProjectConfigManager) OnProjectConfigUpdate(callback func(notif // RemoveOnProjectConfigUpdate removes handler for ProjectConfigUpdate notification with given id func (cm *PollingProjectConfigManager) RemoveOnProjectConfigUpdate(id int) error { if err := cm.notificationCenter.RemoveHandler(id, notification.ProjectConfigUpdate); err != nil { - cmLogger.Warning("Problem with removing notification handler") + cm.logger.Warning("Problem with removing notification handler") return err } return nil @@ -288,7 +289,8 @@ func (cm *PollingProjectConfigManager) setInitialDatafile(datafile []byte) { if len(datafile) != 0 { cm.configLock.Lock() defer cm.configLock.Unlock() - projectConfig, err := datafileprojectconfig.NewDatafileProjectConfig(datafile) + projectConfig, err := datafileprojectconfig.NewDatafileProjectConfig(logging.GetLogger(cm.sdkKey, "DatafileProjectConfig"), + datafile) if projectConfig != nil { err = cm.setConfig(projectConfig) } @@ -303,7 +305,7 @@ func (cm *PollingProjectConfigManager) sendConfigUpdateNotification() { Revision: cm.projectConfig.GetRevision(), } if err := cm.notificationCenter.Send(notification.ProjectConfigUpdate, projectConfigUpdateNotification); err != nil { - cmLogger.Warning("Problem with sending notification") + cm.logger.Warning("Problem with sending notification") } } } diff --git a/pkg/config/static_manager.go b/pkg/config/static_manager.go index 050ee553d..2c685ea64 100644 --- a/pkg/config/static_manager.go +++ b/pkg/config/static_manager.go @@ -20,6 +20,7 @@ package config import ( "errors" "fmt" + "github.com/optimizely/go-sdk/pkg/logging" "sync" "github.com/optimizely/go-sdk/pkg/config/datafileprojectconfig" @@ -32,38 +33,42 @@ type StaticProjectConfigManager struct { projectConfig ProjectConfig optimizelyConfig *OptimizelyConfig configLock sync.Mutex + logger logging.OptimizelyLogProducer } // NewStaticProjectConfigManagerFromURL returns new instance of StaticProjectConfigManager for URL func NewStaticProjectConfigManagerFromURL(sdkKey string) (*StaticProjectConfigManager, error) { - requester := utils.NewHTTPRequester() + requester := utils.NewHTTPRequester(logging.GetLogger(sdkKey, "HTTPRequester")) + + logger := logging.GetLogger(sdkKey, "StaticProjectConfigManager") url := fmt.Sprintf(DatafileURLTemplate, sdkKey) datafile, _, code, e := requester.Get(url) if e != nil { - cmLogger.Error(fmt.Sprintf("request returned with http code=%d", code), e) + logger.Error(fmt.Sprintf("request returned with http code=%d", code), e) return nil, e } - return NewStaticProjectConfigManagerFromPayload(datafile) + return NewStaticProjectConfigManagerFromPayload(logger, datafile) } // NewStaticProjectConfigManagerFromPayload returns new instance of StaticProjectConfigManager for payload -func NewStaticProjectConfigManagerFromPayload(payload []byte) (*StaticProjectConfigManager, error) { - projectConfig, err := datafileprojectconfig.NewDatafileProjectConfig(payload) +func NewStaticProjectConfigManagerFromPayload(logger logging.OptimizelyLogProducer, payload []byte) (*StaticProjectConfigManager, error) { + projectConfig, err := datafileprojectconfig.NewDatafileProjectConfig(logger, payload) if err != nil { return nil, err } - return NewStaticProjectConfigManager(projectConfig), nil + return NewStaticProjectConfigManager(projectConfig, logger), nil } // NewStaticProjectConfigManager creates a new instance of the manager with the given project config -func NewStaticProjectConfigManager(config ProjectConfig) *StaticProjectConfigManager { +func NewStaticProjectConfigManager(config ProjectConfig, logger logging.OptimizelyLogProducer) *StaticProjectConfigManager { return &StaticProjectConfigManager{ projectConfig: config, + logger: logger, } } diff --git a/pkg/decision/bucketer/murmurhashbucketer.go b/pkg/decision/bucketer/murmurhashbucketer.go index 0e946b337..c8f08b03b 100644 --- a/pkg/decision/bucketer/murmurhashbucketer.go +++ b/pkg/decision/bucketer/murmurhashbucketer.go @@ -19,14 +19,13 @@ package bucketer import ( "fmt" + "github.com/optimizely/go-sdk/pkg/logging" "math" "github.com/optimizely/go-sdk/pkg/entities" - "github.com/optimizely/go-sdk/pkg/logging" "github.com/twmb/murmur3" ) -var logger = logging.GetLogger("MurmurhashBucketer") var maxHashValue = float32(math.Pow(2, 32)) // DefaultHashSeed is the hash seed to use for murmurhash @@ -42,12 +41,14 @@ type Bucketer interface { // MurmurhashBucketer generates the bucketing value using the mmh3 algorightm type MurmurhashBucketer struct { hashSeed uint32 + logger logging.OptimizelyLogProducer } // NewMurmurhashBucketer returns a new instance of the murmurhash bucketer -func NewMurmurhashBucketer(hashSeed uint32) *MurmurhashBucketer { +func NewMurmurhashBucketer(sdkKey string, hashSeed uint32) *MurmurhashBucketer { return &MurmurhashBucketer{ hashSeed: hashSeed, + logger: logging.GetLogger(sdkKey, "MurmurhashBucketer"), } } @@ -55,7 +56,7 @@ func NewMurmurhashBucketer(hashSeed uint32) *MurmurhashBucketer { func (b MurmurhashBucketer) Generate(bucketingKey string) int { hasher := murmur3.SeedNew32(b.hashSeed) if _, err := hasher.Write([]byte(bucketingKey)); err != nil { - logger.Error(fmt.Sprintf("Unable to generate a hash for the bucketing key=%s", bucketingKey), err) + b.logger.Error(fmt.Sprintf("Unable to generate a hash for the bucketing key=%s", bucketingKey), err) } hashCode := hasher.Sum32() ratio := float32(hashCode) / maxHashValue diff --git a/pkg/decision/bucketer/murmurhashbucketer_test.go b/pkg/decision/bucketer/murmurhashbucketer_test.go index 7b8a9a949..1b5dcec34 100644 --- a/pkg/decision/bucketer/murmurhashbucketer_test.go +++ b/pkg/decision/bucketer/murmurhashbucketer_test.go @@ -9,7 +9,7 @@ import ( ) func TestBucketToEntity(t *testing.T) { - bucketer := NewMurmurhashBucketer(DefaultHashSeed) + bucketer := NewMurmurhashBucketer("", DefaultHashSeed) experimentID := "1886780721" experimentID2 := "1886780722" diff --git a/pkg/decision/composite_experiment_service.go b/pkg/decision/composite_experiment_service.go index c022cb84f..306adeb28 100644 --- a/pkg/decision/composite_experiment_service.go +++ b/pkg/decision/composite_experiment_service.go @@ -19,13 +19,11 @@ package decision import ( "fmt" + "github.com/optimizely/go-sdk/pkg/logging" "github.com/optimizely/go-sdk/pkg/entities" - "github.com/optimizely/go-sdk/pkg/logging" ) -var ceLogger = logging.GetLogger("CompositeExperimentService") - // CESOptionFunc is used to assign optional configuration options type CESOptionFunc func(*CompositeExperimentService) @@ -48,15 +46,16 @@ type CompositeExperimentService struct { experimentServices []ExperimentService overrideStore ExperimentOverrideStore userProfileService UserProfileService + logger logging.OptimizelyLogProducer } // NewCompositeExperimentService creates a new instance of the CompositeExperimentService -func NewCompositeExperimentService(options ...CESOptionFunc) *CompositeExperimentService { +func NewCompositeExperimentService(sdkKey string, options ...CESOptionFunc) *CompositeExperimentService { // These decision services are applied in order: // 1. Overrides (if supplied) // 2. Whitelist // 3. Bucketing (with User profile integration if supplied) - compositeExperimentService := &CompositeExperimentService{} + compositeExperimentService := &CompositeExperimentService{logger:logging.GetLogger(sdkKey, "CompositeExperimentService")} for _, opt := range options { opt(compositeExperimentService) } @@ -66,13 +65,15 @@ func NewCompositeExperimentService(options ...CESOptionFunc) *CompositeExperimen // Prepend overrides if supplied if compositeExperimentService.overrideStore != nil { - overrideService := NewExperimentOverrideService(compositeExperimentService.overrideStore) + overrideService := NewExperimentOverrideService(logging.GetLogger(sdkKey, "ExperimentOverrideService"), + compositeExperimentService.overrideStore) experimentServices = append([]ExperimentService{overrideService}, experimentServices...) } - experimentBucketerService := NewExperimentBucketerService() + experimentBucketerService := NewExperimentBucketerService(logging.GetLogger(sdkKey, "ExperimentBucketerService")) if compositeExperimentService.userProfileService != nil { - persistingExperimentService := NewPersistingExperimentService(experimentBucketerService, compositeExperimentService.userProfileService) + persistingExperimentService := NewPersistingExperimentService(logging.GetLogger(sdkKey, "PersistingExperimentService"), + experimentBucketerService, compositeExperimentService.userProfileService) experimentServices = append(experimentServices, persistingExperimentService) } else { experimentServices = append(experimentServices, experimentBucketerService) @@ -89,7 +90,7 @@ func (s CompositeExperimentService) GetDecision(decisionContext ExperimentDecisi for _, experimentService := range s.experimentServices { decision, err = experimentService.GetDecision(decisionContext, userContext) if err != nil { - ceLogger.Debug(fmt.Sprintf("%v", err)) + s.logger.Debug(fmt.Sprintf("%v", err)) } if decision.Variation != nil && err == nil { return decision, err diff --git a/pkg/decision/composite_feature_service.go b/pkg/decision/composite_feature_service.go index 1a4be9c34..671131147 100644 --- a/pkg/decision/composite_feature_service.go +++ b/pkg/decision/composite_feature_service.go @@ -24,19 +24,19 @@ import ( "github.com/optimizely/go-sdk/pkg/logging" ) -var cfLogger = logging.GetLogger("CompositeFeatureService") - // CompositeFeatureService is the default out-of-the-box feature decision service type CompositeFeatureService struct { featureServices []FeatureService + logger logging.OptimizelyLogProducer } // NewCompositeFeatureService returns a new instance of the CompositeFeatureService -func NewCompositeFeatureService(compositeExperimentService ExperimentService) *CompositeFeatureService { +func NewCompositeFeatureService(sdkKey string, compositeExperimentService ExperimentService) *CompositeFeatureService { return &CompositeFeatureService{ + logger:logging.GetLogger(sdkKey, "CompositeFeatureService"), featureServices: []FeatureService{ - NewFeatureExperimentService(compositeExperimentService), - NewRolloutService(), + NewFeatureExperimentService(logging.GetLogger(sdkKey, "FeatureExperimentService"), compositeExperimentService), + NewRolloutService(sdkKey), }, } } @@ -48,7 +48,7 @@ func (f CompositeFeatureService) GetDecision(decisionContext FeatureDecisionCont for _, featureDecisionService := range f.featureServices { featureDecision, err = featureDecisionService.GetDecision(decisionContext, userContext) if err != nil { - cfLogger.Debug(fmt.Sprintf("%v", err)) + f.logger.Debug(fmt.Sprintf("%v", err)) } if featureDecision.Variation != nil && err == nil { diff --git a/pkg/decision/composite_service.go b/pkg/decision/composite_service.go index 8fb67da64..d1ed066bb 100644 --- a/pkg/decision/composite_service.go +++ b/pkg/decision/composite_service.go @@ -27,13 +27,12 @@ import ( "github.com/optimizely/go-sdk/pkg/registry" ) -var csLogger = logging.GetLogger("CompositeDecisionService") - // CompositeService is the entry-point into the decision service. It provides out of the box decision making for Features and Experiments. type CompositeService struct { compositeExperimentService ExperimentService compositeFeatureService FeatureService notificationCenter notification.Center + logger logging.OptimizelyLogProducer } // CSOptionFunc is used to pass custom config options into the CompositeService. @@ -49,6 +48,7 @@ func WithCompositeExperimentService(compositeExperimentService ExperimentService // NewCompositeService returns a new instance of the CompositeService with the defaults func NewCompositeService(sdkKey string, options ...CSOptionFunc) *CompositeService { compositeService := &CompositeService{ + logger:logging.GetLogger(sdkKey, "CompositeService"), notificationCenter: registry.GetNotificationCenter(sdkKey), } @@ -57,9 +57,9 @@ func NewCompositeService(sdkKey string, options ...CSOptionFunc) *CompositeServi } if compositeService.compositeExperimentService == nil { - compositeService.compositeExperimentService = NewCompositeExperimentService() + compositeService.compositeExperimentService = NewCompositeExperimentService(sdkKey) } - compositeService.compositeFeatureService = NewCompositeFeatureService(compositeService.compositeExperimentService) + compositeService.compositeFeatureService = NewCompositeFeatureService(sdkKey, compositeService.compositeExperimentService) return compositeService } @@ -132,7 +132,7 @@ func (s CompositeService) GetFeatureDecision(featureDecisionContext FeatureDecis UserContext: userContext, } if err = s.notificationCenter.Send(notification.Decision, decisionNotification); err != nil { - csLogger.Warning("Problem with sending notification") + s.logger.Warning("Problem with sending notification") } } return featureDecision, err @@ -163,7 +163,7 @@ func (s CompositeService) GetExperimentDecision(experimentDecisionContext Experi } if err = s.notificationCenter.Send(notification.Decision, decisionNotification); err != nil { - csLogger.Warning("Error sending sending notification") + s.logger.Warning("Error sending sending notification") } } @@ -176,12 +176,12 @@ func (s CompositeService) OnDecision(callback func(notification.DecisionNotifica if decisionNotification, ok := payload.(notification.DecisionNotification); ok { callback(decisionNotification) } else { - csLogger.Warning(fmt.Sprintf("Unable to convert notification payload %v into DecisionNotification", payload)) + s.logger.Warning(fmt.Sprintf("Unable to convert notification payload %v into DecisionNotification", payload)) } } id, err := s.notificationCenter.AddHandler(notification.Decision, handler) if err != nil { - csLogger.Warning("Problem with adding notification handler") + s.logger.Warning("Problem with adding notification handler") return 0, err } return id, nil @@ -190,7 +190,7 @@ func (s CompositeService) OnDecision(callback func(notification.DecisionNotifica // RemoveOnDecision removes handler for Decision notification with given id func (s CompositeService) RemoveOnDecision(id int) error { if err := s.notificationCenter.RemoveHandler(id, notification.Decision); err != nil { - csLogger.Warning("Problem with removing notification handler") + s.logger.Warning("Problem with removing notification handler") return err } return nil diff --git a/pkg/decision/experiment_bucketer_service.go b/pkg/decision/experiment_bucketer_service.go index f57c28737..94ff8a565 100644 --- a/pkg/decision/experiment_bucketer_service.go +++ b/pkg/decision/experiment_bucketer_service.go @@ -19,27 +19,26 @@ package decision import ( "fmt" + "github.com/optimizely/go-sdk/pkg/logging" "github.com/optimizely/go-sdk/pkg/decision/bucketer" "github.com/optimizely/go-sdk/pkg/decision/evaluator" "github.com/optimizely/go-sdk/pkg/decision/reasons" "github.com/optimizely/go-sdk/pkg/entities" - "github.com/optimizely/go-sdk/pkg/logging" ) -// These variables are package-scoped, meaning that they can be accessed within the same package so we need unique names. -var bLogger = logging.GetLogger("ExperimentBucketerService") - // ExperimentBucketerService makes a decision using the experiment bucketer type ExperimentBucketerService struct { audienceTreeEvaluator evaluator.TreeEvaluator bucketer bucketer.ExperimentBucketer + logger logging.OptimizelyLogProducer } // NewExperimentBucketerService returns a new instance of the ExperimentBucketerService -func NewExperimentBucketerService() *ExperimentBucketerService { +func NewExperimentBucketerService(logger logging.OptimizelyLogProducer) *ExperimentBucketerService { // @TODO(mng): add experiment override service return &ExperimentBucketerService{ + logger:logger, audienceTreeEvaluator: evaluator.NewMixedTreeEvaluator(), bucketer: *bucketer.NewMurmurhashExperimentBucketer(bucketer.DefaultHashSeed), } @@ -68,11 +67,11 @@ func (s ExperimentBucketerService) GetDecision(decisionContext ExperimentDecisio // bucket user into a variation bucketingID, err := userContext.GetBucketingID() if err != nil { - bLogger.Debug(fmt.Sprintf(`Error computing bucketing ID for experiment "%s": "%s"`, experiment.Key, err.Error())) + s.logger.Debug(fmt.Sprintf(`Error computing bucketing ID for experiment "%s": "%s"`, experiment.Key, err.Error())) } if bucketingID != userContext.ID { - bLogger.Debug(fmt.Sprintf(`Using bucketing ID: "%s" for user "%s"`, bucketingID, userContext.ID)) + s.logger.Debug(fmt.Sprintf(`Using bucketing ID: "%s" for user "%s"`, bucketingID, userContext.ID)) } // @TODO: handle error from bucketer variation, reason, _ := s.bucketer.Bucket(bucketingID, *experiment, group) diff --git a/pkg/decision/experiment_override_service.go b/pkg/decision/experiment_override_service.go index dec0e71a3..7e93b0e1e 100644 --- a/pkg/decision/experiment_override_service.go +++ b/pkg/decision/experiment_override_service.go @@ -27,8 +27,6 @@ import ( "github.com/optimizely/go-sdk/pkg/logging" ) -var eosLogger = logging.GetLogger("ExperimentOverrideService") - // ExperimentOverrideKey represents the user ID and experiment associated with an override variation type ExperimentOverrideKey struct { ExperimentKey, UserID string @@ -80,11 +78,13 @@ func (m *MapExperimentOverridesStore) RemoveVariation(overrideKey ExperimentOver // Implements the ExperimentService interface type ExperimentOverrideService struct { Overrides ExperimentOverrideStore + logger logging.OptimizelyLogProducer } // NewExperimentOverrideService returns a pointer to an initialized ExperimentOverrideService -func NewExperimentOverrideService(overrides ExperimentOverrideStore) *ExperimentOverrideService { +func NewExperimentOverrideService(logger logging.OptimizelyLogProducer, overrides ExperimentOverrideStore) *ExperimentOverrideService { return &ExperimentOverrideService{ + logger: logger, Overrides: overrides, } } @@ -107,7 +107,7 @@ func (s ExperimentOverrideService) GetDecision(decisionContext ExperimentDecisio if variation, ok := decisionContext.Experiment.Variations[variationID]; ok { decision.Variation = &variation decision.Reason = reasons.OverrideVariationAssignmentFound - eosLogger.Debug(fmt.Sprintf("Override variation %v found for user %v", variationKey, userContext.ID)) + s.logger.Debug(fmt.Sprintf("Override variation %v found for user %v", variationKey, userContext.ID)) return decision, nil } } diff --git a/pkg/decision/feature_experiment_service.go b/pkg/decision/feature_experiment_service.go index 0b0d33ae9..8890c10e7 100644 --- a/pkg/decision/feature_experiment_service.go +++ b/pkg/decision/feature_experiment_service.go @@ -24,16 +24,16 @@ import ( "github.com/optimizely/go-sdk/pkg/logging" ) -var fesLogger = logging.GetLogger("FeatureExperimentService") - // FeatureExperimentService helps evaluate feature test associated with the feature type FeatureExperimentService struct { compositeExperimentService ExperimentService + logger logging.OptimizelyLogProducer } // NewFeatureExperimentService returns a new instance of the FeatureExperimentService -func NewFeatureExperimentService(compositeExperimentService ExperimentService) *FeatureExperimentService { +func NewFeatureExperimentService(logger logging.OptimizelyLogProducer, compositeExperimentService ExperimentService) *FeatureExperimentService { return &FeatureExperimentService{ + logger: logger, compositeExperimentService: compositeExperimentService, } } @@ -50,7 +50,7 @@ func (f FeatureExperimentService) GetDecision(decisionContext FeatureDecisionCon } experimentDecision, err := f.compositeExperimentService.GetDecision(experimentDecisionContext, userContext) - fesLogger.Debug(fmt.Sprintf( + f.logger.Debug(fmt.Sprintf( `Decision made for feature test with key "%s" for user "%s" with the following reason: "%s".`, feature.Key, userContext.ID, diff --git a/pkg/decision/persisting_experiment_service.go b/pkg/decision/persisting_experiment_service.go index be55c1d09..ae3c0a6e8 100644 --- a/pkg/decision/persisting_experiment_service.go +++ b/pkg/decision/persisting_experiment_service.go @@ -24,19 +24,19 @@ import ( "github.com/optimizely/go-sdk/pkg/logging" ) -var pesLogger = logging.GetLogger("PersistingExperimentService") - // PersistingExperimentService attempts to retrieve a saved decision from the user profile service // for the user before having the ExperimentBucketerService compute it. // If computed, the decision is saved back to the user profile service if provided. type PersistingExperimentService struct { experimentBucketedService ExperimentService userProfileService UserProfileService + logger logging.OptimizelyLogProducer } // NewPersistingExperimentService returns a new instance of the PersistingExperimentService -func NewPersistingExperimentService(experimentBucketerService ExperimentService, userProfileService UserProfileService) *PersistingExperimentService { +func NewPersistingExperimentService(logger logging.OptimizelyLogProducer, experimentBucketerService ExperimentService, userProfileService UserProfileService) *PersistingExperimentService { persistingExperimentService := &PersistingExperimentService{ + logger: logger, experimentBucketedService: experimentBucketerService, userProfileService: userProfileService, } @@ -80,9 +80,9 @@ func (p PersistingExperimentService) getSavedDecision(decisionContext Experiment if savedVariationID, ok := userProfile.ExperimentBucketMap[decisionKey]; ok { if variation, ok := decisionContext.Experiment.Variations[savedVariationID]; ok { experimentDecision.Variation = &variation - pesLogger.Debug(fmt.Sprintf(`User "%s" was previously bucketed into variation "%s" of experiment "%s".`, userContext.ID, variation.Key, decisionContext.Experiment.Key)) + p.logger.Debug(fmt.Sprintf(`User "%s" was previously bucketed into variation "%s" of experiment "%s".`, userContext.ID, variation.Key, decisionContext.Experiment.Key)) } else { - pesLogger.Warning(fmt.Sprintf(`User "%s" was previously bucketed into variation with ID "%s" for experiment "%s", but no matching variation was found.`, userContext.ID, savedVariationID, decisionContext.Experiment.Key)) + p.logger.Warning(fmt.Sprintf(`User "%s" was previously bucketed into variation with ID "%s" for experiment "%s", but no matching variation was found.`, userContext.ID, savedVariationID, decisionContext.Experiment.Key)) } } @@ -97,6 +97,6 @@ func (p PersistingExperimentService) saveDecision(userProfile UserProfile, exper } userProfile.ExperimentBucketMap[decisionKey] = decision.Variation.ID p.userProfileService.Save(userProfile) - pesLogger.Debug(fmt.Sprintf(`Decision saved for user "%s".`, userProfile.ID)) + p.logger.Debug(fmt.Sprintf(`Decision saved for user "%s".`, userProfile.ID)) } } diff --git a/pkg/decision/rollout_service.go b/pkg/decision/rollout_service.go index 596a5172a..30d63f13d 100644 --- a/pkg/decision/rollout_service.go +++ b/pkg/decision/rollout_service.go @@ -27,19 +27,19 @@ import ( "github.com/optimizely/go-sdk/pkg/entities" ) -var rsLogger = logging.GetLogger("RolloutService") - // RolloutService makes a feature decision for a given feature rollout type RolloutService struct { audienceTreeEvaluator evaluator.TreeEvaluator experimentBucketerService ExperimentService + logger logging.OptimizelyLogProducer } // NewRolloutService returns a new instance of the Rollout service -func NewRolloutService() *RolloutService { +func NewRolloutService(sdkKey string) *RolloutService { return &RolloutService{ + logger:logging.GetLogger(sdkKey, "RolloutService"), audienceTreeEvaluator: evaluator.NewMixedTreeEvaluator(), - experimentBucketerService: NewExperimentBucketerService(), + experimentBucketerService: NewExperimentBucketerService(logging.GetLogger(sdkKey, "ExperimentBucketerService")), } } @@ -74,7 +74,7 @@ func (r RolloutService) GetDecision(decisionContext FeatureDecisionContext, user evalResult, _ := r.audienceTreeEvaluator.Evaluate(experiment.AudienceConditionTree, condTreeParams) if !evalResult { featureDecision.Reason = reasons.FailedRolloutTargeting - rsLogger.Debug(fmt.Sprintf(`User "%s" failed targeting for feature rollout with key "%s".`, userContext.ID, feature.Key)) + r.logger.Debug(fmt.Sprintf(`User "%s" failed targeting for feature rollout with key "%s".`, userContext.ID, feature.Key)) return featureDecision, nil } } @@ -92,7 +92,7 @@ func (r RolloutService) GetDecision(decisionContext FeatureDecisionContext, user featureDecision.Experiment = experiment featureDecision.Variation = decision.Variation - rsLogger.Debug(fmt.Sprintf(`Decision made for user "%s" for feature rollout with key "%s": %s.`, userContext.ID, feature.Key, featureDecision.Reason)) + r.logger.Debug(fmt.Sprintf(`Decision made for user "%s" for feature rollout with key "%s": %s.`, userContext.ID, feature.Key, featureDecision.Reason)) return featureDecision, nil } diff --git a/pkg/event/dispatcher.go b/pkg/event/dispatcher.go index 1fc5d8641..6aee10677 100644 --- a/pkg/event/dispatcher.go +++ b/pkg/event/dispatcher.go @@ -19,11 +19,11 @@ package event import ( "fmt" + "github.com/optimizely/go-sdk/pkg/logging" "net/http" "sync" "time" - "github.com/optimizely/go-sdk/pkg/logging" "github.com/optimizely/go-sdk/pkg/metrics" "github.com/optimizely/go-sdk/pkg/utils" ) @@ -32,8 +32,6 @@ const maxRetries = 3 const defaultQueueSize = 1000 const sleepTime = 1 * time.Second -var dispatcherLogger = logging.GetLogger("EventDispatcher") - // Dispatcher dispatches events type Dispatcher interface { DispatchEvent(event LogEvent) (bool, error) @@ -42,6 +40,7 @@ type Dispatcher interface { // HTTPEventDispatcher is the HTTP implementation of the Dispatcher interface type HTTPEventDispatcher struct { requester *utils.HTTPRequester + logger logging.OptimizelyLogProducer } // DispatchEvent dispatches event with callback @@ -53,13 +52,13 @@ func (ed *HTTPEventDispatcher) DispatchEvent(event LogEvent) (bool, error) { // resp.StatusCode == 400 is an error var success bool if err != nil { - dispatcherLogger.Error("http.Post failed:", err) + ed.logger.Error("http.Post failed:", err) success = false } else { if code == http.StatusNoContent { success = true } else { - dispatcherLogger.Error(fmt.Sprintf("http.Post invalid response %d", code), nil) + ed.logger.Error(fmt.Sprintf("http.Post invalid response %d", code), nil) success = false } } @@ -71,6 +70,7 @@ type QueueEventDispatcher struct { eventQueue Queue eventFlushLock sync.Mutex Dispatcher Dispatcher + logger logging.OptimizelyLogProducer // metrics queueSize metrics.Gauge @@ -101,7 +101,7 @@ func (ed *QueueEventDispatcher) flushEvents() { ed.queueSize.Set(float64(ed.eventQueue.Size())) for ed.eventQueue.Size() > 0 { if retryCount > maxRetries { - dispatcherLogger.Error(fmt.Sprintf("event failed to send %d times. It will retry on next event sent", maxRetries), nil) + ed.logger.Error(fmt.Sprintf("event failed to send %d times. It will retry on next event sent", maxRetries), nil) ed.failFlushCounter.Add(1) break } @@ -114,7 +114,7 @@ func (ed *QueueEventDispatcher) flushEvents() { event, ok := items[0].(LogEvent) if !ok { // remove it - dispatcherLogger.Error("invalid type passed to event Dispatcher", nil) + ed.logger.Error("invalid type passed to event Dispatcher", nil) ed.eventQueue.Remove(1) ed.failFlushCounter.Add(1) continue @@ -124,12 +124,12 @@ func (ed *QueueEventDispatcher) flushEvents() { if err == nil { if success { - dispatcherLogger.Debug(fmt.Sprintf("Dispatched log event %+v", event)) + ed.logger.Debug(fmt.Sprintf("Dispatched log event %+v", event)) ed.eventQueue.Remove(1) retryCount = 0 ed.sucessFlush.Add(1) } else { - dispatcherLogger.Warning("dispatch event failed") + ed.logger.Warning("dispatch event failed") // we failed. Sleep some seconds and try again. time.Sleep(sleepTime) // increase retryCount. We exit if we have retried x times. @@ -138,7 +138,7 @@ func (ed *QueueEventDispatcher) flushEvents() { ed.retryFlushCounter.Add(1) } } else { - dispatcherLogger.Error("Error dispatching ", err) + ed.logger.Error("Error dispatching ", err) // we failed. Sleep some seconds and try again. time.Sleep(sleepTime) // increase retryCount. We exit if we have retried x times. @@ -151,7 +151,7 @@ func (ed *QueueEventDispatcher) flushEvents() { } // NewQueueEventDispatcher creates a Dispatcher that queues in memory and then sends via go routine. -func NewQueueEventDispatcher(metricsRegistry metrics.Registry) *QueueEventDispatcher { +func NewQueueEventDispatcher(sdkKey string, metricsRegistry metrics.Registry) *QueueEventDispatcher { var dispatcherMetricsRegistry metrics.Registry if metricsRegistry != nil { @@ -162,7 +162,7 @@ func NewQueueEventDispatcher(metricsRegistry metrics.Registry) *QueueEventDispat return &QueueEventDispatcher{ eventQueue: NewInMemoryQueue(defaultQueueSize), - Dispatcher: &HTTPEventDispatcher{requester: utils.NewHTTPRequester()}, + Dispatcher: &HTTPEventDispatcher{requester: utils.NewHTTPRequester(logging.GetLogger(sdkKey, "HTTPRequester"))}, queueSize: dispatcherMetricsRegistry.GetGauge(metrics.DispatcherQueueSize), retryFlushCounter: dispatcherMetricsRegistry.GetCounter(metrics.DispatcherRetryFlush), diff --git a/pkg/event/factory.go b/pkg/event/factory.go index b281c4005..8fa688a3a 100644 --- a/pkg/event/factory.go +++ b/pkg/event/factory.go @@ -19,19 +19,15 @@ package event import ( "errors" - "fmt" "strings" "time" guuid "github.com/google/uuid" "github.com/optimizely/go-sdk/pkg/config" "github.com/optimizely/go-sdk/pkg/entities" - "github.com/optimizely/go-sdk/pkg/logging" "github.com/optimizely/go-sdk/pkg/utils" ) -var efLogger = logging.GetLogger("EventFactory") - const impressionKey string = "campaign_activated" const attributeType = "custom" const specialPrefix = "$opt_" @@ -234,7 +230,7 @@ func getEventAttributes(projectConfig config.ProjectConfig, attributes map[strin case strings.HasPrefix(key, specialPrefix): visitorAttribute.EntityID = key default: - efLogger.Debug(fmt.Sprintf("Unrecognized attribute %s provided. Pruning before sending event to Optimizely.", key)) + //efLogger.Debug(fmt.Sprintf("Unrecognized attribute %s provided. Pruning before sending event to Optimizely.", key)) continue } visitorAttribute.Key = key diff --git a/pkg/event/processor.go b/pkg/event/processor.go index 4ca9e67bc..fd6ecf8d5 100644 --- a/pkg/event/processor.go +++ b/pkg/event/processor.go @@ -51,7 +51,7 @@ type BatchEventProcessor struct { Ticker *time.Ticker EventDispatcher Dispatcher processing *semaphore.Weighted - + logger logging.OptimizelyLogProducer metricsRegistry metrics.Registry } @@ -66,8 +66,6 @@ const DefaultEventFlushInterval = 30 * time.Second const maxFlushWorkers = 1 -var pLogger = logging.GetLogger("EventProcessor") - // BPOptionConfig is the BatchProcessor options that give you the ability to add one more more options before the processor is initialized. type BPOptionConfig func(qp *BatchEventProcessor) @@ -129,6 +127,8 @@ func NewBatchEventProcessor(options ...BPOptionConfig) *BatchEventProcessor { opt(p) } + p.logger = logging.GetLogger(p.sdkKey, "BatchEventProcessor") + if p.MaxQueueSize == 0 { p.MaxQueueSize = defaultQueueSize } @@ -142,7 +142,7 @@ func NewBatchEventProcessor(options ...BPOptionConfig) *BatchEventProcessor { } if p.BatchSize > p.MaxQueueSize { - pLogger.Warning( + p.logger.Warning( fmt.Sprintf("Batch size %d is larger than queue size %d. Setting to defaults", p.BatchSize, p.MaxQueueSize)) @@ -155,7 +155,7 @@ func NewBatchEventProcessor(options ...BPOptionConfig) *BatchEventProcessor { } if p.EventDispatcher == nil { - dispatcher := NewQueueEventDispatcher(p.metricsRegistry) + dispatcher := NewQueueEventDispatcher(p.sdkKey, p.metricsRegistry) p.EventDispatcher = dispatcher } @@ -165,7 +165,7 @@ func NewBatchEventProcessor(options ...BPOptionConfig) *BatchEventProcessor { // Start does not do any initialization, just starts the ticker func (p *BatchEventProcessor) Start(ctx context.Context) { - pLogger.Info("Batch event processor started") + p.logger.Info("Batch event processor started") p.startTicker(ctx) } @@ -175,7 +175,7 @@ func (p *BatchEventProcessor) Start(ctx context.Context) { func (p *BatchEventProcessor) ProcessEvent(event UserEvent) bool { if p.Q.Size() >= p.MaxQueueSize { - pLogger.Warning("MaxQueueSize has been met. Discarding event") + p.logger.Warning("MaxQueueSize has been met. Discarding event") return false } @@ -188,7 +188,7 @@ func (p *BatchEventProcessor) ProcessEvent(event UserEvent) bool { if p.processing.TryAcquire(1) { // it doesn't matter if the timer has kicked in here. // we just want to start one go routine when the batch size is met. - pLogger.Debug("batch size reached. Flushing routine being called") + p.logger.Debug("batch size reached. Flushing routine being called") go func() { p.flushEvents() p.processing.Release(1) @@ -225,7 +225,7 @@ func (p *BatchEventProcessor) startTicker(ctx context.Context) { case <-p.Ticker.C: p.flushEvents() case <-ctx.Done(): - pLogger.Debug("Event processor stopped, flushing events.") + p.logger.Debug("Event processor stopped, flushing events.") p.flushEvents() d, ok := p.EventDispatcher.(*QueueEventDispatcher) if ok { @@ -265,7 +265,7 @@ func (p *BatchEventProcessor) flushEvents() { for p.eventsCount() > 0 { if failedToSend { - pLogger.Error("last Event Batch failed to send; retry on next flush", errors.New("dispatcher failed")) + p.logger.Error("last Event Batch failed to send; retry on next flush", errors.New("dispatcher failed")) break } events := p.getEvents(p.BatchSize) @@ -280,7 +280,7 @@ func (p *BatchEventProcessor) flushEvents() { } else { if !p.canBatch(&batchEvent, userEvent) { // this could happen if the project config was updated for instance. - pLogger.Info("Can't batch last event. Sending current batch.") + p.logger.Info("Can't batch last event. Sending current batch.") break } else { p.addToBatch(&batchEvent, createVisitorFromUserEvent(userEvent)) @@ -303,15 +303,15 @@ func (p *BatchEventProcessor) flushEvents() { err := notificationCenter.Send(notification.LogEvent, logEvent) if err != nil { - pLogger.Error("Send Log Event notification failed.", err) + p.logger.Error("Send Log Event notification failed.", err) } if success, _ := p.EventDispatcher.DispatchEvent(logEvent); success { - pLogger.Debug("Dispatched event successfully") + p.logger.Debug("Dispatched event successfully") p.remove(batchEventCount) batchEventCount = 0 batchEvent = Batch{} } else { - pLogger.Warning("Failed to dispatch event successfully") + p.logger.Warning("Failed to dispatch event successfully") failedToSend = true } } @@ -326,12 +326,12 @@ func (p *BatchEventProcessor) OnEventDispatch(callback func(logEvent LogEvent)) if ev, ok := payload.(LogEvent); ok { callback(ev) } else { - pLogger.Warning(fmt.Sprintf("Unable to convert notification payload %v into LogEventNotification", payload)) + p.logger.Warning(fmt.Sprintf("Unable to convert notification payload %v into LogEventNotification", payload)) } } id, err := notificationCenter.AddHandler(notification.LogEvent, handler) if err != nil { - pLogger.Error("Problem with adding notification handler.", err) + p.logger.Error("Problem with adding notification handler.", err) return 0, err } return id, nil @@ -342,7 +342,7 @@ func (p *BatchEventProcessor) RemoveOnEventDispatch(id int) error { notificationCenter := registry.GetNotificationCenter(p.sdkKey) if err := notificationCenter.RemoveHandler(id, notification.LogEvent); err != nil { - pLogger.Warning("Problem with removing notification handler.") + p.logger.Warning("Problem with removing notification handler.") return err } return nil diff --git a/pkg/logging/logger.go b/pkg/logging/logger.go index 098467d80..7d9a4b9ad 100644 --- a/pkg/logging/logger.go +++ b/pkg/logging/logger.go @@ -68,9 +68,9 @@ func SetLogLevel(logLevel LogLevel) { } // GetLogger returns a log producer with the given name -func GetLogger(name string) OptimizelyLogProducer { +func GetLogger(sdkKey string, name string) OptimizelyLogProducer { return NamedLogProducer{ - fields: map[string]interface{}{"name": name}, + fields: map[string]interface{}{"name": name, "sdkKey":sdkKey}, } } diff --git a/pkg/notification/center.go b/pkg/notification/center.go index a620fff5d..dd6abbfd6 100644 --- a/pkg/notification/center.go +++ b/pkg/notification/center.go @@ -17,7 +17,10 @@ // Package notification // package notification -import "fmt" +import ( + "fmt" + "github.com/optimizely/go-sdk/pkg/logging" +) // Center handles all notification listeners. It keeps track of the Manager for each type of notification. type Center interface { @@ -33,10 +36,10 @@ type DefaultCenter struct { // NewNotificationCenter returns a new notification center func NewNotificationCenter() *DefaultCenter { - decisionNotificationManager := NewAtomicManager() - projectConfigUpdateNotificationManager := NewAtomicManager() - processLogEventNotificationManager := NewAtomicManager() - trackNotificationManager := NewAtomicManager() + decisionNotificationManager := NewAtomicManager(logging.GetLogger("", "AtomicManager")) + projectConfigUpdateNotificationManager := NewAtomicManager(logging.GetLogger("", "AtomicManager")) + processLogEventNotificationManager := NewAtomicManager(logging.GetLogger("", "AtomicManager")) + trackNotificationManager := NewAtomicManager(logging.GetLogger("", "AtomicManager")) managerMap := make(map[Type]Manager) managerMap[Decision] = decisionNotificationManager managerMap[ProjectConfigUpdate] = projectConfigUpdateNotificationManager diff --git a/pkg/notification/manager.go b/pkg/notification/manager.go index 5b4aa6a14..3f0392b9c 100644 --- a/pkg/notification/manager.go +++ b/pkg/notification/manager.go @@ -19,14 +19,11 @@ package notification import ( "fmt" + "github.com/optimizely/go-sdk/pkg/logging" "sync" "sync/atomic" - - "github.com/optimizely/go-sdk/pkg/logging" ) -var managerLogger = logging.GetLogger("NotificationManager") - // Manager is a generic interface for managing notifications of a particular type type Manager interface { Add(func(interface{})) (int, error) @@ -39,11 +36,13 @@ type AtomicManager struct { handlers map[uint32]func(interface{}) counter uint32 lock sync.RWMutex + logger logging.OptimizelyLogProducer } // NewAtomicManager creates a new instance of the atomic manager -func NewAtomicManager() *AtomicManager { +func NewAtomicManager(logger logging.OptimizelyLogProducer) *AtomicManager { return &AtomicManager{ + logger: logger, handlers: make(map[uint32]func(interface{})), } } @@ -68,7 +67,7 @@ func (am *AtomicManager) Remove(id int) { delete(am.handlers, handlerID) return } - managerLogger.Debug(fmt.Sprintf("Handler for id:%d not found", id)) + am.logger.Debug(fmt.Sprintf("Handler for id:%d not found", id)) } diff --git a/pkg/utils/execgroup.go b/pkg/utils/execgroup.go index b44ceb134..266f7d92f 100644 --- a/pkg/utils/execgroup.go +++ b/pkg/utils/execgroup.go @@ -23,17 +23,16 @@ import ( "sync" ) -var logger = logging.GetLogger("ExecGroup") - // ExecGroup is a utility for managing graceful, blocking cancellation of goroutines. type ExecGroup struct { wg *sync.WaitGroup ctx context.Context cancelFunc context.CancelFunc + logger logging.OptimizelyLogProducer } // NewExecGroup returns constructed object -func NewExecGroup(ctx context.Context) *ExecGroup { +func NewExecGroup(logger logging.OptimizelyLogProducer, ctx context.Context) *ExecGroup { nctx, cancelFn := context.WithCancel(ctx) wg := sync.WaitGroup{} @@ -55,7 +54,7 @@ func (c ExecGroup) Go(f func(ctx context.Context)) { func (c ExecGroup) TerminateAndWait() { if c.cancelFunc == nil { - logger.Error("failed to shut down Execution Context properly", nil) + c.logger.Error("failed to shut down Execution Context properly", nil) return } c.cancelFunc() diff --git a/pkg/utils/requester.go b/pkg/utils/requester.go index 036aac818..ee0ad1327 100644 --- a/pkg/utils/requester.go +++ b/pkg/utils/requester.go @@ -21,19 +21,17 @@ import ( "bytes" "errors" "fmt" + "github.com/optimizely/go-sdk/pkg/logging" "io" "io/ioutil" "net/http" "time" - "github.com/optimizely/go-sdk/pkg/logging" - jsoniter "github.com/json-iterator/go" ) const defaultTTL = 5 * time.Second -var requesterLogger = logging.GetLogger("Requester") var json = jsoniter.ConfigCompatibleWithStandardLibrary // Requester is used to make outbound requests with @@ -79,16 +77,18 @@ type HTTPRequester struct { client http.Client retries int headers []Header + logger logging.OptimizelyLogProducer } // NewHTTPRequester makes Requester with api and parameters. Sets defaults // api has the base part of request's url, like http://localhost/api/v1 -func NewHTTPRequester(params ...func(*HTTPRequester)) *HTTPRequester { +func NewHTTPRequester(logger logging.OptimizelyLogProducer, params ...func(*HTTPRequester)) *HTTPRequester { res := HTTPRequester{ retries: 1, headers: []Header{{"Content-Type", "application/json"}, {"Accept", "application/json"}}, client: http.Client{Timeout: defaultTTL}, + logger: logger, } for _, param := range params { @@ -135,32 +135,32 @@ func (r HTTPRequester) Do(url, method string, body io.Reader, headers []Header) single := func(request *http.Request) (response []byte, responseHeaders http.Header, code int, e error) { resp, doErr := r.client.Do(request) if doErr != nil { - requesterLogger.Error(fmt.Sprintf("failed to send request %v", request), e) + r.logger.Error(fmt.Sprintf("failed to send request %v", request), e) return nil, http.Header{}, 0, doErr } defer func() { if e := resp.Body.Close(); e != nil { - requesterLogger.Warning(fmt.Sprintf("can't close body for %s request, %s", request.URL, e)) + r.logger.Warning(fmt.Sprintf("can't close body for %s request, %s", request.URL, e)) } }() if response, err = ioutil.ReadAll(resp.Body); err != nil { - requesterLogger.Error("failed to read body", err) + r.logger.Error("failed to read body", err) return nil, resp.Header, resp.StatusCode, err } if resp.StatusCode >= http.StatusBadRequest { - requesterLogger.Warning(fmt.Sprintf("error status code=%d", resp.StatusCode)) + r.logger.Warning(fmt.Sprintf("error status code=%d", resp.StatusCode)) return response, resp.Header, resp.StatusCode, errors.New(resp.Status) } return response, resp.Header, resp.StatusCode, nil } - requesterLogger.Debug(fmt.Sprintf("request %s", url)) + r.logger.Debug(fmt.Sprintf("request %s", url)) req, err := http.NewRequest(method, url, body) if err != nil { - requesterLogger.Error(fmt.Sprintf("failed to make request %s", url), err) + r.logger.Error(fmt.Sprintf("failed to make request %s", url), err) return nil, nil, 0, err } @@ -173,10 +173,10 @@ func (r HTTPRequester) Do(url, method string, body io.Reader, headers []Header) if i > 0 { triedMsg = fmt.Sprintf(", tried %d time(s)", i+1) } - requesterLogger.Debug(fmt.Sprintf("completed %s%s", url, triedMsg)) + r.logger.Debug(fmt.Sprintf("completed %s%s", url, triedMsg)) return response, responseHeaders, code, err } - requesterLogger.Debug(fmt.Sprintf("failed %s with %v", url, err)) + r.logger.Debug(fmt.Sprintf("failed %s with %v", url, err)) if i != r.retries { delay := time.Duration(500) * time.Millisecond From 0db3e581dd3aa87d98c64bce8058d3da5afbdbf8 Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Wed, 18 Mar 2020 17:01:25 -0700 Subject: [PATCH 02/28] start fixing tests --- pkg/client/client_test.go | 3 +- pkg/client/factory_test.go | 5 +-- .../datafileprojectconfig/config_test.go | 7 +++-- pkg/config/optimizely_config_test.go | 3 +- pkg/config/polling_manager_test.go | 31 ++++++++++--------- pkg/config/static_manager_test.go | 15 +++++---- .../composite_experiment_service_test.go | 4 +-- .../composite_feature_service_test.go | 4 +-- pkg/decision/composite_service_test.go | 22 ++++++------- .../experiment_override_service_test.go | 3 +- .../feature_experiment_service_test.go | 3 +- .../persisting_experiment_service_test.go | 13 +++++--- pkg/decision/rollout_service_test.go | 2 +- pkg/event/dispatcher_test.go | 6 ++-- pkg/event/processor_test.go | 6 ++-- pkg/logging/level_log_consumer.go | 15 +++++++-- pkg/logging/logger.go | 2 +- pkg/logging/logger_test.go | 8 ++--- 18 files changed, 89 insertions(+), 63 deletions(-) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index fa9b0bd03..c93201a01 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "github.com/optimizely/go-sdk/pkg/logging" "strconv" "sync" "testing" @@ -1988,7 +1989,7 @@ func TestClose(t *testing.T) { mockProcessor := &MockProcessor{} mockDecisionService := new(MockDecisionService) - eg := utils.NewExecGroup(context.Background()) + eg := utils.NewExecGroup(logging.GetLogger("", "ExecGroup"), context.Background()) wg := &sync.WaitGroup{} wg.Add(1) diff --git a/pkg/client/factory_test.go b/pkg/client/factory_test.go index b6dc7d068..631e8359f 100644 --- a/pkg/client/factory_test.go +++ b/pkg/client/factory_test.go @@ -19,6 +19,7 @@ package client import ( "context" "errors" + "github.com/optimizely/go-sdk/pkg/logging" "net/http" "sync" "testing" @@ -87,7 +88,7 @@ func TestClientWithPollingConfigManager(t *testing.T) { func TestClientWithProjectConfigManagerInOptions(t *testing.T) { factory := OptimizelyFactory{} projectConfig := datafileprojectconfig.DatafileProjectConfig{} - configManager := config.NewStaticProjectConfigManager(projectConfig) + configManager := config.NewStaticProjectConfigManager(projectConfig, logging.GetLogger("", "")) optimizelyClient, err := factory.Client(WithConfigManager(configManager)) assert.NoError(t, err) @@ -99,7 +100,7 @@ func TestClientWithProjectConfigManagerInOptions(t *testing.T) { func TestClientWithDecisionServiceAndEventProcessorInOptions(t *testing.T) { factory := OptimizelyFactory{} projectConfig := datafileprojectconfig.DatafileProjectConfig{} - configManager := config.NewStaticProjectConfigManager(projectConfig) + configManager := config.NewStaticProjectConfigManager(projectConfig, logging.GetLogger("", "StaticProjectConfigManager")) decisionService := new(MockDecisionService) processor := &event.BatchEventProcessor{ MaxQueueSize: 100, diff --git a/pkg/config/datafileprojectconfig/config_test.go b/pkg/config/datafileprojectconfig/config_test.go index 6b0a5f83e..fcd63c945 100644 --- a/pkg/config/datafileprojectconfig/config_test.go +++ b/pkg/config/datafileprojectconfig/config_test.go @@ -19,6 +19,7 @@ package datafileprojectconfig import ( "fmt" + "github.com/optimizely/go-sdk/pkg/logging" "testing" "github.com/optimizely/go-sdk/pkg/entities" @@ -27,12 +28,12 @@ import ( ) func TestNewDatafileProjectConfigNil(t *testing.T) { - projectConfig, err := NewDatafileProjectConfig(nil) + projectConfig, err := NewDatafileProjectConfig(logging.GetLogger("", "DatafileProjectConfig"), nil) assert.Error(t, err) assert.Nil(t, projectConfig) jsonDatafileStr := `{"accountID": "123", "revision": "1", "projectId": "12345", "version": "2"}` - projectConfig, err = NewDatafileProjectConfig([]byte(jsonDatafileStr)) + projectConfig, err = NewDatafileProjectConfig(logging.GetLogger("", "DatafileProjectConfig"), []byte(jsonDatafileStr)) assert.Error(t, err) assert.Nil(t, projectConfig) } @@ -41,7 +42,7 @@ func TestNewDatafileProjectConfigNotNil(t *testing.T) { dpc := DatafileProjectConfig{accountID: "123", revision: "1", projectID: "12345"} jsonDatafileStr := `{"accountID": "123", "revision": "1", "projectId": "12345", "version": "4"}` jsonDatafile := []byte(jsonDatafileStr) - projectConfig, err := NewDatafileProjectConfig(jsonDatafile) + projectConfig, err := NewDatafileProjectConfig(logging.GetLogger("", "DatafileProjectConfig"), jsonDatafile) assert.Nil(t, err) assert.NotNil(t, projectConfig) assert.Equal(t, dpc.accountID, projectConfig.accountID) diff --git a/pkg/config/optimizely_config_test.go b/pkg/config/optimizely_config_test.go index f1f17b1e2..3a6c8bf62 100644 --- a/pkg/config/optimizely_config_test.go +++ b/pkg/config/optimizely_config_test.go @@ -18,6 +18,7 @@ package config import ( "encoding/json" + "github.com/optimizely/go-sdk/pkg/logging" "io/ioutil" "testing" @@ -50,7 +51,7 @@ func (s *OptimizelyConfigTestSuite) SetupTest() { s.Fail("error opening file " + dataFileName) } - projectMgr, err := NewStaticProjectConfigManagerFromPayload(dataFile) + projectMgr, err := NewStaticProjectConfigManagerFromPayload(logging.GetLogger("", "NewStaticProjectConfigManagerFromPayload"), dataFile) if err != nil { s.Fail("error creating project manager") } diff --git a/pkg/config/polling_manager_test.go b/pkg/config/polling_manager_test.go index 8f7c2e8b9..979e1ba8e 100644 --- a/pkg/config/polling_manager_test.go +++ b/pkg/config/polling_manager_test.go @@ -18,6 +18,7 @@ package config import ( "context" + "github.com/optimizely/go-sdk/pkg/logging" "net/http" "sync/atomic" "testing" @@ -41,8 +42,10 @@ func (m *MockRequester) Get(uri string, headers ...utils.Header) (response []byt return args.Get(0).([]byte), args.Get(1).(http.Header), args.Int(2), args.Error(3) } +var logger = logging.GetLogger("polling_manager_test", "PollingManager") + func newExecGroup() *utils.ExecGroup { - return utils.NewExecGroup(context.Background()) + return utils.NewExecGroup(logger, context.Background()) } // assertion method to periodically check target function each tick. @@ -103,7 +106,7 @@ func TestNewAsyncPollingProjectConfigManagerWithOptions(t *testing.T) { func TestSyncConfigFetchesDatafileUsingRequester(t *testing.T) { mockDatafile := []byte(`{"revision":"42","version": "4"}`) - projectConfig, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile) + projectConfig, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile) mockRequester := new(MockRequester) mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile, http.Header{}, http.StatusOK, nil) @@ -150,7 +153,7 @@ func TestNewPollingProjectConfigManagerWithSimilarDatafileRevisions(t *testing.T // Test newer datafile should not replace the older one if revisions are the same mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`) mockDatafile2 := []byte(`{"revision":"42","botFiltering":false,"version": "4"}`) - projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1) + projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile1) mockRequester := new(MockRequester) mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile2, http.Header{}, http.StatusOK, nil) @@ -185,7 +188,7 @@ func TestNewAsyncPollingProjectConfigManagerWithSimilarDatafileRevisions(t *test // Test newer datafile should not replace the older one if revisions are the same mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`) mockDatafile2 := []byte(`{"revision":"42","botFiltering":false,"version": "4"}`) - projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1) + projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile1) mockRequester := new(MockRequester) mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile2, http.Header{}, http.StatusOK, nil) @@ -218,7 +221,7 @@ func TestNewAsyncPollingProjectConfigManagerWithSimilarDatafileRevisions(t *test func TestNewPollingProjectConfigManagerWithLastModifiedDates(t *testing.T) { mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`) - projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1) + projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile1) mockRequester := new(MockRequester) modifiedDate := "Wed, 16 Oct 2019 20:16:45 GMT" responseHeaders := http.Header{} @@ -272,8 +275,8 @@ func TestNewPollingProjectConfigManagerWithDifferentDatafileRevisions(t *testing // Test newer datafile should replace the older one if revisions are different mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`) mockDatafile2 := []byte(`{"revision":"43","botFiltering":false,"version": "4"}`) - projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1) - projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile2) + projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile1) + projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile2) mockRequester := new(MockRequester) mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile2, http.Header{}, http.StatusOK, nil) @@ -308,8 +311,8 @@ func TestNewAsyncPollingProjectConfigManagerWithDifferentDatafileRevisions(t *te // Test newer datafile should replace the older one if revisions are different mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`) mockDatafile2 := []byte(`{"revision":"43","botFiltering":false,"version": "4"}`) - projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1) - projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile2) + projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile1) + projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile2) mockRequester := new(MockRequester) mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile2, http.Header{}, http.StatusOK, nil) @@ -344,8 +347,8 @@ func TestNewPollingProjectConfigManagerWithErrorHandling(t *testing.T) { mockDatafile1 := []byte("NOT-VALID") mockDatafile2 := []byte(`{"revision":"43","botFiltering":false,"version": "4"}`) - projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1) - projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile2) + projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile1) + projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile2) mockRequester := new(MockRequester) mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile1, http.Header{}, http.StatusOK, nil).Times(1) @@ -377,8 +380,8 @@ func TestNewAsyncPollingProjectConfigManagerWithErrorHandling(t *testing.T) { mockDatafile1 := []byte("NOT-VALID") mockDatafile2 := []byte(`{"revision":"43","botFiltering":false,"version": "4"}`) - projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1) - projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile2) + projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile1) + projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile2) mockRequester := new(MockRequester) mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile1, http.Header{}, http.StatusOK, nil).Times(1) @@ -442,7 +445,7 @@ func TestNewPollingProjectConfigManagerOnDecision(t *testing.T) { func TestNewAsyncPollingProjectConfigManagerOnDecision(t *testing.T) { mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`) - projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1) + projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile1) mockRequester := new(MockRequester) mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile1, http.Header{}, http.StatusOK, nil) diff --git a/pkg/config/static_manager_test.go b/pkg/config/static_manager_test.go index 03d645378..17cf87182 100644 --- a/pkg/config/static_manager_test.go +++ b/pkg/config/static_manager_test.go @@ -18,6 +18,7 @@ package config import ( "errors" + "github.com/optimizely/go-sdk/pkg/logging" "github.com/optimizely/go-sdk/pkg/notification" "testing" @@ -25,9 +26,11 @@ import ( "github.com/stretchr/testify/assert" ) +var tlogger = logging.GetLogger("", "staticManager") + func TestNewStaticProjectConfigManager(t *testing.T) { projectConfig := datafileprojectconfig.DatafileProjectConfig{} - configManager := NewStaticProjectConfigManager(projectConfig) + configManager := NewStaticProjectConfigManager(projectConfig, tlogger) actual, _ := configManager.GetConfig() assert.Equal(t, projectConfig, actual) @@ -36,15 +39,15 @@ func TestNewStaticProjectConfigManager(t *testing.T) { func TestNewStaticProjectConfigManagerFromPayload(t *testing.T) { mockDatafile := []byte(`{"accountId":"42","projectId":"123""}`) - configManager, err := NewStaticProjectConfigManagerFromPayload(mockDatafile) + configManager, err := NewStaticProjectConfigManagerFromPayload(tlogger, mockDatafile) assert.Error(t, err) mockDatafile = []byte(`{"accountId":"42","projectId":"123",}`) - configManager, err = NewStaticProjectConfigManagerFromPayload(mockDatafile) + configManager, err = NewStaticProjectConfigManagerFromPayload(tlogger, mockDatafile) assert.Error(t, err) mockDatafile = []byte(`{"accountId":"42","projectId":"123","version":"4"}`) - configManager, err = NewStaticProjectConfigManagerFromPayload(mockDatafile) + configManager, err = NewStaticProjectConfigManagerFromPayload(tlogger, mockDatafile) assert.Nil(t, err) assert.Nil(t, configManager.optimizelyConfig) @@ -56,7 +59,7 @@ func TestNewStaticProjectConfigManagerFromPayload(t *testing.T) { func TestStaticGetOptimizelyConfig(t *testing.T) { mockDatafile := []byte(`{"accountId":"42","projectId":"123","version":"4"}`) - configManager, err := NewStaticProjectConfigManagerFromPayload(mockDatafile) + configManager, err := NewStaticProjectConfigManagerFromPayload(tlogger, mockDatafile) assert.Nil(t, err) assert.Nil(t, configManager.optimizelyConfig) @@ -75,7 +78,7 @@ func TestNewStaticProjectConfigManagerFromURL(t *testing.T) { func TestNewStaticProjectConfigManagerOnDecision(t *testing.T) { mockDatafile := []byte(`{"accountId":"42","projectId":"123","version":"4"}`) - configManager, err := NewStaticProjectConfigManagerFromPayload(mockDatafile) + configManager, err := NewStaticProjectConfigManagerFromPayload(tlogger, mockDatafile) assert.Nil(t, err) callback := func(notification notification.ProjectConfigUpdateNotification) { diff --git a/pkg/decision/composite_experiment_service_test.go b/pkg/decision/composite_experiment_service_test.go index 59210b8c6..f6ef383bc 100644 --- a/pkg/decision/composite_experiment_service_test.go +++ b/pkg/decision/composite_experiment_service_test.go @@ -152,7 +152,7 @@ func (s *CompositeExperimentTestSuite) TestGetDecisionReturnsError() { func (s *CompositeExperimentTestSuite) TestNewCompositeExperimentService() { // Assert that the service is instantiated with the correct child services in the right order - compositeExperimentService := NewCompositeExperimentService() + compositeExperimentService := NewCompositeExperimentService("") s.Equal(2, len(compositeExperimentService.experimentServices)) s.IsType(&ExperimentWhitelistService{}, compositeExperimentService.experimentServices[0]) s.IsType(&ExperimentBucketerService{}, compositeExperimentService.experimentServices[1]) @@ -161,7 +161,7 @@ func (s *CompositeExperimentTestSuite) TestNewCompositeExperimentService() { func (s *CompositeExperimentTestSuite) TestNewCompositeExperimentServiceWithCustomOptions() { mockUserProfileService := new(MockUserProfileService) mockExperimentOverrideStore := new(MapExperimentOverridesStore) - compositeExperimentService := NewCompositeExperimentService( + compositeExperimentService := NewCompositeExperimentService("", WithUserProfileService(mockUserProfileService), WithOverrideStore(mockExperimentOverrideStore), ) diff --git a/pkg/decision/composite_feature_service_test.go b/pkg/decision/composite_feature_service_test.go index bfc2a27e0..242e3ea0f 100644 --- a/pkg/decision/composite_feature_service_test.go +++ b/pkg/decision/composite_feature_service_test.go @@ -157,8 +157,8 @@ func (s *CompositeFeatureServiceTestSuite) TestGetDecisionReturnsLastDecisionWit func (s *CompositeFeatureServiceTestSuite) TestNewCompositeFeatureService() { // Assert that the service is instantiated with the correct child services in the right order - compositeExperimentService := NewCompositeExperimentService() - compositeFeatureService := NewCompositeFeatureService(compositeExperimentService) + compositeExperimentService := NewCompositeExperimentService("") + compositeFeatureService := NewCompositeFeatureService("", compositeExperimentService) s.Equal(2, len(compositeFeatureService.featureServices)) s.IsType(&FeatureExperimentService{compositeExperimentService: compositeExperimentService}, compositeFeatureService.featureServices[0]) s.IsType(&RolloutService{}, compositeFeatureService.featureServices[1]) diff --git a/pkg/decision/composite_service_test.go b/pkg/decision/composite_service_test.go index 2c13a9917..001cab17d 100644 --- a/pkg/decision/composite_service_test.go +++ b/pkg/decision/composite_service_test.go @@ -94,8 +94,8 @@ func (s *CompositeServiceFeatureTestSuite) TestDecisionListeners() { func (s *CompositeServiceFeatureTestSuite) TestDecisionListenersNotificationWithFloatVariable() { - compositeExperimentService := NewCompositeExperimentService() - compositeFeatureDecisionService := NewCompositeFeatureService(compositeExperimentService) + compositeExperimentService := NewCompositeExperimentService("") + compositeFeatureDecisionService := NewCompositeFeatureService("", compositeExperimentService) s.decisionContext.Variable = entities.Variable{ DefaultValue: "23.34", ID: "1", @@ -132,8 +132,8 @@ func (s *CompositeServiceFeatureTestSuite) TestDecisionListenersNotificationWith func (s *CompositeServiceFeatureTestSuite) TestDecisionListenersNotificationWithIntegerVariable() { - compositeExperimentService := NewCompositeExperimentService() - compositeFeatureDecisionService := NewCompositeFeatureService(compositeExperimentService) + compositeExperimentService := NewCompositeExperimentService("") + compositeFeatureDecisionService := NewCompositeFeatureService("", compositeExperimentService) s.decisionContext.Variable = entities.Variable{ DefaultValue: "23", ID: "1", @@ -170,8 +170,8 @@ func (s *CompositeServiceFeatureTestSuite) TestDecisionListenersNotificationWith func (s *CompositeServiceFeatureTestSuite) TestDecisionListenersNotificationWithBoolVariable() { - compositeExperimentService := NewCompositeExperimentService() - compositeFeatureDecisionService := NewCompositeFeatureService(compositeExperimentService) + compositeExperimentService := NewCompositeExperimentService("") + compositeFeatureDecisionService := NewCompositeFeatureService("", compositeExperimentService) s.decisionContext.Variable = entities.Variable{ DefaultValue: "true", ID: "1", @@ -208,8 +208,8 @@ func (s *CompositeServiceFeatureTestSuite) TestDecisionListenersNotificationWith func (s *CompositeServiceFeatureTestSuite) TestDecisionListenersNotificationWithWrongTypelVariable() { - compositeExperimentService := NewCompositeExperimentService() - compositeFeatureDecisionService := NewCompositeFeatureService(compositeExperimentService) + compositeExperimentService := NewCompositeExperimentService("") + compositeFeatureDecisionService := NewCompositeFeatureService("", compositeExperimentService) s.decisionContext.Variable = entities.Variable{ DefaultValue: "string", ID: "1", @@ -246,8 +246,8 @@ func (s *CompositeServiceFeatureTestSuite) TestDecisionListenersNotificationWith func (s *CompositeServiceFeatureTestSuite) TestDecisionListenersNotificationWithNoVariable() { - compositeExperimentService := NewCompositeExperimentService() - compositeFeatureDecisionService := NewCompositeFeatureService(compositeExperimentService) + compositeExperimentService := NewCompositeExperimentService("") + compositeFeatureDecisionService := NewCompositeFeatureService("", compositeExperimentService) s.decisionContext.Variable = entities.Variable{} //no variable decisionService := &CompositeService{ @@ -284,7 +284,7 @@ func (s *CompositeServiceFeatureTestSuite) TestNewCompositeService() { } func (s *CompositeServiceFeatureTestSuite) TestNewCompositeServiceWithCustomOptions() { - compositeExperimentService := NewCompositeExperimentService() + compositeExperimentService := NewCompositeExperimentService("") compositeService := NewCompositeService("sdk_key", WithCompositeExperimentService(compositeExperimentService)) s.IsType(compositeExperimentService, compositeService.compositeExperimentService) s.IsType(&CompositeFeatureService{}, compositeService.compositeFeatureService) diff --git a/pkg/decision/experiment_override_service_test.go b/pkg/decision/experiment_override_service_test.go index 8f930008c..e2e077191 100644 --- a/pkg/decision/experiment_override_service_test.go +++ b/pkg/decision/experiment_override_service_test.go @@ -18,6 +18,7 @@ package decision import ( + "github.com/optimizely/go-sdk/pkg/logging" "sync" "testing" @@ -36,7 +37,7 @@ type ExperimentOverrideServiceTestSuite struct { func (s *ExperimentOverrideServiceTestSuite) SetupTest() { s.mockConfig = new(mockProjectConfig) s.overrides = NewMapExperimentOverridesStore() - s.overrideService = NewExperimentOverrideService(s.overrides) + s.overrideService = NewExperimentOverrideService(logging.GetLogger("", ""), s.overrides) } func (s *ExperimentOverrideServiceTestSuite) TestOverridesIncludeVariation() { diff --git a/pkg/decision/feature_experiment_service_test.go b/pkg/decision/feature_experiment_service_test.go index 5b6b7a6bb..02b9388c6 100644 --- a/pkg/decision/feature_experiment_service_test.go +++ b/pkg/decision/feature_experiment_service_test.go @@ -17,6 +17,7 @@ package decision import ( + "github.com/optimizely/go-sdk/pkg/logging" "testing" "github.com/optimizely/go-sdk/pkg/entities" @@ -110,7 +111,7 @@ func (s *FeatureExperimentServiceTestSuite) TestGetDecisionMutex() { func (s *FeatureExperimentServiceTestSuite) TestNewFeatureExperimentService() { compositeExperimentService := &CompositeExperimentService{} - featureExperimentService := NewFeatureExperimentService(compositeExperimentService) + featureExperimentService := NewFeatureExperimentService(logging.GetLogger("", ""), compositeExperimentService) s.IsType(compositeExperimentService, featureExperimentService.compositeExperimentService) } diff --git a/pkg/decision/persisting_experiment_service_test.go b/pkg/decision/persisting_experiment_service_test.go index 434fafa60..7745f8a4a 100644 --- a/pkg/decision/persisting_experiment_service_test.go +++ b/pkg/decision/persisting_experiment_service_test.go @@ -18,6 +18,7 @@ package decision import ( + "github.com/optimizely/go-sdk/pkg/logging" "testing" "github.com/optimizely/go-sdk/pkg/entities" @@ -55,7 +56,8 @@ func (s *PersistingExperimentServiceTestSuite) SetupTest() { } func (s *PersistingExperimentServiceTestSuite) TestNilUserProfileService() { - persistingExperimentService := NewPersistingExperimentService(s.mockExperimentService, nil) + persistingExperimentService := NewPersistingExperimentService(logging.GetLogger("", "NewPersistingExperimentService"), + s.mockExperimentService, nil) decision, err := persistingExperimentService.GetDecision(s.testDecisionContext, testUserContext) s.Equal(s.testComputedDecision, decision) s.NoError(err) @@ -71,7 +73,8 @@ func (s *PersistingExperimentServiceTestSuite) TestSavedVariationFound() { s.mockUserProfileService.On("Lookup", testUserContext.ID).Return(savedUserProfile) s.mockUserProfileService.On("Save", mock.Anything) - persistingExperimentService := NewPersistingExperimentService(s.mockExperimentService, s.mockUserProfileService) + persistingExperimentService := NewPersistingExperimentService(logging.GetLogger("", "NewPersistingExperimentService"), + s.mockExperimentService, s.mockUserProfileService) decision, err := persistingExperimentService.GetDecision(s.testDecisionContext, testUserContext) savedDecision := ExperimentDecision{ Variation: &testExp1113Var2224, @@ -91,7 +94,8 @@ func (s *PersistingExperimentServiceTestSuite) TestNoSavedVariation() { } s.mockUserProfileService.On("Save", updatedUserProfile) - persistingExperimentService := NewPersistingExperimentService(s.mockExperimentService, s.mockUserProfileService) + persistingExperimentService := NewPersistingExperimentService(logging.GetLogger("", "NewPersistingExperimentService"), + s.mockExperimentService, s.mockUserProfileService) decision, err := persistingExperimentService.GetDecision(s.testDecisionContext, testUserContext) s.Equal(s.testComputedDecision, decision) s.NoError(err) @@ -112,7 +116,8 @@ func (s *PersistingExperimentServiceTestSuite) TestSavedVariationNoLongerValid() ExperimentBucketMap: map[UserDecisionKey]string{decisionKey: s.testComputedDecision.Variation.ID}, } s.mockUserProfileService.On("Save", updatedUserProfile) - persistingExperimentService := NewPersistingExperimentService(s.mockExperimentService, s.mockUserProfileService) + persistingExperimentService := NewPersistingExperimentService(logging.GetLogger("", "NewPersistingExperimentService"), + s.mockExperimentService, s.mockUserProfileService) decision, err := persistingExperimentService.GetDecision(s.testDecisionContext, testUserContext) s.Equal(s.testComputedDecision, decision) s.NoError(err) diff --git a/pkg/decision/rollout_service_test.go b/pkg/decision/rollout_service_test.go index 241db7a28..1707538dd 100644 --- a/pkg/decision/rollout_service_test.go +++ b/pkg/decision/rollout_service_test.go @@ -134,7 +134,7 @@ func (s *RolloutServiceTestSuite) TestGetDecisionFailsTargeting() { } func TestNewRolloutService(t *testing.T) { - rolloutService := NewRolloutService() + rolloutService := NewRolloutService("") assert.IsType(t, &evaluator.MixedTreeEvaluator{}, rolloutService.audienceTreeEvaluator) assert.IsType(t, &ExperimentBucketerService{}, rolloutService.experimentBucketerService) } diff --git a/pkg/event/dispatcher_test.go b/pkg/event/dispatcher_test.go index 26867b7f7..c2e86c20e 100644 --- a/pkg/event/dispatcher_test.go +++ b/pkg/event/dispatcher_test.go @@ -103,7 +103,7 @@ func NewMetricsRegistry() *MetricsRegistry { func TestQueueEventDispatcher_DispatchEvent(t *testing.T) { metricsRegistry := NewMetricsRegistry() - q := NewQueueEventDispatcher(metricsRegistry) + q := NewQueueEventDispatcher("", metricsRegistry) sender := &MockDispatcher{Events: NewInMemoryQueue(100)} q.Dispatcher = sender @@ -139,7 +139,7 @@ func TestQueueEventDispatcher_DispatchEvent(t *testing.T) { func TestQueueEventDispatcher_InvalidEvent(t *testing.T) { metricsRegistry := NewMetricsRegistry() - q := NewQueueEventDispatcher(metricsRegistry) + q := NewQueueEventDispatcher("", metricsRegistry) q.Dispatcher = &MockDispatcher{Events: NewInMemoryQueue(100)} config := TestConfig{} @@ -162,7 +162,7 @@ func TestQueueEventDispatcher_InvalidEvent(t *testing.T) { func TestQueueEventDispatcher_FailDispath(t *testing.T) { metricsRegistry := NewMetricsRegistry() - q := NewQueueEventDispatcher(metricsRegistry) + q := NewQueueEventDispatcher("", metricsRegistry) q.Dispatcher = &MockDispatcher{ShouldFail: true, Events: NewInMemoryQueue(100)} eventTags := map[string]interface{}{"revenue": 55.0, "value": 25.1} diff --git a/pkg/event/processor_test.go b/pkg/event/processor_test.go index bb1a478c8..210badd94 100644 --- a/pkg/event/processor_test.go +++ b/pkg/event/processor_test.go @@ -59,13 +59,13 @@ func NewMockDispatcher(queueSize int, shouldFail bool) *MockDispatcher { } func newExecutionContext() *utils.ExecGroup { - return utils.NewExecGroup(context.Background()) + return utils.NewExecGroup(logging.GetLogger("", "NewExecGroup"), context.Background()) } func TestDefaultEventProcessor_ProcessImpression(t *testing.T) { eg := newExecutionContext() processor := NewBatchEventProcessor() - processor.EventDispatcher = NewQueueEventDispatcher(processor.metricsRegistry) + processor.EventDispatcher = NewQueueEventDispatcher("", processor.metricsRegistry) eg.Go(processor.Start) impression := BuildTestImpressionEvent() @@ -425,7 +425,7 @@ func TestChanQueueEventProcessor_ProcessImpression(t *testing.T) { processor := NewBatchEventProcessor( WithQueueSize(100), WithQueue(NewInMemoryQueue(100)), - WithEventDispatcher(&HTTPEventDispatcher{requester: utils.NewHTTPRequester()})) + WithEventDispatcher(&HTTPEventDispatcher{requester: utils.NewHTTPRequester(logging.GetLogger("", "NewHTTPRequester"))})) eg.Go(processor.Start) diff --git a/pkg/logging/level_log_consumer.go b/pkg/logging/level_log_consumer.go index 35ca5a1cd..b18d409db 100644 --- a/pkg/logging/level_log_consumer.go +++ b/pkg/logging/level_log_consumer.go @@ -18,9 +18,9 @@ package logging import ( - "fmt" "io" "log" + "strings" ) // FilteredLevelLogConsumer is an implementation of the OptimizelyLogConsumer that filters by log level @@ -33,8 +33,17 @@ type FilteredLevelLogConsumer struct { func (l *FilteredLevelLogConsumer) Log(level LogLevel, message string, fields map[string]interface{}) { if l.level <= level { // prepends the name and log level to the message - message = fmt.Sprintf("[%s][%s] %s", level, fields["name"], message) - l.logger.Println(message) + messBuilder := strings.Builder{} + for _, v := range fields { + if s, ok := v.(string); ok { + messBuilder.WriteString("[") + messBuilder.WriteString(s) + messBuilder.WriteString("]") + } + } + messBuilder.WriteString(" ") + messBuilder.WriteString(message) + l.logger.Println(messBuilder.String()) } } diff --git a/pkg/logging/logger.go b/pkg/logging/logger.go index 7d9a4b9ad..004322a61 100644 --- a/pkg/logging/logger.go +++ b/pkg/logging/logger.go @@ -70,7 +70,7 @@ func SetLogLevel(logLevel LogLevel) { // GetLogger returns a log producer with the given name func GetLogger(sdkKey string, name string) OptimizelyLogProducer { return NamedLogProducer{ - fields: map[string]interface{}{"name": name, "sdkKey":sdkKey}, + fields: map[string]interface{}{"sdkKey":sdkKey, "name": name}, } } diff --git a/pkg/logging/logger_test.go b/pkg/logging/logger_test.go index ab41de67e..24a439750 100644 --- a/pkg/logging/logger_test.go +++ b/pkg/logging/logger_test.go @@ -46,7 +46,7 @@ func TestNamedLoggerDebug(t *testing.T) { SetLogger(testLogger) - logProducer := GetLogger(testLogName) + logProducer := GetLogger("", testLogName) logProducer.Debug(testLogMessage) testLogger.AssertExpectations(t) assert.Equal(t, []string{testLogMessage}, testLogger.loggedMessages) @@ -60,7 +60,7 @@ func TestNamedLoggerInfo(t *testing.T) { SetLogger(testLogger) - logProducer := GetLogger(testLogName) + logProducer := GetLogger("testSdkKey", testLogName) logProducer.Info(testLogMessage) testLogger.AssertExpectations(t) assert.Equal(t, []string{testLogMessage}, testLogger.loggedMessages) @@ -74,7 +74,7 @@ func TestNamedLoggerWarning(t *testing.T) { SetLogger(testLogger) - logProducer := GetLogger(testLogName) + logProducer := GetLogger("testSdkKey", testLogName) logProducer.Warning(testLogMessage) testLogger.AssertExpectations(t) assert.Equal(t, []string{testLogMessage}, testLogger.loggedMessages) @@ -89,7 +89,7 @@ func TestNamedLoggerError(t *testing.T) { SetLogger(testLogger) err := errors.New("I am an error object") - logProducer := GetLogger(testLogName) + logProducer := GetLogger("", testLogName) logProducer.Error(testLogMessage, err) testLogger.AssertExpectations(t) assert.Equal(t, []string{expectedLogMessage}, testLogger.loggedMessages) From 3e5cbf2ddd997aad2745a303596c300edfc9fdd9 Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Wed, 18 Mar 2020 17:37:11 -0700 Subject: [PATCH 03/28] fix more tests --- pkg/client/client_test.go | 56 +++++++++++++++++++ .../bucketer/murmurhashbucketer_test.go | 2 +- pkg/logging/level_log_consumer.go | 3 + pkg/notification/manager_test.go | 7 ++- pkg/utils/execgroup_test.go | 5 +- pkg/utils/requester_test.go | 35 ++++++------ 6 files changed, 85 insertions(+), 23 deletions(-) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index c93201a01..32b4499b5 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -148,6 +148,7 @@ func TestTrack(t *testing.T) { ConfigManager: ValidProjectConfigManager(), DecisionService: mockDecisionService, EventProcessor: mockProcessor, + logger:logging.GetLogger("", ""), } err := client.Track("sample_conversion", entities.UserContext{ID: "1212121", Attributes: map[string]interface{}{}}, map[string]interface{}{}) @@ -167,6 +168,7 @@ func TestTrackFailEventNotFound(t *testing.T) { ConfigManager: ValidProjectConfigManager(), DecisionService: mockDecisionService, EventProcessor: mockProcessor, + logger:logging.GetLogger("", ""), } err := client.Track("bob", entities.UserContext{ID: "1212121", Attributes: map[string]interface{}{}}, map[string]interface{}{}) @@ -184,6 +186,7 @@ func TestTrackPanics(t *testing.T) { ConfigManager: new(PanickingConfigManager), DecisionService: mockDecisionService, EventProcessor: mockProcessor, + logger:logging.GetLogger("", ""), } err := client.Track("bob", entities.UserContext{ID: "1212121", Attributes: map[string]interface{}{}}, map[string]interface{}{}) @@ -200,6 +203,7 @@ func TestGetEnabledFeaturesPanic(t *testing.T) { client := OptimizelyClient{ ConfigManager: &PanickingConfigManager{}, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } // ensure that the client calms back down and recovers @@ -246,6 +250,7 @@ func TestGetFeatureVariableBooleanWithValidValue(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } result, _ := client.GetFeatureVariableBoolean(testFeatureKey, testVariableKey, testUserContext) assert.Equal(t, true, result) @@ -292,6 +297,7 @@ func TestGetFeatureVariableBooleanWithInvalidValue(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } result, err := client.GetFeatureVariableBoolean(testFeatureKey, testVariableKey, testUserContext) assert.Equal(t, false, result) @@ -339,6 +345,7 @@ func TestGetFeatureVariableBooleanWithInvalidValueType(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } result, err := client.GetFeatureVariableBoolean(testFeatureKey, testVariableKey, testUserContext) assert.Equal(t, false, result) @@ -386,6 +393,7 @@ func TestGetFeatureVariableBooleanWithEmptyValueType(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } result, err := client.GetFeatureVariableBoolean(testFeatureKey, testVariableKey, testUserContext) assert.Equal(t, false, result) @@ -433,6 +441,7 @@ func TestGetFeatureVariableBooleanReturnsDefaultValueIfFeatureNotEnabled(t *test client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } result, err := client.GetFeatureVariableBoolean(testFeatureKey, testVariableKey, testUserContext) assert.Equal(t, false, result) @@ -452,6 +461,7 @@ func TestGetFeatureVariableBoolPanic(t *testing.T) { client := OptimizelyClient{ ConfigManager: &PanickingConfigManager{}, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } // ensure that the client calms back down and recovers @@ -498,6 +508,7 @@ func TestGetFeatureVariableDoubleWithValidValue(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } result, _ := client.GetFeatureVariableDouble(testFeatureKey, testVariableKey, testUserContext) assert.Equal(t, float64(5), result) @@ -544,6 +555,7 @@ func TestGetFeatureVariableDoubleWithInvalidValue(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } result, err := client.GetFeatureVariableDouble(testFeatureKey, testVariableKey, testUserContext) assert.Equal(t, float64(0), result) @@ -591,6 +603,7 @@ func TestGetFeatureVariableDoubleWithInvalidValueType(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } result, err := client.GetFeatureVariableDouble(testFeatureKey, testVariableKey, testUserContext) assert.Equal(t, float64(0), result) @@ -638,6 +651,7 @@ func TestGetFeatureVariableDoubleWithEmptyValueType(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } result, err := client.GetFeatureVariableDouble(testFeatureKey, testVariableKey, testUserContext) assert.Equal(t, float64(0), result) @@ -685,6 +699,7 @@ func TestGetFeatureVariableDoubleReturnsDefaultValueIfFeatureNotEnabled(t *testi client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } result, err := client.GetFeatureVariableDouble(testFeatureKey, testVariableKey, testUserContext) assert.Equal(t, float64(4), result) @@ -704,6 +719,7 @@ func TestGetFeatureVariableDoublePanic(t *testing.T) { client := OptimizelyClient{ ConfigManager: &PanickingConfigManager{}, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } // ensure that the client calms back down and recovers @@ -750,6 +766,7 @@ func TestGetFeatureVariableIntegerWithValidValue(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } result, _ := client.GetFeatureVariableInteger(testFeatureKey, testVariableKey, testUserContext) assert.Equal(t, 5, result) @@ -796,6 +813,7 @@ func TestGetFeatureVariableIntegerWithInvalidValue(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } result, err := client.GetFeatureVariableInteger(testFeatureKey, testVariableKey, testUserContext) assert.Equal(t, 0, result) @@ -843,6 +861,7 @@ func TestGetFeatureVariableIntegerWithInvalidValueType(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } result, err := client.GetFeatureVariableInteger(testFeatureKey, testVariableKey, testUserContext) assert.Equal(t, 0, result) @@ -890,6 +909,7 @@ func TestGetFeatureVariableIntegerWithEmptyValueType(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } result, err := client.GetFeatureVariableInteger(testFeatureKey, testVariableKey, testUserContext) assert.Equal(t, 0, result) @@ -937,6 +957,8 @@ func TestGetFeatureVariableIntegerReturnsDefaultValueIfFeatureNotEnabled(t *test client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), + } result, err := client.GetFeatureVariableInteger(testFeatureKey, testVariableKey, testUserContext) assert.Equal(t, 4, result) @@ -956,6 +978,7 @@ func TestGetFeatureVariableIntegerPanic(t *testing.T) { client := OptimizelyClient{ ConfigManager: &PanickingConfigManager{}, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } // ensure that the client calms back down and recovers @@ -1002,6 +1025,7 @@ func TestGetFeatureVariableStringWithValidValue(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } result, _ := client.GetFeatureVariableString(testFeatureKey, testVariableKey, testUserContext) assert.Equal(t, testVariableValue, result) @@ -1048,6 +1072,7 @@ func TestGetFeatureVariableStringWithInvalidValueType(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } result, err := client.GetFeatureVariableString(testFeatureKey, testVariableKey, testUserContext) assert.Equal(t, "", result) @@ -1095,6 +1120,7 @@ func TestGetFeatureVariableStringWithEmptyValueType(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } result, err := client.GetFeatureVariableString(testFeatureKey, testVariableKey, testUserContext) assert.Equal(t, "", result) @@ -1142,6 +1168,7 @@ func TestGetFeatureVariableStringReturnsDefaultValueIfFeatureNotEnabled(t *testi client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } result, err := client.GetFeatureVariableString(testFeatureKey, testVariableKey, testUserContext) assert.Equal(t, "defaultString", result) @@ -1161,6 +1188,7 @@ func TestGetFeatureVariableStringPanic(t *testing.T) { client := OptimizelyClient{ ConfigManager: &PanickingConfigManager{}, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } // ensure that the client calms back down and recovers @@ -1179,6 +1207,7 @@ func TestGetFeatureVariableErrorCases(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } _, err1 := client.GetFeatureVariableBoolean("test_feature_key", "test_variable_key", testUserContext) _, err2 := client.GetFeatureVariableDouble("test_feature_key", "test_variable_key", testUserContext) @@ -1198,6 +1227,7 @@ func TestGetProjectConfigIsValid(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, + logger:logging.GetLogger("", ""), } actual, err := client.getProjectConfig() @@ -1210,6 +1240,7 @@ func TestGetProjectConfigIsInValid(t *testing.T) { client := OptimizelyClient{ ConfigManager: InValidProjectConfigManager(), + logger:logging.GetLogger("", ""), } actual, err := client.getProjectConfig() @@ -1223,6 +1254,7 @@ func TestGetOptimizelyConfig(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, + logger:logging.GetLogger("", ""), } optimizelyConfig := client.GetOptimizelyConfig() @@ -1268,6 +1300,7 @@ func TestGetFeatureDecisionValid(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } _, featureDecision, err := client.getFeatureDecision(testFeatureKey, testVariableKey, testUserContext) @@ -1313,6 +1346,7 @@ func TestGetFeatureDecisionErrProjectConfig(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } _, _, err := client.getFeatureDecision(testFeatureKey, testVariableKey, testUserContext) @@ -1356,6 +1390,7 @@ func TestGetFeatureDecisionPanicProjectConfig(t *testing.T) { client := OptimizelyClient{ ConfigManager: &PanickingConfigManager{}, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } _, _, err := client.getFeatureDecision(testFeatureKey, testVariableKey, testUserContext) @@ -1390,6 +1425,7 @@ func TestGetFeatureDecisionPanicDecisionService(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: &PanickingDecisionService{}, + logger:logging.GetLogger("", ""), } _, _, err := client.getFeatureDecision(testFeatureKey, testVariableKey, testUserContext) @@ -1435,6 +1471,7 @@ func TestGetFeatureDecisionErrFeatureDecision(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } _, decision, err := client.getFeatureDecision(testFeatureKey, testVariableKey, testUserContext) @@ -1513,6 +1550,7 @@ func TestGetAllFeatureVariables(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } enabled, variationMap, err := client.GetAllFeatureVariables(testFeatureKey, testUserContext) @@ -1562,6 +1600,7 @@ func TestGetAllFeatureVariablesWithError(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } enabled, variationMap, err := client.GetAllFeatureVariables(testFeatureKey, testUserContext) @@ -1585,6 +1624,7 @@ func TestGetAllFeatureVariablesWithoutFeature(t *testing.T) { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: mockDecisionService, + logger:logging.GetLogger("", ""), } enabled, variationMap, err := client.GetAllFeatureVariables(invalidFeatureKey, testUserContext) @@ -1665,6 +1705,7 @@ func (s *ClientTestSuiteAB) TestActivate() { ConfigManager: s.mockConfigManager, DecisionService: s.mockDecisionService, EventProcessor: s.mockEventProcessor, + logger:logging.GetLogger("", ""), } variationKey1, err1 := testClient.Activate("test_exp_1", testUserContext) @@ -1687,6 +1728,7 @@ func (s *ClientTestSuiteAB) TestActivatePanics() { testClient := OptimizelyClient{ ConfigManager: new(PanickingConfigManager), DecisionService: s.mockDecisionService, + logger:logging.GetLogger("", ""), } variationKey, err := testClient.Activate("test_exp_1", testUserContext) @@ -1702,6 +1744,7 @@ func (s *ClientTestSuiteAB) TestActivateInvalidConfig() { mockConfigManager.On("GetConfig").Return(s.mockConfig, expectedError) testClient := OptimizelyClient{ ConfigManager: mockConfigManager, + logger:logging.GetLogger("", ""), } variationKey, err := testClient.Activate("test_exp_1", testUserContext) @@ -1729,6 +1772,7 @@ func (s *ClientTestSuiteAB) TestGetVariation() { testClient := OptimizelyClient{ ConfigManager: s.mockConfigManager, DecisionService: s.mockDecisionService, + logger:logging.GetLogger("", ""), } variationKey, err := testClient.GetVariation("test_exp_1", testUserContext) @@ -1758,6 +1802,7 @@ func (s *ClientTestSuiteAB) TestGetVariationWithDecisionError() { testClient := OptimizelyClient{ ConfigManager: s.mockConfigManager, DecisionService: s.mockDecisionService, + logger:logging.GetLogger("", ""), } variationKey, err := testClient.GetVariation("test_exp_1", testUserContext) @@ -1774,6 +1819,7 @@ func (s *ClientTestSuiteAB) TestGetVariationPanics() { testClient := OptimizelyClient{ ConfigManager: new(PanickingConfigManager), DecisionService: s.mockDecisionService, + logger:logging.GetLogger("", ""), } variationKey, err := testClient.GetVariation("test_exp_1", testUserContext) @@ -1824,6 +1870,7 @@ func (s *ClientTestSuiteFM) TestIsFeatureEnabled() { client := OptimizelyClient{ ConfigManager: s.mockConfigManager, DecisionService: s.mockDecisionService, + logger:logging.GetLogger("", ""), } result, _ := client.IsFeatureEnabled(testFeature.Key, testUserContext) s.True(result) @@ -1861,6 +1908,7 @@ func (s *ClientTestSuiteFM) TestIsFeatureEnabledWithDecisionError() { ConfigManager: s.mockConfigManager, DecisionService: s.mockDecisionService, EventProcessor: s.mockEventProcessor, + logger:logging.GetLogger("", ""), } // should still return the decision because the error is non-fatal @@ -1882,6 +1930,7 @@ func (s *ClientTestSuiteFM) TestIsFeatureEnabledErrorCases() { client := OptimizelyClient{ ConfigManager: s.mockConfigManager, DecisionService: s.mockDecisionService, + logger:logging.GetLogger("", ""), } result, _ := client.IsFeatureEnabled(testFeatureKey, testUserContext) s.False(result) @@ -1895,6 +1944,7 @@ func (s *ClientTestSuiteFM) TestIsFeatureEnabledErrorCases() { client = OptimizelyClient{ ConfigManager: s.mockConfigManager, DecisionService: s.mockDecisionService, + logger:logging.GetLogger("", ""), } result, err := client.IsFeatureEnabled(testFeatureKey, testUserContext) s.NoError(err) @@ -1909,6 +1959,7 @@ func (s *ClientTestSuiteFM) TestIsFeatureEnabledPanic() { client := OptimizelyClient{ ConfigManager: &PanickingConfigManager{}, + logger:logging.GetLogger("", ""), } // ensure that the client calms back down and recovers @@ -1956,6 +2007,7 @@ func (s *ClientTestSuiteFM) TestGetEnabledFeatures() { client := OptimizelyClient{ ConfigManager: s.mockConfigManager, DecisionService: s.mockDecisionService, + logger:logging.GetLogger("", ""), } result, err := client.GetEnabledFeatures(testUserContext) s.NoError(err) @@ -1976,6 +2028,7 @@ func (s *ClientTestSuiteFM) TestGetEnabledFeaturesErrorCases() { client := OptimizelyClient{ ConfigManager: mockConfigManager, DecisionService: s.mockDecisionService, + logger:logging.GetLogger("", ""), } result, err := client.GetEnabledFeatures(testUserContext) s.Error(err) @@ -2003,6 +2056,7 @@ func TestClose(t *testing.T) { DecisionService: mockDecisionService, EventProcessor: mockProcessor, execGroup: eg, + logger:logging.GetLogger("", ""), } client.Close() @@ -2026,6 +2080,7 @@ func (s *ClientTestSuiteTrackEvent) SetupTest() { DecisionService: s.mockDecisionService, EventProcessor: s.mockProcessor, notificationCenter: notification.NewNotificationCenter(), + logger:logging.GetLogger("", ""), } } @@ -2204,6 +2259,7 @@ func (s *ClientTestSuiteTrackNotification) SetupTest() { DecisionService: s.mockDecisionService, EventProcessor: s.mockProcessor, notificationCenter: notification.NewNotificationCenter(), + logger:logging.GetLogger("", ""), } } diff --git a/pkg/decision/bucketer/murmurhashbucketer_test.go b/pkg/decision/bucketer/murmurhashbucketer_test.go index 1b5dcec34..56977afe2 100644 --- a/pkg/decision/bucketer/murmurhashbucketer_test.go +++ b/pkg/decision/bucketer/murmurhashbucketer_test.go @@ -51,7 +51,7 @@ func TestBucketToEntity(t *testing.T) { } func TestGenerateBucketValue(t *testing.T) { - bucketer := NewMurmurhashBucketer(DefaultHashSeed) + bucketer := NewMurmurhashBucketer("", DefaultHashSeed) // copied from unit tests in the other SDKs experimentID := "1886780721" diff --git a/pkg/logging/level_log_consumer.go b/pkg/logging/level_log_consumer.go index b18d409db..57263fad8 100644 --- a/pkg/logging/level_log_consumer.go +++ b/pkg/logging/level_log_consumer.go @@ -34,6 +34,9 @@ func (l *FilteredLevelLogConsumer) Log(level LogLevel, message string, fields ma if l.level <= level { // prepends the name and log level to the message messBuilder := strings.Builder{} + messBuilder.WriteString("[") + messBuilder.WriteString(level.String()) + messBuilder.WriteString("]") for _, v := range fields { if s, ok := v.(string); ok { messBuilder.WriteString("[") diff --git a/pkg/notification/manager_test.go b/pkg/notification/manager_test.go index 459e9b1c7..4f72381eb 100644 --- a/pkg/notification/manager_test.go +++ b/pkg/notification/manager_test.go @@ -1,6 +1,7 @@ package notification import ( + "github.com/optimizely/go-sdk/pkg/logging" "testing" "github.com/stretchr/testify/assert" @@ -28,7 +29,7 @@ func TestAtomicManager(t *testing.T) { mockReceiver.On("handle", payload) mockReceiver.On("handleBetter", payload) - atomicManager := NewAtomicManager() + atomicManager := NewAtomicManager(logging.GetLogger("", "")) result1, _ := atomicManager.Add(mockReceiver.handle) assert.Equal(t, 1, result1) @@ -56,7 +57,7 @@ func TestSendRaceCondition(t *testing.T) { payload := map[string]interface{}{ "key": "test", } - atomicManager := NewAtomicManager() + atomicManager := NewAtomicManager(logging.GetLogger("", "")) result1, result2 := 0, 0 listenerCalled := false @@ -89,7 +90,7 @@ func TestSendRaceCondition(t *testing.T) { func TestAddRaceCondition(t *testing.T) { sync := make(chan interface{}) - atomicManager := NewAtomicManager() + atomicManager := NewAtomicManager(logging.GetLogger("", "")) listener1 := func(interface{}) { diff --git a/pkg/utils/execgroup_test.go b/pkg/utils/execgroup_test.go index 7e7a4bf0f..167d1a3b8 100644 --- a/pkg/utils/execgroup_test.go +++ b/pkg/utils/execgroup_test.go @@ -19,6 +19,7 @@ package utils import ( "context" + "github.com/optimizely/go-sdk/pkg/logging" "sync" "testing" ) @@ -26,7 +27,7 @@ import ( func TestWithContextCancelFunc(t *testing.T) { ctx, cancelFunc := context.WithCancel(context.Background()) - eg := NewExecGroup(ctx) + eg := NewExecGroup(logging.GetLogger("", ""), ctx) wg := &sync.WaitGroup{} wg.Add(1) @@ -41,7 +42,7 @@ func TestWithContextCancelFunc(t *testing.T) { func TestTerminateAndWait(t *testing.T) { - eg := NewExecGroup(context.Background()) + eg := NewExecGroup(logging.GetLogger("", ""), context.Background()) wg := &sync.WaitGroup{} wg.Add(1) diff --git a/pkg/utils/requester_test.go b/pkg/utils/requester_test.go index 35838b9da..eec6ffc4b 100644 --- a/pkg/utils/requester_test.go +++ b/pkg/utils/requester_test.go @@ -19,6 +19,7 @@ package utils import ( "errors" "fmt" + "github.com/optimizely/go-sdk/pkg/logging" "log" "net/http" "net/http/httptest" @@ -39,7 +40,7 @@ func TestHeaders(t *testing.T) { func TestAddHeaders(t *testing.T) { req, _ := http.NewRequest("GET", "", nil) - requester := &HTTPRequester{} + requester := &HTTPRequester{logger:logging.GetLogger("", "")} headers := []Header{{"one", "1"}} fn := Headers(Header{"two", "2"}) @@ -62,13 +63,13 @@ func TestGet(t *testing.T) { defer ts.Close() var httpreq Requester - httpreq = NewHTTPRequester() + httpreq = NewHTTPRequester(logging.GetLogger("", "")) resp, headers, code, err := httpreq.Get(ts.URL + "/good") assert.NotEqual(t, headers.Get("Content-Type"), "") assert.Nil(t, err) assert.Equal(t, "Hello, client\n", string(resp)) - httpreq = NewHTTPRequester() + httpreq = NewHTTPRequester(logging.GetLogger("", "")) _, headers, code, err = httpreq.Get(ts.URL + "/bad") assert.Equal(t, errors.New("400 Bad Request"), err) assert.Equal(t, code, http.StatusBadRequest) @@ -94,13 +95,13 @@ func TestGetObj(t *testing.T) { defer ts.Close() var httpreq Requester - httpreq = NewHTTPRequester() + httpreq = NewHTTPRequester(logging.GetLogger("", "")) r := resp{} err := httpreq.GetObj(ts.URL+"/good", &r) assert.Nil(t, err) assert.Equal(t, resp{Fld1: "Hello, client", Fld2: 123}, r) - httpreq = NewHTTPRequester() + httpreq = NewHTTPRequester(logging.GetLogger("", "")) err = httpreq.GetObj(ts.URL+"/bad", &r) assert.NotNil(t, err) } @@ -124,7 +125,7 @@ func TestPost(t *testing.T) { defer ts.Close() b := body{"one", 1} var httpreq Requester - httpreq = NewHTTPRequester() + httpreq = NewHTTPRequester(logging.GetLogger("", "")) resp, headers, code, err := httpreq.Post(ts.URL+"/good", b) assert.Nil(t, err) @@ -132,7 +133,7 @@ func TestPost(t *testing.T) { assert.Equal(t, "Hello, client\n", string(resp)) assert.Equal(t, code, http.StatusOK) - httpreq = NewHTTPRequester() + httpreq = NewHTTPRequester(logging.GetLogger("", "")) _, _, code, err = httpreq.Post(ts.URL+"/bad", nil) assert.Equal(t, errors.New("400 Bad Request"), err) assert.Equal(t, code, http.StatusBadRequest) @@ -158,21 +159,21 @@ func TestPostObj(t *testing.T) { defer ts.Close() var httpreq Requester - httpreq = NewHTTPRequester() + httpreq = NewHTTPRequester(logging.GetLogger("", "")) b := body{"one", 1} r := body{} err := httpreq.PostObj(ts.URL+"/good", b, &r) assert.Nil(t, err) assert.Equal(t, body{Fld1: "Hello, client", Fld2: 123}, r) - httpreq = NewHTTPRequester() + httpreq = NewHTTPRequester(logging.GetLogger("", "")) err = httpreq.PostObj(ts.URL+"/bad", b, &r) assert.NotNil(t, err) } func TestGetBad(t *testing.T) { - httpreq := NewHTTPRequester() + httpreq := NewHTTPRequester(logging.GetLogger("", "")) _, _, _, err := httpreq.Get("blah12345/good") _, ok := err.(*url.Error) assert.True(t, ok, "url error") @@ -192,7 +193,7 @@ func TestGetBadWithResponse(t *testing.T) { })) defer ts.Close() - httpreq := NewHTTPRequester(Retries(1)) + httpreq := NewHTTPRequester(logging.GetLogger("", ""), Retries(1)) data, _, _, err := httpreq.Get(ts.URL + "/bad") assert.Equal(t, "400 Bad Request", err.Error()) assert.Equal(t, "bad bad response\n", string(data)) @@ -221,7 +222,7 @@ func TestGetRetry(t *testing.T) { })) defer ts.Close() - httpreq := NewHTTPRequester(Retries(10)) + httpreq := NewHTTPRequester(logging.GetLogger("", ""), Retries(10)) st := time.Now() resp, _, _, err := httpreq.Get(ts.URL + "/test") @@ -232,19 +233,19 @@ func TestGetRetry(t *testing.T) { assert.True(t, elapsed >= 400*5*time.Millisecond && elapsed <= 510*5*time.Second, "took %s", elapsed) - httpreq = NewHTTPRequester(Retries(3)) + httpreq = NewHTTPRequester(logging.GetLogger("", ""), Retries(3)) called = 0 _, _, _, err = httpreq.Get(ts.URL + "/test") assert.Equal(t, errors.New("400 Bad Request"), err) assert.Equal(t, 3, called, "called 3 retries") - httpreq = NewHTTPRequester(Retries(1)) + httpreq = NewHTTPRequester(logging.GetLogger("", ""), Retries(1)) called = 0 _, _, _, err = httpreq.Get(ts.URL + "/test") assert.Equal(t, errors.New("400 Bad Request"), err) assert.Equal(t, 1, called, "called 1 retries") - httpreq = NewHTTPRequester() + httpreq = NewHTTPRequester(logging.GetLogger("", "")) called = 0 _, _, _, err = httpreq.Get(ts.URL + "/test") assert.Equal(t, errors.New("400 Bad Request"), err) @@ -252,8 +253,8 @@ func TestGetRetry(t *testing.T) { } func TestString(t *testing.T) { - assert.Equal(t, "{timeout: 5s, retries: 1}", NewHTTPRequester().String()) + assert.Equal(t, "{timeout: 5s, retries: 1}", NewHTTPRequester(logging.GetLogger("", "")).String()) assert.Equal(t, "{timeout: 19s, retries: 10}", - NewHTTPRequester(Retries(10), Timeout(time.Duration(19)*time.Second)).String()) + NewHTTPRequester(logging.GetLogger("", ""), Retries(10), Timeout(time.Duration(19)*time.Second)).String()) } From e181b81da74571f8ba465a415e8d1bd9c7176a58 Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Wed, 18 Mar 2020 18:00:26 -0700 Subject: [PATCH 04/28] fixing tests --- pkg/client/client_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 32b4499b5..d90832334 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -1870,6 +1870,7 @@ func (s *ClientTestSuiteFM) TestIsFeatureEnabled() { client := OptimizelyClient{ ConfigManager: s.mockConfigManager, DecisionService: s.mockDecisionService, + EventProcessor: s.mockEventProcessor, logger:logging.GetLogger("", ""), } result, _ := client.IsFeatureEnabled(testFeature.Key, testUserContext) From 7ae6d3db45e6a6f6ba4ce68604c9decdc1ffcf28 Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Wed, 18 Mar 2020 18:25:20 -0700 Subject: [PATCH 05/28] fixing some errors --- pkg/client/client_test.go | 2 +- pkg/client/factory.go | 2 +- pkg/config/polling_manager_test.go | 2 +- pkg/event/factory.go | 2 +- pkg/event/processor_test.go | 2 +- pkg/logging/level_log_consumer.go | 17 +++++++++-------- pkg/logging/logger.go | 2 +- pkg/utils/execgroup.go | 4 ++-- pkg/utils/execgroup_test.go | 4 ++-- 9 files changed, 19 insertions(+), 18 deletions(-) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index d90832334..b2d1c151d 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -2043,7 +2043,7 @@ func TestClose(t *testing.T) { mockProcessor := &MockProcessor{} mockDecisionService := new(MockDecisionService) - eg := utils.NewExecGroup(logging.GetLogger("", "ExecGroup"), context.Background()) + eg := utils.NewExecGroup(context.Background(), logging.GetLogger("", "ExecGroup")) wg := &sync.WaitGroup{} wg.Add(1) diff --git a/pkg/client/factory.go b/pkg/client/factory.go index 7022b0ea4..db3f565d2 100644 --- a/pkg/client/factory.go +++ b/pkg/client/factory.go @@ -74,7 +74,7 @@ func (f OptimizelyFactory) Client(clientOptions ...OptionFunc) (*OptimizelyClien ctx = context.Background() } - eg := utils.NewExecGroup(logging.GetLogger(f.SDKKey, "ExecGroup"), ctx) + eg := utils.NewExecGroup(ctx, logging.GetLogger(f.SDKKey, "ExecGroup")) appClient := &OptimizelyClient{execGroup: eg, notificationCenter: registry.GetNotificationCenter(f.SDKKey), logger: logging.GetLogger(f.SDKKey, "OptimizelyClient")} diff --git a/pkg/config/polling_manager_test.go b/pkg/config/polling_manager_test.go index 979e1ba8e..2d6aca04b 100644 --- a/pkg/config/polling_manager_test.go +++ b/pkg/config/polling_manager_test.go @@ -45,7 +45,7 @@ func (m *MockRequester) Get(uri string, headers ...utils.Header) (response []byt var logger = logging.GetLogger("polling_manager_test", "PollingManager") func newExecGroup() *utils.ExecGroup { - return utils.NewExecGroup(logger, context.Background()) + return utils.NewExecGroup(context.Background(), logger) } // assertion method to periodically check target function each tick. diff --git a/pkg/event/factory.go b/pkg/event/factory.go index 8fa688a3a..b6819d024 100644 --- a/pkg/event/factory.go +++ b/pkg/event/factory.go @@ -230,7 +230,7 @@ func getEventAttributes(projectConfig config.ProjectConfig, attributes map[strin case strings.HasPrefix(key, specialPrefix): visitorAttribute.EntityID = key default: - //efLogger.Debug(fmt.Sprintf("Unrecognized attribute %s provided. Pruning before sending event to Optimizely.", key)) + // efLogger.Debug(fmt.Sprintf("Unrecognized attribute %s provided. Pruning before sending event to Optimizely.", key)) continue } visitorAttribute.Key = key diff --git a/pkg/event/processor_test.go b/pkg/event/processor_test.go index 210badd94..0fbc3a6db 100644 --- a/pkg/event/processor_test.go +++ b/pkg/event/processor_test.go @@ -59,7 +59,7 @@ func NewMockDispatcher(queueSize int, shouldFail bool) *MockDispatcher { } func newExecutionContext() *utils.ExecGroup { - return utils.NewExecGroup(logging.GetLogger("", "NewExecGroup"), context.Background()) + return utils.NewExecGroup(context.Background(), logging.GetLogger("", "NewExecGroup")) } func TestDefaultEventProcessor_ProcessImpression(t *testing.T) { diff --git a/pkg/logging/level_log_consumer.go b/pkg/logging/level_log_consumer.go index 57263fad8..e69ac6307 100644 --- a/pkg/logging/level_log_consumer.go +++ b/pkg/logging/level_log_consumer.go @@ -34,18 +34,19 @@ func (l *FilteredLevelLogConsumer) Log(level LogLevel, message string, fields ma if l.level <= level { // prepends the name and log level to the message messBuilder := strings.Builder{} - messBuilder.WriteString("[") - messBuilder.WriteString(level.String()) - messBuilder.WriteString("]") + _, _ = messBuilder.WriteString("[") + + _, _ = messBuilder.WriteString(level.String()) + _, _ = messBuilder.WriteString("]") for _, v := range fields { if s, ok := v.(string); ok { - messBuilder.WriteString("[") - messBuilder.WriteString(s) - messBuilder.WriteString("]") + _, _ = messBuilder.WriteString("[") + _, _ = messBuilder.WriteString(s) + _, _ = messBuilder.WriteString("]") } } - messBuilder.WriteString(" ") - messBuilder.WriteString(message) + _, _ = messBuilder.WriteString(" ") + _, _ = messBuilder.WriteString(message) l.logger.Println(messBuilder.String()) } } diff --git a/pkg/logging/logger.go b/pkg/logging/logger.go index 004322a61..cc9bdc2b4 100644 --- a/pkg/logging/logger.go +++ b/pkg/logging/logger.go @@ -68,7 +68,7 @@ func SetLogLevel(logLevel LogLevel) { } // GetLogger returns a log producer with the given name -func GetLogger(sdkKey string, name string) OptimizelyLogProducer { +func GetLogger(sdkKey, name string) OptimizelyLogProducer { return NamedLogProducer{ fields: map[string]interface{}{"sdkKey":sdkKey, "name": name}, } diff --git a/pkg/utils/execgroup.go b/pkg/utils/execgroup.go index 266f7d92f..71d174a26 100644 --- a/pkg/utils/execgroup.go +++ b/pkg/utils/execgroup.go @@ -32,11 +32,11 @@ type ExecGroup struct { } // NewExecGroup returns constructed object -func NewExecGroup(logger logging.OptimizelyLogProducer, ctx context.Context) *ExecGroup { +func NewExecGroup(ctx context.Context, logger logging.OptimizelyLogProducer) *ExecGroup { nctx, cancelFn := context.WithCancel(ctx) wg := sync.WaitGroup{} - return &ExecGroup{wg: &wg, ctx: nctx, cancelFunc: cancelFn} + return &ExecGroup{wg: &wg, ctx: nctx, cancelFunc: cancelFn, logger:logger} } // Go initiates a goroutine with the inputted function. Each invocation increments a shared WaitGroup diff --git a/pkg/utils/execgroup_test.go b/pkg/utils/execgroup_test.go index 167d1a3b8..6357a442a 100644 --- a/pkg/utils/execgroup_test.go +++ b/pkg/utils/execgroup_test.go @@ -27,7 +27,7 @@ import ( func TestWithContextCancelFunc(t *testing.T) { ctx, cancelFunc := context.WithCancel(context.Background()) - eg := NewExecGroup(logging.GetLogger("", ""), ctx) + eg := NewExecGroup(ctx, logging.GetLogger("", "")) wg := &sync.WaitGroup{} wg.Add(1) @@ -42,7 +42,7 @@ func TestWithContextCancelFunc(t *testing.T) { func TestTerminateAndWait(t *testing.T) { - eg := NewExecGroup(logging.GetLogger("", ""), context.Background()) + eg := NewExecGroup(context.Background(), logging.GetLogger("", "")) wg := &sync.WaitGroup{} wg.Add(1) From 5f470d7c38c2ffa0d73e71dd5ff9e9ab35975832 Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Thu, 19 Mar 2020 08:55:31 -0700 Subject: [PATCH 06/28] remove commented out log message --- pkg/event/factory.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/event/factory.go b/pkg/event/factory.go index b6819d024..5fc434949 100644 --- a/pkg/event/factory.go +++ b/pkg/event/factory.go @@ -230,7 +230,6 @@ func getEventAttributes(projectConfig config.ProjectConfig, attributes map[strin case strings.HasPrefix(key, specialPrefix): visitorAttribute.EntityID = key default: - // efLogger.Debug(fmt.Sprintf("Unrecognized attribute %s provided. Pruning before sending event to Optimizely.", key)) continue } visitorAttribute.Key = key From b5634ff3a1c15ea158faee41eafad6bee9e424a3 Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Thu, 19 Mar 2020 09:37:27 -0700 Subject: [PATCH 07/28] fix broken tests --- pkg/client/factory_test.go | 8 ++------ pkg/decision/composite_experiment_service_test.go | 7 ++++++- pkg/decision/composite_feature_service_test.go | 6 ++++++ pkg/decision/feature_experiment_service_test.go | 4 +++- pkg/decision/rollout_service_test.go | 6 +++++- pkg/event/dispatcher.go | 1 + 6 files changed, 23 insertions(+), 9 deletions(-) diff --git a/pkg/client/factory_test.go b/pkg/client/factory_test.go index 631e8359f..bcef219e7 100644 --- a/pkg/client/factory_test.go +++ b/pkg/client/factory_test.go @@ -102,12 +102,8 @@ func TestClientWithDecisionServiceAndEventProcessorInOptions(t *testing.T) { projectConfig := datafileprojectconfig.DatafileProjectConfig{} configManager := config.NewStaticProjectConfigManager(projectConfig, logging.GetLogger("", "StaticProjectConfigManager")) decisionService := new(MockDecisionService) - processor := &event.BatchEventProcessor{ - MaxQueueSize: 100, - FlushInterval: 100, - Q: event.NewInMemoryQueue(100), - EventDispatcher: &MockDispatcher{Events: []event.LogEvent{}}, - } + processor := event.NewBatchEventProcessor(event.WithQueueSize(100),event.WithFlushInterval(100), + event.WithQueue(event.NewInMemoryQueue(100)), event.WithEventDispatcher(&MockDispatcher{Events: []event.LogEvent{}})) optimizelyClient, err := factory.Client(WithConfigManager(configManager), WithDecisionService(decisionService), WithEventProcessor(processor)) assert.NoError(t, err) diff --git a/pkg/decision/composite_experiment_service_test.go b/pkg/decision/composite_experiment_service_test.go index f6ef383bc..fa465d196 100644 --- a/pkg/decision/composite_experiment_service_test.go +++ b/pkg/decision/composite_experiment_service_test.go @@ -18,6 +18,7 @@ package decision import ( "errors" + "github.com/optimizely/go-sdk/pkg/logging" "testing" "github.com/stretchr/testify/suite" @@ -58,7 +59,8 @@ func (s *CompositeExperimentTestSuite) TestGetDecision() { s.mockExperimentService.On("GetDecision", s.testDecisionContext, testUserContext).Return(expectedExperimentDecision, nil) compositeExperimentService := &CompositeExperimentService{ - experimentServices: []ExperimentService{s.mockExperimentService, s.mockExperimentService2}, + experimentServices: []ExperimentService{s.mockExperimentService, s.mockExperimentService2,}, + logger:logging.GetLogger("sdkKey", "ExperimentService"), } decision, err := compositeExperimentService.GetDecision(s.testDecisionContext, testUserContext) s.Equal(expectedExperimentDecision, decision) @@ -85,6 +87,7 @@ func (s *CompositeExperimentTestSuite) TestGetDecisionFallthrough() { compositeExperimentService := &CompositeExperimentService{ experimentServices: []ExperimentService{s.mockExperimentService, s.mockExperimentService2}, + logger:logging.GetLogger("sdkKey", "CompositeExperimentService"), } decision, err := compositeExperimentService.GetDecision(s.testDecisionContext, testUserContext) @@ -107,6 +110,7 @@ func (s *CompositeExperimentTestSuite) TestGetDecisionNoDecisionsMade() { compositeExperimentService := &CompositeExperimentService{ experimentServices: []ExperimentService{s.mockExperimentService, s.mockExperimentService2}, + logger:logging.GetLogger("sdkKey", "CompositeExperimentService"), } decision, err := compositeExperimentService.GetDecision(s.testDecisionContext, testUserContext) @@ -142,6 +146,7 @@ func (s *CompositeExperimentTestSuite) TestGetDecisionReturnsError() { s.mockExperimentService, s.mockExperimentService2, }, + logger:logging.GetLogger("sdkKey", "CompositeExperimentService"), } decision, err := compositeExperimentService.GetDecision(testDecisionContext, testUserContext) s.Equal(expectedDecision, decision) diff --git a/pkg/decision/composite_feature_service_test.go b/pkg/decision/composite_feature_service_test.go index 242e3ea0f..27be36eb7 100644 --- a/pkg/decision/composite_feature_service_test.go +++ b/pkg/decision/composite_feature_service_test.go @@ -18,6 +18,7 @@ package decision import ( "errors" + "github.com/optimizely/go-sdk/pkg/logging" "testing" "github.com/optimizely/go-sdk/pkg/decision/reasons" @@ -64,6 +65,7 @@ func (s *CompositeFeatureServiceTestSuite) TestGetDecision() { s.mockFeatureService, s.mockFeatureService2, }, + logger:logging.GetLogger("sdkKey", "CompositeFeatureService"), } decision, err := compositeFeatureService.GetDecision(s.testFeatureDecisionContext, testUserContext) s.Equal(expectedDecision, decision) @@ -91,6 +93,7 @@ func (s *CompositeFeatureServiceTestSuite) TestGetDecisionFallthrough() { s.mockFeatureService, s.mockFeatureService2, }, + logger:logging.GetLogger("sdkKey", "CompositeFeatureService"), } decision, err := compositeFeatureService.GetDecision(s.testFeatureDecisionContext, testUserContext) s.Equal(expectedDecision, decision) @@ -120,6 +123,8 @@ func (s *CompositeFeatureServiceTestSuite) TestGetDecisionReturnsError() { s.mockFeatureService, s.mockFeatureService2, }, + logger:logging.GetLogger("sdkKey", "CompositeFeatureService"), + } decision, err := compositeFeatureService.GetDecision(s.testFeatureDecisionContext, testUserContext) s.Equal(expectedDecision, decision) @@ -146,6 +151,7 @@ func (s *CompositeFeatureServiceTestSuite) TestGetDecisionReturnsLastDecisionWit s.mockFeatureService, s.mockFeatureService2, }, + logger:logging.GetLogger("sdkKey", "CompositeFeatureService"), } decision, err := compositeFeatureService.GetDecision(s.testFeatureDecisionContext, testUserContext) s.Equal(expectedDecision, decision) diff --git a/pkg/decision/feature_experiment_service_test.go b/pkg/decision/feature_experiment_service_test.go index 02b9388c6..ea1f3257b 100644 --- a/pkg/decision/feature_experiment_service_test.go +++ b/pkg/decision/feature_experiment_service_test.go @@ -58,6 +58,7 @@ func (s *FeatureExperimentServiceTestSuite) TestGetDecision() { featureExperimentService := &FeatureExperimentService{ compositeExperimentService: s.mockExperimentService, + logger:logging.GetLogger("sdkKey", "FeatureExperimentService"), } expectedFeatureDecision := FeatureDecision{ @@ -102,6 +103,7 @@ func (s *FeatureExperimentServiceTestSuite) TestGetDecisionMutex() { } featureExperimentService := &FeatureExperimentService{ compositeExperimentService: s.mockExperimentService, + logger:logging.GetLogger("sdkKey", "FeatureExperimentService"), } decision, err := featureExperimentService.GetDecision(s.testFeatureDecisionContext, testUserContext) s.Equal(expectedFeatureDecision, decision) @@ -110,7 +112,7 @@ func (s *FeatureExperimentServiceTestSuite) TestGetDecisionMutex() { } func (s *FeatureExperimentServiceTestSuite) TestNewFeatureExperimentService() { - compositeExperimentService := &CompositeExperimentService{} + compositeExperimentService := &CompositeExperimentService{logger:logging.GetLogger("sdkKey", "CompositeExperimentService")} featureExperimentService := NewFeatureExperimentService(logging.GetLogger("", ""), compositeExperimentService) s.IsType(compositeExperimentService, featureExperimentService.compositeExperimentService) } diff --git a/pkg/decision/rollout_service_test.go b/pkg/decision/rollout_service_test.go index 1707538dd..5dda6c76e 100644 --- a/pkg/decision/rollout_service_test.go +++ b/pkg/decision/rollout_service_test.go @@ -17,6 +17,7 @@ package decision import ( + "github.com/optimizely/go-sdk/pkg/logging" "testing" "github.com/optimizely/go-sdk/pkg/decision/evaluator" @@ -75,6 +76,7 @@ func (s *RolloutServiceTestSuite) TestGetDecisionHappyPath() { testRolloutService := RolloutService{ audienceTreeEvaluator: s.mockAudienceTreeEvaluator, experimentBucketerService: s.mockExperimentService, + logger:logging.GetLogger("sdkKey", "RolloutService"), } expectedFeatureDecision := FeatureDecision{ Experiment: testExp1112, @@ -101,6 +103,7 @@ func (s *RolloutServiceTestSuite) TestGetDecisionFailsBucketing() { testRolloutService := RolloutService{ audienceTreeEvaluator: s.mockAudienceTreeEvaluator, experimentBucketerService: s.mockExperimentService, + logger:logging.GetLogger("sdkKey", "RolloutService"), } expectedFeatureDecision := FeatureDecision{ Decision: Decision{ @@ -120,6 +123,7 @@ func (s *RolloutServiceTestSuite) TestGetDecisionFailsTargeting() { testRolloutService := RolloutService{ audienceTreeEvaluator: s.mockAudienceTreeEvaluator, experimentBucketerService: s.mockExperimentService, + logger:logging.GetLogger("sdkKey", "RolloutService"), } expectedFeatureDecision := FeatureDecision{ Decision: Decision{ @@ -136,7 +140,7 @@ func (s *RolloutServiceTestSuite) TestGetDecisionFailsTargeting() { func TestNewRolloutService(t *testing.T) { rolloutService := NewRolloutService("") assert.IsType(t, &evaluator.MixedTreeEvaluator{}, rolloutService.audienceTreeEvaluator) - assert.IsType(t, &ExperimentBucketerService{}, rolloutService.experimentBucketerService) + assert.IsType(t, &ExperimentBucketerService{logger:logging.GetLogger("sdkKey", "ExperimentBucketerService")}, rolloutService.experimentBucketerService) } func TestRolloutServiceTestSuite(t *testing.T) { diff --git a/pkg/event/dispatcher.go b/pkg/event/dispatcher.go index 6aee10677..5e03a9655 100644 --- a/pkg/event/dispatcher.go +++ b/pkg/event/dispatcher.go @@ -168,5 +168,6 @@ func NewQueueEventDispatcher(sdkKey string, metricsRegistry metrics.Registry) *Q retryFlushCounter: dispatcherMetricsRegistry.GetCounter(metrics.DispatcherRetryFlush), failFlushCounter: dispatcherMetricsRegistry.GetCounter(metrics.DispatcherFailedFlush), sucessFlush: dispatcherMetricsRegistry.GetCounter(metrics.DispatcherSuccessFlush), + logger: logging.GetLogger(sdkKey, "QueueEventDispatcher"), } } From 21a8b9c2a2541058d8393644b1cd69b3a78714b0 Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Thu, 19 Mar 2020 09:41:40 -0700 Subject: [PATCH 08/28] add test for sdk Key in log --- pkg/logging/level_log_consumer_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/logging/level_log_consumer_test.go b/pkg/logging/level_log_consumer_test.go index ed79284ac..57ca14dd7 100644 --- a/pkg/logging/level_log_consumer_test.go +++ b/pkg/logging/level_log_consumer_test.go @@ -46,9 +46,10 @@ func TestLogFormatting(t *testing.T) { out := &bytes.Buffer{} newLogger := NewFilteredLevelLogConsumer(LogLevelInfo, out) - newLogger.Log(LogLevelInfo, "test message", map[string]interface{}{"name": "test-name"}) + newLogger.Log(LogLevelInfo, "test message", map[string]interface{}{"name": "test-name", "sdkKey": "test-sdkKey"}) assert.Contains(t, out.String(), "test message") assert.Contains(t, out.String(), "[Info]") assert.Contains(t, out.String(), "[test-name]") + assert.Contains(t, out.String(), "[test-sdkKey]") assert.Contains(t, out.String(), "[Optimizely]") } From 4c516871136b56db9101eac45328ea416c1de6c8 Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Thu, 19 Mar 2020 10:09:30 -0700 Subject: [PATCH 09/28] sort imports --- pkg/client/client_test.go | 2 +- pkg/client/factory.go | 2 +- pkg/client/factory_test.go | 2 +- pkg/config/datafileprojectconfig/config.go | 2 +- pkg/config/datafileprojectconfig/config_test.go | 2 +- pkg/config/polling_manager.go | 2 +- pkg/config/polling_manager_test.go | 2 +- pkg/config/static_manager.go | 2 +- pkg/config/static_manager_test.go | 5 +++-- pkg/decision/bucketer/murmurhashbucketer.go | 3 ++- pkg/decision/composite_experiment_service.go | 3 +-- pkg/decision/composite_experiment_service_test.go | 2 +- pkg/decision/composite_feature_service_test.go | 3 ++- pkg/decision/experiment_bucketer_service.go | 2 +- pkg/decision/experiment_override_service_test.go | 3 ++- pkg/decision/feature_experiment_service_test.go | 3 ++- pkg/decision/persisting_experiment_service_test.go | 3 ++- pkg/decision/rollout_service.go | 3 +-- pkg/decision/rollout_service_test.go | 5 ++--- pkg/event/dispatcher.go | 2 +- pkg/notification/manager_test.go | 3 ++- 21 files changed, 30 insertions(+), 26 deletions(-) diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index b2d1c151d..d31e05657 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -20,7 +20,6 @@ import ( "context" "errors" "fmt" - "github.com/optimizely/go-sdk/pkg/logging" "strconv" "sync" "testing" @@ -29,6 +28,7 @@ import ( "github.com/optimizely/go-sdk/pkg/decision" "github.com/optimizely/go-sdk/pkg/entities" "github.com/optimizely/go-sdk/pkg/event" + "github.com/optimizely/go-sdk/pkg/logging" "github.com/optimizely/go-sdk/pkg/notification" "github.com/optimizely/go-sdk/pkg/utils" diff --git a/pkg/client/factory.go b/pkg/client/factory.go index db3f565d2..342fbebf5 100644 --- a/pkg/client/factory.go +++ b/pkg/client/factory.go @@ -20,12 +20,12 @@ package client import ( "context" "errors" - "github.com/optimizely/go-sdk/pkg/logging" "time" "github.com/optimizely/go-sdk/pkg/config" "github.com/optimizely/go-sdk/pkg/decision" "github.com/optimizely/go-sdk/pkg/event" + "github.com/optimizely/go-sdk/pkg/logging" "github.com/optimizely/go-sdk/pkg/metrics" "github.com/optimizely/go-sdk/pkg/registry" "github.com/optimizely/go-sdk/pkg/utils" diff --git a/pkg/client/factory_test.go b/pkg/client/factory_test.go index bcef219e7..4db24ee6a 100644 --- a/pkg/client/factory_test.go +++ b/pkg/client/factory_test.go @@ -19,7 +19,6 @@ package client import ( "context" "errors" - "github.com/optimizely/go-sdk/pkg/logging" "net/http" "sync" "testing" @@ -29,6 +28,7 @@ import ( "github.com/optimizely/go-sdk/pkg/config/datafileprojectconfig" "github.com/optimizely/go-sdk/pkg/decision" "github.com/optimizely/go-sdk/pkg/event" + "github.com/optimizely/go-sdk/pkg/logging" "github.com/optimizely/go-sdk/pkg/metrics" "github.com/optimizely/go-sdk/pkg/utils" diff --git a/pkg/config/datafileprojectconfig/config.go b/pkg/config/datafileprojectconfig/config.go index b69f49882..961f0512d 100644 --- a/pkg/config/datafileprojectconfig/config.go +++ b/pkg/config/datafileprojectconfig/config.go @@ -20,10 +20,10 @@ package datafileprojectconfig import ( "errors" "fmt" - "github.com/optimizely/go-sdk/pkg/logging" "github.com/optimizely/go-sdk/pkg/config/datafileprojectconfig/mappers" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" ) var datafileVersions = map[string]struct{}{ diff --git a/pkg/config/datafileprojectconfig/config_test.go b/pkg/config/datafileprojectconfig/config_test.go index fcd63c945..ec7ad95b1 100644 --- a/pkg/config/datafileprojectconfig/config_test.go +++ b/pkg/config/datafileprojectconfig/config_test.go @@ -19,10 +19,10 @@ package datafileprojectconfig import ( "fmt" - "github.com/optimizely/go-sdk/pkg/logging" "testing" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" "github.com/stretchr/testify/assert" ) diff --git a/pkg/config/polling_manager.go b/pkg/config/polling_manager.go index 1c41d32f1..387558595 100644 --- a/pkg/config/polling_manager.go +++ b/pkg/config/polling_manager.go @@ -20,12 +20,12 @@ package config import ( "context" "fmt" - "github.com/optimizely/go-sdk/pkg/logging" "net/http" "sync" "time" "github.com/optimizely/go-sdk/pkg/config/datafileprojectconfig" + "github.com/optimizely/go-sdk/pkg/logging" "github.com/optimizely/go-sdk/pkg/notification" "github.com/optimizely/go-sdk/pkg/registry" "github.com/optimizely/go-sdk/pkg/utils" diff --git a/pkg/config/polling_manager_test.go b/pkg/config/polling_manager_test.go index 2d6aca04b..0786b5a4c 100644 --- a/pkg/config/polling_manager_test.go +++ b/pkg/config/polling_manager_test.go @@ -18,13 +18,13 @@ package config import ( "context" - "github.com/optimizely/go-sdk/pkg/logging" "net/http" "sync/atomic" "testing" "time" "github.com/optimizely/go-sdk/pkg/config/datafileprojectconfig" + "github.com/optimizely/go-sdk/pkg/logging" "github.com/optimizely/go-sdk/pkg/notification" "github.com/optimizely/go-sdk/pkg/utils" diff --git a/pkg/config/static_manager.go b/pkg/config/static_manager.go index 2c685ea64..0b38c3b27 100644 --- a/pkg/config/static_manager.go +++ b/pkg/config/static_manager.go @@ -20,10 +20,10 @@ package config import ( "errors" "fmt" - "github.com/optimizely/go-sdk/pkg/logging" "sync" "github.com/optimizely/go-sdk/pkg/config/datafileprojectconfig" + "github.com/optimizely/go-sdk/pkg/logging" "github.com/optimizely/go-sdk/pkg/notification" "github.com/optimizely/go-sdk/pkg/utils" ) diff --git a/pkg/config/static_manager_test.go b/pkg/config/static_manager_test.go index 17cf87182..7e991f2bf 100644 --- a/pkg/config/static_manager_test.go +++ b/pkg/config/static_manager_test.go @@ -18,11 +18,12 @@ package config import ( "errors" - "github.com/optimizely/go-sdk/pkg/logging" - "github.com/optimizely/go-sdk/pkg/notification" "testing" "github.com/optimizely/go-sdk/pkg/config/datafileprojectconfig" + "github.com/optimizely/go-sdk/pkg/logging" + "github.com/optimizely/go-sdk/pkg/notification" + "github.com/stretchr/testify/assert" ) diff --git a/pkg/decision/bucketer/murmurhashbucketer.go b/pkg/decision/bucketer/murmurhashbucketer.go index c8f08b03b..076e7e4db 100644 --- a/pkg/decision/bucketer/murmurhashbucketer.go +++ b/pkg/decision/bucketer/murmurhashbucketer.go @@ -19,10 +19,11 @@ package bucketer import ( "fmt" - "github.com/optimizely/go-sdk/pkg/logging" "math" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" + "github.com/twmb/murmur3" ) diff --git a/pkg/decision/composite_experiment_service.go b/pkg/decision/composite_experiment_service.go index 306adeb28..5843f5ee4 100644 --- a/pkg/decision/composite_experiment_service.go +++ b/pkg/decision/composite_experiment_service.go @@ -19,9 +19,8 @@ package decision import ( "fmt" - "github.com/optimizely/go-sdk/pkg/logging" - "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" ) // CESOptionFunc is used to assign optional configuration options diff --git a/pkg/decision/composite_experiment_service_test.go b/pkg/decision/composite_experiment_service_test.go index fa465d196..86f497c6b 100644 --- a/pkg/decision/composite_experiment_service_test.go +++ b/pkg/decision/composite_experiment_service_test.go @@ -18,12 +18,12 @@ package decision import ( "errors" - "github.com/optimizely/go-sdk/pkg/logging" "testing" "github.com/stretchr/testify/suite" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" ) type CompositeExperimentTestSuite struct { diff --git a/pkg/decision/composite_feature_service_test.go b/pkg/decision/composite_feature_service_test.go index 27be36eb7..08d915771 100644 --- a/pkg/decision/composite_feature_service_test.go +++ b/pkg/decision/composite_feature_service_test.go @@ -18,11 +18,12 @@ package decision import ( "errors" - "github.com/optimizely/go-sdk/pkg/logging" "testing" "github.com/optimizely/go-sdk/pkg/decision/reasons" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" + "github.com/stretchr/testify/suite" ) diff --git a/pkg/decision/experiment_bucketer_service.go b/pkg/decision/experiment_bucketer_service.go index 94ff8a565..d8ea3b319 100644 --- a/pkg/decision/experiment_bucketer_service.go +++ b/pkg/decision/experiment_bucketer_service.go @@ -19,12 +19,12 @@ package decision import ( "fmt" - "github.com/optimizely/go-sdk/pkg/logging" "github.com/optimizely/go-sdk/pkg/decision/bucketer" "github.com/optimizely/go-sdk/pkg/decision/evaluator" "github.com/optimizely/go-sdk/pkg/decision/reasons" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" ) // ExperimentBucketerService makes a decision using the experiment bucketer diff --git a/pkg/decision/experiment_override_service_test.go b/pkg/decision/experiment_override_service_test.go index e2e077191..45e19d9b7 100644 --- a/pkg/decision/experiment_override_service_test.go +++ b/pkg/decision/experiment_override_service_test.go @@ -18,12 +18,13 @@ package decision import ( - "github.com/optimizely/go-sdk/pkg/logging" "sync" "testing" "github.com/optimizely/go-sdk/pkg/decision/reasons" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" + "github.com/stretchr/testify/suite" ) diff --git a/pkg/decision/feature_experiment_service_test.go b/pkg/decision/feature_experiment_service_test.go index ea1f3257b..cb68ad885 100644 --- a/pkg/decision/feature_experiment_service_test.go +++ b/pkg/decision/feature_experiment_service_test.go @@ -17,10 +17,11 @@ package decision import ( - "github.com/optimizely/go-sdk/pkg/logging" "testing" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" + "github.com/stretchr/testify/suite" ) diff --git a/pkg/decision/persisting_experiment_service_test.go b/pkg/decision/persisting_experiment_service_test.go index 7745f8a4a..6c6b7380f 100644 --- a/pkg/decision/persisting_experiment_service_test.go +++ b/pkg/decision/persisting_experiment_service_test.go @@ -18,10 +18,11 @@ package decision import ( - "github.com/optimizely/go-sdk/pkg/logging" "testing" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" ) diff --git a/pkg/decision/rollout_service.go b/pkg/decision/rollout_service.go index 30d63f13d..f7245eb22 100644 --- a/pkg/decision/rollout_service.go +++ b/pkg/decision/rollout_service.go @@ -22,9 +22,8 @@ import ( "github.com/optimizely/go-sdk/pkg/decision/evaluator" "github.com/optimizely/go-sdk/pkg/decision/reasons" - "github.com/optimizely/go-sdk/pkg/logging" - "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" ) // RolloutService makes a feature decision for a given feature rollout diff --git a/pkg/decision/rollout_service_test.go b/pkg/decision/rollout_service_test.go index 5dda6c76e..b487ddd44 100644 --- a/pkg/decision/rollout_service_test.go +++ b/pkg/decision/rollout_service_test.go @@ -17,17 +17,16 @@ package decision import ( - "github.com/optimizely/go-sdk/pkg/logging" "testing" "github.com/optimizely/go-sdk/pkg/decision/evaluator" - "github.com/optimizely/go-sdk/pkg/decision/reasons" + "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" - "github.com/optimizely/go-sdk/pkg/entities" ) type RolloutServiceTestSuite struct { diff --git a/pkg/event/dispatcher.go b/pkg/event/dispatcher.go index 5e03a9655..58026f80d 100644 --- a/pkg/event/dispatcher.go +++ b/pkg/event/dispatcher.go @@ -19,11 +19,11 @@ package event import ( "fmt" - "github.com/optimizely/go-sdk/pkg/logging" "net/http" "sync" "time" + "github.com/optimizely/go-sdk/pkg/logging" "github.com/optimizely/go-sdk/pkg/metrics" "github.com/optimizely/go-sdk/pkg/utils" ) diff --git a/pkg/notification/manager_test.go b/pkg/notification/manager_test.go index 4f72381eb..7c3a0f773 100644 --- a/pkg/notification/manager_test.go +++ b/pkg/notification/manager_test.go @@ -1,9 +1,10 @@ package notification import ( - "github.com/optimizely/go-sdk/pkg/logging" "testing" + "github.com/optimizely/go-sdk/pkg/logging" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" ) From 92a63f7ebe0eea49ebec9835df8bbc37598c3452 Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Thu, 19 Mar 2020 11:31:28 -0700 Subject: [PATCH 10/28] remove 1.8 for 1.10 and upgrade 1.13 to 1.14 --- .travis.yml | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/.travis.yml b/.travis.yml index 0b8e64ff9..a30e33933 100644 --- a/.travis.yml +++ b/.travis.yml @@ -47,24 +47,15 @@ jobs: - <<: *test stage: 'Unit test' - env: GIMME_GO_VERSION=1.8.x + env: GIMME_GO_VERSION=1.10.x before_script: - # GO module was not introduced earlier. need symlink to search in GOPATH - - mkdir -p $GOPATH/src/github.com && pushd $GOPATH/src/github.com && ln -s $HOME/build/optimizely optimizely && popd - script: - # Need to download packages explicitly - - mkdir $GOPATH/src/github.com/twmb && cd $GOPATH/src/github.com/twmb && git clone https://github.com/twmb/murmur3.git && cd $TRAVIS_BUILD_DIR - - pushd $GOPATH/src/github.com/twmb/murmur3 && git checkout v1.0.0 && popd - - go get -v -d ./... - # This pkg not in go 1.8 - - go get github.com/stretchr/testify - - pushd $GOPATH/src/github.com/stretchr/testify && git checkout v1.4.0 && popd - # -coverprofile was not introduced in 1.8 - - make test + - go get github.com/mattn/goveralls + after_success: + - $GOPATH/bin/goveralls -coverprofile=profile.cov -service=travis-ci - <<: *test stage: 'Unit test' - env: GIMME_GO_VERSION=1.13.x + env: GIMME_GO_VERSION=1.14.x before_script: - go get github.com/mattn/goveralls after_success: From a794097b31c3b0ea247abc97b31b37ff5d3a3e7d Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Thu, 19 Mar 2020 11:42:29 -0700 Subject: [PATCH 11/28] try and upgrade to golang 1.10 --- .travis.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 0b8e64ff9..4a4490f7c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -47,7 +47,7 @@ jobs: - <<: *test stage: 'Unit test' - env: GIMME_GO_VERSION=1.8.x + env: GIMME_GO_VERSION=1.10.x before_script: # GO module was not introduced earlier. need symlink to search in GOPATH - mkdir -p $GOPATH/src/github.com && pushd $GOPATH/src/github.com && ln -s $HOME/build/optimizely optimizely && popd @@ -56,15 +56,15 @@ jobs: - mkdir $GOPATH/src/github.com/twmb && cd $GOPATH/src/github.com/twmb && git clone https://github.com/twmb/murmur3.git && cd $TRAVIS_BUILD_DIR - pushd $GOPATH/src/github.com/twmb/murmur3 && git checkout v1.0.0 && popd - go get -v -d ./... - # This pkg not in go 1.8 + # This pkg not in go 1.10 - go get github.com/stretchr/testify - pushd $GOPATH/src/github.com/stretchr/testify && git checkout v1.4.0 && popd - # -coverprofile was not introduced in 1.8 + # -coverprofile was not introduced in 1.10 - make test - <<: *test stage: 'Unit test' - env: GIMME_GO_VERSION=1.13.x + env: GIMME_GO_VERSION=1.14.x before_script: - go get github.com/mattn/goveralls after_success: From 67de14e095495a1e57163492dc96ccdc2830085d Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Mon, 23 Mar 2020 10:35:34 -0700 Subject: [PATCH 12/28] add a mapper to hide sdk key. add unit test --- pkg/logging/level_log_consumer_test.go | 4 ++-- pkg/logging/logger.go | 17 ++++++++++++++++- pkg/logging/logger_test.go | 20 ++++++++++++++++++++ 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/pkg/logging/level_log_consumer_test.go b/pkg/logging/level_log_consumer_test.go index 57ca14dd7..2fa051523 100644 --- a/pkg/logging/level_log_consumer_test.go +++ b/pkg/logging/level_log_consumer_test.go @@ -46,10 +46,10 @@ func TestLogFormatting(t *testing.T) { out := &bytes.Buffer{} newLogger := NewFilteredLevelLogConsumer(LogLevelInfo, out) - newLogger.Log(LogLevelInfo, "test message", map[string]interface{}{"name": "test-name", "sdkKey": "test-sdkKey"}) + newLogger.Log(LogLevelInfo, "test message", map[string]interface{}{"name": "test-name", "sdkKey": "testLogFormatting-sdkKey"}) assert.Contains(t, out.String(), "test message") assert.Contains(t, out.String(), "[Info]") assert.Contains(t, out.String(), "[test-name]") - assert.Contains(t, out.String(), "[test-sdkKey]") + assert.Contains(t, out.String(), "[testLogFormatting-sdkKey]") assert.Contains(t, out.String(), "[Optimizely]") } diff --git a/pkg/logging/logger.go b/pkg/logging/logger.go index cc9bdc2b4..534bb8d5e 100644 --- a/pkg/logging/logger.go +++ b/pkg/logging/logger.go @@ -20,7 +20,9 @@ package logging import ( "fmt" "os" + "strconv" "sync" + "sync/atomic" ) // LogLevel represents the level of the log (i.e. Debug, Info, Warning, Error) @@ -32,6 +34,8 @@ func (l LogLevel) String() string { var defaultLogConsumer OptimizelyLogConsumer var mutex = &sync.Mutex{} +var sdkKeyMappings = sync.Map{} +var count int32 = 0 const ( // LogLevelDebug log level @@ -70,10 +74,21 @@ func SetLogLevel(logLevel LogLevel) { // GetLogger returns a log producer with the given name func GetLogger(sdkKey, name string) OptimizelyLogProducer { return NamedLogProducer{ - fields: map[string]interface{}{"sdkKey":sdkKey, "name": name}, + fields: map[string]interface{}{"sdkKeyLogMapping":GetSdkKeyLogMapping(sdkKey), "name": name}, } } +func GetSdkKeyLogMapping(sdkKey string) string { + if logMapping, _ := sdkKeyMappings.LoadOrStore(sdkKey, "optimizely-" + + strconv.Itoa(int(atomic.AddInt32(&count, 1)))); logMapping != nil { + if lm, ok := logMapping.(string);ok { + return lm + } + } + + return "" +} + // NamedLogProducer produces logs prefixed with its name type NamedLogProducer struct { fields map[string]interface{} diff --git a/pkg/logging/logger_test.go b/pkg/logging/logger_test.go index 24a439750..d7f06a593 100644 --- a/pkg/logging/logger_test.go +++ b/pkg/logging/logger_test.go @@ -17,6 +17,7 @@ package logging import ( + "bytes" "errors" "testing" @@ -95,6 +96,25 @@ func TestNamedLoggerError(t *testing.T) { assert.Equal(t, []string{expectedLogMessage}, testLogger.loggedMessages) } +func TestNamedLoggerFields(t *testing.T) { + out := &bytes.Buffer{} + newLogger := NewFilteredLevelLogConsumer(LogLevelDebug, out) + + SetLogger(newLogger) + + logger := GetLogger("TestNamedLoggerFields-sdkKey", "TestNamedLoggerFields") + logger.Debug("test message") + + key := GetSdkKeyLogMapping("TestNamedLoggerFields-sdkKey") + assert.Contains(t, out.String(), "test message") + assert.Contains(t, out.String(), "[Debug]") + assert.Contains(t, out.String(), "[TestNamedLoggerFields]") + assert.Contains(t, out.String(), key) + assert.NotContains(t, out.String(), "TestNamedLoggerFields-sdkKey") + assert.Contains(t, out.String(), "[Optimizely]") + +} + func TestSetLogLevel(t *testing.T) { testLogger := new(MockOptimizelyLogger) testLogger.On("SetLogLevel", LogLevelError) From ea3d92672e77db694e32f87010ff88723bddcaeb Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Mon, 23 Mar 2020 10:52:48 -0700 Subject: [PATCH 13/28] added a comment for lint --- pkg/logging/logger.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/logging/logger.go b/pkg/logging/logger.go index 534bb8d5e..54fbaac2f 100644 --- a/pkg/logging/logger.go +++ b/pkg/logging/logger.go @@ -78,6 +78,7 @@ func GetLogger(sdkKey, name string) OptimizelyLogProducer { } } +// GetSdkKeyLogMapping returns a string that maps to the sdk key that is used for logging (hiding the sdk key) func GetSdkKeyLogMapping(sdkKey string) string { if logMapping, _ := sdkKeyMappings.LoadOrStore(sdkKey, "optimizely-" + strconv.Itoa(int(atomic.AddInt32(&count, 1)))); logMapping != nil { From a21ec56acf37bd86545bd94844f08954d26ee801 Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Mon, 23 Mar 2020 13:23:33 -0700 Subject: [PATCH 14/28] format cleanup --- pkg/client/factory.go | 2 +- pkg/notification/manager.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/client/factory.go b/pkg/client/factory.go index 342fbebf5..db67ab3c1 100644 --- a/pkg/client/factory.go +++ b/pkg/client/factory.go @@ -77,7 +77,7 @@ func (f OptimizelyFactory) Client(clientOptions ...OptionFunc) (*OptimizelyClien eg := utils.NewExecGroup(ctx, logging.GetLogger(f.SDKKey, "ExecGroup")) appClient := &OptimizelyClient{execGroup: eg, notificationCenter: registry.GetNotificationCenter(f.SDKKey), - logger: logging.GetLogger(f.SDKKey, "OptimizelyClient")} + logger: logging.GetLogger(f.SDKKey, "OptimizelyClient")} if f.configManager != nil { appClient.ConfigManager = f.configManager diff --git a/pkg/notification/manager.go b/pkg/notification/manager.go index 3f0392b9c..1c4642a26 100644 --- a/pkg/notification/manager.go +++ b/pkg/notification/manager.go @@ -19,9 +19,10 @@ package notification import ( "fmt" - "github.com/optimizely/go-sdk/pkg/logging" "sync" "sync/atomic" + + "github.com/optimizely/go-sdk/pkg/logging" ) // Manager is a generic interface for managing notifications of a particular type From a92a474b5186125dae0aebe5148f42388ef36f04 Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Mon, 23 Mar 2020 13:33:10 -0700 Subject: [PATCH 15/28] go lint --- pkg/logging/logger.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/logging/logger.go b/pkg/logging/logger.go index 54fbaac2f..9c8ceecb6 100644 --- a/pkg/logging/logger.go +++ b/pkg/logging/logger.go @@ -35,7 +35,7 @@ func (l LogLevel) String() string { var defaultLogConsumer OptimizelyLogConsumer var mutex = &sync.Mutex{} var sdkKeyMappings = sync.Map{} -var count int32 = 0 +var count int32 const ( // LogLevelDebug log level From dd307a86f0fc90ffe31730c9337a566b1207677f Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Mon, 23 Mar 2020 14:18:30 -0700 Subject: [PATCH 16/28] remove go get that is failing --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 4a4490f7c..64c305c93 100644 --- a/.travis.yml +++ b/.travis.yml @@ -55,7 +55,7 @@ jobs: # Need to download packages explicitly - mkdir $GOPATH/src/github.com/twmb && cd $GOPATH/src/github.com/twmb && git clone https://github.com/twmb/murmur3.git && cd $TRAVIS_BUILD_DIR - pushd $GOPATH/src/github.com/twmb/murmur3 && git checkout v1.0.0 && popd - - go get -v -d ./... + # - go get -v -d ./... # This pkg not in go 1.10 - go get github.com/stretchr/testify - pushd $GOPATH/src/github.com/stretchr/testify && git checkout v1.4.0 && popd From c9d69e4e0e617d4512396cdae12ee1b39f7f35ec Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Mon, 23 Mar 2020 14:56:09 -0700 Subject: [PATCH 17/28] put go get back in..fails --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 64c305c93..4a4490f7c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -55,7 +55,7 @@ jobs: # Need to download packages explicitly - mkdir $GOPATH/src/github.com/twmb && cd $GOPATH/src/github.com/twmb && git clone https://github.com/twmb/murmur3.git && cd $TRAVIS_BUILD_DIR - pushd $GOPATH/src/github.com/twmb/murmur3 && git checkout v1.0.0 && popd - # - go get -v -d ./... + - go get -v -d ./... # This pkg not in go 1.10 - go get github.com/stretchr/testify - pushd $GOPATH/src/github.com/stretchr/testify && git checkout v1.4.0 && popd From 0eda1e14bdcbba755ff388e6a7905247ad67e49b Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Mon, 23 Mar 2020 15:19:17 -0700 Subject: [PATCH 18/28] update to go 1.11 --- .travis.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 4a4490f7c..904dd6682 100644 --- a/.travis.yml +++ b/.travis.yml @@ -47,7 +47,7 @@ jobs: - <<: *test stage: 'Unit test' - env: GIMME_GO_VERSION=1.10.x + env: GIMME_GO_VERSION=1.11.x before_script: # GO module was not introduced earlier. need symlink to search in GOPATH - mkdir -p $GOPATH/src/github.com && pushd $GOPATH/src/github.com && ln -s $HOME/build/optimizely optimizely && popd @@ -56,10 +56,10 @@ jobs: - mkdir $GOPATH/src/github.com/twmb && cd $GOPATH/src/github.com/twmb && git clone https://github.com/twmb/murmur3.git && cd $TRAVIS_BUILD_DIR - pushd $GOPATH/src/github.com/twmb/murmur3 && git checkout v1.0.0 && popd - go get -v -d ./... - # This pkg not in go 1.10 + # This pkg not in go 1.11 - go get github.com/stretchr/testify - pushd $GOPATH/src/github.com/stretchr/testify && git checkout v1.4.0 && popd - # -coverprofile was not introduced in 1.10 + # -coverprofile was not introduced in 1.11 - make test - <<: *test From b069b610caed87b9a8ecc7fc9c5bfe4c9f93eb74 Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Mon, 23 Mar 2020 16:10:50 -0700 Subject: [PATCH 19/28] move back to 10 --- .travis.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 904dd6682..4a4490f7c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -47,7 +47,7 @@ jobs: - <<: *test stage: 'Unit test' - env: GIMME_GO_VERSION=1.11.x + env: GIMME_GO_VERSION=1.10.x before_script: # GO module was not introduced earlier. need symlink to search in GOPATH - mkdir -p $GOPATH/src/github.com && pushd $GOPATH/src/github.com && ln -s $HOME/build/optimizely optimizely && popd @@ -56,10 +56,10 @@ jobs: - mkdir $GOPATH/src/github.com/twmb && cd $GOPATH/src/github.com/twmb && git clone https://github.com/twmb/murmur3.git && cd $TRAVIS_BUILD_DIR - pushd $GOPATH/src/github.com/twmb/murmur3 && git checkout v1.0.0 && popd - go get -v -d ./... - # This pkg not in go 1.11 + # This pkg not in go 1.10 - go get github.com/stretchr/testify - pushd $GOPATH/src/github.com/stretchr/testify && git checkout v1.4.0 && popd - # -coverprofile was not introduced in 1.11 + # -coverprofile was not introduced in 1.10 - make test - <<: *test From 8baac24733b4da8534b7caf1a83d93e463cc0f3b Mon Sep 17 00:00:00 2001 From: Juan Carlos Tong Date: Mon, 23 Mar 2020 15:44:59 -0700 Subject: [PATCH 20/28] fix: update new dependency location DATA-DOG/godog moved to cucumber/godog --- tests/integration/main_test.go | 4 ++-- tests/integration/support/steps.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/main_test.go b/tests/integration/main_test.go index cb8d7d03e..54b063ae1 100644 --- a/tests/integration/main_test.go +++ b/tests/integration/main_test.go @@ -21,8 +21,8 @@ import ( "os" "testing" - "github.com/DATA-DOG/godog" - "github.com/DATA-DOG/godog/colors" + "github.com/cucumber/godog" + "github.com/cucumber/godog/colors" "github.com/optimizely/go-sdk/tests/integration/support" ) diff --git a/tests/integration/support/steps.go b/tests/integration/support/steps.go index 2606623d7..28dd27758 100644 --- a/tests/integration/support/steps.go +++ b/tests/integration/support/steps.go @@ -24,7 +24,7 @@ import ( "github.com/optimizely/go-sdk/pkg/decision" - "github.com/DATA-DOG/godog/gherkin" + "github.com/cucumber/gherkin-go" "github.com/google/uuid" "github.com/optimizely/go-sdk/pkg/entities" "github.com/optimizely/go-sdk/tests/integration/models" From e568567e68e54b3b3f427e624ba68ef9bb04f340 Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Mon, 23 Mar 2020 16:48:24 -0700 Subject: [PATCH 21/28] add sdk key logger override and use sdk key for logging --- pkg/logging/logger.go | 13 +++++++++++++ pkg/logging/logger_test.go | 40 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/pkg/logging/logger.go b/pkg/logging/logger.go index 9c8ceecb6..f13b156f6 100644 --- a/pkg/logging/logger.go +++ b/pkg/logging/logger.go @@ -90,6 +90,19 @@ func GetSdkKeyLogMapping(sdkKey string) string { return "" } +// UseSdkKeyForLogging by default the sdk key is masked for logging. +// This sets it to use the SDK Key and should be set before the creating your client factory. +func UseSdkKeyForLogging(sdkKey string) { + SetSdkKeyLogMapping(sdkKey, sdkKey) +} + +// SetSdkKeyLogMapping sets the logging key to use for the Optimizely sdk key. +// By default, the sdk key has masking. This can override the default of optimizely-1,2,3,4,etc.. +// This should be set before creating your Optimizely client factory. +func SetSdkKeyLogMapping(sdkKey string, logMapping string) { + sdkKeyMappings.Store(sdkKey, logMapping) +} + // NamedLogProducer produces logs prefixed with its name type NamedLogProducer struct { fields map[string]interface{} diff --git a/pkg/logging/logger_test.go b/pkg/logging/logger_test.go index d7f06a593..adf5a97e0 100644 --- a/pkg/logging/logger_test.go +++ b/pkg/logging/logger_test.go @@ -115,6 +115,46 @@ func TestNamedLoggerFields(t *testing.T) { } +func TestLogSdkKeyOverride(t *testing.T) { + out := &bytes.Buffer{} + newLogger := NewFilteredLevelLogConsumer(LogLevelDebug, out) + + SetLogger(newLogger) + + key := "test-test-test" + SetSdkKeyLogMapping("TestLogOverride-sdkKey", key) + + logger := GetLogger("TestLogOverride-sdkKey", "TestLogSdkKeyOverride") + logger.Debug("test message") + + assert.Contains(t, out.String(), "test message") + assert.Contains(t, out.String(), "[Debug]") + assert.Contains(t, out.String(), "[TestLogSdkKeyOverride]") + assert.Contains(t, out.String(), key) + assert.NotContains(t, out.String(), "TestNamedLoggerFields-sdkKey") + assert.Contains(t, out.String(), "[Optimizely]") +} + +func TestLogSdkKey(t *testing.T) { + out := &bytes.Buffer{} + newLogger := NewFilteredLevelLogConsumer(LogLevelDebug, out) + + SetLogger(newLogger) + + key := "TestLogSdkKey-sdkKey" + + UseSdkKeyForLogging(key) + + logger := GetLogger(key, "TestLogSdkKeyOverride") + logger.Debug("test message") + + assert.Contains(t, out.String(), "test message") + assert.Contains(t, out.String(), "[Debug]") + assert.Contains(t, out.String(), "[TestLogSdkKeyOverride]") + assert.Contains(t, out.String(), key) + assert.Contains(t, out.String(), "[Optimizely]") +} + func TestSetLogLevel(t *testing.T) { testLogger := new(MockOptimizelyLogger) testLogger.On("SetLogLevel", LogLevelError) From 4ab256eacd7058f86f776e7f138f8807d05612a9 Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Mon, 23 Mar 2020 16:53:03 -0700 Subject: [PATCH 22/28] go lint --- pkg/logging/logger.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/logging/logger.go b/pkg/logging/logger.go index f13b156f6..11acc2322 100644 --- a/pkg/logging/logger.go +++ b/pkg/logging/logger.go @@ -99,7 +99,7 @@ func UseSdkKeyForLogging(sdkKey string) { // SetSdkKeyLogMapping sets the logging key to use for the Optimizely sdk key. // By default, the sdk key has masking. This can override the default of optimizely-1,2,3,4,etc.. // This should be set before creating your Optimizely client factory. -func SetSdkKeyLogMapping(sdkKey string, logMapping string) { +func SetSdkKeyLogMapping(sdkKey, logMapping string) { sdkKeyMappings.Store(sdkKey, logMapping) } From 4291bfd74b55bf75a629b7ce67f595520e878cee Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Tue, 24 Mar 2020 13:35:30 -0700 Subject: [PATCH 23/28] remove sdk key from bucketer, pass in logger --- pkg/decision/bucketer/experiment_bucketer.go | 5 +++-- pkg/decision/bucketer/experiment_bucketer_test.go | 3 ++- pkg/decision/bucketer/murmurhashbucketer.go | 4 ++-- pkg/decision/bucketer/murmurhashbucketer_test.go | 5 +++-- pkg/decision/experiment_bucketer_service.go | 2 +- 5 files changed, 11 insertions(+), 8 deletions(-) diff --git a/pkg/decision/bucketer/experiment_bucketer.go b/pkg/decision/bucketer/experiment_bucketer.go index c0fbe501e..2a380efa0 100644 --- a/pkg/decision/bucketer/experiment_bucketer.go +++ b/pkg/decision/bucketer/experiment_bucketer.go @@ -20,6 +20,7 @@ package bucketer import ( "github.com/optimizely/go-sdk/pkg/decision/reasons" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" ) // ExperimentBucketer is used to bucket the user into a particular entity in the experiment's traffic alloc range @@ -33,9 +34,9 @@ type MurmurhashExperimentBucketer struct { } // NewMurmurhashExperimentBucketer returns a new instance of the murmurhash experiment bucketer -func NewMurmurhashExperimentBucketer(hashSeed uint32) *MurmurhashExperimentBucketer { +func NewMurmurhashExperimentBucketer(logger logging.OptimizelyLogProducer, hashSeed uint32) *MurmurhashExperimentBucketer { return &MurmurhashExperimentBucketer{ - bucketer: MurmurhashBucketer{hashSeed: hashSeed}, + bucketer: MurmurhashBucketer{hashSeed: hashSeed, logger:logger}, } } diff --git a/pkg/decision/bucketer/experiment_bucketer_test.go b/pkg/decision/bucketer/experiment_bucketer_test.go index 7c486cb3a..2d3ebec8c 100644 --- a/pkg/decision/bucketer/experiment_bucketer_test.go +++ b/pkg/decision/bucketer/experiment_bucketer_test.go @@ -1,6 +1,7 @@ package bucketer import ( + "github.com/optimizely/go-sdk/pkg/logging" "testing" "github.com/optimizely/go-sdk/pkg/decision/reasons" @@ -46,7 +47,7 @@ func TestBucketExclusionGroups(t *testing.T) { }, } - bucketer := NewMurmurhashExperimentBucketer(DefaultHashSeed) + bucketer := NewMurmurhashExperimentBucketer(logging.GetLogger("","TestBucketExclusionGroups" ), DefaultHashSeed) // ppid2 + 1886780722 (groupId) will generate bucket value of 2434 which maps to experiment 1 bucketedVariation, reason, _ := bucketer.Bucket("ppid2", experiment1, exclusionGroup) assert.Equal(t, experiment1.Variations["22222"], *bucketedVariation) diff --git a/pkg/decision/bucketer/murmurhashbucketer.go b/pkg/decision/bucketer/murmurhashbucketer.go index 076e7e4db..a9eb25ef2 100644 --- a/pkg/decision/bucketer/murmurhashbucketer.go +++ b/pkg/decision/bucketer/murmurhashbucketer.go @@ -46,10 +46,10 @@ type MurmurhashBucketer struct { } // NewMurmurhashBucketer returns a new instance of the murmurhash bucketer -func NewMurmurhashBucketer(sdkKey string, hashSeed uint32) *MurmurhashBucketer { +func NewMurmurhashBucketer(logger logging.OptimizelyLogProducer, hashSeed uint32) *MurmurhashBucketer { return &MurmurhashBucketer{ hashSeed: hashSeed, - logger: logging.GetLogger(sdkKey, "MurmurhashBucketer"), + logger: logger, } } diff --git a/pkg/decision/bucketer/murmurhashbucketer_test.go b/pkg/decision/bucketer/murmurhashbucketer_test.go index 56977afe2..3db47c552 100644 --- a/pkg/decision/bucketer/murmurhashbucketer_test.go +++ b/pkg/decision/bucketer/murmurhashbucketer_test.go @@ -2,6 +2,7 @@ package bucketer import ( "fmt" + "github.com/optimizely/go-sdk/pkg/logging" "testing" "github.com/optimizely/go-sdk/pkg/entities" @@ -9,7 +10,7 @@ import ( ) func TestBucketToEntity(t *testing.T) { - bucketer := NewMurmurhashBucketer("", DefaultHashSeed) + bucketer := NewMurmurhashBucketer(logging.GetLogger("", "TestBucketToEntity"), DefaultHashSeed) experimentID := "1886780721" experimentID2 := "1886780722" @@ -51,7 +52,7 @@ func TestBucketToEntity(t *testing.T) { } func TestGenerateBucketValue(t *testing.T) { - bucketer := NewMurmurhashBucketer("", DefaultHashSeed) + bucketer := NewMurmurhashBucketer(logging.GetLogger("", "TestGenerateBucketValue"), DefaultHashSeed) // copied from unit tests in the other SDKs experimentID := "1886780721" diff --git a/pkg/decision/experiment_bucketer_service.go b/pkg/decision/experiment_bucketer_service.go index d8ea3b319..1ee16ad78 100644 --- a/pkg/decision/experiment_bucketer_service.go +++ b/pkg/decision/experiment_bucketer_service.go @@ -40,7 +40,7 @@ func NewExperimentBucketerService(logger logging.OptimizelyLogProducer) *Experim return &ExperimentBucketerService{ logger:logger, audienceTreeEvaluator: evaluator.NewMixedTreeEvaluator(), - bucketer: *bucketer.NewMurmurhashExperimentBucketer(bucketer.DefaultHashSeed), + bucketer: *bucketer.NewMurmurhashExperimentBucketer(logger, bucketer.DefaultHashSeed), } } From e808adcc5d83215dd61bf125ad07c0ed95fb41ba Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Tue, 24 Mar 2020 16:18:36 -0700 Subject: [PATCH 24/28] fix order, fix count, add tests --- pkg/logging/level_log_consumer.go | 13 ++++++++-- pkg/logging/logger.go | 9 ++++--- pkg/logging/logger_test.go | 43 +++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/pkg/logging/level_log_consumer.go b/pkg/logging/level_log_consumer.go index e69ac6307..08df46df1 100644 --- a/pkg/logging/level_log_consumer.go +++ b/pkg/logging/level_log_consumer.go @@ -20,6 +20,7 @@ package logging import ( "io" "log" + "sort" "strings" ) @@ -38,8 +39,16 @@ func (l *FilteredLevelLogConsumer) Log(level LogLevel, message string, fields ma _, _ = messBuilder.WriteString(level.String()) _, _ = messBuilder.WriteString("]") - for _, v := range fields { - if s, ok := v.(string); ok { + keys := make([]string, len(fields)) + for k,_ := range fields { + keys = append(keys, k) + } + sort.Strings(keys) + for _, k := range keys { + if s, ok := fields[k].(string);ok { + if s == "" { + continue + } _, _ = messBuilder.WriteString("[") _, _ = messBuilder.WriteString(s) _, _ = messBuilder.WriteString("]") diff --git a/pkg/logging/logger.go b/pkg/logging/logger.go index 11acc2322..cd6bc834a 100644 --- a/pkg/logging/logger.go +++ b/pkg/logging/logger.go @@ -74,17 +74,20 @@ func SetLogLevel(logLevel LogLevel) { // GetLogger returns a log producer with the given name func GetLogger(sdkKey, name string) OptimizelyLogProducer { return NamedLogProducer{ - fields: map[string]interface{}{"sdkKeyLogMapping":GetSdkKeyLogMapping(sdkKey), "name": name}, + fields: map[string]interface{}{"aSdkKeyLogMapping":GetSdkKeyLogMapping(sdkKey), "name": name}, } } // GetSdkKeyLogMapping returns a string that maps to the sdk key that is used for logging (hiding the sdk key) func GetSdkKeyLogMapping(sdkKey string) string { - if logMapping, _ := sdkKeyMappings.LoadOrStore(sdkKey, "optimizely-" + - strconv.Itoa(int(atomic.AddInt32(&count, 1)))); logMapping != nil { + if logMapping, _ := sdkKeyMappings.Load(sdkKey); logMapping != nil { if lm, ok := logMapping.(string);ok { return lm } + } else if sdkKey != "" { + mapping := "optimizely-" + strconv.Itoa(int(atomic.AddInt32(&count, 1))) + sdkKeyMappings.Store(sdkKey, mapping) + return mapping } return "" diff --git a/pkg/logging/logger_test.go b/pkg/logging/logger_test.go index adf5a97e0..eec1922f2 100644 --- a/pkg/logging/logger_test.go +++ b/pkg/logging/logger_test.go @@ -19,6 +19,7 @@ package logging import ( "bytes" "errors" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -155,6 +156,48 @@ func TestLogSdkKey(t *testing.T) { assert.Contains(t, out.String(), "[Optimizely]") } +func TestLoggingOrder(t *testing.T) { + out := &bytes.Buffer{} + newLogger := NewFilteredLevelLogConsumer(LogLevelDebug, out) + + SetLogger(newLogger) + + key := "TestLoggingOrder-sdkKey" + + logger := GetLogger(key, "TestLoggingOrder") + logger.Debug("test message") + + key = GetSdkKeyLogMapping(key) + + response := out.String() + + assert.Contains(t, response, "test message") + assert.Contains(t, response, "[Debug][" + key +"][TestLoggingOrder] test message" ) + assert.True(t, strings.HasPrefix(response, "[Optimizely]")) + +} + +func TestLoggingOrderEmpty(t *testing.T) { + out := &bytes.Buffer{} + newLogger := NewFilteredLevelLogConsumer(LogLevelDebug, out) + + SetLogger(newLogger) + + key := "" + + logger := GetLogger(key, "TestLoggingOrder") + logger.Debug("test message") + + key = GetSdkKeyLogMapping(key) + + response := out.String() + + assert.Contains(t, response, "test message") + assert.Contains(t, response, "[Debug][TestLoggingOrder] test message" ) + assert.True(t, strings.HasPrefix(response, "[Optimizely]")) + +} + func TestSetLogLevel(t *testing.T) { testLogger := new(MockOptimizelyLogger) testLogger.On("SetLogLevel", LogLevelError) From ee14e3e8749dd58c3ca618e083062904b86bb9f9 Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Tue, 24 Mar 2020 16:27:29 -0700 Subject: [PATCH 25/28] fix lint error --- pkg/logging/level_log_consumer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/logging/level_log_consumer.go b/pkg/logging/level_log_consumer.go index 08df46df1..2dbc15bd9 100644 --- a/pkg/logging/level_log_consumer.go +++ b/pkg/logging/level_log_consumer.go @@ -40,7 +40,7 @@ func (l *FilteredLevelLogConsumer) Log(level LogLevel, message string, fields ma _, _ = messBuilder.WriteString(level.String()) _, _ = messBuilder.WriteString("]") keys := make([]string, len(fields)) - for k,_ := range fields { + for k := range fields { keys = append(keys, k) } sort.Strings(keys) From 01e259bc93064a0d8e7ba0eb441949816e7d97d5 Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Wed, 25 Mar 2020 07:53:39 -0700 Subject: [PATCH 26/28] cleanup log writting --- pkg/logging/level_log_consumer.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/pkg/logging/level_log_consumer.go b/pkg/logging/level_log_consumer.go index 2dbc15bd9..b87e09067 100644 --- a/pkg/logging/level_log_consumer.go +++ b/pkg/logging/level_log_consumer.go @@ -18,6 +18,7 @@ package logging import ( + "fmt" "io" "log" "sort" @@ -45,13 +46,8 @@ func (l *FilteredLevelLogConsumer) Log(level LogLevel, message string, fields ma } sort.Strings(keys) for _, k := range keys { - if s, ok := fields[k].(string);ok { - if s == "" { - continue - } - _, _ = messBuilder.WriteString("[") - _, _ = messBuilder.WriteString(s) - _, _ = messBuilder.WriteString("]") + if s, ok := fields[k].(string);ok && s != "" { + fmt.Fprintf(&messBuilder, "[%s]", s) } } _, _ = messBuilder.WriteString(" ") From 910719a4f3ff035e766f4ca2273150316dca183c Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Wed, 25 Mar 2020 08:01:46 -0700 Subject: [PATCH 27/28] cleanup log to use fprintf --- pkg/logging/level_log_consumer.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/logging/level_log_consumer.go b/pkg/logging/level_log_consumer.go index b87e09067..2feaaac94 100644 --- a/pkg/logging/level_log_consumer.go +++ b/pkg/logging/level_log_consumer.go @@ -36,22 +36,22 @@ func (l *FilteredLevelLogConsumer) Log(level LogLevel, message string, fields ma if l.level <= level { // prepends the name and log level to the message messBuilder := strings.Builder{} - _, _ = messBuilder.WriteString("[") - _, _ = messBuilder.WriteString(level.String()) - _, _ = messBuilder.WriteString("]") + fmt.Fprintf(&messBuilder, "[%s]", level.String()) + keys := make([]string, len(fields)) for k := range fields { keys = append(keys, k) } sort.Strings(keys) + for _, k := range keys { if s, ok := fields[k].(string);ok && s != "" { fmt.Fprintf(&messBuilder, "[%s]", s) } } - _, _ = messBuilder.WriteString(" ") - _, _ = messBuilder.WriteString(message) + fmt.Fprintf(&messBuilder, " %s", message) + l.logger.Println(messBuilder.String()) } } From 08d4c373853a1ce94403e33779cd3217891e0acc Mon Sep 17 00:00:00 2001 From: Tom Zurkan Date: Wed, 25 Mar 2020 08:13:26 -0700 Subject: [PATCH 28/28] refactor to make the logger the last argument except in varg parameters --- pkg/client/factory.go | 2 +- pkg/config/datafileprojectconfig/config.go | 2 +- .../datafileprojectconfig/config_test.go | 6 ++--- pkg/config/optimizely_config_test.go | 2 +- pkg/config/polling_manager.go | 5 ++-- pkg/config/polling_manager_test.go | 26 +++++++++---------- pkg/config/static_manager.go | 6 ++--- pkg/config/static_manager_test.go | 10 +++---- pkg/decision/composite_experiment_service.go | 6 ++--- pkg/decision/experiment_override_service.go | 2 +- .../experiment_override_service_test.go | 2 +- pkg/decision/persisting_experiment_service.go | 2 +- .../persisting_experiment_service_test.go | 12 +++------ pkg/logging/level_log_consumer.go | 2 +- 14 files changed, 39 insertions(+), 46 deletions(-) diff --git a/pkg/client/factory.go b/pkg/client/factory.go index db67ab3c1..ecb914fdc 100644 --- a/pkg/client/factory.go +++ b/pkg/client/factory.go @@ -214,7 +214,7 @@ func (f OptimizelyFactory) StaticClient() (*OptimizelyClient, error) { configManager = staticConfigManager } else if f.Datafile != nil { - staticConfigManager, err := config.NewStaticProjectConfigManagerFromPayload(logging.GetLogger(f.SDKKey, "StaticProjectConfigManagerFromPayload"), f.Datafile) + staticConfigManager, err := config.NewStaticProjectConfigManagerFromPayload(f.Datafile, logging.GetLogger(f.SDKKey, "StaticProjectConfigManagerFromPayload")) if err != nil { return nil, err diff --git a/pkg/config/datafileprojectconfig/config.go b/pkg/config/datafileprojectconfig/config.go index 961f0512d..fb45f8d52 100644 --- a/pkg/config/datafileprojectconfig/config.go +++ b/pkg/config/datafileprojectconfig/config.go @@ -173,7 +173,7 @@ func (c DatafileProjectConfig) GetGroupByID(groupID string) (entities.Group, err } // NewDatafileProjectConfig initializes a new datafile from a json byte array using the default JSON datafile parser -func NewDatafileProjectConfig(logger logging.OptimizelyLogProducer, jsonDatafile []byte) (*DatafileProjectConfig, error) { +func NewDatafileProjectConfig(jsonDatafile []byte, logger logging.OptimizelyLogProducer) (*DatafileProjectConfig, error) { datafile, err := Parse(jsonDatafile) if err != nil { logger.Error("Error parsing datafile", err) diff --git a/pkg/config/datafileprojectconfig/config_test.go b/pkg/config/datafileprojectconfig/config_test.go index ec7ad95b1..71cd97250 100644 --- a/pkg/config/datafileprojectconfig/config_test.go +++ b/pkg/config/datafileprojectconfig/config_test.go @@ -28,12 +28,12 @@ import ( ) func TestNewDatafileProjectConfigNil(t *testing.T) { - projectConfig, err := NewDatafileProjectConfig(logging.GetLogger("", "DatafileProjectConfig"), nil) + projectConfig, err := NewDatafileProjectConfig(nil, logging.GetLogger("", "DatafileProjectConfig")) assert.Error(t, err) assert.Nil(t, projectConfig) jsonDatafileStr := `{"accountID": "123", "revision": "1", "projectId": "12345", "version": "2"}` - projectConfig, err = NewDatafileProjectConfig(logging.GetLogger("", "DatafileProjectConfig"), []byte(jsonDatafileStr)) + projectConfig, err = NewDatafileProjectConfig([]byte(jsonDatafileStr), logging.GetLogger("", "DatafileProjectConfig")) assert.Error(t, err) assert.Nil(t, projectConfig) } @@ -42,7 +42,7 @@ func TestNewDatafileProjectConfigNotNil(t *testing.T) { dpc := DatafileProjectConfig{accountID: "123", revision: "1", projectID: "12345"} jsonDatafileStr := `{"accountID": "123", "revision": "1", "projectId": "12345", "version": "4"}` jsonDatafile := []byte(jsonDatafileStr) - projectConfig, err := NewDatafileProjectConfig(logging.GetLogger("", "DatafileProjectConfig"), jsonDatafile) + projectConfig, err := NewDatafileProjectConfig(jsonDatafile, logging.GetLogger("", "DatafileProjectConfig")) assert.Nil(t, err) assert.NotNil(t, projectConfig) assert.Equal(t, dpc.accountID, projectConfig.accountID) diff --git a/pkg/config/optimizely_config_test.go b/pkg/config/optimizely_config_test.go index 3a6c8bf62..8d2c2459e 100644 --- a/pkg/config/optimizely_config_test.go +++ b/pkg/config/optimizely_config_test.go @@ -51,7 +51,7 @@ func (s *OptimizelyConfigTestSuite) SetupTest() { s.Fail("error opening file " + dataFileName) } - projectMgr, err := NewStaticProjectConfigManagerFromPayload(logging.GetLogger("", "NewStaticProjectConfigManagerFromPayload"), dataFile) + projectMgr, err := NewStaticProjectConfigManagerFromPayload(dataFile, logging.GetLogger("", "NewStaticProjectConfigManagerFromPayload")) if err != nil { s.Fail("error creating project manager") } diff --git a/pkg/config/polling_manager.go b/pkg/config/polling_manager.go index 387558595..22dee4a6f 100644 --- a/pkg/config/polling_manager.go +++ b/pkg/config/polling_manager.go @@ -143,7 +143,7 @@ func (cm *PollingProjectConfigManager) SyncConfig() { cm.lastModified = lastModified } - projectConfig, err := datafileprojectconfig.NewDatafileProjectConfig(logging.GetLogger(cm.sdkKey, "NewDatafileProjectConfig"), datafile) + projectConfig, err := datafileprojectconfig.NewDatafileProjectConfig(datafile, logging.GetLogger(cm.sdkKey, "NewDatafileProjectConfig")) if err != nil { cm.logger.Warning("failed to create project config") closeMutex(errors.New("unable to parse datafile")) @@ -289,8 +289,7 @@ func (cm *PollingProjectConfigManager) setInitialDatafile(datafile []byte) { if len(datafile) != 0 { cm.configLock.Lock() defer cm.configLock.Unlock() - projectConfig, err := datafileprojectconfig.NewDatafileProjectConfig(logging.GetLogger(cm.sdkKey, "DatafileProjectConfig"), - datafile) + projectConfig, err := datafileprojectconfig.NewDatafileProjectConfig(datafile, logging.GetLogger(cm.sdkKey, "DatafileProjectConfig")) if projectConfig != nil { err = cm.setConfig(projectConfig) } diff --git a/pkg/config/polling_manager_test.go b/pkg/config/polling_manager_test.go index 0786b5a4c..7218ed8ab 100644 --- a/pkg/config/polling_manager_test.go +++ b/pkg/config/polling_manager_test.go @@ -106,7 +106,7 @@ func TestNewAsyncPollingProjectConfigManagerWithOptions(t *testing.T) { func TestSyncConfigFetchesDatafileUsingRequester(t *testing.T) { mockDatafile := []byte(`{"revision":"42","version": "4"}`) - projectConfig, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile) + projectConfig, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile, logger) mockRequester := new(MockRequester) mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile, http.Header{}, http.StatusOK, nil) @@ -153,7 +153,7 @@ func TestNewPollingProjectConfigManagerWithSimilarDatafileRevisions(t *testing.T // Test newer datafile should not replace the older one if revisions are the same mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`) mockDatafile2 := []byte(`{"revision":"42","botFiltering":false,"version": "4"}`) - projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile1) + projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1, logger) mockRequester := new(MockRequester) mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile2, http.Header{}, http.StatusOK, nil) @@ -188,7 +188,7 @@ func TestNewAsyncPollingProjectConfigManagerWithSimilarDatafileRevisions(t *test // Test newer datafile should not replace the older one if revisions are the same mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`) mockDatafile2 := []byte(`{"revision":"42","botFiltering":false,"version": "4"}`) - projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile1) + projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1, logger) mockRequester := new(MockRequester) mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile2, http.Header{}, http.StatusOK, nil) @@ -221,7 +221,7 @@ func TestNewAsyncPollingProjectConfigManagerWithSimilarDatafileRevisions(t *test func TestNewPollingProjectConfigManagerWithLastModifiedDates(t *testing.T) { mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`) - projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile1) + projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1, logger) mockRequester := new(MockRequester) modifiedDate := "Wed, 16 Oct 2019 20:16:45 GMT" responseHeaders := http.Header{} @@ -275,8 +275,8 @@ func TestNewPollingProjectConfigManagerWithDifferentDatafileRevisions(t *testing // Test newer datafile should replace the older one if revisions are different mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`) mockDatafile2 := []byte(`{"revision":"43","botFiltering":false,"version": "4"}`) - projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile1) - projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile2) + projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1, logger) + projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile2, logger) mockRequester := new(MockRequester) mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile2, http.Header{}, http.StatusOK, nil) @@ -311,8 +311,8 @@ func TestNewAsyncPollingProjectConfigManagerWithDifferentDatafileRevisions(t *te // Test newer datafile should replace the older one if revisions are different mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`) mockDatafile2 := []byte(`{"revision":"43","botFiltering":false,"version": "4"}`) - projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile1) - projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile2) + projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1, logger) + projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile2, logger) mockRequester := new(MockRequester) mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile2, http.Header{}, http.StatusOK, nil) @@ -347,8 +347,8 @@ func TestNewPollingProjectConfigManagerWithErrorHandling(t *testing.T) { mockDatafile1 := []byte("NOT-VALID") mockDatafile2 := []byte(`{"revision":"43","botFiltering":false,"version": "4"}`) - projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile1) - projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile2) + projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1, logger) + projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile2, logger) mockRequester := new(MockRequester) mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile1, http.Header{}, http.StatusOK, nil).Times(1) @@ -380,8 +380,8 @@ func TestNewAsyncPollingProjectConfigManagerWithErrorHandling(t *testing.T) { mockDatafile1 := []byte("NOT-VALID") mockDatafile2 := []byte(`{"revision":"43","botFiltering":false,"version": "4"}`) - projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile1) - projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile2) + projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1, logger) + projectConfig2, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile2, logger) mockRequester := new(MockRequester) mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile1, http.Header{}, http.StatusOK, nil).Times(1) @@ -445,7 +445,7 @@ func TestNewPollingProjectConfigManagerOnDecision(t *testing.T) { func TestNewAsyncPollingProjectConfigManagerOnDecision(t *testing.T) { mockDatafile1 := []byte(`{"revision":"42","botFiltering":true,"version": "4"}`) - projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(logger, mockDatafile1) + projectConfig1, _ := datafileprojectconfig.NewDatafileProjectConfig(mockDatafile1, logger) mockRequester := new(MockRequester) mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile1, http.Header{}, http.StatusOK, nil) diff --git a/pkg/config/static_manager.go b/pkg/config/static_manager.go index 0b38c3b27..93379d008 100644 --- a/pkg/config/static_manager.go +++ b/pkg/config/static_manager.go @@ -50,12 +50,12 @@ func NewStaticProjectConfigManagerFromURL(sdkKey string) (*StaticProjectConfigMa return nil, e } - return NewStaticProjectConfigManagerFromPayload(logger, datafile) + return NewStaticProjectConfigManagerFromPayload(datafile, logger) } // NewStaticProjectConfigManagerFromPayload returns new instance of StaticProjectConfigManager for payload -func NewStaticProjectConfigManagerFromPayload(logger logging.OptimizelyLogProducer, payload []byte) (*StaticProjectConfigManager, error) { - projectConfig, err := datafileprojectconfig.NewDatafileProjectConfig(logger, payload) +func NewStaticProjectConfigManagerFromPayload(payload []byte, logger logging.OptimizelyLogProducer) (*StaticProjectConfigManager, error) { + projectConfig, err := datafileprojectconfig.NewDatafileProjectConfig(payload, logger) if err != nil { return nil, err diff --git a/pkg/config/static_manager_test.go b/pkg/config/static_manager_test.go index 7e991f2bf..45bb0592f 100644 --- a/pkg/config/static_manager_test.go +++ b/pkg/config/static_manager_test.go @@ -40,15 +40,15 @@ func TestNewStaticProjectConfigManager(t *testing.T) { func TestNewStaticProjectConfigManagerFromPayload(t *testing.T) { mockDatafile := []byte(`{"accountId":"42","projectId":"123""}`) - configManager, err := NewStaticProjectConfigManagerFromPayload(tlogger, mockDatafile) + configManager, err := NewStaticProjectConfigManagerFromPayload(mockDatafile, tlogger) assert.Error(t, err) mockDatafile = []byte(`{"accountId":"42","projectId":"123",}`) - configManager, err = NewStaticProjectConfigManagerFromPayload(tlogger, mockDatafile) + configManager, err = NewStaticProjectConfigManagerFromPayload(mockDatafile, tlogger) assert.Error(t, err) mockDatafile = []byte(`{"accountId":"42","projectId":"123","version":"4"}`) - configManager, err = NewStaticProjectConfigManagerFromPayload(tlogger, mockDatafile) + configManager, err = NewStaticProjectConfigManagerFromPayload(mockDatafile, tlogger) assert.Nil(t, err) assert.Nil(t, configManager.optimizelyConfig) @@ -60,7 +60,7 @@ func TestNewStaticProjectConfigManagerFromPayload(t *testing.T) { func TestStaticGetOptimizelyConfig(t *testing.T) { mockDatafile := []byte(`{"accountId":"42","projectId":"123","version":"4"}`) - configManager, err := NewStaticProjectConfigManagerFromPayload(tlogger, mockDatafile) + configManager, err := NewStaticProjectConfigManagerFromPayload(mockDatafile, tlogger) assert.Nil(t, err) assert.Nil(t, configManager.optimizelyConfig) @@ -79,7 +79,7 @@ func TestNewStaticProjectConfigManagerFromURL(t *testing.T) { func TestNewStaticProjectConfigManagerOnDecision(t *testing.T) { mockDatafile := []byte(`{"accountId":"42","projectId":"123","version":"4"}`) - configManager, err := NewStaticProjectConfigManagerFromPayload(tlogger, mockDatafile) + configManager, err := NewStaticProjectConfigManagerFromPayload(mockDatafile, tlogger) assert.Nil(t, err) callback := func(notification notification.ProjectConfigUpdateNotification) { diff --git a/pkg/decision/composite_experiment_service.go b/pkg/decision/composite_experiment_service.go index 5843f5ee4..c8fd3de98 100644 --- a/pkg/decision/composite_experiment_service.go +++ b/pkg/decision/composite_experiment_service.go @@ -64,15 +64,13 @@ func NewCompositeExperimentService(sdkKey string, options ...CESOptionFunc) *Com // Prepend overrides if supplied if compositeExperimentService.overrideStore != nil { - overrideService := NewExperimentOverrideService(logging.GetLogger(sdkKey, "ExperimentOverrideService"), - compositeExperimentService.overrideStore) + overrideService := NewExperimentOverrideService(compositeExperimentService.overrideStore, logging.GetLogger(sdkKey, "ExperimentOverrideService")) experimentServices = append([]ExperimentService{overrideService}, experimentServices...) } experimentBucketerService := NewExperimentBucketerService(logging.GetLogger(sdkKey, "ExperimentBucketerService")) if compositeExperimentService.userProfileService != nil { - persistingExperimentService := NewPersistingExperimentService(logging.GetLogger(sdkKey, "PersistingExperimentService"), - experimentBucketerService, compositeExperimentService.userProfileService) + persistingExperimentService := NewPersistingExperimentService(compositeExperimentService.userProfileService, experimentBucketerService, logging.GetLogger(sdkKey, "PersistingExperimentService")) experimentServices = append(experimentServices, persistingExperimentService) } else { experimentServices = append(experimentServices, experimentBucketerService) diff --git a/pkg/decision/experiment_override_service.go b/pkg/decision/experiment_override_service.go index 7e93b0e1e..b55c58bc4 100644 --- a/pkg/decision/experiment_override_service.go +++ b/pkg/decision/experiment_override_service.go @@ -82,7 +82,7 @@ type ExperimentOverrideService struct { } // NewExperimentOverrideService returns a pointer to an initialized ExperimentOverrideService -func NewExperimentOverrideService(logger logging.OptimizelyLogProducer, overrides ExperimentOverrideStore) *ExperimentOverrideService { +func NewExperimentOverrideService(overrides ExperimentOverrideStore, logger logging.OptimizelyLogProducer) *ExperimentOverrideService { return &ExperimentOverrideService{ logger: logger, Overrides: overrides, diff --git a/pkg/decision/experiment_override_service_test.go b/pkg/decision/experiment_override_service_test.go index 45e19d9b7..f36fc2450 100644 --- a/pkg/decision/experiment_override_service_test.go +++ b/pkg/decision/experiment_override_service_test.go @@ -38,7 +38,7 @@ type ExperimentOverrideServiceTestSuite struct { func (s *ExperimentOverrideServiceTestSuite) SetupTest() { s.mockConfig = new(mockProjectConfig) s.overrides = NewMapExperimentOverridesStore() - s.overrideService = NewExperimentOverrideService(logging.GetLogger("", ""), s.overrides) + s.overrideService = NewExperimentOverrideService(s.overrides, logging.GetLogger("", "")) } func (s *ExperimentOverrideServiceTestSuite) TestOverridesIncludeVariation() { diff --git a/pkg/decision/persisting_experiment_service.go b/pkg/decision/persisting_experiment_service.go index ae3c0a6e8..114ebc75f 100644 --- a/pkg/decision/persisting_experiment_service.go +++ b/pkg/decision/persisting_experiment_service.go @@ -34,7 +34,7 @@ type PersistingExperimentService struct { } // NewPersistingExperimentService returns a new instance of the PersistingExperimentService -func NewPersistingExperimentService(logger logging.OptimizelyLogProducer, experimentBucketerService ExperimentService, userProfileService UserProfileService) *PersistingExperimentService { +func NewPersistingExperimentService(userProfileService UserProfileService, experimentBucketerService ExperimentService, logger logging.OptimizelyLogProducer) *PersistingExperimentService { persistingExperimentService := &PersistingExperimentService{ logger: logger, experimentBucketedService: experimentBucketerService, diff --git a/pkg/decision/persisting_experiment_service_test.go b/pkg/decision/persisting_experiment_service_test.go index 6c6b7380f..a9a750e3b 100644 --- a/pkg/decision/persisting_experiment_service_test.go +++ b/pkg/decision/persisting_experiment_service_test.go @@ -57,8 +57,7 @@ func (s *PersistingExperimentServiceTestSuite) SetupTest() { } func (s *PersistingExperimentServiceTestSuite) TestNilUserProfileService() { - persistingExperimentService := NewPersistingExperimentService(logging.GetLogger("", "NewPersistingExperimentService"), - s.mockExperimentService, nil) + persistingExperimentService := NewPersistingExperimentService(nil, s.mockExperimentService, logging.GetLogger("", "NewPersistingExperimentService")) decision, err := persistingExperimentService.GetDecision(s.testDecisionContext, testUserContext) s.Equal(s.testComputedDecision, decision) s.NoError(err) @@ -74,8 +73,7 @@ func (s *PersistingExperimentServiceTestSuite) TestSavedVariationFound() { s.mockUserProfileService.On("Lookup", testUserContext.ID).Return(savedUserProfile) s.mockUserProfileService.On("Save", mock.Anything) - persistingExperimentService := NewPersistingExperimentService(logging.GetLogger("", "NewPersistingExperimentService"), - s.mockExperimentService, s.mockUserProfileService) + persistingExperimentService := NewPersistingExperimentService(s.mockUserProfileService, s.mockExperimentService, logging.GetLogger("", "NewPersistingExperimentService")) decision, err := persistingExperimentService.GetDecision(s.testDecisionContext, testUserContext) savedDecision := ExperimentDecision{ Variation: &testExp1113Var2224, @@ -95,8 +93,7 @@ func (s *PersistingExperimentServiceTestSuite) TestNoSavedVariation() { } s.mockUserProfileService.On("Save", updatedUserProfile) - persistingExperimentService := NewPersistingExperimentService(logging.GetLogger("", "NewPersistingExperimentService"), - s.mockExperimentService, s.mockUserProfileService) + persistingExperimentService := NewPersistingExperimentService(s.mockUserProfileService, s.mockExperimentService, logging.GetLogger("", "NewPersistingExperimentService")) decision, err := persistingExperimentService.GetDecision(s.testDecisionContext, testUserContext) s.Equal(s.testComputedDecision, decision) s.NoError(err) @@ -117,8 +114,7 @@ func (s *PersistingExperimentServiceTestSuite) TestSavedVariationNoLongerValid() ExperimentBucketMap: map[UserDecisionKey]string{decisionKey: s.testComputedDecision.Variation.ID}, } s.mockUserProfileService.On("Save", updatedUserProfile) - persistingExperimentService := NewPersistingExperimentService(logging.GetLogger("", "NewPersistingExperimentService"), - s.mockExperimentService, s.mockUserProfileService) + persistingExperimentService := NewPersistingExperimentService(s.mockUserProfileService, s.mockExperimentService, logging.GetLogger("", "NewPersistingExperimentService")) decision, err := persistingExperimentService.GetDecision(s.testDecisionContext, testUserContext) s.Equal(s.testComputedDecision, decision) s.NoError(err) diff --git a/pkg/logging/level_log_consumer.go b/pkg/logging/level_log_consumer.go index 2feaaac94..4b0553795 100644 --- a/pkg/logging/level_log_consumer.go +++ b/pkg/logging/level_log_consumer.go @@ -44,7 +44,7 @@ func (l *FilteredLevelLogConsumer) Log(level LogLevel, message string, fields ma keys = append(keys, k) } sort.Strings(keys) - + for _, k := range keys { if s, ok := fields[k].(string);ok && s != "" { fmt.Fprintf(&messBuilder, "[%s]", s)