Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Add course-unawareness option to ExperimentWaffleFlag

Browse Source 
For the Courseware MFE rollout experiment, we want users'
default buckets to be consistent across course runs.

ExperimentWaffleFlag advised that this could be done
by calling `.is_enabled(...)` without a course_key argument;
however, doing so breaks when uing the main_flag.BUCKET_NUM
scheme to apply bucket rules for a specific set of users
or courses.

This commit explicitly adds course-unaware-bucketing via
a new kwarg to ExperimentWaffleFlag.__init__ method.

Furthermore, it fixes ExperimentWaffleFlag.is_enabled(course_key=None)
to work as advertised, by means of calling
.is_enabled_without_course_context on its subordinate flags.

TNL-7405
This commit is contained in:
Kyle McCormick
2020-08-06 16:23:00 -04:00
committed by Kyle McCormick
parent 07f96f2bba
commit b05948153a
4 changed files with 156 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
lms/djangoapps/courseware/toggles.py
View File

@@ -21,7 +21,9 @@ WAFFLE_FLAG_NAMESPACE = WaffleFlagNamespace(name='courseware')
# .. toggle_warnings: Also set settings.LEARNING_MICROFRONTEND_URL and ENABLE_COURSEWARE_MICROFRONTEND.
# .. toggle_tickets: TNL-7000
# .. toggle_status: supported
REDIRECT_TO_COURSEWARE_MICROFRONTEND = ExperimentWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'courseware_mfe')
REDIRECT_TO_COURSEWARE_MICROFRONTEND = ExperimentWaffleFlag(
WAFFLE_FLAG_NAMESPACE, 'courseware_mfe', use_course_aware_bucketing=False
)
# Waffle flag to display a link for the new learner experience to course teams without redirecting students.
#

69
lms/djangoapps/experiments/flags.py
View File

