Skip to content

Optimize evaluation of treatments #210

New issue

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

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

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions engine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/splitio/go-split-commons/v6/engine/hash"

"github.com/splitio/go-toolkit/v5/hasher"
"github.com/splitio/go-toolkit/v5/injection"
"github.com/splitio/go-toolkit/v5/logging"
)

Expand All @@ -24,7 +25,8 @@ func (e *Engine) DoEvaluation(
key string,
bucketingKey string,
attributes map[string]interface{},
) (*string, string) {
ctx *injection.Context,
) (string, string) {
inRollOut := false
for _, condition := range split.Conditions() {
if !inRollOut && condition.ConditionType() == grammar.ConditionTypeRollout {
Expand All @@ -36,19 +38,19 @@ func (e *Engine) DoEvaluation(
" Returning default treatment", split.Name(), key,
))
defaultTreatment := split.DefaultTreatment()
return &defaultTreatment, impressionlabels.NotInSplit
return defaultTreatment, impressionlabels.NotInSplit
}
inRollOut = true
}
}

if condition.Matches(key, &bucketingKey, attributes) {
if condition.Matches(key, &bucketingKey, attributes, ctx) {
bucket := e.calculateBucket(split.Algo(), bucketingKey, split.Seed())
treatment := condition.CalculateTreatment(bucket)
return treatment, condition.Label()
}
}
return nil, impressionlabels.NoConditionMatched
return "", impressionlabels.NoConditionMatched
}

func (e *Engine) calculateBucket(algo int, bucketingKey string, seed int64) int {
Expand Down
24 changes: 12 additions & 12 deletions engine/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,13 @@ func TestTreatmentOnTrafficAllocation1(t *testing.T) {
},
}

split := grammar.NewSplit(&splitDTO, nil, logger)
split := grammar.NewSplit(&splitDTO, logger)

eng := Engine{}
eng.logger = logger
treatment, _ := eng.DoEvaluation(split, "aaaaaaklmnbv", "aaaaaaklmnbv", nil)
treatment, _ := eng.DoEvaluation(split, "aaaaaaklmnbv", "aaaaaaklmnbv", nil, nil)

if *treatment == "default" {
if treatment == "default" {
t.Error("It should not return default treatment.")
}
}
Expand Down Expand Up @@ -134,13 +134,13 @@ func TestTreatmentOnTrafficAllocation99(t *testing.T) {
},
}

split := grammar.NewSplit(&splitDTO, nil, logger)
split := grammar.NewSplit(&splitDTO, logger)

eng := Engine{}
eng.logger = logger
treatment, _ := eng.DoEvaluation(split, "aaaaaaklmnbv", "aaaaaaklmnbv", nil)
treatment, _ := eng.DoEvaluation(split, "aaaaaaklmnbv", "aaaaaaklmnbv", nil, nil)

if *treatment != "default" {
if treatment != "default" {
t.Error("It should return default treatment.")
}
}
Expand Down Expand Up @@ -236,15 +236,15 @@ func TestEvaluations(t *testing.T) {
t.Error("Data was not added for testing consistency")
}

split := grammar.NewSplit(&splitDTO, nil, logger)
split := grammar.NewSplit(&splitDTO, logger)

eng := Engine{}
eng.logger = logger

