Skip to content

Commit

Permalink
Merge pull request #91 from harness/FFM-3702
Browse files Browse the repository at this point in the history 
FFM-3702 Adding customer logger for evaluation
  • Loading branch information
@stephenmcconkey
stephenmcconkey committed Jun 23, 2022
2 parents 82779d3 + 8b311d8 commit b3128e7
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 35 deletions.
2 changes: 1 addition & 1 deletion client/client.go
View file
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
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
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
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
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

0 comments on commit b3128e7

Please sign in to comment.