Skip to content
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
49 changes: 44 additions & 5 deletions optimizely/decision_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,23 @@ def get_variation_for_rollout(
return Decision(experiment=rule, variation=forced_decision_variation,
source=enums.DecisionSources.ROLLOUT, cmab_uuid=None), decide_reasons

# Check local holdouts targeting this rollout rule
local_holdouts = project_config.get_holdouts_for_rule(rule.id)
for holdout in local_holdouts:
holdout_decision = self.get_variation_for_holdout(holdout, user_context, project_config)
decide_reasons.extend(holdout_decision['reasons'])

decision = holdout_decision['decision']
# Check if user was bucketed into holdout (has a variation)
if decision.variation is not None:
message = (
f"The user '{user_id}' is bucketed into local holdout '{holdout.key}' "
f"for rollout rule '{rule.key}' in feature flag '{feature.key}'."
)
self.logger.info(message)
decide_reasons.append(message)
return decision, decide_reasons

bucketing_id, bucket_reasons = self._get_bucketing_id(user_id, attributes)
decide_reasons += bucket_reasons

Expand Down Expand Up @@ -733,9 +750,9 @@ def get_decision_for_flag(
reasons = decide_reasons.copy() if decide_reasons else []
user_id = user_context.user_id

# Check holdouts
holdouts = project_config.get_holdouts_for_flag(feature_flag.key)
for holdout in holdouts:
# Check global holdouts (evaluated before any rules)
global_holdouts = project_config.get_global_holdouts()
for holdout in global_holdouts:
holdout_decision = self.get_variation_for_holdout(holdout, user_context, project_config)
reasons.extend(holdout_decision['reasons'])

Expand All @@ -745,7 +762,7 @@ def get_decision_for_flag(
continue

message = (
f"The user '{user_id}' is bucketed into holdout '{holdout.key}' "
f"The user '{user_id}' is bucketed into global holdout '{holdout.key}' "
f"for feature flag '{feature_flag.key}'."
)
self.logger.info(message)
Expand All @@ -756,7 +773,8 @@ def get_decision_for_flag(
'reasons': reasons
}

# If no holdout decision, check experiments then rollouts
# If no global holdout decision, check experiments then rollouts
# Local holdouts are evaluated within each rule's evaluation
if feature_flag.experimentIds:
for experiment_id in feature_flag.experimentIds:
experiment = project_config.get_experiment_from_id(experiment_id)
Expand All @@ -778,6 +796,27 @@ def get_decision_for_flag(
'reasons': reasons
}

# Check local holdouts targeting this rule
local_holdouts = project_config.get_holdouts_for_rule(experiment.id)
for holdout in local_holdouts:
holdout_decision = self.get_variation_for_holdout(holdout, user_context, project_config)
reasons.extend(holdout_decision['reasons'])

decision = holdout_decision['decision']
# Check if user was bucketed into holdout (has a variation)
if decision.variation is not None:
message = (
f"The user '{user_id}' is bucketed into local holdout '{holdout.key}' "
f"for rule '{experiment.key}' in feature flag '{feature_flag.key}'."
)
self.logger.info(message)
reasons.append(message)
return {
'decision': holdout_decision['decision'],
'error': False,
'reasons': reasons
}

# Get variation for experiment
variation_result = self.get_variation(
project_config, experiment, user_context, user_profile_tracker, reasons, decide_options
Expand Down
18 changes: 14 additions & 4 deletions optimizely/entities.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,7 @@ def __init__(
variations: list[VariationDict],
trafficAllocation: list[TrafficAllocation],
audienceIds: list[str],
includedFlags: Optional[list[str]] = None,
excludedFlags: Optional[list[str]] = None,
includedRules: Optional[list[str]] = None,
audienceConditions: Optional[Sequence[str | list[str]]] = None,
**kwargs: Any
):
Expand All @@ -234,8 +233,10 @@ def __init__(
self.trafficAllocation = trafficAllocation
self.audienceIds = audienceIds
self.audienceConditions = audienceConditions
self.includedFlags = includedFlags or []
self.excludedFlags = excludedFlags or []
# None = global holdout (applies to all rules)
# [] = local holdout with no rules (effectively no holdout)
# [rule_id, ...] = local holdout targeting specific rules
self.includedRules = includedRules

def get_audience_conditions_or_ids(self) -> Sequence[str | list[str]]:
"""Returns audienceConditions if present, otherwise audienceIds.
Expand All @@ -257,6 +258,15 @@ def is_activated(self) -> bool:
"""
return self.status == self.Status.RUNNING

@property
def is_global(self) -> bool:
"""Check if the holdout is global (applies to all rules).

Returns:
True if includedRules is None (global), False otherwise (local).
"""
return self.includedRules is None

def __str__(self) -> str:
return self.key

Expand Down
8 changes: 6 additions & 2 deletions optimizely/helpers/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@
else:
from typing import TypedDict # type: ignore

if version_info < (3, 11):
from typing_extensions import NotRequired
else:
from typing import NotRequired # type: ignore


# Intermediate types for type checking deserialized datafile json before actual class instantiation.
# These aren't used for anything other than type signatures
Expand Down Expand Up @@ -130,5 +135,4 @@ class HoldoutDict(ExperimentDict):
Extends ExperimentDict with holdout-specific properties.
"""
holdoutStatus: HoldoutStatus
includedFlags: list[str]
excludedFlags: list[str]
includedRules: NotRequired[Optional[list[str]]]
114 changes: 53 additions & 61 deletions optimizely/project_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,47 +89,12 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any):
region_value = config.get('region')
self.region: str = region_value or 'US'

# Parse holdouts from datafile and convert to Holdout entities
# Parse holdouts from datafile (processing deferred until after experiments are loaded)
holdouts_data: list[types.HoldoutDict] = config.get('holdouts', [])
self.holdouts: list[entities.Holdout] = []
self.holdout_id_map: dict[str, entities.Holdout] = {}
self.global_holdouts: list[entities.Holdout] = []
self.included_holdouts: dict[str, list[entities.Holdout]] = {}
self.excluded_holdouts: dict[str, list[entities.Holdout]] = {}
self.flag_holdouts_map: dict[str, list[entities.Holdout]] = {}

# Convert holdout dicts to Holdout entities
for holdout_data in holdouts_data:
# Create Holdout entity
holdout = entities.Holdout(**holdout_data)
self.holdouts.append(holdout)

# Only process Running holdouts but doing it here for efficiency like the original Python implementation)
if not holdout.is_activated:
continue

# Map by ID for quick lookup
self.holdout_id_map[holdout.id] = holdout

# Categorize as global vs flag-specific
# Global holdouts: apply to all flags unless explicitly excluded
# Flag-specific holdouts: only apply to explicitly included flags
if not holdout.includedFlags:
# This is a global holdout
self.global_holdouts.append(holdout)

# Track which flags this global holdout excludes
if holdout.excludedFlags:
for flag_id in holdout.excludedFlags:
if flag_id not in self.excluded_holdouts:
self.excluded_holdouts[flag_id] = []
self.excluded_holdouts[flag_id].append(holdout)
else:
# This holdout applies to specific flags only
for flag_id in holdout.includedFlags:
if flag_id not in self.included_holdouts:
self.included_holdouts[flag_id] = []
self.included_holdouts[flag_id].append(holdout)
self.rule_holdouts_map: dict[str, list[entities.Holdout]] = {}

# Utility maps for quick lookup
self.group_id_map: dict[str, entities.Group] = self._generate_key_map(self.groups, 'id', entities.Group)
Expand Down Expand Up @@ -263,21 +228,10 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any):
everyone_else_variation.variables, 'id', entities.Variation.VariableUsage
)

flag_id = feature.id
applicable_holdouts: list[entities.Holdout] = []

# Add global holdouts first, excluding any that are explicitly excluded for this flag
excluded_holdouts = self.excluded_holdouts.get(flag_id, [])
for holdout in self.global_holdouts:
if holdout not in excluded_holdouts:
applicable_holdouts.append(holdout)

# Add flag-specific local holdouts AFTER global holdouts
if flag_id in self.included_holdouts:
applicable_holdouts.extend(self.included_holdouts[flag_id])

if applicable_holdouts:
self.flag_holdouts_map[feature.key] = applicable_holdouts
# Note: Holdout evaluation is now done at the rule level,
# not the flag level. Global holdouts are checked before any rule evaluation,
# and local holdouts are checked within each rule's evaluation.
# The flag_holdouts_map is no longer used.

rollout = None if len(feature.rolloutId) == 0 else self.rollout_id_map[feature.rolloutId]
if rollout:
Expand All @@ -294,7 +248,38 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any):
variations.append(rule_var)
self.flag_variations_map[feature.key] = variations

# Process holdout variations are converted to Variation entities just like experiment variations
# Process holdouts: Convert holdout dicts to Holdout entities and build mappings
# This happens after experiment_id_map is fully populated so we can validate rule IDs
for holdout_data in holdouts_data:
# Create Holdout entity
holdout = entities.Holdout(**holdout_data)
self.holdouts.append(holdout)

# Only process Running holdouts (for efficiency)
if not holdout.is_activated:
continue

# Map by ID for quick lookup
self.holdout_id_map[holdout.id] = holdout

# Categorize as global vs local (rule-specific)
# Global holdouts: includedRules is None (applies to all rules)
# Local holdouts: includedRules is a list (applies to specific rules)
if holdout.is_global:
# This is a global holdout
self.global_holdouts.append(holdout)
else:
# This holdout applies to specific rules only
# includedRules contains rule IDs - validate they exist before mapping
if holdout.includedRules:
for rule_id in holdout.includedRules:
# Only map if the rule exists (silently skip invalid rule IDs)
if rule_id in self.experiment_id_map:
if rule_id not in self.rule_holdouts_map:
self.rule_holdouts_map[rule_id] = []
self.rule_holdouts_map[rule_id].append(holdout)

# Process holdout variations - convert to Variation entities just like experiment variations
if self.holdouts:
for holdout in self.holdouts:
# Initialize variation maps for this holdout
Expand Down Expand Up @@ -912,19 +897,26 @@ def get_flag_variation(

return None

def get_holdouts_for_flag(self, flag_key: str) -> list[entities.Holdout]:
""" Helper method to get holdouts from an applied feature flag.
def get_global_holdouts(self) -> list[entities.Holdout]:
""" Helper method to get all global holdouts.

Args:
flag_key: Key of the feature flag.
Global holdouts apply to all rules unless specifically targeted by local holdouts.

Returns:
The holdouts that apply for a specific flag as Holdout entity objects.
List of global Holdout entity objects.
"""
if not self.holdouts:
return []
return self.global_holdouts

def get_holdouts_for_rule(self, rule_id: str) -> list[entities.Holdout]:
""" Helper method to get local holdouts targeting a specific rule.

return self.flag_holdouts_map.get(flag_key, [])
Args:
rule_id: ID of the rule (experiment or rollout rule).

Returns:
List of Holdout entity objects that target this specific rule.
"""
return self.rule_holdouts_map.get(rule_id, [])

def get_holdout(self, holdout_id: str) -> Optional[entities.Holdout]:
""" Helper method to get holdout from holdout ID.
Expand Down
Loading
Loading