diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a3f90067..52be2145 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -15,6 +15,17 @@ Change Log Unreleased ~~~~~~~~~~ +[3.9.0] - 2020-10-21 +~~~~~~~~~~~~~~~~~~~~ + +Updated +_______ + +* Exposed existing get_code_owner_from_module via the public api. +* Fixed get_code_owner_from_module to not require a call to is_code_owner_mappings_configured beforehand. +* Set the existing code_owner_path_module custom attribute, even for cases where the transaction name was used, rather than the view module. +* Refactor code owner setting processing. + [3.8.0] - 2020-08-31 ~~~~~~~~~~~~~~~~~~~~ diff --git a/edx_django_utils/__init__.py b/edx_django_utils/__init__.py index a06194ce..e728a5bc 100644 --- a/edx_django_utils/__init__.py +++ b/edx_django_utils/__init__.py @@ -2,7 +2,7 @@ EdX utilities for Django Application development.. """ -__version__ = "3.8.0" +__version__ = "3.9.0" default_app_config = ( "edx_django_utils.apps.EdxDjangoUtilsConfig" diff --git a/edx_django_utils/monitoring/__init__.py b/edx_django_utils/monitoring/__init__.py index 91b52a4a..4ebc6f06 100644 --- a/edx_django_utils/monitoring/__init__.py +++ b/edx_django_utils/monitoring/__init__.py @@ -3,6 +3,7 @@ See README.rst for details. """ +from .code_owner.utils import get_code_owner_from_module from .transactions import function_trace, get_current_transaction, ignore_transaction, set_monitoring_transaction_name # "set_custom_metric*" methods are deprecated from .utils import ( diff --git a/edx_django_utils/monitoring/code_owner/__init__.py b/edx_django_utils/monitoring/code_owner/__init__.py index e69de29b..952966e8 100644 --- a/edx_django_utils/monitoring/code_owner/__init__.py +++ b/edx_django_utils/monitoring/code_owner/__init__.py @@ -0,0 +1,6 @@ +""" +This directory should only be used internally. + +Its public API is exposed in the top-level monitoring __init__.py. +See its README.rst for details. +""" diff --git a/edx_django_utils/monitoring/code_owner/middleware.py b/edx_django_utils/monitoring/code_owner/middleware.py index 7660e55f..702e4bd4 100644 --- a/edx_django_utils/monitoring/code_owner/middleware.py +++ b/edx_django_utils/monitoring/code_owner/middleware.py @@ -117,15 +117,17 @@ def _set_code_owner_attribute_from_current_transaction(self, request): """ if not is_code_owner_mappings_configured(): - return None, None + # ensure we don't set code ownership custom attributes if not configured to do so + return None, None # pragma: no cover try: # Example: openedx.core.djangoapps.contentserver.middleware:StaticContentServer transaction_name = get_current_transaction().name if not transaction_name: return None, 'No current transaction name found.' - set_custom_attribute('code_owner_transaction_name', transaction_name) module_name = transaction_name.split(':')[0] + set_custom_attribute('code_owner_transaction_name', transaction_name) + set_custom_attribute('code_owner_path_module', module_name) code_owner = get_code_owner_from_module(module_name) return code_owner, None except Exception as e: # pylint: disable=broad-except @@ -139,9 +141,6 @@ def _set_code_owner_attribute_catch_all(self): (str): code_owner or None if no catch-all configured. """ - if not is_code_owner_mappings_configured(): - return None - try: code_owner = get_code_owner_from_module('*') return code_owner diff --git a/edx_django_utils/monitoring/code_owner/tests/test_middleware.py b/edx_django_utils/monitoring/code_owner/tests/test_middleware.py index 9bce1fc9..5265733b 100644 --- a/edx_django_utils/monitoring/code_owner/tests/test_middleware.py +++ b/edx_django_utils/monitoring/code_owner/tests/test_middleware.py @@ -12,7 +12,7 @@ from edx_django_utils.monitoring.code_owner.middleware import CodeOwnerMonitoringMiddleware from edx_django_utils.monitoring.code_owner.tests.mock_views import MockViewTest -from edx_django_utils.monitoring.code_owner.utils import _process_code_owner_mappings +from edx_django_utils.monitoring.code_owner.utils import clear_cached_mappings class MockMiddlewareViewTest(View): @@ -34,6 +34,7 @@ class CodeOwnerMetricMiddlewareTests(TestCase): def setUp(self): super().setUp() + clear_cached_mappings() self.mock_get_response = Mock() self.middleware = CodeOwnerMonitoringMiddleware(self.mock_get_response) @@ -63,22 +64,18 @@ def test_request_call(self): def test_code_owner_path_mapping_hits_and_misses( self, request_path, expected_owner, mock_set_custom_attribute ): - with patch( - 'edx_django_utils.monitoring.code_owner.utils._PATH_TO_CODE_OWNER_MAPPINGS', - _process_code_owner_mappings() - ): - request = RequestFactory().get(request_path) - self.middleware(request) - expected_path_module = self._REQUEST_PATH_TO_MODULE_PATH[request_path] - self._assert_code_owner_custom_attributes( - mock_set_custom_attribute, expected_code_owner=expected_owner, path_module=expected_path_module - ) + request = RequestFactory().get(request_path) + self.middleware(request) + expected_path_module = self._REQUEST_PATH_TO_MODULE_PATH[request_path] + self._assert_code_owner_custom_attributes( + mock_set_custom_attribute, expected_code_owner=expected_owner, path_module=expected_path_module + ) - mock_set_custom_attribute.reset_mock() - self.middleware.process_exception(request, None) - self._assert_code_owner_custom_attributes( - mock_set_custom_attribute, expected_code_owner=expected_owner, path_module=expected_path_module - ) + mock_set_custom_attribute.reset_mock() + self.middleware.process_exception(request, None) + self._assert_code_owner_custom_attributes( + mock_set_custom_attribute, expected_code_owner=expected_owner, path_module=expected_path_module + ) @override_settings( CODE_OWNER_MAPPINGS={ @@ -96,16 +93,12 @@ def test_code_owner_path_mapping_hits_and_misses( def test_code_owner_path_mapping_with_catch_all( self, request_path, expected_owner, mock_set_custom_attribute ): - with patch( - 'edx_django_utils.monitoring.code_owner.utils._PATH_TO_CODE_OWNER_MAPPINGS', - _process_code_owner_mappings() - ): - request = RequestFactory().get(request_path) - self.middleware(request) - expected_path_module = self._REQUEST_PATH_TO_MODULE_PATH[request_path] - self._assert_code_owner_custom_attributes( - mock_set_custom_attribute, expected_code_owner=expected_owner, path_module=expected_path_module - ) + request = RequestFactory().get(request_path) + self.middleware(request) + expected_path_module = self._REQUEST_PATH_TO_MODULE_PATH[request_path] + self._assert_code_owner_custom_attributes( + mock_set_custom_attribute, expected_code_owner=expected_owner, path_module=expected_path_module + ) @override_settings( CODE_OWNER_MAPPINGS={'team-red': ['edx_django_utils.monitoring.code_owner.tests.mock_views']}, @@ -114,29 +107,35 @@ def test_code_owner_path_mapping_with_catch_all( @patch('edx_django_utils.monitoring.code_owner.middleware.set_custom_attribute') @patch('newrelic.agent') @ddt.data( - ('edx_django_utils.monitoring.code_owner.tests.test_middleware:MockMiddlewareViewTest', None), - ('edx_django_utils.monitoring.code_owner.tests.mock_views:MockViewTest', 'team-red'), + ( + 'edx_django_utils.monitoring.code_owner.tests.test_middleware', + 'edx_django_utils.monitoring.code_owner.tests.test_middleware:MockMiddlewareViewTest', + None + ), + ( + 'edx_django_utils.monitoring.code_owner.tests.mock_views', + 'edx_django_utils.monitoring.code_owner.tests.mock_views:MockViewTest', + 'team-red' + ), ) @ddt.unpack def test_code_owner_transaction_mapping_hits_and_misses( - self, transaction_name, expected_owner, mock_newrelic_agent, mock_set_custom_attribute + self, path_module, transaction_name, expected_owner, mock_newrelic_agent, mock_set_custom_attribute ): mock_newrelic_agent.current_transaction().name = transaction_name - with patch( - 'edx_django_utils.monitoring.code_owner.utils._PATH_TO_CODE_OWNER_MAPPINGS', - _process_code_owner_mappings() - ): - request = RequestFactory().get('/bad/path/') - self.middleware(request) - self._assert_code_owner_custom_attributes( - mock_set_custom_attribute, expected_code_owner=expected_owner, transaction_name=transaction_name - ) + request = RequestFactory().get('/bad/path/') + self.middleware(request) + self._assert_code_owner_custom_attributes( + mock_set_custom_attribute, expected_code_owner=expected_owner, path_module=path_module, + transaction_name=transaction_name + ) - mock_set_custom_attribute.reset_mock() - self.middleware.process_exception(request, None) - self._assert_code_owner_custom_attributes( - mock_set_custom_attribute, expected_code_owner=expected_owner, transaction_name=transaction_name - ) + mock_set_custom_attribute.reset_mock() + self.middleware.process_exception(request, None) + self._assert_code_owner_custom_attributes( + mock_set_custom_attribute, expected_code_owner=expected_owner, path_module=path_module, + transaction_name=transaction_name + ) @override_settings( CODE_OWNER_MAPPINGS={ @@ -148,23 +147,28 @@ def test_code_owner_transaction_mapping_hits_and_misses( @patch('edx_django_utils.monitoring.code_owner.middleware.set_custom_attribute') @patch('newrelic.agent') @ddt.data( - ('edx_django_utils.monitoring.code_owner.tests.test_middleware:MockMiddlewareViewTest', 'team-blue'), - ('edx_django_utils.monitoring.code_owner.tests.mock_views:MockViewTest', 'team-red'), + ( + 'edx_django_utils.monitoring.code_owner.tests.test_middleware', + 'edx_django_utils.monitoring.code_owner.tests.test_middleware:MockMiddlewareViewTest', + 'team-blue' + ), + ( + 'edx_django_utils.monitoring.code_owner.tests.mock_views', + 'edx_django_utils.monitoring.code_owner.tests.mock_views:MockViewTest', + 'team-red' + ), ) @ddt.unpack - def test_code_owner_transaction_mapping__with_catch_all( - self, transaction_name, expected_owner, mock_newrelic_agent, mock_set_custom_attribute + def test_code_owner_transaction_mapping_with_catch_all( + self, path_module, transaction_name, expected_owner, mock_newrelic_agent, mock_set_custom_attribute ): mock_newrelic_agent.current_transaction().name = transaction_name - with patch( - 'edx_django_utils.monitoring.code_owner.utils._PATH_TO_CODE_OWNER_MAPPINGS', - _process_code_owner_mappings() - ): - request = RequestFactory().get('/bad/path/') - self.middleware(request) - self._assert_code_owner_custom_attributes( - mock_set_custom_attribute, expected_code_owner=expected_owner, transaction_name=transaction_name - ) + request = RequestFactory().get('/bad/path/') + self.middleware(request) + self._assert_code_owner_custom_attributes( + mock_set_custom_attribute, expected_code_owner=expected_owner, path_module=path_module, + transaction_name=transaction_name + ) @override_settings( CODE_OWNER_MAPPINGS={'team-red': ['edx_django_utils.monitoring.code_owner.tests.mock_views']}, @@ -174,15 +178,11 @@ def test_code_owner_transaction_mapping__with_catch_all( @patch('newrelic.agent') def test_code_owner_transaction_mapping_error(self, mock_newrelic_agent, mock_set_custom_attribute): mock_newrelic_agent.current_transaction = Mock(side_effect=Exception('forced exception')) - with patch( - 'edx_django_utils.monitoring.code_owner.utils._PATH_TO_CODE_OWNER_MAPPINGS', - _process_code_owner_mappings() - ): - request = RequestFactory().get('/bad/path/') - self.middleware(request) - self._assert_code_owner_custom_attributes( - mock_set_custom_attribute, has_path_error=True, has_transaction_error=True - ) + request = RequestFactory().get('/bad/path/') + self.middleware(request) + self._assert_code_owner_custom_attributes( + mock_set_custom_attribute, has_path_error=True, has_transaction_error=True + ) @patch('edx_django_utils.monitoring.code_owner.middleware.set_custom_attribute') def test_code_owner_no_mappings(self, mock_set_custom_attribute): @@ -190,33 +190,31 @@ def test_code_owner_no_mappings(self, mock_set_custom_attribute): self.middleware(request) mock_set_custom_attribute.assert_not_called() + @patch('edx_django_utils.monitoring.code_owner.middleware.set_custom_attribute') + def test_code_owner_transaction_no_mappings(self, mock_set_custom_attribute): + request = RequestFactory().get('/bad/path/') + self.middleware(request) + mock_set_custom_attribute.assert_not_called() + @override_settings( CODE_OWNER_MAPPINGS={'team-red': ['lms.djangoapps.monitoring.tests.mock_views']}, ) @patch('edx_django_utils.monitoring.code_owner.middleware.set_custom_attribute') def test_no_resolver_for_path_and_no_transaction(self, mock_set_custom_attribute): - with patch( - 'edx_django_utils.monitoring.code_owner.utils._PATH_TO_CODE_OWNER_MAPPINGS', - _process_code_owner_mappings() - ): - request = RequestFactory().get('/bad/path/') - self.middleware(request) - self._assert_code_owner_custom_attributes( - mock_set_custom_attribute, has_path_error=True, has_transaction_error=True - ) + request = RequestFactory().get('/bad/path/') + self.middleware(request) + self._assert_code_owner_custom_attributes( + mock_set_custom_attribute, has_path_error=True, has_transaction_error=True + ) @override_settings( CODE_OWNER_MAPPINGS={'team-red': ['*']}, ) @patch('edx_django_utils.monitoring.code_owner.middleware.set_custom_attribute') def test_catch_all_instead_of_errors(self, mock_set_custom_attribute): - with patch( - 'edx_django_utils.monitoring.code_owner.utils._PATH_TO_CODE_OWNER_MAPPINGS', - _process_code_owner_mappings() - ): - request = RequestFactory().get('/bad/path/') - self.middleware(request) - self._assert_code_owner_custom_attributes(mock_set_custom_attribute, expected_code_owner='team-red') + request = RequestFactory().get('/bad/path/') + self.middleware(request) + self._assert_code_owner_custom_attributes(mock_set_custom_attribute, expected_code_owner='team-red') @override_settings( CODE_OWNER_MAPPINGS=['invalid_setting_as_list'], @@ -224,17 +222,12 @@ def test_catch_all_instead_of_errors(self, mock_set_custom_attribute): ) @patch('edx_django_utils.monitoring.code_owner.middleware.set_custom_attribute') def test_load_config_with_invalid_dict(self, mock_set_custom_attribute): - with patch( - 'edx_django_utils.monitoring.code_owner.utils._PATH_TO_CODE_OWNER_MAPPINGS', - _process_code_owner_mappings() - ): - request = RequestFactory().get('/test/') - self.middleware(request) - expected_path_module = self._REQUEST_PATH_TO_MODULE_PATH['/test/'] - self._assert_code_owner_custom_attributes( - mock_set_custom_attribute, path_module=expected_path_module, - has_path_error=True, has_transaction_error=True - ) + request = RequestFactory().get('/test/') + self.middleware(request) + expected_path_module = self._REQUEST_PATH_TO_MODULE_PATH['/test/'] + self._assert_code_owner_custom_attributes( + mock_set_custom_attribute, path_module=expected_path_module, + ) def _assert_code_owner_custom_attributes(self, mock_set_custom_attribute, expected_code_owner=None, path_module=None, has_path_error=False, diff --git a/edx_django_utils/monitoring/code_owner/tests/test_utils.py b/edx_django_utils/monitoring/code_owner/tests/test_utils.py index df4179af..35b59fea 100644 --- a/edx_django_utils/monitoring/code_owner/tests/test_utils.py +++ b/edx_django_utils/monitoring/code_owner/tests/test_utils.py @@ -9,7 +9,7 @@ from mock import patch from edx_django_utils.monitoring.code_owner.utils import ( - _process_code_owner_mappings, + clear_cached_mappings, get_code_owner_from_module, is_code_owner_mappings_configured ) @@ -20,29 +20,21 @@ class MonitoringUtilsTests(TestCase): """ Tests for the code_owner monitoring utility functions """ + def setUp(self): + super().setUp() + clear_cached_mappings() + @override_settings(CODE_OWNER_MAPPINGS=None) def test_is_config_loaded_with_no_config(self): - with patch( - 'edx_django_utils.monitoring.code_owner.utils._PATH_TO_CODE_OWNER_MAPPINGS', - _process_code_owner_mappings() - ): - self.assertFalse(is_code_owner_mappings_configured(), "Mappings should not be configured.") + 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( - 'edx_django_utils.monitoring.code_owner.utils._PATH_TO_CODE_OWNER_MAPPINGS', - _process_code_owner_mappings() - ): - self.assertTrue(is_code_owner_mappings_configured(), "Mappings should be configured.") + 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( - 'edx_django_utils.monitoring.code_owner.utils._PATH_TO_CODE_OWNER_MAPPINGS', - _process_code_owner_mappings() - ): - self.assertTrue(is_code_owner_mappings_configured(), "Although invalid, mappings should be configured.") + self.assertTrue(is_code_owner_mappings_configured(), "Although invalid, mappings should be configured.") @override_settings(CODE_OWNER_MAPPINGS={ 'team-red': [ @@ -67,22 +59,21 @@ def test_is_config_loaded_with_invalid_dict(self): ) @ddt.unpack def test_code_owner_mapping_hits_and_misses(self, module, expected_owner): - with patch( - 'edx_django_utils.monitoring.code_owner.utils._PATH_TO_CODE_OWNER_MAPPINGS', - _process_code_owner_mappings() - ): - actual_owner = get_code_owner_from_module(module) - self.assertEqual(expected_owner, actual_owner) + 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( - 'edx_django_utils.monitoring.code_owner.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') + @patch('edx_django_utils.monitoring.code_owner.utils.log') + def test_code_owner_mapping_with_invalid_dict(self, mock_logger): + self.assertTrue(is_code_owner_mappings_configured(), "Although invalid, mappings should be configured.") + with self.assertRaises(AssertionError): + get_code_owner_from_module('xblock') + mock_logger.exception.assert_called_with( + 'Error processing code_owner_mappings.', + ) + + def test_code_owner_mapping_with_no_settings(self): + self.assertIsNone(get_code_owner_from_module('xblock')) def test_mapping_performance(self): code_owner_mappings = { @@ -93,14 +84,10 @@ def test_mapping_performance(self): path = 'openedx.core.djangoapps.{}'.format(n) code_owner_mappings['team-red'].append(path) with override_settings(CODE_OWNER_MAPPINGS=code_owner_mappings): - with patch( - 'edx_django_utils.monitoring.code_owner.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.assertLess(average_time, 0.0005, 'Mapping takes {}s which is too slow.'.format(average_time)) + 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.assertLess(average_time, 0.0005, 'Mapping takes {}s which is too slow.'.format(average_time)) diff --git a/edx_django_utils/monitoring/code_owner/utils.py b/edx_django_utils/monitoring/code_owner/utils.py index 6c9da070..6b10c9ad 100644 --- a/edx_django_utils/monitoring/code_owner/utils.py +++ b/edx_django_utils/monitoring/code_owner/utils.py @@ -18,39 +18,45 @@ def get_code_owner_from_module(module): this lookup would match on 'openedx.features.discounts' before 'openedx.features', because the former is more specific. - Uses CODE_OWNER_MAPPINGS Django Setting as detailed in: + See how to: https://github.com/edx/edx-django-utils/blob/master/edx_django_utils/monitoring/docs/how_tos/add_code_owner_custom_attribute_to_an_ida.rst """ - assert _PATH_TO_CODE_OWNER_MAPPINGS != _INVALID_CODE_OWNER_MAPPING,\ - 'CODE_OWNER_MAPPINGS django setting set with invalid configuration. See logs for details.' + code_owner_mappings = get_code_owner_mappings() + if code_owner_mappings is None: + return None 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] + if partial_path in code_owner_mappings: + code_owner = 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. + Returns True if code owner mappings were configured, and False otherwise. """ - return bool(_PATH_TO_CODE_OWNER_MAPPINGS) + return isinstance(get_code_owner_mappings(), dict) + + +# cached lookup table for code owner given a module path. +# do not access this directly, but instead use get_code_owner_mappings. +_PATH_TO_CODE_OWNER_MAPPINGS = None -def _process_code_owner_mappings(): +def get_code_owner_mappings(): """ - Processes the CODE_OWNER_MAPPINGS Django Setting and returns a dict optimized + Returns the contents of the CODE_OWNER_MAPPINGS Django Setting, processed 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. + (dict): dict mapping modules to code owners, or None if there are no + configured mappings, or an empty dict if there is an error processing + the setting. Example return value:: @@ -61,48 +67,50 @@ def _process_code_owner_mappings(): } """ - _CODE_OWNER_MAPPINGS = getattr(settings, 'CODE_OWNER_MAPPINGS', None) - if not _CODE_OWNER_MAPPINGS: + global _PATH_TO_CODE_OWNER_MAPPINGS + + # Return cached processed mappings if already processed + if _PATH_TO_CODE_OWNER_MAPPINGS is not None: + return _PATH_TO_CODE_OWNER_MAPPINGS + + # Uses temporary variable to build mappings to avoid multi-threading issue with a partially + # processed map. Worst case, it is processed more than once at start-up. + path_to_code_owner_mapping = {} + + # .. setting_name: CODE_OWNER_MAPPINGS + # .. setting_default: None + # .. setting_description: Used for monitoring and reporting of ownership. Use a + # dict with keys of code owner name and value as a list of dotted path + # module names owned by the code owner. + code_owner_mappings = getattr(settings, 'CODE_OWNER_MAPPINGS', None) + if code_owner_mappings is None: return None try: - path_to_code_owner_mappings = {} - for code_owner in _CODE_OWNER_MAPPINGS: - path_list = _CODE_OWNER_MAPPINGS[code_owner] + 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 + path_to_code_owner_mapping[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 + path_to_code_owner_mapping[path_without_prefix] = code_owner except Exception as e: # pylint: disable=broad-except log.exception('Error processing code_owner_mappings. {}'.format(e)) - # errors should be unlikely due do scripting the setting values. - # this will also trigger an error custom attribute 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 attributes 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 -# TODO: Update annotations. This is more of a non-toggle setting than a toggle setting. -_CODE_OWNER_MAPPINGS = None - -# TODO: Remove this LMS spcific configuration by replacing with a Django Setting named + + _PATH_TO_CODE_OWNER_MAPPINGS = path_to_code_owner_mapping + return _PATH_TO_CODE_OWNER_MAPPINGS + + +def clear_cached_mappings(): + """ + Clears the cached path to code owner mappings. Useful for testing. + """ + global _PATH_TO_CODE_OWNER_MAPPINGS + _PATH_TO_CODE_OWNER_MAPPINGS = None + + +# TODO: Remove this LMS specific configuration by replacing with a Django Setting named # CODE_OWNER_OPTIONAL_MODULE_PREFIXES that takes a list of module prefixes (without the final period). _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() diff --git a/edx_django_utils/monitoring/docs/how_tos/using_custom_attributes.rst b/edx_django_utils/monitoring/docs/how_tos/using_custom_attributes.rst index 8113264b..96ba12e0 100644 --- a/edx_django_utils/monitoring/docs/how_tos/using_custom_attributes.rst +++ b/edx_django_utils/monitoring/docs/how_tos/using_custom_attributes.rst @@ -8,11 +8,11 @@ Enhanced Monitoring and Custom Attributes What is a Custom Attribute? --------------------------- -A custom attribute is a name/value pair that you define, which will be associated with a request and available in various places in your monitoring software. For example, in Open edX, there is a ``request_user_id`` made available to all requests using the `RequestMetricsMiddleware`_. Note: ``RequestMetricsMiddleware`` should be renamed to ``RequestCustomAttributesMiddleware``. +A custom attribute is a name/value pair that you define, which will be associated with a request and available in various places in your monitoring software. For example, in Open edX, there is a ``request_user_id`` made available to all requests using the `RequestCustomAttributesMiddleware`_. If you are using NewRelic, you can search for PageViews, Transactions, TransactionErrors, etc. using NewRelic Insights, either with custom queries using the Data Explorer (searching "Events", not Metrics). They will also appear in other places, like when viewing an error in NewRelic APM. -.. RequestMetricsMiddleware: https://github.com/edx/edx-drf-extensions/blob/master/edx_rest_framework_extensions/middleware.py#L12-L39 +.. RequestCustomAttributesMiddleware: https://github.com/edx/edx-drf-extensions/blob/e3186555ab234a1a95453eb5ead2420013ddb2f2/edx_rest_framework_extensions/middleware.py#L14-L41 Coding New Custom Attributes ----------------------------