Skip to content

Commit

Permalink
rename custom metric to custom attribute
Browse files Browse the repository at this point in the history
Rename of "custom metric" to "custom attribute" was based
on a decision (ADR) captured in edx-django-utils:
https://github.com/edx/edx-django-utils/blob/master/edx_django_utils/monitoring/docs/decisions/0002-custom-monitoring-language.rst

Deprecated RequestMetricsMiddleware due to rename.  Use
RequestCustomAttributesMiddleware instead.

ARCHBOM-1495
  • Loading branch information
robrap committed Sep 24, 2020
1 parent b96eb50 commit ff138f2
Show file tree
Hide file tree
Showing 13 changed files with 240 additions and 188 deletions.
11 changes: 11 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,17 @@ Change Log
Unreleased
----------

[6.2.0] - 2020-08-24
--------------------

Updated
~~~~~~~

* Renamed "custom metric" to "custom attribute" throughout the repo. This was based on a `decision (ADR) captured in edx-django-utils`_.

* Deprecated RequestMetricsMiddleware due to rename. Use RequestCustomAttributesMiddleware instead.

.. _`decision (ADR) captured in edx-django-utils`: https://github.com/edx/edx-django-utils/blob/master/edx_django_utils/monitoring/docs/decisions/0002-custom-monitoring-language.rst

[6.1.2] - 2020-07-19
--------------------
Expand Down
2 changes: 1 addition & 1 deletion edx_rest_framework_extensions/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
""" edx Django REST Framework extensions. """

__version__ = '6.1.2' # pragma: no cover
__version__ = '6.2.0' # pragma: no cover
12 changes: 6 additions & 6 deletions edx_rest_framework_extensions/auth/bearer/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import requests
from django.contrib.auth import get_user_model
from edx_django_utils.monitoring import set_custom_metric
from edx_django_utils.monitoring import set_custom_attribute
from rest_framework import exceptions
from rest_framework.authentication import BaseAuthentication, get_authorization_header

Expand Down Expand Up @@ -37,16 +37,16 @@ def get_user_info_url(self):
return get_setting('OAUTH2_USER_INFO_URL')

def authenticate(self, request):
set_custom_metric("BearerAuthentication", "Failed") # default value
set_custom_attribute("BearerAuthentication", "Failed") # default value
if not self.get_user_info_url():
logger.warning('The setting OAUTH2_USER_INFO_URL is invalid!')
set_custom_metric("BearerAuthentication", "NoURL")
set_custom_attribute("BearerAuthentication", "NoURL")
return None
set_custom_metric("BearerAuthentication_user_info_url", self.get_user_info_url())
set_custom_attribute("BearerAuthentication_user_info_url", self.get_user_info_url())
auth = get_authorization_header(request).split()

if not auth or auth[0].lower() != b'bearer':
set_custom_metric("BearerAuthentication", "None")
set_custom_attribute("BearerAuthentication", "None")
return None

if len(auth) == 1:
Expand All @@ -55,7 +55,7 @@ def authenticate(self, request):
raise exceptions.AuthenticationFailed('Invalid token header. Token string should not contain spaces.')

output = self.authenticate_credentials(auth[1].decode('utf8'))
set_custom_metric("BearerAuthentication", "Success")
set_custom_attribute("BearerAuthentication", "Success")
return output

