From 6757e727f7b6ee3e100e6c44893c3cea98968ba5 Mon Sep 17 00:00:00 2001 From: Yasir Ali Date: Fri, 10 Jul 2020 18:07:02 +0500 Subject: [PATCH 1/6] Initial commit. --- .../mappers/condition_trees.go | 1 + pkg/decision/evaluator/audience.go | 45 ------------------- pkg/decision/evaluator/condition.go | 32 ++++++++++--- pkg/decision/evaluator/condition_test.go | 5 ++- pkg/decision/evaluator/condition_tree.go | 10 +++-- pkg/decision/evaluator/condition_tree_test.go | 15 ++++--- pkg/decision/evaluator/matchers/exact.go | 10 +++++ pkg/decision/evaluator/matchers/exact_test.go | 5 +++ pkg/decision/evaluator/matchers/gt.go | 9 ++++ pkg/decision/evaluator/matchers/gt_test.go | 3 ++ pkg/decision/evaluator/matchers/lt.go | 9 ++++ pkg/decision/evaluator/matchers/lt_test.go | 3 ++ pkg/decision/evaluator/matchers/substring.go | 9 ++++ .../evaluator/matchers/substring_test.go | 2 + pkg/decision/experiment_bucketer_service.go | 9 +++- .../experiment_bucketer_service_test.go | 30 ++++++++++++- pkg/decision/rollout_service.go | 27 ++++++++--- pkg/entities/audience.go | 9 ++-- 18 files changed, 156 insertions(+), 77 deletions(-) delete mode 100644 pkg/decision/evaluator/audience.go diff --git a/pkg/config/datafileprojectconfig/mappers/condition_trees.go b/pkg/config/datafileprojectconfig/mappers/condition_trees.go index 1f242d82a..cf9c72f97 100644 --- a/pkg/config/datafileprojectconfig/mappers/condition_trees.go +++ b/pkg/config/datafileprojectconfig/mappers/condition_trees.go @@ -127,6 +127,7 @@ func createLeafCondition(typedV map[string]interface{}, node *entities.TreeNode) if err := json.Unmarshal(jsonBody, &condition); err != nil { return err } + condition.StringRepresentation = string(jsonBody) node.Item = condition return nil } diff --git a/pkg/decision/evaluator/audience.go b/pkg/decision/evaluator/audience.go deleted file mode 100644 index c5244a149..000000000 --- a/pkg/decision/evaluator/audience.go +++ /dev/null @@ -1,45 +0,0 @@ -/**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * - * * - * Licensed under the Apache License, Version 2.0 (the "License"); * - * you may not use this file except in compliance with the License. * - * You may obtain a copy of the License at * - * * - * http://www.apache.org/licenses/LICENSE-2.0 * - * * - * Unless required by applicable law or agreed to in writing, software * - * distributed under the License is distributed on an "AS IS" BASIS, * - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * - * See the License for the specific language governing permissions and * - * limitations under the License. * - ***************************************************************************/ - -// Package evaluator // -package evaluator - -import ( - "github.com/optimizely/go-sdk/pkg/entities" -) - -// AudienceEvaluator evaluates an audience against the given user's attributes -type AudienceEvaluator interface { - Evaluate(audience entities.Audience, condTreeParams *entities.TreeParameters) bool -} - -// TypedAudienceEvaluator evaluates typed audiences -type TypedAudienceEvaluator struct { - conditionTreeEvaluator TreeEvaluator -} - -// NewTypedAudienceEvaluator creates a new instance of the TypedAudienceEvaluator -func NewTypedAudienceEvaluator() *TypedAudienceEvaluator { - conditionTreeEvaluator := NewMixedTreeEvaluator() - return &TypedAudienceEvaluator{ - conditionTreeEvaluator: *conditionTreeEvaluator, - } -} - -// Evaluate evaluates the typed audience against the given user's attributes -func (a TypedAudienceEvaluator) Evaluate(audience entities.Audience, condTreeParams *entities.TreeParameters) (evalResult, isValid bool) { - return a.conditionTreeEvaluator.Evaluate(audience.ConditionTree, condTreeParams) -} diff --git a/pkg/decision/evaluator/condition.go b/pkg/decision/evaluator/condition.go index ef8da5d09..5bb960230 100644 --- a/pkg/decision/evaluator/condition.go +++ b/pkg/decision/evaluator/condition.go @@ -22,6 +22,7 @@ import ( "github.com/optimizely/go-sdk/pkg/decision/evaluator/matchers" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" ) const ( @@ -38,14 +39,22 @@ type ItemEvaluator interface { } // CustomAttributeConditionEvaluator evaluates conditions with custom attributes -type CustomAttributeConditionEvaluator struct{} +type CustomAttributeConditionEvaluator struct { + logger logging.OptimizelyLogProducer +} + +// NewCustomAttributeConditionEvaluator creates a custom attribute condition +func NewCustomAttributeConditionEvaluator(logger logging.OptimizelyLogProducer) *CustomAttributeConditionEvaluator { + return &CustomAttributeConditionEvaluator{logger: logger} +} // Evaluate returns true if the given user's attributes match the condition func (c CustomAttributeConditionEvaluator) Evaluate(condition entities.Condition, condTreeParams *entities.TreeParameters) (bool, error) { // We should only be evaluating custom attributes if condition.Type != customAttributeType { - return false, fmt.Errorf(`unable to evaluator condition of type "%s"`, condition.Type) + c.logger.Warning(fmt.Sprintf(`Audience condition "%s" uses an unknown condition type. You may need to upgrade to a newer release of the Optimizely SDK.`, condition.StringRepresentation)) + return false, fmt.Errorf(`unable to evaluate condition of type "%s"`, condition.Type) } var matcher matchers.Matcher @@ -57,6 +66,7 @@ func (c CustomAttributeConditionEvaluator) Evaluate(condition entities.Condition case exactMatchType: matcher = matchers.ExactMatcher{ Condition: condition, + Logger: c.logger, } case existsMatchType: matcher = matchers.ExistsMatcher{ @@ -65,16 +75,20 @@ func (c CustomAttributeConditionEvaluator) Evaluate(condition entities.Condition case ltMatchType: matcher = matchers.LtMatcher{ Condition: condition, + Logger: c.logger, } case gtMatchType: matcher = matchers.GtMatcher{ Condition: condition, + Logger: c.logger, } case substringMatchType: matcher = matchers.SubstringMatcher{ Condition: condition, + Logger: c.logger, } default: + c.logger.Warning(fmt.Sprintf(`Audience condition "%s" uses an unknown match type. You may need to upgrade to a newer release of the Optimizely SDK.`, condition.StringRepresentation)) return false, fmt.Errorf(`invalid Condition matcher "%s"`, condition.Match) } @@ -84,20 +98,28 @@ func (c CustomAttributeConditionEvaluator) Evaluate(condition entities.Condition } // AudienceConditionEvaluator evaluates conditions with audience condition -type AudienceConditionEvaluator struct{} +type AudienceConditionEvaluator struct { + logger logging.OptimizelyLogProducer +} + +// NewAudienceConditionEvaluator creates a audience condition evaluator +func NewAudienceConditionEvaluator(logger logging.OptimizelyLogProducer) *AudienceConditionEvaluator { + return &AudienceConditionEvaluator{logger: logger} +} // Evaluate returns true if the given user's attributes match the condition func (c AudienceConditionEvaluator) Evaluate(audienceID string, condTreeParams *entities.TreeParameters) (bool, error) { if audience, ok := condTreeParams.AudienceMap[audienceID]; ok { + c.logger.Debug(fmt.Sprintf(`Starting to evaluate audience "%s".`, audienceID)) condTree := audience.ConditionTree - conditionTreeEvaluator := NewMixedTreeEvaluator() + conditionTreeEvaluator := NewMixedTreeEvaluator(c.logger) retValue, isValid := conditionTreeEvaluator.Evaluate(condTree, condTreeParams) if !isValid { return false, fmt.Errorf(`an error occurred while evaluating nested tree for audience ID "%s"`, audienceID) } + c.logger.Debug(fmt.Sprintf(`Audience "%s" evaluated to %t.`, audienceID, retValue)) return retValue, nil - } return false, fmt.Errorf(`unable to evaluate nested tree for audience ID "%s"`, audienceID) diff --git a/pkg/decision/evaluator/condition_test.go b/pkg/decision/evaluator/condition_test.go index 5db738a7e..6749ad2b1 100644 --- a/pkg/decision/evaluator/condition_test.go +++ b/pkg/decision/evaluator/condition_test.go @@ -20,11 +20,12 @@ import ( "testing" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" "github.com/stretchr/testify/assert" ) func TestCustomAttributeConditionEvaluator(t *testing.T) { - conditionEvaluator := CustomAttributeConditionEvaluator{} + conditionEvaluator := NewCustomAttributeConditionEvaluator(logging.GetLogger("", "")) condition := entities.Condition{ Match: "exact", Value: "foo", @@ -54,7 +55,7 @@ func TestCustomAttributeConditionEvaluator(t *testing.T) { } func TestCustomAttributeConditionEvaluatorWithoutMatchType(t *testing.T) { - conditionEvaluator := CustomAttributeConditionEvaluator{} + conditionEvaluator := NewCustomAttributeConditionEvaluator(logging.GetLogger("", "")) condition := entities.Condition{ Value: "foo", Name: "string_foo", diff --git a/pkg/decision/evaluator/condition_tree.go b/pkg/decision/evaluator/condition_tree.go index 6022d388c..25a26484f 100644 --- a/pkg/decision/evaluator/condition_tree.go +++ b/pkg/decision/evaluator/condition_tree.go @@ -21,6 +21,7 @@ import ( "fmt" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" ) const customAttributeType = "custom_attribute" @@ -41,11 +42,12 @@ type TreeEvaluator interface { // MixedTreeEvaluator evaluates a tree of mixed node types (condition node or audience nodes) type MixedTreeEvaluator struct { + logger logging.OptimizelyLogProducer } // NewMixedTreeEvaluator creates a condition tree evaluator with the out-of-the-box condition evaluators -func NewMixedTreeEvaluator() *MixedTreeEvaluator { - return &MixedTreeEvaluator{} +func NewMixedTreeEvaluator(logger logging.OptimizelyLogProducer) *MixedTreeEvaluator { + return &MixedTreeEvaluator{logger: logger} } // Evaluate returns whether the userAttributes satisfy the given condition tree and the evaluation of the condition is valid or not (to handle null bubbling) @@ -66,10 +68,10 @@ func (c MixedTreeEvaluator) Evaluate(node *entities.TreeNode, condTreeParams *en var err error switch v := node.Item.(type) { case entities.Condition: - evaluator := CustomAttributeConditionEvaluator{} + evaluator := NewCustomAttributeConditionEvaluator(c.logger) result, err = evaluator.Evaluate(node.Item.(entities.Condition), condTreeParams) case string: - evaluator := AudienceConditionEvaluator{} + evaluator := NewAudienceConditionEvaluator(c.logger) result, err = evaluator.Evaluate(node.Item.(string), condTreeParams) default: fmt.Printf("I don't know about type %T!\n", v) diff --git a/pkg/decision/evaluator/condition_tree_test.go b/pkg/decision/evaluator/condition_tree_test.go index dd9b74ca2..f7209cb8e 100644 --- a/pkg/decision/evaluator/condition_tree_test.go +++ b/pkg/decision/evaluator/condition_tree_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/assert" e "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" ) var stringFooCondition = e.Condition{ @@ -30,7 +31,7 @@ var int42Condition = e.Condition{ } func TestConditionTreeEvaluateSimpleCondition(t *testing.T) { - conditionTreeEvaluator := NewMixedTreeEvaluator() + conditionTreeEvaluator := NewMixedTreeEvaluator(logging.GetLogger("", "")) conditionTree := &e.TreeNode{ Operator: "or", Nodes: []*e.TreeNode{ @@ -61,7 +62,7 @@ func TestConditionTreeEvaluateSimpleCondition(t *testing.T) { } func TestConditionTreeEvaluateMultipleOrConditions(t *testing.T) { - conditionTreeEvaluator := NewMixedTreeEvaluator() + conditionTreeEvaluator := NewMixedTreeEvaluator(logging.GetLogger("", "")) conditionTree := &e.TreeNode{ Operator: "or", Nodes: []*e.TreeNode{ @@ -116,7 +117,7 @@ func TestConditionTreeEvaluateMultipleOrConditions(t *testing.T) { } func TestConditionTreeEvaluateMultipleAndConditions(t *testing.T) { - conditionTreeEvaluator := NewMixedTreeEvaluator() + conditionTreeEvaluator := NewMixedTreeEvaluator(logging.GetLogger("", "")) conditionTree := &e.TreeNode{ Operator: "and", Nodes: []*e.TreeNode{ @@ -171,7 +172,7 @@ func TestConditionTreeEvaluateMultipleAndConditions(t *testing.T) { } func TestConditionTreeEvaluateNotCondition(t *testing.T) { - conditionTreeEvaluator := NewMixedTreeEvaluator() + conditionTreeEvaluator := NewMixedTreeEvaluator(logging.GetLogger("", "")) // [or, [not, stringFooCondition], [not, boolTrueCondition]] conditionTree := &e.TreeNode{ Operator: "or", @@ -237,7 +238,7 @@ func TestConditionTreeEvaluateNotCondition(t *testing.T) { } func TestConditionTreeEvaluateMultipleMixedConditions(t *testing.T) { - conditionTreeEvaluator := NewMixedTreeEvaluator() + conditionTreeEvaluator := NewMixedTreeEvaluator(logging.GetLogger("", "")) // [or, [and, stringFooCondition, boolTrueCondition], [or, [not, stringFooCondition], int42Condition]] conditionTree := &e.TreeNode{ Operator: "or", @@ -371,7 +372,7 @@ func TestConditionTreeEvaluateAnAudienceTreeSingleAudience(t *testing.T) { }, } - conditionTreeEvaluator := NewMixedTreeEvaluator() + conditionTreeEvaluator := NewMixedTreeEvaluator(logging.GetLogger("", "")) // Test matches audience 11111 treeParams := &e.TreeParameters{ @@ -400,7 +401,7 @@ func TestConditionTreeEvaluateAnAudienceTreeMultipleAudiences(t *testing.T) { }, } - conditionTreeEvaluator := NewMixedTreeEvaluator() + conditionTreeEvaluator := NewMixedTreeEvaluator(logging.GetLogger("", "")) // Test only matches audience 11111 treeParams := &e.TreeParameters{ diff --git a/pkg/decision/evaluator/matchers/exact.go b/pkg/decision/evaluator/matchers/exact.go index bf307c08c..a267dcef1 100644 --- a/pkg/decision/evaluator/matchers/exact.go +++ b/pkg/decision/evaluator/matchers/exact.go @@ -22,18 +22,27 @@ import ( "github.com/optimizely/go-sdk/pkg/decision/evaluator/matchers/utils" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" ) // ExactMatcher matches against the "exact" match type type ExactMatcher struct { Condition entities.Condition + Logger logging.OptimizelyLogProducer } // Match returns true if the user's attribute match the condition's string value func (m ExactMatcher) Match(user entities.UserContext) (bool, error) { + + if !user.CheckAttributeExists(m.Condition.Name) { + m.Logger.Debug(fmt.Sprintf(`Audience condition %s evaluated to UNKNOWN because a null value was passed for user attribute "%s".`, m.Condition.StringRepresentation, m.Condition)) + return false, fmt.Errorf(`no attribute named "%s"`, m.Condition.Name) + } + if stringValue, ok := m.Condition.Value.(string); ok { attributeValue, err := user.GetStringAttribute(m.Condition.Name) if err != nil { + m.Logger.Warning(fmt.Sprintf(`Audience condition "%s" evaluated to UNKNOWN because a value of type "%s" was passed for user attribute "%s".`, m.Condition.StringRepresentation, m.Condition.Value, m.Condition.Name)) return false, err } return stringValue == attributeValue, nil @@ -55,5 +64,6 @@ func (m ExactMatcher) Match(user entities.UserContext) (bool, error) { return floatValue == attributeValue, nil } + m.Logger.Warning(fmt.Sprintf(`Audience condition "%s" has an unsupported condition value. You may need to upgrade to a newer release of the Optimizely SDK.`, m.Condition.StringRepresentation)) return false, fmt.Errorf("audience condition %s evaluated to NULL because the condition value type is not supported", m.Condition.Name) } diff --git a/pkg/decision/evaluator/matchers/exact_test.go b/pkg/decision/evaluator/matchers/exact_test.go index d74649964..86e674fce 100644 --- a/pkg/decision/evaluator/matchers/exact_test.go +++ b/pkg/decision/evaluator/matchers/exact_test.go @@ -22,10 +22,12 @@ import ( "github.com/stretchr/testify/assert" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" ) func TestExactMatcherString(t *testing.T) { matcher := ExactMatcher{ + Logger: logging.GetLogger("", ""), Condition: entities.Condition{ Match: "exact", Value: "foo", @@ -67,6 +69,7 @@ func TestExactMatcherString(t *testing.T) { func TestExactMatcherBool(t *testing.T) { matcher := ExactMatcher{ + Logger: logging.GetLogger("", ""), Condition: entities.Condition{ Match: "exact", Value: true, @@ -108,6 +111,7 @@ func TestExactMatcherBool(t *testing.T) { func TestExactMatcherInt(t *testing.T) { matcher := ExactMatcher{ + Logger: logging.GetLogger("", ""), Condition: entities.Condition{ Match: "exact", Value: 42, @@ -160,6 +164,7 @@ func TestExactMatcherInt(t *testing.T) { func TestExactMatcherFloat(t *testing.T) { matcher := ExactMatcher{ + Logger: logging.GetLogger("", ""), Condition: entities.Condition{ Match: "exact", Value: 4.2, diff --git a/pkg/decision/evaluator/matchers/gt.go b/pkg/decision/evaluator/matchers/gt.go index 04832d44b..d64119f75 100644 --- a/pkg/decision/evaluator/matchers/gt.go +++ b/pkg/decision/evaluator/matchers/gt.go @@ -22,23 +22,32 @@ import ( "github.com/optimizely/go-sdk/pkg/decision/evaluator/matchers/utils" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" ) // GtMatcher matches against the "gt" match type type GtMatcher struct { Condition entities.Condition + Logger logging.OptimizelyLogProducer } // Match returns true if the user's attribute is greater than the condition's string value func (m GtMatcher) Match(user entities.UserContext) (bool, error) { + if !user.CheckAttributeExists(m.Condition.Name) { + m.Logger.Debug(fmt.Sprintf(`Audience condition %s evaluated to UNKNOWN because a null value was passed for user attribute "%s".`, m.Condition.StringRepresentation, m.Condition)) + return false, fmt.Errorf(`no attribute named "%s"`, m.Condition.Name) + } + if floatValue, ok := utils.ToFloat(m.Condition.Value); ok { attributeValue, err := user.GetFloatAttribute(m.Condition.Name) if err != nil { + m.Logger.Warning(fmt.Sprintf(`Audience condition "%s" evaluated to UNKNOWN because a value of type "%s" was passed for user attribute "%s".`, m.Condition.StringRepresentation, m.Condition.Value, m.Condition.Name)) return false, err } return floatValue < attributeValue, nil } + m.Logger.Warning(fmt.Sprintf(`Audience condition "%s" has an unsupported condition value. You may need to upgrade to a newer release of the Optimizely SDK.`, m.Condition.StringRepresentation)) return false, fmt.Errorf("audience condition %s evaluated to NULL because the condition value type is not supported", m.Condition.Name) } diff --git a/pkg/decision/evaluator/matchers/gt_test.go b/pkg/decision/evaluator/matchers/gt_test.go index a68eda2f4..3a72ce5a7 100644 --- a/pkg/decision/evaluator/matchers/gt_test.go +++ b/pkg/decision/evaluator/matchers/gt_test.go @@ -22,10 +22,12 @@ import ( "github.com/stretchr/testify/assert" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" ) func TestGtMatcherInt(t *testing.T) { matcher := GtMatcher{ + Logger: logging.GetLogger("", ""), Condition: entities.Condition{ Match: "gt", Value: 42, @@ -89,6 +91,7 @@ func TestGtMatcherInt(t *testing.T) { func TestGtMatcherFloat(t *testing.T) { matcher := GtMatcher{ + Logger: logging.GetLogger("", ""), Condition: entities.Condition{ Match: "gt", Value: 4.2, diff --git a/pkg/decision/evaluator/matchers/lt.go b/pkg/decision/evaluator/matchers/lt.go index ad47eddaf..4ede415df 100644 --- a/pkg/decision/evaluator/matchers/lt.go +++ b/pkg/decision/evaluator/matchers/lt.go @@ -22,23 +22,32 @@ import ( "github.com/optimizely/go-sdk/pkg/decision/evaluator/matchers/utils" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" ) // LtMatcher matches against the "lt" match type type LtMatcher struct { Condition entities.Condition + Logger logging.OptimizelyLogProducer } // Match returns true if the user's attribute is less than the condition's string value func (m LtMatcher) Match(user entities.UserContext) (bool, error) { + if !user.CheckAttributeExists(m.Condition.Name) { + m.Logger.Debug(fmt.Sprintf(`Audience condition %s evaluated to UNKNOWN because a null value was passed for user attribute "%s".`, m.Condition.StringRepresentation, m.Condition)) + return false, fmt.Errorf(`no attribute named "%s"`, m.Condition.Name) + } + if floatValue, ok := utils.ToFloat(m.Condition.Value); ok { attributeValue, err := user.GetFloatAttribute(m.Condition.Name) if err != nil { + m.Logger.Warning(fmt.Sprintf(`Audience condition "%s" evaluated to UNKNOWN because a value of type "%s" was passed for user attribute "%s".`, m.Condition.StringRepresentation, m.Condition.Value, m.Condition.Name)) return false, err } return floatValue > attributeValue, nil } + m.Logger.Warning(fmt.Sprintf(`Audience condition "%s" has an unsupported condition value. You may need to upgrade to a newer release of the Optimizely SDK.`, m.Condition.StringRepresentation)) return false, fmt.Errorf("audience condition %s evaluated to NULL because the condition value type is not supported", m.Condition.Name) } diff --git a/pkg/decision/evaluator/matchers/lt_test.go b/pkg/decision/evaluator/matchers/lt_test.go index e79c05a1c..06cbd0f6e 100644 --- a/pkg/decision/evaluator/matchers/lt_test.go +++ b/pkg/decision/evaluator/matchers/lt_test.go @@ -22,10 +22,12 @@ import ( "github.com/stretchr/testify/assert" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" ) func TestLtMatcherInt(t *testing.T) { matcher := LtMatcher{ + Logger: logging.GetLogger("", ""), Condition: entities.Condition{ Match: "lt", Value: 42, @@ -89,6 +91,7 @@ func TestLtMatcherInt(t *testing.T) { func TestLtMatcherFloat(t *testing.T) { matcher := LtMatcher{ + Logger: logging.GetLogger("", ""), Condition: entities.Condition{ Match: "lt", Value: 4.2, diff --git a/pkg/decision/evaluator/matchers/substring.go b/pkg/decision/evaluator/matchers/substring.go index f25241f02..841626252 100644 --- a/pkg/decision/evaluator/matchers/substring.go +++ b/pkg/decision/evaluator/matchers/substring.go @@ -22,23 +22,32 @@ import ( "strings" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" ) // SubstringMatcher matches against the "substring" match type type SubstringMatcher struct { Condition entities.Condition + Logger logging.OptimizelyLogProducer } // Match returns true if the user's attribute is a substring of the condition's string value func (m SubstringMatcher) Match(user entities.UserContext) (bool, error) { + if !user.CheckAttributeExists(m.Condition.Name) { + m.Logger.Debug(fmt.Sprintf(`Audience condition %s evaluated to UNKNOWN because a null value was passed for user attribute "%s".`, m.Condition.StringRepresentation, m.Condition)) + return false, fmt.Errorf(`no attribute named "%s"`, m.Condition.Name) + } + if stringValue, ok := m.Condition.Value.(string); ok { attributeValue, err := user.GetStringAttribute(m.Condition.Name) if err != nil { + m.Logger.Warning(fmt.Sprintf(`Audience condition "%s" evaluated to UNKNOWN because a value of type "%s" was passed for user attribute "%s".`, m.Condition.StringRepresentation, m.Condition.Value, m.Condition.Name)) return false, err } return strings.Contains(attributeValue, stringValue), nil } + m.Logger.Warning(fmt.Sprintf(`Audience condition "%s" has an unsupported condition value. You may need to upgrade to a newer release of the Optimizely SDK.`, m.Condition.StringRepresentation)) return false, fmt.Errorf("audience condition %s evaluated to NULL because the condition value type is not supported", m.Condition.Name) } diff --git a/pkg/decision/evaluator/matchers/substring_test.go b/pkg/decision/evaluator/matchers/substring_test.go index 9d7db2e00..b06c17a81 100644 --- a/pkg/decision/evaluator/matchers/substring_test.go +++ b/pkg/decision/evaluator/matchers/substring_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/optimizely/go-sdk/pkg/entities" + "github.com/optimizely/go-sdk/pkg/logging" ) func TestSubstringMatcher(t *testing.T) { @@ -31,6 +32,7 @@ func TestSubstringMatcher(t *testing.T) { Value: "foo", Name: "string_foo", }, + Logger: logging.GetLogger("", ""), } // Test match diff --git a/pkg/decision/experiment_bucketer_service.go b/pkg/decision/experiment_bucketer_service.go index 1ee16ad78..c02c14208 100644 --- a/pkg/decision/experiment_bucketer_service.go +++ b/pkg/decision/experiment_bucketer_service.go @@ -38,8 +38,8 @@ type ExperimentBucketerService struct { func NewExperimentBucketerService(logger logging.OptimizelyLogProducer) *ExperimentBucketerService { // @TODO(mng): add experiment override service return &ExperimentBucketerService{ - logger:logger, - audienceTreeEvaluator: evaluator.NewMixedTreeEvaluator(), + logger: logger, + audienceTreeEvaluator: evaluator.NewMixedTreeEvaluator(logger), bucketer: *bucketer.NewMurmurhashExperimentBucketer(logger, bucketer.DefaultHashSeed), } } @@ -52,11 +52,16 @@ func (s ExperimentBucketerService) GetDecision(decisionContext ExperimentDecisio // Determine if user can be part of the experiment if experiment.AudienceConditionTree != nil { condTreeParams := entities.NewTreeParameters(&userContext, decisionContext.ProjectConfig.GetAudienceMap()) + s.logger.Debug(fmt.Sprintf(`Evaluating audiences for experiment "%s".`, experiment.Key)) evalResult, _ := s.audienceTreeEvaluator.Evaluate(experiment.AudienceConditionTree, condTreeParams) + s.logger.Info(fmt.Sprintf(`Audiences for rule %s collectively evaluated to %t.`, experiment.Key, evalResult)) if !evalResult { + s.logger.Info(fmt.Sprintf(`User "%s" does not meet conditions to be in experiment "%s".`, userContext.ID, experiment.Key)) experimentDecision.Reason = reasons.FailedAudienceTargeting return experimentDecision, nil } + } else { + s.logger.Info(fmt.Sprintf(`Audiences for experiment "%s" collectively evaluated to TRUE.`, experiment.Key)) } var group entities.Group diff --git a/pkg/decision/experiment_bucketer_service_test.go b/pkg/decision/experiment_bucketer_service_test.go index 2577735f4..ed5c2a815 100644 --- a/pkg/decision/experiment_bucketer_service_test.go +++ b/pkg/decision/experiment_bucketer_service_test.go @@ -35,14 +35,34 @@ func (m *MockBucketer) Bucket(bucketingID string, experiment entities.Experiment return args.Get(0).(*entities.Variation), args.Get(1).(reasons.Reason), args.Error(2) } +type MockLogger struct { + mock.Mock +} + +func (m *MockLogger) Debug(message string) { + m.Called(message) +} + +func (m *MockLogger) Info(message string) { + m.Called(message) +} + +func (m *MockLogger) Warning(message string) { +} + +func (m *MockLogger) Error(message string, err interface{}) { +} + type ExperimentBucketerTestSuite struct { suite.Suite mockBucketer *MockBucketer + mockLogger *MockLogger mockConfig *mockProjectConfig } func (s *ExperimentBucketerTestSuite) SetupTest() { s.mockBucketer = new(MockBucketer) + s.mockLogger = new(MockLogger) s.mockConfig = new(mockProjectConfig) } @@ -63,9 +83,10 @@ func (s *ExperimentBucketerTestSuite) TestGetDecisionNoTargeting() { ProjectConfig: s.mockConfig, } s.mockBucketer.On("Bucket", testUserContext.ID, testExp1111, entities.Group{}).Return(&testExp1111Var2222, reasons.BucketedIntoVariation, nil) - + s.mockLogger.On("Info", `Audiences for experiment "test_experiment_1111" collectively evaluated to TRUE.`) experimentBucketerService := ExperimentBucketerService{ bucketer: s.mockBucketer, + logger: s.mockLogger, } decision, err := experimentBucketerService.GetDecision(testDecisionContext, testUserContext) s.Equal(expectedDecision, decision) @@ -89,9 +110,12 @@ func (s *ExperimentBucketerTestSuite) TestGetDecisionWithTargetingPasses() { mockAudienceTreeEvaluator.On("Evaluate", mock.Anything, mock.Anything).Return(true, true) experimentBucketerService := ExperimentBucketerService{ audienceTreeEvaluator: mockAudienceTreeEvaluator, + logger: s.mockLogger, bucketer: s.mockBucketer, } s.mockConfig.On("GetAudienceMap").Return(map[string]entities.Audience{}) + s.mockLogger.On("Debug", `Evaluating audiences for experiment "test_targeted_experiment_1116".`) + s.mockLogger.On("Info", `Audiences for rule test_targeted_experiment_1116 collectively evaluated to true.`) testDecisionContext := ExperimentDecisionContext{ Experiment: &testTargetedExp1116, @@ -116,9 +140,13 @@ func (s *ExperimentBucketerTestSuite) TestGetDecisionWithTargetingFails() { mockAudienceTreeEvaluator.On("Evaluate", mock.Anything, mock.Anything).Return(false, true) experimentBucketerService := ExperimentBucketerService{ audienceTreeEvaluator: mockAudienceTreeEvaluator, + logger: s.mockLogger, bucketer: s.mockBucketer, } s.mockConfig.On("GetAudienceMap").Return(map[string]entities.Audience{}) + s.mockLogger.On("Debug", `Evaluating audiences for experiment "test_targeted_experiment_1116".`) + s.mockLogger.On("Info", `Audiences for rule test_targeted_experiment_1116 collectively evaluated to false.`) + s.mockLogger.On("Info", `User "test_user_1" does not meet conditions to be in experiment "test_targeted_experiment_1116".`) testDecisionContext := ExperimentDecisionContext{ Experiment: &testTargetedExp1116, diff --git a/pkg/decision/rollout_service.go b/pkg/decision/rollout_service.go index 17dca3769..451955ea3 100644 --- a/pkg/decision/rollout_service.go +++ b/pkg/decision/rollout_service.go @@ -35,9 +35,10 @@ type RolloutService struct { // NewRolloutService returns a new instance of the Rollout service func NewRolloutService(sdkKey string) *RolloutService { + logger := logging.GetLogger(sdkKey, "RolloutService") return &RolloutService{ - logger: logging.GetLogger(sdkKey, "RolloutService"), - audienceTreeEvaluator: evaluator.NewMixedTreeEvaluator(), + logger: logger, + audienceTreeEvaluator: evaluator.NewMixedTreeEvaluator(logger), experimentBucketerService: NewExperimentBucketerService(logging.GetLogger(sdkKey, "ExperimentBucketerService")), } } @@ -51,12 +52,12 @@ func (r RolloutService) GetDecision(decisionContext FeatureDecisionContext, user feature := decisionContext.Feature rollout := feature.Rollout - evaluateConditionTree := func(experiment *entities.Experiment) bool { + evaluateConditionTree := func(experiment *entities.Experiment, loggingKey string) bool { condTreeParams := entities.NewTreeParameters(&userContext, decisionContext.ProjectConfig.GetAudienceMap()) + r.logger.Debug(fmt.Sprintf(`Evaluating audiences for rule %s.`, loggingKey)) evalResult, _ := r.audienceTreeEvaluator.Evaluate(experiment.AudienceConditionTree, condTreeParams) if !evalResult { featureDecision.Reason = reasons.FailedRolloutTargeting - r.logger.Debug(fmt.Sprintf(`User "%s" failed targeting for feature rollout with key "%s".`, userContext.ID, feature.Key)) } return evalResult } @@ -97,13 +98,19 @@ func (r RolloutService) GetDecision(decisionContext FeatureDecisionContext, user } for index := 0; index < numberOfExperiments-1; index++ { + loggingKey := string(index + 1) experiment := &rollout.Experiments[index] experimentDecisionContext := getExperimentDecisionContext(experiment) // Move to next evaluation if condition tree is available and evaluation fails - if experiment.AudienceConditionTree != nil && !evaluateConditionTree(experiment) { + + evaluationResult := experiment.AudienceConditionTree == nil || evaluateConditionTree(experiment, loggingKey) + r.logger.Info(fmt.Sprintf(`Audiences for rule %s collectively evaluated to %t.`, loggingKey, evaluationResult)) + if !evaluationResult { + r.logger.Debug(fmt.Sprintf(`User "%s" does not meet conditions for targeting rule %s.`, userContext.ID, loggingKey)) // Evaluate this user for the next rule continue } + decision, _ := r.experimentBucketerService.GetDecision(experimentDecisionContext, userContext) if decision.Variation == nil { // Evaluate fall back rule / last rule now @@ -116,8 +123,14 @@ func (r RolloutService) GetDecision(decisionContext FeatureDecisionContext, user experiment := &rollout.Experiments[numberOfExperiments-1] experimentDecisionContext := getExperimentDecisionContext(experiment) // Move to bucketing if conditionTree is unavailable or evaluation passes - if experiment.AudienceConditionTree == nil || evaluateConditionTree(experiment) { - decision, _ := r.experimentBucketerService.GetDecision(experimentDecisionContext, userContext) + evaluationResult := experiment.AudienceConditionTree == nil || evaluateConditionTree(experiment, "Everyone Else") + r.logger.Info(fmt.Sprintf(`Audiences for rule %s collectively evaluated to %t.`, "Everyone Else", evaluationResult)) + + if evaluationResult { + decision, err := r.experimentBucketerService.GetDecision(experimentDecisionContext, userContext) + if err == nil { + r.logger.Debug(fmt.Sprintf(`User "%s" meets conditions for targeting rule "Everyone Else".`, userContext.ID)) + } return getFeatureDecision(experiment, &decision) } diff --git a/pkg/entities/audience.go b/pkg/entities/audience.go index d8abb6d2b..84682c330 100644 --- a/pkg/entities/audience.go +++ b/pkg/entities/audience.go @@ -26,8 +26,9 @@ type Audience struct { // Condition has condition info type Condition struct { - Name string `json:"name"` - Match string `json:"match"` - Type string `json:"type"` - Value interface{} `json:"value"` + Name string `json:"name"` + Match string `json:"match"` + Type string `json:"type"` + Value interface{} `json:"value"` + StringRepresentation string } From dc4c6186c23f02a267f3c3653fe5199314ea52ff Mon Sep 17 00:00:00 2001 From: Yasir Ali Date: Tue, 14 Jul 2020 13:24:16 +0500 Subject: [PATCH 2/6] Unit tests added. --- .../mappers/condition_trees.go | 2 +- .../mappers/condition_trees_test.go | 27 +- pkg/decision/evaluator/condition.go | 11 +- pkg/decision/evaluator/condition_test.go | 85 +++++-- pkg/decision/evaluator/condition_tree.go | 2 +- pkg/decision/evaluator/condition_tree_test.go | 231 +++++++++++------- pkg/decision/evaluator/matchers/exact.go | 13 +- pkg/decision/evaluator/matchers/exact_test.go | 177 +++++++++++--- pkg/decision/evaluator/matchers/gt.go | 9 +- pkg/decision/evaluator/matchers/gt_test.go | 108 ++++++-- pkg/decision/evaluator/matchers/lt.go | 9 +- pkg/decision/evaluator/matchers/lt_test.go | 107 ++++++-- pkg/decision/evaluator/matchers/substring.go | 9 +- .../evaluator/matchers/substring_test.go | 68 +++++- pkg/decision/experiment_bucketer_service.go | 10 +- .../experiment_bucketer_service_test.go | 22 +- pkg/decision/helpers_test.go | 32 +-- pkg/decision/rollout_service.go | 16 +- pkg/decision/rollout_service_test.go | 55 ++++- pkg/entities/audience.go | 2 +- pkg/entities/user_context.go | 11 +- pkg/logging/log_messages.go | 60 +++++ 22 files changed, 801 insertions(+), 265 deletions(-) create mode 100644 pkg/logging/log_messages.go diff --git a/pkg/config/datafileprojectconfig/mappers/condition_trees.go b/pkg/config/datafileprojectconfig/mappers/condition_trees.go index cf9c72f97..2598926b7 100644 --- a/pkg/config/datafileprojectconfig/mappers/condition_trees.go +++ b/pkg/config/datafileprojectconfig/mappers/condition_trees.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * diff --git a/pkg/config/datafileprojectconfig/mappers/condition_trees_test.go b/pkg/config/datafileprojectconfig/mappers/condition_trees_test.go index 9c7e64166..b639eb2b2 100644 --- a/pkg/config/datafileprojectconfig/mappers/condition_trees_test.go +++ b/pkg/config/datafileprojectconfig/mappers/condition_trees_test.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -111,9 +111,10 @@ func TestBuildConditionTreeUsingDatafileAudienceConditions(t *testing.T) { Nodes: []*entities.TreeNode{ { Item: entities.Condition{ - Name: "s_foo", - Type: "custom_attribute", - Value: "foo", + Name: "s_foo", + Type: "custom_attribute", + Value: "foo", + StringRepresentation: `{"name":"s_foo","type":"custom_attribute","value":"foo"}`, }, }, }, @@ -145,10 +146,11 @@ func TestBuildConditionTreeSimpleAudienceCondition(t *testing.T) { Nodes: []*entities.TreeNode{ { Item: entities.Condition{ - Name: "s_foo", - Match: "exact", - Type: "custom_attribute", - Value: "foo", + Name: "s_foo", + Match: "exact", + Type: "custom_attribute", + Value: "foo", + StringRepresentation: `{"match":"exact","name":"s_foo","type":"custom_attribute","value":"foo"}`, }, }, }, @@ -172,10 +174,11 @@ func TestBuildConditionTreeWithLeafNode(t *testing.T) { Nodes: []*entities.TreeNode{ { Item: entities.Condition{ - Name: "s_foo", - Match: "exact", - Type: "custom_attribute", - Value: "foo", + Name: "s_foo", + Match: "exact", + Type: "custom_attribute", + Value: "foo", + StringRepresentation: `{"match":"exact","name":"s_foo","type":"custom_attribute","value":"foo"}`, }, }, }, diff --git a/pkg/decision/evaluator/condition.go b/pkg/decision/evaluator/condition.go index 5bb960230..45e3fa396 100644 --- a/pkg/decision/evaluator/condition.go +++ b/pkg/decision/evaluator/condition.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -53,7 +53,8 @@ func (c CustomAttributeConditionEvaluator) Evaluate(condition entities.Condition // We should only be evaluating custom attributes if condition.Type != customAttributeType { - c.logger.Warning(fmt.Sprintf(`Audience condition "%s" uses an unknown condition type. You may need to upgrade to a newer release of the Optimizely SDK.`, condition.StringRepresentation)) + + c.logger.Warning(fmt.Sprintf(string(logging.UnknownConditionType), condition.StringRepresentation)) return false, fmt.Errorf(`unable to evaluate condition of type "%s"`, condition.Type) } @@ -88,7 +89,7 @@ func (c CustomAttributeConditionEvaluator) Evaluate(condition entities.Condition Logger: c.logger, } default: - c.logger.Warning(fmt.Sprintf(`Audience condition "%s" uses an unknown match type. You may need to upgrade to a newer release of the Optimizely SDK.`, condition.StringRepresentation)) + c.logger.Warning(fmt.Sprintf(string(logging.UnknownMatchType), condition.StringRepresentation)) return false, fmt.Errorf(`invalid Condition matcher "%s"`, condition.Match) } @@ -111,14 +112,14 @@ func NewAudienceConditionEvaluator(logger logging.OptimizelyLogProducer) *Audien func (c AudienceConditionEvaluator) Evaluate(audienceID string, condTreeParams *entities.TreeParameters) (bool, error) { if audience, ok := condTreeParams.AudienceMap[audienceID]; ok { - c.logger.Debug(fmt.Sprintf(`Starting to evaluate audience "%s".`, audienceID)) + c.logger.Debug(fmt.Sprintf(string(logging.AudienceEvaluationStarted), audienceID)) condTree := audience.ConditionTree conditionTreeEvaluator := NewMixedTreeEvaluator(c.logger) retValue, isValid := conditionTreeEvaluator.Evaluate(condTree, condTreeParams) if !isValid { return false, fmt.Errorf(`an error occurred while evaluating nested tree for audience ID "%s"`, audienceID) } - c.logger.Debug(fmt.Sprintf(`Audience "%s" evaluated to %t.`, audienceID, retValue)) + c.logger.Debug(fmt.Sprintf(string(logging.AudienceEvaluatedTo), audienceID, retValue)) return retValue, nil } diff --git a/pkg/decision/evaluator/condition_test.go b/pkg/decision/evaluator/condition_test.go index 6749ad2b1..c609bef12 100644 --- a/pkg/decision/evaluator/condition_test.go +++ b/pkg/decision/evaluator/condition_test.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -17,15 +17,26 @@ package evaluator import ( + "fmt" "testing" "github.com/optimizely/go-sdk/pkg/entities" "github.com/optimizely/go-sdk/pkg/logging" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/suite" ) -func TestCustomAttributeConditionEvaluator(t *testing.T) { - conditionEvaluator := NewCustomAttributeConditionEvaluator(logging.GetLogger("", "")) +type ConditionTestSuite struct { + suite.Suite + mockLogger *MockLogger + conditionEvaluator *CustomAttributeConditionEvaluator +} + +func (s *ConditionTestSuite) SetupTest() { + s.mockLogger = new(MockLogger) + s.conditionEvaluator = NewCustomAttributeConditionEvaluator(s.mockLogger) +} + +func (s *ConditionTestSuite) TestCustomAttributeConditionEvaluator() { condition := entities.Condition{ Match: "exact", Value: "foo", @@ -41,8 +52,8 @@ func TestCustomAttributeConditionEvaluator(t *testing.T) { } condTreeParams := entities.NewTreeParameters(&user, map[string]entities.Audience{}) - result, _ := conditionEvaluator.Evaluate(condition, condTreeParams) - assert.Equal(t, result, true) + result, _ := s.conditionEvaluator.Evaluate(condition, condTreeParams) + s.Equal(result, true) // Test condition fails user = entities.UserContext{ @@ -50,12 +61,11 @@ func TestCustomAttributeConditionEvaluator(t *testing.T) { "string_foo": "not_foo", }, } - result, _ = conditionEvaluator.Evaluate(condition, condTreeParams) - assert.Equal(t, result, false) + result, _ = s.conditionEvaluator.Evaluate(condition, condTreeParams) + s.Equal(result, false) } -func TestCustomAttributeConditionEvaluatorWithoutMatchType(t *testing.T) { - conditionEvaluator := NewCustomAttributeConditionEvaluator(logging.GetLogger("", "")) +func (s *ConditionTestSuite) TestCustomAttributeConditionEvaluatorWithoutMatchType() { condition := entities.Condition{ Value: "foo", Name: "string_foo", @@ -70,8 +80,8 @@ func TestCustomAttributeConditionEvaluatorWithoutMatchType(t *testing.T) { } condTreeParams := entities.NewTreeParameters(&user, map[string]entities.Audience{}) - result, _ := conditionEvaluator.Evaluate(condition, condTreeParams) - assert.Equal(t, result, true) + result, _ := s.conditionEvaluator.Evaluate(condition, condTreeParams) + s.Equal(result, true) // Test condition fails user = entities.UserContext{ @@ -79,6 +89,53 @@ func TestCustomAttributeConditionEvaluatorWithoutMatchType(t *testing.T) { "string_foo": "not_foo", }, } - result, _ = conditionEvaluator.Evaluate(condition, condTreeParams) - assert.Equal(t, result, false) + result, _ = s.conditionEvaluator.Evaluate(condition, condTreeParams) + s.Equal(result, false) +} + +func (s *ConditionTestSuite) TestCustomAttributeConditionEvaluatorWithInvalidMatchType() { + condition := entities.Condition{ + Value: "foo", + Name: "string_foo", + Type: "custom_attribute", + Match: "invalid", + } + + // Test condition fails + user := entities.UserContext{ + Attributes: map[string]interface{}{ + "string_foo": "foo", + }, + } + + condTreeParams := entities.NewTreeParameters(&user, map[string]entities.Audience{}) + s.mockLogger.On("Warning", fmt.Sprintf(string(logging.UnknownMatchType), "")) + result, _ := s.conditionEvaluator.Evaluate(condition, condTreeParams) + s.Equal(result, false) + s.mockLogger.AssertExpectations(s.T()) +} + +func (s *ConditionTestSuite) TestCustomAttributeConditionEvaluatorWithUnknownType() { + condition := entities.Condition{ + Value: "foo", + Name: "string_foo", + Type: "", + } + + // Test condition fails + user := entities.UserContext{ + Attributes: map[string]interface{}{ + "string_foo": "foo", + }, + } + + condTreeParams := entities.NewTreeParameters(&user, map[string]entities.Audience{}) + s.mockLogger.On("Warning", fmt.Sprintf(string(logging.UnknownConditionType), "")) + result, _ := s.conditionEvaluator.Evaluate(condition, condTreeParams) + s.Equal(result, false) + s.mockLogger.AssertExpectations(s.T()) +} + +func TestConditionTestSuite(t *testing.T) { + suite.Run(t, new(ConditionTestSuite)) } diff --git a/pkg/decision/evaluator/condition_tree.go b/pkg/decision/evaluator/condition_tree.go index 25a26484f..3ded0e716 100644 --- a/pkg/decision/evaluator/condition_tree.go +++ b/pkg/decision/evaluator/condition_tree.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * diff --git a/pkg/decision/evaluator/condition_tree_test.go b/pkg/decision/evaluator/condition_tree_test.go index f7209cb8e..af19e0a7e 100644 --- a/pkg/decision/evaluator/condition_tree_test.go +++ b/pkg/decision/evaluator/condition_tree_test.go @@ -1,14 +1,52 @@ +/**************************************************************************** + * Copyright 2019-2020, Optimizely, Inc. and contributors * + * * + * Licensed under the Apache License, Version 2.0 (the "License"); * + * you may not use this file except in compliance with the License. * + * You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, software * + * distributed under the License is distributed on an "AS IS" BASIS, * + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * + * See the License for the specific language governing permissions and * + * limitations under the License. * + ***************************************************************************/ + package evaluator import ( + "fmt" "testing" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/suite" e "github.com/optimizely/go-sdk/pkg/entities" "github.com/optimizely/go-sdk/pkg/logging" ) +type MockLogger struct { + mock.Mock +} + +func (m *MockLogger) Debug(message string) { + m.Called(message) +} + +func (m *MockLogger) Info(message string) { + m.Called(message) +} + +func (m *MockLogger) Warning(message string) { + m.Called(message) +} + +func (m *MockLogger) Error(message string, err interface{}) { + m.Called(message, err) +} + var stringFooCondition = e.Condition{ Type: "custom_attribute", Match: "exact", @@ -30,12 +68,22 @@ var int42Condition = e.Condition{ Value: 42, } -func TestConditionTreeEvaluateSimpleCondition(t *testing.T) { - conditionTreeEvaluator := NewMixedTreeEvaluator(logging.GetLogger("", "")) +type ConditionTreeTestSuite struct { + suite.Suite + mockLogger *MockLogger + conditionTreeEvaluator *MixedTreeEvaluator +} + +func (s *ConditionTreeTestSuite) SetupTest() { + s.mockLogger = new(MockLogger) + s.conditionTreeEvaluator = NewMixedTreeEvaluator(s.mockLogger) +} + +func (s *ConditionTreeTestSuite) TestConditionTreeEvaluateSimpleCondition() { conditionTree := &e.TreeNode{ Operator: "or", Nodes: []*e.TreeNode{ - &e.TreeNode{ + { Item: stringFooCondition, }, }, @@ -48,8 +96,8 @@ func TestConditionTreeEvaluateSimpleCondition(t *testing.T) { }, } condTreeParams := e.NewTreeParameters(&user, map[string]e.Audience{}) - result, _ := conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) - assert.True(t, result) + result, _ := s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + s.True(result) // Test no match user = e.UserContext{ @@ -57,19 +105,18 @@ func TestConditionTreeEvaluateSimpleCondition(t *testing.T) { "string_foo": "not foo", }, } - result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) - assert.False(t, result) + result, _ = s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + s.False(result) } -func TestConditionTreeEvaluateMultipleOrConditions(t *testing.T) { - conditionTreeEvaluator := NewMixedTreeEvaluator(logging.GetLogger("", "")) +func (s *ConditionTreeTestSuite) TestConditionTreeEvaluateMultipleOrConditions() { conditionTree := &e.TreeNode{ Operator: "or", Nodes: []*e.TreeNode{ - &e.TreeNode{ + { Item: stringFooCondition, }, - &e.TreeNode{ + { Item: boolTrueCondition, }, }, @@ -83,8 +130,8 @@ func TestConditionTreeEvaluateMultipleOrConditions(t *testing.T) { } condTreeParams := e.NewTreeParameters(&user, map[string]e.Audience{}) - result, _ := conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) - assert.True(t, result) + result, _ := s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + s.True(result) // Test match bool user = e.UserContext{ @@ -92,8 +139,10 @@ func TestConditionTreeEvaluateMultipleOrConditions(t *testing.T) { "bool_true": true, }, } - result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) - assert.True(t, result) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "string_foo")) + result, _ = s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + + s.True(result) // Test match both user = e.UserContext{ @@ -102,8 +151,8 @@ func TestConditionTreeEvaluateMultipleOrConditions(t *testing.T) { "bool_true": true, }, } - result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) - assert.True(t, result) + result, _ = s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + s.True(result) // Test no match user = e.UserContext{ @@ -112,19 +161,19 @@ func TestConditionTreeEvaluateMultipleOrConditions(t *testing.T) { "bool_true": false, }, } - result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) - assert.False(t, result) + result, _ = s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + s.False(result) + s.mockLogger.AssertExpectations(s.T()) } -func TestConditionTreeEvaluateMultipleAndConditions(t *testing.T) { - conditionTreeEvaluator := NewMixedTreeEvaluator(logging.GetLogger("", "")) +func (s *ConditionTreeTestSuite) TestConditionTreeEvaluateMultipleAndConditions() { conditionTree := &e.TreeNode{ Operator: "and", Nodes: []*e.TreeNode{ - &e.TreeNode{ + { Item: stringFooCondition, }, - &e.TreeNode{ + { Item: boolTrueCondition, }, }, @@ -138,8 +187,9 @@ func TestConditionTreeEvaluateMultipleAndConditions(t *testing.T) { } condTreeParams := e.NewTreeParameters(&user, map[string]e.Audience{}) - result, _ := conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) - assert.False(t, result) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "bool_true")) + result, _ := s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + s.False(result) // Test only bool match with NULL bubbling user = e.UserContext{ @@ -147,8 +197,10 @@ func TestConditionTreeEvaluateMultipleAndConditions(t *testing.T) { "bool_true": true, }, } - result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) - assert.False(t, result) + + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "string_foo")) + result, _ = s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + s.False(result) // Test match both user = e.UserContext{ @@ -157,8 +209,8 @@ func TestConditionTreeEvaluateMultipleAndConditions(t *testing.T) { "bool_true": true, }, } - result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) - assert.True(t, result) + result, _ = s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + s.True(result) // Test no match user = e.UserContext{ @@ -167,28 +219,28 @@ func TestConditionTreeEvaluateMultipleAndConditions(t *testing.T) { "bool_true": false, }, } - result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) - assert.False(t, result) + result, _ = s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + s.False(result) + s.mockLogger.AssertExpectations(s.T()) } -func TestConditionTreeEvaluateNotCondition(t *testing.T) { - conditionTreeEvaluator := NewMixedTreeEvaluator(logging.GetLogger("", "")) +func (s *ConditionTreeTestSuite) TestConditionTreeEvaluateNotCondition() { // [or, [not, stringFooCondition], [not, boolTrueCondition]] conditionTree := &e.TreeNode{ Operator: "or", Nodes: []*e.TreeNode{ - &e.TreeNode{ + { Operator: "not", Nodes: []*e.TreeNode{ - &e.TreeNode{ + { Item: stringFooCondition, }, }, }, - &e.TreeNode{ + { Operator: "not", Nodes: []*e.TreeNode{ - &e.TreeNode{ + { Item: boolTrueCondition, }, }, @@ -204,8 +256,8 @@ func TestConditionTreeEvaluateNotCondition(t *testing.T) { } condTreeParams := e.NewTreeParameters(&user, map[string]e.Audience{}) - result, _ := conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) - assert.True(t, result) + result, _ := s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + s.True(result) // Test match bool user = e.UserContext{ @@ -213,8 +265,9 @@ func TestConditionTreeEvaluateNotCondition(t *testing.T) { "bool_true": false, }, } - result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) - assert.True(t, result) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "string_foo")) + result, _ = s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + s.True(result) // Test match both user = e.UserContext{ @@ -223,8 +276,8 @@ func TestConditionTreeEvaluateNotCondition(t *testing.T) { "bool_true": false, }, } - result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) - assert.True(t, result) + result, _ = s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + s.True(result) // Test no match user = e.UserContext{ @@ -233,39 +286,39 @@ func TestConditionTreeEvaluateNotCondition(t *testing.T) { "bool_true": true, }, } - result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) - assert.False(t, result) + result, _ = s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + s.False(result) + s.mockLogger.AssertExpectations(s.T()) } -func TestConditionTreeEvaluateMultipleMixedConditions(t *testing.T) { - conditionTreeEvaluator := NewMixedTreeEvaluator(logging.GetLogger("", "")) +func (s *ConditionTreeTestSuite) TestConditionTreeEvaluateMultipleMixedConditions() { // [or, [and, stringFooCondition, boolTrueCondition], [or, [not, stringFooCondition], int42Condition]] conditionTree := &e.TreeNode{ Operator: "or", Nodes: []*e.TreeNode{ - &e.TreeNode{ + { Operator: "and", Nodes: []*e.TreeNode{ - &e.TreeNode{ + { Item: stringFooCondition, }, - &e.TreeNode{ + { Item: boolTrueCondition, }, }, }, - &e.TreeNode{ + { Operator: "or", Nodes: []*e.TreeNode{ - &e.TreeNode{ + { Operator: "not", Nodes: []*e.TreeNode{ - &e.TreeNode{ + { Item: stringFooCondition, }, }, }, - &e.TreeNode{ + { Item: int42Condition, }, }, @@ -283,8 +336,8 @@ func TestConditionTreeEvaluateMultipleMixedConditions(t *testing.T) { } condTreeParams := e.NewTreeParameters(&user, map[string]e.Audience{}) - result, _ := conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) - assert.True(t, result) + result, _ := s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + s.True(result) // Test only match the NOT condition user = e.UserContext{ @@ -294,8 +347,8 @@ func TestConditionTreeEvaluateMultipleMixedConditions(t *testing.T) { "int_42": 43, }, } - result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) - assert.True(t, result) + result, _ = s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + s.True(result) // Test only match the int condition user = e.UserContext{ @@ -305,8 +358,8 @@ func TestConditionTreeEvaluateMultipleMixedConditions(t *testing.T) { "int_42": 42, }, } - result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) - assert.True(t, result) + result, _ = s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + s.True(result) // Test no match user = e.UserContext{ @@ -316,8 +369,8 @@ func TestConditionTreeEvaluateMultipleMixedConditions(t *testing.T) { "int_42": 43, }, } - result, _ = conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) - assert.False(t, result) + result, _ = s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) + s.False(result) } var audienceMap = map[string]e.Audience{ @@ -330,10 +383,10 @@ var audience11111 = e.Audience{ ConditionTree: &e.TreeNode{ Operator: "or", Nodes: []*e.TreeNode{ - &e.TreeNode{ + { Operator: "or", Nodes: []*e.TreeNode{ - &e.TreeNode{ + { Item: stringFooCondition, }, }, @@ -347,13 +400,13 @@ var audience11112 = e.Audience{ ConditionTree: &e.TreeNode{ Operator: "or", Nodes: []*e.TreeNode{ - &e.TreeNode{ + { Operator: "and", Nodes: []*e.TreeNode{ - &e.TreeNode{ + { Item: boolTrueCondition, }, - &e.TreeNode{ + { Item: int42Condition, }, }, @@ -362,18 +415,16 @@ var audience11112 = e.Audience{ }, } -func TestConditionTreeEvaluateAnAudienceTreeSingleAudience(t *testing.T) { +func (s *ConditionTreeTestSuite) TestConditionTreeEvaluateAnAudienceTreeSingleAudience() { audienceTree := &e.TreeNode{ Operator: "or", Nodes: []*e.TreeNode{ - &e.TreeNode{ + { Item: audience11111.ID, }, }, } - conditionTreeEvaluator := NewMixedTreeEvaluator(logging.GetLogger("", "")) - // Test matches audience 11111 treeParams := &e.TreeParameters{ User: &e.UserContext{ @@ -384,25 +435,27 @@ func TestConditionTreeEvaluateAnAudienceTreeSingleAudience(t *testing.T) { }, AudienceMap: audienceMap, } - result, _ := conditionTreeEvaluator.Evaluate(audienceTree, treeParams) - assert.True(t, result) + + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.AudienceEvaluationStarted), "11111")) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.AudienceEvaluatedTo), "11111", true)) + result, _ := s.conditionTreeEvaluator.Evaluate(audienceTree, treeParams) + s.True(result) + s.mockLogger.AssertExpectations(s.T()) } -func TestConditionTreeEvaluateAnAudienceTreeMultipleAudiences(t *testing.T) { +func (s *ConditionTreeTestSuite) TestConditionTreeEvaluateAnAudienceTreeMultipleAudiences() { audienceTree := &e.TreeNode{ Operator: "or", Nodes: []*e.TreeNode{ - &e.TreeNode{ + { Item: audience11111.ID, }, - &e.TreeNode{ + { Item: audience11112.ID, }, }, } - conditionTreeEvaluator := NewMixedTreeEvaluator(logging.GetLogger("", "")) - // Test only matches audience 11111 treeParams := &e.TreeParameters{ User: &e.UserContext{ @@ -413,8 +466,10 @@ func TestConditionTreeEvaluateAnAudienceTreeMultipleAudiences(t *testing.T) { }, AudienceMap: audienceMap, } - result, _ := conditionTreeEvaluator.Evaluate(audienceTree, treeParams) - assert.True(t, result) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.AudienceEvaluationStarted), "11111")) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.AudienceEvaluatedTo), "11111", true)) + result, _ := s.conditionTreeEvaluator.Evaluate(audienceTree, treeParams) + s.True(result) // Test only matches audience 11112 treeParams = &e.TreeParameters{ @@ -427,6 +482,16 @@ func TestConditionTreeEvaluateAnAudienceTreeMultipleAudiences(t *testing.T) { }, AudienceMap: audienceMap, } - result, _ = conditionTreeEvaluator.Evaluate(audienceTree, treeParams) - assert.True(t, result) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.AudienceEvaluationStarted), "11111")) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "string_foo")) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.AudienceEvaluationStarted), "11112")) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.AudienceEvaluatedTo), "11112", true)) + + result, _ = s.conditionTreeEvaluator.Evaluate(audienceTree, treeParams) + s.True(result) + s.mockLogger.AssertExpectations(s.T()) +} + +func TestConditionTreeTestSuite(t *testing.T) { + suite.Run(t, new(ConditionTreeTestSuite)) } diff --git a/pkg/decision/evaluator/matchers/exact.go b/pkg/decision/evaluator/matchers/exact.go index a267dcef1..f789b446d 100644 --- a/pkg/decision/evaluator/matchers/exact.go +++ b/pkg/decision/evaluator/matchers/exact.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -35,14 +35,15 @@ type ExactMatcher struct { func (m ExactMatcher) Match(user entities.UserContext) (bool, error) { if !user.CheckAttributeExists(m.Condition.Name) { - m.Logger.Debug(fmt.Sprintf(`Audience condition %s evaluated to UNKNOWN because a null value was passed for user attribute "%s".`, m.Condition.StringRepresentation, m.Condition)) + m.Logger.Debug(fmt.Sprintf(string(logging.NullUserAttribute), m.Condition.StringRepresentation, m.Condition.Name)) return false, fmt.Errorf(`no attribute named "%s"`, m.Condition.Name) } if stringValue, ok := m.Condition.Value.(string); ok { attributeValue, err := user.GetStringAttribute(m.Condition.Name) if err != nil { - m.Logger.Warning(fmt.Sprintf(`Audience condition "%s" evaluated to UNKNOWN because a value of type "%s" was passed for user attribute "%s".`, m.Condition.StringRepresentation, m.Condition.Value, m.Condition.Name)) + val, _ := user.GetAttribute(m.Condition.Name) + m.Logger.Warning(fmt.Sprintf(string(logging.InvalidAttributeValueType), m.Condition.StringRepresentation, val, m.Condition.Name)) return false, err } return stringValue == attributeValue, nil @@ -51,6 +52,8 @@ func (m ExactMatcher) Match(user entities.UserContext) (bool, error) { if boolValue, ok := m.Condition.Value.(bool); ok { attributeValue, err := user.GetBoolAttribute(m.Condition.Name) if err != nil { + val, _ := user.GetAttribute(m.Condition.Name) + m.Logger.Warning(fmt.Sprintf(string(logging.InvalidAttributeValueType), m.Condition.StringRepresentation, val, m.Condition.Name)) return false, err } return boolValue == attributeValue, nil @@ -59,11 +62,13 @@ func (m ExactMatcher) Match(user entities.UserContext) (bool, error) { if floatValue, ok := utils.ToFloat(m.Condition.Value); ok { attributeValue, err := user.GetFloatAttribute(m.Condition.Name) if err != nil { + val, _ := user.GetAttribute(m.Condition.Name) + m.Logger.Warning(fmt.Sprintf(string(logging.InvalidAttributeValueType), m.Condition.StringRepresentation, val, m.Condition.Name)) return false, err } return floatValue == attributeValue, nil } - m.Logger.Warning(fmt.Sprintf(`Audience condition "%s" has an unsupported condition value. You may need to upgrade to a newer release of the Optimizely SDK.`, m.Condition.StringRepresentation)) + m.Logger.Warning(fmt.Sprintf(string(logging.UnsupportedConditionValue), m.Condition.StringRepresentation)) return false, fmt.Errorf("audience condition %s evaluated to NULL because the condition value type is not supported", m.Condition.Name) } diff --git a/pkg/decision/evaluator/matchers/exact_test.go b/pkg/decision/evaluator/matchers/exact_test.go index 86e674fce..809d651aa 100644 --- a/pkg/decision/evaluator/matchers/exact_test.go +++ b/pkg/decision/evaluator/matchers/exact_test.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -17,17 +17,48 @@ package matchers import ( + "fmt" "testing" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/suite" "github.com/optimizely/go-sdk/pkg/entities" "github.com/optimizely/go-sdk/pkg/logging" ) -func TestExactMatcherString(t *testing.T) { +type MockLogger struct { + mock.Mock +} + +func (m *MockLogger) Debug(message string) { + m.Called(message) +} + +func (m *MockLogger) Info(message string) { + m.Called(message) +} + +func (m *MockLogger) Warning(message string) { + m.Called(message) +} + +func (m *MockLogger) Error(message string, err interface{}) { + m.Called(message, err) +} + +type ExactTestSuite struct { + suite.Suite + mockLogger *MockLogger +} + +func (s *ExactTestSuite) SetupTest() { + s.mockLogger = new(MockLogger) +} + +func (s *ExactTestSuite) TestExactMatcherString() { matcher := ExactMatcher{ - Logger: logging.GetLogger("", ""), + Logger: s.mockLogger, Condition: entities.Condition{ Match: "exact", Value: "foo", @@ -42,8 +73,8 @@ func TestExactMatcherString(t *testing.T) { }, } result, err := matcher.Match(user) - assert.NoError(t, err) - assert.True(t, result) + s.NoError(err) + s.True(result) // Test no match user = entities.UserContext{ @@ -53,8 +84,8 @@ func TestExactMatcherString(t *testing.T) { } result, err = matcher.Match(user) - assert.NoError(t, err) - assert.False(t, result) + s.NoError(err) + s.False(result) // Test attribute not found user = entities.UserContext{ @@ -62,14 +93,26 @@ func TestExactMatcherString(t *testing.T) { "string_not_foo": "foo", }, } - + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "string_foo")) _, err = matcher.Match(user) - assert.Error(t, err) + s.Error(err) + + // Test attribute of different type + user = entities.UserContext{ + Attributes: map[string]interface{}{ + "string_foo": 121, + }, + } + s.mockLogger.On("Warning", fmt.Sprintf(string(logging.InvalidAttributeValueType), "", 121, "string_foo")) + result, err = matcher.Match(user) + s.Error(err) + s.False(false) + s.mockLogger.AssertExpectations(s.T()) } -func TestExactMatcherBool(t *testing.T) { +func (s *ExactTestSuite) TestExactMatcherBool() { matcher := ExactMatcher{ - Logger: logging.GetLogger("", ""), + Logger: s.mockLogger, Condition: entities.Condition{ Match: "exact", Value: true, @@ -84,8 +127,8 @@ func TestExactMatcherBool(t *testing.T) { }, } result, err := matcher.Match(user) - assert.NoError(t, err) - assert.True(t, result) + s.NoError(err) + s.True(result) // Test no match user = entities.UserContext{ @@ -95,8 +138,8 @@ func TestExactMatcherBool(t *testing.T) { } result, err = matcher.Match(user) - assert.NoError(t, err) - assert.False(t, result) + s.NoError(err) + s.False(result) // Test attribute not found user = entities.UserContext{ @@ -105,13 +148,26 @@ func TestExactMatcherBool(t *testing.T) { }, } + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "bool_true")) _, err = matcher.Match(user) - assert.Error(t, err) + s.Error(err) + + // Test attribute of different type + user = entities.UserContext{ + Attributes: map[string]interface{}{ + "bool_true": 121, + }, + } + s.mockLogger.On("Warning", fmt.Sprintf(string(logging.InvalidAttributeValueType), "", 121, "bool_true")) + result, err = matcher.Match(user) + s.Error(err) + s.False(result) + s.mockLogger.AssertExpectations(s.T()) } -func TestExactMatcherInt(t *testing.T) { +func (s *ExactTestSuite) TestExactMatcherInt() { matcher := ExactMatcher{ - Logger: logging.GetLogger("", ""), + Logger: s.mockLogger, Condition: entities.Condition{ Match: "exact", Value: 42, @@ -126,8 +182,8 @@ func TestExactMatcherInt(t *testing.T) { }, } result, err := matcher.Match(user) - assert.NoError(t, err) - assert.True(t, result) + s.NoError(err) + s.True(result) // Test match int to float user = entities.UserContext{ @@ -137,8 +193,8 @@ func TestExactMatcherInt(t *testing.T) { } result, err = matcher.Match(user) - assert.NoError(t, err) - assert.True(t, result) + s.NoError(err) + s.True(result) // Test no match user = entities.UserContext{ @@ -148,8 +204,8 @@ func TestExactMatcherInt(t *testing.T) { } result, err = matcher.Match(user) - assert.NoError(t, err) - assert.False(t, result) + s.NoError(err) + s.False(result) // Test attribute not found user = entities.UserContext{ @@ -157,14 +213,26 @@ func TestExactMatcherInt(t *testing.T) { "int_43": 42, }, } - + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "int_42")) _, err = matcher.Match(user) - assert.Error(t, err) + s.Error(err) + + // Test attribute of different type + user = entities.UserContext{ + Attributes: map[string]interface{}{ + "int_42": "test", + }, + } + s.mockLogger.On("Warning", fmt.Sprintf(string(logging.InvalidAttributeValueType), "", "test", "int_42")) + result, err = matcher.Match(user) + s.Error(err) + s.False(result) + s.mockLogger.AssertExpectations(s.T()) } -func TestExactMatcherFloat(t *testing.T) { +func (s *ExactTestSuite) TestExactMatcherFloat() { matcher := ExactMatcher{ - Logger: logging.GetLogger("", ""), + Logger: s.mockLogger, Condition: entities.Condition{ Match: "exact", Value: 4.2, @@ -179,8 +247,8 @@ func TestExactMatcherFloat(t *testing.T) { }, } result, err := matcher.Match(user) - assert.NoError(t, err) - assert.True(t, result) + s.NoError(err) + s.True(result) // Test no match user = entities.UserContext{ @@ -190,8 +258,8 @@ func TestExactMatcherFloat(t *testing.T) { } result, err = matcher.Match(user) - assert.NoError(t, err) - assert.False(t, result) + s.NoError(err) + s.False(result) // Test attribute not found user = entities.UserContext{ @@ -199,7 +267,46 @@ func TestExactMatcherFloat(t *testing.T) { "float_4_3": 4.2, }, } - + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "float_4_2")) _, err = matcher.Match(user) - assert.Error(t, err) + s.Error(err) + + // Test attribute of different type + user = entities.UserContext{ + Attributes: map[string]interface{}{ + "float_4_2": "test", + }, + } + s.mockLogger.On("Warning", fmt.Sprintf(string(logging.InvalidAttributeValueType), "", "test", "float_4_2")) + result, err = matcher.Match(user) + s.Error(err) + s.False(result) + s.mockLogger.AssertExpectations(s.T()) +} + +func (s *ExactTestSuite) TestExactMatcherUnsupportedConditionValue() { + matcher := ExactMatcher{ + Logger: s.mockLogger, + Condition: entities.Condition{ + Match: "exact", + Value: map[string]interface{}{}, + Name: "int_42", + }, + } + + // Test match - unsupported condition value + user := entities.UserContext{ + Attributes: map[string]interface{}{ + "int_42": 42, + }, + } + s.mockLogger.On("Warning", fmt.Sprintf(string(logging.UnsupportedConditionValue), "")) + result, err := matcher.Match(user) + s.Error(err) + s.False(result) + s.mockLogger.AssertExpectations(s.T()) +} + +func TestExactTestSuite(t *testing.T) { + suite.Run(t, new(ExactTestSuite)) } diff --git a/pkg/decision/evaluator/matchers/gt.go b/pkg/decision/evaluator/matchers/gt.go index d64119f75..e21d77e1f 100644 --- a/pkg/decision/evaluator/matchers/gt.go +++ b/pkg/decision/evaluator/matchers/gt.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -35,19 +35,20 @@ type GtMatcher struct { func (m GtMatcher) Match(user entities.UserContext) (bool, error) { if !user.CheckAttributeExists(m.Condition.Name) { - m.Logger.Debug(fmt.Sprintf(`Audience condition %s evaluated to UNKNOWN because a null value was passed for user attribute "%s".`, m.Condition.StringRepresentation, m.Condition)) + m.Logger.Debug(fmt.Sprintf(string(logging.NullUserAttribute), m.Condition.StringRepresentation, m.Condition.Name)) return false, fmt.Errorf(`no attribute named "%s"`, m.Condition.Name) } if floatValue, ok := utils.ToFloat(m.Condition.Value); ok { attributeValue, err := user.GetFloatAttribute(m.Condition.Name) if err != nil { - m.Logger.Warning(fmt.Sprintf(`Audience condition "%s" evaluated to UNKNOWN because a value of type "%s" was passed for user attribute "%s".`, m.Condition.StringRepresentation, m.Condition.Value, m.Condition.Name)) + val, _ := user.GetAttribute(m.Condition.Name) + m.Logger.Warning(fmt.Sprintf(string(logging.InvalidAttributeValueType), m.Condition.StringRepresentation, val, m.Condition.Name)) return false, err } return floatValue < attributeValue, nil } - m.Logger.Warning(fmt.Sprintf(`Audience condition "%s" has an unsupported condition value. You may need to upgrade to a newer release of the Optimizely SDK.`, m.Condition.StringRepresentation)) + m.Logger.Warning(fmt.Sprintf(string(logging.UnsupportedConditionValue), m.Condition.StringRepresentation)) return false, fmt.Errorf("audience condition %s evaluated to NULL because the condition value type is not supported", m.Condition.Name) } diff --git a/pkg/decision/evaluator/matchers/gt_test.go b/pkg/decision/evaluator/matchers/gt_test.go index 3a72ce5a7..c6d7a7423 100644 --- a/pkg/decision/evaluator/matchers/gt_test.go +++ b/pkg/decision/evaluator/matchers/gt_test.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -17,17 +17,27 @@ package matchers import ( + "fmt" "testing" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/suite" "github.com/optimizely/go-sdk/pkg/entities" "github.com/optimizely/go-sdk/pkg/logging" ) -func TestGtMatcherInt(t *testing.T) { +type GtTestSuite struct { + suite.Suite + mockLogger *MockLogger +} + +func (s *GtTestSuite) SetupTest() { + s.mockLogger = new(MockLogger) +} + +func (s *GtTestSuite) TestGtMatcherInt() { matcher := GtMatcher{ - Logger: logging.GetLogger("", ""), + Logger: s.mockLogger, Condition: entities.Condition{ Match: "gt", Value: 42, @@ -42,8 +52,8 @@ func TestGtMatcherInt(t *testing.T) { }, } result, err := matcher.Match(user) - assert.NoError(t, err) - assert.True(t, result) + s.NoError(err) + s.True(result) // Test match int to float user = entities.UserContext{ @@ -53,8 +63,8 @@ func TestGtMatcherInt(t *testing.T) { } result, err = matcher.Match(user) - assert.NoError(t, err) - assert.True(t, result) + s.NoError(err) + s.True(result) // Test no match user = entities.UserContext{ @@ -64,8 +74,8 @@ func TestGtMatcherInt(t *testing.T) { } result, err = matcher.Match(user) - assert.NoError(t, err) - assert.False(t, result) + s.NoError(err) + s.False(result) // Test no match user = entities.UserContext{ @@ -75,8 +85,8 @@ func TestGtMatcherInt(t *testing.T) { } result, err = matcher.Match(user) - assert.NoError(t, err) - assert.False(t, result) + s.NoError(err) + s.False(result) // Test attribute not found user = entities.UserContext{ @@ -84,14 +94,26 @@ func TestGtMatcherInt(t *testing.T) { "int_43": 42, }, } - + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "int_42")) _, err = matcher.Match(user) - assert.Error(t, err) + s.Error(err) + + // Test attribute of different type + user = entities.UserContext{ + Attributes: map[string]interface{}{ + "int_42": true, + }, + } + s.mockLogger.On("Warning", fmt.Sprintf(string(logging.InvalidAttributeValueType), "", true, "int_42")) + result, err = matcher.Match(user) + s.Error(err) + s.False(result) + s.mockLogger.AssertExpectations(s.T()) } -func TestGtMatcherFloat(t *testing.T) { +func (s *GtTestSuite) TestGtMatcherFloat() { matcher := GtMatcher{ - Logger: logging.GetLogger("", ""), + Logger: s.mockLogger, Condition: entities.Condition{ Match: "gt", Value: 4.2, @@ -106,8 +128,8 @@ func TestGtMatcherFloat(t *testing.T) { }, } result, err := matcher.Match(user) - assert.NoError(t, err) - assert.True(t, result) + s.NoError(err) + s.True(result) // Test match user = entities.UserContext{ @@ -116,8 +138,8 @@ func TestGtMatcherFloat(t *testing.T) { }, } result, err = matcher.Match(user) - assert.NoError(t, err) - assert.True(t, result) + s.NoError(err) + s.True(result) // Test no match user = entities.UserContext{ @@ -127,8 +149,8 @@ func TestGtMatcherFloat(t *testing.T) { } result, err = matcher.Match(user) - assert.NoError(t, err) - assert.False(t, result) + s.NoError(err) + s.False(result) // Test attribute not found user = entities.UserContext{ @@ -137,6 +159,46 @@ func TestGtMatcherFloat(t *testing.T) { }, } + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "float_4_2")) _, err = matcher.Match(user) - assert.Error(t, err) + s.Error(err) + + // Test attribute of different type + user = entities.UserContext{ + Attributes: map[string]interface{}{ + "float_4_2": true, + }, + } + s.mockLogger.On("Warning", fmt.Sprintf(string(logging.InvalidAttributeValueType), "", true, "float_4_2")) + result, err = matcher.Match(user) + s.Error(err) + s.False(result) + s.mockLogger.AssertExpectations(s.T()) +} + +func (s *GtTestSuite) TestGtMatcherUnsupportedConditionValue() { + matcher := GtMatcher{ + Logger: s.mockLogger, + Condition: entities.Condition{ + Match: "gt", + Value: false, + Name: "float_4_2", + }, + } + + // Test match - unsupported condition value + user := entities.UserContext{ + Attributes: map[string]interface{}{ + "float_4_2": 4.2, + }, + } + s.mockLogger.On("Warning", fmt.Sprintf(string(logging.UnsupportedConditionValue), "")) + result, err := matcher.Match(user) + s.Error(err) + s.False(result) + s.mockLogger.AssertExpectations(s.T()) +} + +func TestGtTestSuite(t *testing.T) { + suite.Run(t, new(GtTestSuite)) } diff --git a/pkg/decision/evaluator/matchers/lt.go b/pkg/decision/evaluator/matchers/lt.go index 4ede415df..03f2a04ee 100644 --- a/pkg/decision/evaluator/matchers/lt.go +++ b/pkg/decision/evaluator/matchers/lt.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -35,19 +35,20 @@ type LtMatcher struct { func (m LtMatcher) Match(user entities.UserContext) (bool, error) { if !user.CheckAttributeExists(m.Condition.Name) { - m.Logger.Debug(fmt.Sprintf(`Audience condition %s evaluated to UNKNOWN because a null value was passed for user attribute "%s".`, m.Condition.StringRepresentation, m.Condition)) + m.Logger.Debug(fmt.Sprintf(string(logging.NullUserAttribute), m.Condition.StringRepresentation, m.Condition.Name)) return false, fmt.Errorf(`no attribute named "%s"`, m.Condition.Name) } if floatValue, ok := utils.ToFloat(m.Condition.Value); ok { attributeValue, err := user.GetFloatAttribute(m.Condition.Name) if err != nil { - m.Logger.Warning(fmt.Sprintf(`Audience condition "%s" evaluated to UNKNOWN because a value of type "%s" was passed for user attribute "%s".`, m.Condition.StringRepresentation, m.Condition.Value, m.Condition.Name)) + val, _ := user.GetAttribute(m.Condition.Name) + m.Logger.Warning(fmt.Sprintf(string(logging.InvalidAttributeValueType), m.Condition.StringRepresentation, val, m.Condition.Name)) return false, err } return floatValue > attributeValue, nil } - m.Logger.Warning(fmt.Sprintf(`Audience condition "%s" has an unsupported condition value. You may need to upgrade to a newer release of the Optimizely SDK.`, m.Condition.StringRepresentation)) + m.Logger.Warning(fmt.Sprintf(string(logging.UnsupportedConditionValue), m.Condition.StringRepresentation)) return false, fmt.Errorf("audience condition %s evaluated to NULL because the condition value type is not supported", m.Condition.Name) } diff --git a/pkg/decision/evaluator/matchers/lt_test.go b/pkg/decision/evaluator/matchers/lt_test.go index 06cbd0f6e..350b97ef5 100644 --- a/pkg/decision/evaluator/matchers/lt_test.go +++ b/pkg/decision/evaluator/matchers/lt_test.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -17,17 +17,27 @@ package matchers import ( + "fmt" "testing" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/suite" "github.com/optimizely/go-sdk/pkg/entities" "github.com/optimizely/go-sdk/pkg/logging" ) -func TestLtMatcherInt(t *testing.T) { +type LtTestSuite struct { + suite.Suite + mockLogger *MockLogger +} + +func (s *LtTestSuite) SetupTest() { + s.mockLogger = new(MockLogger) +} + +func (s *LtTestSuite) TestLtMatcherInt() { matcher := LtMatcher{ - Logger: logging.GetLogger("", ""), + Logger: s.mockLogger, Condition: entities.Condition{ Match: "lt", Value: 42, @@ -42,8 +52,8 @@ func TestLtMatcherInt(t *testing.T) { }, } result, err := matcher.Match(user) - assert.NoError(t, err) - assert.True(t, result) + s.NoError(err) + s.True(result) // Test match int to float user = entities.UserContext{ @@ -53,8 +63,8 @@ func TestLtMatcherInt(t *testing.T) { } result, err = matcher.Match(user) - assert.NoError(t, err) - assert.True(t, result) + s.NoError(err) + s.True(result) // Test no match user = entities.UserContext{ @@ -64,8 +74,8 @@ func TestLtMatcherInt(t *testing.T) { } result, err = matcher.Match(user) - assert.NoError(t, err) - assert.False(t, result) + s.NoError(err) + s.False(result) // Test no match user = entities.UserContext{ @@ -75,8 +85,8 @@ func TestLtMatcherInt(t *testing.T) { } result, err = matcher.Match(user) - assert.NoError(t, err) - assert.False(t, result) + s.NoError(err) + s.False(result) // Test attribute not found user = entities.UserContext{ @@ -85,13 +95,26 @@ func TestLtMatcherInt(t *testing.T) { }, } + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "int_42")) _, err = matcher.Match(user) - assert.Error(t, err) + s.Error(err) + + // Test attribute of different type + user = entities.UserContext{ + Attributes: map[string]interface{}{ + "int_42": true, + }, + } + s.mockLogger.On("Warning", fmt.Sprintf(string(logging.InvalidAttributeValueType), "", true, "int_42")) + result, err = matcher.Match(user) + s.Error(err) + s.False(result) + s.mockLogger.AssertExpectations(s.T()) } -func TestLtMatcherFloat(t *testing.T) { +func (s *LtTestSuite) TestLtMatcherFloat() { matcher := LtMatcher{ - Logger: logging.GetLogger("", ""), + Logger: s.mockLogger, Condition: entities.Condition{ Match: "lt", Value: 4.2, @@ -106,8 +129,8 @@ func TestLtMatcherFloat(t *testing.T) { }, } result, err := matcher.Match(user) - assert.NoError(t, err) - assert.True(t, result) + s.NoError(err) + s.True(result) // Test match user = entities.UserContext{ @@ -116,8 +139,8 @@ func TestLtMatcherFloat(t *testing.T) { }, } result, err = matcher.Match(user) - assert.NoError(t, err) - assert.True(t, result) + s.NoError(err) + s.True(result) // Test no match user = entities.UserContext{ @@ -127,8 +150,8 @@ func TestLtMatcherFloat(t *testing.T) { } result, err = matcher.Match(user) - assert.NoError(t, err) - assert.False(t, result) + s.NoError(err) + s.False(result) // Test attribute not found user = entities.UserContext{ @@ -137,6 +160,46 @@ func TestLtMatcherFloat(t *testing.T) { }, } + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "float_4_2")) _, err = matcher.Match(user) - assert.Error(t, err) + s.Error(err) + + // Test attribute of different type + user = entities.UserContext{ + Attributes: map[string]interface{}{ + "float_4_2": true, + }, + } + s.mockLogger.On("Warning", fmt.Sprintf(string(logging.InvalidAttributeValueType), "", true, "float_4_2")) + result, err = matcher.Match(user) + s.Error(err) + s.False(result) + s.mockLogger.AssertExpectations(s.T()) +} + +func (s *LtTestSuite) TestLtMatcherUnsupportedConditionValue() { + matcher := LtMatcher{ + Logger: s.mockLogger, + Condition: entities.Condition{ + Match: "lt", + Value: false, + Name: "float_4_2", + }, + } + + // Test match - unsupported condition value + user := entities.UserContext{ + Attributes: map[string]interface{}{ + "float_4_2": 4.2, + }, + } + s.mockLogger.On("Warning", fmt.Sprintf(string(logging.UnsupportedConditionValue), "")) + result, err := matcher.Match(user) + s.Error(err) + s.False(result) + s.mockLogger.AssertExpectations(s.T()) +} + +func TestLtTestSuite(t *testing.T) { + suite.Run(t, new(LtTestSuite)) } diff --git a/pkg/decision/evaluator/matchers/substring.go b/pkg/decision/evaluator/matchers/substring.go index 841626252..b9a9aa1af 100644 --- a/pkg/decision/evaluator/matchers/substring.go +++ b/pkg/decision/evaluator/matchers/substring.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -35,19 +35,20 @@ type SubstringMatcher struct { func (m SubstringMatcher) Match(user entities.UserContext) (bool, error) { if !user.CheckAttributeExists(m.Condition.Name) { - m.Logger.Debug(fmt.Sprintf(`Audience condition %s evaluated to UNKNOWN because a null value was passed for user attribute "%s".`, m.Condition.StringRepresentation, m.Condition)) + m.Logger.Debug(fmt.Sprintf(string(logging.NullUserAttribute), m.Condition.StringRepresentation, m.Condition.Name)) return false, fmt.Errorf(`no attribute named "%s"`, m.Condition.Name) } if stringValue, ok := m.Condition.Value.(string); ok { attributeValue, err := user.GetStringAttribute(m.Condition.Name) if err != nil { - m.Logger.Warning(fmt.Sprintf(`Audience condition "%s" evaluated to UNKNOWN because a value of type "%s" was passed for user attribute "%s".`, m.Condition.StringRepresentation, m.Condition.Value, m.Condition.Name)) + val, _ := user.GetAttribute(m.Condition.Name) + m.Logger.Warning(fmt.Sprintf(string(logging.InvalidAttributeValueType), m.Condition.StringRepresentation, val, m.Condition.Name)) return false, err } return strings.Contains(attributeValue, stringValue), nil } - m.Logger.Warning(fmt.Sprintf(`Audience condition "%s" has an unsupported condition value. You may need to upgrade to a newer release of the Optimizely SDK.`, m.Condition.StringRepresentation)) + m.Logger.Warning(fmt.Sprintf(string(logging.UnsupportedConditionValue), m.Condition.StringRepresentation)) return false, fmt.Errorf("audience condition %s evaluated to NULL because the condition value type is not supported", m.Condition.Name) } diff --git a/pkg/decision/evaluator/matchers/substring_test.go b/pkg/decision/evaluator/matchers/substring_test.go index b06c17a81..3a00b266f 100644 --- a/pkg/decision/evaluator/matchers/substring_test.go +++ b/pkg/decision/evaluator/matchers/substring_test.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -17,22 +17,32 @@ package matchers import ( + "fmt" "testing" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/suite" "github.com/optimizely/go-sdk/pkg/entities" "github.com/optimizely/go-sdk/pkg/logging" ) -func TestSubstringMatcher(t *testing.T) { +type SubstringTestSuite struct { + suite.Suite + mockLogger *MockLogger +} + +func (s *SubstringTestSuite) SetupTest() { + s.mockLogger = new(MockLogger) +} + +func (s *SubstringTestSuite) TestSubstringMatcher() { matcher := SubstringMatcher{ Condition: entities.Condition{ Match: "substring", Value: "foo", Name: "string_foo", }, - Logger: logging.GetLogger("", ""), + Logger: s.mockLogger, } // Test match @@ -43,8 +53,8 @@ func TestSubstringMatcher(t *testing.T) { } result, err := matcher.Match(user) - assert.NoError(t, err) - assert.True(t, result) + s.NoError(err) + s.True(result) // Test no match user = entities.UserContext{ @@ -54,8 +64,8 @@ func TestSubstringMatcher(t *testing.T) { } result, err = matcher.Match(user) - assert.NoError(t, err) - assert.False(t, result) + s.NoError(err) + s.False(result) // Test attribute not found user = entities.UserContext{ @@ -64,6 +74,46 @@ func TestSubstringMatcher(t *testing.T) { }, } + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "string_foo")) _, err = matcher.Match(user) - assert.Error(t, err) + s.Error(err) + + // Test attribute of different type + user = entities.UserContext{ + Attributes: map[string]interface{}{ + "string_foo": true, + }, + } + s.mockLogger.On("Warning", fmt.Sprintf(string(logging.InvalidAttributeValueType), "", true, "string_foo")) + result, err = matcher.Match(user) + s.Error(err) + s.False(result) + s.mockLogger.AssertExpectations(s.T()) +} + +func (s *GtTestSuite) TestSubstringMatcherUnsupportedConditionValue() { + matcher := SubstringMatcher{ + Condition: entities.Condition{ + Match: "substring", + Value: false, + Name: "string_foo", + }, + Logger: s.mockLogger, + } + + // Test match - unsupported condition value + user := entities.UserContext{ + Attributes: map[string]interface{}{ + "string_foo": "foo", + }, + } + s.mockLogger.On("Warning", fmt.Sprintf(string(logging.UnsupportedConditionValue), "")) + result, err := matcher.Match(user) + s.Error(err) + s.False(result) + s.mockLogger.AssertExpectations(s.T()) +} + +func TestSubstringTestSuite(t *testing.T) { + suite.Run(t, new(SubstringTestSuite)) } diff --git a/pkg/decision/experiment_bucketer_service.go b/pkg/decision/experiment_bucketer_service.go index c02c14208..46347d91c 100644 --- a/pkg/decision/experiment_bucketer_service.go +++ b/pkg/decision/experiment_bucketer_service.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -52,16 +52,16 @@ func (s ExperimentBucketerService) GetDecision(decisionContext ExperimentDecisio // Determine if user can be part of the experiment if experiment.AudienceConditionTree != nil { condTreeParams := entities.NewTreeParameters(&userContext, decisionContext.ProjectConfig.GetAudienceMap()) - s.logger.Debug(fmt.Sprintf(`Evaluating audiences for experiment "%s".`, experiment.Key)) + s.logger.Debug(fmt.Sprintf(string(logging.EvaluatingAudiencesForExperiment), experiment.Key)) evalResult, _ := s.audienceTreeEvaluator.Evaluate(experiment.AudienceConditionTree, condTreeParams) - s.logger.Info(fmt.Sprintf(`Audiences for rule %s collectively evaluated to %t.`, experiment.Key, evalResult)) + s.logger.Info(fmt.Sprintf(string(logging.ExperimentAudiencesEvaluatedTo), experiment.Key, evalResult)) if !evalResult { - s.logger.Info(fmt.Sprintf(`User "%s" does not meet conditions to be in experiment "%s".`, userContext.ID, experiment.Key)) + s.logger.Info(fmt.Sprintf(string(logging.UserNotInExperiment), userContext.ID, experiment.Key)) experimentDecision.Reason = reasons.FailedAudienceTargeting return experimentDecision, nil } } else { - s.logger.Info(fmt.Sprintf(`Audiences for experiment "%s" collectively evaluated to TRUE.`, experiment.Key)) + s.logger.Info(fmt.Sprintf(string(logging.ExperimentAudiencesEvaluatedTo), experiment.Key, true)) } var group entities.Group diff --git a/pkg/decision/experiment_bucketer_service_test.go b/pkg/decision/experiment_bucketer_service_test.go index ed5c2a815..b59b2a0ad 100644 --- a/pkg/decision/experiment_bucketer_service_test.go +++ b/pkg/decision/experiment_bucketer_service_test.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -17,9 +17,11 @@ package decision import ( + "fmt" "testing" "github.com/optimizely/go-sdk/pkg/decision/reasons" + "github.com/optimizely/go-sdk/pkg/logging" "github.com/optimizely/go-sdk/pkg/entities" "github.com/stretchr/testify/mock" @@ -48,9 +50,11 @@ func (m *MockLogger) Info(message string) { } func (m *MockLogger) Warning(message string) { + m.Called(message) } func (m *MockLogger) Error(message string, err interface{}) { + m.Called(message, err) } type ExperimentBucketerTestSuite struct { @@ -83,7 +87,7 @@ func (s *ExperimentBucketerTestSuite) TestGetDecisionNoTargeting() { ProjectConfig: s.mockConfig, } s.mockBucketer.On("Bucket", testUserContext.ID, testExp1111, entities.Group{}).Return(&testExp1111Var2222, reasons.BucketedIntoVariation, nil) - s.mockLogger.On("Info", `Audiences for experiment "test_experiment_1111" collectively evaluated to TRUE.`) + s.mockLogger.On("Info", fmt.Sprintf(string(logging.ExperimentAudiencesEvaluatedTo), "test_experiment_1111", true)) experimentBucketerService := ExperimentBucketerService{ bucketer: s.mockBucketer, logger: s.mockLogger, @@ -91,6 +95,7 @@ func (s *ExperimentBucketerTestSuite) TestGetDecisionNoTargeting() { decision, err := experimentBucketerService.GetDecision(testDecisionContext, testUserContext) s.Equal(expectedDecision, decision) s.NoError(err) + s.mockLogger.AssertExpectations(s.T()) } func (s *ExperimentBucketerTestSuite) TestGetDecisionWithTargetingPasses() { @@ -114,8 +119,8 @@ func (s *ExperimentBucketerTestSuite) TestGetDecisionWithTargetingPasses() { bucketer: s.mockBucketer, } s.mockConfig.On("GetAudienceMap").Return(map[string]entities.Audience{}) - s.mockLogger.On("Debug", `Evaluating audiences for experiment "test_targeted_experiment_1116".`) - s.mockLogger.On("Info", `Audiences for rule test_targeted_experiment_1116 collectively evaluated to true.`) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForExperiment), "test_targeted_experiment_1116")) + s.mockLogger.On("Info", fmt.Sprintf(string(logging.ExperimentAudiencesEvaluatedTo), "test_targeted_experiment_1116", true)) testDecisionContext := ExperimentDecisionContext{ Experiment: &testTargetedExp1116, @@ -124,6 +129,7 @@ func (s *ExperimentBucketerTestSuite) TestGetDecisionWithTargetingPasses() { decision, err := experimentBucketerService.GetDecision(testDecisionContext, testUserContext) s.Equal(expectedDecision, decision) s.NoError(err) + s.mockLogger.AssertExpectations(s.T()) } func (s *ExperimentBucketerTestSuite) TestGetDecisionWithTargetingFails() { @@ -144,9 +150,9 @@ func (s *ExperimentBucketerTestSuite) TestGetDecisionWithTargetingFails() { bucketer: s.mockBucketer, } s.mockConfig.On("GetAudienceMap").Return(map[string]entities.Audience{}) - s.mockLogger.On("Debug", `Evaluating audiences for experiment "test_targeted_experiment_1116".`) - s.mockLogger.On("Info", `Audiences for rule test_targeted_experiment_1116 collectively evaluated to false.`) - s.mockLogger.On("Info", `User "test_user_1" does not meet conditions to be in experiment "test_targeted_experiment_1116".`) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForExperiment), "test_targeted_experiment_1116")) + s.mockLogger.On("Info", fmt.Sprintf(string(logging.ExperimentAudiencesEvaluatedTo), "test_targeted_experiment_1116", false)) + s.mockLogger.On("Info", fmt.Sprintf(string(logging.UserNotInExperiment), "test_user_1", "test_targeted_experiment_1116")) testDecisionContext := ExperimentDecisionContext{ Experiment: &testTargetedExp1116, @@ -156,6 +162,8 @@ func (s *ExperimentBucketerTestSuite) TestGetDecisionWithTargetingFails() { s.Equal(expectedDecision, decision) s.NoError(err) s.mockBucketer.AssertNotCalled(s.T(), "Bucket") + s.mockLogger.AssertExpectations(s.T()) + } func TestExperimentBucketerTestSuite(t *testing.T) { diff --git a/pkg/decision/helpers_test.go b/pkg/decision/helpers_test.go index 7a1499050..48f78163a 100644 --- a/pkg/decision/helpers_test.go +++ b/pkg/decision/helpers_test.go @@ -108,7 +108,7 @@ var testExp1111 = entities.Experiment{ "2222": "2222", }, TrafficAllocation: []entities.Range{ - entities.Range{EntityID: "2222", EndOfRange: 10000}, + {EntityID: "2222", EndOfRange: 10000}, }, } @@ -135,7 +135,7 @@ var testExp1112 = entities.Experiment{ AudienceConditionTree: &entities.TreeNode{ Operator: "and", Nodes: []*entities.TreeNode{ - &entities.TreeNode{Item: "test_audience_5555"}, + {Item: "test_audience_5555"}, }, }, ID: "1112", @@ -147,7 +147,7 @@ var testExp1112 = entities.Experiment{ "2222": "2222", }, TrafficAllocation: []entities.Range{ - entities.Range{EntityID: "2222", EndOfRange: 10000}, + {EntityID: "2222", EndOfRange: 10000}, }, } var testExp1117Var2223 = entities.Variation{ID: "2223", Key: "2223"} @@ -156,7 +156,7 @@ var testExp1117 = entities.Experiment{ AudienceConditionTree: &entities.TreeNode{ Operator: "and", Nodes: []*entities.TreeNode{ - &entities.TreeNode{Item: "test_audience_5556"}, + {Item: "test_audience_5556"}, }, }, ID: "1117", @@ -168,7 +168,7 @@ var testExp1117 = entities.Experiment{ "2223": "2223", }, TrafficAllocation: []entities.Range{ - entities.Range{EntityID: "2223", EndOfRange: 10000}, + {EntityID: "2223", EndOfRange: 10000}, }, } var testExp1118Var2224 = entities.Variation{ID: "2224", Key: "2224"} @@ -177,7 +177,7 @@ var testExp1118 = entities.Experiment{ AudienceConditionTree: &entities.TreeNode{ Operator: "and", Nodes: []*entities.TreeNode{ - &entities.TreeNode{Item: "test_audience_5557"}, + {Item: "test_audience_5557"}, }, }, ID: "1118", @@ -189,7 +189,7 @@ var testExp1118 = entities.Experiment{ "2224": "2224", }, TrafficAllocation: []entities.Range{ - entities.Range{EntityID: "2224", EndOfRange: 10000}, + {EntityID: "2224", EndOfRange: 10000}, }, } @@ -225,8 +225,8 @@ var testExp1113 = entities.Experiment{ "2224": "2224", }, TrafficAllocation: []entities.Range{ - entities.Range{EntityID: "2223", EndOfRange: 5000}, - entities.Range{EntityID: "2224", EndOfRange: 10000}, + {EntityID: "2223", EndOfRange: 5000}, + {EntityID: "2224", EndOfRange: 10000}, }, } @@ -247,16 +247,16 @@ var testExp1114 = entities.Experiment{ "2226": "2226", }, TrafficAllocation: []entities.Range{ - entities.Range{EntityID: "2225", EndOfRange: 5000}, - entities.Range{EntityID: "2226", EndOfRange: 10000}, + {EntityID: "2225", EndOfRange: 5000}, + {EntityID: "2226", EndOfRange: 10000}, }, } var testGroup6666 = entities.Group{ ID: "6666", Policy: "random", TrafficAllocation: []entities.Range{ - entities.Range{EntityID: "1113", EndOfRange: 3000}, - entities.Range{EntityID: "1114", EndOfRange: 6000}, + {EntityID: "1113", EndOfRange: 3000}, + {EntityID: "1114", EndOfRange: 6000}, }, } @@ -274,7 +274,7 @@ var testExp1115 = entities.Experiment{ "2227": "2227", }, TrafficAllocation: []entities.Range{ - entities.Range{EntityID: "2227", EndOfRange: 5000}, + {EntityID: "2227", EndOfRange: 5000}, }, } var testFeat3335 = entities.Feature{ @@ -302,7 +302,7 @@ var testTargetedExp1116 = entities.Experiment{ "2228": "2228", }, TrafficAllocation: []entities.Range{ - entities.Range{EntityID: "2228", EndOfRange: 10000}, + {EntityID: "2228", EndOfRange: 10000}, }, } @@ -320,7 +320,7 @@ var testExpWhitelist = entities.Experiment{ "var_2229": "2229", }, TrafficAllocation: []entities.Range{ - entities.Range{EntityID: "2229", EndOfRange: 10000}, + {EntityID: "2229", EndOfRange: 10000}, }, Whitelist: map[string]string{ "test_user_1": "var_2229", diff --git a/pkg/decision/rollout_service.go b/pkg/decision/rollout_service.go index 451955ea3..d01404788 100644 --- a/pkg/decision/rollout_service.go +++ b/pkg/decision/rollout_service.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -19,6 +19,7 @@ package decision import ( "fmt" + "strconv" "github.com/optimizely/go-sdk/pkg/decision/evaluator" "github.com/optimizely/go-sdk/pkg/decision/reasons" @@ -54,7 +55,7 @@ func (r RolloutService) GetDecision(decisionContext FeatureDecisionContext, user evaluateConditionTree := func(experiment *entities.Experiment, loggingKey string) bool { condTreeParams := entities.NewTreeParameters(&userContext, decisionContext.ProjectConfig.GetAudienceMap()) - r.logger.Debug(fmt.Sprintf(`Evaluating audiences for rule %s.`, loggingKey)) + r.logger.Debug(fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), loggingKey)) evalResult, _ := r.audienceTreeEvaluator.Evaluate(experiment.AudienceConditionTree, condTreeParams) if !evalResult { featureDecision.Reason = reasons.FailedRolloutTargeting @@ -98,15 +99,15 @@ func (r RolloutService) GetDecision(decisionContext FeatureDecisionContext, user } for index := 0; index < numberOfExperiments-1; index++ { - loggingKey := string(index + 1) + loggingKey := strconv.Itoa(index + 1) experiment := &rollout.Experiments[index] experimentDecisionContext := getExperimentDecisionContext(experiment) // Move to next evaluation if condition tree is available and evaluation fails evaluationResult := experiment.AudienceConditionTree == nil || evaluateConditionTree(experiment, loggingKey) - r.logger.Info(fmt.Sprintf(`Audiences for rule %s collectively evaluated to %t.`, loggingKey, evaluationResult)) + r.logger.Info(fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), loggingKey, evaluationResult)) if !evaluationResult { - r.logger.Debug(fmt.Sprintf(`User "%s" does not meet conditions for targeting rule %s.`, userContext.ID, loggingKey)) + r.logger.Debug(fmt.Sprintf(string(logging.UserNotInRollout), userContext.ID, loggingKey)) // Evaluate this user for the next rule continue } @@ -124,12 +125,13 @@ func (r RolloutService) GetDecision(decisionContext FeatureDecisionContext, user experimentDecisionContext := getExperimentDecisionContext(experiment) // Move to bucketing if conditionTree is unavailable or evaluation passes evaluationResult := experiment.AudienceConditionTree == nil || evaluateConditionTree(experiment, "Everyone Else") - r.logger.Info(fmt.Sprintf(`Audiences for rule %s collectively evaluated to %t.`, "Everyone Else", evaluationResult)) + r.logger.Info(fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "Everyone Else", evaluationResult)) if evaluationResult { decision, err := r.experimentBucketerService.GetDecision(experimentDecisionContext, userContext) if err == nil { - r.logger.Debug(fmt.Sprintf(`User "%s" meets conditions for targeting rule "Everyone Else".`, userContext.ID)) + + r.logger.Debug(fmt.Sprintf(string(logging.UserInEveryoneElse), userContext.ID)) } return getFeatureDecision(experiment, &decision) } diff --git a/pkg/decision/rollout_service_test.go b/pkg/decision/rollout_service_test.go index 48fe33bdd..1da26681c 100644 --- a/pkg/decision/rollout_service_test.go +++ b/pkg/decision/rollout_service_test.go @@ -17,6 +17,7 @@ package decision import ( + "fmt" "testing" "github.com/optimizely/go-sdk/pkg/decision/evaluator" @@ -37,6 +38,7 @@ type RolloutServiceTestSuite struct { testFeatureDecisionContext FeatureDecisionContext testConditionTreeParams *entities.TreeParameters testUserContext entities.UserContext + mockLogger *MockLogger } func (s *RolloutServiceTestSuite) SetupTest() { @@ -62,11 +64,14 @@ func (s *RolloutServiceTestSuite) SetupTest() { } s.testConditionTreeParams = entities.NewTreeParameters(&s.testUserContext, testAudienceMap) s.mockConfig.On("GetAudienceMap").Return(testAudienceMap) + s.mockLogger = new(MockLogger) } func (s *RolloutServiceTestSuite) TestGetDecisionWithEmptyRolloutID() { - testRolloutService := RolloutService{} + testRolloutService := RolloutService{ + logger: s.mockLogger, + } feature := testFeatRollout3334 feature.Rollout.ID = "" featureDecisionContext := FeatureDecisionContext{ @@ -83,7 +88,9 @@ func (s *RolloutServiceTestSuite) TestGetDecisionWithEmptyRolloutID() { func (s *RolloutServiceTestSuite) TestGetDecisionWithNoExperiments() { - testRolloutService := RolloutService{} + testRolloutService := RolloutService{ + logger: s.mockLogger, + } feature := testFeatRollout3334 feature.Rollout.Experiments = []entities.Experiment{} featureDecisionContext := FeatureDecisionContext{ @@ -110,7 +117,7 @@ func (s *RolloutServiceTestSuite) TestGetDecisionHappyPath() { testRolloutService := RolloutService{ audienceTreeEvaluator: s.mockAudienceTreeEvaluator, experimentBucketerService: s.mockExperimentService, - logger: logging.GetLogger("sdkKey", "RolloutService"), + logger: s.mockLogger, } expectedFeatureDecision := FeatureDecision{ Experiment: testExp1112, @@ -118,10 +125,14 @@ func (s *RolloutServiceTestSuite) TestGetDecisionHappyPath() { Source: Rollout, Decision: Decision{Reason: reasons.BucketedIntoRollout}, } + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), "1")) + s.mockLogger.On("Info", fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "1", true)) + s.mockLogger.On("Debug", `Decision made for user "test_user" for feature rollout with key "test_feature_rollout_3334_key": Bucketed into feature rollout.`) decision, _ := testRolloutService.GetDecision(s.testFeatureDecisionContext, s.testUserContext) s.Equal(expectedFeatureDecision, decision) s.mockAudienceTreeEvaluator.AssertExpectations(s.T()) s.mockExperimentService.AssertExpectations(s.T()) + s.mockLogger.AssertExpectations(s.T()) } func (s *RolloutServiceTestSuite) TestGetDecisionFallbacksToLastWhenFailsBucketing() { @@ -146,7 +157,7 @@ func (s *RolloutServiceTestSuite) TestGetDecisionFallbacksToLastWhenFailsBucketi testRolloutService := RolloutService{ audienceTreeEvaluator: s.mockAudienceTreeEvaluator, experimentBucketerService: s.mockExperimentService, - logger: logging.GetLogger("sdkKey", "RolloutService"), + logger: s.mockLogger, } expectedFeatureDecision := FeatureDecision{ Experiment: testExp1118, @@ -154,10 +165,17 @@ func (s *RolloutServiceTestSuite) TestGetDecisionFallbacksToLastWhenFailsBucketi Source: Rollout, Decision: Decision{Reason: reasons.BucketedIntoRollout}, } + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), "1")) + s.mockLogger.On("Info", fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "1", true)) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), "Everyone Else")) + s.mockLogger.On("Info", fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "Everyone Else", true)) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.UserInEveryoneElse), "test_user")) + s.mockLogger.On("Debug", `Decision made for user "test_user" for feature rollout with key "test_feature_rollout_3334_key": Bucketed into feature rollout.`) decision, _ := testRolloutService.GetDecision(s.testFeatureDecisionContext, s.testUserContext) s.Equal(expectedFeatureDecision, decision) s.mockAudienceTreeEvaluator.AssertExpectations(s.T()) s.mockExperimentService.AssertExpectations(s.T()) + s.mockLogger.AssertExpectations(s.T()) } func (s *RolloutServiceTestSuite) TestGetDecisionWhenFallbackBucketingFails() { @@ -178,17 +196,24 @@ func (s *RolloutServiceTestSuite) TestGetDecisionWhenFallbackBucketingFails() { testRolloutService := RolloutService{ audienceTreeEvaluator: s.mockAudienceTreeEvaluator, experimentBucketerService: s.mockExperimentService, - logger: logging.GetLogger("sdkKey", "RolloutService"), + logger: s.mockLogger, } expectedFeatureDecision := FeatureDecision{ Experiment: testExp1118, Source: Rollout, Decision: Decision{Reason: reasons.FailedRolloutBucketing}, } + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), "1")) + s.mockLogger.On("Info", fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "1", true)) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), "Everyone Else")) + s.mockLogger.On("Info", fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "Everyone Else", true)) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.UserInEveryoneElse), "test_user")) + s.mockLogger.On("Debug", `Decision made for user "test_user" for feature rollout with key "test_feature_rollout_3334_key": Not bucketed into rollout.`) decision, _ := testRolloutService.GetDecision(s.testFeatureDecisionContext, s.testUserContext) s.Equal(expectedFeatureDecision, decision) s.mockAudienceTreeEvaluator.AssertExpectations(s.T()) s.mockExperimentService.AssertExpectations(s.T()) + s.mockLogger.AssertExpectations(s.T()) } func (s *RolloutServiceTestSuite) TestEvaluatesNextIfPreviousTargetingFails() { @@ -207,7 +232,7 @@ func (s *RolloutServiceTestSuite) TestEvaluatesNextIfPreviousTargetingFails() { testRolloutService := RolloutService{ audienceTreeEvaluator: s.mockAudienceTreeEvaluator, experimentBucketerService: s.mockExperimentService, - logger: logging.GetLogger("sdkKey", "RolloutService"), + logger: s.mockLogger, } expectedFeatureDecision := FeatureDecision{ Experiment: testExp1117, @@ -215,10 +240,17 @@ func (s *RolloutServiceTestSuite) TestEvaluatesNextIfPreviousTargetingFails() { Source: Rollout, Decision: Decision{Reason: reasons.BucketedIntoRollout}, } + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), "1")) + s.mockLogger.On("Info", fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "1", false)) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.UserNotInRollout), "test_user", "1")) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), "2")) + s.mockLogger.On("Info", fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "2", true)) + s.mockLogger.On("Debug", `Decision made for user "test_user" for feature rollout with key "test_feature_rollout_3334_key": Bucketed into feature rollout.`) decision, _ := testRolloutService.GetDecision(s.testFeatureDecisionContext, s.testUserContext) s.Equal(expectedFeatureDecision, decision) s.mockAudienceTreeEvaluator.AssertExpectations(s.T()) s.mockExperimentService.AssertExpectations(s.T()) + s.mockLogger.AssertExpectations(s.T()) } func (s *RolloutServiceTestSuite) TestGetDecisionFailsTargeting() { @@ -228,7 +260,7 @@ func (s *RolloutServiceTestSuite) TestGetDecisionFailsTargeting() { testRolloutService := RolloutService{ audienceTreeEvaluator: s.mockAudienceTreeEvaluator, experimentBucketerService: s.mockExperimentService, - logger: logging.GetLogger("sdkKey", "RolloutService"), + logger: s.mockLogger, } expectedFeatureDecision := FeatureDecision{ Decision: Decision{ @@ -236,10 +268,19 @@ func (s *RolloutServiceTestSuite) TestGetDecisionFailsTargeting() { }, Source: Rollout, } + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), "1")) + s.mockLogger.On("Info", fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "1", false)) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.UserNotInRollout), "test_user", "1")) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), "2")) + s.mockLogger.On("Info", fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "2", false)) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.UserNotInRollout), "test_user", "2")) + s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), "Everyone Else")) + s.mockLogger.On("Info", fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "Everyone Else", false)) decision, _ := testRolloutService.GetDecision(s.testFeatureDecisionContext, s.testUserContext) s.Equal(expectedFeatureDecision, decision) s.mockAudienceTreeEvaluator.AssertExpectations(s.T()) s.mockExperimentService.AssertExpectations(s.T()) + s.mockLogger.AssertExpectations(s.T()) } func TestNewRolloutService(t *testing.T) { diff --git a/pkg/entities/audience.go b/pkg/entities/audience.go index 84682c330..df1b48de8 100644 --- a/pkg/entities/audience.go +++ b/pkg/entities/audience.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * diff --git a/pkg/entities/user_context.go b/pkg/entities/user_context.go index df681988e..fc44e8526 100644 --- a/pkg/entities/user_context.go +++ b/pkg/entities/user_context.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -88,6 +88,15 @@ func (u UserContext) GetIntAttribute(attrName string) (int64, error) { return 0, fmt.Errorf(`no int attribute named "%s"`, attrName) } +// GetAttribute returns the value for the specified attribute name in the attributes map. Returns error if not found. +func (u UserContext) GetAttribute(attrName string) (interface{}, error) { + if value, ok := u.Attributes[attrName]; ok { + return value, nil + } + + return 0, fmt.Errorf(`no attribute named "%s"`, attrName) +} + // GetBucketingID returns the bucketing ID to use for the given user func (u UserContext) GetBucketingID() (string, error) { // by default diff --git a/pkg/logging/log_messages.go b/pkg/logging/log_messages.go new file mode 100644 index 000000000..bf2b6d035 --- /dev/null +++ b/pkg/logging/log_messages.go @@ -0,0 +1,60 @@ +/**************************************************************************** + * Copyright 2019-2020, Optimizely, Inc. and contributors * + * * + * Licensed under the Apache License, Version 2.0 (the "License"); * + * you may not use this file except in compliance with the License. * + * You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, software * + * distributed under the License is distributed on an "AS IS" BASIS, * + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * + * See the License for the specific language governing permissions and * + * limitations under the License. * + ***************************************************************************/ + +// Package logging // +package logging + +// LogMessage defines string type for log messages +type LogMessage string + +const ( + // Debug Logs + + // AudienceEvaluationStarted when single audience evaluation is started + AudienceEvaluationStarted LogMessage = `Starting to evaluate audience "%s".` + // AudienceEvaluatedTo when single audience evaluation is completed + AudienceEvaluatedTo LogMessage = `Audience "%s" evaluated to %t.` + // EvaluatingAudiencesForExperiment when audience evaluation is started for an experiment + EvaluatingAudiencesForExperiment LogMessage = `Evaluating audiences for experiment "%s".` + // EvaluatingAudiencesForRollout when audience evaluation is started for a rule + EvaluatingAudiencesForRollout LogMessage = `Evaluating audiences for rule %s.".` + // NullUserAttribute when user attribute is missing or nil + NullUserAttribute LogMessage = `Audience condition %s evaluated to UNKNOWN because a null value was passed for user attribute "%s".` + // UserInEveryoneElse when user is in last rule + UserInEveryoneElse LogMessage = `User "%s" meets conditions for targeting rule "Everyone Else".` + // UserNotInRollout when user is not in rollout/rule + UserNotInRollout LogMessage = `User "%s" does not meet conditions for targeting rule %s.` + + // Info Logs + + // ExperimentAudiencesEvaluatedTo when collective audience evaluation for experiment is completed + ExperimentAudiencesEvaluatedTo LogMessage = `Audiences for experiment %s collectively evaluated to %t.` + // RolloutAudiencesEvaluatedTo when collective audience evaluation for rule is completed + RolloutAudiencesEvaluatedTo LogMessage = `Audiences for rule %s collectively evaluated to %t.` + // UserNotInExperiment when user is not in experiment + UserNotInExperiment LogMessage = `User "%s" does not meet conditions to be in experiment "%s".` + + // Warning logs + + // UnknownConditionType when when condition type is unknown + UnknownConditionType LogMessage = `Audience condition "%s" uses an unknown condition type. You may need to upgrade to a newer release of the Optimizely SDK.` + // UnknownMatchType when match type is unknown + UnknownMatchType LogMessage = `Audience condition "%s" uses an unknown match type. You may need to upgrade to a newer release of the Optimizely SDK.` + // UnsupportedConditionValue when condition value is unsupported + UnsupportedConditionValue LogMessage = `Audience condition "%s" has an unsupported condition value. You may need to upgrade to a newer release of the Optimizely SDK.` + // InvalidAttributeValueType when user attribute value is invalid + InvalidAttributeValueType LogMessage = `Audience condition "%s" evaluated to UNKNOWN because a value of type "%T" was passed for user attribute "%s".` +) From c66092e00a4d68e86e60781e027d7b83828b83c7 Mon Sep 17 00:00:00 2001 From: Yasir Ali Date: Tue, 14 Jul 2020 13:37:30 +0500 Subject: [PATCH 3/6] fixing code duplication. --- pkg/decision/evaluator/matchers/gt.go | 26 ++++++++++++++++---------- pkg/decision/evaluator/matchers/lt.go | 22 +--------------------- 2 files changed, 17 insertions(+), 31 deletions(-) diff --git a/pkg/decision/evaluator/matchers/gt.go b/pkg/decision/evaluator/matchers/gt.go index e21d77e1f..c83d794d5 100644 --- a/pkg/decision/evaluator/matchers/gt.go +++ b/pkg/decision/evaluator/matchers/gt.go @@ -33,22 +33,28 @@ type GtMatcher struct { // Match returns true if the user's attribute is greater than the condition's string value func (m GtMatcher) Match(user entities.UserContext) (bool, error) { + return matchGtOrLt(user, m.Condition, m.Logger, true) +} - if !user.CheckAttributeExists(m.Condition.Name) { - m.Logger.Debug(fmt.Sprintf(string(logging.NullUserAttribute), m.Condition.StringRepresentation, m.Condition.Name)) - return false, fmt.Errorf(`no attribute named "%s"`, m.Condition.Name) +func matchGtOrLt(user entities.UserContext, condition entities.Condition, logger logging.OptimizelyLogProducer, gtMatch bool) (bool, error) { + if !user.CheckAttributeExists(condition.Name) { + logger.Debug(fmt.Sprintf(string(logging.NullUserAttribute), condition.StringRepresentation, condition.Name)) + return false, fmt.Errorf(`no attribute named "%s"`, condition.Name) } - if floatValue, ok := utils.ToFloat(m.Condition.Value); ok { - attributeValue, err := user.GetFloatAttribute(m.Condition.Name) + if floatValue, ok := utils.ToFloat(condition.Value); ok { + attributeValue, err := user.GetFloatAttribute(condition.Name) if err != nil { - val, _ := user.GetAttribute(m.Condition.Name) - m.Logger.Warning(fmt.Sprintf(string(logging.InvalidAttributeValueType), m.Condition.StringRepresentation, val, m.Condition.Name)) + val, _ := user.GetAttribute(condition.Name) + logger.Warning(fmt.Sprintf(string(logging.InvalidAttributeValueType), condition.StringRepresentation, val, condition.Name)) return false, err } - return floatValue < attributeValue, nil + if gtMatch { + return floatValue < attributeValue, nil + } + return floatValue > attributeValue, nil } - m.Logger.Warning(fmt.Sprintf(string(logging.UnsupportedConditionValue), m.Condition.StringRepresentation)) - return false, fmt.Errorf("audience condition %s evaluated to NULL because the condition value type is not supported", m.Condition.Name) + logger.Warning(fmt.Sprintf(string(logging.UnsupportedConditionValue), condition.StringRepresentation)) + return false, fmt.Errorf("audience condition %s evaluated to NULL because the condition value type is not supported", condition.Name) } diff --git a/pkg/decision/evaluator/matchers/lt.go b/pkg/decision/evaluator/matchers/lt.go index 03f2a04ee..cd31d0c5c 100644 --- a/pkg/decision/evaluator/matchers/lt.go +++ b/pkg/decision/evaluator/matchers/lt.go @@ -18,9 +18,6 @@ package matchers import ( - "fmt" - - "github.com/optimizely/go-sdk/pkg/decision/evaluator/matchers/utils" "github.com/optimizely/go-sdk/pkg/entities" "github.com/optimizely/go-sdk/pkg/logging" ) @@ -33,22 +30,5 @@ type LtMatcher struct { // Match returns true if the user's attribute is less than the condition's string value func (m LtMatcher) Match(user entities.UserContext) (bool, error) { - - if !user.CheckAttributeExists(m.Condition.Name) { - m.Logger.Debug(fmt.Sprintf(string(logging.NullUserAttribute), m.Condition.StringRepresentation, m.Condition.Name)) - return false, fmt.Errorf(`no attribute named "%s"`, m.Condition.Name) - } - - if floatValue, ok := utils.ToFloat(m.Condition.Value); ok { - attributeValue, err := user.GetFloatAttribute(m.Condition.Name) - if err != nil { - val, _ := user.GetAttribute(m.Condition.Name) - m.Logger.Warning(fmt.Sprintf(string(logging.InvalidAttributeValueType), m.Condition.StringRepresentation, val, m.Condition.Name)) - return false, err - } - return floatValue > attributeValue, nil - } - - m.Logger.Warning(fmt.Sprintf(string(logging.UnsupportedConditionValue), m.Condition.StringRepresentation)) - return false, fmt.Errorf("audience condition %s evaluated to NULL because the condition value type is not supported", m.Condition.Name) + return matchGtOrLt(user, m.Condition, m.Logger, false) } From 0f1a60d4990ae7879146e129ff77fff5d912caed Mon Sep 17 00:00:00 2001 From: Yasir Ali Date: Wed, 15 Jul 2020 11:55:35 +0500 Subject: [PATCH 4/6] string method added to LogMessage file. --- pkg/decision/evaluator/condition.go | 8 +-- pkg/decision/evaluator/condition_test.go | 4 +- pkg/decision/evaluator/condition_tree_test.go | 24 ++++----- pkg/decision/evaluator/matchers/exact.go | 10 ++-- pkg/decision/evaluator/matchers/exact_test.go | 18 +++---- pkg/decision/evaluator/matchers/gt.go | 6 +-- pkg/decision/evaluator/matchers/gt_test.go | 10 ++-- pkg/decision/evaluator/matchers/lt_test.go | 10 ++-- pkg/decision/evaluator/matchers/substring.go | 6 +-- .../evaluator/matchers/substring_test.go | 6 +-- pkg/decision/experiment_bucketer_service.go | 8 +-- .../experiment_bucketer_service_test.go | 12 ++--- pkg/decision/rollout_service.go | 10 ++-- pkg/decision/rollout_service_test.go | 50 +++++++++---------- pkg/logging/log_messages.go | 6 ++- 15 files changed, 96 insertions(+), 92 deletions(-) diff --git a/pkg/decision/evaluator/condition.go b/pkg/decision/evaluator/condition.go index 45e3fa396..e9e168f27 100644 --- a/pkg/decision/evaluator/condition.go +++ b/pkg/decision/evaluator/condition.go @@ -54,7 +54,7 @@ func (c CustomAttributeConditionEvaluator) Evaluate(condition entities.Condition if condition.Type != customAttributeType { - c.logger.Warning(fmt.Sprintf(string(logging.UnknownConditionType), condition.StringRepresentation)) + c.logger.Warning(fmt.Sprintf(logging.UnknownConditionType.String(), condition.StringRepresentation)) return false, fmt.Errorf(`unable to evaluate condition of type "%s"`, condition.Type) } @@ -89,7 +89,7 @@ func (c CustomAttributeConditionEvaluator) Evaluate(condition entities.Condition Logger: c.logger, } default: - c.logger.Warning(fmt.Sprintf(string(logging.UnknownMatchType), condition.StringRepresentation)) + c.logger.Warning(fmt.Sprintf(logging.UnknownMatchType.String(), condition.StringRepresentation)) return false, fmt.Errorf(`invalid Condition matcher "%s"`, condition.Match) } @@ -112,14 +112,14 @@ func NewAudienceConditionEvaluator(logger logging.OptimizelyLogProducer) *Audien func (c AudienceConditionEvaluator) Evaluate(audienceID string, condTreeParams *entities.TreeParameters) (bool, error) { if audience, ok := condTreeParams.AudienceMap[audienceID]; ok { - c.logger.Debug(fmt.Sprintf(string(logging.AudienceEvaluationStarted), audienceID)) + c.logger.Debug(fmt.Sprintf(logging.AudienceEvaluationStarted.String(), audienceID)) condTree := audience.ConditionTree conditionTreeEvaluator := NewMixedTreeEvaluator(c.logger) retValue, isValid := conditionTreeEvaluator.Evaluate(condTree, condTreeParams) if !isValid { return false, fmt.Errorf(`an error occurred while evaluating nested tree for audience ID "%s"`, audienceID) } - c.logger.Debug(fmt.Sprintf(string(logging.AudienceEvaluatedTo), audienceID, retValue)) + c.logger.Debug(fmt.Sprintf(logging.AudienceEvaluatedTo.String(), audienceID, retValue)) return retValue, nil } diff --git a/pkg/decision/evaluator/condition_test.go b/pkg/decision/evaluator/condition_test.go index c609bef12..f83706392 100644 --- a/pkg/decision/evaluator/condition_test.go +++ b/pkg/decision/evaluator/condition_test.go @@ -109,7 +109,7 @@ func (s *ConditionTestSuite) TestCustomAttributeConditionEvaluatorWithInvalidMat } condTreeParams := entities.NewTreeParameters(&user, map[string]entities.Audience{}) - s.mockLogger.On("Warning", fmt.Sprintf(string(logging.UnknownMatchType), "")) + s.mockLogger.On("Warning", fmt.Sprintf(logging.UnknownMatchType.String(), "")) result, _ := s.conditionEvaluator.Evaluate(condition, condTreeParams) s.Equal(result, false) s.mockLogger.AssertExpectations(s.T()) @@ -130,7 +130,7 @@ func (s *ConditionTestSuite) TestCustomAttributeConditionEvaluatorWithUnknownTyp } condTreeParams := entities.NewTreeParameters(&user, map[string]entities.Audience{}) - s.mockLogger.On("Warning", fmt.Sprintf(string(logging.UnknownConditionType), "")) + s.mockLogger.On("Warning", fmt.Sprintf(logging.UnknownConditionType.String(), "")) result, _ := s.conditionEvaluator.Evaluate(condition, condTreeParams) s.Equal(result, false) s.mockLogger.AssertExpectations(s.T()) diff --git a/pkg/decision/evaluator/condition_tree_test.go b/pkg/decision/evaluator/condition_tree_test.go index af19e0a7e..40fdd20fb 100644 --- a/pkg/decision/evaluator/condition_tree_test.go +++ b/pkg/decision/evaluator/condition_tree_test.go @@ -139,7 +139,7 @@ func (s *ConditionTreeTestSuite) TestConditionTreeEvaluateMultipleOrConditions() "bool_true": true, }, } - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "string_foo")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.NullUserAttribute.String(), "", "string_foo")) result, _ = s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) s.True(result) @@ -187,7 +187,7 @@ func (s *ConditionTreeTestSuite) TestConditionTreeEvaluateMultipleAndConditions( } condTreeParams := e.NewTreeParameters(&user, map[string]e.Audience{}) - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "bool_true")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.NullUserAttribute.String(), "", "bool_true")) result, _ := s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) s.False(result) @@ -198,7 +198,7 @@ func (s *ConditionTreeTestSuite) TestConditionTreeEvaluateMultipleAndConditions( }, } - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "string_foo")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.NullUserAttribute.String(), "", "string_foo")) result, _ = s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) s.False(result) @@ -265,7 +265,7 @@ func (s *ConditionTreeTestSuite) TestConditionTreeEvaluateNotCondition() { "bool_true": false, }, } - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "string_foo")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.NullUserAttribute.String(), "", "string_foo")) result, _ = s.conditionTreeEvaluator.Evaluate(conditionTree, condTreeParams) s.True(result) @@ -436,8 +436,8 @@ func (s *ConditionTreeTestSuite) TestConditionTreeEvaluateAnAudienceTreeSingleAu AudienceMap: audienceMap, } - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.AudienceEvaluationStarted), "11111")) - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.AudienceEvaluatedTo), "11111", true)) + s.mockLogger.On("Debug", fmt.Sprintf(logging.AudienceEvaluationStarted.String(), "11111")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.AudienceEvaluatedTo.String(), "11111", true)) result, _ := s.conditionTreeEvaluator.Evaluate(audienceTree, treeParams) s.True(result) s.mockLogger.AssertExpectations(s.T()) @@ -466,8 +466,8 @@ func (s *ConditionTreeTestSuite) TestConditionTreeEvaluateAnAudienceTreeMultiple }, AudienceMap: audienceMap, } - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.AudienceEvaluationStarted), "11111")) - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.AudienceEvaluatedTo), "11111", true)) + s.mockLogger.On("Debug", fmt.Sprintf(logging.AudienceEvaluationStarted.String(), "11111")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.AudienceEvaluatedTo.String(), "11111", true)) result, _ := s.conditionTreeEvaluator.Evaluate(audienceTree, treeParams) s.True(result) @@ -482,10 +482,10 @@ func (s *ConditionTreeTestSuite) TestConditionTreeEvaluateAnAudienceTreeMultiple }, AudienceMap: audienceMap, } - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.AudienceEvaluationStarted), "11111")) - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "string_foo")) - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.AudienceEvaluationStarted), "11112")) - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.AudienceEvaluatedTo), "11112", true)) + s.mockLogger.On("Debug", fmt.Sprintf(logging.AudienceEvaluationStarted.String(), "11111")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.NullUserAttribute.String(), "", "string_foo")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.AudienceEvaluationStarted.String(), "11112")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.AudienceEvaluatedTo.String(), "11112", true)) result, _ = s.conditionTreeEvaluator.Evaluate(audienceTree, treeParams) s.True(result) diff --git a/pkg/decision/evaluator/matchers/exact.go b/pkg/decision/evaluator/matchers/exact.go index f789b446d..b5c91cc7a 100644 --- a/pkg/decision/evaluator/matchers/exact.go +++ b/pkg/decision/evaluator/matchers/exact.go @@ -35,7 +35,7 @@ type ExactMatcher struct { func (m ExactMatcher) Match(user entities.UserContext) (bool, error) { if !user.CheckAttributeExists(m.Condition.Name) { - m.Logger.Debug(fmt.Sprintf(string(logging.NullUserAttribute), m.Condition.StringRepresentation, m.Condition.Name)) + m.Logger.Debug(fmt.Sprintf(logging.NullUserAttribute.String(), m.Condition.StringRepresentation, m.Condition.Name)) return false, fmt.Errorf(`no attribute named "%s"`, m.Condition.Name) } @@ -43,7 +43,7 @@ func (m ExactMatcher) Match(user entities.UserContext) (bool, error) { attributeValue, err := user.GetStringAttribute(m.Condition.Name) if err != nil { val, _ := user.GetAttribute(m.Condition.Name) - m.Logger.Warning(fmt.Sprintf(string(logging.InvalidAttributeValueType), m.Condition.StringRepresentation, val, m.Condition.Name)) + m.Logger.Warning(fmt.Sprintf(logging.InvalidAttributeValueType.String(), m.Condition.StringRepresentation, val, m.Condition.Name)) return false, err } return stringValue == attributeValue, nil @@ -53,7 +53,7 @@ func (m ExactMatcher) Match(user entities.UserContext) (bool, error) { attributeValue, err := user.GetBoolAttribute(m.Condition.Name) if err != nil { val, _ := user.GetAttribute(m.Condition.Name) - m.Logger.Warning(fmt.Sprintf(string(logging.InvalidAttributeValueType), m.Condition.StringRepresentation, val, m.Condition.Name)) + m.Logger.Warning(fmt.Sprintf(logging.InvalidAttributeValueType.String(), m.Condition.StringRepresentation, val, m.Condition.Name)) return false, err } return boolValue == attributeValue, nil @@ -63,12 +63,12 @@ func (m ExactMatcher) Match(user entities.UserContext) (bool, error) { attributeValue, err := user.GetFloatAttribute(m.Condition.Name) if err != nil { val, _ := user.GetAttribute(m.Condition.Name) - m.Logger.Warning(fmt.Sprintf(string(logging.InvalidAttributeValueType), m.Condition.StringRepresentation, val, m.Condition.Name)) + m.Logger.Warning(fmt.Sprintf(logging.InvalidAttributeValueType.String(), m.Condition.StringRepresentation, val, m.Condition.Name)) return false, err } return floatValue == attributeValue, nil } - m.Logger.Warning(fmt.Sprintf(string(logging.UnsupportedConditionValue), m.Condition.StringRepresentation)) + m.Logger.Warning(fmt.Sprintf(logging.UnsupportedConditionValue.String(), m.Condition.StringRepresentation)) return false, fmt.Errorf("audience condition %s evaluated to NULL because the condition value type is not supported", m.Condition.Name) } diff --git a/pkg/decision/evaluator/matchers/exact_test.go b/pkg/decision/evaluator/matchers/exact_test.go index 809d651aa..82573930c 100644 --- a/pkg/decision/evaluator/matchers/exact_test.go +++ b/pkg/decision/evaluator/matchers/exact_test.go @@ -93,7 +93,7 @@ func (s *ExactTestSuite) TestExactMatcherString() { "string_not_foo": "foo", }, } - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "string_foo")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.NullUserAttribute.String(), "", "string_foo")) _, err = matcher.Match(user) s.Error(err) @@ -103,7 +103,7 @@ func (s *ExactTestSuite) TestExactMatcherString() { "string_foo": 121, }, } - s.mockLogger.On("Warning", fmt.Sprintf(string(logging.InvalidAttributeValueType), "", 121, "string_foo")) + s.mockLogger.On("Warning", fmt.Sprintf(logging.InvalidAttributeValueType.String(), "", 121, "string_foo")) result, err = matcher.Match(user) s.Error(err) s.False(false) @@ -148,7 +148,7 @@ func (s *ExactTestSuite) TestExactMatcherBool() { }, } - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "bool_true")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.NullUserAttribute.String(), "", "bool_true")) _, err = matcher.Match(user) s.Error(err) @@ -158,7 +158,7 @@ func (s *ExactTestSuite) TestExactMatcherBool() { "bool_true": 121, }, } - s.mockLogger.On("Warning", fmt.Sprintf(string(logging.InvalidAttributeValueType), "", 121, "bool_true")) + s.mockLogger.On("Warning", fmt.Sprintf(logging.InvalidAttributeValueType.String(), "", 121, "bool_true")) result, err = matcher.Match(user) s.Error(err) s.False(result) @@ -213,7 +213,7 @@ func (s *ExactTestSuite) TestExactMatcherInt() { "int_43": 42, }, } - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "int_42")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.NullUserAttribute.String(), "", "int_42")) _, err = matcher.Match(user) s.Error(err) @@ -223,7 +223,7 @@ func (s *ExactTestSuite) TestExactMatcherInt() { "int_42": "test", }, } - s.mockLogger.On("Warning", fmt.Sprintf(string(logging.InvalidAttributeValueType), "", "test", "int_42")) + s.mockLogger.On("Warning", fmt.Sprintf(logging.InvalidAttributeValueType.String(), "", "test", "int_42")) result, err = matcher.Match(user) s.Error(err) s.False(result) @@ -267,7 +267,7 @@ func (s *ExactTestSuite) TestExactMatcherFloat() { "float_4_3": 4.2, }, } - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "float_4_2")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.NullUserAttribute.String(), "", "float_4_2")) _, err = matcher.Match(user) s.Error(err) @@ -277,7 +277,7 @@ func (s *ExactTestSuite) TestExactMatcherFloat() { "float_4_2": "test", }, } - s.mockLogger.On("Warning", fmt.Sprintf(string(logging.InvalidAttributeValueType), "", "test", "float_4_2")) + s.mockLogger.On("Warning", fmt.Sprintf(logging.InvalidAttributeValueType.String(), "", "test", "float_4_2")) result, err = matcher.Match(user) s.Error(err) s.False(result) @@ -300,7 +300,7 @@ func (s *ExactTestSuite) TestExactMatcherUnsupportedConditionValue() { "int_42": 42, }, } - s.mockLogger.On("Warning", fmt.Sprintf(string(logging.UnsupportedConditionValue), "")) + s.mockLogger.On("Warning", fmt.Sprintf(logging.UnsupportedConditionValue.String(), "")) result, err := matcher.Match(user) s.Error(err) s.False(result) diff --git a/pkg/decision/evaluator/matchers/gt.go b/pkg/decision/evaluator/matchers/gt.go index c83d794d5..2fc10df76 100644 --- a/pkg/decision/evaluator/matchers/gt.go +++ b/pkg/decision/evaluator/matchers/gt.go @@ -38,7 +38,7 @@ func (m GtMatcher) Match(user entities.UserContext) (bool, error) { func matchGtOrLt(user entities.UserContext, condition entities.Condition, logger logging.OptimizelyLogProducer, gtMatch bool) (bool, error) { if !user.CheckAttributeExists(condition.Name) { - logger.Debug(fmt.Sprintf(string(logging.NullUserAttribute), condition.StringRepresentation, condition.Name)) + logger.Debug(fmt.Sprintf(logging.NullUserAttribute.String(), condition.StringRepresentation, condition.Name)) return false, fmt.Errorf(`no attribute named "%s"`, condition.Name) } @@ -46,7 +46,7 @@ func matchGtOrLt(user entities.UserContext, condition entities.Condition, logger attributeValue, err := user.GetFloatAttribute(condition.Name) if err != nil { val, _ := user.GetAttribute(condition.Name) - logger.Warning(fmt.Sprintf(string(logging.InvalidAttributeValueType), condition.StringRepresentation, val, condition.Name)) + logger.Warning(fmt.Sprintf(logging.InvalidAttributeValueType.String(), condition.StringRepresentation, val, condition.Name)) return false, err } if gtMatch { @@ -55,6 +55,6 @@ func matchGtOrLt(user entities.UserContext, condition entities.Condition, logger return floatValue > attributeValue, nil } - logger.Warning(fmt.Sprintf(string(logging.UnsupportedConditionValue), condition.StringRepresentation)) + logger.Warning(fmt.Sprintf(logging.UnsupportedConditionValue.String(), condition.StringRepresentation)) return false, fmt.Errorf("audience condition %s evaluated to NULL because the condition value type is not supported", condition.Name) } diff --git a/pkg/decision/evaluator/matchers/gt_test.go b/pkg/decision/evaluator/matchers/gt_test.go index c6d7a7423..b96d66d3f 100644 --- a/pkg/decision/evaluator/matchers/gt_test.go +++ b/pkg/decision/evaluator/matchers/gt_test.go @@ -94,7 +94,7 @@ func (s *GtTestSuite) TestGtMatcherInt() { "int_43": 42, }, } - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "int_42")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.NullUserAttribute.String(), "", "int_42")) _, err = matcher.Match(user) s.Error(err) @@ -104,7 +104,7 @@ func (s *GtTestSuite) TestGtMatcherInt() { "int_42": true, }, } - s.mockLogger.On("Warning", fmt.Sprintf(string(logging.InvalidAttributeValueType), "", true, "int_42")) + s.mockLogger.On("Warning", fmt.Sprintf(logging.InvalidAttributeValueType.String(), "", true, "int_42")) result, err = matcher.Match(user) s.Error(err) s.False(result) @@ -159,7 +159,7 @@ func (s *GtTestSuite) TestGtMatcherFloat() { }, } - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "float_4_2")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.NullUserAttribute.String(), "", "float_4_2")) _, err = matcher.Match(user) s.Error(err) @@ -169,7 +169,7 @@ func (s *GtTestSuite) TestGtMatcherFloat() { "float_4_2": true, }, } - s.mockLogger.On("Warning", fmt.Sprintf(string(logging.InvalidAttributeValueType), "", true, "float_4_2")) + s.mockLogger.On("Warning", fmt.Sprintf(logging.InvalidAttributeValueType.String(), "", true, "float_4_2")) result, err = matcher.Match(user) s.Error(err) s.False(result) @@ -192,7 +192,7 @@ func (s *GtTestSuite) TestGtMatcherUnsupportedConditionValue() { "float_4_2": 4.2, }, } - s.mockLogger.On("Warning", fmt.Sprintf(string(logging.UnsupportedConditionValue), "")) + s.mockLogger.On("Warning", fmt.Sprintf(logging.UnsupportedConditionValue.String(), "")) result, err := matcher.Match(user) s.Error(err) s.False(result) diff --git a/pkg/decision/evaluator/matchers/lt_test.go b/pkg/decision/evaluator/matchers/lt_test.go index 350b97ef5..0f78d6ec3 100644 --- a/pkg/decision/evaluator/matchers/lt_test.go +++ b/pkg/decision/evaluator/matchers/lt_test.go @@ -95,7 +95,7 @@ func (s *LtTestSuite) TestLtMatcherInt() { }, } - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "int_42")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.NullUserAttribute.String(), "", "int_42")) _, err = matcher.Match(user) s.Error(err) @@ -105,7 +105,7 @@ func (s *LtTestSuite) TestLtMatcherInt() { "int_42": true, }, } - s.mockLogger.On("Warning", fmt.Sprintf(string(logging.InvalidAttributeValueType), "", true, "int_42")) + s.mockLogger.On("Warning", fmt.Sprintf(logging.InvalidAttributeValueType.String(), "", true, "int_42")) result, err = matcher.Match(user) s.Error(err) s.False(result) @@ -160,7 +160,7 @@ func (s *LtTestSuite) TestLtMatcherFloat() { }, } - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "float_4_2")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.NullUserAttribute.String(), "", "float_4_2")) _, err = matcher.Match(user) s.Error(err) @@ -170,7 +170,7 @@ func (s *LtTestSuite) TestLtMatcherFloat() { "float_4_2": true, }, } - s.mockLogger.On("Warning", fmt.Sprintf(string(logging.InvalidAttributeValueType), "", true, "float_4_2")) + s.mockLogger.On("Warning", fmt.Sprintf(logging.InvalidAttributeValueType.String(), "", true, "float_4_2")) result, err = matcher.Match(user) s.Error(err) s.False(result) @@ -193,7 +193,7 @@ func (s *LtTestSuite) TestLtMatcherUnsupportedConditionValue() { "float_4_2": 4.2, }, } - s.mockLogger.On("Warning", fmt.Sprintf(string(logging.UnsupportedConditionValue), "")) + s.mockLogger.On("Warning", fmt.Sprintf(logging.UnsupportedConditionValue.String(), "")) result, err := matcher.Match(user) s.Error(err) s.False(result) diff --git a/pkg/decision/evaluator/matchers/substring.go b/pkg/decision/evaluator/matchers/substring.go index b9a9aa1af..abbde4f85 100644 --- a/pkg/decision/evaluator/matchers/substring.go +++ b/pkg/decision/evaluator/matchers/substring.go @@ -35,7 +35,7 @@ type SubstringMatcher struct { func (m SubstringMatcher) Match(user entities.UserContext) (bool, error) { if !user.CheckAttributeExists(m.Condition.Name) { - m.Logger.Debug(fmt.Sprintf(string(logging.NullUserAttribute), m.Condition.StringRepresentation, m.Condition.Name)) + m.Logger.Debug(fmt.Sprintf(logging.NullUserAttribute.String(), m.Condition.StringRepresentation, m.Condition.Name)) return false, fmt.Errorf(`no attribute named "%s"`, m.Condition.Name) } @@ -43,12 +43,12 @@ func (m SubstringMatcher) Match(user entities.UserContext) (bool, error) { attributeValue, err := user.GetStringAttribute(m.Condition.Name) if err != nil { val, _ := user.GetAttribute(m.Condition.Name) - m.Logger.Warning(fmt.Sprintf(string(logging.InvalidAttributeValueType), m.Condition.StringRepresentation, val, m.Condition.Name)) + m.Logger.Warning(fmt.Sprintf(logging.InvalidAttributeValueType.String(), m.Condition.StringRepresentation, val, m.Condition.Name)) return false, err } return strings.Contains(attributeValue, stringValue), nil } - m.Logger.Warning(fmt.Sprintf(string(logging.UnsupportedConditionValue), m.Condition.StringRepresentation)) + m.Logger.Warning(fmt.Sprintf(logging.UnsupportedConditionValue.String(), m.Condition.StringRepresentation)) return false, fmt.Errorf("audience condition %s evaluated to NULL because the condition value type is not supported", m.Condition.Name) } diff --git a/pkg/decision/evaluator/matchers/substring_test.go b/pkg/decision/evaluator/matchers/substring_test.go index 3a00b266f..31948c576 100644 --- a/pkg/decision/evaluator/matchers/substring_test.go +++ b/pkg/decision/evaluator/matchers/substring_test.go @@ -74,7 +74,7 @@ func (s *SubstringTestSuite) TestSubstringMatcher() { }, } - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.NullUserAttribute), "", "string_foo")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.NullUserAttribute.String(), "", "string_foo")) _, err = matcher.Match(user) s.Error(err) @@ -84,7 +84,7 @@ func (s *SubstringTestSuite) TestSubstringMatcher() { "string_foo": true, }, } - s.mockLogger.On("Warning", fmt.Sprintf(string(logging.InvalidAttributeValueType), "", true, "string_foo")) + s.mockLogger.On("Warning", fmt.Sprintf(logging.InvalidAttributeValueType.String(), "", true, "string_foo")) result, err = matcher.Match(user) s.Error(err) s.False(result) @@ -107,7 +107,7 @@ func (s *GtTestSuite) TestSubstringMatcherUnsupportedConditionValue() { "string_foo": "foo", }, } - s.mockLogger.On("Warning", fmt.Sprintf(string(logging.UnsupportedConditionValue), "")) + s.mockLogger.On("Warning", fmt.Sprintf(logging.UnsupportedConditionValue.String(), "")) result, err := matcher.Match(user) s.Error(err) s.False(result) diff --git a/pkg/decision/experiment_bucketer_service.go b/pkg/decision/experiment_bucketer_service.go index 46347d91c..e5fe60017 100644 --- a/pkg/decision/experiment_bucketer_service.go +++ b/pkg/decision/experiment_bucketer_service.go @@ -52,16 +52,16 @@ func (s ExperimentBucketerService) GetDecision(decisionContext ExperimentDecisio // Determine if user can be part of the experiment if experiment.AudienceConditionTree != nil { condTreeParams := entities.NewTreeParameters(&userContext, decisionContext.ProjectConfig.GetAudienceMap()) - s.logger.Debug(fmt.Sprintf(string(logging.EvaluatingAudiencesForExperiment), experiment.Key)) + s.logger.Debug(fmt.Sprintf(logging.EvaluatingAudiencesForExperiment.String(), experiment.Key)) evalResult, _ := s.audienceTreeEvaluator.Evaluate(experiment.AudienceConditionTree, condTreeParams) - s.logger.Info(fmt.Sprintf(string(logging.ExperimentAudiencesEvaluatedTo), experiment.Key, evalResult)) + s.logger.Info(fmt.Sprintf(logging.ExperimentAudiencesEvaluatedTo.String(), experiment.Key, evalResult)) if !evalResult { - s.logger.Info(fmt.Sprintf(string(logging.UserNotInExperiment), userContext.ID, experiment.Key)) + s.logger.Info(fmt.Sprintf(logging.UserNotInExperiment.String(), userContext.ID, experiment.Key)) experimentDecision.Reason = reasons.FailedAudienceTargeting return experimentDecision, nil } } else { - s.logger.Info(fmt.Sprintf(string(logging.ExperimentAudiencesEvaluatedTo), experiment.Key, true)) + s.logger.Info(fmt.Sprintf(logging.ExperimentAudiencesEvaluatedTo.String(), experiment.Key, true)) } var group entities.Group diff --git a/pkg/decision/experiment_bucketer_service_test.go b/pkg/decision/experiment_bucketer_service_test.go index b59b2a0ad..b3cb87ed9 100644 --- a/pkg/decision/experiment_bucketer_service_test.go +++ b/pkg/decision/experiment_bucketer_service_test.go @@ -87,7 +87,7 @@ func (s *ExperimentBucketerTestSuite) TestGetDecisionNoTargeting() { ProjectConfig: s.mockConfig, } s.mockBucketer.On("Bucket", testUserContext.ID, testExp1111, entities.Group{}).Return(&testExp1111Var2222, reasons.BucketedIntoVariation, nil) - s.mockLogger.On("Info", fmt.Sprintf(string(logging.ExperimentAudiencesEvaluatedTo), "test_experiment_1111", true)) + s.mockLogger.On("Info", fmt.Sprintf(logging.ExperimentAudiencesEvaluatedTo.String(), "test_experiment_1111", true)) experimentBucketerService := ExperimentBucketerService{ bucketer: s.mockBucketer, logger: s.mockLogger, @@ -119,8 +119,8 @@ func (s *ExperimentBucketerTestSuite) TestGetDecisionWithTargetingPasses() { bucketer: s.mockBucketer, } s.mockConfig.On("GetAudienceMap").Return(map[string]entities.Audience{}) - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForExperiment), "test_targeted_experiment_1116")) - s.mockLogger.On("Info", fmt.Sprintf(string(logging.ExperimentAudiencesEvaluatedTo), "test_targeted_experiment_1116", true)) + s.mockLogger.On("Debug", fmt.Sprintf(logging.EvaluatingAudiencesForExperiment.String(), "test_targeted_experiment_1116")) + s.mockLogger.On("Info", fmt.Sprintf(logging.ExperimentAudiencesEvaluatedTo.String(), "test_targeted_experiment_1116", true)) testDecisionContext := ExperimentDecisionContext{ Experiment: &testTargetedExp1116, @@ -150,9 +150,9 @@ func (s *ExperimentBucketerTestSuite) TestGetDecisionWithTargetingFails() { bucketer: s.mockBucketer, } s.mockConfig.On("GetAudienceMap").Return(map[string]entities.Audience{}) - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForExperiment), "test_targeted_experiment_1116")) - s.mockLogger.On("Info", fmt.Sprintf(string(logging.ExperimentAudiencesEvaluatedTo), "test_targeted_experiment_1116", false)) - s.mockLogger.On("Info", fmt.Sprintf(string(logging.UserNotInExperiment), "test_user_1", "test_targeted_experiment_1116")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.EvaluatingAudiencesForExperiment.String(), "test_targeted_experiment_1116")) + s.mockLogger.On("Info", fmt.Sprintf(logging.ExperimentAudiencesEvaluatedTo.String(), "test_targeted_experiment_1116", false)) + s.mockLogger.On("Info", fmt.Sprintf(logging.UserNotInExperiment.String(), "test_user_1", "test_targeted_experiment_1116")) testDecisionContext := ExperimentDecisionContext{ Experiment: &testTargetedExp1116, diff --git a/pkg/decision/rollout_service.go b/pkg/decision/rollout_service.go index d01404788..18e488ffa 100644 --- a/pkg/decision/rollout_service.go +++ b/pkg/decision/rollout_service.go @@ -55,7 +55,7 @@ func (r RolloutService) GetDecision(decisionContext FeatureDecisionContext, user evaluateConditionTree := func(experiment *entities.Experiment, loggingKey string) bool { condTreeParams := entities.NewTreeParameters(&userContext, decisionContext.ProjectConfig.GetAudienceMap()) - r.logger.Debug(fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), loggingKey)) + r.logger.Debug(fmt.Sprintf(logging.EvaluatingAudiencesForRollout.String(), loggingKey)) evalResult, _ := r.audienceTreeEvaluator.Evaluate(experiment.AudienceConditionTree, condTreeParams) if !evalResult { featureDecision.Reason = reasons.FailedRolloutTargeting @@ -105,9 +105,9 @@ func (r RolloutService) GetDecision(decisionContext FeatureDecisionContext, user // Move to next evaluation if condition tree is available and evaluation fails evaluationResult := experiment.AudienceConditionTree == nil || evaluateConditionTree(experiment, loggingKey) - r.logger.Info(fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), loggingKey, evaluationResult)) + r.logger.Info(fmt.Sprintf(logging.RolloutAudiencesEvaluatedTo.String(), loggingKey, evaluationResult)) if !evaluationResult { - r.logger.Debug(fmt.Sprintf(string(logging.UserNotInRollout), userContext.ID, loggingKey)) + r.logger.Debug(fmt.Sprintf(logging.UserNotInRollout.String(), userContext.ID, loggingKey)) // Evaluate this user for the next rule continue } @@ -125,13 +125,13 @@ func (r RolloutService) GetDecision(decisionContext FeatureDecisionContext, user experimentDecisionContext := getExperimentDecisionContext(experiment) // Move to bucketing if conditionTree is unavailable or evaluation passes evaluationResult := experiment.AudienceConditionTree == nil || evaluateConditionTree(experiment, "Everyone Else") - r.logger.Info(fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "Everyone Else", evaluationResult)) + r.logger.Info(fmt.Sprintf(logging.RolloutAudiencesEvaluatedTo.String(), "Everyone Else", evaluationResult)) if evaluationResult { decision, err := r.experimentBucketerService.GetDecision(experimentDecisionContext, userContext) if err == nil { - r.logger.Debug(fmt.Sprintf(string(logging.UserInEveryoneElse), userContext.ID)) + r.logger.Debug(fmt.Sprintf(logging.UserInEveryoneElse.String(), userContext.ID)) } return getFeatureDecision(experiment, &decision) } diff --git a/pkg/decision/rollout_service_test.go b/pkg/decision/rollout_service_test.go index 1da26681c..f7313712e 100644 --- a/pkg/decision/rollout_service_test.go +++ b/pkg/decision/rollout_service_test.go @@ -125,8 +125,8 @@ func (s *RolloutServiceTestSuite) TestGetDecisionHappyPath() { Source: Rollout, Decision: Decision{Reason: reasons.BucketedIntoRollout}, } - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), "1")) - s.mockLogger.On("Info", fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "1", true)) + s.mockLogger.On("Debug", fmt.Sprintf(logging.EvaluatingAudiencesForRollout.String(), "1")) + s.mockLogger.On("Info", fmt.Sprintf(logging.RolloutAudiencesEvaluatedTo.String(), "1", true)) s.mockLogger.On("Debug", `Decision made for user "test_user" for feature rollout with key "test_feature_rollout_3334_key": Bucketed into feature rollout.`) decision, _ := testRolloutService.GetDecision(s.testFeatureDecisionContext, s.testUserContext) s.Equal(expectedFeatureDecision, decision) @@ -165,11 +165,11 @@ func (s *RolloutServiceTestSuite) TestGetDecisionFallbacksToLastWhenFailsBucketi Source: Rollout, Decision: Decision{Reason: reasons.BucketedIntoRollout}, } - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), "1")) - s.mockLogger.On("Info", fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "1", true)) - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), "Everyone Else")) - s.mockLogger.On("Info", fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "Everyone Else", true)) - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.UserInEveryoneElse), "test_user")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.EvaluatingAudiencesForRollout.String(), "1")) + s.mockLogger.On("Info", fmt.Sprintf(logging.RolloutAudiencesEvaluatedTo.String(), "1", true)) + s.mockLogger.On("Debug", fmt.Sprintf(logging.EvaluatingAudiencesForRollout.String(), "Everyone Else")) + s.mockLogger.On("Info", fmt.Sprintf(logging.RolloutAudiencesEvaluatedTo.String(), "Everyone Else", true)) + s.mockLogger.On("Debug", fmt.Sprintf(logging.UserInEveryoneElse.String(), "test_user")) s.mockLogger.On("Debug", `Decision made for user "test_user" for feature rollout with key "test_feature_rollout_3334_key": Bucketed into feature rollout.`) decision, _ := testRolloutService.GetDecision(s.testFeatureDecisionContext, s.testUserContext) s.Equal(expectedFeatureDecision, decision) @@ -203,11 +203,11 @@ func (s *RolloutServiceTestSuite) TestGetDecisionWhenFallbackBucketingFails() { Source: Rollout, Decision: Decision{Reason: reasons.FailedRolloutBucketing}, } - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), "1")) - s.mockLogger.On("Info", fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "1", true)) - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), "Everyone Else")) - s.mockLogger.On("Info", fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "Everyone Else", true)) - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.UserInEveryoneElse), "test_user")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.EvaluatingAudiencesForRollout.String(), "1")) + s.mockLogger.On("Info", fmt.Sprintf(logging.RolloutAudiencesEvaluatedTo.String(), "1", true)) + s.mockLogger.On("Debug", fmt.Sprintf(logging.EvaluatingAudiencesForRollout.String(), "Everyone Else")) + s.mockLogger.On("Info", fmt.Sprintf(logging.RolloutAudiencesEvaluatedTo.String(), "Everyone Else", true)) + s.mockLogger.On("Debug", fmt.Sprintf(logging.UserInEveryoneElse.String(), "test_user")) s.mockLogger.On("Debug", `Decision made for user "test_user" for feature rollout with key "test_feature_rollout_3334_key": Not bucketed into rollout.`) decision, _ := testRolloutService.GetDecision(s.testFeatureDecisionContext, s.testUserContext) s.Equal(expectedFeatureDecision, decision) @@ -240,11 +240,11 @@ func (s *RolloutServiceTestSuite) TestEvaluatesNextIfPreviousTargetingFails() { Source: Rollout, Decision: Decision{Reason: reasons.BucketedIntoRollout}, } - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), "1")) - s.mockLogger.On("Info", fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "1", false)) - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.UserNotInRollout), "test_user", "1")) - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), "2")) - s.mockLogger.On("Info", fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "2", true)) + s.mockLogger.On("Debug", fmt.Sprintf(logging.EvaluatingAudiencesForRollout.String(), "1")) + s.mockLogger.On("Info", fmt.Sprintf(logging.RolloutAudiencesEvaluatedTo.String(), "1", false)) + s.mockLogger.On("Debug", fmt.Sprintf(logging.UserNotInRollout.String(), "test_user", "1")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.EvaluatingAudiencesForRollout.String(), "2")) + s.mockLogger.On("Info", fmt.Sprintf(logging.RolloutAudiencesEvaluatedTo.String(), "2", true)) s.mockLogger.On("Debug", `Decision made for user "test_user" for feature rollout with key "test_feature_rollout_3334_key": Bucketed into feature rollout.`) decision, _ := testRolloutService.GetDecision(s.testFeatureDecisionContext, s.testUserContext) s.Equal(expectedFeatureDecision, decision) @@ -268,14 +268,14 @@ func (s *RolloutServiceTestSuite) TestGetDecisionFailsTargeting() { }, Source: Rollout, } - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), "1")) - s.mockLogger.On("Info", fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "1", false)) - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.UserNotInRollout), "test_user", "1")) - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), "2")) - s.mockLogger.On("Info", fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "2", false)) - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.UserNotInRollout), "test_user", "2")) - s.mockLogger.On("Debug", fmt.Sprintf(string(logging.EvaluatingAudiencesForRollout), "Everyone Else")) - s.mockLogger.On("Info", fmt.Sprintf(string(logging.RolloutAudiencesEvaluatedTo), "Everyone Else", false)) + s.mockLogger.On("Debug", fmt.Sprintf(logging.EvaluatingAudiencesForRollout.String(), "1")) + s.mockLogger.On("Info", fmt.Sprintf(logging.RolloutAudiencesEvaluatedTo.String(), "1", false)) + s.mockLogger.On("Debug", fmt.Sprintf(logging.UserNotInRollout.String(), "test_user", "1")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.EvaluatingAudiencesForRollout.String(), "2")) + s.mockLogger.On("Info", fmt.Sprintf(logging.RolloutAudiencesEvaluatedTo.String(), "2", false)) + s.mockLogger.On("Debug", fmt.Sprintf(logging.UserNotInRollout.String(), "test_user", "2")) + s.mockLogger.On("Debug", fmt.Sprintf(logging.EvaluatingAudiencesForRollout.String(), "Everyone Else")) + s.mockLogger.On("Info", fmt.Sprintf(logging.RolloutAudiencesEvaluatedTo.String(), "Everyone Else", false)) decision, _ := testRolloutService.GetDecision(s.testFeatureDecisionContext, s.testUserContext) s.Equal(expectedFeatureDecision, decision) s.mockAudienceTreeEvaluator.AssertExpectations(s.T()) diff --git a/pkg/logging/log_messages.go b/pkg/logging/log_messages.go index bf2b6d035..ef376a6d8 100644 --- a/pkg/logging/log_messages.go +++ b/pkg/logging/log_messages.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019-2020, Optimizely, Inc. and contributors * + * Copyright 2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -20,6 +20,10 @@ package logging // LogMessage defines string type for log messages type LogMessage string +func (l LogMessage) String() string { + return string(l) +} + const ( // Debug Logs From 6664cf209fc1a67d93d87037dd7d841b8b0fcd27 Mon Sep 17 00:00:00 2001 From: Yasir Ali Date: Wed, 15 Jul 2020 16:05:51 +0500 Subject: [PATCH 5/6] Initial commit. --- pkg/client/client.go | 19 ++++++- pkg/decision/bucketer/experiment_bucketer.go | 24 ++++++--- .../bucketer/experiment_bucketer_test.go | 49 ++++++++++++++++++- pkg/decision/bucketer/murmurhashbucketer.go | 14 +++--- .../bucketer/murmurhashbucketer_test.go | 36 ++++++++++---- pkg/decision/experiment_bucketer_service.go | 5 ++ .../experiment_bucketer_service_test.go | 3 ++ pkg/decision/feature_experiment_service.go | 12 +---- pkg/logging/log_messages.go | 34 +++++++++++-- 9 files changed, 152 insertions(+), 44 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index 26eafe2d6..9605a3113 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -350,9 +350,16 @@ func (o *OptimizelyClient) getFeatureVariable(featureKey, variableKey string, us variable := featureDecisionContext.Variable if featureDecision.Variation != nil { - if v, ok := featureDecision.Variation.Variables[variable.ID]; ok && featureDecision.Variation.FeatureEnabled { - return v.Value, variable.Type, &featureDecision, nil + if featureDecision.Variation.FeatureEnabled { + if v, ok := featureDecision.Variation.Variables[variable.ID]; ok { + o.logger.Info(fmt.Sprintf(logging.VariableValueForFeatureFlag.String(), v.Value, variable.Key, featureKey)) + return v.Value, variable.Type, &featureDecision, nil + } + } else { + o.logger.Info(fmt.Sprintf(logging.FeatureNotEnabledForUserReturningDefault.String(), featureKey, userContext.ID, variable.DefaultValue)) } + } else { + o.logger.Info(fmt.Sprintf(logging.ReturningDefaultValue.String(), userContext.ID, variableKey, featureKey)) } return variable.DefaultValue, variable.Type, &featureDecision, nil @@ -411,6 +418,13 @@ func (o *OptimizelyClient) GetAllFeatureVariablesWithDecision(featureKey string, if featureDecision.Variation != nil { enabled = featureDecision.Variation.FeatureEnabled + if enabled { + o.logger.Info(fmt.Sprintf(logging.FeatureEnabledForUser.String(), featureKey, userContext.ID)) + } else { + o.logger.Info(fmt.Sprintf(logging.FeatureNotEnabledForUser.String(), featureKey, userContext.ID)) + } + } else { + o.logger.Info(fmt.Sprintf(logging.ReturningAllDefaultValue.String(), userContext.ID, featureKey)) } feature := decisionContext.Feature @@ -427,6 +441,7 @@ func (o *OptimizelyClient) GetAllFeatureVariablesWithDecision(featureKey string, if enabled { if variable, ok := featureDecision.Variation.Variables[v.ID]; ok { val = variable.Value + o.logger.Debug(fmt.Sprintf(logging.VariableValueForFeatureFlag.String(), val, v.Key, featureKey)) } } diff --git a/pkg/decision/bucketer/experiment_bucketer.go b/pkg/decision/bucketer/experiment_bucketer.go index 2a380efa0..637e65570 100644 --- a/pkg/decision/bucketer/experiment_bucketer.go +++ b/pkg/decision/bucketer/experiment_bucketer.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -18,6 +18,8 @@ package bucketer import ( + "fmt" + "github.com/optimizely/go-sdk/pkg/decision/reasons" "github.com/optimizely/go-sdk/pkg/entities" "github.com/optimizely/go-sdk/pkg/logging" @@ -31,28 +33,34 @@ type ExperimentBucketer interface { // MurmurhashExperimentBucketer buckets the user using the mmh3 algorightm type MurmurhashExperimentBucketer struct { bucketer Bucketer + logger logging.OptimizelyLogProducer } // NewMurmurhashExperimentBucketer returns a new instance of the murmurhash experiment bucketer func NewMurmurhashExperimentBucketer(logger logging.OptimizelyLogProducer, hashSeed uint32) *MurmurhashExperimentBucketer { return &MurmurhashExperimentBucketer{ - bucketer: MurmurhashBucketer{hashSeed: hashSeed, logger:logger}, + bucketer: MurmurhashBucketer{hashSeed: hashSeed, logger: logger}, + logger: logger, } } // Bucket buckets the user into the given experiment func (b MurmurhashExperimentBucketer) Bucket(bucketingID string, experiment entities.Experiment, group entities.Group) (*entities.Variation, reasons.Reason, error) { if experiment.GroupID != "" && group.Policy == "random" { - bucketKey := bucketingID + group.ID - bucketedExperimentID := b.bucketer.BucketToEntity(bucketKey, group.TrafficAllocation) - if bucketedExperimentID == "" || bucketedExperimentID != experiment.ID { - // User is not bucketed into provided experiment in mutex group + if bucketedExperimentID := b.bucketer.BucketToEntity(bucketingID, group.ID, group.TrafficAllocation); bucketedExperimentID != "" { + if bucketedExperimentID != experiment.ID { + // User is not bucketed into provided experiment in mutex group + b.logger.Info(fmt.Sprintf(logging.UserNotBucketedIntoExperimentInGroup.String(), bucketingID, experiment.Key, group.ID)) + return nil, reasons.NotBucketedIntoVariation, nil + } + b.logger.Info(fmt.Sprintf(logging.UserBucketedIntoExperimentInGroup.String(), bucketingID, experiment.Key, group.ID)) + } else { + b.logger.Info(fmt.Sprintf(logging.UserNotBucketedIntoAnyExperimentInGroup.String(), bucketingID, group.ID)) return nil, reasons.NotBucketedIntoVariation, nil } } - bucketKey := bucketingID + experiment.ID - bucketedVariationID := b.bucketer.BucketToEntity(bucketKey, experiment.TrafficAllocation) + bucketedVariationID := b.bucketer.BucketToEntity(bucketingID, experiment.ID, experiment.TrafficAllocation) if bucketedVariationID == "" { // User is not bucketed into a variation in the experiment, return nil variation return nil, reasons.NotBucketedIntoVariation, nil diff --git a/pkg/decision/bucketer/experiment_bucketer_test.go b/pkg/decision/bucketer/experiment_bucketer_test.go index 2d3ebec8c..d34038c9a 100644 --- a/pkg/decision/bucketer/experiment_bucketer_test.go +++ b/pkg/decision/bucketer/experiment_bucketer_test.go @@ -1,16 +1,56 @@ +/**************************************************************************** + * Copyright 2019-2020, Optimizely, Inc. and contributors * + * * + * Licensed under the Apache License, Version 2.0 (the "License"); * + * you may not use this file except in compliance with the License. * + * You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, software * + * distributed under the License is distributed on an "AS IS" BASIS, * + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * + * See the License for the specific language governing permissions and * + * limitations under the License. * + ***************************************************************************/ + +// Package bucketer // package bucketer import ( - "github.com/optimizely/go-sdk/pkg/logging" + "fmt" "testing" "github.com/optimizely/go-sdk/pkg/decision/reasons" + "github.com/optimizely/go-sdk/pkg/logging" "github.com/optimizely/go-sdk/pkg/entities" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" ) +type MockLogger struct { + mock.Mock +} + +func (m *MockLogger) Debug(message string) { + m.Called(message) +} + +func (m *MockLogger) Info(message string) { + m.Called(message) +} + +func (m *MockLogger) Warning(message string) { + m.Called(message) +} + +func (m *MockLogger) Error(message string, err interface{}) { + m.Called(message, err) +} + func TestBucketExclusionGroups(t *testing.T) { + mockLogger := MockLogger{} experiment1 := entities.Experiment{ ID: "1886780721", Key: "experiment_1", @@ -47,8 +87,13 @@ func TestBucketExclusionGroups(t *testing.T) { }, } - bucketer := NewMurmurhashExperimentBucketer(logging.GetLogger("","TestBucketExclusionGroups" ), DefaultHashSeed) + bucketer := NewMurmurhashExperimentBucketer(&mockLogger, DefaultHashSeed) // ppid2 + 1886780722 (groupId) will generate bucket value of 2434 which maps to experiment 1 + mockLogger.On("Debug", fmt.Sprintf(logging.UserAssignedToBucketValue.String(), 2434, "ppid2")) + mockLogger.On("Info", fmt.Sprintf(logging.UserBucketedIntoExperimentInGroup.String(), "ppid2", "experiment_1", "1886780722")) + mockLogger.On("Debug", fmt.Sprintf(logging.UserAssignedToBucketValue.String(), 4299, "ppid2")) + mockLogger.On("Info", fmt.Sprintf(logging.UserNotBucketedIntoExperimentInGroup.String(), "ppid2", "experiment_2", "1886780722")) + bucketedVariation, reason, _ := bucketer.Bucket("ppid2", experiment1, exclusionGroup) assert.Equal(t, experiment1.Variations["22222"], *bucketedVariation) assert.Equal(t, reasons.BucketedIntoVariation, reason) diff --git a/pkg/decision/bucketer/murmurhashbucketer.go b/pkg/decision/bucketer/murmurhashbucketer.go index a9eb25ef2..508c3464c 100644 --- a/pkg/decision/bucketer/murmurhashbucketer.go +++ b/pkg/decision/bucketer/murmurhashbucketer.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -36,7 +36,7 @@ const maxTrafficValue = 10000 // Bucketer is used to generate bucket value using bucketing key type Bucketer interface { Generate(bucketingKey string) int - BucketToEntity(bucketKey string, trafficAllocations []entities.Range) (entityID string) + BucketToEntity(bucketingID, entityID string, trafficAllocations []entities.Range) string } // MurmurhashBucketer generates the bucketing value using the mmh3 algorightm @@ -49,7 +49,7 @@ type MurmurhashBucketer struct { func NewMurmurhashBucketer(logger logging.OptimizelyLogProducer, hashSeed uint32) *MurmurhashBucketer { return &MurmurhashBucketer{ hashSeed: hashSeed, - logger: logger, + logger: logger, } } @@ -64,10 +64,10 @@ func (b MurmurhashBucketer) Generate(bucketingKey string) int { return int(ratio * maxTrafficValue) } -// BucketToEntity buckets into a traffic against given bucketKey -func (b MurmurhashBucketer) BucketToEntity(bucketKey string, trafficAllocations []entities.Range) (entityID string) { - bucketValue := b.Generate(bucketKey) - +// BucketToEntity buckets into a traffic against given bucketingId and entityID +func (b MurmurhashBucketer) BucketToEntity(bucketingID, entityID string, trafficAllocations []entities.Range) string { + bucketValue := b.Generate(bucketingID + entityID) + b.logger.Debug(fmt.Sprintf(logging.UserAssignedToBucketValue.String(), bucketValue, bucketingID)) var currentEndOfRange int for _, trafficAllocationRange := range trafficAllocations { currentEndOfRange = trafficAllocationRange.EndOfRange diff --git a/pkg/decision/bucketer/murmurhashbucketer_test.go b/pkg/decision/bucketer/murmurhashbucketer_test.go index 3db47c552..d0542d511 100644 --- a/pkg/decision/bucketer/murmurhashbucketer_test.go +++ b/pkg/decision/bucketer/murmurhashbucketer_test.go @@ -1,10 +1,28 @@ +/**************************************************************************** + * Copyright 2019-2020, Optimizely, Inc. and contributors * + * * + * Licensed under the Apache License, Version 2.0 (the "License"); * + * you may not use this file except in compliance with the License. * + * You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, software * + * distributed under the License is distributed on an "AS IS" BASIS, * + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * + * See the License for the specific language governing permissions and * + * limitations under the License. * + ***************************************************************************/ + +// Package bucketer // package bucketer import ( "fmt" - "github.com/optimizely/go-sdk/pkg/logging" "testing" + "github.com/optimizely/go-sdk/pkg/logging" + "github.com/optimizely/go-sdk/pkg/entities" "github.com/stretchr/testify/assert" ) @@ -16,13 +34,11 @@ func TestBucketToEntity(t *testing.T) { experimentID2 := "1886780722" // bucket value 5254 - bucketingKey1 := fmt.Sprintf("%s%s", "ppid1", experimentID) + bucketingID1 := "ppid1" // bucket value 4299 - bucketingKey2 := fmt.Sprintf("%s%s", "ppid2", experimentID) + bucketingID2 := "ppid2" // bucket value 2434 - bucketingKey3 := fmt.Sprintf("%s%s", "ppid2", experimentID2) - // bucket value 5439 - bucketingKey4 := fmt.Sprintf("%s%s", "ppid3", experimentID) + bucketingID3 := "ppid3" variation1 := "1234567123" variation2 := "5949300123" @@ -41,14 +57,14 @@ func TestBucketToEntity(t *testing.T) { }, } - assert.Equal(t, variation2, bucketer.BucketToEntity(bucketingKey1, trafficAlloc)) - assert.Equal(t, variation1, bucketer.BucketToEntity(bucketingKey2, trafficAlloc)) + assert.Equal(t, variation2, bucketer.BucketToEntity(bucketingID1, experimentID, trafficAlloc)) + assert.Equal(t, variation1, bucketer.BucketToEntity(bucketingID2, experimentID, trafficAlloc)) // bucket to empty variation range - assert.Equal(t, "", bucketer.BucketToEntity(bucketingKey3, trafficAlloc)) + assert.Equal(t, "", bucketer.BucketToEntity(bucketingID2, experimentID2, trafficAlloc)) // bucket outside of range (not in experiment) - assert.Equal(t, "", bucketer.BucketToEntity(bucketingKey4, trafficAlloc)) + assert.Equal(t, "", bucketer.BucketToEntity(bucketingID3, experimentID, trafficAlloc)) } func TestGenerateBucketValue(t *testing.T) { diff --git a/pkg/decision/experiment_bucketer_service.go b/pkg/decision/experiment_bucketer_service.go index e5fe60017..a9f17fe42 100644 --- a/pkg/decision/experiment_bucketer_service.go +++ b/pkg/decision/experiment_bucketer_service.go @@ -80,6 +80,11 @@ func (s ExperimentBucketerService) GetDecision(decisionContext ExperimentDecisio } // @TODO: handle error from bucketer variation, reason, _ := s.bucketer.Bucket(bucketingID, *experiment, group) + if variation != nil { + s.logger.Info(fmt.Sprintf(logging.UserBucketedIntoVariationInExperiment.String(), userContext.ID, variation.Key, experiment.Key)) + } else { + s.logger.Info(fmt.Sprintf(logging.UserNotBucketedIntoVariation.String(), userContext.ID)) + } experimentDecision.Reason = reason experimentDecision.Variation = variation return experimentDecision, nil diff --git a/pkg/decision/experiment_bucketer_service_test.go b/pkg/decision/experiment_bucketer_service_test.go index b3cb87ed9..17716b182 100644 --- a/pkg/decision/experiment_bucketer_service_test.go +++ b/pkg/decision/experiment_bucketer_service_test.go @@ -88,6 +88,8 @@ func (s *ExperimentBucketerTestSuite) TestGetDecisionNoTargeting() { } s.mockBucketer.On("Bucket", testUserContext.ID, testExp1111, entities.Group{}).Return(&testExp1111Var2222, reasons.BucketedIntoVariation, nil) s.mockLogger.On("Info", fmt.Sprintf(logging.ExperimentAudiencesEvaluatedTo.String(), "test_experiment_1111", true)) + s.mockLogger.On("Info", fmt.Sprintf(logging.UserBucketedIntoVariationInExperiment.String(), "test_user_1", "2222", "test_experiment_1111")) + experimentBucketerService := ExperimentBucketerService{ bucketer: s.mockBucketer, logger: s.mockLogger, @@ -121,6 +123,7 @@ func (s *ExperimentBucketerTestSuite) TestGetDecisionWithTargetingPasses() { s.mockConfig.On("GetAudienceMap").Return(map[string]entities.Audience{}) s.mockLogger.On("Debug", fmt.Sprintf(logging.EvaluatingAudiencesForExperiment.String(), "test_targeted_experiment_1116")) s.mockLogger.On("Info", fmt.Sprintf(logging.ExperimentAudiencesEvaluatedTo.String(), "test_targeted_experiment_1116", true)) + s.mockLogger.On("Info", fmt.Sprintf(logging.UserBucketedIntoVariationInExperiment.String(), "test_user_1", "2228", "test_targeted_experiment_1116")) testDecisionContext := ExperimentDecisionContext{ Experiment: &testTargetedExp1116, diff --git a/pkg/decision/feature_experiment_service.go b/pkg/decision/feature_experiment_service.go index 8890c10e7..d4f9c35e5 100644 --- a/pkg/decision/feature_experiment_service.go +++ b/pkg/decision/feature_experiment_service.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019, Optimizely, Inc. and contributors * + * Copyright 2019-2020, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -18,8 +18,6 @@ package decision import ( - "fmt" - "github.com/optimizely/go-sdk/pkg/entities" "github.com/optimizely/go-sdk/pkg/logging" ) @@ -33,7 +31,7 @@ type FeatureExperimentService struct { // NewFeatureExperimentService returns a new instance of the FeatureExperimentService func NewFeatureExperimentService(logger logging.OptimizelyLogProducer, compositeExperimentService ExperimentService) *FeatureExperimentService { return &FeatureExperimentService{ - logger: logger, + logger: logger, compositeExperimentService: compositeExperimentService, } } @@ -50,12 +48,6 @@ func (f FeatureExperimentService) GetDecision(decisionContext FeatureDecisionCon } experimentDecision, err := f.compositeExperimentService.GetDecision(experimentDecisionContext, userContext) - 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, - experimentDecision.Reason, - )) // Variation not nil means we got a decision and should return it if experimentDecision.Variation != nil { diff --git a/pkg/logging/log_messages.go b/pkg/logging/log_messages.go index ef376a6d8..fc2dca545 100644 --- a/pkg/logging/log_messages.go +++ b/pkg/logging/log_messages.go @@ -34,22 +34,46 @@ const ( // EvaluatingAudiencesForExperiment when audience evaluation is started for an experiment EvaluatingAudiencesForExperiment LogMessage = `Evaluating audiences for experiment "%s".` // EvaluatingAudiencesForRollout when audience evaluation is started for a rule - EvaluatingAudiencesForRollout LogMessage = `Evaluating audiences for rule %s.".` + EvaluatingAudiencesForRollout LogMessage = `Evaluating audiences for rule "%s".` // NullUserAttribute when user attribute is missing or nil - NullUserAttribute LogMessage = `Audience condition %s evaluated to UNKNOWN because a null value was passed for user attribute "%s".` + NullUserAttribute LogMessage = `Audience condition "%s" evaluated to UNKNOWN because a null value was passed for user attribute "%s".` // UserInEveryoneElse when user is in last rule UserInEveryoneElse LogMessage = `User "%s" meets conditions for targeting rule "Everyone Else".` // UserNotInRollout when user is not in rollout/rule - UserNotInRollout LogMessage = `User "%s" does not meet conditions for targeting rule %s.` + UserNotInRollout LogMessage = `User "%s" does not meet conditions for targeting rule "%s".` + // UserAssignedToBucketValue when user is assigned to a bucket value + UserAssignedToBucketValue LogMessage = `Assigned bucket "%d" to user with bucketing ID "%s".` // Info Logs // ExperimentAudiencesEvaluatedTo when collective audience evaluation for experiment is completed - ExperimentAudiencesEvaluatedTo LogMessage = `Audiences for experiment %s collectively evaluated to %t.` + ExperimentAudiencesEvaluatedTo LogMessage = `Audiences for experiment "%s" collectively evaluated to %t.` // RolloutAudiencesEvaluatedTo when collective audience evaluation for rule is completed - RolloutAudiencesEvaluatedTo LogMessage = `Audiences for rule %s collectively evaluated to %t.` + RolloutAudiencesEvaluatedTo LogMessage = `Audiences for rule "%s" collectively evaluated to %t.` // UserNotInExperiment when user is not in experiment UserNotInExperiment LogMessage = `User "%s" does not meet conditions to be in experiment "%s".` + // UserBucketedIntoExperimentInGroup when user is bucketed to experiment group + UserBucketedIntoExperimentInGroup LogMessage = `User "%s" is in experiment "%s" of group "%s".` + // UserNotBucketedIntoExperimentInGroup when user is not bucketed to experiment group + UserNotBucketedIntoExperimentInGroup LogMessage = `User "%s" is not in experiment "%s" of group "%s".` + // UserNotBucketedIntoAnyExperimentInGroup when user is not bucketed to any experiment group + UserNotBucketedIntoAnyExperimentInGroup LogMessage = `User "%s" is not in any experiment of group "%s".` + // UserBucketedIntoVariationInExperiment when user is bucketed to a variation in experiment + UserBucketedIntoVariationInExperiment LogMessage = `User "%s" is in variation "%s" of experiment "%s"` + // UserNotBucketedIntoVariation when user not bucketed to a variation + UserNotBucketedIntoVariation LogMessage = `User "%s" is in no variation.` + // VariableValueForFeatureFlag when got variable value for variable of feature flag + VariableValueForFeatureFlag LogMessage = `Got variable value "%s" for variable "%s" of feature flag "%s".` + // FeatureEnabledForUser when feature is enabled for user + FeatureEnabledForUser LogMessage = `Feature "%s" is enabled for user "%s".` + // FeatureNotEnabledForUser when feature is not enabled for user + FeatureNotEnabledForUser LogMessage = `Feature "%s" is not enabled for user "%s".` + // FeatureNotEnabledForUserReturningDefault when returning default value because feature is not enabled for user + FeatureNotEnabledForUserReturningDefault LogMessage = `Feature "%s" is not enabled for user "%s". Returning the default variable value "%s".` + // ReturningDefaultValue when user is not in variation or rollout rule + ReturningDefaultValue LogMessage = `User "%s" is not in any variation or rollout rule. Returning default value for variable "%s" of feature flag "%s".` + // ReturningAllDefaultValue when user is not in any variation or rollout rule + ReturningAllDefaultValue LogMessage = `User "%s" is not in any variation or rollout rule. Returning default value for all variables of feature flag "%s".` // Warning logs From c486a10deac5af104e8ddc2b0fb22592e8ac6e59 Mon Sep 17 00:00:00 2001 From: Yasir Ali Date: Mon, 24 Aug 2020 16:17:59 +0500 Subject: [PATCH 6/6] fixes. --- pkg/decision/bucketer/experiment_bucketer.go | 6 ++--- .../bucketer/experiment_bucketer_test.go | 24 +++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/pkg/decision/bucketer/experiment_bucketer.go b/pkg/decision/bucketer/experiment_bucketer.go index 637e65570..12d363d35 100644 --- a/pkg/decision/bucketer/experiment_bucketer.go +++ b/pkg/decision/bucketer/experiment_bucketer.go @@ -50,12 +50,12 @@ func (b MurmurhashExperimentBucketer) Bucket(bucketingID string, experiment enti if bucketedExperimentID := b.bucketer.BucketToEntity(bucketingID, group.ID, group.TrafficAllocation); bucketedExperimentID != "" { if bucketedExperimentID != experiment.ID { // User is not bucketed into provided experiment in mutex group - b.logger.Info(fmt.Sprintf(logging.UserNotBucketedIntoExperimentInGroup.String(), bucketingID, experiment.Key, group.ID)) + b.logger.Debug(fmt.Sprintf(logging.UserNotBucketedIntoExperimentInGroup.String(), bucketingID, experiment.Key, group.ID)) return nil, reasons.NotBucketedIntoVariation, nil } - b.logger.Info(fmt.Sprintf(logging.UserBucketedIntoExperimentInGroup.String(), bucketingID, experiment.Key, group.ID)) + b.logger.Debug(fmt.Sprintf(logging.UserBucketedIntoExperimentInGroup.String(), bucketingID, experiment.Key, group.ID)) } else { - b.logger.Info(fmt.Sprintf(logging.UserNotBucketedIntoAnyExperimentInGroup.String(), bucketingID, group.ID)) + b.logger.Debug(fmt.Sprintf(logging.UserNotBucketedIntoAnyExperimentInGroup.String(), bucketingID, group.ID)) return nil, reasons.NotBucketedIntoVariation, nil } } diff --git a/pkg/decision/bucketer/experiment_bucketer_test.go b/pkg/decision/bucketer/experiment_bucketer_test.go index d34038c9a..8e8d83865 100644 --- a/pkg/decision/bucketer/experiment_bucketer_test.go +++ b/pkg/decision/bucketer/experiment_bucketer_test.go @@ -55,12 +55,12 @@ func TestBucketExclusionGroups(t *testing.T) { ID: "1886780721", Key: "experiment_1", Variations: map[string]entities.Variation{ - "22222": entities.Variation{ID: "22222", Key: "exp_1_var_1"}, - "22223": entities.Variation{ID: "22223", Key: "exp_1_var_2"}, + "22222": {ID: "22222", Key: "exp_1_var_1"}, + "22223": {ID: "22223", Key: "exp_1_var_2"}, }, TrafficAllocation: []entities.Range{ - entities.Range{EntityID: "22222", EndOfRange: 4999}, - entities.Range{EntityID: "22223", EndOfRange: 10000}, + {EntityID: "22222", EndOfRange: 4999}, + {EntityID: "22223", EndOfRange: 10000}, }, GroupID: "1886780722", } @@ -68,12 +68,12 @@ func TestBucketExclusionGroups(t *testing.T) { ID: "1886780723", Key: "experiment_2", Variations: map[string]entities.Variation{ - "22224": entities.Variation{ID: "22224", Key: "exp_2_var_1"}, - "22225": entities.Variation{ID: "22225", Key: "exp_2_var_2"}, + "22224": {ID: "22224", Key: "exp_2_var_1"}, + "22225": {ID: "22225", Key: "exp_2_var_2"}, }, TrafficAllocation: []entities.Range{ - entities.Range{EntityID: "22224", EndOfRange: 4999}, - entities.Range{EntityID: "22225", EndOfRange: 10000}, + {EntityID: "22224", EndOfRange: 4999}, + {EntityID: "22225", EndOfRange: 10000}, }, GroupID: "1886780722", } @@ -82,17 +82,17 @@ func TestBucketExclusionGroups(t *testing.T) { ID: "1886780722", Policy: "random", TrafficAllocation: []entities.Range{ - entities.Range{EntityID: "1886780721", EndOfRange: 2500}, - entities.Range{EntityID: "1886780723", EndOfRange: 5000}, + {EntityID: "1886780721", EndOfRange: 2500}, + {EntityID: "1886780723", EndOfRange: 5000}, }, } bucketer := NewMurmurhashExperimentBucketer(&mockLogger, DefaultHashSeed) // ppid2 + 1886780722 (groupId) will generate bucket value of 2434 which maps to experiment 1 mockLogger.On("Debug", fmt.Sprintf(logging.UserAssignedToBucketValue.String(), 2434, "ppid2")) - mockLogger.On("Info", fmt.Sprintf(logging.UserBucketedIntoExperimentInGroup.String(), "ppid2", "experiment_1", "1886780722")) + mockLogger.On("Debug", fmt.Sprintf(logging.UserBucketedIntoExperimentInGroup.String(), "ppid2", "experiment_1", "1886780722")) mockLogger.On("Debug", fmt.Sprintf(logging.UserAssignedToBucketValue.String(), 4299, "ppid2")) - mockLogger.On("Info", fmt.Sprintf(logging.UserNotBucketedIntoExperimentInGroup.String(), "ppid2", "experiment_2", "1886780722")) + mockLogger.On("Debug", fmt.Sprintf(logging.UserNotBucketedIntoExperimentInGroup.String(), "ppid2", "experiment_2", "1886780722")) bucketedVariation, reason, _ := bucketer.Bucket("ppid2", experiment1, exclusionGroup) assert.Equal(t, experiment1.Variations["22222"], *bucketedVariation)