FFM-11332 - Refactor evaluation logic to remove inefficiencies in the…

… GetAttr(ibute) function (#155) * Refactor to remove further inefficiencies in getAttr func
harness · Apr 29, 2024 · 3cececf · 3cececf
1 parent 47f888f
commit 3cececf
Show file tree

Hide file tree

Showing 8 changed files with 133 additions and 79 deletions.
diff --git a/.harness/Build_ff_golang_server_sdk_1713530975185-pr-trigger-input-set-1713530976580.yaml b/.harness/Build_ff_golang_server_sdk_1713530975185-pr-trigger-input-set-1713530976580.yaml
diff --git a/.harness/Build_ff_golang_server_sdk_1713530975185-push-trigger-input-set-1713530978099.yaml b/.harness/Build_ff_golang_server_sdk_1713530975185-push-trigger-input-set-1713530978099.yaml
diff --git a/.harness/Build_ff_golang_server_sdk_1713968153901-pr-trigger-input-set-1713968155094.yaml b/.harness/Build_ff_golang_server_sdk_1713968153901-pr-trigger-input-set-1713968155094.yaml
diff --git a/.harness/Build_ff_golang_server_sdk_1713968153901-push-trigger-input-set-1713968156768.yaml b/.harness/Build_ff_golang_server_sdk_1713968153901-push-trigger-input-set-1713968156768.yaml
diff --git a/evaluation/evaluator.go b/evaluation/evaluator.go
@@ -86,15 +86,13 @@ func NewEvaluator(query Query, postEvalCallback PostEvaluateCallback, logger log
 
 func (e Evaluator) evaluateClause(clause *rest.Clause, target *Target) bool {
 	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 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
 	}
 

diff --git a/evaluation/evaluator_test.go b/evaluation/evaluator_test.go
@@ -1988,7 +1988,7 @@ func TestEvaluator_JSONVariation(t *testing.T) {
 
 // BENCHMARK
 func BenchmarkEvaluateClause_NilClause(b *testing.B) {
-	evaluator := Evaluator{}
+	evaluator := Evaluator{logger: logger.NoOpLogger{}}
 	var clause *rest.Clause = nil
 	target := &Target{
 		Identifier: "harness",
@@ -1999,7 +1999,7 @@ func BenchmarkEvaluateClause_NilClause(b *testing.B) {
 }
 
 func BenchmarkEvaluateClause_EmptyOperator(b *testing.B) {
-	evaluator := Evaluator{}
+	evaluator := Evaluator{logger: logger.NoOpLogger{}}
 	clause := &rest.Clause{
 		Op:     "",
 		Values: []string{"harness"},
@@ -2010,7 +2010,7 @@ func BenchmarkEvaluateClause_EmptyOperator(b *testing.B) {
 }
 
 func BenchmarkEvaluateClause_NilValues(b *testing.B) {
-	evaluator := Evaluator{}
+	evaluator := Evaluator{logger: logger.NoOpLogger{}}
 	clause := &rest.Clause{
 		Values: nil,
 	}
@@ -2020,7 +2020,7 @@ func BenchmarkEvaluateClause_NilValues(b *testing.B) {
 }
 
 func BenchmarkEvaluateClause_EmptyValues(b *testing.B) {
-	evaluator := Evaluator{}
+	evaluator := Evaluator{logger: logger.NoOpLogger{}}
 	clause := &rest.Clause{
 		Values: []string{},
 	}
@@ -2030,7 +2030,7 @@ func BenchmarkEvaluateClause_EmptyValues(b *testing.B) {
 }
 
 func BenchmarkEvaluateClause_WrongOperator(b *testing.B) {
-	evaluator := Evaluator{}
+	evaluator := Evaluator{logger: logger.NoOpLogger{}}
 	clause := &rest.Clause{
 		Attribute: "identifier",
 		Op:        "greaterthan",
@@ -2045,7 +2045,7 @@ func BenchmarkEvaluateClause_WrongOperator(b *testing.B) {
 }
 
 func BenchmarkEvaluateClause_EmptyAttribute(b *testing.B) {
-	evaluator := Evaluator{}
+	evaluator := Evaluator{logger: logger.NoOpLogger{}}
 	clause := &rest.Clause{
 		Attribute: "",
 		Op:        "equalOperator",
@@ -2060,7 +2060,7 @@ func BenchmarkEvaluateClause_EmptyAttribute(b *testing.B) {
 }
 
 func BenchmarkEvaluateClause_MatchOperator(b *testing.B) {
-	evaluator := Evaluator{}
+	evaluator := Evaluator{logger: logger.NoOpLogger{}}
 	clause := &rest.Clause{
 		Attribute: "identifier",
 		Op:        "matchOperator",
@@ -2075,7 +2075,7 @@ func BenchmarkEvaluateClause_MatchOperator(b *testing.B) {
 }
 
 func BenchmarkEvaluateClause_MatchOperatorError(b *testing.B) {
-	evaluator := Evaluator{}
+	evaluator := Evaluator{logger: logger.NoOpLogger{}}
 	clause := &rest.Clause{
 		Attribute: "identifier",
 		Op:        "matchOperator",
@@ -2090,7 +2090,7 @@ func BenchmarkEvaluateClause_MatchOperatorError(b *testing.B) {
 }
 
 func BenchmarkEvaluateClause_InOperator(b *testing.B) {
-	evaluator := Evaluator{}
+	evaluator := Evaluator{logger: logger.NoOpLogger{}}
 	clause := &rest.Clause{
 		Attribute: "identifier",
 		Op:        "inOperator",
@@ -2105,7 +2105,7 @@ func BenchmarkEvaluateClause_InOperator(b *testing.B) {
 }
 
 func BenchmarkEvaluateClause_InOperatorNotFound(b *testing.B) {
-	evaluator := Evaluator{}
+	evaluator := Evaluator{logger: logger.NoOpLogger{}}
 	clause := &rest.Clause{
 		Attribute: "identifier",
 		Op:        "inOperator",
@@ -2120,7 +2120,7 @@ func BenchmarkEvaluateClause_InOperatorNotFound(b *testing.B) {
 }
 
 func BenchmarkEvaluateClause_EqualOperator(b *testing.B) {
-	evaluator := Evaluator{}
+	evaluator := Evaluator{logger: logger.NoOpLogger{}}
 	clause := &rest.Clause{
 		Attribute: "identifier",
 		Op:        "equalOperator",
@@ -2135,7 +2135,7 @@ func BenchmarkEvaluateClause_EqualOperator(b *testing.B) {
 }
 
 func BenchmarkEvaluateClause_EqualSensitiveOperator(b *testing.B) {
-	evaluator := Evaluator{}
+	evaluator := Evaluator{logger: logger.NoOpLogger{}}
 	clause := &rest.Clause{
 		Attribute: "identifier",
 		Op:        "equalSensitiveOperator",
@@ -2150,7 +2150,7 @@ func BenchmarkEvaluateClause_EqualSensitiveOperator(b *testing.B) {
 }
 
 func BenchmarkEvaluateClause_GTOperator(b *testing.B) {
-	evaluator := Evaluator{}
+	evaluator := Evaluator{logger: logger.NoOpLogger{}}
 	clause := &rest.Clause{
 		Attribute: "identifier",
 		Op:        "gtOperator",
@@ -2165,7 +2165,7 @@ func BenchmarkEvaluateClause_GTOperator(b *testing.B) {
 }
 
 func BenchmarkEvaluateClause_GTOperatorNegative(b *testing.B) {
-	evaluator := Evaluator{}
+	evaluator := Evaluator{logger: logger.NoOpLogger{}}
 	clause := &rest.Clause{
 		Attribute: "identifier",
 		Op:        "gtOperator",
@@ -2180,7 +2180,7 @@ func BenchmarkEvaluateClause_GTOperatorNegative(b *testing.B) {
 }
 
 func BenchmarkEvaluateClause_StartsWithOperator(b *testing.B) {
-	evaluator := Evaluator{}
+	evaluator := Evaluator{logger: logger.NoOpLogger{}}
 	clause := &rest.Clause{
 		Attribute: "identifier",
 		Op:        "startsWithOperator",
@@ -2195,7 +2195,7 @@ func BenchmarkEvaluateClause_StartsWithOperator(b *testing.B) {
 }
 
 func BenchmarkEvaluateClause_EndsWithOperator(b *testing.B) {
-	evaluator := Evaluator{}
+	evaluator := Evaluator{logger: logger.NoOpLogger{}}
 	clause := &rest.Clause{
 		Attribute: "identifier",
 		Op:        "endsWithOperator",
@@ -2210,7 +2210,7 @@ func BenchmarkEvaluateClause_EndsWithOperator(b *testing.B) {
 }
 
 func BenchmarkEvaluateClause_ContainsOperator(b *testing.B) {
-	evaluator := Evaluator{}
+	evaluator := Evaluator{logger: logger.NoOpLogger{}}
 	clause := &rest.Clause{
 		Attribute: "identifier",
 		Op:        "containsOperator",
@@ -2225,7 +2225,7 @@ func BenchmarkEvaluateClause_ContainsOperator(b *testing.B) {
 }
 
 func BenchmarkEvaluateClause_SegmentMatchOperator(b *testing.B) {
-	evaluator := Evaluator{query: testRepo}
+	evaluator := Evaluator{query: testRepo, logger: logger.NoOpLogger{}}
 	clause := &rest.Clause{
 		Op:     "segmentMatchOperator",
 		Values: []string{"beta"},

diff --git a/evaluation/util.go b/evaluation/util.go
@@ -2,8 +2,11 @@ package evaluation
 
 import (
 	"fmt"
+	"strconv"
 	"strings"
 
+	jsoniter "github.com/json-iterator/go"
+
 	"github.com/harness/ff-golang-server-sdk/sdk_codes"
 
 	"github.com/harness/ff-golang-server-sdk/log"
@@ -12,19 +15,36 @@ import (
 )
 
 func getAttrValue(target *Target, attr string) string {
-	if target == nil {
+	if target == nil || attr == "" {
 		return ""
 	}
 
-	switch strings.ToLower(attr) {
+	switch attr {
 	case "identifier":
 		return target.Identifier
 	case "name":
 		return target.Name
 	default:
 		if target.Attributes != nil {
 			if val, ok := (*target.Attributes)[attr]; ok {
-				return fmt.Sprint(val)
+				switch v := val.(type) {
+				case string:
+					return v
+				case int:
+					return strconv.Itoa(v)
+				case float64:
+					return strconv.FormatFloat(v, 'f', -1, 64)
+				case bool:
+					return strconv.FormatBool(v)
+				case map[string]interface{}:
+					marshalledValue, err := jsoniter.MarshalToString(v)
+					if err != nil {
+						return fmt.Sprint(v)
+					}
+					return marshalledValue
+				default:
+					return fmt.Sprint(v)
+				}
 			}
 		}
 	}

diff --git a/evaluation/util_test.go b/evaluation/util_test.go
@@ -437,3 +437,95 @@ func Test_isTargetInList(t *testing.T) {
 		})
 	}
 }
+
+// Benchmark scenarios
+func BenchmarkGetAttrValueNilTarget(b *testing.B) {
+	for i := 0; i < b.N; i++ {
+		getAttrValue(nil, "identifier")
+	}
+}
+
+func BenchmarkGetAttrValueEmptyAttribute(b *testing.B) {
+	var target Target
+	for i := 0; i < b.N; i++ {
+		getAttrValue(&target, "")
+	}
+}
+
+func BenchmarkGetAttrValueIdentifier(b *testing.B) {
+	var target = Target{Identifier: "1234"}
+	for i := 0; i < b.N; i++ {
+		getAttrValue(&target, "identifier")
+	}
+}
+
+func BenchmarkGetAttrValueName(b *testing.B) {
+	var target = Target{Name: "targetName"}
+	for i := 0; i < b.N; i++ {
+		getAttrValue(&target, "name")
+	}
+}
+
+func BenchmarkGetAttrValueUnmatched(b *testing.B) {
+	var target = Target{}
+	for i := 0; i < b.N; i++ {
+		getAttrValue(&target, "unmatched")
+	}
+}
+
+func BenchmarkGetAttrValueStringAttr(b *testing.B) {
+	attributes := map[string]interface{}{"city": "New York"}
+	var target = Target{Attributes: &attributes}
+	for i := 0; i < b.N; i++ {
+		getAttrValue(&target, "city")
+	}
+}
+
+func BenchmarkGetAttrValueIntegerAttr(b *testing.B) {
+	attributes := map[string]interface{}{"age": 30}
+	var target = Target{Attributes: &attributes}
+	for i := 0; i < b.N; i++ {
+		getAttrValue(&target, "age")
+	}
+}
+
+func BenchmarkGetAttrValueFloatAttr(b *testing.B) {
+	attributes := map[string]interface{}{"temperature": 98.6}
+	var target = Target{Attributes: &attributes}
+	for i := 0; i < b.N; i++ {
+		getAttrValue(&target, "temperature")
+	}
+}
+
+func BenchmarkGetAttrValueBoolAttr(b *testing.B) {
+	attributes := map[string]interface{}{"active": true}
+	var target = Target{Attributes: &attributes}
+	for i := 0; i < b.N; i++ {
+		getAttrValue(&target, "active")
+	}
+}
+
+func BenchmarkGetAttrValueBasicJSON(b *testing.B) {
+	basicJSON := map[string]interface{}{
+		"myName": "Stephen",
+	}
+	attributes := map[string]interface{}{"basicJson": basicJSON}
+	var target = Target{Attributes: &attributes}
+	for i := 0; i < b.N; i++ {
+		getAttrValue(&target, "basicJson")
+	}
+}
+
+func BenchmarkGetAttrValueComplexJSON(b *testing.B) {
+	complexJSON := map[string]interface{}{
+		"config": map[string]interface{}{
+			"setting": "on",
+			"level":   5,
+		},
+	}
+	attributes := map[string]interface{}{"complexJSON": complexJSON}
+	var target = Target{Attributes: &attributes}
+	for i := 0; i < b.N; i++ {
+		getAttrValue(&target, "complexJSON")
+	}
+}