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
This commit is contained in:
@@ -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).
|
||||
|
||||
@@ -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
|
||||
=====================
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user