diff --git a/pkg/client/client.go b/pkg/client/client.go index f55ddbba6..9f4f232c5 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -218,11 +218,7 @@ func (o *OptimizelyClient) decide(userContext *OptimizelyUserContext, key string } if !allOptions.DisableDecisionEvent { - if ue, ok := event.CreateImpressionUserEvent(decisionContext.ProjectConfig, featureDecision.Experiment, - featureDecision.Variation, usrContext, key, featureDecision.Experiment.Key, featureDecision.Source, flagEnabled, featureDecision.CmabUUID); ok { - o.EventProcessor.ProcessEvent(ue) - eventSent = true - } + eventSent = o.dispatchDecisionEvents(decisionContext.ProjectConfig, featureDecision, usrContext, key, flagEnabled) } variableMap := map[string]interface{}{} @@ -245,6 +241,34 @@ func (o *OptimizelyClient) decide(userContext *OptimizelyUserContext, key string return NewOptimizelyDecision(variationKey, ruleKey, key, flagEnabled, optimizelyJSON, *userContext, reasonsToReport) } +func (o *OptimizelyClient) dispatchDecisionEvents(projectConfig config.ProjectConfig, featureDecision decision.FeatureDecision, usrContext entities.UserContext, key string, flagEnabled bool) bool { + eventSent := false + if ue, ok := event.CreateImpressionUserEvent(projectConfig, featureDecision.Experiment, + featureDecision.Variation, usrContext, key, featureDecision.Experiment.Key, featureDecision.Source, flagEnabled, featureDecision.CmabUUID); ok { + o.EventProcessor.ProcessEvent(ue) + eventSent = true + } + if o.sendHoldoutImpression(projectConfig, featureDecision, usrContext, key) { + eventSent = true + } + return eventSent +} + +// sendHoldoutImpression emits a holdout impression event when a holdout was bypassed for a +// targeted delivery rule (the decision carries the bucketed holdout experiment/variation). +// It returns true if an event was dispatched. +func (o *OptimizelyClient) sendHoldoutImpression(projectConfig config.ProjectConfig, featureDecision decision.FeatureDecision, userContext entities.UserContext, key string) bool { + if featureDecision.HoldoutExperiment == nil || featureDecision.HoldoutVariation == nil { + return false + } + if hue, hok := event.CreateImpressionUserEvent(projectConfig, *featureDecision.HoldoutExperiment, + featureDecision.HoldoutVariation, userContext, key, featureDecision.HoldoutExperiment.Key, decision.Holdout, featureDecision.HoldoutVariation.FeatureEnabled, nil); hok { + o.EventProcessor.ProcessEvent(hue) + return true + } + return false +} + func (o *OptimizelyClient) decideForKeys(userContext OptimizelyUserContext, keys []string, options *decide.Options) map[string]OptimizelyDecision { var err error defer func() { @@ -522,6 +546,8 @@ func (o *OptimizelyClient) IsFeatureEnabled(featureKey string, userContext entit featureDecision.Variation, userContext, featureKey, featureDecision.Experiment.Key, featureDecision.Source, result, featureDecision.CmabUUID); ok && featureDecision.Source != "" { o.EventProcessor.ProcessEvent(ue) } + // Send holdout impression when holdout is bypassed for targeted delivery + o.sendHoldoutImpression(decisionContext.ProjectConfig, featureDecision, userContext, featureKey) return result, err } @@ -887,6 +913,8 @@ func (o *OptimizelyClient) GetDetailedFeatureDecisionUnsafe(featureKey string, u featureDecision.Variation, userContext, featureKey, featureDecision.Experiment.Key, featureDecision.Source, decisionInfo.Enabled, featureDecision.CmabUUID); ok { o.EventProcessor.ProcessEvent(ue) } + // Send holdout impression when holdout is bypassed for targeted delivery + o.sendHoldoutImpression(decisionContext.ProjectConfig, featureDecision, userContext, featureKey) } } diff --git a/pkg/config/datafileprojectconfig/entities/entities.go b/pkg/config/datafileprojectconfig/entities/entities.go index 57d2e46a9..ec9deabf1 100644 --- a/pkg/config/datafileprojectconfig/entities/entities.go +++ b/pkg/config/datafileprojectconfig/entities/entities.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019,2021-2025, Optimizely, Inc. and contributors * + * Copyright 2019,2021-2026, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -128,7 +128,8 @@ type Holdout struct { TrafficAllocation []TrafficAllocation `json:"trafficAllocation"` // IncludedRules carries per-rule targeting for local holdouts. Required on // `localHoldouts` entries; ignored/stripped on `holdouts` entries at parse time. - IncludedRules *[]string `json:"includedRules,omitempty"` + IncludedRules *[]string `json:"includedRules,omitempty"` + ExcludeTargetedDeliveries bool `json:"excludeTargetedDeliveries"` } // Integration represents a integration from the Optimizely datafile diff --git a/pkg/config/datafileprojectconfig/mappers/holdout.go b/pkg/config/datafileprojectconfig/mappers/holdout.go index c0050ff99..8d43f211e 100644 --- a/pkg/config/datafileprojectconfig/mappers/holdout.go +++ b/pkg/config/datafileprojectconfig/mappers/holdout.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2025, Optimizely, Inc. and contributors * + * Copyright 2025-2026, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -168,14 +168,15 @@ func mapHoldout(datafileHoldout datafileEntities.Holdout) entities.Holdout { } return entities.Holdout{ - ID: datafileHoldout.ID, - Key: datafileHoldout.Key, - Status: entities.HoldoutStatus(datafileHoldout.Status), - AudienceIds: datafileHoldout.AudienceIds, - AudienceConditions: datafileHoldout.AudienceConditions, - Variations: variations, - TrafficAllocation: trafficAllocation, - AudienceConditionTree: audienceConditionTree, - IncludedRules: datafileHoldout.IncludedRules, + ID: datafileHoldout.ID, + Key: datafileHoldout.Key, + Status: entities.HoldoutStatus(datafileHoldout.Status), + AudienceIds: datafileHoldout.AudienceIds, + AudienceConditions: datafileHoldout.AudienceConditions, + Variations: variations, + TrafficAllocation: trafficAllocation, + AudienceConditionTree: audienceConditionTree, + IncludedRules: datafileHoldout.IncludedRules, + ExcludeTargetedDeliveries: datafileHoldout.ExcludeTargetedDeliveries, } } diff --git a/pkg/config/datafileprojectconfig/mappers/holdout_test.go b/pkg/config/datafileprojectconfig/mappers/holdout_test.go index a44b01251..e4e51799a 100644 --- a/pkg/config/datafileprojectconfig/mappers/holdout_test.go +++ b/pkg/config/datafileprojectconfig/mappers/holdout_test.go @@ -561,6 +561,49 @@ func TestMapHoldoutsIsGlobalProperty(t *testing.T) { assert.False(t, localHoldoutWithRules.IsGlobal(), "non-nil IncludedRules with rules should NOT be global") } +func TestMapHoldoutsExcludeTargetedDeliveriesMapped(t *testing.T) { + rawGlobal := []datafileEntities.Holdout{ + { + ID: "holdout_etd", + Key: "holdout_with_etd", + Status: "Running", + ExcludeTargetedDeliveries: true, + Variations: []datafileEntities.Variation{ + {ID: "var_1", Key: "variation_1"}, + }, + TrafficAllocation: []datafileEntities.TrafficAllocation{ + {EntityID: "var_1", EndOfRange: 10000}, + }, + }, + } + + holdoutList, _, _, _ := MapHoldouts(rawGlobal, nil, &captureLogger{}) + + assert.Len(t, holdoutList, 1) + assert.True(t, holdoutList[0].ExcludeTargetedDeliveries) +} + +func TestMapHoldoutsExcludeTargetedDeliveriesDefaultsFalse(t *testing.T) { + rawGlobal := []datafileEntities.Holdout{ + { + ID: "holdout_no_etd", + Key: "holdout_without_etd", + Status: "Running", + Variations: []datafileEntities.Variation{ + {ID: "var_1", Key: "variation_1"}, + }, + TrafficAllocation: []datafileEntities.TrafficAllocation{ + {EntityID: "var_1", EndOfRange: 10000}, + }, + }, + } + + holdoutList, _, _, _ := MapHoldouts(rawGlobal, nil, &captureLogger{}) + + assert.Len(t, holdoutList, 1) + assert.False(t, holdoutList[0].ExcludeTargetedDeliveries) +} + func TestMapHoldoutsDuplicateIDsAcrossSectionsLogsWarning(t *testing.T) { // If the same holdout ID appears in both sections, a warning must be logged // and the later (local) entry overwrites the earlier (global) one in the ID map. diff --git a/pkg/decision/composite_feature_service.go b/pkg/decision/composite_feature_service.go index cb3168cf0..073e7ebf4 100644 --- a/pkg/decision/composite_feature_service.go +++ b/pkg/decision/composite_feature_service.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019-2025, Optimizely, Inc. and contributors * + * Copyright 2019-2026, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -27,19 +27,25 @@ import ( type CompositeFeatureService struct { holdoutService *HoldoutService featureServices []FeatureService - logger logging.OptimizelyLogProducer + // rolloutService is the same instance held in featureServices; it is referenced directly + // (rather than by slice index) for the ExcludeTargetedDeliveries path, so the logic does not + // depend on the ordering of featureServices. + rolloutService FeatureService + logger logging.OptimizelyLogProducer } // NewCompositeFeatureService returns a new instance of the CompositeFeatureService func NewCompositeFeatureService(sdkKey string, compositeExperimentService ExperimentService) *CompositeFeatureService { holdoutService := NewHoldoutService(sdkKey) + rolloutService := NewRolloutService(sdkKey) return &CompositeFeatureService{ holdoutService: holdoutService, logger: logging.GetLogger(sdkKey, "CompositeFeatureService"), featureServices: []FeatureService{ NewFeatureExperimentService(logging.GetLogger(sdkKey, "FeatureExperimentService"), compositeExperimentService, holdoutService), - NewRolloutService(sdkKey), + rolloutService, }, + rolloutService: rolloutService, } } @@ -52,6 +58,9 @@ func (f CompositeFeatureService) GetDecision(decisionContext FeatureDecisionCont holdoutDecision, holdoutReasons, _ := f.holdoutService.GetGlobalDecision(decisionContext, userContext, options) reasons.Append(holdoutReasons) if holdoutDecision.Variation != nil { + if holdoutDecision.Holdout != nil && holdoutDecision.Holdout.ExcludeTargetedDeliveries { + return f.getDecisionWithExcludedTD(holdoutDecision, decisionContext, userContext, options, reasons) + } return holdoutDecision, reasons, nil } } @@ -65,7 +74,6 @@ func (f CompositeFeatureService) GetDecision(decisionContext FeatureDecisionCont if err != nil { f.logger.Debug(err.Error()) reasons.AddError(err.Error()) - // Return the error to let the caller handle it properly return FeatureDecision{}, reasons, err } @@ -75,3 +83,40 @@ func (f CompositeFeatureService) GetDecision(decisionContext FeatureDecisionCont } return featureDecision, reasons, err } + +// getDecisionWithExcludedTD handles the exclude_targeted_deliveries holdout logic. +// When a holdout has ExcludeTargetedDeliveries set, AB/MAB/CMAB experiments are +// blocked (holdout returned) but targeted delivery rules are allowed through. +func (f CompositeFeatureService) getDecisionWithExcludedTD(holdoutDecision FeatureDecision, decisionContext FeatureDecisionContext, userContext entities.UserContext, options *decide.Options, reasons decide.DecisionReasons) (FeatureDecision, decide.DecisionReasons, error) { + holdoutExp := holdoutDecision.Experiment + holdoutVar := holdoutDecision.Variation + + reasons.AddInfo("Holdout \"%s\" has excludeTargetedDeliveries enabled, continuing to rollout evaluation.", holdoutDecision.Holdout.Key) + + // Skip experiment evaluation entirely (A/B/MAB/CMAB are blocked by holdout). + // Evaluate rollout service only (targeted deliveries are excluded from holdout blocking). + if f.rolloutService != nil { + rolloutDecision, rolloutReasons, err := f.rolloutService.GetDecision(decisionContext, userContext, options) + reasons.Append(rolloutReasons) + if err != nil { + f.logger.Debug(err.Error()) + reasons.AddError(err.Error()) + return FeatureDecision{}, reasons, err + } + if rolloutDecision.Variation != nil { + rolloutDecision.HoldoutExperiment = &holdoutExp + rolloutDecision.HoldoutVariation = holdoutVar + return rolloutDecision, reasons, nil + } + } + + emptyDecision := FeatureDecision{ + // Match the rollout service's no-match behavior (see RolloutService.GetDecision), + // so a served impression under sendFlagDecisions carries ruleType "rollout" rather + // than a blank value. + Source: Rollout, + HoldoutExperiment: &holdoutExp, + HoldoutVariation: holdoutVar, + } + return emptyDecision, reasons, nil +} diff --git a/pkg/decision/composite_feature_service_test.go b/pkg/decision/composite_feature_service_test.go index 06cbff369..54f38f914 100644 --- a/pkg/decision/composite_feature_service_test.go +++ b/pkg/decision/composite_feature_service_test.go @@ -25,6 +25,8 @@ import ( "github.com/optimizely/go-sdk/v2/pkg/entities" "github.com/optimizely/go-sdk/v2/pkg/logging" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" ) @@ -198,6 +200,315 @@ func (s *CompositeFeatureServiceTestSuite) TestGetDecisionWithCmabError() { s.mockFeatureService2.AssertNotCalled(s.T(), "GetDecision") } +func TestExcludeTDFalseBlocksEverything(t *testing.T) { + mockConfig := new(mockProjectConfig) + mockBucketer := new(MockExperimentBucketer) + mockAudienceEval := new(MockAudienceTreeEvaluator) + mockLogger := new(MockLogger) + + holdoutVar := entities.Variation{ID: "holdout_var", Key: "holdout_variation"} + holdout := entities.Holdout{ + ID: "holdout_etd_false", + Key: "holdout_exclude_td_false", + Status: entities.HoldoutStatusRunning, + ExcludeTargetedDeliveries: false, + Variations: map[string]entities.Variation{"holdout_var": holdoutVar}, + TrafficAllocation: []entities.Range{{EntityID: "holdout_var", EndOfRange: 10000}}, + } + + mockConfig.On("GetGlobalHoldouts").Return([]entities.Holdout{holdout}) + mockConfig.On("GetAudienceMap").Return(map[string]entities.Audience{}) + mockBucketer.On("Bucket", "test_user", mock.AnythingOfType("entities.Experiment"), entities.Group{}).Return(&holdoutVar, reasons.Reason(""), nil) + mockLogger.On("Debug", mock.Anything).Return() + mockLogger.On("Info", mock.Anything).Return() + + holdoutService := &HoldoutService{ + audienceTreeEvaluator: mockAudienceEval, + bucketer: mockBucketer, + logger: mockLogger, + } + + mockFeatureService := new(MockFeatureDecisionService) + mockRolloutService := new(MockFeatureDecisionService) + + compositeFeatureService := &CompositeFeatureService{ + holdoutService: holdoutService, + featureServices: []FeatureService{mockFeatureService, mockRolloutService}, + rolloutService: mockRolloutService, + logger: logging.GetLogger("", "CompositeFeatureService"), + } + + feature := entities.Feature{ID: "feat_1", Key: "test_feature"} + decisionContext := FeatureDecisionContext{ + Feature: &feature, + ProjectConfig: mockConfig, + } + userContext := entities.UserContext{ID: "test_user"} + options := &decide.Options{} + + decision, _, err := compositeFeatureService.GetDecision(decisionContext, userContext, options) + + assert.NoError(t, err) + assert.NotNil(t, decision.Variation) + assert.Equal(t, holdoutVar.ID, decision.Variation.ID) + assert.Equal(t, Holdout, decision.Source) + mockFeatureService.AssertNotCalled(t, "GetDecision") + mockRolloutService.AssertNotCalled(t, "GetDecision") +} + +func TestExcludeTDTrueBlocksABExperiment(t *testing.T) { + mockConfig := new(mockProjectConfig) + mockBucketer := new(MockExperimentBucketer) + mockAudienceEval := new(MockAudienceTreeEvaluator) + mockLogger := new(MockLogger) + + holdoutVar := entities.Variation{ID: "holdout_var", Key: "holdout_variation"} + holdout := entities.Holdout{ + ID: "holdout_etd_true", + Key: "holdout_exclude_td_true", + Status: entities.HoldoutStatusRunning, + ExcludeTargetedDeliveries: true, + Variations: map[string]entities.Variation{"holdout_var": holdoutVar}, + TrafficAllocation: []entities.Range{{EntityID: "holdout_var", EndOfRange: 10000}}, + } + + mockConfig.On("GetGlobalHoldouts").Return([]entities.Holdout{holdout}) + mockConfig.On("GetAudienceMap").Return(map[string]entities.Audience{}) + mockBucketer.On("Bucket", "test_user", mock.AnythingOfType("entities.Experiment"), entities.Group{}).Return(&holdoutVar, reasons.Reason(""), nil) + mockLogger.On("Debug", mock.Anything).Return() + mockLogger.On("Info", mock.Anything).Return() + + holdoutService := &HoldoutService{ + audienceTreeEvaluator: mockAudienceEval, + bucketer: mockBucketer, + logger: mockLogger, + } + + mockFeatureService := new(MockFeatureDecisionService) + mockRolloutService := new(MockFeatureDecisionService) + + feature := entities.Feature{ID: "feat_1", Key: "test_feature"} + decisionContext := FeatureDecisionContext{ + Feature: &feature, + ProjectConfig: mockConfig, + } + userContext := entities.UserContext{ID: "test_user"} + options := &decide.Options{IncludeReasons: true} + decisionReasons := decide.NewDecisionReasons(options) + + emptyDecision := FeatureDecision{} + mockRolloutService.On("GetDecision", decisionContext, userContext, options).Return(emptyDecision, decisionReasons, nil) + + compositeFeatureService := &CompositeFeatureService{ + holdoutService: holdoutService, + featureServices: []FeatureService{mockFeatureService, mockRolloutService}, + rolloutService: mockRolloutService, + logger: logging.GetLogger("", "CompositeFeatureService"), + } + + decision, resultReasons, err := compositeFeatureService.GetDecision(decisionContext, userContext, options) + + assert.NoError(t, err) + assert.Nil(t, decision.Variation) + assert.NotNil(t, decision.HoldoutExperiment) + assert.NotNil(t, decision.HoldoutVariation) + assert.Equal(t, holdoutVar.ID, decision.HoldoutVariation.ID) + mockFeatureService.AssertNotCalled(t, "GetDecision") + mockRolloutService.AssertExpectations(t) + + reportedReasons := resultReasons.ToReport() + assert.Contains(t, reportedReasons, "Holdout \"holdout_exclude_td_true\" has excludeTargetedDeliveries enabled, continuing to rollout evaluation.") +} + +func TestExcludeTDTrueAllowsTDRollout(t *testing.T) { + mockConfig := new(mockProjectConfig) + mockBucketer := new(MockExperimentBucketer) + mockAudienceEval := new(MockAudienceTreeEvaluator) + mockLogger := new(MockLogger) + + holdoutVar := entities.Variation{ID: "holdout_var", Key: "holdout_variation"} + holdout := entities.Holdout{ + ID: "holdout_etd_true", + Key: "holdout_exclude_td_true", + Status: entities.HoldoutStatusRunning, + ExcludeTargetedDeliveries: true, + Variations: map[string]entities.Variation{"holdout_var": holdoutVar}, + TrafficAllocation: []entities.Range{{EntityID: "holdout_var", EndOfRange: 10000}}, + } + + mockConfig.On("GetGlobalHoldouts").Return([]entities.Holdout{holdout}) + mockConfig.On("GetAudienceMap").Return(map[string]entities.Audience{}) + mockBucketer.On("Bucket", "test_user", mock.AnythingOfType("entities.Experiment"), entities.Group{}).Return(&holdoutVar, reasons.Reason(""), nil) + mockLogger.On("Debug", mock.Anything).Return() + mockLogger.On("Info", mock.Anything).Return() + + holdoutService := &HoldoutService{ + audienceTreeEvaluator: mockAudienceEval, + bucketer: mockBucketer, + logger: mockLogger, + } + + rolloutVar := entities.Variation{ID: "rollout_var", Key: "rollout_variation"} + rolloutDecision := FeatureDecision{ + Variation: &rolloutVar, + Source: Rollout, + Experiment: entities.Experiment{ID: "rollout_1", Key: "rollout_rule"}, + } + + mockFeatureService := new(MockFeatureDecisionService) + mockRolloutService := new(MockFeatureDecisionService) + + feature := entities.Feature{ID: "feat_1", Key: "test_feature"} + decisionContext := FeatureDecisionContext{ + Feature: &feature, + ProjectConfig: mockConfig, + } + userContext := entities.UserContext{ID: "test_user"} + options := &decide.Options{IncludeReasons: true} + decisionReasons := decide.NewDecisionReasons(options) + + mockRolloutService.On("GetDecision", decisionContext, userContext, options).Return(rolloutDecision, decisionReasons, nil) + + compositeFeatureService := &CompositeFeatureService{ + holdoutService: holdoutService, + featureServices: []FeatureService{mockFeatureService, mockRolloutService}, + rolloutService: mockRolloutService, + logger: logging.GetLogger("", "CompositeFeatureService"), + } + + decision, resultReasons, err := compositeFeatureService.GetDecision(decisionContext, userContext, options) + + assert.NoError(t, err) + assert.NotNil(t, decision.Variation) + assert.Equal(t, rolloutVar.ID, decision.Variation.ID) + assert.Equal(t, Rollout, decision.Source) + assert.NotNil(t, decision.HoldoutExperiment) + assert.NotNil(t, decision.HoldoutVariation) + assert.Equal(t, holdoutVar.ID, decision.HoldoutVariation.ID) + mockFeatureService.AssertNotCalled(t, "GetDecision") + mockRolloutService.AssertExpectations(t) + + reportedReasons := resultReasons.ToReport() + assert.Contains(t, reportedReasons, "Holdout \"holdout_exclude_td_true\" has excludeTargetedDeliveries enabled, continuing to rollout evaluation.") +} + +func TestExcludeTDTrueNoDownstreamMatchReturnsEmpty(t *testing.T) { + mockConfig := new(mockProjectConfig) + mockBucketer := new(MockExperimentBucketer) + mockAudienceEval := new(MockAudienceTreeEvaluator) + mockLogger := new(MockLogger) + + holdoutVar := entities.Variation{ID: "holdout_var", Key: "holdout_variation"} + holdout := entities.Holdout{ + ID: "holdout_etd_true", + Key: "holdout_exclude_td_true", + Status: entities.HoldoutStatusRunning, + ExcludeTargetedDeliveries: true, + Variations: map[string]entities.Variation{"holdout_var": holdoutVar}, + TrafficAllocation: []entities.Range{{EntityID: "holdout_var", EndOfRange: 10000}}, + } + + mockConfig.On("GetGlobalHoldouts").Return([]entities.Holdout{holdout}) + mockConfig.On("GetAudienceMap").Return(map[string]entities.Audience{}) + mockBucketer.On("Bucket", "test_user", mock.AnythingOfType("entities.Experiment"), entities.Group{}).Return(&holdoutVar, reasons.Reason(""), nil) + mockLogger.On("Debug", mock.Anything).Return() + mockLogger.On("Info", mock.Anything).Return() + + holdoutService := &HoldoutService{ + audienceTreeEvaluator: mockAudienceEval, + bucketer: mockBucketer, + logger: mockLogger, + } + + mockFeatureService := new(MockFeatureDecisionService) + mockRolloutService := new(MockFeatureDecisionService) + + feature := entities.Feature{ID: "feat_1", Key: "test_feature"} + decisionContext := FeatureDecisionContext{ + Feature: &feature, + ProjectConfig: mockConfig, + } + userContext := entities.UserContext{ID: "test_user"} + options := &decide.Options{IncludeReasons: true} + decisionReasons := decide.NewDecisionReasons(options) + + emptyDecision := FeatureDecision{} + mockRolloutService.On("GetDecision", decisionContext, userContext, options).Return(emptyDecision, decisionReasons, nil) + + compositeFeatureService := &CompositeFeatureService{ + holdoutService: holdoutService, + featureServices: []FeatureService{mockFeatureService, mockRolloutService}, + rolloutService: mockRolloutService, + logger: logging.GetLogger("", "CompositeFeatureService"), + } + + decision, resultReasons, err := compositeFeatureService.GetDecision(decisionContext, userContext, options) + + assert.NoError(t, err) + assert.Nil(t, decision.Variation) + // No downstream match still reports the rollout source, matching RolloutService's + // own no-match behavior, so a served impression carries ruleType "rollout". + assert.Equal(t, Rollout, decision.Source) + assert.NotNil(t, decision.HoldoutExperiment) + assert.NotNil(t, decision.HoldoutVariation) + assert.Equal(t, holdoutVar.ID, decision.HoldoutVariation.ID) + mockFeatureService.AssertNotCalled(t, "GetDecision") + mockRolloutService.AssertExpectations(t) + + reportedReasons := resultReasons.ToReport() + assert.Contains(t, reportedReasons, "Holdout \"holdout_exclude_td_true\" has excludeTargetedDeliveries enabled, continuing to rollout evaluation.") +} + +func TestExcludeTDMissingFieldDefaultsFalse(t *testing.T) { + holdout := entities.Holdout{ + ID: "holdout_no_etd", + Key: "holdout_missing_field", + Status: entities.HoldoutStatusRunning, + } + + assert.False(t, holdout.ExcludeTargetedDeliveries) +} + +func TestLocalHoldoutIgnoresExcludeTargetedDeliveries(t *testing.T) { + mockConfig := new(mockProjectConfig) + mockBucketer := new(MockExperimentBucketer) + mockAudienceEval := new(MockAudienceTreeEvaluator) + mockLogger := new(MockLogger) + + holdoutVar := entities.Variation{ID: "local_holdout_var", Key: "local_holdout_variation"} + localHoldout := entities.Holdout{ + ID: "local_holdout_etd", + Key: "local_holdout_with_etd", + Status: entities.HoldoutStatusRunning, + ExcludeTargetedDeliveries: true, + Variations: map[string]entities.Variation{"local_holdout_var": holdoutVar}, + TrafficAllocation: []entities.Range{{EntityID: "local_holdout_var", EndOfRange: 10000}}, + } + + ruleID := "rule_123" + mockConfig.On("GetHoldoutsForRule", ruleID).Return([]entities.Holdout{localHoldout}) + mockConfig.On("GetAudienceMap").Return(map[string]entities.Audience{}) + mockBucketer.On("Bucket", "test_user", mock.AnythingOfType("entities.Experiment"), entities.Group{}).Return(&holdoutVar, reasons.Reason(""), nil) + mockLogger.On("Debug", mock.Anything).Return() + mockLogger.On("Info", mock.Anything).Return() + + holdoutService := &HoldoutService{ + audienceTreeEvaluator: mockAudienceEval, + bucketer: mockBucketer, + logger: mockLogger, + } + + userContext := entities.UserContext{ID: "test_user"} + options := &decide.Options{} + + decision, _, err := holdoutService.GetLocalDecisionForRule(ruleID, mockConfig, userContext, options) + + assert.NoError(t, err) + assert.NotNil(t, decision.Variation) + assert.Equal(t, holdoutVar.ID, decision.Variation.ID) + assert.Equal(t, Holdout, decision.Source) +} + func (s *CompositeFeatureServiceTestSuite) TestNewCompositeFeatureService() { // Assert that the service is instantiated with the correct child services in the right order compositeExperimentService := NewCompositeExperimentService("") diff --git a/pkg/decision/entities.go b/pkg/decision/entities.go index fec60903b..e663854cd 100644 --- a/pkg/decision/entities.go +++ b/pkg/decision/entities.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019-2025, Optimizely, Inc. and contributors * + * Copyright 2019-2026, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -71,6 +71,14 @@ type FeatureDecision struct { Experiment entities.Experiment Variation *entities.Variation CmabUUID *string + // Holdout is the holdout the user was bucketed into, set by HoldoutService so the + // composite service can inspect holdout properties (e.g. ExcludeTargetedDeliveries). + Holdout *entities.Holdout + // HoldoutExperiment and HoldoutVariation are set by CompositeFeatureService only when a + // holdout with ExcludeTargetedDeliveries is bypassed for a targeted delivery rule. They let + // the client emit a holdout impression alongside the served rollout decision. + HoldoutExperiment *entities.Experiment + HoldoutVariation *entities.Variation } // ExperimentDecision contains the decision information about an experiment diff --git a/pkg/decision/holdout_service.go b/pkg/decision/holdout_service.go index 95b6a1f71..10594c088 100644 --- a/pkg/decision/holdout_service.go +++ b/pkg/decision/holdout_service.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2025, Optimizely, Inc. and contributors * + * Copyright 2025-2026, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -109,6 +109,7 @@ func (h HoldoutService) GetGlobalDecision(decisionContext FeatureDecisionContext featureDecision := FeatureDecision{ Experiment: experimentForBucketing, Variation: variation, + Holdout: holdout, Source: Holdout, } return featureDecision, reasons, nil diff --git a/pkg/entities/experiment.go b/pkg/entities/experiment.go index 2798b02ff..d44638fc6 100644 --- a/pkg/entities/experiment.go +++ b/pkg/entities/experiment.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019,2021-2025 Optimizely, Inc. and contributors * + * Copyright 2019,2021-2026 Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -93,7 +93,8 @@ type Holdout struct { // IncludedRules carries per-rule targeting for local holdouts. The datafile // section determines scope; `DatafileProjectConfig` strips `IncludedRules` on // entries from the `holdouts` section, so nil here is equivalent to global. - IncludedRules *[]string + IncludedRules *[]string + ExcludeTargetedDeliveries bool } // IsGlobal returns true if this holdout is global (applies to all rules across all flags).