diff --git a/openedx/core/djangoapps/waffle_utils/__init__.py b/openedx/core/djangoapps/waffle_utils/__init__.py
index 1066a1eecfa7afe757476cfa6d536d84d9d13208..7d965bf45c2feb5b678113c433059d411b1ba1c5 100644
--- a/openedx/core/djangoapps/waffle_utils/__init__.py
+++ b/openedx/core/djangoapps/waffle_utils/__init__.py
@@ -299,26 +299,19 @@ class WaffleFlagNamespace(six.with_metaclass(ABCMeta, WaffleNamespace)):
def _set_waffle_flag_metric(self, name, value):
"""
- If ENABLE_WAFFLE_FLAG_METRIC is True, adds name/value to cached values
- and sets metric if the value changed.
+ For any flag name in _WAFFLE_FLAG_CUSTOM_METRIC_SET, add name/value
+ to cached values and set custom metric if the value changed.
- Metric flag values could be False, True, or Both. The value Both would
- mean that the flag had both a True and False value at different times
- through the transaction. This is most likely due to having a
- check_before_waffle_callback, as is the case with CourseWaffleFlag.
+ The name of the custom metric will match the name of the flag.
+ The value of the custom metric could be False, True, or Both.
- Example metric value::
-
- "{'another.course.flag': 'False', 'some.flag': 'False', 'some.course.flag': 'Both'}"
-
- Warning: NewRelic does not recommend large custom metric values due to
- the potential performance impact on the agent, so you may just want to
- enable when researching usage of a particular flag. Metric values longer
- than 255 are truncated.
+ The value Both would mean that the flag had both a True and False
+ value at different times during the transaction. This is most
+ likely due to having a check_before_waffle_callback, as is the
+ case with CourseWaffleFlag.
"""
- is_waffle_flag_metric_enabled = getattr(settings, _ENABLE_WAFFLE_FLAG_METRIC, False)
- if not is_waffle_flag_metric_enabled:
+ if name not in _WAFFLE_FLAG_CUSTOM_METRIC_SET:
return
flag_metric_data = self._get_request_cache().setdefault('flag_metric', {})
@@ -332,20 +325,32 @@ class WaffleFlagNamespace(six.with_metaclass(ABCMeta, WaffleNamespace)):
is_value_change = True
if is_value_change:
- set_custom_metric('waffle_flags', str(flag_metric_data))
+ set_custom_metric(name, flag_metric_data[name])
+
+
+def _get_waffle_flag_custom_metrics_set():
+ """
+ Returns a set based on the Django setting WAFFLE_FLAG_CUSTOM_METRICS (list).
+ """
+ waffle_flag_custom_metrics = getattr(settings, _WAFFLE_FLAG_CUSTOM_METRICS, None)
+ waffle_flag_custom_metrics = waffle_flag_custom_metrics if waffle_flag_custom_metrics else []
+ return set(waffle_flag_custom_metrics)
# .. toggle_name: ENABLE_WAFFLE_FLAG_METRIC
# .. toggle_implementation: DjangoSetting
# .. toggle_default: False
- # .. toggle_description: Set to True to enable a custom metric with waffle flag values (True, False, Both).
+ # .. toggle_description: A list of waffle flag to track with custom metrics having values of (True, False, or Both).
# .. toggle_category: monitoring
- # .. toggle_use_cases: opt_out
+ # .. toggle_use_cases: opt_in
# .. toggle_creation_date: 2020-06-17
# .. toggle_expiration_date: None
# .. toggle_tickets: None
# .. toggle_status: supported
- # .. toggle_warnings: Metric of flags could be large and heavier than typical metrics.
-_ENABLE_WAFFLE_FLAG_METRIC = 'ENABLE_WAFFLE_FLAG_METRIC'
+ # .. toggle_warnings: Intent is for temporary research (1 day - several weeks) of a flag's usage.
+_WAFFLE_FLAG_CUSTOM_METRICS = 'WAFFLE_FLAG_CUSTOM_METRICS'
+
+# set of waffle flags that should be instrumented with custom metrics
+_WAFFLE_FLAG_CUSTOM_METRIC_SET = _get_waffle_flag_custom_metrics_set()
class WaffleFlag(object):
diff --git a/openedx/core/djangoapps/waffle_utils/tests/test_init.py b/openedx/core/djangoapps/waffle_utils/tests/test_init.py
index 3d3524861c1dff4cc2dbce3672844d84b9bee538..bdf34674feaae29c20ef8bc7d04b96b899e4dade 100644
--- a/openedx/core/djangoapps/waffle_utils/tests/test_init.py
+++ b/openedx/core/djangoapps/waffle_utils/tests/test_init.py
@@ -12,7 +12,13 @@ from mock import call, patch
from opaque_keys.edx.keys import CourseKey
from waffle.testutils import override_flag
-from .. import CourseWaffleFlag, WaffleFlagNamespace, WaffleSwitchNamespace, WaffleSwitch
+from .. import (
+ _get_waffle_flag_custom_metrics_set,
+ CourseWaffleFlag,
+ WaffleFlagNamespace,
+ WaffleSwitchNamespace,
+ WaffleSwitch,
+)
from ..models import WaffleFlagCourseOverrideModel
@@ -25,6 +31,8 @@ class TestCourseWaffleFlag(TestCase):
NAMESPACE_NAME = "test_namespace"
FLAG_NAME = "test_flag"
NAMESPACED_FLAG_NAME = NAMESPACE_NAME + "." + FLAG_NAME
+ FLAG_2_NAME = "test_flag_2"
+ NAMESPACED_FLAG_2_NAME = NAMESPACE_NAME + "." + FLAG_2_NAME
TEST_COURSE_KEY = CourseKey.from_string("edX/DemoX/Demo_Course")
TEST_COURSE_2_KEY = CourseKey.from_string("edX/DemoX/Demo_Course_2")
@@ -38,7 +46,7 @@ class TestCourseWaffleFlag(TestCase):
crum.set_current_request(request)
RequestCache.clear_all_namespaces()
- @override_settings(ENABLE_WAFFLE_FLAG_METRIC=True)
+ @override_settings(WAFFLE_FLAG_CUSTOM_METRICS=[NAMESPACED_FLAG_NAME])
@patch('openedx.core.djangoapps.waffle_utils.set_custom_metric')
@ddt.data(
{'course_override': WaffleFlagCourseOverrideModel.ALL_CHOICES.on, 'waffle_enabled': False, 'result': True},
@@ -51,36 +59,40 @@ class TestCourseWaffleFlag(TestCase):
Tests various combinations of a flag being set in waffle and overridden
for a course.
"""
- with patch.object(WaffleFlagCourseOverrideModel, 'override_value', return_value=data['course_override']):
- with override_flag(self.NAMESPACED_FLAG_NAME, active=data['waffle_enabled']):
- # check twice to test that the result is properly cached
- self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY), data['result'])
- self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY), data['result'])
- # result is cached, so override check should happen once
- WaffleFlagCourseOverrideModel.override_value.assert_called_once_with(
- self.NAMESPACED_FLAG_NAME,
- self.TEST_COURSE_KEY
- )
-
- self._assert_waffle_flag_metric(mock_set_custom_metric, expected_flag_value=str(data['result']))
- mock_set_custom_metric.reset_mock()
-
- # check flag for a second course
- if data['course_override'] == WaffleFlagCourseOverrideModel.ALL_CHOICES.unset:
- # When course override wasn't set for the first course, the second course will get the same
- # cached value from waffle.
- second_value = data['waffle_enabled']
- self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_2_KEY), second_value)
- else:
- # When course override was set for the first course, it should not apply to the second
- # course which should get the default value of False.
- second_value = False
- self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_2_KEY), second_value)
+ with patch(
+ 'openedx.core.djangoapps.waffle_utils._WAFFLE_FLAG_CUSTOM_METRIC_SET',
+ _get_waffle_flag_custom_metrics_set(),
+ ):
+ with patch.object(WaffleFlagCourseOverrideModel, 'override_value', return_value=data['course_override']):
+ with override_flag(self.NAMESPACED_FLAG_NAME, active=data['waffle_enabled']):
+ # check twice to test that the result is properly cached
+ self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY), data['result'])
+ self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_KEY), data['result'])
+ # result is cached, so override check should happen once
+ WaffleFlagCourseOverrideModel.override_value.assert_called_once_with(
+ self.NAMESPACED_FLAG_NAME,
+ self.TEST_COURSE_KEY
+ )
+
+ self._assert_waffle_flag_metric(mock_set_custom_metric, expected_flag_value=str(data['result']))
+ mock_set_custom_metric.reset_mock()
+
+ # check flag for a second course
+ if data['course_override'] == WaffleFlagCourseOverrideModel.ALL_CHOICES.unset:
+ # When course override wasn't set for the first course, the second course will get the same
+ # cached value from waffle.
+ second_value = data['waffle_enabled']
+ self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_2_KEY), second_value)
+ else:
+ # When course override was set for the first course, it should not apply to the second
+ # course which should get the default value of False.
+ second_value = False
+ self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_2_KEY), second_value)
expected_flag_value = None if second_value == data['result'] else 'Both'
self._assert_waffle_flag_metric(mock_set_custom_metric, expected_flag_value=expected_flag_value)
- @override_settings(ENABLE_WAFFLE_FLAG_METRIC=True)
+ @override_settings(WAFFLE_FLAG_CUSTOM_METRICS=[NAMESPACED_FLAG_NAME])
@patch('openedx.core.djangoapps.waffle_utils.set_custom_metric')
@ddt.data(
{'flag_undefined_default': None, 'result': False},
@@ -97,19 +109,23 @@ class TestCourseWaffleFlag(TestCase):
flag_undefined_default=data['flag_undefined_default']
)
- with patch.object(
- WaffleFlagCourseOverrideModel,
- 'override_value',
- return_value=WaffleFlagCourseOverrideModel.ALL_CHOICES.unset
+ with patch(
+ 'openedx.core.djangoapps.waffle_utils._WAFFLE_FLAG_CUSTOM_METRIC_SET',
+ _get_waffle_flag_custom_metrics_set(),
):
- # check twice to test that the result is properly cached
- self.assertEqual(test_course_flag.is_enabled(self.TEST_COURSE_KEY), data['result'])
- self.assertEqual(test_course_flag.is_enabled(self.TEST_COURSE_KEY), data['result'])
- # result is cached, so override check should happen once
- WaffleFlagCourseOverrideModel.override_value.assert_called_once_with(
- self.NAMESPACED_FLAG_NAME,
- self.TEST_COURSE_KEY
- )
+ with patch.object(
+ WaffleFlagCourseOverrideModel,
+ 'override_value',
+ return_value=WaffleFlagCourseOverrideModel.ALL_CHOICES.unset
+ ):
+ # check twice to test that the result is properly cached
+ self.assertEqual(test_course_flag.is_enabled(self.TEST_COURSE_KEY), data['result'])
+ self.assertEqual(test_course_flag.is_enabled(self.TEST_COURSE_KEY), data['result'])
+ # result is cached, so override check should happen once
+ WaffleFlagCourseOverrideModel.override_value.assert_called_once_with(
+ self.NAMESPACED_FLAG_NAME,
+ self.TEST_COURSE_KEY
+ )
self._assert_waffle_flag_metric(mock_set_custom_metric, expected_flag_value=str(data['result']))
@@ -130,16 +146,28 @@ class TestCourseWaffleFlag(TestCase):
)
self.assertEqual(test_course_flag.is_enabled(self.TEST_COURSE_KEY), data['result'])
+ @ddt.data(
+ {'expected_count': 0, 'waffle_flag_metric_setting': None},
+ {'expected_count': 1, 'waffle_flag_metric_setting': [NAMESPACED_FLAG_NAME]},
+ {'expected_count': 2, 'waffle_flag_metric_setting': [NAMESPACED_FLAG_NAME, NAMESPACED_FLAG_2_NAME]},
+ )
@patch('openedx.core.djangoapps.waffle_utils.set_custom_metric')
- def test_waffle_flag_metric_disabled(self, mock_set_custom_metric):
- test_course_flag = CourseWaffleFlag(self.TEST_NAMESPACE, self.FLAG_NAME)
- test_course_flag.is_enabled(self.TEST_COURSE_KEY)
- self.assertEqual(mock_set_custom_metric.call_count, 0)
+ def test_waffle_flag_metric_for_various_settings(self, data, mock_set_custom_metric):
+ with override_settings(WAFFLE_FLAG_CUSTOM_METRICS=data['waffle_flag_metric_setting']):
+ with patch(
+ 'openedx.core.djangoapps.waffle_utils._WAFFLE_FLAG_CUSTOM_METRIC_SET',
+ _get_waffle_flag_custom_metrics_set(),
+ ):
+ test_course_flag = CourseWaffleFlag(self.TEST_NAMESPACE, self.FLAG_NAME)
+ test_course_flag.is_enabled(self.TEST_COURSE_KEY)
+ test_course_flag_2 = CourseWaffleFlag(self.TEST_NAMESPACE, self.FLAG_2_NAME)
+ test_course_flag_2.is_enabled(self.TEST_COURSE_KEY)
+
+ self.assertEqual(mock_set_custom_metric.call_count, data['expected_count'])
def _assert_waffle_flag_metric(self, mock_set_custom_metric, expected_flag_value=None):
if expected_flag_value:
- expected_metric_value = str({self.NAMESPACED_FLAG_NAME: expected_flag_value})
- expected_calls = [call('waffle_flags', expected_metric_value)]
+ expected_calls = [call(self.NAMESPACED_FLAG_NAME, expected_flag_value)]
mock_set_custom_metric.assert_has_calls(expected_calls)
self.assertEqual(mock_set_custom_metric.call_count, 1)
else: