From bd1cb6ba7c20cfd07a11c502d8ddc6629b4c4548 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Thu, 17 Oct 2024 17:38:39 -0400 Subject: [PATCH] feat: original copy of code_owner code This is the code_owner code as found in edx-django-utils. --- .../datadog_monitoring/code_owner/__init__.py | 6 + .../code_owner/middleware.py | 175 ++++++++++ .../datadog_monitoring/code_owner/utils.py | 305 +++++++++++++++++ ..._code_owner_custom_attribute_to_an_ida.rst | 85 +++++ ..._monitoring_for_squad_or_theme_changes.rst | 44 +++ .../tests/code_owner/__init__.py | 0 .../tests/code_owner/mock_views.py | 12 + .../tests/code_owner/test_middleware.py | 321 ++++++++++++++++++ .../tests/code_owner/test_utils.py | 146 ++++++++ 9 files changed, 1094 insertions(+) create mode 100644 edx_arch_experiments/datadog_monitoring/code_owner/__init__.py create mode 100644 edx_arch_experiments/datadog_monitoring/code_owner/middleware.py create mode 100644 edx_arch_experiments/datadog_monitoring/code_owner/utils.py create mode 100644 edx_arch_experiments/datadog_monitoring/docs/how_tos/add_code_owner_custom_attribute_to_an_ida.rst create mode 100644 edx_arch_experiments/datadog_monitoring/docs/how_tos/update_monitoring_for_squad_or_theme_changes.rst create mode 100644 edx_arch_experiments/datadog_monitoring/tests/code_owner/__init__.py create mode 100644 edx_arch_experiments/datadog_monitoring/tests/code_owner/mock_views.py create mode 100644 edx_arch_experiments/datadog_monitoring/tests/code_owner/test_middleware.py create mode 100644 edx_arch_experiments/datadog_monitoring/tests/code_owner/test_utils.py diff --git a/edx_arch_experiments/datadog_monitoring/code_owner/__init__.py b/edx_arch_experiments/datadog_monitoring/code_owner/__init__.py new file mode 100644 index 0000000..952966e --- /dev/null +++ b/edx_arch_experiments/datadog_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_arch_experiments/datadog_monitoring/code_owner/middleware.py b/edx_arch_experiments/datadog_monitoring/code_owner/middleware.py new file mode 100644 index 0000000..1af5583 --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/code_owner/middleware.py @@ -0,0 +1,175 @@ +""" +Middleware for code_owner custom attribute +""" +import logging + +from django.urls import resolve +from django.urls.exceptions import Resolver404 + +from ..utils import set_custom_attribute +from .utils import ( + _get_catch_all_code_owner, + get_code_owner_from_module, + is_code_owner_mappings_configured, + set_code_owner_custom_attributes +) + +try: + import newrelic.agent +except ImportError: + newrelic = None # pylint: disable=invalid-name + +log = logging.getLogger(__name__) + + +class MonitoringTransaction(): + """ + Represents a monitoring transaction (likely the current transaction). + """ + def __init__(self, transaction): + self.transaction = transaction + + @property + def name(self): + """ + The name of the transaction. + + For NewRelic, the name may look like: + openedx.core.djangoapps.contentserver.middleware:StaticContentServer + + """ + if self.transaction and hasattr(self.transaction, 'name'): + return self.transaction.name + return None + + +def get_current_transaction(): + """ + Returns the current transaction. This is only used internally and won't + be ported over to the backends framework, because transactions will be + very different based on the backend. + """ + current_transaction = None + if newrelic: + current_transaction = newrelic.agent.current_transaction() + + return MonitoringTransaction(current_transaction) + + +class CodeOwnerMonitoringMiddleware: + """ + Django middleware object to set custom attributes for the owner of each view. + + For instructions on usage, see: + https://github.com/openedx/edx-django-utils/blob/master/edx_django_utils/monitoring/docs/how_tos/add_code_owner_custom_attribute_to_an_ida.rst + + Custom attributes set: + - code_owner: The owning team mapped to the current view. + - code_owner_module: The module found from the request or current transaction. + - code_owner_path_error: The error mapping by path, if code_owner isn't found in other ways. + - code_owner_transaction_error: The error mapping by transaction, if code_owner isn't found in other ways. + - code_owner_transaction_name: The current transaction name used to try to map to code_owner. + This can be used to find missing mappings. + + """ + def __init__(self, get_response): + self.get_response = get_response + + def __call__(self, request): + response = self.get_response(request) + self._set_code_owner_attribute(request) + return response + + def process_exception(self, request, exception): # pylint: disable=W0613 + self._set_code_owner_attribute(request) + + def _set_code_owner_attribute(self, request): + """ + Sets the code_owner custom attribute for the request. + """ + code_owner = None + module = self._get_module_from_request(request) + if module: + code_owner = get_code_owner_from_module(module) + if not code_owner: + code_owner = _get_catch_all_code_owner() + + if code_owner: + set_code_owner_custom_attributes(code_owner) + + def _get_module_from_request(self, request): + """ + Get the module from the request path or the current transaction. + + Side-effects: + Sets code_owner_module custom attribute, used to determine code_owner. + If module was not found, may set code_owner_path_error and/or + code_owner_transaction_error custom attributes if applicable. + + Returns: + str: module name or None if not found + + """ + if not is_code_owner_mappings_configured(): + return None + + module, path_error = self._get_module_from_request_path(request) + if module: + set_custom_attribute('code_owner_module', module) + return module + + module, transaction_error = self._get_module_from_current_transaction() + if module: + set_custom_attribute('code_owner_module', module) + return module + + # monitor errors if module was not found + if path_error: + set_custom_attribute('code_owner_path_error', path_error) + if transaction_error: + set_custom_attribute('code_owner_transaction_error', transaction_error) + return None + + def _get_module_from_request_path(self, request): + """ + Uses the request path to get the view_func module. + + Returns: + (str, str): (module, error_message), where at least one of these should be None + + """ + try: + view_func, _, _ = resolve(request.path) + module = view_func.__module__ + return module, None + # TODO: Replace ImportError with ModuleNotFoundError when Python 3.5 support is dropped. + except (ImportError, Resolver404) as e: + return None, str(e) + except Exception as e: # pragma: no cover + # will remove broad exceptions after ensuring all proper cases are covered + set_custom_attribute('deprecated_broad_except__get_module_from_request_path', e.__class__) + return None, str(e) + + def _get_module_from_current_transaction(self): + """ + Uses the current transaction to get the module. + + Side-effects: + Sets code_owner_transaction_name custom attribute, used to determine code_owner + + Returns: + (str, str): (module, error_message), where at least one of these should be None + + """ + 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.' + module = transaction_name.split(':')[0] + set_custom_attribute('code_owner_transaction_name', transaction_name) + return module, None + except Exception as e: + # will remove broad exceptions after ensuring all proper cases are covered + set_custom_attribute('deprecated_broad_except___get_module_from_current_transaction', e.__class__) + return None, str(e) diff --git a/edx_arch_experiments/datadog_monitoring/code_owner/utils.py b/edx_arch_experiments/datadog_monitoring/code_owner/utils.py new file mode 100644 index 0000000..b8a108c --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/code_owner/utils.py @@ -0,0 +1,305 @@ +""" +Utilities for monitoring code_owner +""" +import logging +import re +from functools import wraps + +from django.conf import settings + +from ..utils import set_custom_attribute + +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. + + See how to: + https://github.com/openedx/edx-django-utils/blob/master/edx_django_utils/monitoring/docs/how_tos/add_code_owner_custom_attribute_to_an_ida.rst + + """ + if not module: + return None + + code_owner_mappings = get_code_owner_mappings() + if not code_owner_mappings: + 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 code_owner_mappings: + code_owner = code_owner_mappings[partial_path] + return code_owner + return None + + +def is_code_owner_mappings_configured(): + """ + Returns True if code owner mappings were configured, and False otherwise. + """ + 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 get_code_owner_mappings(): + """ + Returns the contents of the CODE_OWNER_MAPPINGS Django Setting, processed + for efficient lookup by path. + + Returns: + (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:: + + { + 'xblock_django': 'team-red', + 'openedx.core.djangoapps.xblock': 'team-red', + 'badges': 'team-blue', + } + + """ + 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', {}) + + try: + for code_owner in code_owner_mappings: + path_list = code_owner_mappings[code_owner] + for path in path_list: + 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_mapping[path_without_prefix] = code_owner + except TypeError as e: + log.exception( + 'Error processing CODE_OWNER_MAPPINGS. {}'.format(e) # pylint: disable=logging-format-interpolation + ) + raise e + + _PATH_TO_CODE_OWNER_MAPPINGS = path_to_code_owner_mapping + return _PATH_TO_CODE_OWNER_MAPPINGS + + +def _get_catch_all_code_owner(): + """ + If the catch-all module "*" is configured, return the code_owner. + + Returns: + (str): code_owner or None if no catch-all configured. + + """ + try: + code_owner = get_code_owner_from_module('*') + return code_owner + except Exception as e: # pragma: no cover + # will remove broad exceptions after ensuring all proper cases are covered + set_custom_attribute('deprecated_broad_except___get_module_from_current_transaction', e.__class__) + return None + + +def set_code_owner_attribute_from_module(module): + """ + Updates the code_owner and code_owner_module custom attributes. + + Celery tasks or other non-web functions do not use middleware, so we need + an alternative way to set the code_owner custom attribute. + + Note: These settings will be overridden by the CodeOwnerMonitoringMiddleware. + This method can't be used to override web functions at this time. + + Usage:: + + set_code_owner_attribute_from_module(__name__) + + """ + set_custom_attribute('code_owner_module', module) + code_owner = get_code_owner_from_module(module) + if not code_owner: + code_owner = _get_catch_all_code_owner() + + if code_owner: + set_code_owner_custom_attributes(code_owner) + + +def set_code_owner_custom_attributes(code_owner): + """ + Sets custom metrics for code_owner, code_owner_theme, and code_owner_squad + """ + if not code_owner: # pragma: no cover + return + set_custom_attribute('code_owner', code_owner) + theme = _get_theme_from_code_owner(code_owner) + if theme: + set_custom_attribute('code_owner_theme', theme) + squad = _get_squad_from_code_owner(code_owner) + if squad: + set_custom_attribute('code_owner_squad', squad) + + +def set_code_owner_attribute(wrapped_function): + """ + Decorator to set the code_owner and code_owner_module custom attributes. + + Celery tasks or other non-web functions do not use middleware, so we need + an alternative way to set the code_owner custom attribute. + + Usage:: + + @task() + @set_code_owner_attribute + def example_task(): + ... + + Note: If the decorator can't be used for some reason, call + ``set_code_owner_attribute_from_module`` directly. + + An untested potential alternative is documented in the ADR covering this decision: + docs/decisions/0003-code-owner-for-celery-tasks.rst + + """ + @wraps(wrapped_function) + def new_function(*args, **kwargs): + set_code_owner_attribute_from_module(wrapped_function.__module__) + return wrapped_function(*args, **kwargs) + return new_function + + +def clear_cached_mappings(): + """ + Clears the cached code owner mappings. Useful for testing. + """ + global _PATH_TO_CODE_OWNER_MAPPINGS + _PATH_TO_CODE_OWNER_MAPPINGS = None + global _CODE_OWNER_TO_THEME_AND_SQUAD_MAPPINGS + _CODE_OWNER_TO_THEME_AND_SQUAD_MAPPINGS = None + + +# TODO: Retire this once edx-platform import_shims is no longer used. +# Note: This should be ready for removal because import_shims has been removed. +# See https://github.com/openedx/edx-platform/tree/854502b560bda74ef898501bb2a95ce238cf794c/import_shims +_OPTIONAL_MODULE_PREFIX_PATTERN = re.compile(r'^(lms|common|openedx\.core)\.djangoapps\.') + + +# Cached lookup table for code owner theme and squad given a code owner. +# - Although code owner is "theme-squad", a hyphen may also be in the theme or squad name, so this ensures we get both +# correctly from config. +# Do not access this directly, but instead use get_code_owner_theme_squad_mappings. +_CODE_OWNER_TO_THEME_AND_SQUAD_MAPPINGS = None + + +def get_code_owner_theme_squad_mappings(): + """ + Returns the contents of the CODE_OWNER_THEMES Django Setting, processed + for efficient lookup by path. + + Returns: + (dict): dict mapping code owners to a dict containing the squad and theme, or + an empty dict if there are no configured mappings. + + Example return value:: + + { + 'theme-x-team-red': { + 'theme': 'theme-x', + 'squad': 'team-red', + }, + 'theme-x-team-blue': { + 'theme': 'theme-x', + 'squad': 'team-blue', + }, + } + + """ + global _CODE_OWNER_TO_THEME_AND_SQUAD_MAPPINGS + + # Return cached processed mappings if already processed + if _CODE_OWNER_TO_THEME_AND_SQUAD_MAPPINGS is not None: + return _CODE_OWNER_TO_THEME_AND_SQUAD_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. + code_owner_to_theme_and_squad_mapping = {} + + # .. setting_name: CODE_OWNER_THEMES + # .. setting_default: None + # .. setting_description: Used for monitoring and reporting of ownership. Use a + # dict with keys of code owner themes and values as a list of code owner names + # including theme and squad, separated with a hyphen. + code_owner_themes = getattr(settings, 'CODE_OWNER_THEMES', {}) + + try: + for theme in code_owner_themes: + code_owner_list = code_owner_themes[theme] + for code_owner in code_owner_list: + squad = code_owner.split(theme + '-', 1)[1] + code_owner_details = { + 'theme': theme, + 'squad': squad, + } + code_owner_to_theme_and_squad_mapping[code_owner] = code_owner_details + except TypeError as e: + log.exception( + 'Error processing CODE_OWNER_THEMES setting. {}'.format(e) # pylint: disable=logging-format-interpolation + ) + raise e + + _CODE_OWNER_TO_THEME_AND_SQUAD_MAPPINGS = code_owner_to_theme_and_squad_mapping + return _CODE_OWNER_TO_THEME_AND_SQUAD_MAPPINGS + + +def _get_theme_from_code_owner(code_owner): + """ + Returns theme for a code_owner (e.g. 'theme-my-squad' => 'theme') + """ + mappings = get_code_owner_theme_squad_mappings() + if mappings is None: # pragma: no cover + return None + + if code_owner in mappings: + return mappings[code_owner]['theme'] + + return None + + +def _get_squad_from_code_owner(code_owner): + """ + Returns squad for a code_owner (e.g. 'theme-my-squad' => 'my-squad') + """ + mappings = get_code_owner_theme_squad_mappings() + if mappings is None: # pragma: no cover + return None + + if code_owner in mappings: + return mappings[code_owner]['squad'] + + return None diff --git a/edx_arch_experiments/datadog_monitoring/docs/how_tos/add_code_owner_custom_attribute_to_an_ida.rst b/edx_arch_experiments/datadog_monitoring/docs/how_tos/add_code_owner_custom_attribute_to_an_ida.rst new file mode 100644 index 0000000..b1a50b5 --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/docs/how_tos/add_code_owner_custom_attribute_to_an_ida.rst @@ -0,0 +1,85 @@ +Add Code_Owner Custom Attributes to an IDA +========================================== + +.. contents:: + :local: + :depth: 2 + +What are the code owner custom attributes? +------------------------------------------ + +The code owner custom attributes 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`_. + +The code owner custom attributes consist of: + +* code_owner: The owner name. When themes and squads are used, this will be the theme and squad names joined by a hyphen. +* code_owner_theme: The theme name of the owner. +* code_owner_squad: The squad name of the owner. Use this to avoid issues when theme name changes. + +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 :doc:`using_custom_attributes`. + +.. _ADR on monitoring by code owner: https://github.com/openedx/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.CodeOwnerMonitoringMiddleware`` as described in the README to make this functionality available. Then it is ready to be configured. + +Handling celery tasks +--------------------- + +Celery tasks require use of a special decorator to set the ``code_owner`` custom attributes because no middleware will be run. + +Here is an example:: + + @task() + @set_code_owner_attribute + def example_task(): + ... + +If the task is not compatible with additional decorators, you can use the following alternative:: + + @task() + def example_task(): + set_code_owner_attribute_from_module(__name__) + ... + +An untested potential alternative to the decorator is documented in the `Code Owner for Celery Tasks ADR`_, should we run into maintenance issues using the decorator. + +.. _Code Owner for Celery Tasks ADR: https://github.com/openedx/edx-django-utils/blob/master/edx_django_utils/monitoring/docs/decisions/0003-code-owner-for-celery-tasks.rst +Configuring your app settings +----------------------------- + +Once the Middleware is made available, simply set the Django Settings ``CODE_OWNER_MAPPINGS`` and ``CODE_OWNER_THEMES`` 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: + theme-x-team-red: + - xblock_django + - openedx.core.djangoapps.xblock + theme-x-team-blue: + - '*' # IMPORTANT: you must surround * with quotes in yml + + # YAML format of example CODE_OWNER_THEMES + CODE_OWNER_THEMES: + theme-x: + - theme-x-team-red + - theme-x-team-blue + +How to find and fix code_owner mappings +--------------------------------------- + +If you are missing the ``code_owner`` custom attributes 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/openedx/edx-django-utils/blob/c022565/edx_django_utils/monitoring/internal/code_owner/middleware.py#L30-L34 + +Updating New Relic monitoring +----------------------------- + +To update monitoring in the event of a squad or theme name change, see :doc:`update_monitoring_for_squad_or_theme_changes`. diff --git a/edx_arch_experiments/datadog_monitoring/docs/how_tos/update_monitoring_for_squad_or_theme_changes.rst b/edx_arch_experiments/datadog_monitoring/docs/how_tos/update_monitoring_for_squad_or_theme_changes.rst new file mode 100644 index 0000000..c00011a --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/docs/how_tos/update_monitoring_for_squad_or_theme_changes.rst @@ -0,0 +1,44 @@ +Update Monitoring for Squad or Theme Changes +============================================ + +.. contents:: + :local: + :depth: 2 + +Understanding code owner custom attributes +------------------------------------------ + +If you first need some background on the ``code_owner_squad`` and ``code_owner_theme`` custom attributes, see :doc:`add_code_owner_custom_attribute_to_an_ida`. + +Expand and contract name changes +-------------------------------- + +NRQL (New Relic Query Language) statements that use the ``code_owner_squad`` or ``code_owner_theme`` (or ``code_owner``) custom attributes may be found in alert conditions or dashboards. + +To change a squad or theme name, you should *expand* the NRQL before the change, and *contract* the NRQL after the change. + +.. note:: + + For edx.org, it is useful to wait a month before implementing the contract phase of the monitoring update. + +Example expand phase NRQL:: + + code_owner_squad IN ('old-squad-name', 'new-squad-name') + code_owner_theme IN ('old-theme-name', 'new-theme-name') + +Example contract phase NRQL:: + + code_owner_squad = 'new-squad-name' + code_owner_theme = 'new-theme-name' + +To find the relevant NRQL to update, see `Searching New Relic NRQL`_. + +Searching New Relic NRQL +------------------------ + +See :doc:`search_new_relic` for general information about the ``new_relic_search.py`` script. + +This script can be especially useful for helping with the expand/contract phase when changing squad or theme names. For example, you could use the following:: + + new_relic_search.py --regex old-squad-name + new_relic_search.py --regex new-squad-name diff --git a/edx_arch_experiments/datadog_monitoring/tests/code_owner/__init__.py b/edx_arch_experiments/datadog_monitoring/tests/code_owner/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/edx_arch_experiments/datadog_monitoring/tests/code_owner/mock_views.py b/edx_arch_experiments/datadog_monitoring/tests/code_owner/mock_views.py new file mode 100644 index 0000000..fddc5b4 --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/tests/code_owner/mock_views.py @@ -0,0 +1,12 @@ +""" +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 django.views.generic import View + + +class MockViewTest(View): + """ + Mock view for use in testing. + """ diff --git a/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_middleware.py b/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_middleware.py new file mode 100644 index 0000000..aad9e2a --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_middleware.py @@ -0,0 +1,321 @@ +""" +Tests for the code_owner monitoring middleware +""" +from unittest import TestCase +from unittest.mock import ANY, MagicMock, Mock, call, patch + +import ddt +from django.test import RequestFactory, override_settings +from django.urls import re_path +from django.views.generic import View + +from edx_django_utils.monitoring import CodeOwnerMonitoringMiddleware +from edx_django_utils.monitoring.internal.code_owner.utils import clear_cached_mappings + +from .mock_views import MockViewTest + + +class MockMiddlewareViewTest(View): + pass + + +urlpatterns = [ + re_path(r'^middleware-test/$', MockMiddlewareViewTest.as_view()), + re_path(r'^test/$', MockViewTest.as_view()), +] + +SET_CUSTOM_ATTRIBUTE_MOCK = MagicMock() + + +# Enables the same mock to be used from different modules, using +# patch with new_callable=get_set_custom_attribute_mock +def get_set_custom_attribute_mock(): + return SET_CUSTOM_ATTRIBUTE_MOCK + + +@ddt.ddt +class CodeOwnerMetricMiddlewareTests(TestCase): + """ + Tests for the code_owner monitoring utility functions + """ + urls = 'lms.djangoapps.monitoring.tests.test_middleware.test_urls' + + def setUp(self): + super().setUp() + clear_cached_mappings() + SET_CUSTOM_ATTRIBUTE_MOCK.reset_mock() + self.mock_get_response = Mock() + self.middleware = CodeOwnerMonitoringMiddleware(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/': 'edx_django_utils.monitoring.tests.code_owner.test_middleware', + '/test/': 'edx_django_utils.monitoring.tests.code_owner.mock_views', + } + + @override_settings( + CODE_OWNER_MAPPINGS={'team-red': ['edx_django_utils.monitoring.tests.code_owner.mock_views']}, + CODE_OWNER_THEMES={'team': ['team-red']}, + ROOT_URLCONF=__name__, + ) + @patch( + 'edx_django_utils.monitoring.internal.code_owner.utils.set_custom_attribute', + new_callable=get_set_custom_attribute_mock + ) + @patch( + 'edx_django_utils.monitoring.internal.code_owner.middleware.set_custom_attribute', + new_callable=get_set_custom_attribute_mock + ) + @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_attribute, _ + ): + 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, + check_theme_and_squad=True + ) + + 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, + check_theme_and_squad=True + ) + + @override_settings( + CODE_OWNER_MAPPINGS={ + 'team-red': ['edx_django_utils.monitoring.tests.code_owner.mock_views'], + 'team-blue': ['*'], + }, + ROOT_URLCONF=__name__, + ) + @patch( + 'edx_django_utils.monitoring.internal.code_owner.utils.set_custom_attribute', + new_callable=get_set_custom_attribute_mock + ) + @patch( + 'edx_django_utils.monitoring.internal.code_owner.middleware.set_custom_attribute', + new_callable=get_set_custom_attribute_mock + ) + @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_attribute, _ + ): + 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.tests.code_owner.mock_views']}, + ROOT_URLCONF=__name__, + ) + @patch( + 'edx_django_utils.monitoring.internal.code_owner.utils.set_custom_attribute', + new_callable=get_set_custom_attribute_mock + ) + @patch( + 'edx_django_utils.monitoring.internal.code_owner.middleware.set_custom_attribute', + new_callable=get_set_custom_attribute_mock + ) + @patch('newrelic.agent') + @ddt.data( + ( + 'edx_django_utils.monitoring.tests.code_owner.test_middleware', + 'edx_django_utils.monitoring.tests.code_owner.test_middleware:MockMiddlewareViewTest', + None + ), + ( + 'edx_django_utils.monitoring.tests.code_owner.mock_views', + 'edx_django_utils.monitoring.tests.code_owner.mock_views:MockViewTest', + 'team-red' + ), + ) + @ddt.unpack + def test_code_owner_transaction_mapping_hits_and_misses( # pylint: disable=too-many-positional-arguments + self, path_module, transaction_name, expected_owner, mock_newrelic_agent, mock_set_custom_attribute, _ + ): + mock_newrelic_agent.current_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, path_module=path_module, + transaction_name=transaction_name + ) + + @override_settings( + CODE_OWNER_MAPPINGS={ + 'team-red': ['edx_django_utils.monitoring.tests.code_owner.mock_views'], + 'team-blue': ['*'], + }, + ROOT_URLCONF=__name__, + ) + @patch( + 'edx_django_utils.monitoring.internal.code_owner.utils.set_custom_attribute', + new_callable=get_set_custom_attribute_mock + ) + @patch( + 'edx_django_utils.monitoring.internal.code_owner.middleware.set_custom_attribute', + new_callable=get_set_custom_attribute_mock + ) + @patch('newrelic.agent') + @ddt.data( + ( + 'edx_django_utils.monitoring.tests.code_owner.test_middleware', + 'edx_django_utils.monitoring.tests.code_owner.test_middleware:MockMiddlewareViewTest', + 'team-blue' + ), + ( + 'edx_django_utils.monitoring.tests.code_owner.mock_views', + 'edx_django_utils.monitoring.tests.code_owner.mock_views:MockViewTest', + 'team-red' + ), + ) + @ddt.unpack + def test_code_owner_transaction_mapping_with_catch_all( # pylint: disable=too-many-positional-arguments + self, path_module, transaction_name, expected_owner, mock_newrelic_agent, mock_set_custom_attribute, _ + ): + mock_newrelic_agent.current_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.tests.code_owner.mock_views']}, + ROOT_URLCONF=__name__, + ) + @patch( + 'edx_django_utils.monitoring.internal.code_owner.utils.set_custom_attribute', + new_callable=get_set_custom_attribute_mock + ) + @patch( + 'edx_django_utils.monitoring.internal.code_owner.middleware.set_custom_attribute', + new_callable=get_set_custom_attribute_mock + ) + @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')) + 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.internal.code_owner.utils.set_custom_attribute') + def test_code_owner_no_mappings(self, mock_set_custom_attribute): + request = RequestFactory().get('/test/') + self.middleware(request) + mock_set_custom_attribute.assert_not_called() + + @patch('edx_django_utils.monitoring.internal.code_owner.utils.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.internal.code_owner.utils.set_custom_attribute', + new_callable=get_set_custom_attribute_mock + ) + @patch( + 'edx_django_utils.monitoring.internal.code_owner.middleware.set_custom_attribute', + new_callable=get_set_custom_attribute_mock + ) + def test_no_resolver_for_path_and_no_transaction(self, mock_set_custom_attribute, _): + 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.internal.code_owner.utils.set_custom_attribute', + new_callable=get_set_custom_attribute_mock + ) + @patch( + 'edx_django_utils.monitoring.internal.code_owner.middleware.set_custom_attribute', + new_callable=get_set_custom_attribute_mock + ) + def test_catch_all_with_errors(self, mock_set_custom_attribute, _): + 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, expected_code_owner='team-red' + ) + + @override_settings( + CODE_OWNER_MAPPINGS=['invalid_setting_as_list'], + ROOT_URLCONF=__name__, + ) + def test_load_config_with_invalid_dict(self): + request = RequestFactory().get('/test/') + with self.assertRaises(TypeError): + self.middleware(request) + + def _assert_code_owner_custom_attributes( # pylint: disable=too-many-positional-arguments + self, mock_set_custom_attribute, expected_code_owner=None, + path_module=None, has_path_error=False, + transaction_name=None, has_transaction_error=False, + check_theme_and_squad=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)) + if check_theme_and_squad: + call_list.append(call('code_owner_theme', expected_code_owner.split('-')[0])) + call_list.append(call('code_owner_squad', expected_code_owner.split('-')[1])) + if path_module: + call_list.append(call('code_owner_module', path_module)) + if has_path_error: + call_list.append(call('code_owner_path_error', ANY)) + if transaction_name: + call_list.append(call('code_owner_transaction_name', transaction_name)) + if has_transaction_error: + call_list.append(call('code_owner_transaction_error', ANY)) + # TODO: Remove this list filtering once the ``deprecated_broad_except_XXX`` custom attributes have been removed. + actual_filtered_call_list = [ + mock_call + for mock_call in mock_set_custom_attribute.call_args_list + if not mock_call[0][0].startswith('deprecated_') + ] + mock_set_custom_attribute.assert_has_calls(call_list, any_order=True) + self.assertEqual( + len(actual_filtered_call_list), len(call_list), + f'Expected calls {call_list} vs actual calls {actual_filtered_call_list}' + ) diff --git a/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_utils.py b/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_utils.py new file mode 100644 index 0000000..6507d84 --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_utils.py @@ -0,0 +1,146 @@ +""" +Tests for the code_owner monitoring middleware +""" +import timeit +from unittest import TestCase +from unittest.mock import call, patch + +import ddt +from django.test import override_settings + +from edx_django_utils.monitoring import ( + get_code_owner_from_module, + set_code_owner_attribute, + set_code_owner_attribute_from_module +) +from edx_django_utils.monitoring.internal.code_owner.utils import clear_cached_mappings + + +@set_code_owner_attribute +def decorated_function(pass_through): + """ + For testing the set_code_owner_attribute decorator. + """ + return pass_through + + +@ddt.ddt +class MonitoringUtilsTests(TestCase): + """ + Tests for the code_owner monitoring utility functions + """ + def setUp(self): + super().setUp() + clear_cached_mappings() + + @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): + actual_owner = get_code_owner_from_module(module) + self.assertEqual(expected_owner, actual_owner) + + @override_settings(CODE_OWNER_MAPPINGS=['invalid_setting_as_list']) + @patch('edx_django_utils.monitoring.internal.code_owner.utils.log') + def test_code_owner_mapping_with_invalid_dict(self, mock_logger): + with self.assertRaises(TypeError): + get_code_owner_from_module('xblock') + + mock_logger.exception.assert_called_with( + 'Error processing CODE_OWNER_MAPPINGS. list indices must be integers or slices, not str', + ) + + def test_code_owner_mapping_with_no_settings(self): + self.assertIsNone(get_code_owner_from_module('xblock')) + + def test_code_owner_mapping_with_no_module(self): + self.assertIsNone(get_code_owner_from_module(None)) + + 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 = f'openedx.core.djangoapps.{n}' + code_owner_mappings['team-red'].append(path) + with override_settings(CODE_OWNER_MAPPINGS=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, f'Mapping takes {average_time}s which is too slow.') + + @override_settings( + CODE_OWNER_MAPPINGS={'team-red': ['edx_django_utils.monitoring.tests.code_owner.test_utils']}, + CODE_OWNER_THEMES={'team': ['team-red']}, + ) + @patch('edx_django_utils.monitoring.internal.code_owner.utils.set_custom_attribute') + def test_set_code_owner_attribute_success(self, mock_set_custom_attribute): + self.assertEqual(decorated_function('test'), 'test') + self._assert_set_custom_attribute( + mock_set_custom_attribute, code_owner='team-red', module=__name__, check_theme_and_squad=True + ) + + @override_settings( + CODE_OWNER_MAPPINGS={'team-red': ['edx_django_utils.monitoring.tests.code_owner.test_utils']}, + CODE_OWNER_THEMES='invalid-setting', + ) + def test_set_code_owner_attribute_with_invalid_setting(self): + with self.assertRaises(TypeError): + decorated_function('test') + + @override_settings(CODE_OWNER_MAPPINGS={ + 'team-red': ['*'] + }) + @patch('edx_django_utils.monitoring.internal.code_owner.utils.set_custom_attribute') + def test_set_code_owner_attribute_catch_all(self, mock_set_custom_attribute): + self.assertEqual(decorated_function('test'), 'test') + self._assert_set_custom_attribute(mock_set_custom_attribute, code_owner='team-red', module=__name__) + + @patch('edx_django_utils.monitoring.internal.code_owner.utils.set_custom_attribute') + def test_set_code_owner_attribute_no_mappings(self, mock_set_custom_attribute): + self.assertEqual(decorated_function('test'), 'test') + self._assert_set_custom_attribute(mock_set_custom_attribute, code_owner=None, module=__name__) + + @override_settings(CODE_OWNER_MAPPINGS={ + 'team-red': ['edx_django_utils.monitoring.tests.code_owner.test_utils'] + }) + @patch('edx_django_utils.monitoring.internal.code_owner.utils.set_custom_attribute') + def test_set_code_owner_attribute_from_module_success(self, mock_set_custom_attribute): + set_code_owner_attribute_from_module(__name__) + self._assert_set_custom_attribute(mock_set_custom_attribute, code_owner='team-red', module=__name__) + + def _assert_set_custom_attribute(self, mock_set_custom_attribute, code_owner, module, check_theme_and_squad=False): + """ + Helper to assert that the proper set_custom_metric calls were made. + """ + call_list = [] + if code_owner: + call_list.append(call('code_owner', code_owner)) + if check_theme_and_squad: + call_list.append(call('code_owner_theme', code_owner.split('-')[0])) + call_list.append(call('code_owner_squad', code_owner.split('-')[1])) + call_list.append(call('code_owner_module', module)) + mock_set_custom_attribute.assert_has_calls(call_list, any_order=True)