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.

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

feat: create feature exposures model #141

Merged
merged 8 commits into from
Mar 12, 2024
Merged
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
54 changes: 54 additions & 0 deletions shared/django_apps/rollouts/migrations/0004_featureexposure.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# Generated by Django 5.0.3 on 2024-03-12 20:06

import django.db.models.deletion
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("rollouts", "0003_alter_featureflag_proportion_and_more"),
]

# BEGIN;
# --
# -- Create model FeatureExposure
# --
# CREATE TABLE "feature_exposures" ("exposure_id" integer NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, "owner" integer NULL, "repo" integer NULL, "timestamp" timestamp with time zone NOT NULL, "feature_flag_id" varchar(200) NOT NULL, "feature_flag_variant_id" integer NOT NULL);
# ALTER TABLE "feature_exposures" ADD CONSTRAINT "feature_exposures_feature_flag_id_628f8212_fk_feature_f" FOREIGN KEY ("feature_flag_id") REFERENCES "feature_flags" ("name") DEFERRABLE INITIALLY DEFERRED;
# ALTER TABLE "feature_exposures" ADD CONSTRAINT "feature_exposures_feature_flag_variant_bfb854ff_fk_feature_f" FOREIGN KEY ("feature_flag_variant_id") REFERENCES "feature_flag_variants" ("variant_id") DEFERRABLE INITIALLY DEFERRED;
# CREATE INDEX "feature_exposures_feature_flag_id_628f8212" ON "feature_exposures" ("feature_flag_id");
# CREATE INDEX "feature_exposures_feature_flag_id_628f8212_like" ON "feature_exposures" ("feature_flag_id" varchar_pattern_ops);
# CREATE INDEX "feature_exposures_feature_flag_variant_id_bfb854ff" ON "feature_exposures" ("feature_flag_variant_id");
# COMMIT;

operations = [
migrations.CreateModel(
name="FeatureExposure",
fields=[
("exposure_id", models.AutoField(primary_key=True, serialize=False)),
("owner", models.IntegerField(blank=True, null=True)),
("repo", models.IntegerField(blank=True, null=True)),
("timestamp", models.DateTimeField()),
(
"feature_flag",
models.ForeignKey(
on_delete=django.db.models.deletion.CASCADE,
related_name="exposures",
to="rollouts.featureflag",
),
),
(
"feature_flag_variant",
models.ForeignKey(
on_delete=django.db.models.deletion.CASCADE,
related_name="exposures",
to="rollouts.featureflagvariant",
),
),
],
options={
"db_table": "feature_exposures",
},
),
]
41 changes: 41 additions & 0 deletions shared/django_apps/rollouts/models.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from random import choice

from django.core.exceptions import ValidationError
from django.db import models
from django_better_admin_arrayfield.models.fields import ArrayField

Expand Down Expand Up @@ -74,3 +75,43 @@ class Meta:

def __str__(self):
return self.feature_flag.__str__() + ": " + self.name


class FeatureExposure(models.Model):
"""
Represents a feature variant being exposed to an entity (repo or owner) at
a point in time. Used to keep track of when features and variants have been enabled
and who they affected for metrics purposes.
"""

exposure_id = models.AutoField(primary_key=True)
feature_flag = models.ForeignKey(
"FeatureFlag", on_delete=models.CASCADE, related_name="exposures"
)
feature_flag_variant = models.ForeignKey(
"FeatureFlagVariant", on_delete=models.CASCADE, related_name="exposures"
)

# Weak foreign keys to Owner and Respository models respectively
owner = models.IntegerField(null=True, blank=True)
repo = models.IntegerField(null=True, blank=True)

timestamp = models.DateTimeField(null=False)

def clean(self):
if not self.owner and not self.repo:
raise ValidationError(
"Exposure must have either a corresponding owner or repo"
)

super(FeatureExposure, self).clean()

def save(self, *args, **kwargs):
self.full_clean()
super(FeatureExposure, self).save(*args, **kwargs)

class Meta:
db_table = "feature_exposures"
# indexes = [ # don't use indexes for now
# models.Index(fields=['feature_flag', 'timestamp'], name='feature_flag_timestamp_idx'),
# ]
56 changes: 48 additions & 8 deletions shared/rollouts/__init__.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,26 @@
import logging
from enum import Enum
from functools import cached_property

import mmh3
from asgiref.sync import sync_to_async
from cachetools.func import lru_cache, ttl_cache
from django.utils import timezone

from shared.django_apps.rollouts.models import FeatureFlag, FeatureFlagVariant
from shared.django_apps.rollouts.models import (
FeatureExposure,
FeatureFlag,
FeatureFlagVariant,
)

log = logging.getLogger("__name__")


class IdentifierType(Enum):
OWNERID = "ownerid"
REPOID = "repoid"


