Skip to content

FFM-3702 Adding customer logger for evaluation #91

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 23, 2022
Merged

FFM-3702 Adding customer logger for evaluation #91

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
66982c6
FFM-3702 Adding customer logger for evaluation
stephenmcconkey Jun 23, 2022
00dc412
Fix linting issues
stephenmcconkey Jun 23, 2022
95c61a6
Add noop logger to evaluator tests
stephenmcconkey Jun 23, 2022
8b311d8
goimports files
stephenmcconkey Jun 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func NewCfClient(sdkKey string, options ...ConfigOption) (*CfClient, error) {
return nil, err
}
client.repository = repository.New(lruCache)
client.evaluator, err = evaluation.NewEvaluator(client.repository, client)
client.evaluator, err = evaluation.NewEvaluator(client.repository, client, config.Logger)
if err != nil {
return nil, err
}
Expand Down
37 changes: 20 additions & 17 deletions evaluation/evaluator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import (
"strconv"
"strings"

"github.com/harness/ff-golang-server-sdk/log"
"github.com/harness/ff-golang-server-sdk/logger"

"github.com/harness/ff-golang-server-sdk/rest"
)

Expand Down Expand Up @@ -50,14 +51,16 @@ type PostEvaluateCallback interface {
type Evaluator struct {
query Query
postEvalCallback PostEvaluateCallback
logger logger.Logger
}

// NewEvaluator constructs evaluator with query instance
func NewEvaluator(query Query, postEvalCallback PostEvaluateCallback) (*Evaluator, error) {
func NewEvaluator(query Query, postEvalCallback PostEvaluateCallback, logger logger.Logger) (*Evaluator, error) {
if query == nil {
return nil, ErrQueryProviderMissing
}
return &Evaluator{
logger: logger,
query: query,
postEvalCallback: postEvalCallback,
}, nil
Expand Down Expand Up @@ -233,13 +236,13 @@ func (e Evaluator) isTargetIncludedOrExcludedInSegment(segmentList []string, tar
}
// Should Target be excluded - if in excluded list we return false
if segment.Excluded != nil && isTargetInList(target, *segment.Excluded) {
log.Debugf("Target %s excluded from segment %s via exclude list", target.Name, segment.Name)
e.logger.Debugf("Target %s excluded from segment %s via exclude list", target.Name, segment.Name)
return false
}

// Should Target be included - if in included list we return true
if segment.Included != nil && isTargetInList(target, *segment.Included) {
log.Debugf(
e.logger.Debugf(
"Target %s included in segment %s via include list",
target.Name,
segment.Name)
Expand All @@ -249,7 +252,7 @@ func (e Evaluator) isTargetIncludedOrExcludedInSegment(segmentList []string, tar
// Should Target be included via segment rules
rules := segment.Rules
if rules != nil && e.evaluateClauses(*rules, target) {
log.Debugf(
e.logger.Debugf(
"Target %s included in segment %s via rules", target.Name, segment.Name)
return true
}
Expand All @@ -259,32 +262,32 @@ func (e Evaluator) isTargetIncludedOrExcludedInSegment(segmentList []string, tar

func (e Evaluator) checkPreRequisite(fc *rest.FeatureConfig, target *Target) (bool, error) {
if e.query == nil {
log.Errorf(ErrQueryProviderMissing.Error())
e.logger.Errorf(ErrQueryProviderMissing.Error())
return true, ErrQueryProviderMissing
}
prerequisites := fc.Prerequisites
if prerequisites != nil {
log.Infof(
e.logger.Debugf(
"Checking pre requisites %v of parent feature %v",
prerequisites,
fc.Feature)
for _, pre := range *prerequisites {
prereqFeature := pre.Feature
prereqFeatureConfig, err := e.query.GetFlag(prereqFeature)
if err != nil {
log.Errorf(
e.logger.Errorf(
"Could not retrieve the pre requisite details of feature flag : %v", prereqFeature)
return true, nil
}

prereqEvaluatedVariation, err := e.evaluateFlag(prereqFeatureConfig, target)
if err != nil {
log.Errorf(
e.logger.Errorf(
"Could not evaluate the prerequisite details of feature flag : %v", prereqFeature)
return true, nil
}

log.Infof(
e.logger.Debugf(
"Pre requisite flag %v has variation %v for target %v",
prereqFeatureConfig.Feature,
prereqEvaluatedVariation,
Expand All @@ -293,7 +296,7 @@ func (e Evaluator) checkPreRequisite(fc *rest.FeatureConfig, target *Target) (bo
// Compare if the pre requisite variation is a possible valid value of
// the pre requisite FF
validPrereqVariations := pre.Variations
log.Infof(
e.logger.Debugf(
"Pre requisite flag %v should have the variations %v",
prereqFeatureConfig.Feature,
validPrereqVariations)
Expand All @@ -311,7 +314,7 @@ func (e Evaluator) checkPreRequisite(fc *rest.FeatureConfig, target *Target) (bo
func (e Evaluator) evaluate(identifier string, target *Target, kind string) (rest.Variation, error) {

if e.query == nil {
log.Errorf(ErrQueryProviderMissing.Error())
e.logger.Errorf(ErrQueryProviderMissing.Error())
return rest.Variation{}, ErrQueryProviderMissing
}
flag, err := e.query.GetFlag(identifier)
Expand Down Expand Up @@ -349,7 +352,7 @@ func (e Evaluator) evaluate(identifier string, target *Target, kind string) (res
func (e Evaluator) BoolVariation(identifier string, target *Target, defaultValue bool) bool {
variation, err := e.evaluate(identifier, target, "boolean")
if err != nil {
log.Errorf("Error while evaluating boolean flag '%s', err: %v", identifier, err)
e.logger.Errorf("Error while evaluating boolean flag '%s', err: %v", identifier, err)
return defaultValue
}
return strings.ToLower(variation.Value) == "true"
Expand All @@ -360,7 +363,7 @@ func (e Evaluator) StringVariation(identifier string, target *Target, defaultVal

variation, err := e.evaluate(identifier, target, "string")
if err != nil {
log.Errorf("Error while evaluating string flag '%s', err: %v", identifier, err)
e.logger.Errorf("Error while evaluating string flag '%s', err: %v", identifier, err)
return defaultValue
}
return variation.Value
Expand All @@ -371,7 +374,7 @@ func (e Evaluator) IntVariation(identifier string, target *Target, defaultValue

variation, err := e.evaluate(identifier, target, "int")
if err != nil {
log.Errorf("Error while evaluating int flag '%s', err: %v", identifier, err)
e.logger.Errorf("Error while evaluating int flag '%s', err: %v", identifier, err)
return defaultValue
}
val, err := strconv.Atoi(variation.Value)
Expand All @@ -386,7 +389,7 @@ func (e Evaluator) NumberVariation(identifier string, target *Target, defaultVal

variation, err := e.evaluate(identifier, target, "number")
if err != nil {
log.Errorf("Error while evaluating number flag '%s', err: %v", identifier, err)
e.logger.Errorf("Error while evaluating number flag '%s', err: %v", identifier, err)
return defaultValue
}
val, err := strconv.ParseFloat(variation.Value, 64)
Expand All @@ -402,7 +405,7 @@ func (e Evaluator) JSONVariation(identifier string, target *Target,

variation, err := e.evaluate(identifier, target, "json")
if err != nil {
log.Errorf("Error while evaluating json flag '%s', err: %v", identifier, err)
e.logger.Errorf("Error while evaluating json flag '%s', err: %v", identifier, err)
return defaultValue
}
val := make(map[string]interface{})
Expand Down
49 changes: 33 additions & 16 deletions evaluation/evaluator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"reflect"
"testing"

"github.com/harness/ff-golang-server-sdk/logger"

"github.com/harness/ff-golang-server-sdk/rest"
)

Expand Down Expand Up @@ -303,9 +305,11 @@ func (m TestRepository) GetFlag(identifier string) (rest.FeatureConfig, error) {
}

func TestNewEvaluator(t *testing.T) {
eval, _ := NewEvaluator(testRepo, nil)
noOpLogger := logger.NewNoOpLogger()
eval, _ := NewEvaluator(testRepo, nil, noOpLogger)
type args struct {
query Query
query Query
logger logger.Logger
}
tests := []struct {
name string
Expand All @@ -322,15 +326,16 @@ func TestNewEvaluator(t *testing.T) {
{
name: "should return test repo",
args: args{
query: testRepo,
query: testRepo,
logger: noOpLogger,
},
want: eval,
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := NewEvaluator(tt.args.query, nil)
got, err := NewEvaluator(tt.args.query, nil, noOpLogger)
if (err != nil) != tt.wantErr {
t.Errorf("NewEvaluator() error = %v, wantErr %v", err, tt.wantErr)
return
Expand Down Expand Up @@ -616,7 +621,8 @@ func TestEvaluator_evaluateClause(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
e := Evaluator{
query: tt.fields.query,
query: tt.fields.query,
logger: logger.NewNoOpLogger(),
}
if got := e.evaluateClause(tt.args.clause, tt.args.target); got != tt.want {
t.Errorf("Evaluator.evaluateClause() = %v, want %v", got, tt.want)
Expand Down Expand Up @@ -797,7 +803,8 @@ func TestEvaluator_evaluateRules(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
e := Evaluator{
query: tt.fields.query,
query: tt.fields.query,
logger: logger.NewNoOpLogger(),
}
if got := e.evaluateRules(tt.args.servingRules, tt.args.target); got != tt.want {
t.Errorf("Evaluator.evaluateRules() = %v, want %v", got, tt.want)
Expand Down Expand Up @@ -915,7 +922,8 @@ func TestEvaluator_evaluateVariationMap(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
e := Evaluator{
query: tt.fields.query,
query: tt.fields.query,
logger: logger.NewNoOpLogger(),
}
if got := e.evaluateVariationMap(tt.args.variationsMap, tt.args.target); got != tt.want {
t.Errorf("Evaluator.evaluateVariationMap() = %v, want %v", got, tt.want)
Expand Down Expand Up @@ -1080,7 +1088,8 @@ func TestEvaluator_evaluateFlag(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
e := Evaluator{
query: tt.fields.query,
query: tt.fields.query,
logger: logger.NewNoOpLogger(),
}
got, err := e.evaluateFlag(tt.args.fc, tt.args.target)
if (err != nil) != tt.wantErr {
Expand Down Expand Up @@ -1181,7 +1190,8 @@ func TestEvaluator_isTargetIncludedOrExcludedInSegment(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
e := Evaluator{
query: tt.fields.query,
query: tt.fields.query,
logger: logger.NewNoOpLogger(),
}
if got := e.isTargetIncludedOrExcludedInSegment(tt.args.segmentList, tt.args.target); got != tt.want {
t.Errorf("Evaluator.isTargetIncludedOrExcludedInSegment() = %v, want %v", got, tt.want)
Expand Down Expand Up @@ -1263,7 +1273,8 @@ func TestEvaluator_checkPreRequisite(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
e := Evaluator{
query: tt.fields.query,
query: tt.fields.query,
logger: logger.NewNoOpLogger(),
}
got, err := e.checkPreRequisite(tt.args.parent, tt.args.target)
if (err != nil) != tt.wantErr {
Expand Down Expand Up @@ -1380,7 +1391,8 @@ func TestEvaluator_evaluate(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
e := Evaluator{
query: tt.fields.query,
query: tt.fields.query,
logger: logger.NewNoOpLogger(),
}
got, err := e.evaluate(tt.args.identifier, tt.args.target, tt.args.kind)
if (err != nil) != tt.wantErr {
Expand Down Expand Up @@ -1451,7 +1463,8 @@ func TestEvaluator_BoolVariation(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
e := Evaluator{
query: tt.fields.query,
query: tt.fields.query,
logger: logger.NewNoOpLogger(),
}
if got := e.BoolVariation(tt.args.identifier, tt.args.target, tt.args.defaultValue); got != tt.want {
t.Errorf("Evaluator.BoolVariation() = %v, want %v", got, tt.want)
Expand Down Expand Up @@ -1517,7 +1530,8 @@ func TestEvaluator_StringVariation(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
e := Evaluator{
query: tt.fields.query,
query: tt.fields.query,
logger: logger.NewNoOpLogger(),
}
if got := e.StringVariation(tt.args.identifier, tt.args.target, tt.args.defaultValue); got != tt.want {
t.Errorf("Evaluator.StringVariation() = %v, want %v", got, tt.want)
Expand Down Expand Up @@ -1595,7 +1609,8 @@ func TestEvaluator_IntVariation(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
e := Evaluator{
query: tt.fields.query,
query: tt.fields.query,
logger: logger.NewNoOpLogger(),
}
if got := e.IntVariation(tt.args.identifier, tt.args.target, tt.args.defaultValue); got != tt.want {
t.Errorf("Evaluator.IntVariation() = %v, want %v", got, tt.want)
Expand Down Expand Up @@ -1673,7 +1688,8 @@ func TestEvaluator_NumberVariation(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
e := Evaluator{
query: tt.fields.query,
query: tt.fields.query,
logger: logger.NewNoOpLogger(),
}
if got := e.NumberVariation(tt.args.identifier, tt.args.target, tt.args.defaultValue); got != tt.want {
t.Errorf("Evaluator.NumberVariation() = %v, want %v", got, tt.want)
Expand Down Expand Up @@ -1758,7 +1774,8 @@ func TestEvaluator_JSONVariation(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
e := Evaluator{
query: tt.fields.query,
query: tt.fields.query,
logger: logger.NewNoOpLogger(),
}
if got := e.JSONVariation(tt.args.identifier, tt.args.target, tt.args.defaultValue); !reflect.DeepEqual(got, tt.want) {
t.Errorf("Evaluator.JSONVariation() = %v, want %v", got, tt.want)
Expand Down
46 changes: 46 additions & 0 deletions logger/nooplogger.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package logger

// NoOpLogger is a type that implements the Logger interface but does nothing
// when it's methods are called
type NoOpLogger struct{}

// NewNoOpLogger returns a NoOpLogger
func NewNoOpLogger() NoOpLogger {
return NoOpLogger{}
}

// Debug does nothing on a NoOpLogger
func (m NoOpLogger) Debug(args ...interface{}) {}

// Debugf does nothing on a NoOpLogger
func (m NoOpLogger) Debugf(template string, args ...interface{}) {}

// Info does nothing on a NoOpLogger
func (m NoOpLogger) Info(args ...interface{}) {}

// Infof does nothing on a NoOpLogger
func (m NoOpLogger) Infof(template string, args ...interface{}) {}

// Warn does nothing on a NoOpLogger
func (m NoOpLogger) Warn(args ...interface{}) {}

// Warnf does nothing on a NoOpLogger
func (m NoOpLogger) Warnf(template string, args ...interface{}) {}

// Error does nothing on a NoOpLogger
func (m NoOpLogger) Error(args ...interface{}) {}

// Errorf does nothing on a NoOpLogger
func (m NoOpLogger) Errorf(template string, args ...interface{}) {}

// Panic does nothing on a NoOpLogger
func (m NoOpLogger) Panic(args ...interface{}) {}

// Panicf does nothing on a NoOpLogger
func (m NoOpLogger) Panicf(template string, args ...interface{}) {}

// Fatal does nothing on a NoOpLogger
func (m NoOpLogger) Fatal(args ...interface{}) {}

// Fatalf does nothing on a NoOpLogger
func (m NoOpLogger) Fatalf(template string, args ...interface{}) {}
4 changes: 3 additions & 1 deletion tests/evaluator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"reflect"
"testing"

"github.com/harness/ff-golang-server-sdk/logger"

"github.com/harness/ff-golang-server-sdk/evaluation"

"github.com/harness/ff-golang-server-sdk/log"
Expand Down Expand Up @@ -77,7 +79,7 @@ func TestEvaluator(t *testing.T) {
t.Error(err)
}
repo := repository.New(lruCache)
evaluator, err := evaluation.NewEvaluator(repo, nil)
evaluator, err := evaluation.NewEvaluator(repo, nil, logger.NewNoOpLogger())
if err != nil {
t.Error(err)
}
Expand Down