bug: [fix performance test not working with multiple packages] (FF-3110) (#68)

leoromanovsky · web-flow · commit 07f878fe2df0 · 2024-09-06T17:26:20.000-07:00
* bug: [remove applicationlogger package; flatten into 1] (FF-3110)

* upgrade github actions

* Possibly fix paths

* run test 5 times

* simplify github

* switch order
diff --git a/.github/workflows/memory-profile-compare.yml b/.github/workflows/memory-profile-compare.yml
@@ -6,65 +6,41 @@ jobs:
   compare-memory-profiles:
     runs-on: ubuntu-latest
     steps:
-    - name: Checkout main branch
-      uses: actions/checkout@v2
+    - name: Checkout code
+      uses: actions/checkout@v3
       with:
-        ref: 'main'
-    
-    - name: Set up Go (main branch)
-      uses: actions/setup-go@v2
-      with:
-        go-version: '1.19'
-    
-    - name: Generate memory profile for main branch
-      run: make profile-memory OUTFILE_SUFFIX=.main
-      
-    - name: Save main branch memory profile
-      uses: actions/upload-artifact@v2
-      with:
-        name: memprofile-main
-        path: | 
-          memprofile.main.out
-          memprofile.main.text 
-    
-    - name: Checkout feature branch
-      uses: actions/checkout@v2
-      with:
-        ref: ${{ github.head_ref }}
-    
-    - name: Set up Go (feature branch)
-      uses: actions/setup-go@v2
-      with:
-        go-version: '1.19'
-    
-    - name: Generate memory profile for feature branch
-      run: make profile-memory OUTFILE_SUFFIX=.feat
-      
-    - name: Save feat branch memory profile
-      uses: actions/upload-artifact@v2
-      with:
-        name: memprofile-feat
-        path: | 
-          memprofile.feat.out
-          memprofile.feat.text 
+        fetch-depth: 0
 
-    - name: Download main branch memory profile
-      uses: actions/download-artifact@v2
+    - name: Set up Go
+      uses: actions/setup-go@v4
       with:
-        name: memprofile-main
-        path: .
-    
-    - name: Display structure of downloaded files
-      run: ls -R
-    
+        go-version: '1.21'
+
+    - name: Generate memory profiles
+      run: |
+        git checkout ${{ github.head_ref }}
+        make profile-memory OUTFILE_SUFFIX=.feat || { echo "Failed to generate feature profile"; ls -l; cat Makefile; exit 1; }
+        git checkout main
+        make profile-memory OUTFILE_SUFFIX=.main || { echo "Failed to generate main profile"; ls -l; cat Makefile; exit 1; }
+
     - name: Compare memory profiles
-      run: make profile-memory-compare BASE_FILE=memprofile.main.out FEAT_FILE=memprofile.feat.out > memory-profile-comparison.text
-    
-    - name: Show memory profile comparison
-      run: cat memory-profile-comparison.text
+      run: |
+        ls -l
+        if [ ! -f memprofile.main.out ] || [ ! -f memprofile.feat.out ]; then
+          echo "Error: One or both memory profile files are missing."
+          exit 1
+        fi
+        make profile-memory-compare BASE_FILE=memprofile.main.out FEAT_FILE=memprofile.feat.out > memory-profile-comparison.text || { echo "Failed to compare profiles"; cat Makefile; exit 1; }
+        echo "Memory Profile Comparison:"
+        cat memory-profile-comparison.text
 
-    - name: Upload comparison result
-      uses: actions/upload-artifact@v2
+    - name: Upload artifacts
+      uses: actions/upload-artifact@v3
       with:
-        name: memory-profile-comparison
-        path: memory-profile-comparison.text
+        name: memory-profiles
+        path: |
+          memprofile.main.out
+          memprofile.main.text
+          memprofile.feat.out
+          memprofile.feat.text
+          memory-profile-comparison.text
diff --git a/Makefile b/Makefile
@@ -29,12 +29,9 @@ lint:
 
 ## profile-memory - Run test and generate memory profile
 profile-memory: test-data
-	@cd eppoclient && \
-	{ \
-	echo "Using OUTFILE_SUFFIX: $$OUTFILE_SUFFIX"; \
-	go test -run Test_e2e -memprofile ../memprofile$$OUTFILE_SUFFIX.out ./...; \
-	go tool pprof -text -nodecount=50 ../memprofile$$OUTFILE_SUFFIX.out > ../memprofile$$OUTFILE_SUFFIX.text; \
-	}
+	@echo "Using OUTFILE_SUFFIX: $(OUTFILE_SUFFIX)"
+	go test -v ./... -run=Test_e2e -count=5 -memprofile=memprofile$(OUTFILE_SUFFIX).out
+	go tool pprof -text memprofile$(OUTFILE_SUFFIX).out > memprofile$(OUTFILE_SUFFIX).text
 
 ## profile-memory-compare - Compare two memory profiles
 ## example: make profile-memory-compare BASE_FILE=memprofile1.out FEAT_FILE=memprofile2.out
diff --git a/doc.go b/doc.go
diff --git a/eppoclient/applicationlogger.go b/eppoclient/applicationlogger.go
@@ -1,14 +1,64 @@
-package applicationlogger
+package eppoclient
 
 import (
 	"regexp"
+
+	"go.uber.org/zap"
 )
 
+type ApplicationLogger interface {
+	Debug(args ...interface{})
+	Info(args ...interface{})
+	Infof(template string, args ...interface{})
+	Warn(args ...interface{})
+	Warnf(template string, args ...interface{})
+	Error(args ...interface{})
+	Errorf(template string, args ...interface{})
+}
+
+// ZapLogger The default logger for the Eppo SDK
+type ZapLogger struct {
+	logger *zap.Logger
+}
+
+func NewZapLogger(logger *zap.Logger) *ZapLogger {
+	return &ZapLogger{logger: logger}
+}
+
+func (z *ZapLogger) Debug(args ...interface{}) {
+	z.logger.Sugar().Debug(args...)
+}
+
+func (z *ZapLogger) Info(args ...interface{}) {
+	z.logger.Sugar().Info(args...)
+}
+
+func (z *ZapLogger) Infof(template string, args ...interface{}) {
+	z.logger.Sugar().Infof(template, args...)
+}
+
+func (z *ZapLogger) Warn(args ...interface{}) {
+	z.logger.Sugar().Warn(args...)
+}
+
+func (z *ZapLogger) Warnf(template string, args ...interface{}) {
+	z.logger.Sugar().Warnf(template, args...)
+}
+
+func (z *ZapLogger) Error(args ...interface{}) {
+	z.logger.Sugar().Error(args...)
+}
+
+func (z *ZapLogger) Errorf(template string, args ...interface{}) {
+	z.logger.Sugar().Errorf(template, args...)
+}
+
+// ScrubbingLogger is an ApplicationLogger that scrubs sensitive information from logs
 type ScrubbingLogger struct {
-	innerLogger Logger
+	innerLogger ApplicationLogger
 }
 
-func NewScrubbingLogger(innerLogger Logger) *ScrubbingLogger {
+func NewScrubbingLogger(innerLogger ApplicationLogger) *ScrubbingLogger {
 	return &ScrubbingLogger{innerLogger: innerLogger}
 }
 
diff --git a/eppoclient/applicationlogger/interface.go b/eppoclient/applicationlogger/interface.go
diff --git a/eppoclient/applicationlogger/zap_logger.go b/eppoclient/applicationlogger/zap_logger.go
diff --git a/eppoclient/client.go b/eppoclient/client.go
@@ -3,8 +3,6 @@ package eppoclient
 import (
 	"fmt"
 	"time"
-
-	"github.com/Eppo-exp/golang-sdk/v5/eppoclient/applicationlogger"
 )
 
 type Attributes map[string]interface{}
@@ -16,15 +14,15 @@ type EppoClient struct {
 	configRequestor    *configurationRequestor
 	poller             *poller
 	logger             IAssignmentLogger
-	applicationLogger  applicationlogger.Logger
+	applicationLogger  ApplicationLogger
 }
 
 func newEppoClient(
 	configurationStore *configurationStore,
 	configRequestor *configurationRequestor,
 	poller *poller,
 	assignmentLogger IAssignmentLogger,
-	applicationLogger applicationlogger.Logger,
+	applicationLogger ApplicationLogger,
 ) *EppoClient {
 	return &EppoClient{
 		configurationStore: configurationStore,
diff --git a/eppoclient/client_test.go b/eppoclient/client_test.go
@@ -6,13 +6,11 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"go.uber.org/zap"
-
-	"github.com/Eppo-exp/golang-sdk/v5/eppoclient/applicationlogger"
 )
 
 var (
 	zapLogger, _      = zap.NewDevelopment()
-	applicationLogger = applicationlogger.NewZapLogger(zapLogger)
+	applicationLogger = NewZapLogger(zapLogger)
 )
 
 func Test_AssignBlankExperiment(t *testing.T) {
diff --git a/eppoclient/config.go b/eppoclient/config.go
@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/Eppo-exp/golang-sdk/v5/eppoclient/applicationlogger"
 	"go.uber.org/zap"
 )
 
@@ -16,7 +15,7 @@ type Config struct {
 	SdkKey            string
 	AssignmentLogger  IAssignmentLogger
 	PollerInterval    time.Duration
-	ApplicationLogger applicationlogger.Logger
+	ApplicationLogger ApplicationLogger
 }
 
 func (cfg *Config) validate() error {
@@ -37,7 +36,7 @@ func (cfg *Config) validate() error {
 		if err != nil {
 			return fmt.Errorf("failed to create default logger: %v", err)
 		}
-		cfg.ApplicationLogger = applicationlogger.NewZapLogger(defaultLogger)
+		cfg.ApplicationLogger = NewZapLogger(defaultLogger)
 	}
 
 	return nil
diff --git a/eppoclient/configurationrequestor.go b/eppoclient/configurationrequestor.go
@@ -2,8 +2,6 @@ package eppoclient
 
 import (
 	"encoding/json"
-
-	"github.com/Eppo-exp/golang-sdk/v5/eppoclient/applicationlogger"
 )
 
 const CONFIG_ENDPOINT = "/flag-config/v1/config"
@@ -12,10 +10,10 @@ const BANDIT_ENDPOINT = "/flag-config/v1/bandits"
 type configurationRequestor struct {
 	httpClient        httpClient
 	configStore       *configurationStore
-	applicationLogger applicationlogger.Logger
+	applicationLogger ApplicationLogger
 }
 
-func newConfigurationRequestor(httpClient httpClient, configStore *configurationStore, applicationLogger applicationlogger.Logger) *configurationRequestor {
+func newConfigurationRequestor(httpClient httpClient, configStore *configurationStore, applicationLogger ApplicationLogger) *configurationRequestor {
 	return &configurationRequestor{
 		httpClient:        httpClient,
 		configStore:       configStore,
diff --git a/eppoclient/evalflags.go b/eppoclient/evalflags.go
@@ -3,8 +3,6 @@ package eppoclient
 import (
 	"fmt"
 	"time"
-
-	"github.com/Eppo-exp/golang-sdk/v5/eppoclient/applicationlogger"
 )
 
 func (flag flagConfiguration) verifyType(ty variationType) error {
@@ -15,7 +13,7 @@ func (flag flagConfiguration) verifyType(ty variationType) error {
 	}
 }
 
-func (flag flagConfiguration) eval(subjectKey string, subjectAttributes Attributes, applicationLogger applicationlogger.Logger) (interface{}, *AssignmentEvent, error) {
+func (flag flagConfiguration) eval(subjectKey string, subjectAttributes Attributes, applicationLogger ApplicationLogger) (interface{}, *AssignmentEvent, error) {
 	if !flag.Enabled {
 		return nil, nil, ErrFlagNotEnabled
 	}
@@ -86,7 +84,7 @@ func augmentWithSubjectKey(subjectAttributes Attributes, subjectKey string) Attr
 	return augmentedSubjectAttributes
 }
 
-func (allocation allocation) findMatchingSplit(subjectKey string, augmentedSubjectAttributes Attributes, totalShards int64, now time.Time, applicationLogger applicationlogger.Logger) *split {
+func (allocation allocation) findMatchingSplit(subjectKey string, augmentedSubjectAttributes Attributes, totalShards int64, now time.Time, applicationLogger ApplicationLogger) *split {
 	if !allocation.StartAt.IsZero() && now.Before(allocation.StartAt) {
 		return nil
 	}
diff --git a/eppoclient/poller.go b/eppoclient/poller.go
@@ -2,18 +2,16 @@ package eppoclient
 
 import (
 	"time"
-
-	"github.com/Eppo-exp/golang-sdk/v5/eppoclient/applicationlogger"
 )
 
 type poller struct {
 	interval          time.Duration
 	callback          func()
 	isStopped         bool `default:"false"`
-	applicationLogger applicationlogger.Logger
+	applicationLogger ApplicationLogger
 }
 
-func newPoller(interval time.Duration, callback func(), applicationLogger applicationlogger.Logger) *poller {
+func newPoller(interval time.Duration, callback func(), applicationLogger ApplicationLogger) *poller {
 	var pl = &poller{}
 
 	pl.interval = interval
diff --git a/eppoclient/rules.go b/eppoclient/rules.go
@@ -7,11 +7,10 @@ import (
 	"regexp"
 	"strconv"
 
-	"github.com/Eppo-exp/golang-sdk/v5/eppoclient/applicationlogger"
 	semver "github.com/Masterminds/semver/v3"
 )
 
-func (rule rule) matches(subjectAttributes Attributes, applicationLogger ...applicationlogger.Logger) bool {
+func (rule rule) matches(subjectAttributes Attributes, applicationLogger ...ApplicationLogger) bool {
 	for _, condition := range rule.Conditions {
 		if !condition.matches(subjectAttributes, applicationLogger...) {
 			return false
@@ -21,7 +20,7 @@ func (rule rule) matches(subjectAttributes Attributes, applicationLogger ...appl
 	return true
 }
 
-func (condition condition) matches(subjectAttributes Attributes, applicationLogger ...applicationlogger.Logger) bool {
+func (condition condition) matches(subjectAttributes Attributes, applicationLogger ...ApplicationLogger) bool {
 	subjectValue, exists := subjectAttributes[condition.Attribute]
 	if condition.Operator == "IS_NULL" {
 		isNull := !exists || subjectValue == nil
diff --git a/eppoclient/scrubbing_logger_test.go b/eppoclient/scrubbing_logger_test.go

Original file line number	Diff line number	Diff line change
`@@ -6,13 +6,11 @@ import (`
`6`	`6`	`"github.com/stretchr/testify/assert"`
`7`	`7`	`"github.com/stretchr/testify/mock"`
`8`	`8`	`"go.uber.org/zap"`
`9`		`-`
`10`		`- "github.com/Eppo-exp/golang-sdk/v5/eppoclient/applicationlogger"`
`11`	`9`	`)`
`12`	`10`
`13`	`11`	`var (`
`14`	`12`	`zapLogger, _ = zap.NewDevelopment()`
`15`		`- applicationLogger = applicationlogger.NewZapLogger(zapLogger)`
	`13`	`+ applicationLogger = NewZapLogger(zapLogger)`
`16`	`14`	`)`
`17`	`15`
`18`	`16`	`func Test_AssignBlankExperiment(t *testing.T) {`
Original file line number	Diff line number	Diff line change
`@@ -4,7 +4,6 @@ import (`
`4`	`4`	`"fmt"`
`5`	`5`	`"time"`
`6`	`6`
`7`		`- "github.com/Eppo-exp/golang-sdk/v5/eppoclient/applicationlogger"`
`8`	`7`	`"go.uber.org/zap"`
`9`	`8`	`)`
`10`	`9`
`@@ -16,7 +15,7 @@ type Config struct {`
`16`	`15`	`SdkKey string`
`17`	`16`	`AssignmentLogger IAssignmentLogger`
`18`	`17`	`PollerInterval time.Duration`
`19`		`- ApplicationLogger applicationlogger.Logger`
	`18`	`+ ApplicationLogger ApplicationLogger`
`20`	`19`	`}`
`21`	`20`
`22`	`21`	`func (cfg *Config) validate() error {`
`@@ -37,7 +36,7 @@ func (cfg *Config) validate() error {`
`37`	`36`	`if err != nil {`
`38`	`37`	`return fmt.Errorf("failed to create default logger: %v", err)`
`39`	`38`	`}`
`40`		`- cfg.ApplicationLogger = applicationlogger.NewZapLogger(defaultLogger)`
	`39`	`+ cfg.ApplicationLogger = NewZapLogger(defaultLogger)`
`41`	`40`	`}`
`42`	`41`
`43`	`42`	`return nil`