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

remove temp_view_func_compare custom metric (#24211)

Browse Source 
In an earlier PR, we moved logic from process_view to process
request, so mapping would happen earlier in the middleware
lifecycle, and the code_owner metric would be set for requests
that never made it to process_view.

The temp_view_func_compare custom metric was added temporarily to
ensure this earlier refactor did not introduce any unaccounted for
differences. It did not, so we are removing this temporary metric.

ARCHBOM-1263
This commit is contained in:
Robert Raposa
2020-06-12 08:53:20 -04:00
committed by GitHub
parent 46ae66a146
commit 7e56d89bcc
2 changed files with 0 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -3,7 +3,6 @@ Middleware for monitoring the LMS
"""
import logging
from django.urls import Resolver404, resolve
from edx_django_utils.cache import DEFAULT_REQUEST_CACHE
from edx_django_utils.monitoring import set_custom_metric
from .utils import get_code_owner_from_module, is_code_owner_mappings_configured
@@ -30,11 +29,6 @@ class CodeOwnerMetricMiddleware:
response = self.get_response(request)
return response
def process_view(self, request, view_func, view_args, view_kwargs):
self._set_view_func_compare_metric(view_func)
_VIEW_FUNC_MODULE_METRIC_CACHE_KEY = '{}.view_func_module_metric'.format(__file__)
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.
@@ -45,7 +39,6 @@ class CodeOwnerMetricMiddleware:
try:
view_func, _, _ = resolve(request.path)
view_func_module = view_func.__module__
DEFAULT_REQUEST_CACHE.set(self._VIEW_FUNC_MODULE_METRIC_CACHE_KEY, 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:
@@ -54,19 +47,3 @@ class CodeOwnerMetricMiddleware:
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)
def _set_view_func_compare_metric(self, view_func):
"""
Set temporary metric to ensure that the view_func of `process_view` always matches
the one from using `resolve` on the request.
"""
try:
view_func_module = view_func.__module__
cached_response = DEFAULT_REQUEST_CACHE.get_cached_response(self._VIEW_FUNC_MODULE_METRIC_CACHE_KEY)
if cached_response.is_found:
view_func_compare = 'success' if view_func_module == cached_response.value else view_func_module
else:
view_func_compare = 'missing'
set_custom_metric('temp_view_func_compare', view_func_compare)
except Exception as e:
set_custom_metric('temp_view_func_compare_error', e)

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

@@ -5,7 +5,6 @@ import ddt
from django.conf.urls import url
from django.test import override_settings, RequestFactory
from django.urls import resolve
from edx_django_utils.cache import RequestCache
from mock import call, patch, Mock
from rest_framework.views import APIView
from unittest import TestCase
@@ -35,7 +34,6 @@ class CodeOwnerMetricMiddlewareTests(TestCase):
def setUp(self):
super().setUp()
RequestCache.clear_all_namespaces()
self.mock_get_response = Mock()
self.middleware = CodeOwnerMetricMiddleware(self.mock_get_response)
@@ -69,7 +67,6 @@ class CodeOwnerMetricMiddlewareTests(TestCase):
request = RequestFactory().get(request_path)
self.middleware(request)
view_func, _, _ = resolve(request_path)
self.middleware.process_view(None, view_func, None, None)
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
@@ -107,41 +104,6 @@ class CodeOwnerMetricMiddlewareTests(TestCase):
expected_view_func_module, mock_set_custom_metric, has_error=True
)
@override_settings(
CODE_OWNER_MAPPINGS={'team-red': ['lms.djangoapps.monitoring.tests.mock_views']},
ROOT_URLCONF=__name__,
)
@patch('lms.djangoapps.monitoring.middleware.set_custom_metric')
def test_failed_compare_in_process_view(self, mock_set_custom_metric):
"""
Tests that the process_view gets the same function. This is to conservatively
test that the resolve method results in all the same metrics.
"""
class MockWithMockModule():
"""
Added inside function to enable quick clean-up when this is removed.
"""
def __init__(self, mocked_module_name):
self._mocked_module_name = mocked_module_name
def __getattribute__(self, name):
if name == '__module__':
return self._mocked_module_name
return super().__getattribute__(name)
with patch('lms.djangoapps.monitoring.utils._PATH_TO_CODE_OWNER_MAPPINGS', _process_code_owner_mappings()):
request_path = '/test/'
expected_owner = 'team-red'
request = RequestFactory().get(request_path)
self.middleware(request)
expected_view_func_module = self._REQUEST_PATH_TO_MODULE_PATH[request_path]
mock_view_func = MockWithMockModule('different.module')
self.middleware.process_view(None, mock_view_func, None, None)
self._assert_code_owner_custom_metrics(
expected_view_func_module, mock_set_custom_metric, expected_code_owner=expected_owner,
process_view_func=mock_view_func,
)
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,
@@ -153,11 +115,6 @@ class CodeOwnerMetricMiddlewareTests(TestCase):
call_list.append(call('code_owner', expected_code_owner))
if has_error:
call_list.append(call('code_owner_mapping_error', ANY))
if view_func_module and not has_error:
if process_view_func:
call_list.append(call('temp_view_func_compare', process_view_func.__module__))
else:
call_list.append(call('temp_view_func_compare', 'success'))
mock_set_custom_metric.assert_has_calls(call_list)
self.assertEqual(
len(mock_set_custom_metric.call_args_list), len(call_list),