Skip to content
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.

Sign up for GitHub

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

refactor: remove proportion and salt params from Feature #142

Merged
merged 2 commits into from
Mar 12, 2024
Merged

refactor: remove proportion and salt params from Feature #142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
27412c2
remove proportion and salt
daniel-codecov Mar 4, 2024
d8e272a
update tests
daniel-codecov Mar 12, 2024
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
70 changes: 29 additions & 41 deletions shared/rollouts/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ class IdentifierType(Enum):
class Feature:
"""
Represents a feature and its rollout parameters, fetched from the database (see django_apps/rollouts/models.py).
Given an identifier (repo_id, owner_id, etc..), it can decide which variant of a feature (if any)
Given an identifier (repo_id, owner_id, etc..), it will decide which variant of a feature (if any)
should be used for the request. Each variant will have a `value` that will be returned if that variant is
decided to be used. For example: if you want an ON and OFF variant for your feature, you could have the values
be true and false respectively

The parameters are fetched and updated roughly every 5 minutes, meaning it can take up to 5 minutes for changes
to show up here. You can modify these values in the database via Django Admin.
You can modify the parameters of your feature flag via Django Admin. The parameters are fetched and updated roughly
every 5 minutes, meaning it can take up to 5 minutes for changes to show up here.

If you instantiate a `Feature` instance with a new name, the associated database entry
will be created for you. Otherwise, the existing database entry will be used to populate
Expand All @@ -40,32 +40,40 @@ class Feature:
Examples:

A simple on/off feature rolled out to 20% of repos:
# By default, features have no variants — you create them via Django Admin. You also create the `on` variant there.
MY_FEATURE_BY_REPO = Feature("my_feature", 0.2)
# By default, features have no variants — you create them via Django Admin. You can create the `on`
# variant there, along with setting the proportion and salt for the flag.
MY_FEATURE_BY_REPO = Feature("my_feature")

# DB:
# FeatureFlag:
# name: my_feature
# proportion: 0.0 # default value
# salt: ajsdopijaejapvjghiujnarapsjf # default is randomly generated
#
# FeatureFlagVariant:
# name: my_feature_on
# feature_flag: my_feature
# proportion: 1
# value: true

A simple A/B test rolled out to 10% of users (5% test, 5% control):
MY_EXPERIMENT_BY_USER = Feature(
"my_experiment",
0.1,
)
MY_EXPERIMENT_BY_USER = Feature("my_experiment")

# DB:
# FeatureFlag:
# name: my_experiment
# proportion: 0.1
# salt: foajdisjfosdjrandomfsfsdfsfsfs
#
# FeatureFlagVariant:
# name: MY_EXPERIMENT_BY_USER_TEST
# feature_flag: MY_EXPERIMENT_BY_USER
# name: test
# feature_flag: my_experiment
# proportion: 0.5
# value: true
#
# FeatureFlagVariant:
# name: MY_EXPERIMENT_BY_USER_CONTROL
# feature_flag: MY_EXPERIMENT_BY_USER
# name: control
# feature_flag: my_experiment
# proportion: 0.5
# value: false

Expand All @@ -77,10 +85,7 @@ class Feature:
old_behavior()

Parameters:
- `name`: a unique name for the experiment.
- `proportion`: a float between 0 and 1 representing how much of the
population should get a variant of the feature. 0.5 means 50%.
- `salt`: a way to effectively re-shuffle which bucket each id falls into
- `name`: a unique name for the experiment

If you discover a bug and roll back your feature, it's good practice to
change the salt to any other string before restarting the rollout. Changing
Expand All @@ -90,36 +95,19 @@ class Feature:

HASHSPACE = 2**128

def __init__(self, name, proportion=None, salt=None):
assert not proportion or proportion >= 0 and proportion <= 1.0
assert not salt or isinstance(salt, str)

def __init__(self, name):
self.name = name
self.feature_flag = None
self.ff_variants = None

args = {"name": name}
if proportion:
args["proportion"] = proportion
if salt:
args["salt"] = salt

# so it is hashable
self.args = tuple(sorted(args.items()))

def check_value(self, owner_id=None, repo_id=None, default=False):
"""
Returns the value of the applicable feature variant for an identifier. This is commonly a boolean for feature variants
that represent an ON variant and an OFF variant, but could be other values aswell. You can modify the values in
feature variants via Django Admin.
"""
# Will only run and refresh values from the database every ~5 minutes due to TTL cache
self._fetch_and_set_from_db(self.args)

if (
self.args
): # to create a default when `check_value()` is run for the first time
self.args = None
self._fetch_and_set_from_db()

if owner_id and not repo_id:
return self._check_value(owner_id, IdentifierType.OWNERID, default)
Expand All @@ -146,9 +134,9 @@ def _buckets(self):
- A salt

The range of possible hash values is divvied up into buckets based on the
`proportion` and `variants` members. The hash for this repo will fall into
one of those buckets and the corresponding variant (or default value) will
be returned.
`proportion` of the feature flag and its `variants`. The hash for this repo
will fall into one of those buckets and the corresponding variant (or default
value) will be returned.
"""
test_population = int(self.feature_flag.proportion * Feature.HASHSPACE)

