diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 1cf8e8d..bf5d34a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,16 @@ Change Log Unreleased ~~~~~~~~~~ +[5.1.0] - 2024-10-23 +~~~~~~~~~~~~~~~~~~~~ +Added +----- +* Added Datadog monitoring app which adds code owner monitoring. This is the first step in moving code owner code from edx-django-utils to this plugin. + + * Adds near duplicate of code owner middleware from edx-django-utils. + * Adds code owner for celery using Datadog span processing of celery.run spans. + * Uses temporary span tags names using ``_2``, like ``code_owner_2``, for rollout and comparison with the original span tags. + [5.0.0] - 2024-10-22 ~~~~~~~~~~~~~~~~~~~~ Removed diff --git a/edx_arch_experiments/__init__.py b/edx_arch_experiments/__init__.py index 068150b..5a61fdb 100644 --- a/edx_arch_experiments/__init__.py +++ b/edx_arch_experiments/__init__.py @@ -2,4 +2,4 @@ A plugin to include applications under development by the architecture team at 2U. """ -__version__ = '5.0.0' +__version__ = '5.1.0' diff --git a/edx_arch_experiments/datadog_monitoring/README.rst b/edx_arch_experiments/datadog_monitoring/README.rst new file mode 100644 index 0000000..5498d75 --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/README.rst @@ -0,0 +1,6 @@ +Datadog Monitoring +################### + +When installed in the LMS as a plugin app, the ``datadog_monitoring`` app adds additional monitoring. + +This is where our code_owner_2 monitoring code lives, for example. diff --git a/edx_arch_experiments/datadog_monitoring/__init__.py b/edx_arch_experiments/datadog_monitoring/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/edx_arch_experiments/datadog_monitoring/apps.py b/edx_arch_experiments/datadog_monitoring/apps.py new file mode 100644 index 0000000..725e645 --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/apps.py @@ -0,0 +1,58 @@ +""" +App for 2U-specific edx-platform Datadog monitoring. +""" + +import logging + +from django.apps import AppConfig + +from .code_owner.utils import get_code_owner_from_module + +log = logging.getLogger(__name__) + + +class DatadogMonitoringSpanProcessor: + """Datadog span processor that adds custom monitoring (e.g. code owner tags).""" + + def on_span_start(self, span): + """ + Adds custom monitoring at span creation time. + + Specifically, adds code owner span tag for celery run spans. + """ + if not span or not hasattr(span, 'name') or not hasattr(span, 'resource'): + return + + if span.name == 'celery.run': + # We can use this for celery spans, because the resource name is more predictable + # and available from the start. For django requests, we'll instead continue to use + # django middleware for setting code owner. + get_code_owner_from_module(span.resource) + + def on_span_finish(self, span): + pass + + def shutdown(self, _timeout): + pass + + +class DatadogMonitoring(AppConfig): + """ + Django application to handle 2U-specific Datadog monitoring. + """ + name = 'edx_arch_experiments.datadog_monitoring' + + # Mark this as a plugin app + plugin_app = {} + + def ready(self): + try: + from ddtrace import tracer # pylint: disable=import-outside-toplevel + + tracer._span_processors.append(DatadogMonitoringSpanProcessor()) # pylint: disable=protected-access + log.info("Attached DatadogMonitoringSpanProcessor") + except ImportError: + log.warning( + "Unable to attach DatadogMonitoringSpanProcessor" + " -- ddtrace module not found." + ) 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..f679959 --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/code_owner/middleware.py @@ -0,0 +1,89 @@ +""" +Middleware for code_owner_2 custom attribute +""" +import logging + +from django.urls import resolve +from edx_django_utils.monitoring import set_custom_attribute + +from .utils import get_code_owner_from_module, is_code_owner_mappings_configured, set_code_owner_custom_attributes + +log = logging.getLogger(__name__) + + +class CodeOwnerMonitoringMiddleware: + """ + Django middleware object to set custom attributes for the owner of each view. + + For instructions on usage, see: + https://github.com/edx/edx-arch-experiments/blob/master/edx_arch_experiments/datadog_monitoring/docs/how_tos/add_code_owner_custom_attribute_to_an_ida.rst + + Custom attributes set: + - code_owner_2: The owning team mapped to the current view. + - code_owner_2_module: The module found from the request or current transaction. + - code_owner_2_path_error: The error mapping by path, if code_owner_2 isn't found in other ways. + + """ + 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_2 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 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_2_module custom attribute, used to determine code_owner_2. + If module was not found, may set code_owner_2_path_error custom attribute + 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_2_module', module) + return module + + # monitor errors if module was not found + if path_error: + set_custom_attribute('code_owner_2_path_error', path_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 + except Exception as e: # pragma: no cover, pylint: disable=broad-exception-caught + 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..abb0086 --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/code_owner/utils.py @@ -0,0 +1,287 @@ +""" +Utilities for monitoring code_owner_2 +""" +import logging +import re +from functools import wraps + +from django.conf import settings +from edx_django_utils.monitoring 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', None) + if code_owner_mappings is None: + return None + + 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 set_code_owner_attribute_from_module(module): + """ + Updates the code_owner_2 and code_owner_2_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_2 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_2_attribute_from_module(__name__) + + """ + set_custom_attribute('code_owner_2_module', module) + code_owner = get_code_owner_from_module(module) + + if code_owner: + set_code_owner_custom_attributes(code_owner) + + +def set_code_owner_custom_attributes(code_owner): + """ + Sets custom metrics for code_owner_2, code_owner_2_theme, and code_owner_2_squad + """ + if not code_owner: # pragma: no cover + return + set_custom_attribute('code_owner_2', code_owner) + theme = _get_theme_from_code_owner(code_owner) + if theme: + set_custom_attribute('code_owner_2_theme', theme) + squad = _get_squad_from_code_owner(code_owner) + if squad: + set_custom_attribute('code_owner_2_squad', squad) + + +def set_code_owner_attribute(wrapped_function): + """ + Decorator to set the code_owner_2 and code_owner_2_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_2 custom attribute. + + Usage:: + + @task() + @set_code_owner_2_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..7392e3c --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/docs/how_tos/add_code_owner_custom_attribute_to_an_ida.rst @@ -0,0 +1,69 @@ +Using Code_Owner Custom Span Tags +================================= + +.. contents:: + :local: + :depth: 2 + +What are the code owner custom span tags? +------------------------------------------ + +The code owner custom span tags 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`_. However, it was first moved to edx-django-utils to be used in any IDA. It was later moved to this 2U-specific plugin because it is for 2U. + +The code owner custom attributes consist of: + +* code_owner_2: The owner name. When themes and squads are used, this will be the theme and squad names joined by a hyphen. +* code_owner_2_theme: The theme name of the owner. +* code_owner_2_squad: The squad name of the owner. Use this to avoid issues when theme name changes. + +Note: The ``_2`` of the code_owner_2 naming is for initial rollout to compare with edx-django-utils span tags. Ultimately, we will use adjusted names, which may include dropping the theme. + +If you want to learn more about custom span tags in general, see `Enhanced Monitoring and 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 +.. _Enhanced Monitoring and Custom Attributes: https://edx.readthedocs.io/projects/edx-django-utils/en/latest/monitoring/how_tos/using_custom_attributes.html + +Setting up the Middleware +------------------------- + +You simply need to add ``edx_arch_experiments/datadog_monitoring/code_owner/middleware.CodeOwnerMonitoringMiddleware`` to get code owner span tags on Django requests. + +Handling celery tasks +--------------------- + +For celery tasks, this plugin will automatically detect and add code owner span tags to any span with ``operation_name:celery.run``. + +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_2`` 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 supporting tags like ``code_owner_2_module`` and ``code_owner_2_path_error`` to determine what the appropriate mappings should be. + +Updating Datadog monitoring +--------------------------- + +To update monitoring in the event of a squad or theme name change, see `Update Monitoring for Squad or Theme Changes`_. + +.. _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..8e79a4d --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/docs/how_tos/update_monitoring_for_squad_or_theme_changes.rst @@ -0,0 +1,37 @@ +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_2_squad`` and ``code_owner_2_theme`` custom attributes, see `Using Code_Owner Custom Span Tags`_. + +.. _Using Code_Owner Custom Span Tags: https://github.com/openedx/edx-arch-experiments/blob/master/edx_arch_experiments/datadog_monitoring/docs/how_tos/add_code_owner_custom_attribute_to_an_ida.rst + +Expand and contract name changes +-------------------------------- + +Datadog monitors or dashboards may use the ``code_owner_2_squad`` or ``code_owner_2_theme`` (or ``code_owner_2``) custom span tags. + +To change a squad or theme name, you should *expand* before the change, and *contract* after the change. + +Example expand phase:: + + code_owner_2_squad:('old-squad-name', 'new-squad-name') + code_owner_2_theme:('old-theme-name', 'new-theme-name') + +Example contract phase NRQL:: + + code_owner_2_squad:'new-squad-name' + code_owner_2_theme:'new-theme-name' + +To find relevant usage of these span tags, see `Searching Datadog monitors and dashboards`_. + +Searching Datadog monitors and dashboards +----------------------------------------- + +TODO: This section needs to be updated as part of https://github.com/edx/edx-arch-experiments/issues/786, once the script has been migrated for use with Datadog. diff --git a/edx_arch_experiments/datadog_monitoring/tests/__init__.py b/edx_arch_experiments/datadog_monitoring/tests/__init__.py new file mode 100644 index 0000000..e69de29 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..54c5830 --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_middleware.py @@ -0,0 +1,151 @@ +""" +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_arch_experiments.datadog_monitoring.code_owner.middleware import CodeOwnerMonitoringMiddleware +from edx_arch_experiments.datadog_monitoring.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_arch_experiments.datadog_monitoring.tests.code_owner.test_middleware', + '/test/': 'edx_arch_experiments.datadog_monitoring.tests.code_owner.mock_views', + } + + @override_settings( + CODE_OWNER_MAPPINGS={'team-red': ['edx_arch_experiments.datadog_monitoring.tests.code_owner.mock_views']}, + CODE_OWNER_THEMES={'team': ['team-red']}, + ROOT_URLCONF=__name__, + ) + @patch( + 'edx_arch_experiments.datadog_monitoring.code_owner.middleware.set_custom_attribute', + new_callable=get_set_custom_attribute_mock + ) + @patch( + 'edx_arch_experiments.datadog_monitoring.code_owner.utils.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( + ROOT_URLCONF=__name__, + ) + @patch('edx_arch_experiments.datadog_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_attribute.assert_not_called() + + @override_settings( + CODE_OWNER_MAPPINGS={'team-red': ['lms.djangoapps.monitoring.tests.mock_views']}, + ) + @patch( + 'edx_arch_experiments.datadog_monitoring.code_owner.middleware.set_custom_attribute', + new_callable=get_set_custom_attribute_mock + ) + def test_no_resolver_for_path(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 + ) + + @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( + self, mock_set_custom_attribute, expected_code_owner=None, + path_module=None, has_path_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_2', expected_code_owner)) + if check_theme_and_squad: + call_list.append(call('code_owner_2_theme', expected_code_owner.split('-')[0])) + call_list.append(call('code_owner_2_squad', expected_code_owner.split('-')[1])) + if path_module: + call_list.append(call('code_owner_2_module', path_module)) + if has_path_error: + call_list.append(call('code_owner_2_path_error', ANY)) + mock_set_custom_attribute.assert_has_calls(call_list, any_order=True) + self.assertEqual( + len(mock_set_custom_attribute.call_args_list), len(call_list), + f'Expected calls {call_list} vs actual calls {mock_set_custom_attribute.call_args_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..d4ca5a0 --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_utils.py @@ -0,0 +1,138 @@ +""" +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_arch_experiments.datadog_monitoring.code_owner.utils import ( + clear_cached_mappings, + get_code_owner_from_module, + set_code_owner_attribute, + set_code_owner_attribute_from_module, +) + + +@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_arch_experiments.datadog_monitoring.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_arch_experiments.datadog_monitoring.tests.code_owner.test_utils']}, + CODE_OWNER_THEMES={'team': ['team-red']}, + ) + @patch('edx_arch_experiments.datadog_monitoring.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_arch_experiments.datadog_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') + + @patch('edx_arch_experiments.datadog_monitoring.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_arch_experiments.datadog_monitoring.tests.code_owner.test_utils'] + }) + @patch('edx_arch_experiments.datadog_monitoring.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_2', code_owner)) + if check_theme_and_squad: + call_list.append(call('code_owner_2_theme', code_owner.split('-')[0])) + call_list.append(call('code_owner_2_squad', code_owner.split('-')[1])) + call_list.append(call('code_owner_2_module', module)) + mock_set_custom_attribute.assert_has_calls(call_list, any_order=True) diff --git a/edx_arch_experiments/datadog_monitoring/tests/test_app.py b/edx_arch_experiments/datadog_monitoring/tests/test_app.py new file mode 100644 index 0000000..ca539c8 --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/tests/test_app.py @@ -0,0 +1,74 @@ +""" +Tests for plugin app. +""" +from unittest.mock import patch + +from ddtrace import tracer +from django.test import TestCase + +from .. import apps + + +class FakeSpan: + """A fake Span instance with span name and resource.""" + def __init__(self, name, resource): + self.name = name + self.resource = resource + + +class TestDatadogMonitoringApp(TestCase): + """Tests for TestDatadogMonitoringApp.""" + + def setUp(self): + # Remove custom span processor from previous runs. + # pylint: disable=protected-access + tracer._span_processors = [ + sp for sp in tracer._span_processors if type(sp).__name__ != 'DatadogMonitoringSpanProcessor' + ] + + def test_add_processor(self): + def initialize(): + apps.DatadogMonitoring('edx_arch_experiments.datadog_monitoring', apps).ready() + + def get_processor_list(): + # pylint: disable=protected-access + return [type(sp).__name__ for sp in tracer._span_processors] + + initialize() + assert sorted(get_processor_list()) == [ + 'DatadogMonitoringSpanProcessor', 'EndpointCallCounterProcessor', 'TopLevelSpanProcessor', + ] + + +class TestDatadogMonitoringSpanProcessor(TestCase): + """Tests for DatadogMonitoringSpanProcessor.""" + + @patch('edx_arch_experiments.datadog_monitoring.apps.get_code_owner_from_module') + def test_celery_span(self, mock_get_code_owner): + """ Tests processor with a celery span. """ + proc = apps.DatadogMonitoringSpanProcessor() + celery_span = FakeSpan('celery.run', 'test.module.for.celery.task') + + proc.on_span_start(celery_span) + + mock_get_code_owner.assert_called_once_with('test.module.for.celery.task') + + @patch('edx_arch_experiments.datadog_monitoring.apps.get_code_owner_from_module') + def test_other_span(self, mock_get_code_owner): + """ Tests processor with a non-celery span. """ + proc = apps.DatadogMonitoringSpanProcessor() + celery_span = FakeSpan('other.span', 'test.resource.name') + + proc.on_span_start(celery_span) + + mock_get_code_owner.assert_not_called() + + @patch('edx_arch_experiments.datadog_monitoring.apps.get_code_owner_from_module') + def test_non_span(self, mock_get_code_owner): + """ Tests processor with an object that doesn't have span name or resource. """ + proc = apps.DatadogMonitoringSpanProcessor() + non_span = object() + + proc.on_span_start(non_span) + + mock_get_code_owner.assert_not_called() diff --git a/requirements/dev.txt b/requirements/dev.txt index 65ad589..4d98aa8 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -26,6 +26,10 @@ build==1.2.2 # via # -r requirements/pip-tools.txt # pip-tools +bytecode==0.15.1 + # via + # -r requirements/quality.txt + # ddtrace cachetools==5.5.0 # via # -r requirements/ci.txt @@ -81,6 +85,12 @@ cryptography==43.0.1 # secretstorage ddt==1.7.2 # via -r requirements/quality.txt +ddtrace==2.14.4 + # via -r requirements/quality.txt +deprecated==1.2.14 + # via + # -r requirements/quality.txt + # opentelemetry-api diff-cover==9.2.0 # via -r requirements/dev.in dill==0.3.8 @@ -150,6 +160,10 @@ edx-opaque-keys==2.11.0 # edx-drf-extensions edx-toggles==5.2.0 # via -r requirements/quality.txt +envier==0.6.1 + # via + # -r requirements/quality.txt + # ddtrace filelock==3.16.1 # via # -r requirements/ci.txt @@ -163,6 +177,7 @@ importlib-metadata==8.4.0 # via # -r requirements/quality.txt # keyring + # opentelemetry-api # twine iniconfig==2.0.0 # via @@ -239,6 +254,10 @@ nh3==0.2.18 # via # -r requirements/quality.txt # readme-renderer +opentelemetry-api==1.27.0 + # via + # -r requirements/quality.txt + # ddtrace packaging==24.1 # via # -r requirements/ci.txt @@ -276,6 +295,10 @@ pluggy==1.5.0 # tox polib==1.2.0 # via edx-i18n-tools +protobuf==5.28.3 + # via + # -r requirements/quality.txt + # ddtrace psutil==6.0.0 # via # -r requirements/quality.txt @@ -431,6 +454,7 @@ twine==5.1.1 typing-extensions==4.12.2 # via # -r requirements/quality.txt + # ddtrace # edx-opaque-keys urllib3==2.2.3 # via @@ -445,6 +469,15 @@ wheel==0.44.0 # via # -r requirements/pip-tools.txt # pip-tools +wrapt==1.16.0 + # via + # -r requirements/quality.txt + # ddtrace + # deprecated +xmltodict==0.14.2 + # via + # -r requirements/quality.txt + # ddtrace zipp==3.20.2 # via # -r requirements/quality.txt diff --git a/requirements/doc.txt b/requirements/doc.txt index d0bb9e7..23c3cc5 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -17,6 +17,10 @@ attrs==24.2.0 # referencing babel==2.16.0 # via sphinx +bytecode==0.15.1 + # via + # -r requirements/test.txt + # ddtrace certifi==2024.8.30 # via # -r requirements/test.txt @@ -49,6 +53,12 @@ cryptography==43.0.1 # pyjwt ddt==1.7.2 # via -r requirements/test.txt +ddtrace==2.14.4 + # via -r requirements/test.txt +deprecated==1.2.14 + # via + # -r requirements/test.txt + # opentelemetry-api django==4.2.16 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt @@ -109,12 +119,20 @@ edx-sphinx-theme==3.1.0 # via -r requirements/doc.in edx-toggles==5.2.0 # via -r requirements/test.txt +envier==0.6.1 + # via + # -r requirements/test.txt + # ddtrace idna==3.10 # via # -r requirements/test.txt # requests imagesize==1.4.1 # via sphinx +importlib-metadata==8.4.0 + # via + # -r requirements/test.txt + # opentelemetry-api iniconfig==2.0.0 # via # -r requirements/test.txt @@ -140,6 +158,10 @@ newrelic==9.13.0 # edx-django-utils nh3==0.2.18 # via readme-renderer +opentelemetry-api==1.27.0 + # via + # -r requirements/test.txt + # ddtrace packaging==24.1 # via # -r requirements/test.txt @@ -153,6 +175,10 @@ pluggy==1.5.0 # via # -r requirements/test.txt # pytest +protobuf==5.28.3 + # via + # -r requirements/test.txt + # ddtrace psutil==6.0.0 # via # -r requirements/test.txt @@ -263,11 +289,25 @@ text-unidecode==1.3 typing-extensions==4.12.2 # via # -r requirements/test.txt + # ddtrace # edx-opaque-keys urllib3==2.2.3 # via # -r requirements/test.txt # requests +wrapt==1.16.0 + # via + # -r requirements/test.txt + # ddtrace + # deprecated +xmltodict==0.14.2 + # via + # -r requirements/test.txt + # ddtrace +zipp==3.20.2 + # via + # -r requirements/test.txt + # importlib-metadata # The following packages are considered to be unsafe in a requirements file: setuptools==75.1.0 diff --git a/requirements/quality.txt b/requirements/quality.txt index bc99982..240bc6e 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -19,6 +19,10 @@ attrs==24.2.0 # referencing backports-tarfile==1.2.0 # via jaraco-context +bytecode==0.15.1 + # via + # -r requirements/test.txt + # ddtrace certifi==2024.8.30 # via # -r requirements/test.txt @@ -57,6 +61,12 @@ cryptography==43.0.1 # secretstorage ddt==1.7.2 # via -r requirements/test.txt +ddtrace==2.14.4 + # via -r requirements/test.txt +deprecated==1.2.14 + # via + # -r requirements/test.txt + # opentelemetry-api dill==0.3.8 # via pylint django==4.2.16 @@ -113,13 +123,19 @@ edx-opaque-keys==2.11.0 # edx-drf-extensions edx-toggles==5.2.0 # via -r requirements/test.txt +envier==0.6.1 + # via + # -r requirements/test.txt + # ddtrace idna==3.10 # via # -r requirements/test.txt # requests importlib-metadata==8.4.0 # via + # -r requirements/test.txt # keyring + # opentelemetry-api # twine iniconfig==2.0.0 # via @@ -171,6 +187,10 @@ newrelic==9.13.0 # edx-django-utils nh3==0.2.18 # via readme-renderer +opentelemetry-api==1.27.0 + # via + # -r requirements/test.txt + # ddtrace packaging==24.1 # via # -r requirements/test.txt @@ -187,6 +207,10 @@ pluggy==1.5.0 # via # -r requirements/test.txt # pytest +protobuf==5.28.3 + # via + # -r requirements/test.txt + # ddtrace psutil==6.0.0 # via # -r requirements/test.txt @@ -308,14 +332,26 @@ twine==5.1.1 typing-extensions==4.12.2 # via # -r requirements/test.txt + # ddtrace # edx-opaque-keys urllib3==2.2.3 # via # -r requirements/test.txt # requests # twine +wrapt==1.16.0 + # via + # -r requirements/test.txt + # ddtrace + # deprecated +xmltodict==0.14.2 + # via + # -r requirements/test.txt + # ddtrace zipp==3.20.2 - # via importlib-metadata + # via + # -r requirements/test.txt + # importlib-metadata # The following packages are considered to be unsafe in a requirements file: setuptools==75.1.0 diff --git a/requirements/test.in b/requirements/test.in index 37a4119..c5dffdf 100644 --- a/requirements/test.in +++ b/requirements/test.in @@ -8,3 +8,4 @@ pytest-django # pytest extension for better Django support pytest-randomly # pytest extension for discovering order-sensitive tests code-annotations # provides commands used by the pii_check make target. ddt # data-driven tests +ddtrace # Required for testing datadog_monitoring app and middleware diff --git a/requirements/test.txt b/requirements/test.txt index 94c920d..27d6f5e 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -13,6 +13,8 @@ attrs==24.2.0 # -r requirements/base.txt # jsonschema # referencing +bytecode==0.15.1 + # via ddtrace certifi==2024.8.30 # via # -r requirements/base.txt @@ -44,6 +46,10 @@ cryptography==43.0.1 # pyjwt ddt==1.7.2 # via -r requirements/test.in +ddtrace==2.14.4 + # via -r requirements/test.in +deprecated==1.2.14 + # via opentelemetry-api # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/base.txt @@ -93,10 +99,14 @@ edx-opaque-keys==2.11.0 # edx-drf-extensions edx-toggles==5.2.0 # via -r requirements/base.txt +envier==0.6.1 + # via ddtrace idna==3.10 # via # -r requirements/base.txt # requests +importlib-metadata==8.4.0 + # via opentelemetry-api iniconfig==2.0.0 # via pytest jinja2==3.1.4 @@ -117,6 +127,8 @@ newrelic==9.13.0 # via # -r requirements/base.txt # edx-django-utils +opentelemetry-api==1.27.0 + # via ddtrace packaging==24.1 # via pytest pbr==6.1.0 @@ -125,6 +137,8 @@ pbr==6.1.0 # stevedore pluggy==1.5.0 # via pytest +protobuf==5.28.3 + # via ddtrace psutil==6.0.0 # via # -r requirements/base.txt @@ -204,11 +218,20 @@ text-unidecode==1.3 typing-extensions==4.12.2 # via # -r requirements/base.txt + # ddtrace # edx-opaque-keys urllib3==2.2.3 # via # -r requirements/base.txt # requests +wrapt==1.16.0 + # via + # ddtrace + # deprecated +xmltodict==0.14.2 + # via ddtrace +zipp==3.20.2 + # via importlib-metadata # The following packages are considered to be unsafe in a requirements file: setuptools==75.1.0