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

move CodeOwnerMetricMiddleware from LMS (#24508)

Browse Source 
The CodeOwnerMetricMiddleware is being moved from
edx-platform to edx-django-utils so it can be used
in multiple IDAs.

ARCHBOM-1356
This commit is contained in:
Robert Raposa
2020-07-17 11:35:40 -04:00
committed by GitHub
parent 129ed1b9ab
commit ef51a44794
8 changed files with 10 additions and 406 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
lms/djangoapps/monitoring/README.rst Normal file
View File

@@ -0,0 +1,9 @@
This directory contains utilities for adding a code_owner custom metric for help with split-ownership of the LMS.
For details on the decision to implement the code_owner custom metric, 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:
https://github.com/edx/edx-django-utils/tree/master/edx_django_utils/monitoring/code_owner
This directory continues to contain scripts that can help generate the appropriate ownership mappings for the LMS.

11
lms/djangoapps/monitoring/apps.py
View File

@@ -1,11 +0,0 @@
"""
Configuration for the monitoring Django application.
"""
from django.apps import AppConfig
class MonitoringConfig(AppConfig):
"""
Configuration class for the monitoring Django application.
"""
name = 'monitoring'

49
lms/djangoapps/monitoring/middleware.py
View File

@@ -1,49 +0,0 @@
"""
Middleware for monitoring the LMS
"""
import logging
from django.urls import Resolver404, resolve
from edx_django_utils.monitoring import set_custom_metric
from .utils import get_code_owner_from_module, is_code_owner_mappings_configured
log = logging.getLogger(__name__)
class CodeOwnerMetricMiddleware:
"""
Django middleware object to set custom metrics for the owner of each view.
Custom metrics set:
- code_owner: The owning team mapped to the current view.
- code_owner_mapping_error: If there are any errors when trying to perform the mapping.
- view_func_module: The __module__ of the view_func, which can be used to
find missing mappings.
"""
def __init__(self, get_response):
self.get_response = get_response
def __call__(self, request):
self._set_owner_metrics_for_request(request)
response = self.get_response(request)
return response
def _set_owner_metrics_for_request(self, request):
"""
Uses the request path to find the view_func and then sets code owner metrics based on the view.
"""
if not is_code_owner_mappings_configured():
return
try:
view_func, _, _ = resolve(request.path)
view_func_module = view_func.__module__
set_custom_metric('view_func_module', view_func_module)
code_owner = get_code_owner_from_module(view_func_module)
if code_owner:
set_custom_metric('code_owner', code_owner)
except Resolver404:
set_custom_metric('code_owner_mapping_error', "Couldn't resolve view for request path {}".format(request.path))
except Exception as e:
set_custom_metric('code_owner_mapping_error', e)

13
lms/djangoapps/monitoring/tests/mock_views.py
View File

@@ -1,13 +0,0 @@
"""
Mock views with a different module to enable testing of mapping
code_owner to modules. Trying to mock __module__ on a view was
getting too complex.
"""
from rest_framework.views import APIView
class MockViewTest(APIView):
"""
Mock view for use in testing.
"""
pass

122
lms/djangoapps/monitoring/tests/test_middleware.py
View File

@@ -1,122 +0,0 @@
"""
Tests for the LMS monitoring middleware
"""
import ddt
from django.conf.urls import url
from django.test import override_settings, RequestFactory
from django.urls import resolve
from mock import call, patch, Mock
from rest_framework.views import APIView
from unittest import TestCase
from unittest.mock import ANY
from lms.djangoapps.monitoring.middleware import CodeOwnerMetricMiddleware
from lms.djangoapps.monitoring.tests.mock_views import MockViewTest
from lms.djangoapps.monitoring.utils import _process_code_owner_mappings
class MockMiddlewareViewTest(APIView):
pass
urlpatterns = [
url(r'^middleware-test/$', MockMiddlewareViewTest.as_view()),
url(r'^test/$', MockViewTest.as_view()),
]
@ddt.ddt
class CodeOwnerMetricMiddlewareTests(TestCase):
"""
Tests for the LMS monitoring utility functions
"""
urls = 'lms.djangoapps.monitoring.tests.test_middleware.test_urls'
def setUp(self):
super().setUp()
self.mock_get_response = Mock()
self.middleware = CodeOwnerMetricMiddleware(self.mock_get_response)
def test_init(self):
self.assertEqual(self.middleware.get_response, self.mock_get_response)
def test_request_call(self):
self.mock_get_response.return_value = 'test-response'
request = Mock()
self.assertEqual(self.middleware(request), 'test-response')
_REQUEST_PATH_TO_MODULE_PATH = {
'/middleware-test/': 'test_middleware',
'/test/': 'lms.djangoapps.monitoring.tests.mock_views',
}
@override_settings(
CODE_OWNER_MAPPINGS={'team-red': ['lms.djangoapps.monitoring.tests.mock_views']},
ROOT_URLCONF=__name__,
)
@patch('lms.djangoapps.monitoring.middleware.set_custom_metric')
@ddt.data(
('/middleware-test/', None),
('/test/', 'team-red'),
)
@ddt.unpack
def test_code_owner_mapping_hits_and_misses(
self, request_path, expected_owner, mock_set_custom_metric
):
with patch('lms.djangoapps.monitoring.utils._PATH_TO_CODE_OWNER_MAPPINGS', _process_code_owner_mappings()):
request = RequestFactory().get(request_path)
self.middleware(request)
view_func, _, _ = resolve(request_path)
expected_view_func_module = self._REQUEST_PATH_TO_MODULE_PATH[request_path]
self._assert_code_owner_custom_metrics(
expected_view_func_module, mock_set_custom_metric, expected_code_owner=expected_owner
)
@patch('lms.djangoapps.monitoring.middleware.set_custom_metric')
def test_code_owner_no_mappings(self, mock_set_custom_metric):
request = RequestFactory().get('/test/')
self.middleware(request)
mock_set_custom_metric.assert_not_called()
@override_settings(
CODE_OWNER_MAPPINGS={'team-red': ['lms.djangoapps.monitoring.tests.mock_views']},
)
@patch('lms.djangoapps.monitoring.middleware.set_custom_metric')
def test_no_resolver_for_request_path(self, mock_set_custom_metric):
with patch('lms.djangoapps.monitoring.utils._PATH_TO_CODE_OWNER_MAPPINGS', _process_code_owner_mappings()):
request = RequestFactory().get('/bad/path/')
self.middleware(request)
self._assert_code_owner_custom_metrics(
None, mock_set_custom_metric, has_error=True
)
@override_settings(
CODE_OWNER_MAPPINGS=['invalid_setting_as_list'],
ROOT_URLCONF=__name__,
)
@patch('lms.djangoapps.monitoring.middleware.set_custom_metric')
def test_load_config_with_invalid_dict(self, mock_set_custom_metric):
with patch('lms.djangoapps.monitoring.utils._PATH_TO_CODE_OWNER_MAPPINGS', _process_code_owner_mappings()):
request = RequestFactory().get('/test/')
self.middleware(request)
expected_view_func_module = self._REQUEST_PATH_TO_MODULE_PATH['/test/']
self._assert_code_owner_custom_metrics(
expected_view_func_module, mock_set_custom_metric, has_error=True
)
def _assert_code_owner_custom_metrics(
self, view_func_module, mock_set_custom_metric, expected_code_owner=None, has_error=False,
process_view_func=None,
):
call_list = []
if view_func_module:
call_list.append(call('view_func_module', view_func_module))
if expected_code_owner:
call_list.append(call('code_owner', expected_code_owner))
if has_error:
call_list.append(call('code_owner_mapping_error', ANY))
mock_set_custom_metric.assert_has_calls(call_list)
self.assertEqual(
len(mock_set_custom_metric.call_args_list), len(call_list),
'Expected calls {} vs actual calls {}'.format(call_list, mock_set_custom_metric.call_args_list)
)

93
lms/djangoapps/monitoring/tests/test_monitoring_utils.py
View File

@@ -1,93 +0,0 @@
"""
Tests for the LMS monitoring middleware
Note: File named test_monitoring_utils.py instead of test_utils.py because of an error
in Jenkins with test collection and a conflict with another test_utils.py file.
"""
import ddt
import timeit
from django.test import override_settings
from mock import call, patch, Mock
from unittest import TestCase
from lms.djangoapps.monitoring.utils import (
_process_code_owner_mappings,
get_code_owner_from_module,
is_code_owner_mappings_configured,
)
@ddt.ddt
class MonitoringUtilsTests(TestCase):
"""
Tests for the LMS monitoring utility functions
"""
@override_settings(CODE_OWNER_MAPPINGS=None)
def test_is_config_loaded_with_no_config(self):
with patch('lms.djangoapps.monitoring.utils._PATH_TO_CODE_OWNER_MAPPINGS', _process_code_owner_mappings()):
self.assertFalse(is_code_owner_mappings_configured(), "Mappings should not be configured.")
@override_settings(CODE_OWNER_MAPPINGS={'team-red': ['openedx.core.djangoapps.xblock']})
def test_is_config_loaded_with_valid_dict(self):
with patch('lms.djangoapps.monitoring.utils._PATH_TO_CODE_OWNER_MAPPINGS', _process_code_owner_mappings()):
self.assertTrue(is_code_owner_mappings_configured(), "Mappings should be configured.")
@override_settings(CODE_OWNER_MAPPINGS=['invalid_setting_as_list'])
def test_is_config_loaded_with_invalid_dict(self):
with patch('lms.djangoapps.monitoring.utils._PATH_TO_CODE_OWNER_MAPPINGS', _process_code_owner_mappings()):
self.assertTrue(is_code_owner_mappings_configured(), "Although invalid, mappings should be configured.")
@override_settings(CODE_OWNER_MAPPINGS={
'team-red': [
'openedx.core.djangoapps.xblock',
'lms.djangoapps.grades',
],
'team-blue': [
'common.djangoapps.xblock_django',
],
})
@ddt.data(
('xbl', None),
('xblock_2', None),
('xblock', 'team-red'),
('openedx.core.djangoapps', None),
('openedx.core.djangoapps.xblock', 'team-red'),
('openedx.core.djangoapps.xblock.views', 'team-red'),
('grades', 'team-red'),
('lms.djangoapps.grades', 'team-red'),
('xblock_django', 'team-blue'),
('common.djangoapps.xblock_django', 'team-blue'),
)
@ddt.unpack
def test_code_owner_mapping_hits_and_misses(self, module, expected_owner):
with patch('lms.djangoapps.monitoring.utils._PATH_TO_CODE_OWNER_MAPPINGS', _process_code_owner_mappings()):
actual_owner = get_code_owner_from_module(module)
self.assertEqual(expected_owner, actual_owner)
@override_settings(CODE_OWNER_MAPPINGS=['invalid_setting_as_list'])
def test_load_config_with_invalid_dict(self):
with patch('lms.djangoapps.monitoring.utils._PATH_TO_CODE_OWNER_MAPPINGS', _process_code_owner_mappings()):
self.assertTrue(is_code_owner_mappings_configured(), "Although invalid, mappings should be configured.")
with self.assertRaises(AssertionError):
get_code_owner_from_module('xblock')
def test_mapping_performance(self):
code_owner_mappings = {
'team-red': []
}
# create a long list of mappings that are nearly identical
for n in range(1, 200):
path = 'openedx.core.djangoapps.{}'.format(n)
code_owner_mappings['team-red'].append(path)
with override_settings(CODE_OWNER_MAPPINGS=code_owner_mappings):
with patch(
'lms.djangoapps.monitoring.utils._PATH_TO_CODE_OWNER_MAPPINGS', _process_code_owner_mappings()
):
call_iterations = 100
time = timeit.timeit(
# test a module name that matches nearly to the end, but doesn't actually match
lambda: get_code_owner_from_module('openedx.core.djangoapps.XXX.views'), number=call_iterations
)
average_time = time / call_iterations
self.assertTrue(average_time < 0.0005, 'Mapping takes {}s which is too slow.'.format(average_time))

114
lms/djangoapps/monitoring/utils.py
View File

@@ -1,114 +0,0 @@
"""
Utilities for monitoring the LMS
"""
import logging
import re
from django.conf import settings
log = logging.getLogger(__name__)
def get_code_owner_from_module(module):
"""
Attempts lookup of code_owner based on a code module,
finding the most specific match. If no match, returns None.
For example, if the module were 'openedx.features.discounts.views',
this lookup would match on 'openedx.features.discounts' before
'openedx.features', because the former is more specific.
"""
assert _PATH_TO_CODE_OWNER_MAPPINGS != _INVALID_CODE_OWNER_MAPPING,\
'CODE_OWNER_MAPPINGS django setting set with invalid configuration. See logs for details.'
module_parts = module.split('.')
# To make the most specific match, start with the max number of parts
for number_of_parts in range(len(module_parts), 0, -1):
partial_path = '.'.join(module_parts[0:number_of_parts])
if partial_path in _PATH_TO_CODE_OWNER_MAPPINGS:
code_owner = _PATH_TO_CODE_OWNER_MAPPINGS[partial_path]
return code_owner
return None
def is_code_owner_mappings_configured():
"""
Returs True if code owner mappings were configured, and False otherwise.
"""
return bool(_PATH_TO_CODE_OWNER_MAPPINGS)
def _process_code_owner_mappings():
"""
Processes the CODE_OWNER_MAPPINGS Django Setting and returns a dict optimized
for efficient lookup by path.
Returns:
(dict): optimized dict for success processing, None if there are no
configured mappings, or _INVALID_CODE_OWNER_MAPPING if there is an
error processing the setting.
Example CODE_OWNER_MAPPINGS Django Setting::
CODE_OWNER_MAPPINGS = {
'team-red': [
'xblock_django',
'openedx.core.djangoapps.xblock',
],
'team-blue': [
'badges',
],
}
Example return value::
{
'xblock_django': 'team-red',
'openedx.core.djangoapps.xblock': 'team-red',
'badges': 'team-blue',
}
"""
_CODE_OWNER_MAPPINGS = getattr(settings, 'CODE_OWNER_MAPPINGS', None)
if not _CODE_OWNER_MAPPINGS:
return None
try:
path_to_code_owner_mappings = {}
for code_owner in _CODE_OWNER_MAPPINGS:
path_list = _CODE_OWNER_MAPPINGS[code_owner]
for path in path_list:
path_to_code_owner_mappings[path] = code_owner
optional_module_prefix_match = _OPTIONAL_MODULE_PREFIX_PATTERN.match(path)
# if path has an optional prefix, also add the module name without the prefix
if optional_module_prefix_match:
path_without_prefix = path[optional_module_prefix_match.end():]
path_to_code_owner_mappings[path_without_prefix] = code_owner
return path_to_code_owner_mappings
except Exception as e:
log.exception('Error processing code_owner_mappings. {}'.format(e))
# errors should be unlikely due do scripting the setting values.
# this will trigger an error custom metric that can be alerted on.
return _INVALID_CODE_OWNER_MAPPING
# .. toggle_name: CODE_OWNER_MAPPINGS
# .. toggle_implementation: DjangoSetting
# .. toggle_default: None
# .. toggle_description: Used to set monitoring custom metrics for owner. Dict with keys of code owner and value as list of dotted path module names owned by code owner.
# .. toggle_category: monitoring
# .. toggle_use_cases: open_edx
# .. toggle_creation_date: 2020-05-29
# .. toggle_expiration_date: None
# .. toggle_tickets: None
# .. toggle_status: supported
# .. toggle_warnings: None
# Set to settings.CODE_OWNER_MAPPINGS during processing for simpler testing
_CODE_OWNER_MAPPINGS = None
# The following module prefixes are optional in a view's reported module:
# 'common.djangoapps.', 'lms.djangoapps.', 'openedx.core.djangoapps.'
_OPTIONAL_MODULE_PREFIX_PATTERN = re.compile(r'^(lms|common|openedx\.core)\.djangoapps\.')
_INVALID_CODE_OWNER_MAPPING = 'invalid-code-owner-mapping'
# lookup table for code owner given a module path
_PATH_TO_CODE_OWNER_MAPPINGS = _process_code_owner_mappings()

5
lms/envs/common.py
View File

@@ -1499,7 +1499,7 @@ MIDDLEWARE = [
'edx_django_utils.monitoring.middleware.MonitoringCustomMetricsMiddleware',
# Generate code ownership metrics. Keep this immediately after RequestCacheMiddleware.
'lms.djangoapps.monitoring.middleware.CodeOwnerMetricMiddleware',
'edx_django_utils.monitoring.code_owner.middleware.CodeOwnerMetricMiddleware',
# Cookie monitoring
'openedx.core.lib.request_utils.CookieMetricsMiddleware',
@@ -2561,9 +2561,6 @@ INSTALLED_APPS = [
'openedx.core.djangoapps.schedules',
'rest_framework_jwt',
# Monitoring utilities for LMS
'lms.djangoapps.monitoring.apps.MonitoringConfig',
# Learning Sequence Navigation
'openedx.core.djangoapps.content.learning_sequences.apps.LearningSequencesConfig',