Expand Down Expand Up @@ -200,7 +188,7 @@ def _is_valid_rollout(self):
)

@ttl_cache(maxsize=64, ttl=300) # 5 minute time-to-live cache
def _fetch_and_set_from_db(self, args=None):
def _fetch_and_set_from_db(self):
"""
Updates the instance with the newest values from database, and clears other caches so
that their values can be recalculated.
Expand All @@ -213,7 +201,7 @@ def _fetch_and_set_from_db(self, args=None):

if not new_feature_flag:
# create default feature flag
new_feature_flag = FeatureFlag.objects.create(**dict(args))
new_feature_flag = FeatureFlag.objects.create(name=self.name)

clear_cache = False

Expand Down
36 changes: 20 additions & 16 deletions tests/unit/test_rollouts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,7 @@ def test_buckets(self):
FeatureFlagVariant.objects.create(
name="complex_c", feature_flag=complex, proportion=1 / 3, value=3
)
complex_feature = Feature(
"complex",
0.5,
)
complex_feature = Feature("complex")

# # To make the math simpler, let's pretend our hash function can only
# # return 200 different values.
Expand Down Expand Up @@ -57,7 +54,7 @@ def test_fully_rolled_out(self):
# If the feature is 100% rolled out and only has one variant, then we
# should skip the hashing and bucket stuff and just return the single
# possible value.
feature = Feature("rolled_out", 1.0)
feature = Feature("rolled_out")
assert feature.check_value(owner_id=123, default=False) == True
assert not hasattr(feature.__dict__, "_buckets")

Expand All @@ -84,7 +81,6 @@ def test_overrides(self):
# hashing and just return the value for that variant.
feature = Feature(
"overrides",
1.0,
)

assert feature.check_value(owner_id=321, default=1) == 2
Expand All @@ -101,7 +97,7 @@ def test_not_in_test_gets_default(self):
proportion=1.0,
value=True,
)
feature = Feature("not_in_test", 0.1)
feature = Feature("not_in_test")
# If the feature is only 10% rolled out, 2**128-1 is way past the end of
# the test population and should get a default value back.
with patch("mmh3.hash128", return_value=2**128 - 1):
Expand All @@ -124,10 +120,7 @@ def test_return_values_for_each_bucket(self):
value="second bucket",
)

feature = Feature(
"return_values_for_each_bucket",
1.0,
)
feature = Feature("return_values_for_each_bucket")
# To make the math simpler, let's pretend our hash function can only
# return 100 different values.
with patch.object(Feature, "HASHSPACE", 100):
Expand All @@ -136,7 +129,18 @@ def test_return_values_for_each_bucket(self):
# return a value in the first bucket and then a value in the second.
with patch("mmh3.hash128", side_effect=[33, 66]):
assert feature.check_value(owner_id=123, default="c") == "first bucket"
assert feature.check_value(owner_id=123, default="c") == "second bucket"
assert feature.check_value(owner_id=124, default="c") == "second bucket"

def test_default_feature_flag_created(self):
name = "my_default_feature"
my_default_feature = Feature(name)

my_default_feature.check_value(owner_id=123123123)

feature_flag = FeatureFlag.objects.filter(name=name).first()

assert feature_flag is not None
assert feature_flag.proportion == 0


class TestFeatureExposures(TestCase):
Expand All @@ -153,8 +157,8 @@ def test_exposure_created(self):

owner_id = 123123123

MY_FEATURE = Feature("my_feature")
MY_FEATURE.check_value(owner_id=owner_id)
my_feature = Feature("my_feature")
my_feature.check_value(owner_id=owner_id)

exposure = FeatureExposure.objects.all().first()

Expand All @@ -174,8 +178,8 @@ def test_exposure_not_created(self):
with patch.object(Feature, "create_exposure") as create_exposure:
owner_id = 123123123

MY_FEATURE = Feature("my_feature")
MY_FEATURE.check_value(owner_id=owner_id)
my_feature = Feature("my_feature")
my_feature.check_value(owner_id=owner_id)

exposure = FeatureExposure.objects.first()

Expand Down
Loading