diff --git a/analyticsservice/analytics.go b/analyticsservice/analytics.go index 0014282..dd5da8e 100644 --- a/analyticsservice/analytics.go +++ b/analyticsservice/analytics.go @@ -26,7 +26,7 @@ const ( variationValueAttribute string = "featureValue" targetAttribute string = "target" sdkVersionAttribute string = "SDK_VERSION" - SdkVersion string = "0.1.19" + SdkVersion string = "0.1.20" sdkTypeAttribute string = "SDK_TYPE" sdkType string = "server" sdkLanguageAttribute string = "SDK_LANGUAGE" diff --git a/evaluation/evaluator.go b/evaluation/evaluator.go index 7de7716..43fe225 100644 --- a/evaluation/evaluator.go +++ b/evaluation/evaluator.go @@ -85,58 +85,44 @@ func NewEvaluator(query Query, postEvalCallback PostEvaluateCallback, logger log } func (e Evaluator) evaluateClause(clause *rest.Clause, target *Target) bool { - if clause == nil { - return false - } - - values := clause.Values - if len(values) == 0 { - return false - } - value := values[0] - - operator := clause.Op - if operator == "" { - e.logger.Warnf("Clause has no valid operator: Clause (%v)", clause) + if clause == nil || len(clause.Values) == 0 || clause.Op == "" { + e.logger.Debugf("Clause cannot be evaluated because operator is either nil, has no values or operation: Clause (%v)", clause) return false } + value := clause.Values[0] attrValue := getAttrValue(target, clause.Attribute) - if operator != segmentMatchOperator && !attrValue.IsValid() { - e.logger.Debugf("Operator is not a segment match and attribute value is not valid: Operator (%s), attributeVal (%s)", operator, attrValue.String()) + + if clause.Op != segmentMatchOperator && attrValue == "" { + e.logger.Debugf("Operator is not a segment match and attribute value is not valid: Operator (%s), attributeVal (%s)", clause.Op, attrValue) return false } - object := reflectValueToString(attrValue) - - switch operator { + switch clause.Op { case startsWithOperator: - return strings.HasPrefix(object, value) + return strings.HasPrefix(attrValue, value) case endsWithOperator: - return strings.HasSuffix(object, value) + return strings.HasSuffix(attrValue, value) case matchOperator: - found, err := regexp.MatchString(value, object) - if err != nil || !found { - return false - } - return true + found, err := regexp.MatchString(value, attrValue) + return err == nil && found case containsOperator: - return strings.Contains(object, value) + return strings.Contains(attrValue, value) case equalOperator: - return strings.EqualFold(object, value) + return strings.EqualFold(attrValue, value) case equalSensitiveOperator: - return object == value + return attrValue == value case inOperator: - for _, val := range values { - if val == object { + for _, val := range clause.Values { + if val == attrValue { return true } } return false case gtOperator: - return object > value + return attrValue > value case segmentMatchOperator: - return e.isTargetIncludedOrExcludedInSegment(values, target) + return e.isTargetIncludedOrExcludedInSegment(clause.Values, target) default: return false } diff --git a/evaluation/evaluator_test.go b/evaluation/evaluator_test.go index 99accd4..3a1871e 100644 --- a/evaluation/evaluator_test.go +++ b/evaluation/evaluator_test.go @@ -1799,3 +1799,255 @@ func TestEvaluator_JSONVariation(t *testing.T) { }) } } + +// BENCHMARK +func BenchmarkEvaluateClause_NilClause(b *testing.B) { + evaluator := Evaluator{} + var clause *rest.Clause = nil + target := &Target{ + Identifier: "harness", + } + for i := 0; i < b.N; i++ { + evaluator.evaluateClause(clause, target) + } +} + +func BenchmarkEvaluateClause_EmptyOperator(b *testing.B) { + evaluator := Evaluator{} + clause := &rest.Clause{ + Op: "", + Values: []string{"harness"}, + } + for i := 0; i < b.N; i++ { + evaluator.evaluateClause(clause, nil) + } +} + +func BenchmarkEvaluateClause_NilValues(b *testing.B) { + evaluator := Evaluator{} + clause := &rest.Clause{ + Values: nil, + } + for i := 0; i < b.N; i++ { + evaluator.evaluateClause(clause, nil) + } +} + +func BenchmarkEvaluateClause_EmptyValues(b *testing.B) { + evaluator := Evaluator{} + clause := &rest.Clause{ + Values: []string{}, + } + for i := 0; i < b.N; i++ { + evaluator.evaluateClause(clause, nil) + } +} + +func BenchmarkEvaluateClause_WrongOperator(b *testing.B) { + evaluator := Evaluator{} + clause := &rest.Clause{ + Attribute: "identifier", + Op: "greaterthan", + Values: []string{"harness"}, + } + target := &Target{ + Identifier: "harness", + } + for i := 0; i < b.N; i++ { + evaluator.evaluateClause(clause, target) + } +} + +func BenchmarkEvaluateClause_EmptyAttribute(b *testing.B) { + evaluator := Evaluator{} + clause := &rest.Clause{ + Attribute: "", + Op: "equalOperator", + Values: []string{"harness"}, + } + target := &Target{ + Identifier: "harness", + } + for i := 0; i < b.N; i++ { + evaluator.evaluateClause(clause, target) + } +} + +func BenchmarkEvaluateClause_MatchOperator(b *testing.B) { + evaluator := Evaluator{} + clause := &rest.Clause{ + Attribute: "identifier", + Op: "matchOperator", + Values: []string{"^harness$"}, + } + target := &Target{ + Identifier: "harness", + } + for i := 0; i < b.N; i++ { + evaluator.evaluateClause(clause, target) + } +} + +func BenchmarkEvaluateClause_MatchOperatorError(b *testing.B) { + evaluator := Evaluator{} + clause := &rest.Clause{ + Attribute: "identifier", + Op: "matchOperator", + Values: []string{"^harness(wings$"}, + } + target := &Target{ + Identifier: "harness", + } + for i := 0; i < b.N; i++ { + evaluator.evaluateClause(clause, target) + } +} + +func BenchmarkEvaluateClause_InOperator(b *testing.B) { + evaluator := Evaluator{} + clause := &rest.Clause{ + Attribute: "identifier", + Op: "inOperator", + Values: []string{"harness", "wings-software"}, + } + target := &Target{ + Identifier: "harness", + } + for i := 0; i < b.N; i++ { + evaluator.evaluateClause(clause, target) + } +} + +func BenchmarkEvaluateClause_InOperatorNotFound(b *testing.B) { + evaluator := Evaluator{} + clause := &rest.Clause{ + Attribute: "identifier", + Op: "inOperator", + Values: []string{"harness1", "wings-software"}, + } + target := &Target{ + Identifier: "harness", + } + for i := 0; i < b.N; i++ { + evaluator.evaluateClause(clause, target) + } +} + +func BenchmarkEvaluateClause_EqualOperator(b *testing.B) { + evaluator := Evaluator{} + clause := &rest.Clause{ + Attribute: "identifier", + Op: "equalOperator", + Values: []string{"harness"}, + } + target := &Target{ + Identifier: "harness", + } + for i := 0; i < b.N; i++ { + evaluator.evaluateClause(clause, target) + } +} + +func BenchmarkEvaluateClause_EqualSensitiveOperator(b *testing.B) { + evaluator := Evaluator{} + clause := &rest.Clause{ + Attribute: "identifier", + Op: "equalSensitiveOperator", + Values: []string{"harness"}, + } + target := &Target{ + Identifier: "Harness", + } + for i := 0; i < b.N; i++ { + evaluator.evaluateClause(clause, target) + } +} + +func BenchmarkEvaluateClause_GTOperator(b *testing.B) { + evaluator := Evaluator{} + clause := &rest.Clause{ + Attribute: "identifier", + Op: "gtOperator", + Values: []string{"A"}, + } + target := &Target{ + Identifier: "B", + } + for i := 0; i < b.N; i++ { + evaluator.evaluateClause(clause, target) + } +} + +func BenchmarkEvaluateClause_GTOperatorNegative(b *testing.B) { + evaluator := Evaluator{} + clause := &rest.Clause{ + Attribute: "identifier", + Op: "gtOperator", + Values: []string{"B"}, + } + target := &Target{ + Identifier: "A", + } + for i := 0; i < b.N; i++ { + evaluator.evaluateClause(clause, target) + } +} + +func BenchmarkEvaluateClause_StartsWithOperator(b *testing.B) { + evaluator := Evaluator{} + clause := &rest.Clause{ + Attribute: "identifier", + Op: "startsWithOperator", + Values: []string{"harness"}, + } + target := &Target{ + Identifier: "harness - wings software", + } + for i := 0; i < b.N; i++ { + evaluator.evaluateClause(clause, target) + } +} + +func BenchmarkEvaluateClause_EndsWithOperator(b *testing.B) { + evaluator := Evaluator{} + clause := &rest.Clause{ + Attribute: "identifier", + Op: "endsWithOperator", + Values: []string{"harness"}, + } + target := &Target{ + Identifier: "wings software - harness", + } + for i := 0; i < b.N; i++ { + evaluator.evaluateClause(clause, target) + } +} + +func BenchmarkEvaluateClause_ContainsOperator(b *testing.B) { + evaluator := Evaluator{} + clause := &rest.Clause{ + Attribute: "identifier", + Op: "containsOperator", + Values: []string{"harness"}, + } + target := &Target{ + Identifier: "wings harness software", + } + for i := 0; i < b.N; i++ { + evaluator.evaluateClause(clause, target) + } +} + +func BenchmarkEvaluateClause_SegmentMatchOperator(b *testing.B) { + evaluator := Evaluator{query: testRepo} + clause := &rest.Clause{ + Op: "segmentMatchOperator", + Values: []string{"beta"}, + } + target := &Target{ + Identifier: "harness", + } + for i := 0; i < b.N; i++ { + evaluator.evaluateClause(clause, target) + } +} diff --git a/evaluation/util.go b/evaluation/util.go index 1625f57..54e3e64 100644 --- a/evaluation/util.go +++ b/evaluation/util.go @@ -2,64 +2,33 @@ package evaluation import ( "fmt" - "github.com/harness/ff-golang-server-sdk/sdk_codes" - "reflect" - "strconv" "strings" + "github.com/harness/ff-golang-server-sdk/sdk_codes" + "github.com/harness/ff-golang-server-sdk/log" "github.com/harness/ff-golang-server-sdk/rest" "github.com/spaolacci/murmur3" ) -func getAttrValue(target *Target, attr string) reflect.Value { - var value reflect.Value +func getAttrValue(target *Target, attr string) string { if target == nil { - return value + return "" } - attrs := make(map[string]interface{}) - if target.Attributes != nil { - attrs = *target.Attributes - } - - attrVal, ok := attrs[attr] // first check custom attributes - if ok { - value = reflect.ValueOf(attrVal) - } else { - // We only have two fields here, so we will access the fields directly, and use reflection if we start adding - // more in the future - switch strings.ToLower(attr) { - case "identifier": - value = reflect.ValueOf(target.Identifier) - case "name": - value = reflect.ValueOf(target.Name) - default: - value = reflect.ValueOf("") - } - } - return value -} - -func reflectValueToString(val reflect.Value) string { - stringValue := "" - switch val.Kind() { - case reflect.Int, reflect.Int64: - stringValue = strconv.FormatInt(val.Int(), 10) - case reflect.Bool: - stringValue = strconv.FormatBool(val.Bool()) - case reflect.String: - stringValue = val.String() - case reflect.Array, reflect.Chan, reflect.Complex128, reflect.Complex64, reflect.Func, reflect.Interface, - reflect.Invalid, reflect.Ptr, reflect.Slice, reflect.Struct, reflect.Uintptr, reflect.UnsafePointer, - reflect.Float32, reflect.Float64, reflect.Int16, reflect.Int32, reflect.Int8, reflect.Map, reflect.Uint, - reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uint8: - stringValue = fmt.Sprintf("%v", val) + switch strings.ToLower(attr) { + case "identifier": + return target.Identifier + case "name": + return target.Name default: - // Use string formatting as last ditch effort for any unexpected values - stringValue = fmt.Sprintf("%v", val) + if target.Attributes != nil { + if val, ok := (*target.Attributes)[attr]; ok { + return fmt.Sprint(val) + } + } } - return stringValue + return "" } func findVariation(variations []rest.Variation, identifier string) (rest.Variation, error) { @@ -85,20 +54,20 @@ func getNormalizedNumber(identifier, bucketBy string) int { func isEnabled(target *Target, bucketBy string, percentage int) bool { value := getAttrValue(target, bucketBy) - identifier := value.String() - if identifier == "" { - var oldBB = bucketBy - bucketBy = "identifier" - value = getAttrValue(target, bucketBy) - identifier = value.String() - if identifier == "" { + if value == "" { + // If the original bucketBy attribute is not found, fallback to "identifier". + value = getAttrValue(target, "identifier") + if value == "" { return false } - log.Warnf("%s BucketBy attribute not found in target attributes, falling back to 'identifier': missing=%s, using value=%s", sdk_codes.MissingBucketBy, oldBB, identifier) + log.Warnf("%s BucketBy attribute not found in target attributes, falling back to 'identifier': missing=%s, using value=%s", sdk_codes.MissingBucketBy, bucketBy, value) + bucketBy = "identifier" } - bucketID := getNormalizedNumber(identifier, bucketBy) - log.Debugf("MM3 percentage_check=%d bucket_by=%s value=%s bucket=%d", percentage, bucketBy, identifier, bucketID) + // Calculate bucketID once with the resolved value and bucketBy. + bucketID := getNormalizedNumber(value, bucketBy) + log.Debugf("MM3 percentage_check=%d bucket_by=%s value=%s bucket=%d", percentage, bucketBy, value, bucketID) + return percentage > 0 && bucketID <= percentage } diff --git a/evaluation/util_test.go b/evaluation/util_test.go index 4b6920a..fe2daaf 100644 --- a/evaluation/util_test.go +++ b/evaluation/util_test.go @@ -2,7 +2,6 @@ package evaluation import ( "reflect" - "strconv" "testing" "github.com/harness/ff-golang-server-sdk/rest" @@ -14,119 +13,35 @@ func Test_getAttrValueIsNil(t *testing.T) { attr string } tests := []struct { - name string - args args - want reflect.Value + name string + args args + wantStr string }{ { - name: "when target is nil should return Value{}", + name: "when target is nil should return empty string", args: args{ - attr: identifier, + attr: "identifier", }, - want: reflect.Value{}, + wantStr: "", }, { - name: "wrong attribute should return ValueOf('')", + name: "wrong attribute should return empty string", args: args{ target: &Target{ - Identifier: harness, + Identifier: "harness", Attributes: &map[string]interface{}{ "email": "enver.bisevac@harness.io", }, }, attr: "no_identifier", }, - want: reflect.ValueOf(""), - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := getAttrValue(tt.args.target, tt.args.attr); !reflect.DeepEqual(got, tt.want) { - t.Errorf("getAttrValue() = %v, want %v", got, tt.want) - } - }) - } -} - -func Test_reflectValueToString(t *testing.T) { - type args struct { - attr interface{} - } - tests := []struct { - name string - args args - want string - }{ - { - name: "string value", - args: args{ - attr: "email@harness.io", - }, - want: "email@harness.io", - }, - { - name: "int value", - args: args{ - attr: 20, - }, - want: "20", - }, - { - name: "int64 value", - args: args{ - attr: int64(20), - }, - want: "20", - }, - { - name: "float32 value", - args: args{ - attr: float32(20), - }, - want: "20", - }, - { - name: "float32 digit value", - args: args{ - attr: float32(20.5678), - }, - want: "20.5678", - }, - { - name: "float64 value", - args: args{ - attr: float64(20), - }, - want: "20", - }, - { - name: "float64 digit value", - args: args{ - attr: float64(20.5678), - }, - want: "20.5678", - }, - { - name: "bool true value", - args: args{ - attr: true, - }, - want: "true", - }, - { - name: "bool false value", - args: args{ - attr: false, - }, - want: "false", + wantStr: "", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - value := reflect.ValueOf(tt.args.attr) - got := reflectValueToString(value) - if got != tt.want { - t.Errorf("valueToString() = %v, want %v", got, tt.want) + if got := getAttrValue(tt.args.target, tt.args.attr); got != tt.wantStr { + t.Errorf("getAttrValue() = %v, want %v", got, tt.wantStr) } }) } @@ -141,108 +56,86 @@ func Test_getAttrValue(t *testing.T) { tests := []struct { name string args args - want reflect.Value wantStr string }{ { name: "check identifier", args: args{ target: &Target{ - Identifier: harness, + Identifier: "harness", }, - attr: identifier, + attr: "identifier", }, - want: reflect.ValueOf(harness), wantStr: "harness", }, { name: "check name", args: args{ target: &Target{ - Name: harness, + Name: "harness", }, attr: "name", }, - want: reflect.ValueOf(harness), wantStr: "harness", }, { name: "check attributes", args: args{ target: &Target{ - Identifier: identifier, + Identifier: "identifier", Attributes: &map[string]interface{}{ "email": email, }, }, attr: "email", }, - want: reflect.ValueOf(email), wantStr: email, }, { name: "check integer attributes", args: args{ target: &Target{ - Identifier: identifier, + Identifier: "identifier", Attributes: &map[string]interface{}{ "age": 123, }, }, attr: "age", }, - want: reflect.ValueOf(123), wantStr: "123", }, { name: "check int64 attributes", args: args{ target: &Target{ - Identifier: identifier, + Identifier: "identifier", Attributes: &map[string]interface{}{ "age": int64(123), }, }, attr: "age", }, - want: reflect.ValueOf(int64(123)), wantStr: "123", }, { name: "check boolean attributes", args: args{ target: &Target{ - Identifier: identifier, + Identifier: "identifier", Attributes: &map[string]interface{}{ "active": true, }, }, attr: "active", }, - want: reflect.ValueOf(true), wantStr: "true", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := getAttrValue(tt.args.target, tt.args.attr) - if !reflect.DeepEqual(got.Interface(), tt.want.Interface()) { - t.Errorf("getAttrValue() = %v, want %v", got, tt.want) - } - - expObjString := "" - //nolint - switch got.Kind() { - case reflect.Int, reflect.Int64: - expObjString = strconv.FormatInt(got.Int(), 10) - case reflect.Bool: - expObjString = strconv.FormatBool(got.Bool()) - case reflect.String: - expObjString = got.String() - } - - if expObjString != tt.wantStr { - t.Errorf("getAttrValue() expObjString= %v, want %v", got.String(), tt.wantStr) + gotStr := getAttrValue(tt.args.target, tt.args.attr) + if gotStr != tt.wantStr { + t.Errorf("getAttrValue() = %v, want %v", gotStr, tt.wantStr) } }) }