Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Rename more "custom metric" references to "custom attribute" (#25018)

Browse Source 
* Rename CookieMetricsMiddleware to CookingMonitoringMiddleware

This fixes a misuse of New Relic terminology. Here we are in fact using
custom attributes; custom metrics are a different thing that we may start
using in the future.

* Rename metric -> attribute in courseware New Relic code

* Fix my weird typo

* Replace `s/metric/attribute/g` in waffle_utils and in caller

This changes:

- `WAFFLE_FLAG_CUSTOM_METRICS`
- `WaffleFlagNamespace._set_waffle_flag_metric`
- `_get_waffle_flag_custom_metrics_set` and some other unreferenced
  internals

* Avoid direct use of newrelic (while we're in there)

* Fix existing typo in line I touched

* A few other "metric" names in files touched in previous renaming PRs

* Make dependency constraint note terse, and add description
This commit is contained in:
Tim McCormack
2020-09-28 11:35:44 -04:00
committed by GitHub
parent 2c15bbba2d 5b331b2d4d
commit ec65018306
10 changed files with 76 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
cms/envs/common.py
View File

@@ -661,7 +661,7 @@ MIDDLEWARE = [
'edx_django_utils.monitoring.middleware.MonitoringMemoryMiddleware',
# Cookie monitoring
'openedx.core.lib.request_utils.CookieMetricsMiddleware',
'openedx.core.lib.request_utils.CookieMonitoringMiddleware',
'openedx.core.djangoapps.header_control.middleware.HeaderControlMiddleware',
'django.middleware.cache.UpdateCacheMiddleware',
@@ -724,7 +724,7 @@ MIDDLEWARE = [
# Enables force_django_cache_miss functionality for TieredCache.
'edx_django_utils.cache.middleware.TieredCacheMiddleware',
# Outputs monitoring metrics for a request.
# Adds monitoring attributes to requests.
'edx_rest_framework_extensions.middleware.RequestCustomAttributesMiddleware',
'edx_rest_framework_extensions.auth.jwt.middleware.EnsureJWTAuthSettingsMiddleware',

22
lms/djangoapps/courseware/user_state_client.py
View File

@@ -106,23 +106,23 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
usage_key = student_module.module_state_key.map_into_course(student_module.course_id)
yield (student_module, usage_key)
def _nr_metric_name(self, function_name, stat_name, block_type=None):
def _nr_attribute_name(self, function_name, stat_name, block_type=None):
"""
Return a metric name (string) representing the provided descriptors.
The return value is directly usable for custom NR metrics.
Return an attribute name (string) representing the provided descriptors.
The return value is directly usable for New Relic custom attributes.
"""
if block_type is None:
metric_name_parts = ['xb_user_state', function_name, stat_name]
attribute_name_parts = ['xb_user_state', function_name, stat_name]
else:
metric_name_parts = ['xb_user_state', function_name, block_type, stat_name]
return '.'.join(metric_name_parts)
attribute_name_parts = ['xb_user_state', function_name, block_type, stat_name]
return '.'.join(attribute_name_parts)
def _nr_stat_accumulate(self, function_name, stat_name, value):
"""
Accumulate arbitrary NR stats (not specific to block types).
"""
monitoring_utils.accumulate(
self._nr_metric_name(function_name, stat_name),
self._nr_attribute_name(function_name, stat_name),
value
)
@@ -137,11 +137,11 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
Accumulate NR stats related to block types.
"""
monitoring_utils.accumulate(
self._nr_metric_name(function_name, stat_name),
self._nr_attribute_name(function_name, stat_name),
value,
)
monitoring_utils.accumulate(
self._nr_metric_name(function_name, stat_name, block_type=block_type),
self._nr_attribute_name(function_name, stat_name, block_type=block_type),
value,
)
@@ -190,7 +190,7 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
if state == {}:
continue
# collect statistics for metric reporting
# collect statistics for custom attribute reporting
self._nr_block_stat_increment('get_many', usage_key.block_type, 'blocks_out')
self._nr_block_stat_accumulate('get_many', usage_key.block_type, 'size', state_length)
total_block_count += 1
@@ -204,7 +204,7 @@ class DjangoXBlockUserStateClient(XBlockUserStateClient):
}
yield XBlockUserState(username, usage_key, state, module.modified, scope)
# The rest of this method exists only to report metrics.
# The rest of this method exists only to report custom attributes.
finish_time = time()
duration = (finish_time - evt_time) * 1000 # milliseconds
self._nr_stat_accumulate('get_many', 'duration', duration)

4
lms/envs/common.py
View File

@@ -1684,7 +1684,7 @@ MIDDLEWARE = [
'edx_django_utils.monitoring.code_owner.middleware.CodeOwnerMonitoringMiddleware',
# Cookie monitoring
'openedx.core.lib.request_utils.CookieMetricsMiddleware',
'openedx.core.lib.request_utils.CookieMonitoringMiddleware',
'mobile_api.middleware.AppVersionUpgrade',
'openedx.core.djangoapps.header_control.middleware.HeaderControlMiddleware',
@@ -1763,7 +1763,7 @@ MIDDLEWARE = [
# Enables force_django_cache_miss functionality for TieredCache.
'edx_django_utils.cache.middleware.TieredCacheMiddleware',
# Outputs monitoring metrics for a request.
# Adds monitoring attributes to requests.
'edx_rest_framework_extensions.middleware.RequestCustomAttributesMiddleware',
'edx_rest_framework_extensions.auth.jwt.middleware.EnsureJWTAuthSettingsMiddleware',

4
openedx/core/djangoapps/oauth_dispatch/tests/test_views.py
View File

@@ -234,7 +234,7 @@ class TestAccessTokenView(AccessTokenLoginMixin, mixins.AccessTokenMixin, _Dispa
)
@ddt.unpack
@patch('edx_django_utils.monitoring.set_custom_attribute')
def test_access_token_metrics(self, token_type, expected_token_type, mock_set_custom_attribute):
def test_access_token_attributes(self, token_type, expected_token_type, mock_set_custom_attribute):
response = self._post_request(self.user, self.dot_app, token_type=token_type)
self.assertEqual(response.status_code, 200)
expected_calls = [
@@ -244,7 +244,7 @@ class TestAccessTokenView(AccessTokenLoginMixin, mixins.AccessTokenMixin, _Dispa
mock_set_custom_attribute.assert_has_calls(expected_calls, any_order=True)
@patch('edx_django_utils.monitoring.set_custom_attribute')
def test_access_token_metrics_for_bad_request(self, mock_set_custom_attribute):
def test_access_token_attributes_for_bad_request(self, mock_set_custom_attribute):
grant_type = dot_models.Application.GRANT_PASSWORD
invalid_body = {
'grant_type': grant_type.replace('-', '_'),

4
openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py
View File

@@ -197,7 +197,7 @@ class ScheduleSendEmailTestMixin(FilteredQueryCountMixin):
@ddt.data(1, 10, 100)
@patch.object(tasks, 'ace')
@patch.object(resolvers, 'set_custom_attribute')
def test_schedule_bin(self, schedule_count, mock_metric, mock_ace):
def test_schedule_bin(self, schedule_count, mock_attribute, mock_ace):
with patch.object(self.task, 'async_send_task') as mock_schedule_send:
current_day, offset, target_day, upgrade_deadline = self._get_dates()
schedules = [
@@ -226,7 +226,7 @@ class ScheduleSendEmailTestMixin(FilteredQueryCountMixin):
site_id=self.site_config.site.id, target_day_str=target_day_str, day_offset=offset, bin_num=b,
))
num_schedules = mock_metric.call_args[0][1]
num_schedules = mock_attribute.call_args[0][1]
if b in bins_in_use:
self.assertGreater(num_schedules, 0)
else:

58
openedx/core/djangoapps/waffle_utils/__init__.py
View File

@@ -48,7 +48,7 @@ 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
Also see ``WAFFLE_FLAG_CUSTOM_METRICS`` and docstring for _set_waffle_flag_metric
Also see ``WAFFLE_FLAG_CUSTOM_ATTRIBUTES`` and docstring for _set_waffle_flag_attribute
for temporarily instrumenting/monitoring waffle flag usage.
"""
@@ -285,12 +285,12 @@ class WaffleFlagNamespace(six.with_metaclass(ABCMeta, WaffleNamespace)):
if value is not None:
# Do not cache value for the callback, because the key might be different.
# The callback needs to handle its own caching if it wants it.
self._set_waffle_flag_metric(namespaced_flag_name, value)
self._set_waffle_flag_attribute(namespaced_flag_name, value)
return value
value = self._cached_flags.get(namespaced_flag_name)
if value is not None:
self._set_waffle_flag_metric(namespaced_flag_name, value)
self._set_waffle_flag_attribute(namespaced_flag_name, value)
return value
request = crum.get_current_request()
@@ -301,14 +301,14 @@ class WaffleFlagNamespace(six.with_metaclass(ABCMeta, WaffleNamespace)):
# in a normal request context. This case seems to occur when
# a page redirects to a 404, or for celery workers.
value = self._is_flag_active_for_everyone(namespaced_flag_name)
self._set_waffle_flag_metric(namespaced_flag_name, value)
self._set_waffle_flag_attribute(namespaced_flag_name, value)
set_custom_attribute('warn_flag_no_request_return_value', value)
return value
value = flag_is_active(request, namespaced_flag_name)
self._cached_flags[namespaced_flag_name] = value
self._set_waffle_flag_metric(namespaced_flag_name, value)
self._set_waffle_flag_attribute(namespaced_flag_name, value)
return value
def _is_flag_active_for_everyone(self, namespaced_flag_name):
@@ -325,14 +325,14 @@ class WaffleFlagNamespace(six.with_metaclass(ABCMeta, WaffleNamespace)):
except Flag.DoesNotExist:
return False
def _set_waffle_flag_metric(self, name, value):
def _set_waffle_flag_attribute(self, name, value):
"""
For any flag name in _WAFFLE_FLAG_CUSTOM_METRIC_SET, add name/value
to cached values and set custom metric if the value changed.
For any flag name in _WAFFLE_FLAG_CUSTOM_ATTRIBUTE_SET, add name/value
to cached values and set custom attribute if the value changed.
The name of the custom metric will have the prefix ``flag_`` and the
The name of the custom attribute 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 of the custom attribute could be False, True, or Both.
The value Both would mean that the flag had both a True and False
value at different times during the transaction. This is most
@@ -347,49 +347,49 @@ class WaffleFlagNamespace(six.with_metaclass(ABCMeta, WaffleNamespace)):
WHERE flag_my.waffle.flag IS NOT NULL
FACET appName, flag_my.waffle.flag
Important: Remember to configure ``WAFFLE_FLAG_CUSTOM_METRICS`` for
Important: Remember to configure ``WAFFLE_FLAG_CUSTOM_ATTRIBUTES`` 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:
if name not in _WAFFLE_FLAG_CUSTOM_ATTRIBUTE_SET:
return
flag_metric_data = self._get_request_cache().setdefault('flag_metric', {})
flag_attribute_data = self._get_request_cache().setdefault('flag_attribute', {})
is_value_change = False
if name not in flag_metric_data:
flag_metric_data[name] = str(value)
if name not in flag_attribute_data:
flag_attribute_data[name] = str(value)
is_value_change = True
else:
if flag_metric_data[name] != str(value):
flag_metric_data[name] = 'Both'
if flag_attribute_data[name] != str(value):
flag_attribute_data[name] = 'Both'
is_value_change = True
if is_value_change:
metric_name = 'flag_{}'.format(name)
set_custom_attribute(metric_name, flag_metric_data[name])
attribute_name = 'flag_{}'.format(name)
set_custom_attribute(attribute_name, flag_attribute_data[name])
def _get_waffle_flag_custom_metrics_set():
def _get_waffle_flag_custom_attributes_set():
"""
Returns a set based on the Django setting WAFFLE_FLAG_CUSTOM_METRICS (list).
Returns a set based on the Django setting WAFFLE_FLAG_CUSTOM_ATTRIBUTES (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)
waffle_flag_custom_attributes = getattr(settings, _WAFFLE_FLAG_CUSTOM_ATTRIBUTES, None)
waffle_flag_custom_attributes = waffle_flag_custom_attributes if waffle_flag_custom_attributes else []
return set(waffle_flag_custom_attributes)
# .. toggle_name: WAFFLE_FLAG_CUSTOM_METRICS
# .. toggle_name: WAFFLE_FLAG_CUSTOM_ATTRIBUTES
# .. toggle_implementation: DjangoSetting
# .. toggle_default: False
# .. toggle_description: A list of waffle flag to track with custom metrics having
# .. toggle_description: A list of waffle flags to track with custom attributes having
# values of (True, False, or Both).
# .. toggle_use_cases: opt_in
# .. toggle_creation_date: 2020-06-17
# .. toggle_warnings: Intent is for temporary research (1 day - several weeks) of a flag's usage.
_WAFFLE_FLAG_CUSTOM_METRICS = 'WAFFLE_FLAG_CUSTOM_METRICS'
_WAFFLE_FLAG_CUSTOM_ATTRIBUTES = 'WAFFLE_FLAG_CUSTOM_ATTRIBUTES'
# set of waffle flags that should be instrumented with custom metrics
_WAFFLE_FLAG_CUSTOM_METRIC_SET = _get_waffle_flag_custom_metrics_set()
# set of waffle flags that should be instrumented with custom attributes
_WAFFLE_FLAG_CUSTOM_ATTRIBUTE_SET = _get_waffle_flag_custom_attributes_set()
class WaffleFlag(object):

36
openedx/core/djangoapps/waffle_utils/tests/test_init.py
View File

@@ -13,7 +13,7 @@ from opaque_keys.edx.keys import CourseKey
from waffle.testutils import override_flag
from .. import (
_get_waffle_flag_custom_metrics_set,
_get_waffle_flag_custom_attributes_set,
CourseWaffleFlag,
WaffleFlagNamespace,
WaffleSwitchNamespace,
@@ -46,7 +46,7 @@ class TestCourseWaffleFlag(TestCase):
crum.set_current_request(request)
RequestCache.clear_all_namespaces()
@override_settings(WAFFLE_FLAG_CUSTOM_METRICS=[NAMESPACED_FLAG_NAME])
@override_settings(WAFFLE_FLAG_CUSTOM_ATTRIBUTES=[NAMESPACED_FLAG_NAME])
@patch('openedx.core.djangoapps.waffle_utils.set_custom_attribute')
@ddt.data(
{'course_override': WaffleFlagCourseOverrideModel.ALL_CHOICES.on, 'waffle_enabled': False, 'result': True},
@@ -60,8 +60,8 @@ class TestCourseWaffleFlag(TestCase):
for a course.
"""
with patch(
'openedx.core.djangoapps.waffle_utils._WAFFLE_FLAG_CUSTOM_METRIC_SET',
_get_waffle_flag_custom_metrics_set(),
'openedx.core.djangoapps.waffle_utils._WAFFLE_FLAG_CUSTOM_ATTRIBUTE_SET',
_get_waffle_flag_custom_attributes_set(),
):
with patch.object(WaffleFlagCourseOverrideModel, 'override_value', return_value=data['course_override']):
with override_flag(self.NAMESPACED_FLAG_NAME, active=data['waffle_enabled']):
@@ -74,7 +74,7 @@ class TestCourseWaffleFlag(TestCase):
self.TEST_COURSE_KEY
)
self._assert_waffle_flag_metric(mock_set_custom_attribute, expected_flag_value=str(data['result']))
self._assert_waffle_flag_attribute(mock_set_custom_attribute, expected_flag_value=str(data['result']))
mock_set_custom_attribute.reset_mock()
# check flag for a second course
@@ -90,9 +90,9 @@ class TestCourseWaffleFlag(TestCase):
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_attribute, expected_flag_value=expected_flag_value)
self._assert_waffle_flag_attribute(mock_set_custom_attribute, expected_flag_value=expected_flag_value)
@override_settings(WAFFLE_FLAG_CUSTOM_METRICS=[NAMESPACED_FLAG_NAME])
@override_settings(WAFFLE_FLAG_CUSTOM_ATTRIBUTES=[NAMESPACED_FLAG_NAME])
@patch('openedx.core.djangoapps.waffle_utils.set_custom_attribute')
def test_undefined_waffle_flag(self, mock_set_custom_attribute):
"""
@@ -105,8 +105,8 @@ class TestCourseWaffleFlag(TestCase):
)
with patch(
'openedx.core.djangoapps.waffle_utils._WAFFLE_FLAG_CUSTOM_METRIC_SET',
_get_waffle_flag_custom_metrics_set(),
'openedx.core.djangoapps.waffle_utils._WAFFLE_FLAG_CUSTOM_ATTRIBUTE_SET',
_get_waffle_flag_custom_attributes_set(),
):
with patch.object(
WaffleFlagCourseOverrideModel,
@@ -122,7 +122,7 @@ class TestCourseWaffleFlag(TestCase):
self.TEST_COURSE_KEY
)
self._assert_waffle_flag_metric(
self._assert_waffle_flag_attribute(
mock_set_custom_attribute,
expected_flag_value=str(False),
)
@@ -153,19 +153,19 @@ class TestCourseWaffleFlag(TestCase):
self.assertEqual(test_course_flag.is_enabled(self.TEST_COURSE_KEY), True)
@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]},
{'expected_count': 0, 'waffle_flag_attribute_setting': None},
{'expected_count': 1, 'waffle_flag_attribute_setting': [NAMESPACED_FLAG_NAME]},
{'expected_count': 2, 'waffle_flag_attribute_setting': [NAMESPACED_FLAG_NAME, NAMESPACED_FLAG_2_NAME]},
)
@patch('openedx.core.djangoapps.waffle_utils.set_custom_attribute')
def test_waffle_flag_metric_for_various_settings(self, data, mock_set_custom_attribute):
def test_waffle_flag_attribute_for_various_settings(self, data, mock_set_custom_attribute):
"""
Test that custom attributes are recorded when waffle flag accessed.
"""
with override_settings(WAFFLE_FLAG_CUSTOM_METRICS=data['waffle_flag_metric_setting']):
with override_settings(WAFFLE_FLAG_CUSTOM_ATTRIBUTES=data['waffle_flag_attribute_setting']):
with patch(
'openedx.core.djangoapps.waffle_utils._WAFFLE_FLAG_CUSTOM_METRIC_SET',
_get_waffle_flag_custom_metrics_set(),
'openedx.core.djangoapps.waffle_utils._WAFFLE_FLAG_CUSTOM_ATTRIBUTE_SET',
_get_waffle_flag_custom_attributes_set(),
):
test_course_flag = CourseWaffleFlag(self.TEST_NAMESPACE, self.FLAG_NAME, __name__)
test_course_flag.is_enabled(self.TEST_COURSE_KEY)
@@ -174,7 +174,7 @@ class TestCourseWaffleFlag(TestCase):
self.assertEqual(mock_set_custom_attribute.call_count, data['expected_count'])
def _assert_waffle_flag_metric(self, mock_set_custom_attribute, expected_flag_value=None):
def _assert_waffle_flag_attribute(self, mock_set_custom_attribute, expected_flag_value=None):
"""
Assert that a custom attribute was set as expected on the mock.
"""

21
openedx/core/lib/request_utils.py
View File

@@ -7,6 +7,7 @@ import crum
from django.conf import settings
from django.test.client import RequestFactory
from django.utils.deprecation import MiddlewareMixin
from edx_django_utils.monitoring import set_custom_attribute
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey
from six.moves.urllib.parse import urlparse
@@ -14,11 +15,6 @@ from six.moves.urllib.parse import urlparse
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
from openedx.core.djangoapps.waffle_utils import WaffleFlag, WaffleFlagNamespace
try:
import newrelic.agent
except ImportError:
newrelic = None # pylint: disable=invalid-name
# accommodates course api urls, excluding any course api routes that do not fall under v*/courses, such as v1/blocks.
COURSE_REGEX = re.compile(r'^(.*?/courses/)(?!v[0-9]+/[^/]+){}'.format(settings.COURSE_ID_PATTERN))
@@ -95,21 +91,18 @@ def course_id_from_url(url):
return None
class CookieMetricsMiddleware(MiddlewareMixin):
class CookieMonitoringMiddleware(MiddlewareMixin):
"""
Middleware for monitoring the size and growth of all our cookies, to see if
we're running into browser limits.
"""
def process_request(self, request):
"""
Emit custom metrics for cookie size values for every cookie we have.
Emit custom attributes for cookie size values for every cookie we have.
Don't log contents of cookies because that might cause a security issue.
We just want to see if any cookies are growing out of control.
"""
if not newrelic:
return
if not CAPTURE_COOKIE_SIZES.is_enabled():
return
@@ -118,10 +111,10 @@ class CookieMetricsMiddleware(MiddlewareMixin):
for name, value in request.COOKIES.items()
}
for name, size in cookie_names_to_size.items():
metric_name = 'cookies.{}.size'.format(name)
newrelic.agent.add_custom_parameter(metric_name, size)
log.debug(u'%s = %d', metric_name, size)
attribute_name = 'cookies.{}.size'.format(name)
set_custom_attribute(attribute_name, size)
log.debug(u'%s = %d', attribute_name, size)
total_cookie_size = sum(cookie_names_to_size.values())
newrelic.agent.add_custom_parameter('cookies_total_size', total_cookie_size)
set_custom_attribute('cookies_total_size', total_cookie_size)
log.debug(u'cookies_total_size = %d', total_cookie_size)

4
openedx/features/course_experience/__init__.py
View File

@@ -58,12 +58,12 @@ class DefaultTrueWaffleFlagNamespace(WaffleFlagNamespace):
set_custom_attribute('warn_flag_no_request', True)
# Return the default value if not in a request context.
# Same as the original implementation
self._set_waffle_flag_metric(namespaced_flag_name, value)
self._set_waffle_flag_attribute(namespaced_flag_name, value)
return True
self._cached_flags[namespaced_flag_name] = value
self._set_waffle_flag_metric(namespaced_flag_name, value)
self._set_waffle_flag_attribute(namespaced_flag_name, value)
return value

4
requirements/edx/base.in
View File

@@ -78,9 +78,7 @@ edx-celeryutils
edx-completion
edx-django-release-util # Release utils for the edx release pipeline
edx-django-sites-extensions
# edx-django-utils 3.8.0 renames metric -> attribute and deprecates the old
# methods; edx-platform calls the new names
edx-django-utils>=3.8.0
edx-django-utils>=3.8.0 # Utilities for cache, monitoring, and plugins; 3.8.0+ for set_custom_attribute method
edx-drf-extensions
edx-enterprise
edx-milestones