def authenticate_credentials(self, token):
Expand Down
18 changes: 9 additions & 9 deletions edx_rest_framework_extensions/auth/jwt/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ class JwtAuthCookieMiddleware(MiddlewareMixin):
See the full decision here:
https://github.com/edx/edx-platform/blob/master/openedx/core/djangoapps/oauth_dispatch/docs/decisions/0009-jwt-in-session-cookie.rst
Also, sets the metric 'request_jwt_cookie' with one of the following values:
Also, sets the custom attribute 'request_jwt_cookie' with one of the following values:
'success': Value when reconstitution is successful.
'not-requested': Value when jwt cookie authentication was not requested by the client.
'missing-both': Value when both cookies are missing and reconstitution is not possible.
Expand All @@ -192,8 +192,8 @@ class JwtAuthCookieMiddleware(MiddlewareMixin):
"""

def _get_missing_cookie_message_and_metric(self, cookie_name):
""" Returns tuple with missing cookie (log_message, metric_value) """
def _get_missing_cookie_message_and_attribute(self, cookie_name):
""" Returns tuple with missing cookie (log_message, custom_attribute_value) """
cookie_missing_message = '{} cookie is missing. JWT auth cookies will not be reconstituted.'.format(
cookie_name
)
Expand All @@ -219,7 +219,7 @@ def process_view(self, request, view_func, view_args, view_kwargs): # pylint: d
request.user = SimpleLazyObject(lambda: _get_user_from_jwt(request, view_func))

if not use_jwt_cookie_requested:
metric_value = 'not-requested'
attribute_value = 'not-requested'
elif header_payload_cookie and signature_cookie:
# Reconstitute JWT auth cookie if split cookies are available and jwt cookie
# authentication was requested by the client.
Expand All @@ -228,23 +228,23 @@ def process_view(self, request, view_func, view_args, view_kwargs): # pylint: d
JWT_DELIMITER,
signature_cookie,
)
metric_value = 'success'
attribute_value = 'success'
elif header_payload_cookie or signature_cookie:
# Log unexpected case of only finding one cookie.
if not header_payload_cookie:
log_message, metric_value = self._get_missing_cookie_message_and_metric(
log_message, attribute_value = self._get_missing_cookie_message_and_attribute(
jwt_cookie_header_payload_name()
)
if not signature_cookie:
log_message, metric_value = self._get_missing_cookie_message_and_metric(
log_message, attribute_value = self._get_missing_cookie_message_and_attribute(
jwt_cookie_signature_name()
)
log.warning(log_message)
else:
metric_value = 'missing-both'
attribute_value = 'missing-both'
log.warning('Both JWT auth cookies missing. JWT auth cookies will not be reconstituted.')

monitoring.set_custom_metric('request_jwt_cookie', metric_value)
monitoring.set_custom_attribute('request_jwt_cookie', attribute_value)


def _get_user_from_jwt(request, view_func):
Expand Down
26 changes: 14 additions & 12 deletions edx_rest_framework_extensions/auth/jwt/tests/test_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -364,21 +364,21 @@ def setUp(self):
self.request.session = 'mock session'
self.middleware = JwtAuthCookieMiddleware()

@patch('edx_django_utils.monitoring.set_custom_metric')
def test_do_not_use_jwt_cookies(self, mock_set_custom_metric):
@patch('edx_django_utils.monitoring.set_custom_attribute')
def test_do_not_use_jwt_cookies(self, mock_set_custom_attribute):
self.middleware.process_view(self.request, None, None, None)
self.assertIsNone(self.request.COOKIES.get(jwt_cookie_name()))
mock_set_custom_metric.assert_called_once_with('request_jwt_cookie', 'not-requested')
mock_set_custom_attribute.assert_called_once_with('request_jwt_cookie', 'not-requested')

@ddt.data(
(jwt_cookie_header_payload_name(), jwt_cookie_signature_name()),
(jwt_cookie_signature_name(), jwt_cookie_header_payload_name()),
)
@ddt.unpack
@patch('edx_rest_framework_extensions.auth.jwt.middleware.log')
@patch('edx_django_utils.monitoring.set_custom_metric')
@patch('edx_django_utils.monitoring.set_custom_attribute')
def test_missing_cookies(
self, set_cookie_name, missing_cookie_name, mock_set_custom_metric, mock_log
self, set_cookie_name, missing_cookie_name, mock_set_custom_attribute, mock_log
):
self.request.META[USE_JWT_COOKIE_HEADER] = 'true'
self.request.COOKIES[set_cookie_name] = 'test'
Expand All @@ -388,23 +388,25 @@ def test_missing_cookies(
'%s cookie is missing. JWT auth cookies will not be reconstituted.' %
missing_cookie_name
)
mock_set_custom_metric.assert_called_once_with('request_jwt_cookie', 'missing-{}'.format(missing_cookie_name))
mock_set_custom_attribute.assert_called_once_with(
'request_jwt_cookie', 'missing-{}'.format(missing_cookie_name)
)

@patch('edx_django_utils.monitoring.set_custom_metric')
def test_no_cookies(self, mock_set_custom_metric):
@patch('edx_django_utils.monitoring.set_custom_attribute')
def test_no_cookies(self, mock_set_custom_attribute):
self.request.META[USE_JWT_COOKIE_HEADER] = 'true'
self.middleware.process_view(self.request, None, None, None)
self.assertIsNone(self.request.COOKIES.get(jwt_cookie_name()))
mock_set_custom_metric.assert_called_once_with('request_jwt_cookie', 'missing-both')
mock_set_custom_attribute.assert_called_once_with('request_jwt_cookie', 'missing-both')

@patch('edx_django_utils.monitoring.set_custom_metric')
def test_success(self, mock_set_custom_metric):
@patch('edx_django_utils.monitoring.set_custom_attribute')
def test_success(self, mock_set_custom_attribute):
self.request.META[USE_JWT_COOKIE_HEADER] = 'true'
self.request.COOKIES[jwt_cookie_header_payload_name()] = 'header.payload'
self.request.COOKIES[jwt_cookie_signature_name()] = 'signature'
self.middleware.process_view(self.request, None, None, None)
self.assertEqual(self.request.COOKIES[jwt_cookie_name()], 'header.payload.signature')
mock_set_custom_metric.assert_called_once_with('request_jwt_cookie', 'success')
mock_set_custom_attribute.assert_called_once_with('request_jwt_cookie', 'success')

_LOG_WARN_AUTHENTICATION_FAILED = 0
_LOG_WARN_MISSING_JWT_AUTHENTICATION_CLASS = 1
Expand Down
78 changes: 45 additions & 33 deletions edx_rest_framework_extensions/middleware.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
"""
Middleware to ensure best practices of DRF and other endpoints.
"""
import warnings

from django.utils.deprecation import MiddlewareMixin
from edx_django_utils import monitoring
from edx_django_utils.cache import DEFAULT_REQUEST_CACHE
Expand All @@ -9,14 +11,14 @@
from edx_rest_framework_extensions.auth.jwt.cookies import jwt_cookie_name


class RequestMetricsMiddleware(MiddlewareMixin):
class RequestCustomAttributesMiddleware(MiddlewareMixin):
"""
Adds various request related metrics.
Adds various request related custom attributes.
Possible metrics include:
Possible custom attributes include:
request_authenticated_user_set_in_middleware:
Example values: 'process_request', 'process_view', 'process_response',
or 'process_exception'. Metric won't exist if user is not authenticated.
or 'process_exception'. Attribute won't exist if user is not authenticated.
request_auth_type_guess: Example values include: no-user, unauthenticated,
jwt, bearer, other-token-type, jwt-cookie, or session-or-other
Note: These are just guesses because if a token was expired, for example,
Expand All @@ -31,7 +33,7 @@ class RequestMetricsMiddleware(MiddlewareMixin):
MIDDLEWARE = (
'edx_django_utils.cache.middleware.RequestCacheMiddleware',
'edx_rest_framework_extensions.middleware.RequestMetricsMiddleware',
'edx_rest_framework_extensions.middleware.RequestCustomAttributesMiddleware',
)
This middleware should also appear after any authentication middleware.
Expand All @@ -51,68 +53,68 @@ def process_view(self, request, view_func, view_args, view_kwargs): # pylint: d

