diff --git a/openedx/core/djangoapps/waffle_utils/__init__.py b/openedx/core/djangoapps/waffle_utils/__init__.py index d563a42611..1066a1eecf 100644 --- a/openedx/core/djangoapps/waffle_utils/__init__.py +++ b/openedx/core/djangoapps/waffle_utils/__init__.py @@ -67,6 +67,8 @@ from contextlib import contextmanager import crum import six +from django.conf import settings +from edx_django_utils.monitoring import set_custom_metric from opaque_keys.edx.keys import CourseKey from waffle import flag_is_active, switch_is_active @@ -291,8 +293,60 @@ class WaffleFlagNamespace(six.with_metaclass(ABCMeta, WaffleNamespace)): return bool(flag_undefined_default) self._cached_flags[namespaced_flag_name] = value + + self._set_waffle_flag_metric(namespaced_flag_name, value) return value + 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. + + 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. + + 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. + + """ + is_waffle_flag_metric_enabled = getattr(settings, _ENABLE_WAFFLE_FLAG_METRIC, False) + if not is_waffle_flag_metric_enabled: + return + + flag_metric_data = self._get_request_cache().setdefault('flag_metric', {}) + is_value_change = False + if name not in flag_metric_data: + flag_metric_data[name] = str(value) + is_value_change = True + else: + if flag_metric_data[name] != str(value): + flag_metric_data[name] = 'Both' + is_value_change = True + + if is_value_change: + set_custom_metric('waffle_flags', str(flag_metric_data)) + + # .. 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_category: monitoring + # .. toggle_use_cases: opt_out + # .. 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' + 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 3cc1d6c6e2..3d3524861c 100644 --- a/openedx/core/djangoapps/waffle_utils/tests/test_init.py +++ b/openedx/core/djangoapps/waffle_utils/tests/test_init.py @@ -6,8 +6,9 @@ import crum import ddt from django.test import TestCase from django.test.client import RequestFactory +from django.test.utils import override_settings from edx_django_utils.cache import RequestCache -from mock import patch +from mock import call, patch from opaque_keys.edx.keys import CourseKey from waffle.testutils import override_flag @@ -37,13 +38,15 @@ class TestCourseWaffleFlag(TestCase): crum.set_current_request(request) RequestCache.clear_all_namespaces() + @override_settings(ENABLE_WAFFLE_FLAG_METRIC=True) + @patch('openedx.core.djangoapps.waffle_utils.set_custom_metric') @ddt.data( {'course_override': WaffleFlagCourseOverrideModel.ALL_CHOICES.on, 'waffle_enabled': False, 'result': True}, {'course_override': WaffleFlagCourseOverrideModel.ALL_CHOICES.off, 'waffle_enabled': True, 'result': False}, {'course_override': WaffleFlagCourseOverrideModel.ALL_CHOICES.unset, 'waffle_enabled': True, 'result': True}, {'course_override': WaffleFlagCourseOverrideModel.ALL_CHOICES.unset, 'waffle_enabled': False, 'result': False}, ) - def test_course_waffle_flag(self, data): + def test_course_waffle_flag(self, data, mock_set_custom_metric): """ Tests various combinations of a flag being set in waffle and overridden for a course. @@ -59,22 +62,32 @@ class TestCourseWaffleFlag(TestCase): 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. - self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_2_KEY), data['waffle_enabled']) + 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. - self.assertEqual(self.TEST_COURSE_FLAG.is_enabled(self.TEST_COURSE_2_KEY), 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) + @patch('openedx.core.djangoapps.waffle_utils.set_custom_metric') @ddt.data( {'flag_undefined_default': None, 'result': False}, {'flag_undefined_default': False, 'result': False}, {'flag_undefined_default': True, 'result': True}, ) - def test_undefined_waffle_flag(self, data): + def test_undefined_waffle_flag(self, data, mock_set_custom_metric): """ Test flag with various defaults provided for undefined waffle flags. """ @@ -98,6 +111,8 @@ class TestCourseWaffleFlag(TestCase): self.TEST_COURSE_KEY ) + self._assert_waffle_flag_metric(mock_set_custom_metric, expected_flag_value=str(data['result'])) + @ddt.data( {'flag_undefined_default': None, 'result': False}, {'flag_undefined_default': False, 'result': False}, @@ -115,6 +130,21 @@ class TestCourseWaffleFlag(TestCase): ) self.assertEqual(test_course_flag.is_enabled(self.TEST_COURSE_KEY), data['result']) + @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 _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)] + mock_set_custom_metric.assert_has_calls(expected_calls) + self.assertEqual(mock_set_custom_metric.call_count, 1) + else: + self.assertEqual(mock_set_custom_metric.call_count, 0) + class TestWaffleSwitch(TestCase): """