From f060872878efec02fd49af9e390ba0674a23a29b Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Tue, 23 Jun 2020 13:55:06 -0400 Subject: [PATCH] update waffle flag and docs (#24299) - update ADR to provide more alternatives for updating the default value of a flag. - add a `flag_` prefix to the flag metrics - add module-level note about flag metrics - add NewRelic query example and warning - fix typo in toggle annotation ARCHBOM-1302 --- .../core/djangoapps/waffle_utils/__init__.py | 32 +++++++++++++------ .../0001-refactor-waffle-flag-default.rst | 10 ++++-- .../waffle_utils/tests/test_init.py | 3 +- 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/openedx/core/djangoapps/waffle_utils/__init__.py b/openedx/core/djangoapps/waffle_utils/__init__.py index ebce19205f..0bb3872f8f 100644 --- a/openedx/core/djangoapps/waffle_utils/__init__.py +++ b/openedx/core/djangoapps/waffle_utils/__init__.py @@ -44,14 +44,12 @@ To test WaffleSwitchNamespace, use the provided context managers. For example: with WAFFLE_SWITCHES.override(waffle.ESTIMATE_FIRST_ATTEMPTED, active=True): ... -For long-lived flags, you may want to change the default for the flag from "off" -to "on", so that it is "on" by default in devstack, sandboxes, or new Open edX -releases, more closely matching what is in Production. This is for flags that -can't yet be deleted, for example if there are still course overrides. +For long-lived flags, you may want to change the default for devstack, sandboxes, +or new Open edX releases. For help with this, see: +openedx/core/djangoapps/waffle_utils/docs/decisions/0001-refactor-waffle-flag-default.rst - * To do so, add a migration that adds the flag as active if the record doesn't - already exist. For more details, see: - openedx/core/djangoapps/waffle_utils/docs/decisions/0001-refactor-waffle-flag-default.rst +Also see ``WAFFLE_FLAG_CUSTOM_METRICS`` and docstring for _set_waffle_flag_metric +for temporarily instrumenting/monitoring waffle flag usage. """ @@ -311,7 +309,8 @@ class WaffleFlagNamespace(six.with_metaclass(ABCMeta, WaffleNamespace)): For any flag name in _WAFFLE_FLAG_CUSTOM_METRIC_SET, add name/value to cached values and set custom metric if the value changed. - The name of the custom metric will match the name of the flag. + The name of the custom metric will have the prefix ``flag_`` and the + suffix will match the name of the flag. The value of the custom metric could be False, True, or Both. The value Both would mean that the flag had both a True and False @@ -319,6 +318,18 @@ class WaffleFlagNamespace(six.with_metaclass(ABCMeta, WaffleNamespace)): likely due to having a check_before_waffle_callback, as is the case with CourseWaffleFlag. + An example NewRelic query to see the values of a flag in different + environments, if your waffle flag was named ``my.waffle.flag`` might + look like:: + + SELECT count(*) FROM Transaction + WHERE flag_my.waffle.flag IS NOT NULL + FACET appName, flag_my.waffle.flag + + Important: Remember to configure ``WAFFLE_FLAG_CUSTOM_METRICS`` for + LMS, Studio and Workers in order to see waffle flag usage in all + edx-platform environments. + """ if name not in _WAFFLE_FLAG_CUSTOM_METRIC_SET: return @@ -334,7 +345,8 @@ class WaffleFlagNamespace(six.with_metaclass(ABCMeta, WaffleNamespace)): is_value_change = True if is_value_change: - set_custom_metric(name, flag_metric_data[name]) + metric_name = 'flag_{}'.format(name) + set_custom_metric(metric_name, flag_metric_data[name]) def _get_waffle_flag_custom_metrics_set(): @@ -345,7 +357,7 @@ def _get_waffle_flag_custom_metrics_set(): 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_name: WAFFLE_FLAG_CUSTOM_METRICS # .. toggle_implementation: DjangoSetting # .. toggle_default: False # .. toggle_description: A list of waffle flag to track with custom metrics having values of (True, False, or Both). diff --git a/openedx/core/djangoapps/waffle_utils/docs/decisions/0001-refactor-waffle-flag-default.rst b/openedx/core/djangoapps/waffle_utils/docs/decisions/0001-refactor-waffle-flag-default.rst index 9f7d5b4f56..3c8489daaa 100644 --- a/openedx/core/djangoapps/waffle_utils/docs/decisions/0001-refactor-waffle-flag-default.rst +++ b/openedx/core/djangoapps/waffle_utils/docs/decisions/0001-refactor-waffle-flag-default.rst @@ -17,14 +17,18 @@ Decision ======== * Retire the ``flag_undefined_default`` argument for WaffleFlag and CourseWaffleFlag that allowed you to change the default to True in code. -* In the future, the alternative would be to add a migration that adds an active (True) waffle flag database record if a record doesn't already exist. +* Do not introduce any new ability to adjust the default in code. +* In the future, alternative solutions might include: - * If the record already exists, do not replace it whether or not it is active. Its value is already represented in the database and may have been set via admin. + * The flag can simply be removed at this time if it was meant to be temporary. + * The flag could be replaced with a Django Setting, if the features of a flag are not needed for the permanent toggle. + * You could a migration that adds an active (True) waffle flag database record if a record doesn't already exist. + * You could introduce and replace an *enable* flag with a new *disable* flag. This might be useful when a permanent toggle is desired that requires features of a waffle flag, and where disabling would now be the exceptional case, since the default would be inactive (False). Consequences ============ -* We will need to add the appropriate migrations for each flag currently using ``flag_undefined_default=True``, and then finally remove the deprecated ``flag_undefined_default`` argument. +* We will need to implement one of the alternate solutions for each flag currently using ``flag_undefined_default=True``, and then finally remove the deprecated ``flag_undefined_default`` argument. Rejected Alternatives ===================== diff --git a/openedx/core/djangoapps/waffle_utils/tests/test_init.py b/openedx/core/djangoapps/waffle_utils/tests/test_init.py index 7a60ee3a3c..f29b16e40f 100644 --- a/openedx/core/djangoapps/waffle_utils/tests/test_init.py +++ b/openedx/core/djangoapps/waffle_utils/tests/test_init.py @@ -171,7 +171,8 @@ class TestCourseWaffleFlag(TestCase): def _assert_waffle_flag_metric(self, mock_set_custom_metric, expected_flag_value=None, flag_undefined_default=None): if expected_flag_value: - expected_calls = [call(self.NAMESPACED_FLAG_NAME, expected_flag_value)] + expected_flag_name = 'flag_{}'.format(self.NAMESPACED_FLAG_NAME) + expected_calls = [call(expected_flag_name, expected_flag_value)] mock_set_custom_metric.assert_has_calls(expected_calls) expected_call_count = 2 if flag_undefined_default else 1 self.assertEqual(mock_set_custom_metric.call_count, expected_call_count)