for _, tr := range treatmentsResults {
treatment, _ := eng.DoEvaluation(split, tr.Key, tr.Key, nil)
treatment, _ := eng.DoEvaluation(split, tr.Key, tr.Key, nil, nil)

if *treatment != tr.Result {
if treatment != tr.Result {
t.Error("Checking expected treatment " + tr.Result + " for key: " + tr.Key)
}
}
Expand All @@ -266,14 +266,14 @@ func TestNoConditionMatched(t *testing.T) {
Conditions: []dtos.ConditionDTO{},
}

split := grammar.NewSplit(&splitDTO, nil, logger)
split := grammar.NewSplit(&splitDTO, logger)

eng := Engine{}
eng.logger = logger

treatment, err := eng.DoEvaluation(split, "aaaaaaklmnbv", "aaaaaaklmnbv", nil)
treatment, err := eng.DoEvaluation(split, "aaaaaaklmnbv", "aaaaaaklmnbv", nil, nil)

if treatment != nil {
if len(treatment) > 0 {
t.Error("It should be nil.")
}

Expand Down
51 changes: 25 additions & 26 deletions engine/evaluator/evaluator.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"fmt"
"time"

"github.com/splitio/go-split-commons/v6/dtos"
"github.com/splitio/go-split-commons/v6/engine"
"github.com/splitio/go-split-commons/v6/engine/evaluator/impressionlabels"
"github.com/splitio/go-split-commons/v6/engine/grammar"
Expand Down Expand Up @@ -38,40 +37,41 @@ type Results struct {

// Evaluator struct is the main evaluator
type Evaluator struct {
splitStorage storage.SplitStorageConsumer
splitProducer grammar.SplitProducer
segmentStorage storage.SegmentStorageConsumer
eng *engine.Engine
logger logging.LoggerInterface
ctx *injection.Context
}

// NewEvaluator instantiates an Evaluator struct and returns a reference to it
func NewEvaluator(
splitStorage storage.SplitStorageConsumer,
splitProducer grammar.SplitProducer,
segmentStorage storage.SegmentStorageConsumer,
eng *engine.Engine,
logger logging.LoggerInterface,

) *Evaluator {
return &Evaluator{
splitStorage: splitStorage,
e := &Evaluator{
splitProducer: splitProducer,
segmentStorage: segmentStorage,
eng: eng,
logger: logger,
ctx: injection.NewContext(),
}

e.ctx.AddDependency("segmentStorage", e.segmentStorage)
e.ctx.AddDependency("evaluator", e)
return e
}

func (e *Evaluator) evaluateTreatment(key string, bucketingKey string, featureFlag string, splitDto *dtos.SplitDTO, attributes map[string]interface{}) *Result {
func (e *Evaluator) evaluateTreatment(key string, bucketingKey string, featureFlag string, split *grammar.Split, attributes map[string]interface{}) *Result {
var config *string
if splitDto == nil {
if split == nil {
e.logger.Warning(fmt.Sprintf("Feature flag %s not found, returning control.", featureFlag))
return &Result{Treatment: Control, Label: impressionlabels.SplitNotFound, Config: config}
}

ctx := injection.NewContext()
ctx.AddDependency("segmentStorage", e.segmentStorage)
ctx.AddDependency("evaluator", e)

split := grammar.NewSplit(splitDto, ctx, e.logger)

if split.Killed() {
e.logger.Warning(fmt.Sprintf(
"Feature flag %s has been killed, returning default treatment: %s",
Expand All @@ -93,24 +93,23 @@ func (e *Evaluator) evaluateTreatment(key string, bucketingKey string, featureFl
}
}

treatment, label := e.eng.DoEvaluation(split, key, bucketingKey, attributes)
if treatment == nil {
treatment, label := e.eng.DoEvaluation(split, key, bucketingKey, attributes, e.ctx)
if len(treatment) == 0 {
e.logger.Warning(fmt.Sprintf(
"No condition matched, returning default treatment: %s",
split.DefaultTreatment(),
))
defaultTreatment := split.DefaultTreatment()
treatment = &defaultTreatment
treatment = defaultTreatment
label = impressionlabels.NoConditionMatched
}

if _, ok := split.Configurations()[*treatment]; ok {
treatmentConfig := split.Configurations()[*treatment]
if treatmentConfig, ok := split.Configurations()[treatment]; ok {
config = &treatmentConfig
}

return &Result{
Treatment: *treatment,
Treatment: treatment,
Label: label,
SplitChangeNumber: split.ChangeNumber(),
Config: config,
Expand All @@ -121,12 +120,12 @@ func (e *Evaluator) evaluateTreatment(key string, bucketingKey string, featureFl
// EvaluateFeature returns a struct with the resulting treatment and extra information for the impression
func (e *Evaluator) EvaluateFeature(key string, bucketingKey *string, featureFlag string, attributes map[string]interface{}) *Result {
before := time.Now()
splitDto := e.splitStorage.Split(featureFlag)
split := e.splitProducer.GetSplit(featureFlag, e.logger)

if bucketingKey == nil {
bucketingKey = &key
}
result := e.evaluateTreatment(key, *bucketingKey, featureFlag, splitDto, attributes)
result := e.evaluateTreatment(key, *bucketingKey, featureFlag, split, attributes)
after := time.Now()

result.EvaluationTime = after.Sub(before)
Expand All @@ -136,17 +135,17 @@ func (e *Evaluator) EvaluateFeature(key string, bucketingKey *string, featureFla
// EvaluateFeatures returns a struct with the resulting treatment and extra information for the impression
func (e *Evaluator) EvaluateFeatures(key string, bucketingKey *string, featureFlags []string, attributes map[string]interface{}) Results {
var results = Results{
Evaluations: make(map[string]Result),
Evaluations: make(map[string]Result, len(featureFlags)),
EvaluationTime: 0,
}
before := time.Now()
splits := e.splitStorage.FetchMany(featureFlags)
splits := e.splitProducer.GetSplits(featureFlags, e.logger)

if bucketingKey == nil {
bucketingKey = &key
}
for _, featureFlag := range featureFlags {
results.Evaluations[featureFlag] = *e.evaluateTreatment(key, *bucketingKey, featureFlag, splits[featureFlag], attributes)
for featureFlag, split := range splits {
results.Evaluations[featureFlag] = *e.evaluateTreatment(key, *bucketingKey, featureFlag, split, attributes)
}

after := time.Now()
Expand All @@ -166,7 +165,7 @@ func (e *Evaluator) EvaluateFeatureByFlagSets(key string, bucketingKey *string,
// GetFeatureFlagNamesByFlagSets return flags that belong to some flag set
func (e *Evaluator) getFeatureFlagNamesByFlagSets(flagSets []string) []string {
uniqueFlags := make(map[string]struct{})
flagsBySets := e.splitStorage.GetNamesByFlagSets(flagSets)
flagsBySets := e.splitProducer.GetNamesByFlagSets(flagSets)
for set, flags := range flagsBySets {
if len(flags) == 0 {
e.logger.Warning(fmt.Sprintf("you passed %s Flag Set that does not contain cached feature flag names, please double check what Flag Sets are in use in the Split user interface.", set))
Expand Down
68 changes: 40 additions & 28 deletions engine/evaluator/evaluator_test.go
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
package evaluator

import (
"iter"
"testing"

"github.com/splitio/go-split-commons/v6/dtos"
"github.com/splitio/go-split-commons/v6/engine/grammar"
"github.com/splitio/go-split-commons/v6/flagsets"
"github.com/splitio/go-split-commons/v6/storage/inmemory/mutexmap"
"github.com/splitio/go-split-commons/v6/storage/mocks"
"github.com/splitio/go-split-commons/v6/storage/producer"

"github.com/splitio/go-toolkit/v5/datastructures/set"
"github.com/splitio/go-toolkit/v5/logging"
)

type mockStorage struct{}
type mockProducer struct{}

var mysplittest = &dtos.SplitDTO{
Algo: 2,
Expand Down Expand Up @@ -190,7 +192,7 @@ var mysplittest4 = &dtos.SplitDTO{
},
}

func (s *mockStorage) Split(
func (s *mockProducer) split(
feature string,
) *dtos.SplitDTO {
switch feature {
Expand All @@ -206,33 +208,43 @@ func (s *mockStorage) Split(
}
return nil
}
func (s *mockStorage) FetchMany(
feature []string,
) map[string]*dtos.SplitDTO {
splits := make(map[string]*dtos.SplitDTO)
splits["mysplittest"] = mysplittest
splits["mysplittest2"] = mysplittest2
splits["mysplittest3"] = mysplittest3
splits["mysplittest4"] = mysplittest4
splits["mysplittest5"] = nil
return splits

func (s *mockProducer) GetSplit(
feature string,
logger logging.LoggerInterface,
) *grammar.Split {
dto := s.split(feature)
if dto == nil {
return nil
}
return grammar.NewSplit(dto, logger)
}
func (s *mockStorage) All() []dtos.SplitDTO { return make([]dtos.SplitDTO, 0) }
func (s *mockStorage) SegmentNames() *set.ThreadUnsafeSet { return nil }
func (s *mockStorage) LargeSegmentNames() *set.ThreadUnsafeSet { return nil }
func (s *mockStorage) SplitNames() []string { return make([]string, 0) }
func (s *mockStorage) TrafficTypeExists(trafficType string) bool { return true }
func (s *mockStorage) ChangeNumber() (int64, error) { return 0, nil }
func (s *mockStorage) GetNamesByFlagSets(sets []string) map[string][]string {

func (s *mockProducer) GetSplits(
features []string,
logger logging.LoggerInterface,
) iter.Seq2[string, *grammar.Split] {
return func(yield func(string, *grammar.Split) bool) {
for _, feature := range features {
split := s.GetSplit(feature, logger)
if !yield(feature, split) {
return
}
}
}
}

func (s *mockProducer) GetNamesByFlagSets(sets []string) map[string][]string {
return make(map[string][]string)
}
func (s *mockStorage) GetAllFlagSetNames() []string { return make([]string, 0) }

var _ grammar.SplitProducer = (*mockProducer)(nil)

func TestSplitWithoutConfigurations(t *testing.T) {
logger := logging.NewLogger(nil)

evaluator := NewEvaluator(
&mockStorage{},
&mockProducer{},
nil,
nil,
logger)
Expand All @@ -253,7 +265,7 @@ func TestSplitWithtConfigurations(t *testing.T) {
logger := logging.NewLogger(nil)

evaluator := NewEvaluator(
&mockStorage{},
&mockProducer{},
nil,
nil,
logger)
Expand All @@ -274,7 +286,7 @@ func TestSplitWithtConfigurationsButKilled(t *testing.T) {
logger := logging.NewLogger(nil)

evaluator := NewEvaluator(
&mockStorage{},
&mockProducer{},
nil,
nil,
logger)
Expand All @@ -295,7 +307,7 @@ func TestSplitWithConfigurationsButKilledWithConfigsOnDefault(t *testing.T) {
logger := logging.NewLogger(nil)

evaluator := NewEvaluator(
&mockStorage{},
&mockProducer{},
nil,
nil,
logger)
Expand All @@ -316,7 +328,7 @@ func TestMultipleEvaluations(t *testing.T) {
logger := logging.NewLogger(nil)

evaluator := NewEvaluator(
&mockStorage{},
&mockProducer{},
nil,
nil,
logger)
Expand Down Expand Up @@ -416,7 +428,7 @@ func TestEvaluationByFlagSets(t *testing.T) {
}

evaluator := NewEvaluator(
mockedStorage,
producer.NewSimpleProducer(mockedStorage),
nil,
nil,
logger)
Expand Down Expand Up @@ -478,7 +490,7 @@ func TestEvaluationByFlagSetsASetEmpty(t *testing.T) {
}

evaluator := NewEvaluator(
mockedStorage,
producer.NewSimpleProducer(mockedStorage),
nil,
nil,
logger)
Expand Down
Loading