def process_response(self, request, response):
"""
Add metrics for various details of the request.
Add custom attributes for various details of the request.
"""
self._cache_if_authenticated_user_found_in_middleware(request, 'process_response')
self._set_all_request_metrics(request)
self._set_all_request_attributes(request)
return response

def process_exception(self, request, exception): # pylint: disable=unused-argument
"""
Django middleware handler to process an exception
"""
self._cache_if_authenticated_user_found_in_middleware(request, 'process_exception')
self._set_all_request_metrics(request)
self._set_all_request_attributes(request)

def _set_all_request_metrics(self, request):
def _set_all_request_attributes(self, request):
"""
Sets all the request metrics
Sets all the request custom attributes
"""
self._set_request_auth_type_guess_metric(request)
self._set_request_user_agent_metrics(request)
self._set_request_referer_metric(request)
self._set_request_user_id_metric(request)
self._set_request_authenticated_user_found_in_middleware_metric()
self._set_request_auth_type_guess_attribute(request)
self._set_request_user_agent_attributes(request)
self._set_request_referer_attribute(request)
self._set_request_user_id_attribute(request)
self._set_request_authenticated_user_found_in_middleware_attribute()

def _set_request_user_id_metric(self, request):
def _set_request_user_id_attribute(self, request):
"""
Add request_user_id metric
Add request_user_id custom attribute
Metrics:
Custom Attributes:
request_user_id
"""
if hasattr(request, 'user') and hasattr(request.user, 'id') and request.user.id:
monitoring.set_custom_metric('request_user_id', request.user.id)
monitoring.set_custom_attribute('request_user_id', request.user.id)

