Skip to content

Commit

Permalink
make decision group assignment override the visitor one
Browse files Browse the repository at this point in the history
  • Loading branch information
guillaume-abtasty committed Jan 11, 2024
1 parent 41481a3 commit 84fc37f
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 51 deletions.
10 changes: 8 additions & 2 deletions allocation.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,19 @@ func genHashFloat(visitorID string, vgID string) (float32, error) {
}

// getRandomAllocation returns a random allocation for a variationGroup
func getRandomAllocation(visitorID string, variationGroup *VariationGroup, isCumulativeAlloc bool) (*Variation, error) {
func getRandomAllocation(visitorID string, decisionGroup string, variationGroup *VariationGroup, isCumulativeAlloc bool) (*Variation, error) {
// performance shortcut to prevent hash generation
if len(variationGroup.Variations) == 1 && variationGroup.Variations[0].Allocation == 100 {
return variationGroup.Variations[0], nil
}

z, err := genHashFloat(visitorID, variationGroup.ID)
// Use decision group by default for decision hash, otherwise use visitor ID
decisionID := visitorID
if decisionGroup != "" {
decisionID = decisionGroup
}

z, err := genHashFloat(decisionID, variationGroup.ID)
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions allocation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
func testVariationGroupAlloc(vg VariationGroup, t *testing.T, isCumulativeAlloc bool) {
counts := []int{}
for i := 1; i < 100000; i++ {
vAlloc, err := getRandomAllocation(strconv.Itoa(rand.Int()), &vg, isCumulativeAlloc)
vAlloc, err := getRandomAllocation(strconv.Itoa(rand.Int()), "", &vg, isCumulativeAlloc)

if err != nil {
t.Errorf("error when getting alloc: %v", err)
Expand Down Expand Up @@ -146,7 +146,7 @@ func TestVariationAllocation(t *testing.T) {
allocErrors := []error{}
nbTrials := 100000
for i := 1; i < nbTrials; i++ {
_, err := getRandomAllocation(strconv.Itoa(rand.Int()), &variationsGroupInfo, false)
_, err := getRandomAllocation(strconv.Itoa(rand.Int()), "", &variationsGroupInfo, false)

if err != nil {
allocErrors = append(allocErrors, err)
Expand Down
75 changes: 48 additions & 27 deletions decision.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ type assignmentResult struct {
}

type allVisitorAssignments struct {
Standard *VisitorAssignments
Anonymous *VisitorAssignments
Standard *VisitorAssignments
Anonymous *VisitorAssignments
DecisionGroup *VisitorAssignments
}

func getCache(
Expand Down Expand Up @@ -76,7 +77,6 @@ func getCache(
}(cacheChan)
}

var assignmentsDG *VisitorAssignments
for i := 0; i < nbRoutines; i++ {
r := <-cacheChan
switch r.visitorType {
Expand All @@ -85,25 +85,38 @@ func getCache(
case "anonymous":
allAssignments.Anonymous = r.result
case "decisionGroup":
assignmentsDG = r.result
allAssignments.DecisionGroup = r.result
}
err = r.err
}

// Assign decision group assignments to visitor
if assignmentsDG != nil {
if allAssignments.Standard == nil {
allAssignments.Standard = &VisitorAssignments{Assignments: map[string]*VisitorCache{}}
}
// Override standard assignments with decision group ones
for k, v := range assignmentsDG.getAssignments() {
allAssignments.Standard.Assignments[k] = v
}
}

return allAssignments, err
}

// Refactored helper function to handle saving of cache assignments
func saveCacheAssignments(
wg *sync.WaitGroup,
handlers DecisionHandlers,
envID string,
id string,
assignments map[string]*VisitorCache,
now time.Time,
logInfo string,
) {
wg.Add(1)
go func() {
defer wg.Done()
logger.Logf(InfoLevel, "saving assignments cache for %s: %s", logInfo, id)
err := handlers.SaveCache(envID, id, &VisitorAssignments{
Timestamp: now.Unix(),
Assignments: assignments,
})
if err != nil {
logger.Logf(ErrorLevel, "error occurred on cache saving for %s: %v", id, err)
}
}()
}

// GetDecision return a decision response from visitor & environment infos
func GetDecision(
visitorInfos Visitor,
Expand All @@ -121,8 +134,8 @@ func GetDecision(
triggerHit := options.TriggerHit

if decisionGroup != "" {
// 1bis. Compute decision group if set
decisionGroup = envID + ":" + base64.StdEncoding.EncodeToString([]byte(decisionGroup))
// encode decision group if set
decisionGroup = base64.StdEncoding.EncodeToString([]byte(decisionGroup))
}

decisionResponse := &decision_response.DecisionResponse{}
Expand Down Expand Up @@ -156,7 +169,7 @@ func GetDecision(
// and if 1vis1test or XP-C is enabled or at least one campaign as multiple variations,
enableCache := environmentInfos.CacheEnabled && (hasMultipleVariations || environmentInfos.SingleAssignment || environmentInfos.UseReconciliation)

// 2.a Run parallel execution
// 2.b Load all cache in parallel
allCacheAssignments := &allVisitorAssignments{}
var err error
if enableCache {
Expand Down Expand Up @@ -203,22 +216,32 @@ func GetDecision(
}
}

var vid string
isNew := false
isNewAnonymous := false
existingAssignment, ok := allCacheAssignments.Standard.getAssignment(vg.ID)
existingAssignmentAnonymous, okAnonymous := allCacheAssignments.Anonymous.getAssignment(vg.ID)
existingAssignmentDecisionGroup, okDecisionGroup := allCacheAssignments.DecisionGroup.getAssignment(vg.ID)

var existingVariation *Variation
var existingAnonymousVariation *Variation
if ok || okAnonymous {
var existingDecisionGroupVariation *Variation

if ok || okAnonymous || okDecisionGroup {
for _, v := range vg.Variations {
if ok && v.ID == existingAssignment.VariationID {
existingVariation = v
}
if okAnonymous && v.ID == existingAssignmentAnonymous.VariationID {
existingAnonymousVariation = v
}
if okDecisionGroup && v.ID == existingAssignmentDecisionGroup.VariationID {
existingDecisionGroupVariation = v
}
}

// If decision group variation is found, override the cached variation
if existingDecisionGroupVariation != nil {
existingVariation = existingDecisionGroupVariation
}

if existingVariation == nil && existingAnonymousVariation == nil {
Expand All @@ -227,6 +250,7 @@ func GetDecision(
continue
}
}

var chosenVariation *Variation

// manage the bucket allocation of the visitor
Expand All @@ -236,20 +260,18 @@ func GetDecision(
// If already has variation && assigned variation ID exist, visitor should not be re-assigned
if ok && existingVariation != nil {
logger.Logf(DebugLevel, "visitor ID %s already assigned to variation ID %s", visitorID, existingVariation.ID)
vid = existingAssignment.VariationID
chosenVariation = existingVariation
enableBucketAllocation = false
} else if enableReconciliation && okAnonymous && existingAnonymousVariation != nil {
// If reconciliation is on, find anonymous variation as set vid to that variation ID
logger.Logf(DebugLevel, "anonymous ID %s already assigned to variation ID %s", anonymousID, existingAnonymousVariation.ID)
vid = existingAssignmentAnonymous.VariationID
chosenVariation = existingAnonymousVariation
enableBucketAllocation = false
isNew = true
} else {
// Else compute new allocation
logger.Logf(DebugLevel, "assigning visitor ID %s to new variation", visitorID)
chosenVariation, err = getRandomAllocation(visitorID, vg, options.IsCumulativeAlloc)
chosenVariation, err = getRandomAllocation(visitorID, decisionGroup, vg, options.IsCumulativeAlloc)
if err != nil {
if err == VisitorNotTrackedError {
logger.Logf(InfoLevel, err.Error())
Expand All @@ -262,7 +284,6 @@ func GetDecision(
continue
}
logger.Logf(DebugLevel, "visitor ID %s got assigned to variation ID %s", visitorID, chosenVariation.ID)
vid = chosenVariation.ID
isNew = true
isNewAnonymous = true
}
Expand All @@ -285,7 +306,7 @@ func GetDecision(
alreadyActivated := ok && existingAssignment.Activated
if triggerHit && !alreadyActivated || isNew {
newVGAssignments[vg.ID] = &VisitorCache{
VariationID: vid,
VariationID: chosenVariation.ID,
Activated: triggerHit,
}
}
Expand All @@ -296,7 +317,7 @@ func GetDecision(
alreadyActivatedAnonymous := okAnonymous && existingAssignmentAnonymous.Activated
if enableReconciliation && (triggerHit && !alreadyActivatedAnonymous || isNewAnonymous) {
newVGAssignmentsAnonymous[vg.ID] = &VisitorCache{
VariationID: vid,
VariationID: chosenVariation.ID,
Activated: triggerHit,
}
}
Expand All @@ -311,7 +332,7 @@ func GetDecision(
VisitorID: visitorID,
AnonymousID: anonymousIDActivate,
VariationGroupID: vg.ID,
VariationID: vid,
VariationID: chosenVariation.ID,
})
}

Expand Down
48 changes: 28 additions & 20 deletions decision_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ func TestGetCache(t *testing.T) {

assignments, err = getCache(envID, visitorID, anonymousID, decisionGroup, false, localGetCache)
assert.Nil(t, err)
// Check that decision group assignment filled out the empty standard assignment
assert.EqualValues(t, newAssignmentsDG, assignments.Standard.getAssignments())
assert.EqualValues(t, newAssignmentsDG, assignments.DecisionGroup.getAssignments())
assert.Nil(t, assignments.Standard)
assert.Nil(t, assignments.Anonymous)

cache[envID+visitorID] = &VisitorAssignments{
Expand All @@ -144,29 +144,24 @@ func TestGetCache(t *testing.T) {

assignments, err = getCache(envID, visitorID, anonymousID, decisionGroup, false, localGetCache)
assert.Nil(t, err)
// Check that the standard assignments got overriden by the decision group assignments
assert.EqualValues(t, newAssignmentsDG["vg_id"], assignments.Standard.getAssignments()["vg_id"])
assert.EqualValues(t, newAssignments["vg2_id"], assignments.Standard.getAssignments()["vg2_id"])
assert.EqualValues(t, newAssignmentsDG["vg3_id"], assignments.Standard.getAssignments()["vg3_id"])
assert.EqualValues(t, newAssignmentsDG, assignments.DecisionGroup.getAssignments())
assert.EqualValues(t, newAssignments, assignments.Standard.getAssignments())
assert.Nil(t, assignments.Anonymous)

cache[envID+anonymousID] = &VisitorAssignments{
Assignments: maps.Clone(newAssignments),
}
cache[envID+decisionGroup] = &VisitorAssignments{
Assignments: maps.Clone(newAssignmentsDG),
}

assignments, err = getCache(envID, visitorID, anonymousID, decisionGroup, true, localGetCache)
assert.Nil(t, err)
assert.Equal(t, 3, len(assignments.Standard.getAssignments()))
assert.EqualValues(t, newAssignmentsDG, assignments.DecisionGroup.getAssignments())
assert.EqualValues(t, newAssignments, assignments.Standard.getAssignments())
assert.EqualValues(t, newAssignments, assignments.Anonymous.getAssignments())
}

func TestDecisionCache(t *testing.T) {
vi := Visitor{}
vi.ID = "v1"
vi.DecisionGroup = "dg"
vi.Context = &targeting.Context{
Standard: targeting.ContextMap{
"isVIP": structpb.NewBoolValue(true),
Expand Down Expand Up @@ -195,18 +190,26 @@ func TestDecisionCache(t *testing.T) {
// check that campaign matching visitor is returned. Also check that the second variation is set
assert.Nil(t, err)
assert.Len(t, decision.Campaigns, 1)
assert.Equal(t, decision.Campaigns[0].Variation.Id.Value, "vgav2")
assert.Equal(t, "vgav2", decision.Campaigns[0].Variation.Id.Value)

vi.DecisionGroup = "dg"
decision, err = GetDecision(vi, ei, options, handlers)

// check that campaign matching visitor is returned. Also check that the second variation is set
assert.Nil(t, err)
assert.Len(t, decision.Campaigns, 1)
assert.Equal(t, "vgav1", decision.Campaigns[0].Variation.Id.Value)

// change the allocation so that visitor should change variation if the cache is disabled
ei.Campaigns[0].VariationGroups[0].Variations[0].Allocation = 90
ei.Campaigns[0].VariationGroups[0].Variations[1].Allocation = 10
ei.Campaigns[0].VariationGroups[0].Variations[0].Allocation = 10
ei.Campaigns[0].VariationGroups[0].Variations[1].Allocation = 90

decision, err = GetDecision(vi, ei, options, handlers)

// check that campaign matching visitor is returned. Also check that the first variation is not chosen
assert.Nil(t, err)
assert.Len(t, decision.Campaigns, 1)
assert.Equal(t, decision.Campaigns[0].Variation.Id.Value, "vgav1")
assert.Equal(t, "vgav2", decision.Campaigns[0].Variation.Id.Value)

// Reset the allocations
ei.Campaigns[0].VariationGroups[0].Variations[0].Allocation = 50
Expand All @@ -219,16 +222,20 @@ func TestDecisionCache(t *testing.T) {
decision, _ = GetDecision(vi, ei, options, handlers)

// check that campaign matching visitor is returned. Also check that the first variation is not chosen
assert.Equal(t, decision.Campaigns[0].Variation.Id.Value, "vgav2")
assert.Equal(t, decision.Campaigns[0].Variation.Id.Value, "vgav1")

// change the allocation so that visitor should change variation if the cache is disabled
ei.Campaigns[0].VariationGroups[0].Variations[0].Allocation = 90
ei.Campaigns[0].VariationGroups[0].Variations[1].Allocation = 10
ei.Campaigns[0].VariationGroups[0].Variations[0].Allocation = 10
ei.Campaigns[0].VariationGroups[0].Variations[1].Allocation = 90

decision, _ = GetDecision(vi, ei, options, handlers)

// check that campaign matching visitor is returned. Also check that the first variation is not chosen
assert.Equal(t, decision.Campaigns[0].Variation.Id.Value, "vgav2")
assert.Equal(t, decision.Campaigns[0].Variation.Id.Value, "vgav1")

// Reset the allocations
ei.Campaigns[0].VariationGroups[0].Variations[0].Allocation = 50
ei.Campaigns[0].VariationGroups[0].Variations[1].Allocation = 50
}

func TestDecisionBucketInNoCache(t *testing.T) {
Expand Down Expand Up @@ -269,7 +276,6 @@ func TestDecisionBucketInNoCache(t *testing.T) {
func TestVisitorShouldNotBeAssignedWhenVariationDeleted(t *testing.T) {
vi := Visitor{}
vi.ID = "v1"
vi.DecisionGroup = "dg"
vi.Context = &targeting.Context{
Standard: targeting.ContextMap{
"isVIP": structpb.NewBoolValue(true),
Expand All @@ -278,6 +284,7 @@ func TestVisitorShouldNotBeAssignedWhenVariationDeleted(t *testing.T) {

ei := Environment{}
ei.ID = "e123"
ei.CacheEnabled = true
ei.Campaigns = campaigns
for _, vg := range ei.Campaigns[0].VariationGroups {
vg.Campaign = ei.Campaigns[0]
Expand All @@ -296,6 +303,7 @@ func TestVisitorShouldNotBeAssignedWhenVariationDeleted(t *testing.T) {
// delete variation and check that visitor is not returned
campaignVars := ei.Campaigns[0].VariationGroups[0].Variations
ei.Campaigns[0].VariationGroups[0].Variations = campaignVars[1:]

decision, _ := GetDecision(vi, ei, options, handlers)

assert.Len(t, decision.Campaigns, 0)
Expand Down

0 comments on commit 84fc37f

Please sign in to comment.