Skip to content

Commit

Permalink
feat: create feature exposures model (#141)
Browse files Browse the repository at this point in the history
* create model + migrations

* modify interface

* add table name

* fix tests

* update docs

* update migration file

* add tests

* cleanup
  • Loading branch information
daniel-codecov authored Mar 12, 2024
1 parent 5cbdf92 commit a98be92
Show file tree
Hide file tree
Showing 4 changed files with 202 additions and 17 deletions.
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()

0 comments on commit a98be92

Please sign in to comment.