def _set_request_referer_metric(self, request):
def _set_request_referer_attribute(self, request):
"""
Add metric 'request_referer' for http referer.
Add custom attribute 'request_referer' for http referer.
"""
if 'HTTP_REFERER' in request.META and request.META['HTTP_REFERER']:
monitoring.set_custom_metric('request_referer', request.META['HTTP_REFERER'])
monitoring.set_custom_attribute('request_referer', request.META['HTTP_REFERER'])

def _set_request_user_agent_metrics(self, request):
def _set_request_user_agent_attributes(self, request):
"""
Add metrics for user agent for python.
Add custom attributes for user agent for python.
Metrics:
Custom Attributes:
request_user_agent
request_client_name: The client name from edx-rest-api-client calls.
"""
if 'HTTP_USER_AGENT' in request.META and request.META['HTTP_USER_AGENT']:
user_agent = request.META['HTTP_USER_AGENT']
monitoring.set_custom_metric('request_user_agent', user_agent)
monitoring.set_custom_attribute('request_user_agent', user_agent)
if user_agent:
# Example agent string from edx-rest-api-client:
# python-requests/2.9.1 edx-rest-api-client/1.7.2 ecommerce
# See https://github.com/edx/edx-rest-api-client/commit/692903c30b157f7a4edabc2f53aae1742db3a019
user_agent_parts = user_agent.split()
if len(user_agent_parts) == 3 and user_agent_parts[1].startswith('edx-rest-api-client/'):
monitoring.set_custom_metric('request_client_name', user_agent_parts[2])
monitoring.set_custom_attribute('request_client_name', user_agent_parts[2])

def _set_request_auth_type_guess_metric(self, request):
def _set_request_auth_type_guess_attribute(self, request):
"""
Add metric 'request_auth_type_guess' for the authentication type used.
Add custom attribute 'request_auth_type_guess' for the authentication type used.
NOTE: This is a best guess at this point. Possible values include:
no-user
Expand All @@ -137,17 +139,17 @@ def _set_request_auth_type_guess_metric(self, request):
auth_type = 'jwt-cookie'
else:
auth_type = 'session-or-other'
monitoring.set_custom_metric('request_auth_type_guess', auth_type)
monitoring.set_custom_attribute('request_auth_type_guess', auth_type)

AUTHENTICATED_USER_FOUND_CACHE_KEY = 'edx-drf-extensions.authenticated_user_found_in_middleware'

def _set_request_authenticated_user_found_in_middleware_metric(self):
def _set_request_authenticated_user_found_in_middleware_attribute(self):
"""
Add metric 'request_authenticated_user_found_in_middleware' if authenticated user was found.
Add custom attribute 'request_authenticated_user_found_in_middleware' if authenticated user was found.
"""
cached_response = DEFAULT_REQUEST_CACHE.get_cached_response(self.AUTHENTICATED_USER_FOUND_CACHE_KEY)
if cached_response.is_found:
monitoring.set_custom_metric(
monitoring.set_custom_attribute(
'request_authenticated_user_found_in_middleware',
cached_response.value
)
Expand All @@ -164,3 +166,13 @@ def _cache_if_authenticated_user_found_in_middleware(self, request, value):

if hasattr(request, 'user') and request.user and request.user.is_authenticated:
DEFAULT_REQUEST_CACHE.set(self.AUTHENTICATED_USER_FOUND_CACHE_KEY, value)


class RequestMetricsMiddleware(RequestCustomAttributesMiddleware):
"""
Deprecated class for handling middleware. Class has been renamed to RequestCustomAttributesMiddleware.
"""
def __init__(self, *args, **kwargs):
super(RequestMetricsMiddleware, self).__init__(*args, **kwargs)
msg = "Use 'RequestCustomAttributesMiddleware' in place of 'RequestMetricsMiddleware'."
warnings.warn(msg, DeprecationWarning)
Loading

0 comments on commit ff138f2

Please sign in to comment.