From ba9ee4e15197dc4c81108c9156aab83fdd84cc61 Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Fri, 18 Sep 2020 09:33:50 -0400 Subject: [PATCH] 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. --- cms/templates/widgets/user_dropdown.html | 2 +- common/djangoapps/edxmako/shortcuts.py | 4 +-- common/djangoapps/student/views/management.py | 8 ++--- .../third_party_auth/api/permissions.py | 2 +- .../course_home_api/dates/v1/views.py | 6 ++-- .../course_home_api/outline/v1/views.py | 6 ++-- .../course_home_api/progress/v1/views.py | 6 ++-- lms/djangoapps/courseware/module_render.py | 4 +-- lms/djangoapps/courseware/tests/test_views.py | 6 ++-- lms/djangoapps/courseware/views/index.py | 4 +-- lms/djangoapps/courseware/views/views.py | 10 +++--- lms/djangoapps/grades/tasks.py | 6 ++-- lms/djangoapps/monitoring/README.rst | 7 ++-- .../0001-monitoring-by-code-owner.rst | 10 +++--- lms/envs/common.py | 6 ++-- .../djangoapps/oauth_dispatch/adapters/dot.py | 8 ++--- openedx/core/djangoapps/oauth_dispatch/jwt.py | 6 ++-- .../oauth_dispatch/tests/test_views.py | 12 +++---- .../core/djangoapps/oauth_dispatch/views.py | 6 ++-- .../commands/tests/send_email_base.py | 2 +- .../core/djangoapps/schedules/resolvers.py | 4 +-- openedx/core/djangoapps/schedules/tasks.py | 20 +++++------ .../core/djangoapps/user_authn/views/login.py | 28 +++++++-------- .../core/djangoapps/waffle_utils/__init__.py | 6 ++-- .../waffle_utils/tests/test_init.py | 36 +++++++++++-------- openedx/core/lib/api/authentication.py | 12 +++---- openedx/core/lib/api/permissions.py | 4 +-- .../features/course_experience/__init__.py | 4 +-- requirements/edx/base.in | 4 ++- 29 files changed, 124 insertions(+), 115 deletions(-) diff --git a/cms/templates/widgets/user_dropdown.html b/cms/templates/widgets/user_dropdown.html index 5129cd19cf..88d03079c4 100644 --- a/cms/templates/widgets/user_dropdown.html +++ b/cms/templates/widgets/user_dropdown.html @@ -4,7 +4,7 @@ from django.conf import settings from django.urls import reverse from django.utils.translation import ugettext as _ - from edx_django_utils.monitoring import set_custom_metric + from edx_django_utils.monitoring import set_custom_attribute from student.roles import GlobalStaff %> diff --git a/common/djangoapps/edxmako/shortcuts.py b/common/djangoapps/edxmako/shortcuts.py index 8c37739594..61aec6a30a 100644 --- a/common/djangoapps/edxmako/shortcuts.py +++ b/common/djangoapps/edxmako/shortcuts.py @@ -24,7 +24,7 @@ from six.moves.urllib.parse import urljoin from django.core.validators import URLValidator from django.core.exceptions import ValidationError -from edx_django_utils.monitoring import set_custom_metric +from edx_django_utils.monitoring import set_custom_attribute from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.theming.helpers import is_request_in_themed_site from xmodule.util.xmodule_django import get_current_request_hostname @@ -95,7 +95,7 @@ def marketing_link(name): return reverse(link_map[name]) except NoReverseMatch: log.debug(u"Cannot find corresponding link for name: %s", name) - set_custom_metric('unresolved_marketing_link', name) + set_custom_attribute('unresolved_marketing_link', name) return '#' else: log.debug(u"Cannot find corresponding link for name: %s", name) diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 24f4bb1a33..443c928035 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -314,7 +314,7 @@ def change_enrollment(request, check_access=True): # Allow us to monitor performance of this transaction on a per-course basis since we often roll-out features # on a per-course basis. - monitoring_utils.set_custom_metric('course_id', text_type(course_id)) + monitoring_utils.set_custom_attribute('course_id', text_type(course_id)) if action == "enroll": # Make sure the course exists @@ -486,12 +486,12 @@ def activate_account(request, key): """ # If request is in Studio call the appropriate view if theming_helpers.get_project_root_name().lower() == u'cms': - monitoring_utils.set_custom_metric('student_activate_account', 'cms') + monitoring_utils.set_custom_attribute('student_activate_account', 'cms') return activate_account_studio(request, key) - # TODO: Use metric to determine if there are any `activate_account` calls for cms in Production. + # TODO: Use custom attribute to determine if there are any `activate_account` calls for cms in Production. # If not, the templates wouldn't be needed for cms, but we still need a way to activate for cms tests. - monitoring_utils.set_custom_metric('student_activate_account', 'lms') + monitoring_utils.set_custom_attribute('student_activate_account', 'lms') try: registration = Registration.objects.get(activation_key=key) except (Registration.DoesNotExist, Registration.MultipleObjectsReturned): diff --git a/common/djangoapps/third_party_auth/api/permissions.py b/common/djangoapps/third_party_auth/api/permissions.py index 94da048a6a..b9b2423386 100644 --- a/common/djangoapps/third_party_auth/api/permissions.py +++ b/common/djangoapps/third_party_auth/api/permissions.py @@ -49,7 +49,7 @@ class JwtHasTpaProviderFilterForRequestedProvider(BasePermission): return False -# TODO: Remove ApiKeyHeaderPermission. Check deprecated_api_key_header custom metric for active usage. +# TODO: Remove ApiKeyHeaderPermission. Check deprecated_api_key_header custom attribute for active usage. _NOT_JWT_RESTRICTED_TPA_PERMISSIONS = ( C(NotJwtRestrictedApplication) & (C(IsSuperuser) | ApiKeyHeaderPermission | C(IsStaff)) diff --git a/lms/djangoapps/course_home_api/dates/v1/views.py b/lms/djangoapps/course_home_api/dates/v1/views.py index ec1e77cf16..b9cf8cd080 100644 --- a/lms/djangoapps/course_home_api/dates/v1/views.py +++ b/lms/djangoapps/course_home_api/dates/v1/views.py @@ -77,9 +77,9 @@ class DatesTabView(RetrieveAPIView): raise Http404 # Enable NR tracing for this view based on course - monitoring_utils.set_custom_metric('course_id', course_key_string) - monitoring_utils.set_custom_metric('user_id', request.user.id) - monitoring_utils.set_custom_metric('is_staff', request.user.is_staff) + monitoring_utils.set_custom_attribute('course_id', course_key_string) + monitoring_utils.set_custom_attribute('user_id', request.user.id) + monitoring_utils.set_custom_attribute('is_staff', request.user.is_staff) course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=False) diff --git a/lms/djangoapps/course_home_api/outline/v1/views.py b/lms/djangoapps/course_home_api/outline/v1/views.py index eef2b3d98c..6811428ce7 100644 --- a/lms/djangoapps/course_home_api/outline/v1/views.py +++ b/lms/djangoapps/course_home_api/outline/v1/views.py @@ -133,9 +133,9 @@ class OutlineTabView(RetrieveAPIView): raise Http404 # Enable NR tracing for this view based on course - monitoring_utils.set_custom_metric('course_id', course_key_string) - monitoring_utils.set_custom_metric('user_id', request.user.id) - monitoring_utils.set_custom_metric('is_staff', request.user.is_staff) + monitoring_utils.set_custom_attribute('course_id', course_key_string) + monitoring_utils.set_custom_attribute('user_id', request.user.id) + monitoring_utils.set_custom_attribute('is_staff', request.user.is_staff) course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=False) diff --git a/lms/djangoapps/course_home_api/progress/v1/views.py b/lms/djangoapps/course_home_api/progress/v1/views.py index 6aa5fdaadb..e90e62a67b 100644 --- a/lms/djangoapps/course_home_api/progress/v1/views.py +++ b/lms/djangoapps/course_home_api/progress/v1/views.py @@ -98,9 +98,9 @@ class ProgressTabView(RetrieveAPIView): course_key = CourseKey.from_string(course_key_string) # Enable NR tracing for this view based on course - monitoring_utils.set_custom_metric('course_id', course_key_string) - monitoring_utils.set_custom_metric('user_id', request.user.id) - monitoring_utils.set_custom_metric('is_staff', request.user.is_staff) + monitoring_utils.set_custom_attribute('course_id', course_key_string) + monitoring_utils.set_custom_attribute('user_id', request.user.id) + monitoring_utils.set_custom_attribute('is_staff', request.user.is_staff) _, request.user = setup_masquerade( request, diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 5695750169..56bdccb55d 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -25,7 +25,7 @@ from django.utils.text import slugify from django.views.decorators.clickjacking import xframe_options_exempt from django.views.decorators.csrf import csrf_exempt from edx_django_utils.cache import RequestCache -from edx_django_utils.monitoring import set_custom_metrics_for_course_key, set_monitoring_transaction_name +from edx_django_utils.monitoring import set_custom_attributes_for_course_key, set_monitoring_transaction_name from edx_proctoring.api import get_attempt_status_summary from edx_proctoring.services import ProctoringService from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication @@ -1159,7 +1159,7 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, course except InvalidKeyError: raise Http404 - set_custom_metrics_for_course_key(course_key) + set_custom_attributes_for_course_key(course_key) with modulestore().bulk_operations(course_key): try: diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index cde89978b5..39713c9579 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -3120,8 +3120,8 @@ class DatesTabTestCase(ModuleStoreTestCase): self.assertEqual(response.status_code, 200) @RELATIVE_DATES_FLAG.override(active=True) - @patch('edx_django_utils.monitoring.set_custom_metric') - def test_defaults(self, mock_set_custom_metric): + @patch('edx_django_utils.monitoring.set_custom_attribute') + def test_defaults(self, mock_set_custom_attribute): enrollment = CourseEnrollmentFactory(course_id=self.course.id, user=self.user, mode=CourseMode.VERIFIED) now = datetime.now(utc) with self.store.bulk_operations(self.course.id): @@ -3169,7 +3169,7 @@ class DatesTabTestCase(ModuleStoreTestCase): response = self._get_response(self.course) - mock_set_custom_metric.assert_has_calls(expected_calls, any_order=True) + mock_set_custom_attribute.assert_has_calls(expected_calls, any_order=True) self.assertContains(response, subsection.display_name) # Don't show the Verification Deadline for audit self.assertNotContains(response, 'Verification Deadline') diff --git a/lms/djangoapps/courseware/views/index.py b/lms/djangoapps/courseware/views/index.py index 5dc879adf0..63de82313e 100644 --- a/lms/djangoapps/courseware/views/index.py +++ b/lms/djangoapps/courseware/views/index.py @@ -21,7 +21,7 @@ from django.utils.translation import ugettext as _ from django.views.decorators.cache import cache_control from django.views.decorators.csrf import ensure_csrf_cookie from django.views.generic import View -from edx_django_utils.monitoring import set_custom_metrics_for_course_key +from edx_django_utils.monitoring import set_custom_attributes_for_course_key from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey from web_fragments.fragment import Fragment @@ -134,7 +134,7 @@ class CoursewareIndex(View): self.url = request.path try: - set_custom_metrics_for_course_key(self.course_key) + set_custom_attributes_for_course_key(self.course_key) self._clean_position() with modulestore().bulk_operations(self.course_key): diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index ae867e3d1b..5362fe5178 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -33,7 +33,7 @@ from django.views.decorators.csrf import ensure_csrf_cookie from django.views.decorators.http import require_GET, require_http_methods, require_POST from django.views.generic import View from edx_django_utils import monitoring as monitoring_utils -from edx_django_utils.monitoring import set_custom_metrics_for_course_key +from edx_django_utils.monitoring import set_custom_attributes_for_course_key from ipware.ip import get_ip from markupsafe import escape from opaque_keys import InvalidKeyError @@ -604,7 +604,7 @@ class CourseTabView(EdxFragmentView): # Must come after masquerading on creation of page context self.register_user_access_warning_messages(request, course) - set_custom_metrics_for_course_key(course_key) + set_custom_attributes_for_course_key(course_key) return super(CourseTabView, self).get(request, course=course, page_context=page_context, **kwargs) except Exception as exception: # pylint: disable=broad-except return CourseTabView.handle_exceptions(request, course_key, course, exception) @@ -1017,9 +1017,9 @@ def dates(request, course_id): course_key = CourseKey.from_string(course_id) # Enable NR tracing for this view based on course - monitoring_utils.set_custom_metric('course_id', text_type(course_key)) - monitoring_utils.set_custom_metric('user_id', request.user.id) - monitoring_utils.set_custom_metric('is_staff', request.user.is_staff) + monitoring_utils.set_custom_attribute('course_id', text_type(course_key)) + monitoring_utils.set_custom_attribute('user_id', request.user.id) + monitoring_utils.set_custom_attribute('is_staff', request.user.is_staff) course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=False) diff --git a/lms/djangoapps/grades/tasks.py b/lms/djangoapps/grades/tasks.py index 530e1851fe..cbd517f9c1 100644 --- a/lms/djangoapps/grades/tasks.py +++ b/lms/djangoapps/grades/tasks.py @@ -12,7 +12,7 @@ from django.conf import settings from django.contrib.auth.models import User from django.core.exceptions import ValidationError from django.db.utils import DatabaseError -from edx_django_utils.monitoring import set_custom_metric, set_custom_metrics_for_course_key +from edx_django_utils.monitoring import set_custom_attribute, set_custom_attributes_for_course_key from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys.edx.locator import CourseLocator from submissions import api as sub_api @@ -213,8 +213,8 @@ def _recalculate_subsection_grade(self, **kwargs): scored_block_usage_key = UsageKey.from_string(kwargs['usage_id']).replace(course_key=course_key) - set_custom_metrics_for_course_key(course_key) - set_custom_metric('usage_id', six.text_type(scored_block_usage_key)) + set_custom_attributes_for_course_key(course_key) + set_custom_attribute('usage_id', six.text_type(scored_block_usage_key)) # The request cache is not maintained on celery workers, # where this code runs. So we take the values from the diff --git a/lms/djangoapps/monitoring/README.rst b/lms/djangoapps/monitoring/README.rst index e8936c5571..a6909b0847 100644 --- a/lms/djangoapps/monitoring/README.rst +++ b/lms/djangoapps/monitoring/README.rst @@ -1,9 +1,10 @@ -This directory contains utilities for adding a code_owner custom metric for help with split-ownership of the LMS. +This directory contains utilities for adding a code_owner custom attribute for help with split-ownership of the LMS. -For details on the decision to implement the code_owner custom metric, see: +For details on the decision to implement the code_owner custom attribute, see: lms/djangoapps/monitoring/docs/decisions/0001-monitoring-by-code-owner.rst -Originally, this directory contained the ``CodeOwnerMetricMiddleware``, but that has since been moved to: +Originally, this directory contained the ``CodeOwnerMetricMiddleware``, but that has since been moved to https://github.com/edx/edx-django-utils/tree/master/edx_django_utils/monitoring/code_owner +and renamed ``CodeOwnerMonitoringMiddleware``. This directory continues to contain scripts that can help generate the appropriate ownership mappings for the LMS. diff --git a/lms/djangoapps/monitoring/docs/decisions/0001-monitoring-by-code-owner.rst b/lms/djangoapps/monitoring/docs/decisions/0001-monitoring-by-code-owner.rst index fefb466403..6425e8e12a 100644 --- a/lms/djangoapps/monitoring/docs/decisions/0001-monitoring-by-code-owner.rst +++ b/lms/djangoapps/monitoring/docs/decisions/0001-monitoring-by-code-owner.rst @@ -14,11 +14,11 @@ It is currently difficult for different teams to have team-based on-calls rotati Decision ======== -We will implement a custom metric "code_owner" that can be used in NewRelic (or other monitoring solutions that are made pluggable). +We will implement a custom attribute "code_owner" that can be used in NewRelic (or other monitoring solutions that are made pluggable). -The new custom metric makes it simple to query NewRelic for all Transactions or TransactionErrors that are associated with requests with a specific owner. This enables a team to quickly identify data that they own, for use in NewRelic alerts or NewRelic dashboards. +The new custom attribute makes it simple to query NewRelic for all Transactions or TransactionErrors that are associated with requests with a specific owner. This enables a team to quickly identify data that they own, for use in NewRelic alerts or NewRelic dashboards. -To minimize maintenance, the value of the "code_owner" metric will be populated using the source-of-truth of ownership of various parts of edx-platform. +To minimize maintenance, the value of the "code_owner" attribute will be populated using the source-of-truth of ownership of various parts of edx-platform. See `Rejected Alternatives`_ for details of the decision **not** to split the NewRelic application into multiple NewRelic applications. @@ -27,9 +27,9 @@ Note: "owner" is a MySql reserved word, which NewRelic cautions against using, s Consequences ============ -This metric should be quickly available for use with custom alerts and custom dashboards. +This attribute should be quickly available for use with custom alerts and custom dashboards. -In the future, this metric could potentially be added to logging as well. +In the future, this attribute could potentially be added to logging as well. Rejected Alternatives ===================== diff --git a/lms/envs/common.py b/lms/envs/common.py index 27c755246d..5f2d723cc2 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1677,10 +1677,10 @@ MIDDLEWARE = [ # A newer and safer request cache. 'edx_django_utils.cache.middleware.RequestCacheMiddleware', - 'edx_django_utils.monitoring.middleware.MonitoringCustomMetricsMiddleware', + 'edx_django_utils.monitoring.middleware.CachedCustomMonitoringMiddleware', - # Generate code ownership metrics. Keep this immediately after RequestCacheMiddleware. - 'edx_django_utils.monitoring.code_owner.middleware.CodeOwnerMetricMiddleware', + # Generate code ownership attributes. Keep this immediately after RequestCacheMiddleware. + 'edx_django_utils.monitoring.code_owner.middleware.CodeOwnerMonitoringMiddleware', # Cookie monitoring 'openedx.core.lib.request_utils.CookieMetricsMiddleware', diff --git a/openedx/core/djangoapps/oauth_dispatch/adapters/dot.py b/openedx/core/djangoapps/oauth_dispatch/adapters/dot.py index bd6dd41e24..5d513585cb 100644 --- a/openedx/core/djangoapps/oauth_dispatch/adapters/dot.py +++ b/openedx/core/djangoapps/oauth_dispatch/adapters/dot.py @@ -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 diff --git a/openedx/core/djangoapps/oauth_dispatch/jwt.py b/openedx/core/djangoapps/oauth_dispatch/jwt.py index ae3f98ffdc..6a1be45f0b 100644 --- a/openedx/core/djangoapps/oauth_dispatch/jwt.py +++ b/openedx/core/djangoapps/oauth_dispatch/jwt.py @@ -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: diff --git a/openedx/core/djangoapps/oauth_dispatch/tests/test_views.py b/openedx/core/djangoapps/oauth_dispatch/tests/test_views.py index 4b3323fd9e..38946d2a8c 100644 --- a/openedx/core/djangoapps/oauth_dispatch/tests/test_views.py +++ b/openedx/core/djangoapps/oauth_dispatch/tests/test_views.py @@ -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): """ diff --git a/openedx/core/djangoapps/oauth_dispatch/views.py b/openedx/core/djangoapps/oauth_dispatch/views.py index 15120c4e35..834bccf55a 100644 --- a/openedx/core/djangoapps/oauth_dispatch/views.py +++ b/openedx/core/djangoapps/oauth_dispatch/views.py @@ -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) diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py b/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py index a9beb4311f..a3fc7e8463 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py @@ -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() diff --git a/openedx/core/djangoapps/schedules/resolvers.py b/openedx/core/djangoapps/schedules/resolvers.py index ec5e5c8053..903cec7ae2 100644 --- a/openedx/core/djangoapps/schedules/resolvers.py +++ b/openedx/core/djangoapps/schedules/resolvers.py @@ -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 diff --git a/openedx/core/djangoapps/schedules/tasks.py b/openedx/core/djangoapps/schedules/tasks.py index d8132e6aa9..2150d8fe7c 100644 --- a/openedx/core/djangoapps/schedules/tasks.py +++ b/openedx/core/djangoapps/schedules/tasks.py @@ -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) diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index 75ec14e53d..d0e9a66947 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -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 diff --git a/openedx/core/djangoapps/waffle_utils/__init__.py b/openedx/core/djangoapps/waffle_utils/__init__.py index 20cf06480f..536e72654e 100644 --- a/openedx/core/djangoapps/waffle_utils/__init__.py +++ b/openedx/core/djangoapps/waffle_utils/__init__.py @@ -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(): diff --git a/openedx/core/djangoapps/waffle_utils/tests/test_init.py b/openedx/core/djangoapps/waffle_utils/tests/test_init.py index 5aa3a0f7c0..d7f03ffc71 100644 --- a/openedx/core/djangoapps/waffle_utils/tests/test_init.py +++ b/openedx/core/djangoapps/waffle_utils/tests/test_init.py @@ -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): diff --git a/openedx/core/lib/api/authentication.py b/openedx/core/lib/api/authentication.py index 7f71fd8994..c5a87354c4 100644 --- a/openedx/core/lib/api/authentication.py +++ b/openedx/core/lib/api/authentication.py @@ -6,7 +6,7 @@ import django.utils.timezone from oauth2_provider import models as dot_models from rest_framework.exceptions import AuthenticationFailed from rest_framework.authentication import BaseAuthentication, get_authorization_header -from edx_django_utils.monitoring import set_custom_metric +from edx_django_utils.monitoring import set_custom_attribute OAUTH2_TOKEN_ERROR = 'token_error' OAUTH2_TOKEN_ERROR_EXPIRED = 'token_expired' @@ -39,7 +39,7 @@ class BearerAuthentication(BaseAuthentication): fails. """ - set_custom_metric("BearerAuthentication", "Failed") # default value + set_custom_attribute("BearerAuthentication", "Failed") # default value auth = get_authorization_header(request).split() if len(auth) == 1: @@ -54,12 +54,12 @@ class BearerAuthentication(BaseAuthentication): if auth and auth[0].lower() == b'bearer': access_token = auth[1].decode('utf8') else: - set_custom_metric("BearerAuthentication", "None") + set_custom_attribute("BearerAuthentication", "None") return None user, token = self.authenticate_credentials(access_token) - set_custom_metric("BearerAuthentication", "Success") + set_custom_attribute("BearerAuthentication", "Success") return user, token @@ -93,13 +93,13 @@ class BearerAuthentication(BaseAuthentication): user = token.user # Check to make sure the users have activated their account (by confirming their email) if not self.allow_inactive_users and not user.is_active: - set_custom_metric("BearerAuthentication_user_active", False) + set_custom_attribute("BearerAuthentication_user_active", False) msg = 'User inactive or deleted: %s' % user.get_username() raise AuthenticationFailed({ 'error_code': OAUTH2_USER_NOT_ACTIVE_ERROR, 'developer_message': msg}) else: - set_custom_metric("BearerAuthentication_user_active", True) + set_custom_attribute("BearerAuthentication_user_active", True) return user, token diff --git a/openedx/core/lib/api/permissions.py b/openedx/core/lib/api/permissions.py index ebdc1b7e7e..df889079bc 100644 --- a/openedx/core/lib/api/permissions.py +++ b/openedx/core/lib/api/permissions.py @@ -5,7 +5,7 @@ API library for Django REST Framework permissions-oriented workflows from django.conf import settings from django.http import Http404 -from edx_django_utils.monitoring import set_custom_metric +from edx_django_utils.monitoring import set_custom_attribute from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey from rest_condition import C @@ -37,7 +37,7 @@ class ApiKeyHeaderPermission(permissions.BasePermission): audit_log("ApiKeyHeaderPermission used", path=request.path, ip=request.META.get("REMOTE_ADDR")) - set_custom_metric('deprecated_api_key_header', True) + set_custom_attribute('deprecated_api_key_header', True) return True return False diff --git a/openedx/features/course_experience/__init__.py b/openedx/features/course_experience/__init__.py index bc9e7cec64..6dd571d577 100644 --- a/openedx/features/course_experience/__init__.py +++ b/openedx/features/course_experience/__init__.py @@ -3,7 +3,7 @@ Unified course experience settings and helper methods. """ import crum from django.utils.translation import ugettext as _ -from edx_django_utils.monitoring import set_custom_metric +from edx_django_utils.monitoring import set_custom_attribute from waffle import flag_is_active from lms.djangoapps.experiments.flags import ExperimentWaffleFlag @@ -55,7 +55,7 @@ class DefaultTrueWaffleFlagNamespace(WaffleFlagNamespace): if request: value = flag_is_active(request, namespaced_flag_name) else: - set_custom_metric('warn_flag_no_request', True) + 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) diff --git a/requirements/edx/base.in b/requirements/edx/base.in index 4e9cfa45ff..3437facb87 100644 --- a/requirements/edx/base.in +++ b/requirements/edx/base.in @@ -78,7 +78,9 @@ edx-celeryutils edx-completion edx-django-release-util # Release utils for the edx release pipeline edx-django-sites-extensions -edx-django-utils +# 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-drf-extensions edx-enterprise edx-milestones