ARCHBOM-1494: Refer to custom attributes, not metrics, especially with edx-django-utils (#25010)

This uses the new names introduced in edx-django-utils
3.8.0 (edx/edx-django-utils#59), which we're already using, as
well as updating a few other locations where we incorrectly refer
to New Relic custom metrics instead of custom attributes.

Includes a couple of unrelated lint fixes in a file I modified.
This commit is contained in:
Tim McCormack
2020-09-18 09:33:50 -04:00
committed by GitHub
parent f037767cb4
commit ba9ee4e151
29 changed files with 124 additions and 115 deletions

View File

@@ -2,7 +2,7 @@
Adapter to isolate django-oauth-toolkit dependencies
"""
from edx_django_utils.monitoring import set_custom_metric
from edx_django_utils.monitoring import set_custom_attribute
from oauth2_provider import models
from openedx.core.djangoapps.oauth_dispatch.models import RestrictedApplication
@@ -123,8 +123,8 @@ class DOTAdapter(object):
filter_set_before_orgs = filter_set.copy()
filter_set.update([org_relation.to_jwt_filter_claim() for org_relation in application.organizations.all()])
set_custom_metric('filter_set_before_orgs', list(filter_set_before_orgs))
set_custom_metric('filter_set_after_orgs', list(filter_set))
set_custom_metric('filter_set_difference', list(filter_set.difference(filter_set_before_orgs)))
set_custom_attribute('filter_set_before_orgs', list(filter_set_before_orgs))
set_custom_attribute('filter_set_after_orgs', list(filter_set))
set_custom_attribute('filter_set_difference', list(filter_set.difference(filter_set_before_orgs)))
return filter_set

View File

@@ -5,7 +5,7 @@ import json
from time import time
from django.conf import settings
from edx_django_utils.monitoring import set_custom_metric
from edx_django_utils.monitoring import set_custom_attribute
from edx_rbac.utils import create_role_auth_claim_for_user
from jwkest import jwk
from jwkest.jws import JWS
@@ -146,7 +146,7 @@ def _compute_time_fields(expires_in):
"""
now = int(time())
expires_in = expires_in or settings.JWT_AUTH['JWT_EXPIRATION']
set_custom_metric('jwt_expires_in', expires_in)
set_custom_attribute('jwt_expires_in', expires_in)
return now, now + expires_in
@@ -195,7 +195,7 @@ def _attach_profile_claim(payload, user):
def _encode_and_sign(payload, use_asymmetric_key, secret):
"""Encode and sign the provided payload."""
set_custom_metric('jwt_is_asymmetric', use_asymmetric_key)
set_custom_attribute('jwt_is_asymmetric', use_asymmetric_key)
keys = jwk.KEYS()
if use_asymmetric_key:

View File

@@ -233,18 +233,18 @@ class TestAccessTokenView(AccessTokenLoginMixin, mixins.AccessTokenMixin, _Dispa
(None, 'no_token_type_supplied'),
)
@ddt.unpack
@patch('edx_django_utils.monitoring.set_custom_metric')
def test_access_token_metrics(self, token_type, expected_token_type, mock_set_custom_metric):
@patch('edx_django_utils.monitoring.set_custom_attribute')
def test_access_token_metrics(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 = [
call('oauth_token_type', expected_token_type),
call('oauth_grant_type', 'password'),
]
mock_set_custom_metric.assert_has_calls(expected_calls, any_order=True)
mock_set_custom_attribute.assert_has_calls(expected_calls, any_order=True)
@patch('edx_django_utils.monitoring.set_custom_metric')
def test_access_token_metrics_for_bad_request(self, mock_set_custom_metric):
@patch('edx_django_utils.monitoring.set_custom_attribute')
def test_access_token_metrics_for_bad_request(self, mock_set_custom_attribute):
grant_type = dot_models.Application.GRANT_PASSWORD
invalid_body = {
'grant_type': grant_type.replace('-', '_'),
@@ -255,7 +255,7 @@ class TestAccessTokenView(AccessTokenLoginMixin, mixins.AccessTokenMixin, _Dispa
call('oauth_token_type', 'no_token_type_supplied'),
call('oauth_grant_type', 'password'),
]
mock_set_custom_metric.assert_has_calls(expected_calls, any_order=True)
mock_set_custom_attribute.assert_has_calls(expected_calls, any_order=True)
def test_restricted_jwt_access_token(self):
"""

View File

@@ -34,7 +34,7 @@ class _DispatchingView(View):
Returns the appropriate adapter based on the OAuth client linked to the request.
"""
client_id = self._get_client_id(request)
monitoring_utils.set_custom_metric('oauth_client_id', client_id)
monitoring_utils.set_custom_attribute('oauth_client_id', client_id)
return self.dot_adapter
@@ -92,8 +92,8 @@ class AccessTokenView(_DispatchingView):
token_type = request.POST.get('token_type',
request.META.get('HTTP_X_TOKEN_TYPE', 'no_token_type_supplied')).lower()
monitoring_utils.set_custom_metric('oauth_token_type', token_type)
monitoring_utils.set_custom_metric('oauth_grant_type', request.POST.get('grant_type', ''))
monitoring_utils.set_custom_attribute('oauth_token_type', token_type)
monitoring_utils.set_custom_attribute('oauth_grant_type', request.POST.get('grant_type', ''))
if response.status_code == 200 and token_type == 'jwt':
response.content = self._build_jwt_response_from_access_token_response(request, response)

View File

@@ -196,7 +196,7 @@ class ScheduleSendEmailTestMixin(FilteredQueryCountMixin):
@ddt.data(1, 10, 100)
@patch.object(tasks, 'ace')
@patch.object(resolvers, 'set_custom_metric')
@patch.object(resolvers, 'set_custom_attribute')
def test_schedule_bin(self, schedule_count, mock_metric, mock_ace):
with patch.object(self.task, 'async_send_task') as mock_schedule_send:
current_day, offset, target_day, upgrade_deadline = self._get_dates()

View File

@@ -12,7 +12,7 @@ from django.db.models import F, Q
from django.urls import reverse
from edx_ace.recipient import Recipient
from edx_ace.recipient_resolver import RecipientResolver
from edx_django_utils.monitoring import function_trace, set_custom_metric
from edx_django_utils.monitoring import function_trace, set_custom_attribute
from edx_when.api import get_schedules_with_due_date
from opaque_keys.edx.keys import CourseKey
@@ -160,7 +160,7 @@ class BinnedSchedulesBaseResolver(PrefixedDebugLoggerMixin, RecipientResolver):
LOG.info(u'Number of schedules = %d', num_schedules)
# This should give us a sense of the volume of data being processed by each task.
set_custom_metric('num_schedules', num_schedules)
set_custom_attribute('num_schedules', num_schedules)
return schedules

View File

@@ -16,7 +16,7 @@ from django.db.utils import DatabaseError
from edx_ace import ace
from edx_ace.message import Message
from edx_ace.utils.date import deserialize, serialize
from edx_django_utils.monitoring import set_custom_metric
from edx_django_utils.monitoring import set_custom_attribute
from eventtracking import tracker
from opaque_keys.edx.keys import CourseKey
@@ -304,28 +304,28 @@ def _is_delivery_enabled(site, delivery_config_var, log_prefix):
def _annotate_for_monitoring(message_type, site, bin_num=None, target_day_str=None, day_offset=None, course_key=None):
# This identifies the type of message being sent, for example: schedules.recurring_nudge3.
set_custom_metric('message_name', '{0}.{1}'.format(message_type.app_label, message_type.name))
set_custom_attribute('message_name', '{0}.{1}'.format(message_type.app_label, message_type.name))
# The domain name of the site we are sending the message for.
set_custom_metric('site', site.domain)
set_custom_attribute('site', site.domain)
# This is the "bin" of data being processed. We divide up the work into chunks so that we don't tie up celery
# workers for too long. This could help us identify particular bins that are problematic.
if bin_num:
set_custom_metric('bin', bin_num)
set_custom_attribute('bin', bin_num)
# The date we are processing data for.
if target_day_str:
set_custom_metric('target_day', target_day_str)
set_custom_attribute('target_day', target_day_str)
# The number of days relative to the current date to process data for.
if day_offset:
set_custom_metric('day_offset', day_offset)
set_custom_attribute('day_offset', day_offset)
# If we're processing these according to a course_key rather than bin we can use this to identify problematic keys.
if course_key:
set_custom_metric('course_key', course_key)
set_custom_attribute('course_key', course_key)
# A unique identifier for this batch of messages being sent.
set_custom_metric('send_uuid', message_type.uuid)
set_custom_attribute('send_uuid', message_type.uuid)
def _annonate_send_task_for_monitoring(msg):
# A unique identifier for this batch of messages being sent.
set_custom_metric('send_uuid', msg.send_uuid)
set_custom_attribute('send_uuid', msg.send_uuid)
# A unique identifier for this particular message.
set_custom_metric('uuid', msg.uuid)
set_custom_attribute('uuid', msg.uuid)

View File

@@ -23,7 +23,7 @@ from django.utils.translation import ugettext as _
from django.views.decorators.csrf import csrf_exempt, csrf_protect, ensure_csrf_cookie
from django.views.decorators.debug import sensitive_post_parameters
from django.views.decorators.http import require_http_methods
from edx_django_utils.monitoring import set_custom_metric
from edx_django_utils.monitoring import set_custom_attribute
from ratelimit.decorators import ratelimit
from ratelimitbackend.exceptions import RateLimitException
from rest_framework.views import APIView
@@ -296,7 +296,7 @@ def _check_user_auth_flow(site, user):
# User has a nonstandard email so we record their id.
# we don't record their e-mail in case there is sensitive info accidentally
# in there.
set_custom_metric('login_tpa_domain_shortcircuit_user_id', user.id)
set_custom_attribute('login_tpa_domain_shortcircuit_user_id', user.id)
log.warn("User %s has nonstandard e-mail. Shortcircuiting THIRD_PART_AUTH_ONLY_DOMAIN check.", user.id)
return
user_domain = email_parts[1].strip().lower()
@@ -399,7 +399,7 @@ def login_user(request):
first_party_auth_requested = bool(request.POST.get('email')) or bool(request.POST.get('password'))
is_user_third_party_authenticated = False
set_custom_metric('login_user_course_id', request.POST.get('course_id'))
set_custom_attribute('login_user_course_id', request.POST.get('course_id'))
try:
if third_party_auth_requested and not first_party_auth_requested:
@@ -413,10 +413,10 @@ def login_user(request):
try:
user = _do_third_party_auth(request)
is_user_third_party_authenticated = True
set_custom_metric('login_user_tpa_success', True)
set_custom_attribute('login_user_tpa_success', True)
except AuthFailedError as e:
set_custom_metric('login_user_tpa_success', False)
set_custom_metric('login_user_tpa_failure_msg', e.value)
set_custom_attribute('login_user_tpa_success', False)
set_custom_attribute('login_user_tpa_failure_msg', e.value)
# user successfully authenticated with a third party provider, but has no linked Open edX account
response_content = e.get_response()
@@ -454,9 +454,9 @@ def login_user(request):
# Ensure that the external marketing site can
# detect that the user is logged in.
response = set_logged_in_cookies(request, response, possibly_authenticated_user)
set_custom_metric('login_user_auth_failed_error', False)
set_custom_metric('login_user_response_status', response.status_code)
set_custom_metric('login_user_redirect_url', redirect_url)
set_custom_attribute('login_user_auth_failed_error', False)
set_custom_attribute('login_user_response_status', response.status_code)
set_custom_attribute('login_user_redirect_url', redirect_url)
return response
except AuthFailedError as error:
response_content = error.get_response()
@@ -465,8 +465,8 @@ def login_user(request):
response_content['email'] = user.email
response = JsonResponse(response_content, status=400)
set_custom_metric('login_user_auth_failed_error', True)
set_custom_metric('login_user_response_status', response.status_code)
set_custom_attribute('login_user_auth_failed_error', True)
set_custom_attribute('login_user_response_status', response.status_code)
return response
@@ -540,12 +540,12 @@ def _parse_analytics_param_for_course_id(request):
# Works for an HttpRequest but not a rest_framework.request.Request.
# Note: This case seems to be used for tests only.
request.POST = modified_request
set_custom_metric('login_user_request_type', 'django')
set_custom_attribute('login_user_request_type', 'django')
else:
# The request must be a rest_framework.request.Request.
# Note: Only DRF seems to be used in Production.
request._data = modified_request # pylint: disable=protected-access
set_custom_metric('login_user_request_type', 'drf')
set_custom_attribute('login_user_request_type', 'drf')
# Include the course ID if it's specified in the analytics info
# so it can be included in analytics events.
@@ -555,7 +555,7 @@ def _parse_analytics_param_for_course_id(request):
if "enroll_course_id" in analytics:
modified_request["course_id"] = analytics.get("enroll_course_id")
except (ValueError, TypeError):
set_custom_metric('shim_analytics_course_id', 'parse-error')
set_custom_attribute('shim_analytics_course_id', 'parse-error')
log.error(
u"Could not parse analytics object sent to user API: {analytics}".format(
analytics=analytics

View File

@@ -63,7 +63,7 @@ from weakref import WeakSet
import crum
import six
from django.conf import settings
from edx_django_utils.monitoring import set_custom_metric
from edx_django_utils.monitoring import set_custom_attribute
from opaque_keys.edx.keys import CourseKey
from waffle import flag_is_active, switch_is_active
@@ -302,7 +302,7 @@ class WaffleFlagNamespace(six.with_metaclass(ABCMeta, WaffleNamespace)):
# 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)
set_custom_metric('warn_flag_no_request_return_value', value)
set_custom_attribute('warn_flag_no_request_return_value', value)
return value
value = flag_is_active(request, namespaced_flag_name)
@@ -367,7 +367,7 @@ class WaffleFlagNamespace(six.with_metaclass(ABCMeta, WaffleNamespace)):
if is_value_change:
metric_name = 'flag_{}'.format(name)
set_custom_metric(metric_name, flag_metric_data[name])
set_custom_attribute(metric_name, flag_metric_data[name])
def _get_waffle_flag_custom_metrics_set():

View File

@@ -47,14 +47,14 @@ class TestCourseWaffleFlag(TestCase):
RequestCache.clear_all_namespaces()
@override_settings(WAFFLE_FLAG_CUSTOM_METRICS=[NAMESPACED_FLAG_NAME])
@patch('openedx.core.djangoapps.waffle_utils.set_custom_metric')
@patch('openedx.core.djangoapps.waffle_utils.set_custom_attribute')
@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, mock_set_custom_metric):
def test_course_waffle_flag(self, data, mock_set_custom_attribute):
"""
Tests various combinations of a flag being set in waffle and overridden
for a course.
@@ -74,8 +74,8 @@ 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()
self._assert_waffle_flag_metric(mock_set_custom_attribute, expected_flag_value=str(data['result']))
mock_set_custom_attribute.reset_mock()
# check flag for a second course
if data['course_override'] == WaffleFlagCourseOverrideModel.ALL_CHOICES.unset:
@@ -90,11 +90,11 @@ 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_metric, expected_flag_value=expected_flag_value)
self._assert_waffle_flag_metric(mock_set_custom_attribute, expected_flag_value=expected_flag_value)
@override_settings(WAFFLE_FLAG_CUSTOM_METRICS=[NAMESPACED_FLAG_NAME])
@patch('openedx.core.djangoapps.waffle_utils.set_custom_metric')
def test_undefined_waffle_flag(self, mock_set_custom_metric):
@patch('openedx.core.djangoapps.waffle_utils.set_custom_attribute')
def test_undefined_waffle_flag(self, mock_set_custom_attribute):
"""
Test flag with undefined waffle flag.
"""
@@ -123,7 +123,7 @@ class TestCourseWaffleFlag(TestCase):
)
self._assert_waffle_flag_metric(
mock_set_custom_metric,
mock_set_custom_attribute,
expected_flag_value=str(False),
)
@@ -157,8 +157,11 @@ class TestCourseWaffleFlag(TestCase):
{'expected_count': 1, 'waffle_flag_metric_setting': [NAMESPACED_FLAG_NAME]},
{'expected_count': 2, 'waffle_flag_metric_setting': [NAMESPACED_FLAG_NAME, NAMESPACED_FLAG_2_NAME]},
)
@patch('openedx.core.djangoapps.waffle_utils.set_custom_metric')
def test_waffle_flag_metric_for_various_settings(self, data, mock_set_custom_metric):
@patch('openedx.core.djangoapps.waffle_utils.set_custom_attribute')
def test_waffle_flag_metric_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 patch(
'openedx.core.djangoapps.waffle_utils._WAFFLE_FLAG_CUSTOM_METRIC_SET',
@@ -169,16 +172,19 @@ class TestCourseWaffleFlag(TestCase):
test_course_flag_2 = CourseWaffleFlag(self.TEST_NAMESPACE, self.FLAG_2_NAME, __name__)
test_course_flag_2.is_enabled(self.TEST_COURSE_KEY)
self.assertEqual(mock_set_custom_metric.call_count, data['expected_count'])
self.assertEqual(mock_set_custom_attribute.call_count, data['expected_count'])
def _assert_waffle_flag_metric(self, mock_set_custom_metric, expected_flag_value=None):
def _assert_waffle_flag_metric(self, mock_set_custom_attribute, expected_flag_value=None):
"""
Assert that a custom attribute was set as expected on the mock.
"""
if 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)
self.assertEqual(mock_set_custom_metric.call_count, 1)
mock_set_custom_attribute.assert_has_calls(expected_calls)
self.assertEqual(mock_set_custom_attribute.call_count, 1)
else:
self.assertEqual(mock_set_custom_metric.call_count, 0)
self.assertEqual(mock_set_custom_attribute.call_count, 0)
class TestWaffleSwitch(TestCase):