From 2ebb3a61d6bbf6ba7fcbbd6423df8fe01e302693 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Wed, 2 Sep 2020 14:05:57 -0400 Subject: [PATCH] ARCHBOM-1490: rename "custom metric" to "custom attribute" (#59) Rename "custom metric" to "custom attribute" throughout the monitoring library. This decision can be read about in the ADR 0002-custom-monitoring-language.rst. The following have been deprecated: * set_custom_metric (use set_custom_attribute) * set_custom_metric_for_course_key (use set_custom_attribute_for_course_key) * MonitoringCustomMetricsMiddleware (use CachedCustomMonitoringMiddleware) * CachedCustomMonitoringMiddleware.accumulate_metric (use CachedCustomMonitoringMiddleware.accumulate_attribute) * CodeOwnerMetricMiddleware (use CodeOwnerMonitoringMiddleware) ARCHBOM-1490 --- CHANGELOG.rst | 21 ++++ edx_django_utils/__init__.py | 2 +- edx_django_utils/monitoring/README.rst | 6 +- edx_django_utils/monitoring/__init__.py | 10 +- .../monitoring/code_owner/middleware.py | 75 ++++++++------ .../code_owner/tests/test_middleware.py | 98 +++++++++---------- .../monitoring/code_owner/utils.py | 6 +- .../0001-monitoring-by-code-owner.rst | 6 +- .../0002-custom-monitoring-language.rst | 38 +++++++ ..._code_owner_custom_attribute_to_an_ida.rst | 46 +++++++++ ...add_code_owner_custom_metric_to_an_ida.rst | 46 --------- ...etrics.rst => using_custom_attributes.rst} | 56 +++++------ edx_django_utils/monitoring/middleware.py | 83 +++++++++++----- .../monitoring/tests/test_monitoring.py | 33 ++++--- edx_django_utils/monitoring/utils.py | 69 ++++++++----- 15 files changed, 369 insertions(+), 226 deletions(-) create mode 100644 edx_django_utils/monitoring/docs/decisions/0002-custom-monitoring-language.rst create mode 100644 edx_django_utils/monitoring/docs/how_tos/add_code_owner_custom_attribute_to_an_ida.rst delete mode 100644 edx_django_utils/monitoring/docs/how_tos/add_code_owner_custom_metric_to_an_ida.rst rename edx_django_utils/monitoring/docs/how_tos/{using_custom_metrics.rst => using_custom_attributes.rst} (61%) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 1136a5a8..a3f90067 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -15,6 +15,23 @@ Change Log Unreleased ~~~~~~~~~~ +[3.8.0] - 2020-08-31 +~~~~~~~~~~~~~~~~~~~~ + +Updated +_______ + +* Renamed "custom metric" to "custom attribute" throughout the monitoring library. This decision can be read about in the ADR 0002-custom-monitoring-language.rst. The following have been deprecated: + + * set_custom_metric (use set_custom_attribute) + * set_custom_metrics_for_course_key (use set_custom_attributes_for_course_key) + * MonitoringCustomMetricsMiddleware (use CachedCustomMonitoringMiddleware) + * CachedCustomMonitoringMiddleware.accumulate_metric (use CachedCustomMonitoringMiddleware.accumulate_attribute) + + * This wasn't meant to be used publicly, but was deprecated just in case. + + * CodeOwnerMetricMiddleware (use CodeOwnerMonitoringMiddleware) + [3.7.4] - 2020-08-29 ~~~~~~~~~~~~~~~~~~~~ @@ -23,10 +40,14 @@ Unreleased [3.7.3] - 2020-08-12 ~~~~~~~~~~~~~~~~~~~~ +Updated +_______ + * Upgrade psutil to latest version [3.7.2] - 2020-08-10 ~~~~~~~~~~~~~~~~~~~~ + Updated _______ diff --git a/edx_django_utils/__init__.py b/edx_django_utils/__init__.py index 11337f6d..a06194ce 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.7.4" +__version__ = "3.8.0" default_app_config = ( "edx_django_utils.apps.EdxDjangoUtilsConfig" diff --git a/edx_django_utils/monitoring/README.rst b/edx_django_utils/monitoring/README.rst index 48cfebb1..ce51293d 100644 --- a/edx_django_utils/monitoring/README.rst +++ b/edx_django_utils/monitoring/README.rst @@ -22,8 +22,8 @@ Here is how you add the middleware: # Generate code ownership metrics. Keep this immediately after RequestCacheMiddleware. ... # Monitoring middleware must come after RequestCacheMiddleware - 'edx_django_utils.monitoring.code_owner.middleware.CodeOwnerMetricMiddleware', - 'edx_django_utils.monitoring.middleware.MonitoringCustomMetricsMiddleware', + 'edx_django_utils.monitoring.code_owner.middleware.CodeOwnerMonitoringMiddleware', + 'edx_django_utils.monitoring.middleware.CachedCustomMonitoringMiddleware', 'edx_django_utils.monitoring.middleware.MonitoringMemoryMiddleware', ) @@ -35,4 +35,4 @@ In addition to adding the MonitoringMemoryMiddleware, you will need to enable a Code Owner Custom Metric ------------------------ -See docstrings for ``CodeOwnerMetricMiddleware`` for configuring the ``code_owner`` custom metric for your IDA. +See docstrings for ``CodeOwnerMonitoringMiddleware`` for configuring the ``code_owner`` custom attribute for your IDA. diff --git a/edx_django_utils/monitoring/__init__.py b/edx_django_utils/monitoring/__init__.py index 31271bba..91b52a4a 100644 --- a/edx_django_utils/monitoring/__init__.py +++ b/edx_django_utils/monitoring/__init__.py @@ -4,4 +4,12 @@ See README.rst for details. """ from .transactions import function_trace, get_current_transaction, ignore_transaction, set_monitoring_transaction_name -from .utils import accumulate, increment, set_custom_metric, set_custom_metrics_for_course_key +# "set_custom_metric*" methods are deprecated +from .utils import ( + accumulate, + increment, + set_custom_attribute, + set_custom_attributes_for_course_key, + set_custom_metric, + set_custom_metrics_for_course_key +) diff --git a/edx_django_utils/monitoring/code_owner/middleware.py b/edx_django_utils/monitoring/code_owner/middleware.py index 893b2505..7660e55f 100644 --- a/edx_django_utils/monitoring/code_owner/middleware.py +++ b/edx_django_utils/monitoring/code_owner/middleware.py @@ -1,25 +1,26 @@ """ -Middleware for code_owner custom metric +Middleware for code_owner custom attribute """ import logging +import warnings from django.urls import resolve -from edx_django_utils.monitoring import get_current_transaction, set_custom_metric +from edx_django_utils.monitoring import get_current_transaction, set_custom_attribute from .utils import get_code_owner_from_module, is_code_owner_mappings_configured log = logging.getLogger(__name__) -class CodeOwnerMetricMiddleware: +class CodeOwnerMonitoringMiddleware: """ - Django middleware object to set custom metrics for the owner of each view. + Django middleware object to set custom attributes for the owner of each view. For instructions on usage, see: - https://github.com/edx/edx-django-utils/blob/master/edx_django_utils/monitoring/docs/how_tos/add_code_owner_custom_metric_to_an_ida.rst + 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 - Custom metrics set: + Custom attributes 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. - code_owner_path_error: The error mapping by path, if code_owner isn't found in other ways. @@ -35,58 +36,58 @@ def __init__(self, get_response): def __call__(self, request): response = self.get_response(request) - self._set_code_owner_metric(request) + self._set_code_owner_attribute(request) return response def process_exception(self, request, exception): - self._set_code_owner_metric(request) + self._set_code_owner_attribute(request) - def _set_code_owner_metric(self, request): + def _set_code_owner_attribute(self, request): """ - Sets the code_owner custom metric, as well as several supporting custom metrics. + Sets the code_owner custom attribute, as well as several supporting custom attributes. - See CodeOwnerMetricMiddleware docstring for a complete list of metrics. + See CodeOwnerMonitoringMiddleware docstring for a complete list of attributes. """ - code_owner, path_error = self._set_code_owner_metric_from_path(request) + code_owner, path_error = self._set_code_owner_attribute_from_path(request) if code_owner: - set_custom_metric('code_owner', code_owner) + set_custom_attribute('code_owner', code_owner) return if not path_error: # module found, but mapping wasn't configured - code_owner = self._set_code_owner_metric_catch_all() + code_owner = self._set_code_owner_attribute_catch_all() if code_owner: - set_custom_metric('code_owner', code_owner) + set_custom_attribute('code_owner', code_owner) return - code_owner, transaction_error = self._set_code_owner_metric_from_current_transaction(request) + code_owner, transaction_error = self._set_code_owner_attribute_from_current_transaction(request) if code_owner: - set_custom_metric('code_owner', code_owner) + set_custom_attribute('code_owner', code_owner) return if not transaction_error: # transaction name found, but mapping wasn't configured - code_owner = self._set_code_owner_metric_catch_all() + code_owner = self._set_code_owner_attribute_catch_all() if code_owner: - set_custom_metric('code_owner', code_owner) + set_custom_attribute('code_owner', code_owner) return - code_owner = self._set_code_owner_metric_catch_all() + code_owner = self._set_code_owner_attribute_catch_all() if code_owner: - set_custom_metric('code_owner', code_owner) + set_custom_attribute('code_owner', code_owner) return # only report errors if code_owner couldn't be found, including catch-all if path_error: - set_custom_metric('code_owner_path_error', path_error) + set_custom_attribute('code_owner_path_error', path_error) if transaction_error: - set_custom_metric('code_owner_transaction_error', transaction_error) + set_custom_attribute('code_owner_transaction_error', transaction_error) - def _set_code_owner_metric_from_path(self, request): + def _set_code_owner_attribute_from_path(self, request): """ - Uses the request path to find the view_func and then sets code owner metrics based on the view. + Uses the request path to find the view_func and then sets code owner attributes based on the view. Side-effects: - Sets code_owner_path_module custom metric, used to determine code_owner + Sets code_owner_path_module custom attribute, used to determine code_owner Returns: (str, str): (code_owner, error_message), where at least one of these should be None @@ -98,18 +99,18 @@ def _set_code_owner_metric_from_path(self, request): try: view_func, _, _ = resolve(request.path) path_module = view_func.__module__ - set_custom_metric('code_owner_path_module', path_module) + set_custom_attribute('code_owner_path_module', path_module) code_owner = get_code_owner_from_module(path_module) return code_owner, None except Exception as e: # pylint: disable=broad-except return None, str(e) - def _set_code_owner_metric_from_current_transaction(self, request): + def _set_code_owner_attribute_from_current_transaction(self, request): """ - Uses the current transaction name to set the code owner metric. + Uses the current transaction name to set the code owner attribute. Side-effects: - Sets code_owner_transaction_name custom metric, used to determine code_owner + Sets code_owner_transaction_name custom attribute, used to determine code_owner Returns: (str, str): (code_owner, error_message), where at least one of these should be None @@ -123,14 +124,14 @@ def _set_code_owner_metric_from_current_transaction(self, request): transaction_name = get_current_transaction().name if not transaction_name: return None, 'No current transaction name found.' - set_custom_metric('code_owner_transaction_name', transaction_name) + set_custom_attribute('code_owner_transaction_name', transaction_name) module_name = transaction_name.split(':')[0] code_owner = get_code_owner_from_module(module_name) return code_owner, None except Exception as e: # pylint: disable=broad-except return None, str(e) - def _set_code_owner_metric_catch_all(self): + def _set_code_owner_attribute_catch_all(self): """ If the catch-all module "*" is configured, return the code_owner. @@ -146,3 +147,13 @@ def _set_code_owner_metric_catch_all(self): return code_owner except Exception: # pylint: disable=broad-except; #pragma: no cover return None + + +class CodeOwnerMetricMiddleware(CodeOwnerMonitoringMiddleware): + """ + Deprecated class for handling middleware. Class has been renamed to CodeOwnerMonitoringMiddleware. + """ + def __init__(self, *args, **kwargs): # pragma: no cover + super(CodeOwnerMetricMiddleware, self).__init__(*args, **kwargs) + msg = "Use 'CodeOwnerMonitoringMiddleware' in place of 'CodeOwnerMetricMiddleware'." + warnings.warn(msg, DeprecationWarning) 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 33a6b76a..9bce1fc9 100644 --- a/edx_django_utils/monitoring/code_owner/tests/test_middleware.py +++ b/edx_django_utils/monitoring/code_owner/tests/test_middleware.py @@ -10,7 +10,7 @@ from django.views.generic import View from mock import Mock, call, patch -from edx_django_utils.monitoring.code_owner.middleware import CodeOwnerMetricMiddleware +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 @@ -35,7 +35,7 @@ class CodeOwnerMetricMiddlewareTests(TestCase): def setUp(self): super().setUp() self.mock_get_response = Mock() - self.middleware = CodeOwnerMetricMiddleware(self.mock_get_response) + self.middleware = CodeOwnerMonitoringMiddleware(self.mock_get_response) def test_init(self): self.assertEqual(self.middleware.get_response, self.mock_get_response) @@ -54,14 +54,14 @@ def test_request_call(self): CODE_OWNER_MAPPINGS={'team-red': ['edx_django_utils.monitoring.code_owner.tests.mock_views']}, ROOT_URLCONF=__name__, ) - @patch('edx_django_utils.monitoring.code_owner.middleware.set_custom_metric') + @patch('edx_django_utils.monitoring.code_owner.middleware.set_custom_attribute') @ddt.data( ('/middleware-test/', None), ('/test/', 'team-red'), ) @ddt.unpack def test_code_owner_path_mapping_hits_and_misses( - self, request_path, expected_owner, mock_set_custom_metric + self, request_path, expected_owner, mock_set_custom_attribute ): with patch( 'edx_django_utils.monitoring.code_owner.utils._PATH_TO_CODE_OWNER_MAPPINGS', @@ -70,14 +70,14 @@ def test_code_owner_path_mapping_hits_and_misses( request = RequestFactory().get(request_path) self.middleware(request) expected_path_module = self._REQUEST_PATH_TO_MODULE_PATH[request_path] - self._assert_code_owner_custom_metrics( - mock_set_custom_metric, expected_code_owner=expected_owner, path_module=expected_path_module + self._assert_code_owner_custom_attributes( + mock_set_custom_attribute, expected_code_owner=expected_owner, path_module=expected_path_module ) - mock_set_custom_metric.reset_mock() + mock_set_custom_attribute.reset_mock() self.middleware.process_exception(request, None) - self._assert_code_owner_custom_metrics( - mock_set_custom_metric, expected_code_owner=expected_owner, path_module=expected_path_module + self._assert_code_owner_custom_attributes( + mock_set_custom_attribute, expected_code_owner=expected_owner, path_module=expected_path_module ) @override_settings( @@ -87,14 +87,14 @@ def test_code_owner_path_mapping_hits_and_misses( }, ROOT_URLCONF=__name__, ) - @patch('edx_django_utils.monitoring.code_owner.middleware.set_custom_metric') + @patch('edx_django_utils.monitoring.code_owner.middleware.set_custom_attribute') @ddt.data( ('/middleware-test/', 'team-blue'), ('/test/', 'team-red'), ) @ddt.unpack def test_code_owner_path_mapping_with_catch_all( - self, request_path, expected_owner, mock_set_custom_metric + self, request_path, expected_owner, mock_set_custom_attribute ): with patch( 'edx_django_utils.monitoring.code_owner.utils._PATH_TO_CODE_OWNER_MAPPINGS', @@ -103,15 +103,15 @@ def test_code_owner_path_mapping_with_catch_all( request = RequestFactory().get(request_path) self.middleware(request) expected_path_module = self._REQUEST_PATH_TO_MODULE_PATH[request_path] - self._assert_code_owner_custom_metrics( - mock_set_custom_metric, expected_code_owner=expected_owner, path_module=expected_path_module + 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']}, ROOT_URLCONF=__name__, ) - @patch('edx_django_utils.monitoring.code_owner.middleware.set_custom_metric') + @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), @@ -119,7 +119,7 @@ def test_code_owner_path_mapping_with_catch_all( ) @ddt.unpack def test_code_owner_transaction_mapping_hits_and_misses( - self, transaction_name, expected_owner, mock_newrelic_agent, mock_set_custom_metric + self, transaction_name, expected_owner, mock_newrelic_agent, mock_set_custom_attribute ): mock_newrelic_agent.current_transaction().name = transaction_name with patch( @@ -128,14 +128,14 @@ def test_code_owner_transaction_mapping_hits_and_misses( ): request = RequestFactory().get('/bad/path/') self.middleware(request) - self._assert_code_owner_custom_metrics( - mock_set_custom_metric, expected_code_owner=expected_owner, transaction_name=transaction_name + self._assert_code_owner_custom_attributes( + mock_set_custom_attribute, expected_code_owner=expected_owner, transaction_name=transaction_name ) - mock_set_custom_metric.reset_mock() + mock_set_custom_attribute.reset_mock() self.middleware.process_exception(request, None) - self._assert_code_owner_custom_metrics( - mock_set_custom_metric, expected_code_owner=expected_owner, transaction_name=transaction_name + self._assert_code_owner_custom_attributes( + mock_set_custom_attribute, expected_code_owner=expected_owner, transaction_name=transaction_name ) @override_settings( @@ -145,7 +145,7 @@ def test_code_owner_transaction_mapping_hits_and_misses( }, ROOT_URLCONF=__name__, ) - @patch('edx_django_utils.monitoring.code_owner.middleware.set_custom_metric') + @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'), @@ -153,7 +153,7 @@ def test_code_owner_transaction_mapping_hits_and_misses( ) @ddt.unpack def test_code_owner_transaction_mapping__with_catch_all( - self, transaction_name, expected_owner, mock_newrelic_agent, mock_set_custom_metric + self, transaction_name, expected_owner, mock_newrelic_agent, mock_set_custom_attribute ): mock_newrelic_agent.current_transaction().name = transaction_name with patch( @@ -162,17 +162,17 @@ def test_code_owner_transaction_mapping__with_catch_all( ): request = RequestFactory().get('/bad/path/') self.middleware(request) - self._assert_code_owner_custom_metrics( - mock_set_custom_metric, expected_code_owner=expected_owner, transaction_name=transaction_name + self._assert_code_owner_custom_attributes( + mock_set_custom_attribute, expected_code_owner=expected_owner, transaction_name=transaction_name ) @override_settings( CODE_OWNER_MAPPINGS={'team-red': ['edx_django_utils.monitoring.code_owner.tests.mock_views']}, ROOT_URLCONF=__name__, ) - @patch('edx_django_utils.monitoring.code_owner.middleware.set_custom_metric') + @patch('edx_django_utils.monitoring.code_owner.middleware.set_custom_attribute') @patch('newrelic.agent') - def test_code_owner_transaction_mapping_error(self, mock_newrelic_agent, mock_set_custom_metric): + 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', @@ -180,50 +180,50 @@ def test_code_owner_transaction_mapping_error(self, mock_newrelic_agent, mock_se ): request = RequestFactory().get('/bad/path/') self.middleware(request) - self._assert_code_owner_custom_metrics( - mock_set_custom_metric, has_path_error=True, has_transaction_error=True + 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_metric') - def test_code_owner_no_mappings(self, mock_set_custom_metric): + @patch('edx_django_utils.monitoring.code_owner.middleware.set_custom_attribute') + def test_code_owner_no_mappings(self, mock_set_custom_attribute): request = RequestFactory().get('/test/') self.middleware(request) - mock_set_custom_metric.assert_not_called() + 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_metric') - def test_no_resolver_for_path_and_no_transaction(self, mock_set_custom_metric): + @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_metrics( - mock_set_custom_metric, has_path_error=True, has_transaction_error=True + 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_metric') - def test_catch_all_instead_of_errors(self, mock_set_custom_metric): + @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_metrics(mock_set_custom_metric, expected_code_owner='team-red') + self._assert_code_owner_custom_attributes(mock_set_custom_attribute, expected_code_owner='team-red') @override_settings( CODE_OWNER_MAPPINGS=['invalid_setting_as_list'], ROOT_URLCONF=__name__, ) - @patch('edx_django_utils.monitoring.code_owner.middleware.set_custom_metric') - def test_load_config_with_invalid_dict(self, mock_set_custom_metric): + @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() @@ -231,15 +231,15 @@ def test_load_config_with_invalid_dict(self, mock_set_custom_metric): request = RequestFactory().get('/test/') self.middleware(request) expected_path_module = self._REQUEST_PATH_TO_MODULE_PATH['/test/'] - self._assert_code_owner_custom_metrics( - mock_set_custom_metric, path_module=expected_path_module, + self._assert_code_owner_custom_attributes( + mock_set_custom_attribute, path_module=expected_path_module, has_path_error=True, has_transaction_error=True ) - def _assert_code_owner_custom_metrics(self, mock_set_custom_metric, expected_code_owner=None, - path_module=None, has_path_error=False, - transaction_name=None, has_transaction_error=False): - """ Performs a set of assertions around having set the proper custom metrics. """ + def _assert_code_owner_custom_attributes(self, mock_set_custom_attribute, expected_code_owner=None, + path_module=None, has_path_error=False, + transaction_name=None, has_transaction_error=False): + """ Performs a set of assertions around having set the proper custom attributes. """ call_list = [] if expected_code_owner: call_list.append(call('code_owner', expected_code_owner)) @@ -251,8 +251,8 @@ def _assert_code_owner_custom_metrics(self, mock_set_custom_metric, expected_cod call_list.append(call('code_owner_transaction_name', transaction_name)) if has_transaction_error: call_list.append(call('code_owner_transaction_error', ANY)) - mock_set_custom_metric.assert_has_calls(call_list, any_order=True) + mock_set_custom_attribute.assert_has_calls(call_list, any_order=True) 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) + len(mock_set_custom_attribute.call_args_list), len(call_list), + 'Expected calls {} vs actual calls {}'.format(call_list, mock_set_custom_attribute.call_args_list) ) diff --git a/edx_django_utils/monitoring/code_owner/utils.py b/edx_django_utils/monitoring/code_owner/utils.py index 85ee82fc..6c9da070 100644 --- a/edx_django_utils/monitoring/code_owner/utils.py +++ b/edx_django_utils/monitoring/code_owner/utils.py @@ -19,7 +19,7 @@ def get_code_owner_from_module(module): 'openedx.features', because the former is more specific. Uses CODE_OWNER_MAPPINGS Django Setting as detailed in: - https://github.com/edx/edx-django-utils/blob/master/edx_django_utils/monitoring/docs/how_tos/add_code_owner_custom_metric_to_an_ida.rst + 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,\ @@ -81,14 +81,14 @@ def _process_code_owner_mappings(): 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 metric that can be alerted on. + # 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 metrics for owner. Dict with keys of code owner and value as +# .. 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 diff --git a/edx_django_utils/monitoring/docs/decisions/0001-monitoring-by-code-owner.rst b/edx_django_utils/monitoring/docs/decisions/0001-monitoring-by-code-owner.rst index 5b2fd1ce..0ea25517 100644 --- a/edx_django_utils/monitoring/docs/decisions/0001-monitoring-by-code-owner.rst +++ b/edx_django_utils/monitoring/docs/decisions/0001-monitoring-by-code-owner.rst @@ -9,7 +9,7 @@ Accepted Context ======= -We originally implemented the "code_owner" custom metric in edx-platform for split-ownership of the LMS. See the original `ADR in edx-platform for monitoring by code owner`_. +We originally implemented the "code_owner" custom attribute in edx-platform for split-ownership of the LMS. See the original `ADR in edx-platform for monitoring by code owner`_. Owners wanted to be able to see transactions that they owned, in any IDA. @@ -18,13 +18,13 @@ Owners wanted to be able to see transactions that they owned, in any IDA. Decision ======== -We will move the "code_owner" custom metric code to these shared monitoring utilities so it is available for all IDAs. +We will move the "code_owner" custom attribute code to these shared monitoring utilities so it is available for all IDAs. The ability to add a catch-all configuration if there are no other matches will also be added in follow-up work. Consequences ============ -IDA owners will be able to add middleware and a Django Setting to have the same "code_owner" metric available across all IDAs that are owned. +IDA owners will be able to add middleware and a Django Setting to have the same "code_owner" attribute available across all IDAs that are owned. At this time, in the case of an IDA with split-ownership, maintenance of the Django Setting is still manual. In other words, new paths with new owners will needed to be added to the setting. Otherwise, the catch-all (if configured) will be marked as the code owner. diff --git a/edx_django_utils/monitoring/docs/decisions/0002-custom-monitoring-language.rst b/edx_django_utils/monitoring/docs/decisions/0002-custom-monitoring-language.rst new file mode 100644 index 00000000..707edd48 --- /dev/null +++ b/edx_django_utils/monitoring/docs/decisions/0002-custom-monitoring-language.rst @@ -0,0 +1,38 @@ +Custom Monitoring Language +========================== + +Status +------ + +Accepted + +Context +------- + +Although the monitoring library is meant to encapsulate monitoring functionality, our primary implementation works with New Relic. New Relic has the concept of "events" (with "attributes") and "metrics". However, this library was original written using the term "metric" when referring to New Relic "attributes", and not for New Relic "metrics". + +Decision +-------- + +When referring to New Relic event attributes, we will use the term "attribute", and not the word "metric". We will continue to use the term "custom" for attributes that we define, rather than those that come standard from New Relic. Note: New Relic uses the terms "attribute" and "parameter" interchangeably, but the term "attribute" is more popular in the UI. + +Additionally, we will use the term "custom monitoring" as a more general term for any of our custom additions for monitoring purposes, whether this refers to "custom events", "custom attributes", or "custom metrics". + +Consequences +------------ + +* Switching from "metric" to "attribute" enables us to save the term "metric" for when and if we encapsulate methods that actually write New Relic metrics. +* Because many methods will need to be renamed, we will deprecate the older methods and classes to maintain backward-compatibility until we retire all uses of the old names. + +Resources +--------- + +* `New Relic custom events`_ + + * `New Relic custom attributes`_ + +* `New Relic custom metrics`_ + +.. _New Relic custom events: https://docs.newrelic.com/docs/insights/event-data-sources/custom-events +.. _New Relic custom attributes: https://docs.newrelic.com/docs/insights/event-data-sources/custom-events/new-relic-apm-report-custom-attributes +.. _New Relic custom metrics: https://docs.newrelic.com/docs/agents/manage-apm-agents/agent-data/collect-custom-metrics diff --git a/edx_django_utils/monitoring/docs/how_tos/add_code_owner_custom_attribute_to_an_ida.rst b/edx_django_utils/monitoring/docs/how_tos/add_code_owner_custom_attribute_to_an_ida.rst new file mode 100644 index 00000000..c6ffc627 --- /dev/null +++ b/edx_django_utils/monitoring/docs/how_tos/add_code_owner_custom_attribute_to_an_ida.rst @@ -0,0 +1,46 @@ +Add Code_Owner Custom Attribute to an IDA +========================================= + +.. contents:: + :local: + :depth: 2 + +What is the code_owner custom attribute? +---------------------------------------- + +The code_owner custom attribute can be used to create custom dashboards and alerts for monitoring the things that you own. It was originally introduced for the LMS, as is described in this `ADR on monitoring by code owner`_. + +You can now easily add this same attribute to any IDA so that your dashboards and alerts can work across multiple IDAs at once. + +If you want to know about custom attributes in general, see: using_custom_attributes.rst. + +.. _ADR on monitoring by code owner: https://github.com/edx/edx-platform/blob/master/lms/djangoapps/monitoring/docs/decisions/0001-monitoring-by-code-owner.rst + +Setting up the Middleware +------------------------- + +You simply need to add ``edx_django_utils.monitoring.code_owner.middleware.CodeOwnerMonitoringMiddleware`` as described in the README to make this functionality available. Then it is ready to be configured. + +Configuring your app settings +----------------------------- + +Once the Middleware is made available, simply set the Django Setting ``CODE_OWNER_MAPPINGS`` appropriately. + +The following example shows how you can include an optional config for a catch-all using ``'*'``. Although you might expect this example to use Python, it is intentionally illustrated in YAML because the catch-all requires special care in YAML. + +:: + + # YAML format of example CODE_OWNER_MAPPINGS + CODE_OWNER_MAPPINGS: + team-red: + - xblock_django + - openedx.core.djangoapps.xblock + team-blue: + - '*' # IMPORTANT: you must surround * with quotes in yml + +How to find and fix code_owner mappings +--------------------------------------- + +If you are missing the `code_owner` custom attribute on a particular Transaction or Error, or if `code_owner` is matching the catch-all, but you want to add a more specific mapping, you can use the other `code_owner supporting attributes`_ to determine what the appropriate mappings should be. + +.. _code_owner supporting attributes: https://github.com/edx/edx-django-utils/blob/7db8301af21760f8bca188b3c6c95a8ae873baf7/edx_django_utils/monitoring/code_owner/middleware.py#L28-L34 diff --git a/edx_django_utils/monitoring/docs/how_tos/add_code_owner_custom_metric_to_an_ida.rst b/edx_django_utils/monitoring/docs/how_tos/add_code_owner_custom_metric_to_an_ida.rst deleted file mode 100644 index 9074c0f6..00000000 --- a/edx_django_utils/monitoring/docs/how_tos/add_code_owner_custom_metric_to_an_ida.rst +++ /dev/null @@ -1,46 +0,0 @@ -Add Code_Owner Custom Metric to an IDA -====================================== - -.. contents:: - :local: - :depth: 2 - -What is the code_owner custom metric? -------------------------------------- - -The code_owner custom metric can be used to create custom dashboards and alerts for monitoring the things that you own. It was originally introduced for the LMS, as is described in this `ADR on monitoring by code owner`_. - -You can now easily add this same metric to any IDA so that your dashboards and alerts can work across multiple IDAs at once. - -If you want to know about custom metrics in general, see: using_custom_metrics.rst. - -.. _ADR on monitoring by code owner: https://github.com/edx/edx-platform/blob/master/lms/djangoapps/monitoring/docs/decisions/0001-monitoring-by-code-owner.rst - -Setting up the Middleware -------------------------- - -You simply need to add ``edx_django_utils.monitoring.code_owner.middleware.CodeOwnerMetricMiddleware`` as described in the README to make this functionality available. Then it is ready to be configured. - -Configuring your app settings ------------------------------ - -Once the Middleware is made available, simply set the Django Setting ``CODE_OWNER_MAPPINGS`` appropriately. - -The following example shows how you can include an optional config for a catch-all using ``'*'``. Although you might expect this example to use Python, it is intentionally illustrated in YAML because the catch-all requires special care in YAML. - -:: - - # YAML format of example CODE_OWNER_MAPPINGS - CODE_OWNER_MAPPINGS: - team-red: - - xblock_django - - openedx.core.djangoapps.xblock - team-blue: - - '*' # IMPORTANT: you must surround * with quotes in yml - -How to find and fix code_owner mappings ---------------------------------------- - -If you are missing the `code_owner` custom metric on a particular Transaction or Error, or if `code_owner` is matching the catch-all, but you want to add a more specific mapping, you can use the other `code_owner supporting metrics`_ to determine what the appropriate mappings should be. - -.. _code_owner supporting metrics: https://github.com/edx/edx-django-utils/blob/7db8301af21760f8bca188b3c6c95a8ae873baf7/edx_django_utils/monitoring/code_owner/middleware.py#L28-L34 diff --git a/edx_django_utils/monitoring/docs/how_tos/using_custom_metrics.rst b/edx_django_utils/monitoring/docs/how_tos/using_custom_attributes.rst similarity index 61% rename from edx_django_utils/monitoring/docs/how_tos/using_custom_metrics.rst rename to edx_django_utils/monitoring/docs/how_tos/using_custom_attributes.rst index 89611a95..8113264b 100644 --- a/edx_django_utils/monitoring/docs/how_tos/using_custom_metrics.rst +++ b/edx_django_utils/monitoring/docs/how_tos/using_custom_attributes.rst @@ -1,27 +1,27 @@ -Enhanced Monitoring and Custom Metrics -====================================== +Enhanced Monitoring and Custom Attributes +========================================= .. contents:: :local: :depth: 2 -What is a Custom Metric? -------------------------- +What is a Custom Attribute? +--------------------------- -A custom metric 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`_. +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``. 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 +.. RequestMetricsMiddleware: https://github.com/edx/edx-drf-extensions/blob/master/edx_rest_framework_extensions/middleware.py#L12-L39 -Coding New Custom Metrics -------------------------- +Coding New Custom Attributes +---------------------------- -After setting up Middleware as detailed in README.rst, adding a new custom metric is as simple as:: +After setting up Middleware as detailed in README.rst, adding a new custom attribute is as simple as:: from openedx.core.djangoapps import monitoring_utils - monitoring_utils.set_custom_metric(name, value) + monitoring_utils.set_custom_attribute(name, value) * Supported values are strings, bools and numbers. * Earlier values will be overwritten if the same name is set multiple times during a request. @@ -35,8 +35,8 @@ For a complete list of the public methods available, see the ``__init__.py`` fil If you require functionality from ``newrelic.agent`` that hasn't yet been abstracted, please add any additional functionality to keep it encapsulated. -Tips for Using Custom Metrics ------------------------------ +Tips for Using Custom Attributes +-------------------------------- * A single name with an enum-like string value is often better than a set of related names with boolean values. For example: @@ -50,53 +50,53 @@ Tips for Using Custom Metrics * is_request_auth_jwt_cookie: True/False, * is_request_auth_session: True/False. -* When possible, set the custom metric for both the positive and negative case. This helps avoid misinterpreting an incorrect query that doesn't turn up a metric. -* Avoid dynamic metric names. More than 2000 metrics (for NewRelic) may cause issues. +* When possible, set the custom attribute for both the positive and negative case. This helps avoid misinterpreting an incorrect query that doesn't turn up an attribute. +* Avoid dynamic attribute names. More than 2000 attributes (for NewRelic) may cause issues. -Common Use Cases for Custom Metrics ------------------------------------ +Common Use Cases for Custom Attributes +-------------------------------------- -Although logging could also be used, with custom metrics, you don't need to worry about blowing out the logs if it is called many times, and you can query it along with all the pre-existing custom metrics. +Although logging could also be used, with custom attributes, you don't need to worry about blowing out the logs if it is called many times, and you can query it along with all the pre-existing custom attributes. -Consider using ``temp_`` as a prefix to any metric name you plan to remove soon after it is added. +Consider using ``temp_`` as a prefix to any attribute name you plan to remove soon after it is added. Deprecating/Removing Code ~~~~~~~~~~~~~~~~~~~~~~~~~ -Ever wonder if some code is actually used or not in Production? Add a temporary custom metric and query it in Production. +Ever wonder if some code is actually used or not in Production? Add a temporary custom attribute and query it in Production. -In addition to `Tips for Using Custom Metrics`_, remember to: +In addition to `Tips for Using Custom Attributes`_, remember to: * Check the ``appName`` to see where code might only be used in a Stage environment. * Check Transaction ``response.status`` to see if code is being used on successful transactions. -* Use ``metric-name is not null`` to find all rows with a value. -* Look at the metric values in addition to getting non-null counts. A value of 'None' is not null, but would be counted. -* If the metric is added to a library, ensure all applications have the upgraded library before deciding whether or not the code is in use. -* Check across multiple environments (``appName`` in NewRelic). For example, for edx.org, you can ensure that the metric is also not in use in Edge, which is sometimes different than Production. +* Use `` is not null`` to find all rows with a value. +* Look at the attribute values in addition to getting non-null counts. A value of 'None' is not null, but would be counted. +* If the attribute is added to a library, ensure all applications have the upgraded library before deciding whether or not the code is in use. +* Check across multiple environments (``appName`` in NewRelic). For example, for edx.org, you can ensure that the attribute is also not in use in Edge, which is sometimes different than Production. Debugging Production Issues ~~~~~~~~~~~~~~~~~~~~~~~~~~~ -Add a temporary custom metric to gain visibility into code running in Production. If using NewRelic, the custom metric will also be associated with the error in the case of debugging errors. +Add a temporary custom attribute to gain visibility into code running in Production. If using NewRelic, the custom attribute will also be associated with the error in the case of debugging errors. If you are seeing unexpected failures in Insights, review Transaction ``request.method`` and ``request_user_agent`` to see if bots are making unexpected calls that are failing. Refactoring Code ~~~~~~~~~~~~~~~~ -Want to dark launch some refactored code and ensure it works before switching to it? Add temporary metrics for the old value, new value, and whether they differ. This will provide quick insight into whether or not there is an issue, and if so, potentially why. +Want to dark launch some refactored code and ensure it works before switching to it? Add temporary attributes for the old value, new value, and whether they differ. This will provide quick insight into whether or not there is an issue, and if so, potentially why. Advanced Cases with Caching ~~~~~~~~~~~~~~~~~~~~~~~~~~~ -If you want to track something like an ordered list built up at different points in a request, you could add the list to a RequestCache, and update both the cached version and the metric with every change. This enables you to look-up the old value and append to it as needed. +If you want to track something like an ordered list built up at different points in a request, you could add the list to a RequestCache, and update both the cached version and the attribute with every change. This enables you to look-up the old value and append to it as needed. For example, this could be used to track Authentication classes or Permission classes used. NewRelic Query Language Examples -------------------------------- -If you are using NewRelic Insights, here are some NewRelic Query Language (NRQL) examples using existing custom metrics. +If you are using NewRelic Insights, here are some NewRelic Query Language (NRQL) examples using existing custom attributes. Simpler NRQL examples ~~~~~~~~~~~~~~~~~~~~~ diff --git a/edx_django_utils/monitoring/middleware.py b/edx_django_utils/monitoring/middleware.py index 835bf720..3c4214cc 100644 --- a/edx_django_utils/monitoring/middleware.py +++ b/edx_django_utils/monitoring/middleware.py @@ -1,15 +1,11 @@ """ -Middleware for handling the storage, aggregation, and reporting of custom -metrics for monitoring. +Middleware for monitoring. -At this time, the custom metrics can only be reported to New Relic. - -This middleware will only call on the newrelic agent if there are any metrics -to report for this request, so it will not incur any processing overhead for -request handlers which do not record custom metrics. +At this time, monitoring details can only be reported to New Relic. """ import logging +import warnings from uuid import uuid4 import psutil @@ -28,39 +24,48 @@ _DEFAULT_NAMESPACE = 'edx_django_utils.monitoring' -_REQUEST_CACHE_NAMESPACE = '{}.custom_metrics'.format(_DEFAULT_NAMESPACE) +_REQUEST_CACHE_NAMESPACE = '{}.custom_attributes'.format(_DEFAULT_NAMESPACE) -class MonitoringCustomMetricsMiddleware(MiddlewareMixin): +class CachedCustomMonitoringMiddleware(MiddlewareMixin): """ - The middleware class for adding custom metrics. + The middleware class for handling cached custom attributes, batch reported + at the end of a request. Make sure to add below the request cache in MIDDLEWARE. + + This middleware will only call on the newrelic agent if there are any attributes + to report for this request, so it will not incur any processing overhead for + request handlers which do not record custom attributes. + + Note: New Relic adds custom attributes to events, which is what is being used here. + """ + REQUIRED_MIDDLEWARE = [ + 'edx_django_utils.cache.middleware.RequestCacheMiddleware', + 'edx_django_utils.monitoring.middleware.CachedCustomMonitoringMiddleware', + ] def __init__(self, *args, **kwargs): - super(MonitoringCustomMetricsMiddleware, self).__init__(*args, **kwargs) + super(CachedCustomMonitoringMiddleware, self).__init__(*args, **kwargs) # checks proper dependency order as well. - _check_middleware_dependencies(self, required_middleware=[ - 'edx_django_utils.cache.middleware.RequestCacheMiddleware', - 'edx_django_utils.monitoring.middleware.MonitoringCustomMetricsMiddleware', - ]) + _check_middleware_dependencies(self, required_middleware=self.REQUIRED_MIDDLEWARE) @classmethod - def _get_metrics_cache(cls): + def _get_attributes_cache(cls): """ Get a reference to the part of the request cache wherein we store New - Relic custom metrics related to the current request. + Relic custom attributes related to the current request. """ return RequestCache(namespace=_REQUEST_CACHE_NAMESPACE) @classmethod - def accumulate_metric(cls, name, value): + def accumulate_attribute(cls, name, value): """ - Accumulate a custom metric (name and value) in the metrics cache. + Accumulate a custom attribute (name and value) in the attributes cache. """ - metrics_cache = cls._get_metrics_cache() - cached_response = metrics_cache.get_cached_response(name) + attributes_cache = cls._get_attributes_cache() + cached_response = attributes_cache.get_cached_response(name) if cached_response.is_found: try: accumulated_value = value + cached_response.value @@ -74,20 +79,29 @@ def accumulate_metric(cls, name, value): return else: accumulated_value = value - metrics_cache.set(name, accumulated_value) + attributes_cache.set(name, accumulated_value) + + @classmethod + def accumulate_metric(cls, name, value): # pragma: no cover + """ + Deprecated method replaced by accumulate_attribute. + """ + msg = "Use 'accumulate_attribute' in place of 'accumulate_metric'." + warnings.warn(msg, DeprecationWarning) + cls.accumulate_attribute(name, value) @classmethod def _batch_report(cls): """ - Report the collected custom metrics to New Relic. + Report the collected custom attributes to New Relic. """ - if not newrelic: + if not newrelic: # pragma: no cover return - metrics_cache = cls._get_metrics_cache() - for key, value in metrics_cache.data.items(): + attributes_cache = cls._get_attributes_cache() + for key, value in attributes_cache.data.items(): _set_custom_attribute(key, value) - # Whether or not there was an exception, report any custom NR metrics that + # Whether or not there was an exception, report any custom NR attributes that # may have been collected. def process_response(self, request, response): @@ -114,6 +128,21 @@ def _set_custom_attribute(key, value): newrelic.agent.add_custom_parameter(key, value) +class MonitoringCustomMetricsMiddleware(CachedCustomMonitoringMiddleware): + """ + Deprecated class for handling middleware. Class has been renamed to CachedCustomMonitoringMiddleware. + """ + REQUIRED_MIDDLEWARE = [ + 'edx_django_utils.cache.middleware.RequestCacheMiddleware', + 'edx_django_utils.monitoring.middleware.MonitoringCustomMetricsMiddleware', + ] + + def __init__(self, *args, **kwargs): + super(MonitoringCustomMetricsMiddleware, self).__init__(*args, **kwargs) + msg = "Use 'CachedCustomMonitoringMiddleware' in place of 'MonitoringCustomMetricsMiddleware'." + warnings.warn(msg, DeprecationWarning) + + class MonitoringMemoryMiddleware(MiddlewareMixin): """ Middleware for monitoring memory usage. diff --git a/edx_django_utils/monitoring/tests/test_monitoring.py b/edx_django_utils/monitoring/tests/test_monitoring.py index 8436a35e..4cbc434b 100644 --- a/edx_django_utils/monitoring/tests/test_monitoring.py +++ b/edx_django_utils/monitoring/tests/test_monitoring.py @@ -1,30 +1,34 @@ """ -Tests for monitoring custom metrics. +Tests for custom monitoring. """ from django.test import TestCase, override_settings from mock import call, patch from edx_django_utils import monitoring from edx_django_utils.cache import RequestCache -from edx_django_utils.monitoring.middleware import MonitoringCustomMetricsMiddleware, MonitoringMemoryMiddleware +from edx_django_utils.monitoring.middleware import ( + CachedCustomMonitoringMiddleware, + MonitoringCustomMetricsMiddleware, + MonitoringMemoryMiddleware +) -class TestMonitoringCustomMetrics(TestCase): +class TestCustomMonitoringMiddleware(TestCase): """ Test the monitoring_utils middleware and helpers """ def setUp(self): - super(TestMonitoringCustomMetrics, self).setUp() + super(TestCustomMonitoringMiddleware, self).setUp() RequestCache.clear_all_namespaces() @patch('newrelic.agent') @override_settings(MIDDLEWARE=[ 'edx_django_utils.cache.middleware.RequestCacheMiddleware', - 'edx_django_utils.monitoring.middleware.MonitoringCustomMetricsMiddleware', + 'edx_django_utils.monitoring.middleware.CachedCustomMonitoringMiddleware', ]) def test_accumulate_and_increment(self, mock_newrelic_agent): """ - Test normal usage of collecting custom metrics and reporting to New Relic + Test normal usage of collecting custom attributes and reporting to New Relic """ monitoring.accumulate('hello', 10) monitoring.accumulate('world', 10) @@ -32,15 +36,15 @@ def test_accumulate_and_increment(self, mock_newrelic_agent): monitoring.increment('foo') monitoring.increment('foo') - # based on the metric data above, we expect the following calls to newrelic: + # based on the attribute data above, we expect the following calls to newrelic: nr_agent_calls_expected = [ call('hello', 10), call('world', 20), call('foo', 2), ] - # fake a response to trigger metrics reporting - MonitoringCustomMetricsMiddleware().process_response( + # fake a response to trigger attributes reporting + CachedCustomMonitoringMiddleware().process_response( 'fake request', 'fake response', ) @@ -86,6 +90,13 @@ def test_accumulate_with_illegal_value(self, mock_newrelic_agent): # Assert call args to newrelic.agent.add_custom_parameter(). mock_newrelic_agent.add_custom_parameter.assert_has_calls(nr_agent_calls_expected, any_order=True) + @override_settings(MIDDLEWARE=[ + 'edx_django_utils.cache.middleware.RequestCacheMiddleware', + 'edx_django_utils.monitoring.middleware.CachedCustomMonitoringMiddleware', + ]) + def test_cached_custom_monitoring_middleware_dependencies_success(self): + CachedCustomMonitoringMiddleware() + @override_settings(MIDDLEWARE=[ 'edx_django_utils.cache.middleware.RequestCacheMiddleware', 'edx_django_utils.monitoring.middleware.MonitoringCustomMetricsMiddleware', @@ -94,9 +105,9 @@ def test_custom_metrics_middleware_dependencies_success(self): MonitoringCustomMetricsMiddleware() @override_settings(MIDDLEWARE=['some.Middleware']) - def test_custom_metrics_middleware_dependencies_failure(self): + def test_cached_custom_monitoring_middleware_dependencies_failure(self): with self.assertRaises(AssertionError): - MonitoringCustomMetricsMiddleware() + CachedCustomMonitoringMiddleware() @override_settings(MIDDLEWARE=[ 'edx_django_utils.cache.middleware.RequestCacheMiddleware', diff --git a/edx_django_utils/monitoring/utils.py b/edx_django_utils/monitoring/utils.py index 21ddf7ad..69845235 100644 --- a/edx_django_utils/monitoring/utils.py +++ b/edx_django_utils/monitoring/utils.py @@ -1,6 +1,6 @@ """ This is an interface to the monitoring_utils middleware. Functions -defined in this module can be used to report monitoring custom metrics. +defined in this module can be used for custom monitoring. Usage: @@ -8,17 +8,20 @@ ... monitoring_utils.accumulate('xb_user_state.get_many.num_items', 4) -There is no need to do anything else. The metrics are automatically cleared -before the next request. +There is no need to do anything else. The custom attributes are automatically +cleared before the next request. -We try to keep track of our custom metrics at: +We try to keep track of our custom monitoring at: +TODO: ARCHBOM-1490: Update to Custom Monitoring https://openedx.atlassian.net/wiki/display/PERF/Custom+Metrics+in+New+Relic -At this time, these custom metrics will only be reported to New Relic. +At this time, the custom monitoring will only be reported to New Relic. Please remember to expose any new methods in the `__init__.py` file. """ +import warnings + from . import middleware try: @@ -29,39 +32,40 @@ def accumulate(name, value): """ - Accumulate monitoring custom metric for the current request. + Accumulate monitoring custom attribute for the current request. - The named metric is accumulated by a numerical amount using the sum. All - metrics are queued up in the request_cache for this request. At the end of + The named attribute is accumulated by a numerical amount using the sum. All + attributes are queued up in the request_cache for this request. At the end of the request, the monitoring_utils middleware will batch report all - queued accumulated metrics to the monitoring tool (e.g. New Relic). + queued accumulated attributes to the monitoring tool (e.g. New Relic). Arguments: - name (str): The metric name. It should be period-delimited, and + name (str): The attribute name. It should be period-delimited, and increase in specificity from left to right. For example: 'xb_user_state.get_many.num_items'. - value (number): The amount to accumulate into the named metric. When - accumulate() is called multiple times for a given metric name + value (number): The amount to accumulate into the named attribute. When + accumulate() is called multiple times for a given attribute name during a request, the sum of the values for each call is reported - for that metric. For metrics which don't make sense to accumulate, - make sure to only call this function once during a request. + for that attribute. For attributes which don't make sense to accumulate, + use ``set_custom_attribute`` instead. + """ - middleware.MonitoringCustomMetricsMiddleware.accumulate_metric(name, value) + middleware.CachedCustomMonitoringMiddleware.accumulate_attribute(name, value) def increment(name): """ - Increment a monitoring custom metric representing a counter. + Increment a monitoring custom attribute representing a counter. - Here we simply accumulate a new custom metric with a value of 1, and the - middleware should automatically aggregate this metric. + Here we simply accumulate a new custom attribute with a value of 1, and the + middleware should automatically aggregate this attribute. """ accumulate(name, 1) -def set_custom_metrics_for_course_key(course_key): +def set_custom_attributes_for_course_key(course_key): """ - Set monitoring custom metrics related to a course key. + Set monitoring custom attributes related to a course key. This is not cached, and only support reporting to New Relic Insights. @@ -71,12 +75,33 @@ def set_custom_metrics_for_course_key(course_key): newrelic.agent.add_custom_parameter('org', str(course_key.org)) -def set_custom_metric(key, value): +def set_custom_metrics_for_course_key(course_key): # pragma: no cover + """ + Deprecated method to set monitoring custom attributes for course key. + """ + msg = "Use 'set_custom_attributes_for_course_key' in place of 'set_custom_metrics_for_course_key'." + warnings.warn(msg, DeprecationWarning) + set_custom_attribute('deprecated_set_custom_metric_for_course_key', True) + set_custom_attributes_for_course_key(course_key) + + +def set_custom_attribute(key, value): """ - Set monitoring custom metric. + Set monitoring custom attribute. This is not cached, and only support reporting to New Relic Insights. """ if newrelic: # pragma: no cover + # note: parameter is new relic's older name for attributes newrelic.agent.add_custom_parameter(key, value) + + +def set_custom_metric(key, value): # pragma: no cover + """ + Deprecated method to set monitoring custom attribute. + """ + msg = "Use 'set_custom_attribute' in place of 'set_custom_metric'." + warnings.warn(msg, DeprecationWarning) + set_custom_attribute('deprecated_set_custom_metric', True) + set_custom_attribute(key, value)