Skip to content

Commit efec465

Browse files
authored
add impression event handling for holdouts (#428)
1 parent 4c85352 commit efec465

File tree

2 files changed

+130
-1
lines changed

2 files changed

+130
-1
lines changed

pkg/event/factory.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,26 @@ func CreateImpressionUserEvent(
127127
cmabUUID *string,
128128
) (UserEvent, bool) {
129129

130-
if (ruleType == decisionPkg.Rollout || variation == nil) && !projectConfig.SendFlagDecisions() {
130+
// Determine if we should send impression event based on decision source
131+
shouldSendImpression := false
132+
switch ruleType {
133+
case decisionPkg.FeatureTest:
134+
shouldSendImpression = true
135+
case decisionPkg.Holdout:
136+
shouldSendImpression = true // Always send for holdouts (matches Swift/JS reference implementations)
137+
case decisionPkg.Rollout:
138+
shouldSendImpression = projectConfig.SendFlagDecisions()
139+
default:
140+
// For backward compatibility, send events for unknown rule types (e.g., "experiment")
141+
shouldSendImpression = true
142+
}
143+
144+
// Don't send if user wasn't bucketed and SendFlagDecisions is disabled
145+
if variation == nil && !projectConfig.SendFlagDecisions() {
146+
shouldSendImpression = false
147+
}
148+
149+
if !shouldSendImpression {
131150
return UserEvent{}, false
132151
}
133152

pkg/event/factory_test.go

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ func TestCreateImpressionUserEvent(t *testing.T) {
200200
expected bool
201201
}{
202202
{decision.FeatureTest, true},
203+
{decision.Holdout, true}, // Holdouts should always send impression events
203204
{"experiment", true},
204205
{"anything-else", true},
205206
{decision.Rollout, false},
@@ -259,6 +260,7 @@ func TestCreateImpressionUserEventWithCmabUUID(t *testing.T) {
259260
expected bool
260261
}{
261262
{decision.FeatureTest, true, true},
263+
{decision.Holdout, true, true}, // Holdouts should send impression events
262264
{"experiment", true, true},
263265
{"anything-else", true, true},
264266
{decision.Rollout, true, false}, // Should return false for Rollout
@@ -333,3 +335,111 @@ func TestCreateImpressionUserEventWithCmabUUID(t *testing.T) {
333335
metaData := userEvent.Impression.Metadata
334336
assert.Equal(t, testUUID, *metaData.CmabUUID)
335337
}
338+
339+
func TestCreateImpressionUserEventForHoldout(t *testing.T) {
340+
tc := TestConfig{}
341+
342+
testHoldout := entities.Experiment{
343+
Key: "test_holdout",
344+
LayerID: "",
345+
ID: "holdout_123",
346+
}
347+
348+
testHoldoutVariation := entities.Variation{
349+
Key: "holdout_variation",
350+
ID: "holdout_var_123",
351+
}
352+
353+
// Test 1: Holdout with variation should ALWAYS send impression event
354+
t.Run("holdout with variation sends impression event", func(t *testing.T) {
355+
userEvent, ok := CreateImpressionUserEvent(
356+
tc,
357+
testHoldout,
358+
&testHoldoutVariation,
359+
userContext,
360+
"test_flag",
361+
testHoldout.Key,
362+
decision.Holdout,
363+
false, // enabled is false for holdouts typically
364+
nil,
365+
)
366+
367+
assert.True(t, ok, "Impression event should be created for holdout with variation")
368+
assert.NotNil(t, userEvent.Impression)
369+
assert.Equal(t, "test_flag", userEvent.Impression.Metadata.FlagKey)
370+
assert.Equal(t, testHoldout.Key, userEvent.Impression.Metadata.RuleKey)
371+
assert.Equal(t, decision.Holdout, userEvent.Impression.Metadata.RuleType)
372+
assert.Equal(t, false, userEvent.Impression.Metadata.Enabled)
373+
})
374+
375+
// Test 2: Holdout with nil variation and SendFlagDecisions=false should NOT send
376+
t.Run("holdout with nil variation and SendFlagDecisions=false does not send", func(t *testing.T) {
377+
tc.sendFlagDecisions = false
378+
userEvent, ok := CreateImpressionUserEvent(
379+
tc,
380+
testHoldout,
381+
nil, // No variation
382+
userContext,
383+
"test_flag",
384+
testHoldout.Key,
385+
decision.Holdout,
386+
false,
387+
nil,
388+
)
389+
390+
assert.False(t, ok, "Impression event should NOT be created for holdout without variation when SendFlagDecisions=false")
391+
assert.Nil(t, userEvent.Impression)
392+
})
393+
394+
// Test 3: Holdout with nil variation but SendFlagDecisions=true should send
395+
t.Run("holdout with nil variation and SendFlagDecisions=true sends event", func(t *testing.T) {
396+
tc.sendFlagDecisions = true
397+
userEvent, ok := CreateImpressionUserEvent(
398+
tc,
399+
testHoldout,
400+
nil, // No variation
401+
userContext,
402+
"test_flag",
403+
testHoldout.Key,
404+
decision.Holdout,
405+
false,
406+
nil,
407+
)
408+
409+
assert.True(t, ok, "Impression event should be created for holdout even without variation when SendFlagDecisions=true")
410+
assert.NotNil(t, userEvent.Impression)
411+
assert.Equal(t, decision.Holdout, userEvent.Impression.Metadata.RuleType)
412+
})
413+
414+
// Test 4: Verify holdout impression event structure matches reference SDKs
415+
t.Run("holdout impression event structure", func(t *testing.T) {
416+
tc.sendFlagDecisions = false
417+
userEvent, ok := CreateImpressionUserEvent(
418+
tc,
419+
testHoldout,
420+
&testHoldoutVariation,
421+
userContext,
422+
"flag_key",
423+
"holdout_key",
424+
decision.Holdout,
425+
false,
426+
nil,
427+
)
428+
429+
assert.True(t, ok)
430+
impression := userEvent.Impression
431+
432+
// Verify core fields
433+
assert.Equal(t, "flag_key", impression.Metadata.FlagKey)
434+
assert.Equal(t, "holdout_key", impression.Metadata.RuleKey)
435+
assert.Equal(t, decision.Holdout, impression.Metadata.RuleType)
436+
assert.Equal(t, "holdout_variation", impression.Metadata.VariationKey)
437+
assert.Equal(t, false, impression.Metadata.Enabled)
438+
assert.Nil(t, impression.Metadata.CmabUUID) // Holdouts don't use CMAB
439+
440+
// Verify IDs are set correctly
441+
assert.Equal(t, testHoldoutVariation.ID, impression.VariationID)
442+
assert.Equal(t, testHoldout.ID, impression.ExperimentID)
443+
assert.Equal(t, "", impression.CampaignID) // Empty for holdouts (no layer)
444+
})
445+
}

0 commit comments

Comments
 (0)