class Feature:
"""
Represents a feature and its rollout parameters, fetched from the database (see django_apps/rollouts/models.py).
Expand Down Expand Up @@ -96,7 +107,7 @@ def __init__(self, name, proportion=None, salt=None):
# so it is hashable
self.args = tuple(sorted(args.items()))

def check_value(self, identifier, default=False):
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
Expand All @@ -110,7 +121,13 @@ def check_value(self, identifier, default=False):
): # to create a default when `check_value()` is run for the first time
self.args = None

return self._check_value(identifier, default)
if owner_id and not repo_id:
return self._check_value(owner_id, IdentifierType.OWNERID, default)
if repo_id and not owner_id:
return self._check_value(repo_id, IdentifierType.REPOID, default)
raise Exception(
"Must pass in exactly one of owner_id or repo_id keyword arguments to check_value()"
)

@sync_to_async
def check_value_async(self, identifier, default=False):
Expand Down Expand Up @@ -143,15 +160,21 @@ def _buckets(self):

return buckets

def _get_override_variant(self, identifier):
def _get_override_variant(self, identifier, identifier_type: IdentifierType):
"""
Retrieves the feature variant applicable to the given identifer according to
defined overrides. Returns None if no override is found.
"""
for variant in self.ff_variants:
if (
identifier in variant.override_owner_ids
or identifier in variant.override_repo_ids
identifier_type == IdentifierType.OWNERID
and identifier in variant.override_owner_ids
):
return variant

if (
identifier_type == IdentifierType.REPOID
and identifier in variant.override_repo_ids
):
return variant
return None
Expand Down Expand Up @@ -231,14 +254,14 @@ def _fetch_and_set_from_db(self, args=None):
# In this case, we are okay with sharing a cache across instances, and the
# instances are all global constants so they won't be torn down anyway.
@lru_cache(maxsize=64)
def _check_value(self, identifier, default):
def _check_value(self, identifier, identifier_type: IdentifierType, default):
"""
This function will have its cache invalidated when `_fetch_and_set_from_db()` pulls new data so that
variant values can be returned using the most up-to-date values from the database
"""

# check if an override exists
override_variant = self._get_override_variant(identifier)
override_variant = self._get_override_variant(identifier, identifier_type)

if override_variant:
return override_variant.value
Expand All @@ -253,6 +276,7 @@ def _check_value(self, identifier, default):
)
for bucket, variant in self._buckets:
if key <= bucket:
self.create_exposure(variant, identifier, identifier_type)
return variant.value

return default
Expand All @@ -267,3 +291,19 @@ def _is_different(self, inst1, inst2):
return False

return True

def create_exposure(self, variant, identifier, identifier_type: IdentifierType):
"""
Creates an exposure record indicating that a feature variant has been applied to
an entity (repo or owner) at a current point in time.
"""
args = {
"feature_flag": self.feature_flag,
"feature_flag_variant": variant,
"timestamp": timezone.now(),
}
if identifier_type == IdentifierType.OWNERID:
args["owner"] = identifier
elif identifier_type == IdentifierType.REPOID:
args["repo"] = identifier
FeatureExposure.objects.create(**args)
68 changes: 59 additions & 9 deletions tests/unit/test_rollouts.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
from unittest.mock import PropertyMock, patch
from unittest.mock import patch

from django.test import TestCase

from shared.django_apps.rollouts.models import FeatureFlag, FeatureFlagVariant
from shared.django_apps.rollouts.models import (
FeatureExposure,
FeatureFlag,
FeatureFlagVariant,
)
from shared.rollouts import Feature


Expand All @@ -29,7 +33,7 @@ def test_buckets(self):
# # return 200 different values.
with patch.object(Feature, "HASHSPACE", 200):

complex_feature.check_value("garbage") # to force fetch values from db
complex_feature.check_value(owner_id=1234) # to force fetch values from db

# Because the top-level feature proportion is 0.5, we are only using the
# first 50% of our 200 hash values as our test population: [0..100]
Expand All @@ -54,7 +58,7 @@ def test_fully_rolled_out(self):
# should skip the hashing and bucket stuff and just return the single
# possible value.
feature = Feature("rolled_out", 1.0)
assert feature.check_value("any", default=False) == True
assert feature.check_value(owner_id=123, default=False) == True
assert not hasattr(feature.__dict__, "_buckets")

def test_overrides(self):
Expand Down Expand Up @@ -83,8 +87,8 @@ def test_overrides(self):
1.0,
)

assert feature.check_value(321, default=1) == 2
assert feature.check_value(123, default=2) == 1
assert feature.check_value(owner_id=321, default=1) == 2
assert feature.check_value(owner_id=123, default=2) == 1
assert not hasattr(feature.__dict__, "_buckets")

def test_not_in_test_gets_default(self):
Expand All @@ -101,7 +105,7 @@ def test_not_in_test_gets_default(self):
# 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):
assert feature.check_value("not in test", default="default") == "default"
assert feature.check_value(owner_id=123, default="default") == "default"

def test_return_values_for_each_bucket(self):
return_values_for_each_bucket = FeatureFlag.objects.create(
Expand Down Expand Up @@ -131,5 +135,51 @@ def test_return_values_for_each_bucket(self):
# the buckets are [0..50] and [51..100]. Mock the hash function to
# 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("any1", default="c") == "first bucket"
assert feature.check_value("any2", default="c") == "second bucket"
assert feature.check_value(owner_id=123, default="c") == "first bucket"
assert feature.check_value(owner_id=123, default="c") == "second bucket"


class TestFeatureExposures(TestCase):
def test_exposure_created(self):
complex = FeatureFlag.objects.create(
name="my_feature", proportion=1.0, salt="random_salt"
)
enabled = FeatureFlagVariant.objects.create(
name="enabled", feature_flag=complex, proportion=1.0, value=True
)
FeatureFlagVariant.objects.create(
name="disabled", feature_flag=complex, proportion=0, value=False
)

owner_id = 123123123

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

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

assert exposure is not None
assert exposure.owner == owner_id
assert exposure.feature_flag == complex
assert exposure.feature_flag_variant == enabled

def test_exposure_not_created(self):
complex = FeatureFlag.objects.create(
name="my_feature", proportion=1.0, salt="random_salt"
)
FeatureFlagVariant.objects.create(
name="enabled", feature_flag=complex, proportion=0, value=True
)

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)

exposure = FeatureExposure.objects.first()

# Should not create an exposure because the owner was not exposed to any
# explicit variant, it was assigned the default behaviour
assert exposure is None
create_exposure.assert_not_called()
Loading