@@ -30,8 +30,14 @@ class ExperimentWaffleFlag(CourseWaffleFlag):
"main_flag.BUCKET_NUM" (e.g. "course_experience.animated_exy.0") to force
users that pass the first main waffle check into a specific bucket experience.
If you pass this flag a course key, tracking calls to segment will be made per-course-run
(rather than one call overall) and will include the course key.
If a user is not forced into a specific bucket by one of the aforementioned smaller flags,
then they will be randomly assigned a default bucket based on a consistent hash of:
* (flag_name, course_key, username) if use_course_aware_bucketing=True, or
* (flag_name, username) if use_course_aware_bucketing=False.
Note that you may call `.get_bucket` without a course_key, in which case:
* the smaller flags will be evaluated without course context, and
* the default bucket will be evaluated as if use_course_aware_bucketing=False.
You can also control whether the experiment only affects future enrollments by setting
an ExperimentKeyValue model object with a key of 'enrollment_start' to the date of the
@@ -56,7 +62,15 @@ class ExperimentWaffleFlag(CourseWaffleFlag):
...
"""
def __init__(self, waffle_namespace, flag_name, num_buckets=2, experiment_id=None, **kwargs):
def __init__(
self,
waffle_namespace,
flag_name,
num_buckets=2,
experiment_id=None,
use_course_aware_bucketing=True,
**kwargs
):
super().__init__(waffle_namespace, flag_name, **kwargs)
self.num_buckets = num_buckets
self.experiment_id = experiment_id
@@ -64,6 +78,7 @@ class ExperimentWaffleFlag(CourseWaffleFlag):
CourseWaffleFlag(waffle_namespace, '{}.{}'.format(flag_name, bucket))
for bucket in range(num_buckets)
]
self.use_course_aware_bucketing = use_course_aware_bucketing
def _cache_bucket(self, key, value):
request_cache = RequestCache('experiments')
@@ -119,12 +134,7 @@ class ExperimentWaffleFlag(CourseWaffleFlag):
# Keep some imports in here, because this class is commonly used at a module level, and we want to avoid
# circular imports for any models.
from experiments.models import ExperimentKeyValue
from lms.djangoapps.courseware.access import has_access
from lms.djangoapps.courseware.masquerade import (
setup_masquerade,
is_masquerading,
get_specific_masquerading_user,
)
from lms.djangoapps.courseware.masquerade import get_specific_masquerading_user
request = get_current_request()
if not request:
@@ -141,8 +151,16 @@ class ExperimentWaffleFlag(CourseWaffleFlag):
else:
masquerading_as_specific_student = True
# Use course key in experiment name to separate caches and segment calls per-course-run
experiment_name = self.namespaced_flag_name + ('.{}'.format(course_key) if course_key else '')
# If a course key is passed in, include it in the experiment name
# in order to separate caches and analytics calls per course-run.
# If we are using course-aware bucketing, then also append that course key
# to `bucketing_group_name`, such that users can be hashed into different
# buckets for different course-runs.
experiment_name = bucketing_group_name = self.namespaced_flag_name
if course_key:
experiment_name += ".{}".format(course_key)
if course_key and self.use_course_aware_bucketing:
bucketing_group_name += ".{}".format(course_key)
# Check if we have a cache for this request already
request_cache = RequestCache('experiments')
@@ -162,16 +180,28 @@ class ExperimentWaffleFlag(CourseWaffleFlag):
if not self._is_enrollment_inside_date_bounds(values, user, course_key):
return self._cache_bucket(experiment_name, 0)
bucket = stable_bucketing_hash_group(experiment_name, self.num_buckets, user.username)
# Now check if the user is forced into a particular bucket, using our subordinate bucket flags
# Determine the user's bucket.
# First check if forced into a particular bucket, using our subordinate bucket flags.
# If not, calculate their default bucket using a consistent hash function.
for i, bucket_flag in enumerate(self.bucket_flags):
if bucket_flag.is_enabled(course_key):
forced_into_bucket = (
bucket_flag.is_enabled(course_key) if course_key
else bucket_flag.is_enabled_without_course_context()
)
if forced_into_bucket:
bucket = i
break
else:
bucket = stable_bucketing_hash_group(
bucketing_group_name, self.num_buckets, user.username
)
session_key = 'tracked.{}'.format(experiment_name)
if track and hasattr(request, 'session') and session_key not in request.session and not masquerading_as_specific_student:
if (
track and hasattr(request, 'session') and
session_key not in request.session and
not masquerading_as_specific_student
):
segment.track(
user_id=user.id,
event_name='edx.bi.experiment.user.bucketed',
@@ -198,10 +228,13 @@ class ExperimentWaffleFlag(CourseWaffleFlag):
return self.is_enabled()
def is_experiment_on(self, course_key=None):
# If no course_key is supplied check the global flag irrespective of courses
"""
Return whether the overall experiment flag is enabled for this user.
This disregards `.bucket_flags`.
"""
if course_key is None:
return super().is_enabled_without_course_context()
return super().is_enabled(course_key)
@contextmanager

101
lms/djangoapps/experiments/tests/test_flags.py
View File

@@ -16,6 +16,7 @@ from experiments.factories import ExperimentKeyValueFactory
from experiments.flags import ExperimentWaffleFlag
from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory
from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag
from openedx.core.djangoapps.waffle_utils.models import WaffleFlagCourseOverrideModel
from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag
from student.tests.factories import CourseEnrollmentFactory, UserFactory
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
@@ -156,3 +157,103 @@ class ExperimentWaffleFlagTests(SharedModuleStoreTestCase):
with self.flag.override(active=active, bucket=bucket_override):
self.assertEqual(self.flag.get_bucket(), expected_bucket)
self.assertEqual(self.flag.is_experiment_on(), active)
class ExperimentWaffleFlagCourseAwarenessTest(SharedModuleStoreTestCase):
"""
Tests for how course context awareness/unawareness interacts with the
ExperimentWaffleFlag class.
"""
course_aware_flag = ExperimentWaffleFlag(
'exp', 'aware', num_buckets=20, use_course_aware_bucketing=True,
)
course_aware_subflag = CourseWaffleFlag('exp', 'aware.1')
course_unaware_flag = ExperimentWaffleFlag(
'exp', 'unaware', num_buckets=20, use_course_aware_bucketing=False,
)
course_unaware_subflag = CourseWaffleFlag('exp', 'unaware.1')
course_key_1 = CourseKey.from_string("x/y/1")
course_key_2 = CourseKey.from_string("x/y/22")
course_key_3 = CourseKey.from_string("x/y/333")
@classmethod
def setUpTestData(cls):
super().setUpTestData()
# Force all users into Bucket 1 for course at `course_key_1`.
WaffleFlagCourseOverrideModel.objects.create(
waffle_flag="exp.aware.1", course_id=cls.course_key_1, enabled=True
)
WaffleFlagCourseOverrideModel.objects.create(
waffle_flag="exp.unaware.1", course_id=cls.course_key_1, enabled=True
)
cls.user = UserFactory()
def setUp(self):
super().setUp()
self.request = RequestFactory().request()
self.request.session = {}
self.request.site = SiteFactory()
self.request.user = self.user
self.addCleanup(set_current_request, None)
set_current_request(self.request)
self.addCleanup(RequestCache.clear_all_namespaces)
# Enable all experiment waffle flags.
experiment_waffle_flag_patcher = patch.object(
ExperimentWaffleFlag, 'is_experiment_on', return_value=True
)
experiment_waffle_flag_patcher.start()
self.addCleanup(experiment_waffle_flag_patcher.stop)
# Use our custom fake `stable_bucketing_hash_group` implementation.
stable_bucket_patcher = patch(
'experiments.flags.stable_bucketing_hash_group', self._mock_stable_bucket
)
stable_bucket_patcher.start()
self.addCleanup(stable_bucket_patcher.stop)
@staticmethod
def _mock_stable_bucket(group_name, *_args, **_kwargs):
"""
A fake version of `stable_bucketing_hash_group` that just returns
the length of `group_name`.
"""
return len(group_name)
def test_course_aware_bucketing(self):
"""
Test behavior of an experiment flag configured wtih course-aware bucket hashing.
"""
# Expect queries for Course 1 to be forced into Bucket 1
# due to `course_aware_subflag`.
assert self.course_aware_flag.get_bucket(self.course_key_1) == 1
# Because we are using course-aware bucket hashing, different
# courses may default to different buckets.
# In the case of Courses 2 and 3 here, we expect two different buckets.
assert self.course_aware_flag.get_bucket(self.course_key_2) == 16
assert self.course_aware_flag.get_bucket(self.course_key_3) == 17
# We can still query a course-aware flag outside of course context,
# which has its own default bucket.
assert self.course_aware_flag.get_bucket() == 9
def test_course_unaware_bucketing(self):
"""
Test behavior of an experiment flag configured wtih course-unaware bucket hashing.
"""
# Expect queries for Course 1 to be forced into Bucket 1
# due to `course_unaware_subflag`.
# This should happen in spite of the fact that *default* bucketing
# is unaware of courses.
assert self.course_unaware_flag.get_bucket(self.course_key_1) == 1
# Expect queries for Course 2, queries for Course 3, and queries outside
# the context of the course to all be hashed into the same default bucket.
assert self.course_unaware_flag.get_bucket(self.course_key_2) == 11
assert self.course_unaware_flag.get_bucket(self.course_key_3) == 11
assert self.course_unaware_flag.get_bucket() == 11

2
openedx/core/djangoapps/waffle_utils/__init__.py
View File

@@ -477,7 +477,7 @@ class CourseWaffleFlag(WaffleFlag):
"""
return self._is_enabled()
def is_enabled(self, course_key=None):
def is_enabled(self, course_key):
"""
Returns whether or not the flag is enabled within the context of a given course.