From a1572dafcef82f8167d366e59ffa62ffb66805fc Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Thu, 18 Jun 2020 12:21:57 -0400 Subject: [PATCH] add waffle_flag custom metric (#24244) If setting ENABLE_WAFFLE_FLAG_METRIC is True, a custom metric will be added with the values of all WaffleFlag and CourseWaffleFlags seen during the transaction. 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. TODO: A how_to can be added later if we find this useful, including helpful querying tips. ARCHBOM-132 --- .../core/djangoapps/waffle_utils/__init__.py | 54 +++++++++++++++++++ .../waffle_utils/tests/test_init.py | 40 ++++++++++++-- 2 files changed, 89 insertions(+), 5 deletions(-) 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): """