From 2d8642e0d22464a3d15bc7670aac8434c3e73092 Mon Sep 17 00:00:00 2001 From: rahul-apphelix Date: Tue, 1 Sep 2026 18:07:06 +0530 Subject: [PATCH] chore: Remove non-celery code owner monitoring code and docs Removes the CodeOwnerMonitoringMiddleware, its supporting utilities, and the new_relic_search.py script/docs, since this web-request-based monitoring is 2U-specific and has already been replaced by plugin-based instrumentation. The celery task decorator (set_code_owner_attribute) and its shared helpers are intentionally left in place, since it is still in active use and its removal is tracked separately. Refs openedx/edx-django-utils#469 --- docs/index.rst | 1 - docs/monitoring/how_tos/search_new_relic.rst | 1 - edx_django_utils/monitoring/README.rst | 6 - edx_django_utils/monitoring/__init__.py | 1 - ..._code_owner_custom_attribute_to_an_ida.rst | 7 +- .../docs/how_tos/search_new_relic.rst | 6 - ..._monitoring_for_squad_or_theme_changes.rst | 12 - .../internal/code_owner/middleware.py | 175 -------- .../monitoring/internal/code_owner/utils.py | 3 - .../monitoring/scripts/new_relic_search.py | 383 ------------------ .../monitoring/tests/code_owner/mock_views.py | 12 - .../tests/code_owner/test_middleware.py | 321 --------------- 12 files changed, 1 insertion(+), 927 deletions(-) delete mode 100644 docs/monitoring/how_tos/search_new_relic.rst delete mode 100644 edx_django_utils/monitoring/docs/how_tos/search_new_relic.rst delete mode 100644 edx_django_utils/monitoring/internal/code_owner/middleware.py delete mode 100644 edx_django_utils/monitoring/scripts/new_relic_search.py delete mode 100644 edx_django_utils/monitoring/tests/code_owner/mock_views.py delete mode 100644 edx_django_utils/monitoring/tests/code_owner/test_middleware.py diff --git a/docs/index.rst b/docs/index.rst index a57208ab..a15f0ced 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -42,7 +42,6 @@ EdX utilities for Django Application development. monitoring/how_tos/add_code_owner_custom_attribute_to_an_ida monitoring/how_tos/using_custom_attributes - monitoring/how_tos/search_new_relic_nrql monitoring/how_tos/update_monitoring_for_squad_or_theme_changes .. toctree:: diff --git a/docs/monitoring/how_tos/search_new_relic.rst b/docs/monitoring/how_tos/search_new_relic.rst deleted file mode 100644 index 4905b412..00000000 --- a/docs/monitoring/how_tos/search_new_relic.rst +++ /dev/null @@ -1 +0,0 @@ -.. include:: ../../../edx_django_utils/monitoring/docs/how_tos/search_new_relic.rst diff --git a/edx_django_utils/monitoring/README.rst b/edx_django_utils/monitoring/README.rst index 2359bc57..d65ef048 100644 --- a/edx_django_utils/monitoring/README.rst +++ b/edx_django_utils/monitoring/README.rst @@ -81,7 +81,6 @@ Here is how you add the middleware: 'edx_django_utils.monitoring.MonitoringSupportMiddleware', 'edx_django_utils.monitoring.DeploymentMonitoringMiddleware', 'edx_django_utils.monitoring.CookieMonitoringMiddleware', - 'edx_django_utils.monitoring.CodeOwnerMonitoringMiddleware', 'edx_django_utils.monitoring.FrontendMonitoringMiddleware', 'edx_django_utils.monitoring.MonitoringMemoryMiddleware', ) @@ -99,11 +98,6 @@ In order to use the monitoring signals, import them as follows:: from edx_django_utils.monitoring.signals import monitoring_support_process_response -Code Owner Custom Attribute ---------------------------- - -See docstring for ``CodeOwnerMonitoringMiddleware`` for configuring the ``code_owner`` custom attribute for your IDA. - Cookie Monitoring Middleware ---------------------------- diff --git a/edx_django_utils/monitoring/__init__.py b/edx_django_utils/monitoring/__init__.py index 7c199d89..d4563673 100644 --- a/edx_django_utils/monitoring/__init__.py +++ b/edx_django_utils/monitoring/__init__.py @@ -5,7 +5,6 @@ See README.rst for additional details. """ from .internal.backends import DatadogBackend, NewRelicBackend, OpenTelemetryBackend, TelemetryBackend -from .internal.code_owner.middleware import CodeOwnerMonitoringMiddleware from .internal.code_owner.utils import ( get_code_owner_from_module, set_code_owner_attribute, 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 index b1a50b5c..5ebb662a 100644 --- 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 @@ -22,11 +22,6 @@ If you want to know about custom attributes in general, see :doc:`using_custom_a .. _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 --------------------- @@ -52,7 +47,7 @@ An untested potential alternative to the decorator is documented in the `Code Ow Configuring your app settings ----------------------------- -Once the Middleware is made available, simply set the Django Settings ``CODE_OWNER_MAPPINGS`` and ``CODE_OWNER_THEMES`` appropriately. +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. diff --git a/edx_django_utils/monitoring/docs/how_tos/search_new_relic.rst b/edx_django_utils/monitoring/docs/how_tos/search_new_relic.rst deleted file mode 100644 index e4524a4b..00000000 --- a/edx_django_utils/monitoring/docs/how_tos/search_new_relic.rst +++ /dev/null @@ -1,6 +0,0 @@ -Searching New Relic -=================== - -The search script `new_relic_search.py`_ is generally useful for searching NRQL (New Relic Query Language) and text widgets in New Relic. It searches the NRQL in New Relic alert policies, and in NRQL and text widgets in New Relic dashboards. Use ``--help`` for more details. - -.. _new_relic_search.py: https://github.com/openedx/edx-django-utils/blob/master/edx_django_utils/monitoring/scripts/new_relic_search.py diff --git a/edx_django_utils/monitoring/docs/how_tos/update_monitoring_for_squad_or_theme_changes.rst b/edx_django_utils/monitoring/docs/how_tos/update_monitoring_for_squad_or_theme_changes.rst index c00011a3..d479a127 100644 --- a/edx_django_utils/monitoring/docs/how_tos/update_monitoring_for_squad_or_theme_changes.rst +++ b/edx_django_utils/monitoring/docs/how_tos/update_monitoring_for_squad_or_theme_changes.rst @@ -30,15 +30,3 @@ 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_django_utils/monitoring/internal/code_owner/middleware.py b/edx_django_utils/monitoring/internal/code_owner/middleware.py deleted file mode 100644 index 1af5583a..00000000 --- a/edx_django_utils/monitoring/internal/code_owner/middleware.py +++ /dev/null @@ -1,175 +0,0 @@ -""" -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_django_utils/monitoring/internal/code_owner/utils.py b/edx_django_utils/monitoring/internal/code_owner/utils.py index f927238a..9775147c 100644 --- a/edx_django_utils/monitoring/internal/code_owner/utils.py +++ b/edx_django_utils/monitoring/internal/code_owner/utils.py @@ -136,9 +136,6 @@ def set_code_owner_attribute_from_module(module): 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__) diff --git a/edx_django_utils/monitoring/scripts/new_relic_search.py b/edx_django_utils/monitoring/scripts/new_relic_search.py deleted file mode 100644 index 227ae86f..00000000 --- a/edx_django_utils/monitoring/scripts/new_relic_search.py +++ /dev/null @@ -1,383 +0,0 @@ -""" -This script takes a regex to search through the New Relic alert policies -and New dashboards. - -This script makes use of New Relic's GraphQL API. See https://api.newrelic.com/graphiql. - -For help:: - - python edx_django_utils/monitoring/scripts/new_relic_search.py --help - -""" -import json -import os -import re -from string import Template - -import click -import requests - - -@click.command() -@click.option( - '--regex', - required=True, - help="The regex to use to search in alert policies and dashboards.", -) -@click.option( - '--policy_id', - multiple=True, - help="Optionally provide a specific policy id to check. Multiple can be supplied.", -) -@click.option( - '--dashboard_guid', - multiple=True, - help="Optionally provide a specific dashboard guid to check. Multiple can be supplied.", -) -@click.option( - '--skip_text_widgets', - is_flag=True, - default=False, - help="Optionally skip text widgets and only search NRQL in dashboards", -) -@click.option( - '--retries', - required=False, - default=1, - help="Optionally specify the number of times to retry a graphql request before exiting. Default is 1.", -) -def main(regex, policy_id, dashboard_guid, skip_text_widgets, retries): - """ - Search NRQL and markdown in New Relic alert policies and dashboards using regex. - - Example usage: - - new_relic_search.py --regex tnl - - Note: The search ignores case since NRQL is case insensitive. - - Pre-requisite, set the following environment variable (in a safe way): - - NEW_RELIC_API_KEY=XXXXXXX - - See https://docs.newrelic.com/docs/apis/intro-apis/new-relic-api-keys/#user-api-key for details - on setting an API key. - - To skip alert policies or dashboards, just use a non-existent id, like --policy_id 0 or --dashboard_guid 0. To only - search NRQL and not markdown text, use --skip_text_widgets. - - """ - # Set environment variables - api_key = os.environ['NEW_RELIC_API_KEY'] - headers = { - "Content-Type": "application/json", - "X-Api-Key": api_key, - } - - compiled_regex = re.compile(regex) - - account_ids = get_account_ids(headers, retries) - for account_id in account_ids: - search_alert_policies(compiled_regex, account_id, headers, policy_id, retries) - print() - search_dashboards(compiled_regex, headers, dashboard_guid, skip_text_widgets, retries) - print(flush=True) - - -def get_with_retries(url, headers, params, retries): - response = requests.get(url, headers=headers, params=params) - # this is a bit silly but it allows us to retry on HTTP errors iff we have retries left - try: - response.raise_for_status() # could be an HTTP error response - except requests.exceptions.HTTPError as http_error: - if retries == 0: - raise Exception from http_error - print(f"HTTPError when fetching request: {http_error}. Retrying") - return get_with_retries(url, headers, params, retries=retries-1) - - response_data = response.json() - - # sometimes errors come back as 'errors' in the data block instead of HTTP errors - errors = response_data['data'].get('errors', None) - if errors is not None and retries > 0: - print(f"Errors when fetching request: {errors}. Retrying") - return get_with_retries(url, headers, params, retries=retries-1) - else: - # if the response succeeded or we're at 0 retries, return whatever we got - return response_data - - -def get_account_ids(headers, retries): - """ - Returns a list of the New Relic account ids to be searched. - """ - response_data = get_with_retries( - 'https://api.newrelic.com/graphql', - headers, - {'query': """ - { - actor { - accounts { - id - } - } - } - """}, - retries, - ) - account_ids = [account['id'] for account in response_data['data']['actor']['accounts']] - return account_ids - - -# Template with variables: -# - account_id -# - cursor; can be null or cursor string -ALERT_POLICY_LIST_TEMPLATE = Template(""" - { - actor { - account(id: ${account_id}) { - alerts { - policiesSearch(cursor: ${cursor}) { - policies { - id - name - } - nextCursor - } - } - } - } - } -""") - -# Template with variables: -# - account_id -# - policy_id -NRQL_ALERT_CONDITIONS_TEMPLATE = Template(""" - { - actor { - account(id: ${account_id}) { - alerts { - nrqlConditionsSearch(searchCriteria: {policyId: ${policy_id}}) { - nrqlConditions { - nrql { - query - } - name - } - } - } - } - } - } -""") - - -def search_alert_policies(regex, account_id, headers, policy_id, retries): - """ - Searches New Relic alert policy NRQL using the regex argument. - - Arguments: - regex (re.Pattern): compiled regex used to compare against NRQL - account_id (int): the id of the New Relic account in which to search. - headers (dict): headers required to make http requests to New Relic. - policy_id (tuple): optional tuple of policy ids supplied from the command-line. - retries (int): optional number of times to retry a failed request supplied from the command line. Default is 1. - """ - policies = [] - cursor = 'null' - while True: - response_data = get_with_retries( - 'https://api.newrelic.com/graphql', - headers, - {'query': ALERT_POLICY_LIST_TEMPLATE.substitute( - account_id=account_id, - cursor=cursor - )}, - retries, - ) - query_results = response_data['data']['actor']['account']['alerts']['policiesSearch'] - policies += query_results['policies'] - if not query_results['nextCursor']: - break - cursor = f"\"{query_results['nextCursor']}\"" - # Note: policy_id is an optional tuple of policy ids supplied from the command-line. - if policy_id: - policies = [policy for policy in policies if policy['id'] in policy_id] - print(f"Searching for regex {regex.pattern} in {len(policies)} alert policies in account {account_id}...") - policy_ids_printed = {} - for policy in policies: - print('.', end='', flush=True) - - # get the NRQL alert conditions from the alert policy - response_data = get_with_retries( - 'https://api.newrelic.com/graphql', - headers, - {'query': NRQL_ALERT_CONDITIONS_TEMPLATE.substitute( - account_id=account_id, - policy_id=policy['id'], - )}, - retries, - ) - - nrql_conditions = response_data['data']['actor']['account']['alerts']['nrqlConditionsSearch']['nrqlConditions'] - for nrql_condition in nrql_conditions: - nrql_query = nrql_condition['nrql']['query'] - if regex.search(nrql_query, re.IGNORECASE): - # Print the alert policy header for the first alert condition matched - if policy['id'] not in policy_ids_printed: - policy_ids_printed[policy['id']] = True - print('\n') - print( - f"Found in \"{policy['name']}\" " - # NOTE: The API doesn't provide a link to the policy, so this is a static link to - # the alert policies home page. - f"(policy_id={policy['id']}, search_link=https://onenr.io/0X8woZOZvQx):" - ) - print('') - - # Print the alert condition that matched - print(f"- {nrql_condition['name']}: {nrql_query}") - - if policy_ids_printed: - command_line = '' - for policy_id in policy_ids_printed.keys(): - command_line += f'--policy_id {policy_id} ' - print("\n\nRun again with found policies: {}".format(command_line)) - else: - print("\n\nNo alert policies matched.") - - -# Retrieves list of dashboard and dashboard page entities -# Template with variable cursor; can be null or cursor string -DASHBOARD_LIST_QUERY_TEMPLATE = Template(""" - { - actor { - entitySearch(queryBuilder: {type: DASHBOARD}) { - results(cursor: ${cursor}) { - entities { - ... on DashboardEntityOutline { - guid - name - accountId - dashboardParentGuid - permalink - } - } - nextCursor - } - count - } - } - } -""") - -# Retrieves dashboard entities. Does not work for dashboard pages. -DASHBOARD_ENTITY_QUERY = """ - query ($guids: EntityGuid!) { - actor { - entities(guids: $guids) { - ... on DashboardEntity { - guid - pages { - widgets { - rawConfiguration - title - } - } - name - } - } - } - } -""" - - -def search_dashboards(regex, headers, dashboard_guid, skip_text_widgets, retries): - """ - Searches New Relic alert policy NRQL using the regex argument. - - Arguments: - regex (re.Pattern): compiled regex used to compare against NRQL - headers (dict): headers required to make http requests to New Relic - dashboard_guid (tuple): optional tuple of dashboard guids supplied from the command-line. - skip_text_widgets (bool): optional flag to only search NRQL widgets, supplied from command line - retries (int): optional number of times to retry requests, supplied from the command line. Default is 1. - """ - # load details of all dashboards - dashboards = [] - cursor = 'null' - while True: - response_data = get_with_retries( - 'https://api.newrelic.com/graphql', - headers, - {'query': DASHBOARD_LIST_QUERY_TEMPLATE.substitute(cursor=cursor)}, - retries, - ) - query_results = response_data['data']['actor']['entitySearch']['results'] - dashboards += query_results['entities'] - if not query_results['nextCursor']: - break - cursor = f"\"{query_results['nextCursor']}\"" - # Filter out dashboard pages, which the dashboard entity query will not be able to find. - # - Note: This could probably be handled in the original query, but not sure how. - dashboards = [dashboard for dashboard in dashboards if dashboard['dashboardParentGuid'] is None] - # Note: dashboard_guid is an optional tuple of dashboard guids supplied from the command-line. - if dashboard_guid: - dashboards = [dashboard for dashboard in dashboards if dashboard['guid'] in dashboard_guid] - print(f"Searching for regex {regex.pattern} in {len(dashboards)} dashboards...") - dashboard_guids_printed = {} - for dashboard in dashboards: - print('.', end='', flush=True) - found = False - - # get the dashboard details - response_data = get_with_retries( - 'https://api.newrelic.com/graphql', - headers, - { - 'query': DASHBOARD_ENTITY_QUERY, - 'variables': json.dumps({'guids': dashboard['guid']}), - }, - retries, - ) - - if response_data['data']['actor']['entities'][0]['pages']: - for page in response_data['data']['actor']['entities'][0]['pages']: - for widget in page['widgets']: - found_text = None - if 'text' in widget['rawConfiguration'] and not skip_text_widgets: - widget_text = widget['rawConfiguration']['text'] - if regex.search(widget_text, re.IGNORECASE): - found = True - found_text = widget_text - if 'nrqlQueries' in widget['rawConfiguration']: - for nrql_query in widget['rawConfiguration']['nrqlQueries']: - query = nrql_query['query'] - if regex.search(query, re.IGNORECASE): - found = True - found_text = query - # Print the dashboard header for the first widget/nrql that matches - if found: - if dashboard['guid'] not in dashboard_guids_printed: - dashboard_guids_printed[dashboard['guid']] = True - print('\n') - print( - f"Found in \"{dashboard['name']}\" " - f"(guid={dashboard['guid']}, link={dashboard['permalink']}):" - ) - print('') - # Print the widget NRQL that matches - print(f"- {widget['title']}: {found_text}") - - if dashboard_guids_printed: - command_line = '' - for dashboard_guid in dashboard_guids_printed.keys(): - command_line += f'--dashboard_guid {dashboard_guid} ' - print("\n\nRun again with found dashboards: {}".format(command_line)) - else: - print("\n\nNo dashboards found that match.") - - -if __name__ == "__main__": - main() # pylint: disable=no-value-for-parameter diff --git a/edx_django_utils/monitoring/tests/code_owner/mock_views.py b/edx_django_utils/monitoring/tests/code_owner/mock_views.py deleted file mode 100644 index fddc5b41..00000000 --- a/edx_django_utils/monitoring/tests/code_owner/mock_views.py +++ /dev/null @@ -1,12 +0,0 @@ -""" -Mock views with a different module to enable testing of mapping -code_owner to modules. Trying to mock __module__ on a view was -getting too complex. -""" -from django.views.generic import View - - -class MockViewTest(View): - """ - Mock view for use in testing. - """ diff --git a/edx_django_utils/monitoring/tests/code_owner/test_middleware.py b/edx_django_utils/monitoring/tests/code_owner/test_middleware.py deleted file mode 100644 index 030ee44b..00000000 --- a/edx_django_utils/monitoring/tests/code_owner/test_middleware.py +++ /dev/null @@ -1,321 +0,0 @@ -""" -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 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 = [ - path('middleware-test/', MockMiddlewareViewTest.as_view()), - path('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.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_attribute.assert_not_called() - - @patch('edx_django_utils.monitoring.internal.code_owner.middleware.set_custom_attribute') - def test_code_owner_transaction_no_mappings(self, mock_set_custom_attribute): - request = RequestFactory().get('/bad/path/') - self.middleware(request) - mock_set_custom_attribute.assert_not_called() - - @override_settings( - CODE_OWNER_MAPPINGS={'team-red': ['lms.djangoapps.monitoring.tests.mock_views']}, - ) - @patch( - 'edx_django_utils.monitoring.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}' - )