From 32aa239b65d9bd28ad371b1221bfddb0e095d456 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Thu, 17 Oct 2024 17:38:39 -0400 Subject: [PATCH 01/19] 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) From 5cbbf4880d1c1a283c0950bb5e8e6de75751dd3e Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Wed, 23 Oct 2024 13:04:47 -0400 Subject: [PATCH 02/19] feat: refactor code_owner code from edx-dajango-utils Initial rollout of moving code_owner monitoring 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. See https://github.com/edx/edx-arch-experiments/issues/784 --- CHANGELOG.rst | 10 + edx_arch_experiments/__init__.py | 2 +- .../datadog_monitoring/README.rst | 6 + .../datadog_monitoring/__init__.py | 0 .../datadog_monitoring/apps.py | 53 +++++ .../code_owner/middleware.py | 107 ++------- .../datadog_monitoring/code_owner/utils.py | 49 ++--- ..._code_owner_custom_attribute_to_an_ida.rst | 50 ++--- ..._monitoring_for_squad_or_theme_changes.rst | 35 ++- .../datadog_monitoring/tests/__init__.py | 0 .../tests/code_owner/test_middleware.py | 207 ++---------------- .../tests/code_owner/test_utils.py | 34 ++- .../datadog_monitoring/tests/test_app.py | 62 ++++++ requirements/base.txt | 2 +- requirements/ci.txt | 2 +- requirements/pip-tools.txt | 2 +- requirements/pip.txt | 2 +- requirements/scripts.txt | 2 +- requirements/test.in | 1 + 19 files changed, 229 insertions(+), 397 deletions(-) create mode 100644 edx_arch_experiments/datadog_monitoring/README.rst create mode 100644 edx_arch_experiments/datadog_monitoring/__init__.py create mode 100644 edx_arch_experiments/datadog_monitoring/apps.py create mode 100644 edx_arch_experiments/datadog_monitoring/tests/__init__.py create mode 100644 edx_arch_experiments/datadog_monitoring/tests/test_app.py 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..6aafb76 --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/apps.py @@ -0,0 +1,53 @@ +""" +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): + if not span or not getattr(span, 'name') or not getattr(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 + # QUESTION: Do we want to publish a base constraint that avoids DD major changes without first testing them? + 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/middleware.py b/edx_arch_experiments/datadog_monitoring/code_owner/middleware.py index 1af5583..f06b64d 100644 --- a/edx_arch_experiments/datadog_monitoring/code_owner/middleware.py +++ b/edx_arch_experiments/datadog_monitoring/code_owner/middleware.py @@ -1,75 +1,32 @@ """ -Middleware for code_owner custom attribute +Middleware for code_owner_2 custom attribute """ import logging from django.urls import resolve -from django.urls.exceptions import Resolver404 -from ..utils import set_custom_attribute +from edx_django_utils.monitoring 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 + 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: 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. + - 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): @@ -85,14 +42,12 @@ def process_exception(self, request, exception): # pylint: disable=W0613 def _set_code_owner_attribute(self, request): """ - Sets the code_owner custom attribute for the 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 not code_owner: - code_owner = _get_catch_all_code_owner() if code_owner: set_code_owner_custom_attributes(code_owner) @@ -102,9 +57,9 @@ 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. + 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 @@ -115,19 +70,12 @@ def _get_module_from_request(self, request): 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) + 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_path_error', path_error) - if transaction_error: - set_custom_attribute('code_owner_transaction_error', transaction_error) + set_custom_attribute('code_owner_2_path_error', path_error) return None def _get_module_from_request_path(self, request): @@ -142,34 +90,5 @@ def _get_module_from_request_path(self, request): 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 index b8a108c..bd71206 100644 --- a/edx_arch_experiments/datadog_monitoring/code_owner/utils.py +++ b/edx_arch_experiments/datadog_monitoring/code_owner/utils.py @@ -1,5 +1,5 @@ """ -Utilities for monitoring code_owner +Utilities for monitoring code_owner_2 """ import logging import re @@ -7,7 +7,7 @@ from django.conf import settings -from ..utils import set_custom_attribute +from edx_django_utils.monitoring import set_custom_attribute log = logging.getLogger(__name__) @@ -88,7 +88,9 @@ def get_code_owner_mappings(): # .. 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', {}) + 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: @@ -110,42 +112,23 @@ def get_code_owner_mappings(): 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. + 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 custom attribute. + 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_attribute_from_module(__name__) + set_code_owner_2_attribute_from_module(__name__) """ - set_custom_attribute('code_owner_module', module) + set_custom_attribute('code_owner_2_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) @@ -153,30 +136,30 @@ def set_code_owner_attribute_from_module(module): def set_code_owner_custom_attributes(code_owner): """ - Sets custom metrics for code_owner, code_owner_theme, and code_owner_squad + 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', code_owner) + set_custom_attribute('code_owner_2', code_owner) theme = _get_theme_from_code_owner(code_owner) if theme: - set_custom_attribute('code_owner_theme', theme) + set_custom_attribute('code_owner_2_theme', theme) squad = _get_squad_from_code_owner(code_owner) if squad: - set_custom_attribute('code_owner_squad', squad) + set_custom_attribute('code_owner_2_squad', squad) def set_code_owner_attribute(wrapped_function): """ - Decorator to set the code_owner and code_owner_module custom attributes. + 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 custom attribute. + an alternative way to set the code_owner_2 custom attribute. Usage:: @task() - @set_code_owner_attribute + @set_code_owner_2_attribute def example_task(): ... 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 index b1a50b5..7392e3c 100644 --- 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 @@ -1,54 +1,38 @@ -Add Code_Owner Custom Attributes to an IDA -========================================== +Using Code_Owner Custom Span Tags +================================= .. contents:: :local: :depth: 2 -What are the code owner custom attributes? +What are the code owner custom span tags? ------------------------------------------ -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 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: 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. +* 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. -You can now easily add this same attribute to any IDA so that your dashboards and alerts can work across multiple IDAs at once. +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 know about custom attributes in general, see :doc:`using_custom_attributes`. +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_django_utils.monitoring.CodeOwnerMonitoringMiddleware`` as described in the README to make this functionality available. Then it is ready to be configured. +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 --------------------- -Celery tasks require use of a special decorator to set the ``code_owner`` custom attributes because no middleware will be run. +For celery tasks, this plugin will automatically detect and add code owner span tags to any span with ``operation_name:celery.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 ----------------------------- @@ -75,11 +59,11 @@ The following example shows how you can include an optional config for a catch-a 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. +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. -.. _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 Datadog monitoring +--------------------------- -Updating New Relic monitoring ------------------------------ +To update monitoring in the event of a squad or theme name change, see `Update Monitoring for Squad or Theme Changes`_. -To update monitoring in the event of a squad or theme name change, see :doc:`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 index c00011a..8e79a4d 100644 --- 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 @@ -8,37 +8,30 @@ Update Monitoring for Squad or Theme Changes 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`. +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 -------------------------------- -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:: +Datadog monitors or dashboards may use the ``code_owner_2_squad`` or ``code_owner_2_theme`` (or ``code_owner_2``) custom span tags. - For edx.org, it is useful to wait a month before implementing the contract phase of the monitoring update. +To change a squad or theme name, you should *expand* before the change, and *contract* after the change. -Example expand phase NRQL:: +Example expand phase:: - code_owner_squad IN ('old-squad-name', 'new-squad-name') - code_owner_theme IN ('old-theme-name', 'new-theme-name') + 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_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 ------------------------- + code_owner_2_squad:'new-squad-name' + code_owner_2_theme:'new-theme-name' -See :doc:`search_new_relic` for general information about the ``new_relic_search.py`` script. +To find relevant usage of these span tags, see `Searching Datadog monitors and dashboards`_. -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:: +Searching Datadog monitors and dashboards +----------------------------------------- - new_relic_search.py --regex old-squad-name - new_relic_search.py --regex new-squad-name +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/test_middleware.py b/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_middleware.py index aad9e2a..3a5e635 100644 --- a/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_middleware.py +++ b/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_middleware.py @@ -9,8 +9,8 @@ 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 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 @@ -56,21 +56,21 @@ def test_request_call(self): 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', + '/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_django_utils.monitoring.tests.code_owner.mock_views']}, + 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_django_utils.monitoring.internal.code_owner.utils.set_custom_attribute', + 'edx_arch_experiments.datadog_monitoring.code_owner.middleware.set_custom_attribute', new_callable=get_set_custom_attribute_mock ) @patch( - 'edx_django_utils.monitoring.internal.code_owner.middleware.set_custom_attribute', + 'edx_arch_experiments.datadog_monitoring.code_owner.utils.set_custom_attribute', new_callable=get_set_custom_attribute_mock ) @ddt.data( @@ -97,186 +97,26 @@ def test_code_owner_path_mapping_hits_and_misses( ) @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') + @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() - @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', + 'edx_arch_experiments.datadog_monitoring.code_owner.middleware.set_custom_attribute', new_callable=get_set_custom_attribute_mock ) - def test_catch_all_with_errors(self, mock_set_custom_attribute, _): + 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, has_transaction_error=True, expected_code_owner='team-red' + mock_set_custom_attribute, has_path_error=True ) @override_settings( @@ -291,31 +131,20 @@ def test_load_config_with_invalid_dict(self): 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)) + call_list.append(call('code_owner_2', 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])) + 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_module', path_module)) + call_list.append(call('code_owner_2_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_') - ] + 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(actual_filtered_call_list), len(call_list), - f'Expected calls {call_list} vs actual calls {actual_filtered_call_list}' + 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 index 6507d84..ad7ac93 100644 --- a/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_utils.py +++ b/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_utils.py @@ -8,12 +8,12 @@ import ddt from django.test import override_settings -from edx_django_utils.monitoring import ( +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 ) -from edx_django_utils.monitoring.internal.code_owner.utils import clear_cached_mappings @set_code_owner_attribute @@ -60,7 +60,7 @@ def test_code_owner_mapping_hits_and_misses(self, module, expected_owner): 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') + @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') @@ -93,10 +93,10 @@ def test_mapping_performance(self): 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_MAPPINGS={'team-red': ['edx_arch_experiments.datadog_monitoring.tests.code_owner.test_utils']}, CODE_OWNER_THEMES={'team': ['team-red']}, ) - @patch('edx_django_utils.monitoring.internal.code_owner.utils.set_custom_attribute') + @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( @@ -104,30 +104,22 @@ def test_set_code_owner_attribute_success(self, mock_set_custom_attribute): ) @override_settings( - CODE_OWNER_MAPPINGS={'team-red': ['edx_django_utils.monitoring.tests.code_owner.test_utils']}, + 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') - @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') + @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_django_utils.monitoring.tests.code_owner.test_utils'] + 'team-red': ['edx_arch_experiments.datadog_monitoring.tests.code_owner.test_utils'] }) - @patch('edx_django_utils.monitoring.internal.code_owner.utils.set_custom_attribute') + @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__) @@ -138,9 +130,9 @@ def _assert_set_custom_attribute(self, mock_set_custom_attribute, code_owner, mo """ call_list = [] if code_owner: - call_list.append(call('code_owner', code_owner)) + call_list.append(call('code_owner_2', 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)) + 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..3688037 --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/tests/test_app.py @@ -0,0 +1,62 @@ +""" +Tests for plugin app. +""" +from unittest.mock import call, patch + +from ddtrace import tracer +from django.test import TestCase, override_settings + +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() diff --git a/requirements/base.txt b/requirements/base.txt index f07f33a..83b1049 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with Python 3.11 +# This file is autogenerated by pip-compile with Python 3.13 # by the following command: # # make upgrade diff --git a/requirements/ci.txt b/requirements/ci.txt index deec652..854926a 100644 --- a/requirements/ci.txt +++ b/requirements/ci.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with Python 3.11 +# This file is autogenerated by pip-compile with Python 3.13 # by the following command: # # make upgrade diff --git a/requirements/pip-tools.txt b/requirements/pip-tools.txt index c6ff62d..a268261 100644 --- a/requirements/pip-tools.txt +++ b/requirements/pip-tools.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with Python 3.11 +# This file is autogenerated by pip-compile with Python 3.13 # by the following command: # # make upgrade diff --git a/requirements/pip.txt b/requirements/pip.txt index 346a061..173c476 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with Python 3.11 +# This file is autogenerated by pip-compile with Python 3.13 # by the following command: # # make upgrade diff --git a/requirements/scripts.txt b/requirements/scripts.txt index 29520f1..5ac006d 100644 --- a/requirements/scripts.txt +++ b/requirements/scripts.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with Python 3.11 +# This file is autogenerated by pip-compile with Python 3.13 # by the following command: # # make upgrade 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 From 68e019d1a78d98135cf0c2c83657ad69f3d5aab5 Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Thu, 24 Oct 2024 14:09:42 +0000 Subject: [PATCH 03/19] fixup! Re-compile requirements for 3.11 on Linux (without `--upgrade`) --- requirements/base.txt | 24 ++++++++++++------------ requirements/ci.txt | 8 ++++---- requirements/pip-tools.txt | 8 ++++---- requirements/pip.txt | 4 ++-- requirements/scripts.txt | 28 ++++++++++++++-------------- 5 files changed, 36 insertions(+), 36 deletions(-) diff --git a/requirements/base.txt b/requirements/base.txt index 83b1049..cd7273a 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with Python 3.13 +# This file is autogenerated by pip-compile with Python 3.11 # by the following command: # # make upgrade @@ -16,7 +16,7 @@ cffi==1.17.1 # via # cryptography # pynacl -charset-normalizer==3.4.0 +charset-normalizer==3.3.2 # via requests click==8.1.7 # via @@ -24,7 +24,7 @@ click==8.1.7 # edx-django-utils code-annotations==1.8.0 # via edx-toggles -cryptography==43.0.3 +cryptography==43.0.1 # via pyjwt django==4.2.16 # via @@ -52,13 +52,13 @@ djangorestframework==3.15.2 # -r requirements/base.in # drf-jwt # edx-drf-extensions -dnspython==2.7.0 +dnspython==2.6.1 # via pymongo drf-jwt==1.19.2 # via edx-drf-extensions -edx-codejail==3.5.1 +edx-codejail==3.4.1 # via -r requirements/base.in -edx-django-utils==7.0.0 +edx-django-utils==5.15.0 # via # -r requirements/base.in # edx-drf-extensions @@ -75,15 +75,15 @@ jinja2==3.1.4 # via code-annotations jsonschema==4.23.0 # via -r requirements/base.in -jsonschema-specifications==2024.10.1 +jsonschema-specifications==2023.12.1 # via jsonschema -markupsafe==3.0.2 +markupsafe==2.1.5 # via jinja2 -newrelic==10.2.0 +newrelic==9.13.0 # via edx-django-utils pbr==6.1.0 # via stevedore -psutil==6.1.0 +psutil==6.0.0 # via edx-django-utils pycparser==2.22 # via cffi @@ -91,7 +91,7 @@ pyjwt[crypto]==2.9.0 # via # drf-jwt # edx-drf-extensions -pymongo==4.10.1 +pymongo==4.9.1 # via edx-opaque-keys pynacl==1.5.0 # via edx-django-utils @@ -128,5 +128,5 @@ urllib3==2.2.3 # via requests # The following packages are considered to be unsafe in a requirements file: -setuptools==75.2.0 +setuptools==75.1.0 # via -r requirements/base.in diff --git a/requirements/ci.txt b/requirements/ci.txt index 854926a..3625d4c 100644 --- a/requirements/ci.txt +++ b/requirements/ci.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with Python 3.13 +# This file is autogenerated by pip-compile with Python 3.11 # by the following command: # # make upgrade @@ -10,7 +10,7 @@ chardet==5.2.0 # via tox colorama==0.4.6 # via tox -distlib==0.3.9 +distlib==0.3.8 # via virtualenv filelock==3.16.1 # via @@ -28,7 +28,7 @@ pluggy==1.5.0 # via tox pyproject-api==1.8.0 # via tox -tox==4.23.2 +tox==4.20.0 # via -r requirements/ci.in -virtualenv==20.27.0 +virtualenv==20.26.5 # via tox diff --git a/requirements/pip-tools.txt b/requirements/pip-tools.txt index a268261..49831ac 100644 --- a/requirements/pip-tools.txt +++ b/requirements/pip-tools.txt @@ -1,10 +1,10 @@ # -# This file is autogenerated by pip-compile with Python 3.13 +# This file is autogenerated by pip-compile with Python 3.11 # by the following command: # # make upgrade # -build==1.2.2.post1 +build==1.2.2 # via pip-tools click==8.1.7 # via pip-tools @@ -12,7 +12,7 @@ packaging==24.1 # via build pip-tools==7.4.1 # via -r requirements/pip-tools.in -pyproject-hooks==1.2.0 +pyproject-hooks==1.1.0 # via # build # pip-tools @@ -22,5 +22,5 @@ wheel==0.44.0 # The following packages are considered to be unsafe in a requirements file: pip==24.2 # via pip-tools -setuptools==75.2.0 +setuptools==75.1.0 # via pip-tools diff --git a/requirements/pip.txt b/requirements/pip.txt index 173c476..36c777e 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with Python 3.13 +# This file is autogenerated by pip-compile with Python 3.11 # by the following command: # # make upgrade @@ -10,5 +10,5 @@ wheel==0.44.0 # The following packages are considered to be unsafe in a requirements file: pip==24.2 # via -r requirements/pip.in -setuptools==75.2.0 +setuptools==75.1.0 # via -r requirements/pip.in diff --git a/requirements/scripts.txt b/requirements/scripts.txt index 5ac006d..7cce70b 100644 --- a/requirements/scripts.txt +++ b/requirements/scripts.txt @@ -1,5 +1,5 @@ # -# This file is autogenerated by pip-compile with Python 3.13 +# This file is autogenerated by pip-compile with Python 3.11 # by the following command: # # make upgrade @@ -25,7 +25,7 @@ cffi==1.17.1 # -r requirements/base.txt # cryptography # pynacl -charset-normalizer==3.4.0 +charset-normalizer==3.3.2 # via # -r requirements/base.txt # requests @@ -38,9 +38,9 @@ code-annotations==1.8.0 # via # -r requirements/base.txt # edx-toggles -confluent-kafka[avro]==2.6.0 +confluent-kafka[avro]==2.5.3 # via -r requirements/scripts.in -cryptography==43.0.3 +cryptography==43.0.1 # via # -r requirements/base.txt # pyjwt @@ -73,7 +73,7 @@ djangorestframework==3.15.2 # -r requirements/base.txt # drf-jwt # edx-drf-extensions -dnspython==2.7.0 +dnspython==2.6.1 # via # -r requirements/base.txt # pymongo @@ -83,9 +83,9 @@ drf-jwt==1.19.2 # edx-drf-extensions edx-ccx-keys==1.3.0 # via openedx-events -edx-codejail==3.5.1 +edx-codejail==3.4.1 # via -r requirements/base.txt -edx-django-utils==7.0.0 +edx-django-utils==5.15.0 # via # -r requirements/base.txt # edx-drf-extensions @@ -120,25 +120,25 @@ jinja2==3.1.4 # code-annotations jsonschema==4.23.0 # via -r requirements/base.txt -jsonschema-specifications==2024.10.1 +jsonschema-specifications==2023.12.1 # via # -r requirements/base.txt # jsonschema -markupsafe==3.0.2 +markupsafe==2.1.5 # via # -r requirements/base.txt # jinja2 -newrelic==10.2.0 +newrelic==9.13.0 # via # -r requirements/base.txt # edx-django-utils -openedx-events==9.15.0 +openedx-events==9.14.1 # via edx-event-bus-kafka pbr==6.1.0 # via # -r requirements/base.txt # stevedore -psutil==6.1.0 +psutil==6.0.0 # via # -r requirements/base.txt # edx-django-utils @@ -151,7 +151,7 @@ pyjwt[crypto]==2.9.0 # -r requirements/base.txt # drf-jwt # edx-drf-extensions -pymongo==4.10.1 +pymongo==4.9.1 # via # -r requirements/base.txt # edx-opaque-keys @@ -215,5 +215,5 @@ urllib3==2.2.3 # requests # The following packages are considered to be unsafe in a requirements file: -setuptools==75.2.0 +setuptools==75.1.0 # via -r requirements/base.txt From d2373784c9960c33402b4b3bde63813adfb662dc Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Thu, 24 Oct 2024 10:11:31 -0400 Subject: [PATCH 04/19] fixup! fix some quality failures --- .../datadog_monitoring/apps.py | 9 +++++++-- .../code_owner/middleware.py | 9 ++------- .../datadog_monitoring/code_owner/utils.py | 1 - .../tests/code_owner/test_middleware.py | 5 +++-- .../tests/code_owner/test_utils.py | 2 +- .../datadog_monitoring/tests/test_app.py | 18 +++++++++++++++--- 6 files changed, 28 insertions(+), 16 deletions(-) diff --git a/edx_arch_experiments/datadog_monitoring/apps.py b/edx_arch_experiments/datadog_monitoring/apps.py index 6aafb76..725e645 100644 --- a/edx_arch_experiments/datadog_monitoring/apps.py +++ b/edx_arch_experiments/datadog_monitoring/apps.py @@ -15,7 +15,12 @@ class DatadogMonitoringSpanProcessor: """Datadog span processor that adds custom monitoring (e.g. code owner tags).""" def on_span_start(self, span): - if not span or not getattr(span, 'name') or not getattr(span, 'resource'): + """ + 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': @@ -43,7 +48,7 @@ class DatadogMonitoring(AppConfig): def ready(self): try: from ddtrace import tracer # pylint: disable=import-outside-toplevel - # QUESTION: Do we want to publish a base constraint that avoids DD major changes without first testing them? + tracer._span_processors.append(DatadogMonitoringSpanProcessor()) # pylint: disable=protected-access log.info("Attached DatadogMonitoringSpanProcessor") except ImportError: diff --git a/edx_arch_experiments/datadog_monitoring/code_owner/middleware.py b/edx_arch_experiments/datadog_monitoring/code_owner/middleware.py index f06b64d..f679959 100644 --- a/edx_arch_experiments/datadog_monitoring/code_owner/middleware.py +++ b/edx_arch_experiments/datadog_monitoring/code_owner/middleware.py @@ -4,14 +4,9 @@ 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 -) +from .utils import get_code_owner_from_module, is_code_owner_mappings_configured, set_code_owner_custom_attributes log = logging.getLogger(__name__) @@ -90,5 +85,5 @@ def _get_module_from_request_path(self, request): view_func, _, _ = resolve(request.path) module = view_func.__module__ return module, None - except Exception as e: # pragma: no cover + 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 index bd71206..abb0086 100644 --- a/edx_arch_experiments/datadog_monitoring/code_owner/utils.py +++ b/edx_arch_experiments/datadog_monitoring/code_owner/utils.py @@ -6,7 +6,6 @@ from functools import wraps from django.conf import settings - from edx_django_utils.monitoring import set_custom_attribute log = logging.getLogger(__name__) 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 index 3a5e635..54c5830 100644 --- a/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_middleware.py +++ b/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_middleware.py @@ -128,10 +128,11 @@ def test_load_config_with_invalid_dict(self): with self.assertRaises(TypeError): self.middleware(request) - def _assert_code_owner_custom_attributes( # pylint: disable=too-many-positional-arguments + 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): + check_theme_and_squad=False + ): """ Performs a set of assertions around having set the proper custom attributes. """ call_list = [] if expected_code_owner: 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 index ad7ac93..d4ca5a0 100644 --- a/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_utils.py +++ b/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_utils.py @@ -12,7 +12,7 @@ clear_cached_mappings, get_code_owner_from_module, set_code_owner_attribute, - set_code_owner_attribute_from_module + set_code_owner_attribute_from_module, ) diff --git a/edx_arch_experiments/datadog_monitoring/tests/test_app.py b/edx_arch_experiments/datadog_monitoring/tests/test_app.py index 3688037..ca539c8 100644 --- a/edx_arch_experiments/datadog_monitoring/tests/test_app.py +++ b/edx_arch_experiments/datadog_monitoring/tests/test_app.py @@ -1,10 +1,10 @@ """ Tests for plugin app. """ -from unittest.mock import call, patch +from unittest.mock import patch from ddtrace import tracer -from django.test import TestCase, override_settings +from django.test import TestCase from .. import apps @@ -22,7 +22,9 @@ class TestDatadogMonitoringApp(TestCase): 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'] + tracer._span_processors = [ + sp for sp in tracer._span_processors if type(sp).__name__ != 'DatadogMonitoringSpanProcessor' + ] def test_add_processor(self): def initialize(): @@ -60,3 +62,13 @@ def test_other_span(self, mock_get_code_owner): 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() From 433efbf5fd0b950627b0101f59520a41334945b4 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Wed, 13 Nov 2024 18:45:23 -0500 Subject: [PATCH 05/19] fixup! fix code review feedback 1. switch get_code_owner_from_module => set_code_owner_attribute_from_module. 2. Update tests with deeper mock of set_custom_attribute. 3. Remove decorator set_code_owner_attribute and associated tests. 4. Fix minor rst issues. --- .../datadog_monitoring/apps.py | 4 +-- .../datadog_monitoring/code_owner/utils.py | 29 ---------------- ..._monitoring_for_squad_or_theme_changes.rst | 4 +-- .../tests/code_owner/test_utils.py | 33 ------------------- .../datadog_monitoring/tests/test_app.py | 18 +++++----- 5 files changed, 13 insertions(+), 75 deletions(-) diff --git a/edx_arch_experiments/datadog_monitoring/apps.py b/edx_arch_experiments/datadog_monitoring/apps.py index 725e645..41ce467 100644 --- a/edx_arch_experiments/datadog_monitoring/apps.py +++ b/edx_arch_experiments/datadog_monitoring/apps.py @@ -6,7 +6,7 @@ from django.apps import AppConfig -from .code_owner.utils import get_code_owner_from_module +from .code_owner.utils import set_code_owner_attribute_from_module log = logging.getLogger(__name__) @@ -27,7 +27,7 @@ def on_span_start(self, span): # 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) + set_code_owner_attribute_from_module(span.resource) def on_span_finish(self, span): pass diff --git a/edx_arch_experiments/datadog_monitoring/code_owner/utils.py b/edx_arch_experiments/datadog_monitoring/code_owner/utils.py index abb0086..ecff3af 100644 --- a/edx_arch_experiments/datadog_monitoring/code_owner/utils.py +++ b/edx_arch_experiments/datadog_monitoring/code_owner/utils.py @@ -3,7 +3,6 @@ """ import logging import re -from functools import wraps from django.conf import settings from edx_django_utils.monitoring import set_custom_attribute @@ -148,34 +147,6 @@ def set_code_owner_custom_attributes(code_owner): 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. 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 index 8e79a4d..695a90a 100644 --- 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 @@ -10,7 +10,7 @@ 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 +.. _Using Code_Owner Custom Span Tags: https://github.com/edx/edx-arch-experiments/blob/main/edx_arch_experiments/datadog_monitoring/docs/how_tos/add_code_owner_custom_attribute_to_an_ida.rst Expand and contract name changes -------------------------------- @@ -24,7 +24,7 @@ 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:: +Example contract phase:: code_owner_2_squad:'new-squad-name' code_owner_2_theme:'new-theme-name' 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 index d4ca5a0..be17856 100644 --- a/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_utils.py +++ b/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_utils.py @@ -11,19 +11,10 @@ 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): """ @@ -92,30 +83,6 @@ def test_mapping_performance(self): 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'] }) diff --git a/edx_arch_experiments/datadog_monitoring/tests/test_app.py b/edx_arch_experiments/datadog_monitoring/tests/test_app.py index ca539c8..e835b52 100644 --- a/edx_arch_experiments/datadog_monitoring/tests/test_app.py +++ b/edx_arch_experiments/datadog_monitoring/tests/test_app.py @@ -43,32 +43,32 @@ def get_processor_list(): 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): + @patch('edx_arch_experiments.datadog_monitoring.code_owner.utils.set_custom_attribute') + def test_celery_span(self, mock_set_custom_attribute): """ 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') + mock_set_custom_attribute.assert_called_once_with('code_owner_2_module', '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): + @patch('edx_arch_experiments.datadog_monitoring.code_owner.utils.set_custom_attribute') + def test_other_span(self, mock_set_custom_attribute): """ 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() + mock_set_custom_attribute.assert_not_called() - @patch('edx_arch_experiments.datadog_monitoring.apps.get_code_owner_from_module') - def test_non_span(self, mock_get_code_owner): + @patch('edx_arch_experiments.datadog_monitoring.code_owner.utils.set_custom_attribute') + def test_non_span(self, mock_set_custom_attribute): """ 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() + mock_set_custom_attribute.assert_not_called() From 81fad6d1b141153a379b33e0aa09bb42272a9764 Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Thu, 14 Nov 2024 19:20:24 +0000 Subject: [PATCH 06/19] fixup! Run `make upgrade` --- requirements/base.txt | 28 ++++++------ requirements/ci.txt | 8 ++-- requirements/dev.txt | 62 ++++++++++++++++++++------ requirements/doc.txt | 90 +++++++++++++++++++++----------------- requirements/pip-tools.txt | 14 +++--- requirements/pip.txt | 6 +-- requirements/quality.txt | 60 ++++++++++++++++++++----- requirements/scripts.txt | 32 +++++++------- requirements/test.txt | 39 +++++++++++++---- 9 files changed, 221 insertions(+), 118 deletions(-) diff --git a/requirements/base.txt b/requirements/base.txt index cd7273a..bcf4cd1 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -16,15 +16,15 @@ cffi==1.17.1 # via # cryptography # pynacl -charset-normalizer==3.3.2 +charset-normalizer==3.4.0 # via requests click==8.1.7 # via # code-annotations # edx-django-utils -code-annotations==1.8.0 +code-annotations==1.8.1 # via edx-toggles -cryptography==43.0.1 +cryptography==43.0.3 # via pyjwt django==4.2.16 # via @@ -52,13 +52,13 @@ djangorestframework==3.15.2 # -r requirements/base.in # drf-jwt # edx-drf-extensions -dnspython==2.6.1 +dnspython==2.7.0 # via pymongo drf-jwt==1.19.2 # via edx-drf-extensions -edx-codejail==3.4.1 +edx-codejail==3.5.2 # via -r requirements/base.in -edx-django-utils==5.15.0 +edx-django-utils==7.0.0 # via # -r requirements/base.in # edx-drf-extensions @@ -75,15 +75,15 @@ jinja2==3.1.4 # via code-annotations jsonschema==4.23.0 # via -r requirements/base.in -jsonschema-specifications==2023.12.1 +jsonschema-specifications==2024.10.1 # via jsonschema -markupsafe==2.1.5 +markupsafe==3.0.2 # via jinja2 -newrelic==9.13.0 +newrelic==10.2.0 # via edx-django-utils pbr==6.1.0 # via stevedore -psutil==6.0.0 +psutil==6.1.0 # via edx-django-utils pycparser==2.22 # via cffi @@ -91,7 +91,7 @@ pyjwt[crypto]==2.9.0 # via # drf-jwt # edx-drf-extensions -pymongo==4.9.1 +pymongo==4.10.1 # via edx-opaque-keys pynacl==1.5.0 # via edx-django-utils @@ -105,7 +105,7 @@ referencing==0.35.1 # jsonschema-specifications requests==2.32.3 # via edx-drf-extensions -rpds-py==0.20.0 +rpds-py==0.21.0 # via # jsonschema # referencing @@ -113,7 +113,7 @@ semantic-version==2.10.0 # via edx-drf-extensions six==1.16.0 # via edx-codejail -sqlparse==0.5.1 +sqlparse==0.5.2 # via django stevedore==5.3.0 # via @@ -128,5 +128,5 @@ urllib3==2.2.3 # via requests # The following packages are considered to be unsafe in a requirements file: -setuptools==75.1.0 +setuptools==75.5.0 # via -r requirements/base.in diff --git a/requirements/ci.txt b/requirements/ci.txt index 3625d4c..e128790 100644 --- a/requirements/ci.txt +++ b/requirements/ci.txt @@ -10,13 +10,13 @@ chardet==5.2.0 # via tox colorama==0.4.6 # via tox -distlib==0.3.8 +distlib==0.3.9 # via virtualenv filelock==3.16.1 # via # tox # virtualenv -packaging==24.1 +packaging==24.2 # via # pyproject-api # tox @@ -28,7 +28,7 @@ pluggy==1.5.0 # via tox pyproject-api==1.8.0 # via tox -tox==4.20.0 +tox==4.23.2 # via -r requirements/ci.in -virtualenv==20.26.5 +virtualenv==20.27.1 # via tox diff --git a/requirements/dev.txt b/requirements/dev.txt index 9f0fb85..15d4c48 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -26,6 +26,10 @@ build==1.2.2.post1 # via # -r requirements/pip-tools.txt # pip-tools +bytecode==0.16.0 + # via + # -r requirements/quality.txt + # ddtrace cachetools==5.5.0 # via # -r requirements/ci.txt @@ -61,7 +65,7 @@ click-log==0.4.0 # via # -r requirements/quality.txt # edx-lint -code-annotations==1.8.0 +code-annotations==1.8.1 # via # -r requirements/quality.txt # edx-lint @@ -70,7 +74,7 @@ colorama==0.4.6 # via # -r requirements/ci.txt # tox -coverage[toml]==7.6.4 +coverage[toml]==7.6.5 # via # -r requirements/quality.txt # pytest-cov @@ -81,6 +85,12 @@ cryptography==43.0.3 # secretstorage ddt==1.7.2 # via -r requirements/quality.txt +ddtrace==2.16.1 + # 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.9 @@ -131,7 +141,7 @@ drf-jwt==1.19.2 # via # -r requirements/quality.txt # edx-drf-extensions -edx-codejail==3.5.1 +edx-codejail==3.5.2 # via -r requirements/quality.txt edx-django-utils==7.0.0 # via @@ -142,7 +152,7 @@ edx-drf-extensions==10.5.0 # via -r requirements/quality.txt edx-i18n-tools==1.6.3 # via -r requirements/dev.in -edx-lint==5.4.0 +edx-lint==5.4.1 # via -r requirements/quality.txt edx-opaque-keys==2.11.0 # via @@ -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.5.0 # via # -r requirements/quality.txt # keyring + # opentelemetry-api # twine iniconfig==2.0.0 # via @@ -208,7 +223,7 @@ lxml[html-clean,html_clean]==5.3.0 # via # edx-i18n-tools # lxml-html-clean -lxml-html-clean==0.3.1 +lxml-html-clean==0.4.0 # via lxml markdown-it-py==3.0.0 # via @@ -239,7 +254,11 @@ nh3==0.2.18 # via # -r requirements/quality.txt # readme-renderer -packaging==24.1 +opentelemetry-api==1.28.1 + # via + # -r requirements/quality.txt + # ddtrace +packaging==24.2 # via # -r requirements/ci.txt # -r requirements/pip-tools.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.1.0 # via # -r requirements/quality.txt @@ -342,7 +365,7 @@ pytest==8.3.3 # pytest-cov # pytest-django # pytest-randomly -pytest-cov==5.0.0 +pytest-cov==6.0.0 # via -r requirements/quality.txt pytest-django==4.9.0 # via -r requirements/quality.txt @@ -380,11 +403,11 @@ rfc3986==2.0.0 # via # -r requirements/quality.txt # twine -rich==13.9.3 +rich==13.9.4 # via # -r requirements/quality.txt # twine -rpds-py==0.20.0 +rpds-py==0.21.0 # via # -r requirements/quality.txt # jsonschema @@ -406,7 +429,7 @@ snowballstemmer==2.2.0 # via # -r requirements/quality.txt # pydocstyle -sqlparse==0.5.1 +sqlparse==0.5.2 # via # -r requirements/quality.txt # django @@ -431,21 +454,31 @@ twine==5.1.1 typing-extensions==4.12.2 # via # -r requirements/quality.txt + # ddtrace # edx-opaque-keys urllib3==2.2.3 # via # -r requirements/quality.txt # requests # twine -virtualenv==20.27.0 +virtualenv==20.27.1 # via # -r requirements/ci.txt # tox -wheel==0.44.0 +wheel==0.45.0 # via # -r requirements/pip-tools.txt # pip-tools -zipp==3.20.2 +wrapt==1.16.0 + # via + # -r requirements/quality.txt + # ddtrace + # deprecated +xmltodict==0.14.2 + # via + # -r requirements/quality.txt + # ddtrace +zipp==3.21.0 # via # -r requirements/quality.txt # importlib-metadata @@ -453,9 +486,10 @@ zipp==3.20.2 # The following packages are considered to be unsafe in a requirements file: pip==24.2 # via + # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/pip-tools.txt # pip-tools -setuptools==75.2.0 +setuptools==75.5.0 # via # -r requirements/pip-tools.txt # -r requirements/quality.txt diff --git a/requirements/doc.txt b/requirements/doc.txt index e695779..90e7779 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -6,7 +6,7 @@ # accessible-pygments==0.0.5 # via pydata-sphinx-theme -alabaster==0.7.16 +alabaster==1.0.0 # via sphinx asgiref==3.8.1 # via @@ -23,18 +23,10 @@ babel==2.16.0 # sphinx beautifulsoup4==4.12.3 # via pydata-sphinx-theme -billiard==4.2.0 - # via - # -r requirements/test.txt - # celery -bytecode==0.15.1 - -bytecode==0.15.1 +bytecode==0.16.0 # via # -r requirements/test.txt # ddtrace -celery==5.4.0 - # via -r requirements/test.txt certifi==2024.8.30 # via # -r requirements/test.txt @@ -53,11 +45,11 @@ click==8.1.7 # -r requirements/test.txt # code-annotations # edx-django-utils -code-annotations==1.8.0 +code-annotations==1.8.1 # via # -r requirements/test.txt # edx-toggles -coverage[toml]==7.6.4 +coverage[toml]==7.6.5 # via # -r requirements/test.txt # pytest-cov @@ -67,9 +59,16 @@ cryptography==43.0.3 # pyjwt ddt==1.7.2 # via -r requirements/test.txt +ddtrace==2.16.1 + # 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 + # -r requirements/test.txt # django-crum # django-waffle # djangorestframework @@ -110,7 +109,7 @@ drf-jwt==1.19.2 # via # -r requirements/test.txt # edx-drf-extensions -edx-codejail==3.5.1 +edx-codejail==3.5.2 # via -r requirements/test.txt edx-django-utils==7.0.0 # via @@ -125,20 +124,20 @@ edx-opaque-keys==2.11.0 # edx-drf-extensions edx-toggles==5.2.0 # via -r requirements/test.txt -envier==0.5.2 +envier==0.6.1 # via # -r requirements/test.txt # ddtrace -exceptiongroup==1.2.2 - # via - # cattrs - # pytest idna==3.10 # via # -r requirements/test.txt # requests imagesize==1.4.1 # via sphinx +importlib-metadata==8.5.0 + # via + # -r requirements/test.txt + # opentelemetry-api iniconfig==2.0.0 # via # -r requirements/test.txt @@ -164,10 +163,13 @@ newrelic==10.2.0 # edx-django-utils nh3==0.2.18 # via readme-renderer -packaging==24.1 +opentelemetry-api==1.28.1 + # via + # -r requirements/test.txt + # ddtrace +packaging==24.2 # via # -r requirements/test.txt - # pydata-sphinx-theme # pytest # sphinx pbr==6.1.0 @@ -178,6 +180,10 @@ pluggy==1.5.0 # via # -r requirements/test.txt # pytest +protobuf==5.28.3 + # via + # -r requirements/test.txt + # ddtrace psutil==6.1.0 # via # -r requirements/test.txt @@ -186,7 +192,7 @@ pycparser==2.22 # via # -r requirements/test.txt # cffi -pydata-sphinx-theme==0.15.4 +pydata-sphinx-theme==0.16.0 # via sphinx-book-theme pygments==2.18.0 # via @@ -214,7 +220,7 @@ pytest==8.3.3 # pytest-cov # pytest-django # pytest-randomly -pytest-cov==5.0.0 +pytest-cov==6.0.0 # via -r requirements/test.txt pytest-django==4.9.0 # via -r requirements/test.txt @@ -242,7 +248,7 @@ requests==2.32.3 # sphinx restructuredtext-lint==1.4.0 # via doc8 -rpds-py==0.20.0 +rpds-py==0.21.0 # via # -r requirements/test.txt # jsonschema @@ -257,9 +263,9 @@ six==1.16.0 # edx-codejail snowballstemmer==2.2.0 # via sphinx -soupsieve==2.5 +soupsieve==2.6 # via beautifulsoup4 -sphinx==5.3.0 +sphinx==8.1.3 # via # -r requirements/doc.in # pydata-sphinx-theme @@ -278,7 +284,7 @@ sphinxcontrib-qthelp==2.0.0 # via sphinx sphinxcontrib-serializinghtml==2.0.0 # via sphinx -sqlparse==0.5.1 +sqlparse==0.5.2 # via # -r requirements/test.txt # django @@ -293,28 +299,30 @@ text-unidecode==1.3 # via # -r requirements/test.txt # python-slugify -tomli==2.0.1 - # via - # coverage - # doc8 - # pytest typing-extensions==4.12.2 # via # -r requirements/test.txt - # pydata-sphinx-theme -tzdata==2024.1 - # via - # -r requirements/test.txt - # celery - # kombu + # ddtrace # edx-opaque-keys + # pydata-sphinx-theme urllib3==2.2.3 # via # -r requirements/test.txt # requests - -# The following packages are considered to be unsafe in a requirements file: -setuptools==75.2.0 +wrapt==1.16.0 # via # -r requirements/test.txt - # sphinx + # ddtrace + # deprecated +xmltodict==0.14.2 + # via + # -r requirements/test.txt + # ddtrace +zipp==3.21.0 + # via + # -r requirements/test.txt + # importlib-metadata + +# The following packages are considered to be unsafe in a requirements file: +setuptools==75.5.0 + # via -r requirements/test.txt diff --git a/requirements/pip-tools.txt b/requirements/pip-tools.txt index 49831ac..1d2a7e5 100644 --- a/requirements/pip-tools.txt +++ b/requirements/pip-tools.txt @@ -4,23 +4,25 @@ # # make upgrade # -build==1.2.2 +build==1.2.2.post1 # via pip-tools click==8.1.7 # via pip-tools -packaging==24.1 +packaging==24.2 # via build pip-tools==7.4.1 # via -r requirements/pip-tools.in -pyproject-hooks==1.1.0 +pyproject-hooks==1.2.0 # via # build # pip-tools -wheel==0.44.0 +wheel==0.45.0 # via pip-tools # The following packages are considered to be unsafe in a requirements file: pip==24.2 - # via pip-tools -setuptools==75.1.0 + # via + # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt + # pip-tools +setuptools==75.5.0 # via pip-tools diff --git a/requirements/pip.txt b/requirements/pip.txt index 36c777e..828273e 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -4,11 +4,11 @@ # # make upgrade # -wheel==0.44.0 +wheel==0.45.0 # via -r requirements/pip.in # The following packages are considered to be unsafe in a requirements file: -pip==24.2 +pip==24.3.1 # via -r requirements/pip.in -setuptools==75.1.0 +setuptools==75.5.0 # via -r requirements/pip.in diff --git a/requirements/quality.txt b/requirements/quality.txt index e82b403..74a0ec6 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.16.0 + # via + # -r requirements/test.txt + # ddtrace certifi==2024.8.30 # via # -r requirements/test.txt @@ -41,12 +45,12 @@ click==8.1.7 # edx-lint click-log==0.4.0 # via edx-lint -code-annotations==1.8.0 +code-annotations==1.8.1 # via # -r requirements/test.txt # edx-lint # edx-toggles -coverage[toml]==7.6.4 +coverage[toml]==7.6.5 # via # -r requirements/test.txt # pytest-cov @@ -57,6 +61,12 @@ cryptography==43.0.3 # secretstorage ddt==1.7.2 # via -r requirements/test.txt +ddtrace==2.16.1 + # via -r requirements/test.txt +deprecated==1.2.14 + # via + # -r requirements/test.txt + # opentelemetry-api dill==0.3.9 # via pylint django==4.2.16 @@ -96,7 +106,7 @@ drf-jwt==1.19.2 # via # -r requirements/test.txt # edx-drf-extensions -edx-codejail==3.5.1 +edx-codejail==3.5.2 # via -r requirements/test.txt edx-django-utils==7.0.0 # via @@ -105,7 +115,7 @@ edx-django-utils==7.0.0 # edx-toggles edx-drf-extensions==10.5.0 # via -r requirements/test.txt -edx-lint==5.4.0 +edx-lint==5.4.1 # via -r requirements/quality.in edx-opaque-keys==2.11.0 # via @@ -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.5.0 # via + # -r requirements/test.txt # keyring + # opentelemetry-api # twine iniconfig==2.0.0 # via @@ -171,7 +187,11 @@ newrelic==10.2.0 # edx-django-utils nh3==0.2.18 # via readme-renderer -packaging==24.1 +opentelemetry-api==1.28.1 + # via + # -r requirements/test.txt + # ddtrace +packaging==24.2 # via # -r requirements/test.txt # pytest @@ -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.1.0 # via # -r requirements/test.txt @@ -236,7 +260,7 @@ pytest==8.3.3 # pytest-cov # pytest-django # pytest-randomly -pytest-cov==5.0.0 +pytest-cov==6.0.0 # via -r requirements/test.txt pytest-django==4.9.0 # via -r requirements/test.txt @@ -267,9 +291,9 @@ requests-toolbelt==1.0.0 # via twine rfc3986==2.0.0 # via twine -rich==13.9.3 +rich==13.9.4 # via twine -rpds-py==0.20.0 +rpds-py==0.21.0 # via # -r requirements/test.txt # jsonschema @@ -287,7 +311,7 @@ six==1.16.0 # edx-lint snowballstemmer==2.2.0 # via pydocstyle -sqlparse==0.5.1 +sqlparse==0.5.2 # via # -r requirements/test.txt # django @@ -308,15 +332,27 @@ 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 -zipp==3.20.2 - # via importlib-metadata +wrapt==1.16.0 + # via + # -r requirements/test.txt + # ddtrace + # deprecated +xmltodict==0.14.2 + # via + # -r requirements/test.txt + # ddtrace +zipp==3.21.0 + # via + # -r requirements/test.txt + # importlib-metadata # The following packages are considered to be unsafe in a requirements file: -setuptools==75.2.0 +setuptools==75.5.0 # via -r requirements/test.txt diff --git a/requirements/scripts.txt b/requirements/scripts.txt index 7cce70b..6d4d7dd 100644 --- a/requirements/scripts.txt +++ b/requirements/scripts.txt @@ -25,7 +25,7 @@ cffi==1.17.1 # -r requirements/base.txt # cryptography # pynacl -charset-normalizer==3.3.2 +charset-normalizer==3.4.0 # via # -r requirements/base.txt # requests @@ -34,13 +34,13 @@ click==8.1.7 # -r requirements/base.txt # code-annotations # edx-django-utils -code-annotations==1.8.0 +code-annotations==1.8.1 # via # -r requirements/base.txt # edx-toggles -confluent-kafka[avro]==2.5.3 +confluent-kafka[avro]==2.6.0 # via -r requirements/scripts.in -cryptography==43.0.1 +cryptography==43.0.3 # via # -r requirements/base.txt # pyjwt @@ -73,7 +73,7 @@ djangorestframework==3.15.2 # -r requirements/base.txt # drf-jwt # edx-drf-extensions -dnspython==2.6.1 +dnspython==2.7.0 # via # -r requirements/base.txt # pymongo @@ -83,9 +83,9 @@ drf-jwt==1.19.2 # edx-drf-extensions edx-ccx-keys==1.3.0 # via openedx-events -edx-codejail==3.4.1 +edx-codejail==3.5.2 # via -r requirements/base.txt -edx-django-utils==5.15.0 +edx-django-utils==7.0.0 # via # -r requirements/base.txt # edx-drf-extensions @@ -120,25 +120,25 @@ jinja2==3.1.4 # code-annotations jsonschema==4.23.0 # via -r requirements/base.txt -jsonschema-specifications==2023.12.1 +jsonschema-specifications==2024.10.1 # via # -r requirements/base.txt # jsonschema -markupsafe==2.1.5 +markupsafe==3.0.2 # via # -r requirements/base.txt # jinja2 -newrelic==9.13.0 +newrelic==10.2.0 # via # -r requirements/base.txt # edx-django-utils -openedx-events==9.14.1 +openedx-events==9.15.0 # via edx-event-bus-kafka pbr==6.1.0 # via # -r requirements/base.txt # stevedore -psutil==6.0.0 +psutil==6.1.0 # via # -r requirements/base.txt # edx-django-utils @@ -151,7 +151,7 @@ pyjwt[crypto]==2.9.0 # -r requirements/base.txt # drf-jwt # edx-drf-extensions -pymongo==4.9.1 +pymongo==4.10.1 # via # -r requirements/base.txt # edx-opaque-keys @@ -177,7 +177,7 @@ requests==2.32.3 # -r requirements/base.txt # confluent-kafka # edx-drf-extensions -rpds-py==0.20.0 +rpds-py==0.21.0 # via # -r requirements/base.txt # jsonschema @@ -191,7 +191,7 @@ six==1.16.0 # -r requirements/base.txt # edx-ccx-keys # edx-codejail -sqlparse==0.5.1 +sqlparse==0.5.2 # via # -r requirements/base.txt # django @@ -215,5 +215,5 @@ urllib3==2.2.3 # requests # The following packages are considered to be unsafe in a requirements file: -setuptools==75.1.0 +setuptools==75.5.0 # via -r requirements/base.txt diff --git a/requirements/test.txt b/requirements/test.txt index 117a3c5..1cecc5c 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.16.0 + # via ddtrace certifi==2024.8.30 # via # -r requirements/base.txt @@ -31,12 +33,12 @@ click==8.1.7 # -r requirements/base.txt # code-annotations # edx-django-utils -code-annotations==1.8.0 +code-annotations==1.8.1 # via # -r requirements/base.txt # -r requirements/test.in # edx-toggles -coverage[toml]==7.6.4 +coverage[toml]==7.6.5 # via pytest-cov cryptography==43.0.3 # via @@ -44,6 +46,10 @@ cryptography==43.0.3 # pyjwt ddt==1.7.2 # via -r requirements/test.in +ddtrace==2.16.1 + # 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 @@ -78,7 +84,7 @@ drf-jwt==1.19.2 # via # -r requirements/base.txt # edx-drf-extensions -edx-codejail==3.5.1 +edx-codejail==3.5.2 # via -r requirements/base.txt edx-django-utils==7.0.0 # via @@ -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.5.0 + # via opentelemetry-api iniconfig==2.0.0 # via pytest jinja2==3.1.4 @@ -117,7 +127,9 @@ newrelic==10.2.0 # via # -r requirements/base.txt # edx-django-utils -packaging==24.1 +opentelemetry-api==1.28.1 + # via ddtrace +packaging==24.2 # via pytest pbr==6.1.0 # via @@ -125,6 +137,8 @@ pbr==6.1.0 # stevedore pluggy==1.5.0 # via pytest +protobuf==5.28.3 + # via ddtrace psutil==6.1.0 # via # -r requirements/base.txt @@ -151,7 +165,7 @@ pytest==8.3.3 # pytest-cov # pytest-django # pytest-randomly -pytest-cov==5.0.0 +pytest-cov==6.0.0 # via -r requirements/test.in pytest-django==4.9.0 # via -r requirements/test.in @@ -174,7 +188,7 @@ requests==2.32.3 # via # -r requirements/base.txt # edx-drf-extensions -rpds-py==0.20.0 +rpds-py==0.21.0 # via # -r requirements/base.txt # jsonschema @@ -187,7 +201,7 @@ six==1.16.0 # via # -r requirements/base.txt # edx-codejail -sqlparse==0.5.1 +sqlparse==0.5.2 # via # -r requirements/base.txt # django @@ -204,12 +218,21 @@ 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.21.0 + # via importlib-metadata # The following packages are considered to be unsafe in a requirements file: -setuptools==75.2.0 +setuptools==75.5.0 # via -r requirements/base.txt From 16f008d263aa9ba9c5bd70a8ef6d588e1f95453d Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Thu, 14 Nov 2024 15:58:45 -0500 Subject: [PATCH 07/19] fixup! make pylint happy --- .../datadog_monitoring/tests/code_owner/test_middleware.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 54c5830..eb4313d 100644 --- a/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_middleware.py +++ b/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_middleware.py @@ -132,7 +132,7 @@ 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 - ): + ): # pylint: disable=too-many-positional-arguments """ Performs a set of assertions around having set the proper custom attributes. """ call_list = [] if expected_code_owner: From d76bc9dc7b15c242a4a52826dab39d027096257a Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Wed, 20 Nov 2024 14:13:52 -0500 Subject: [PATCH 08/19] fixup! add celery as dependency --- requirements/base.in | 5 ++- requirements/base.txt | 45 +++++++++++++++++++--- requirements/dev.txt | 82 ++++++++++++++++++++++++++++++---------- requirements/doc.txt | 70 ++++++++++++++++++++++++++++++---- requirements/quality.txt | 77 +++++++++++++++++++++++++++++-------- requirements/scripts.txt | 64 ++++++++++++++++++++++++++++--- requirements/test.txt | 70 ++++++++++++++++++++++++++++++---- 7 files changed, 351 insertions(+), 62 deletions(-) diff --git a/requirements/base.in b/requirements/base.in index 0c30bbe..3a3af4f 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -1,8 +1,9 @@ # Core requirements for using this application -c constraints.txt -Django # Web application framework -edx_django_utils +celery # Asynchronous task execution library +Django # Web application framework +edx_django_utils # Basic utilities for plugins, monitoring, and more django-waffle # Configuration switches and flags -- used by config_watcher app edx-codejail # Actual codejail library; used by codejail_service app djangorestframework # Used by codejail_service app diff --git a/requirements/base.txt b/requirements/base.txt index bcf4cd1..a569ee4 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -4,12 +4,18 @@ # # make upgrade # +amqp==5.3.1 + # via kombu asgiref==3.8.1 # via django attrs==24.2.0 # via # jsonschema # referencing +billiard==4.2.1 + # via celery +celery==5.4.0 + # via -r requirements/base.in certifi==2024.8.30 # via requests cffi==1.17.1 @@ -20,9 +26,19 @@ charset-normalizer==3.4.0 # via requests click==8.1.7 # via + # celery + # click-didyoumean + # click-plugins + # click-repl # code-annotations # edx-django-utils -code-annotations==1.8.1 +click-didyoumean==0.3.1 + # via celery +click-plugins==1.1.1 + # via celery +click-repl==0.3.0 + # via celery +code-annotations==1.8.2 # via edx-toggles cryptography==43.0.3 # via pyjwt @@ -41,7 +57,7 @@ django-crum==0.7.9 # via # edx-django-utils # edx-toggles -django-waffle==4.1.0 +django-waffle==4.2.0 # via # -r requirements/base.in # edx-django-utils @@ -77,17 +93,21 @@ jsonschema==4.23.0 # via -r requirements/base.in jsonschema-specifications==2024.10.1 # via jsonschema +kombu==5.4.2 + # via celery markupsafe==3.0.2 # via jinja2 -newrelic==10.2.0 +newrelic==10.3.0 # via edx-django-utils pbr==6.1.0 # via stevedore +prompt-toolkit==3.0.48 + # via click-repl psutil==6.1.0 # via edx-django-utils pycparser==2.22 # via cffi -pyjwt[crypto]==2.9.0 +pyjwt[crypto]==2.10.0 # via # drf-jwt # edx-drf-extensions @@ -95,6 +115,8 @@ pymongo==4.10.1 # via edx-opaque-keys pynacl==1.5.0 # via edx-django-utils +python-dateutil==2.9.0.post0 + # via celery python-slugify==8.0.4 # via code-annotations pyyaml==6.0.2 @@ -112,7 +134,9 @@ rpds-py==0.21.0 semantic-version==2.10.0 # via edx-drf-extensions six==1.16.0 - # via edx-codejail + # via + # edx-codejail + # python-dateutil sqlparse==0.5.2 # via django stevedore==5.3.0 @@ -124,8 +148,19 @@ text-unidecode==1.3 # via python-slugify typing-extensions==4.12.2 # via edx-opaque-keys +tzdata==2024.2 + # via + # celery + # kombu urllib3==2.2.3 # via requests +vine==5.1.0 + # via + # amqp + # celery + # kombu +wcwidth==0.2.13 + # via prompt-toolkit # The following packages are considered to be unsafe in a requirements file: setuptools==75.5.0 diff --git a/requirements/dev.txt b/requirements/dev.txt index 15d4c48..2a24e01 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -4,6 +4,10 @@ # # make upgrade # +amqp==5.3.1 + # via + # -r requirements/quality.txt + # kombu asgiref==3.8.1 # via # -r requirements/quality.txt @@ -22,6 +26,10 @@ backports-tarfile==1.2.0 # via # -r requirements/quality.txt # jaraco-context +billiard==4.2.1 + # via + # -r requirements/quality.txt + # celery build==1.2.2.post1 # via # -r requirements/pip-tools.txt @@ -34,6 +42,8 @@ cachetools==5.5.0 # via # -r requirements/ci.txt # tox +celery==5.4.0 + # via -r requirements/quality.txt certifi==2024.8.30 # via # -r requirements/quality.txt @@ -56,16 +66,32 @@ click==8.1.7 # via # -r requirements/pip-tools.txt # -r requirements/quality.txt + # celery + # click-didyoumean # click-log + # click-plugins + # click-repl # code-annotations # edx-django-utils # edx-lint # pip-tools +click-didyoumean==0.3.1 + # via + # -r requirements/quality.txt + # celery click-log==0.4.0 # via # -r requirements/quality.txt # edx-lint -code-annotations==1.8.1 +click-plugins==1.1.1 + # via + # -r requirements/quality.txt + # celery +click-repl==0.3.0 + # via + # -r requirements/quality.txt + # celery +code-annotations==1.8.2 # via # -r requirements/quality.txt # edx-lint @@ -74,7 +100,7 @@ colorama==0.4.6 # via # -r requirements/ci.txt # tox -coverage[toml]==7.6.5 +coverage[toml]==7.6.7 # via # -r requirements/quality.txt # pytest-cov @@ -82,12 +108,11 @@ cryptography==43.0.3 # via # -r requirements/quality.txt # pyjwt - # secretstorage ddt==1.7.2 # via -r requirements/quality.txt -ddtrace==2.16.1 +ddtrace==2.16.3 # via -r requirements/quality.txt -deprecated==1.2.14 +deprecated==1.2.15 # via # -r requirements/quality.txt # opentelemetry-api @@ -118,7 +143,7 @@ django-crum==0.7.9 # -r requirements/quality.txt # edx-django-utils # edx-toggles -django-waffle==4.1.0 +django-waffle==4.2.0 # via # -r requirements/quality.txt # edx-django-utils @@ -199,11 +224,6 @@ jaraco-functools==4.1.0 # via # -r requirements/quality.txt # keyring -jeepney==0.8.0 - # via - # -r requirements/quality.txt - # keyring - # secretstorage jinja2==3.1.4 # via # -r requirements/quality.txt @@ -219,11 +239,15 @@ keyring==25.5.0 # via # -r requirements/quality.txt # twine +kombu==5.4.2 + # via + # -r requirements/quality.txt + # celery lxml[html-clean,html_clean]==5.3.0 # via # edx-i18n-tools # lxml-html-clean -lxml-html-clean==0.4.0 +lxml-html-clean==0.4.1 # via lxml markdown-it-py==3.0.0 # via @@ -246,7 +270,7 @@ more-itertools==10.5.0 # -r requirements/quality.txt # jaraco-classes # jaraco-functools -newrelic==10.2.0 +newrelic==10.3.0 # via # -r requirements/quality.txt # edx-django-utils @@ -254,7 +278,7 @@ nh3==0.2.18 # via # -r requirements/quality.txt # readme-renderer -opentelemetry-api==1.28.1 +opentelemetry-api==1.28.2 # via # -r requirements/quality.txt # ddtrace @@ -295,6 +319,10 @@ pluggy==1.5.0 # tox polib==1.2.0 # via edx-i18n-tools +prompt-toolkit==3.0.48 + # via + # -r requirements/quality.txt + # click-repl protobuf==5.28.3 # via # -r requirements/quality.txt @@ -317,7 +345,7 @@ pygments==2.18.0 # diff-cover # readme-renderer # rich -pyjwt[crypto]==2.9.0 +pyjwt[crypto]==2.10.0 # via # -r requirements/quality.txt # drf-jwt @@ -371,6 +399,10 @@ pytest-django==4.9.0 # via -r requirements/quality.txt pytest-randomly==3.16.0 # via -r requirements/quality.txt +python-dateutil==2.9.0.post0 + # via + # -r requirements/quality.txt + # celery python-slugify==8.0.4 # via # -r requirements/quality.txt @@ -412,10 +444,6 @@ rpds-py==0.21.0 # -r requirements/quality.txt # jsonschema # referencing -secretstorage==3.3.3 - # via - # -r requirements/quality.txt - # keyring semantic-version==2.10.0 # via # -r requirements/quality.txt @@ -425,6 +453,7 @@ six==1.16.0 # -r requirements/quality.txt # edx-codejail # edx-lint + # python-dateutil snowballstemmer==2.2.0 # via # -r requirements/quality.txt @@ -456,15 +485,30 @@ typing-extensions==4.12.2 # -r requirements/quality.txt # ddtrace # edx-opaque-keys +tzdata==2024.2 + # via + # -r requirements/quality.txt + # celery + # kombu urllib3==2.2.3 # via # -r requirements/quality.txt # requests # twine +vine==5.1.0 + # via + # -r requirements/quality.txt + # amqp + # celery + # kombu virtualenv==20.27.1 # via # -r requirements/ci.txt # tox +wcwidth==0.2.13 + # via + # -r requirements/quality.txt + # prompt-toolkit wheel==0.45.0 # via # -r requirements/pip-tools.txt diff --git a/requirements/doc.txt b/requirements/doc.txt index 90e7779..e239cb8 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -8,6 +8,10 @@ accessible-pygments==0.0.5 # via pydata-sphinx-theme alabaster==1.0.0 # via sphinx +amqp==5.3.1 + # via + # -r requirements/test.txt + # kombu asgiref==3.8.1 # via # -r requirements/test.txt @@ -23,10 +27,16 @@ babel==2.16.0 # sphinx beautifulsoup4==4.12.3 # via pydata-sphinx-theme +billiard==4.2.1 + # via + # -r requirements/test.txt + # celery bytecode==0.16.0 # via # -r requirements/test.txt # ddtrace +celery==5.4.0 + # via -r requirements/test.txt certifi==2024.8.30 # via # -r requirements/test.txt @@ -43,13 +53,29 @@ charset-normalizer==3.4.0 click==8.1.7 # via # -r requirements/test.txt + # celery + # click-didyoumean + # click-plugins + # click-repl # code-annotations # edx-django-utils -code-annotations==1.8.1 +click-didyoumean==0.3.1 + # via + # -r requirements/test.txt + # celery +click-plugins==1.1.1 + # via + # -r requirements/test.txt + # celery +click-repl==0.3.0 + # via + # -r requirements/test.txt + # celery +code-annotations==1.8.2 # via # -r requirements/test.txt # edx-toggles -coverage[toml]==7.6.5 +coverage[toml]==7.6.7 # via # -r requirements/test.txt # pytest-cov @@ -59,9 +85,9 @@ cryptography==43.0.3 # pyjwt ddt==1.7.2 # via -r requirements/test.txt -ddtrace==2.16.1 +ddtrace==2.16.3 # via -r requirements/test.txt -deprecated==1.2.14 +deprecated==1.2.15 # via # -r requirements/test.txt # opentelemetry-api @@ -81,7 +107,7 @@ django-crum==0.7.9 # -r requirements/test.txt # edx-django-utils # edx-toggles -django-waffle==4.1.0 +django-waffle==4.2.0 # via # -r requirements/test.txt # edx-django-utils @@ -153,17 +179,21 @@ jsonschema-specifications==2024.10.1 # via # -r requirements/test.txt # jsonschema +kombu==5.4.2 + # via + # -r requirements/test.txt + # celery markupsafe==3.0.2 # via # -r requirements/test.txt # jinja2 -newrelic==10.2.0 +newrelic==10.3.0 # via # -r requirements/test.txt # edx-django-utils nh3==0.2.18 # via readme-renderer -opentelemetry-api==1.28.1 +opentelemetry-api==1.28.2 # via # -r requirements/test.txt # ddtrace @@ -180,6 +210,10 @@ pluggy==1.5.0 # via # -r requirements/test.txt # pytest +prompt-toolkit==3.0.48 + # via + # -r requirements/test.txt + # click-repl protobuf==5.28.3 # via # -r requirements/test.txt @@ -201,7 +235,7 @@ pygments==2.18.0 # pydata-sphinx-theme # readme-renderer # sphinx -pyjwt[crypto]==2.9.0 +pyjwt[crypto]==2.10.0 # via # -r requirements/test.txt # drf-jwt @@ -226,6 +260,10 @@ pytest-django==4.9.0 # via -r requirements/test.txt pytest-randomly==3.16.0 # via -r requirements/test.txt +python-dateutil==2.9.0.post0 + # via + # -r requirements/test.txt + # celery python-slugify==8.0.4 # via # -r requirements/test.txt @@ -261,6 +299,7 @@ six==1.16.0 # via # -r requirements/test.txt # edx-codejail + # python-dateutil snowballstemmer==2.2.0 # via sphinx soupsieve==2.6 @@ -305,10 +344,25 @@ typing-extensions==4.12.2 # ddtrace # edx-opaque-keys # pydata-sphinx-theme +tzdata==2024.2 + # via + # -r requirements/test.txt + # celery + # kombu urllib3==2.2.3 # via # -r requirements/test.txt # requests +vine==5.1.0 + # via + # -r requirements/test.txt + # amqp + # celery + # kombu +wcwidth==0.2.13 + # via + # -r requirements/test.txt + # prompt-toolkit wrapt==1.16.0 # via # -r requirements/test.txt diff --git a/requirements/quality.txt b/requirements/quality.txt index 74a0ec6..ce5ba05 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -4,6 +4,10 @@ # # make upgrade # +amqp==5.3.1 + # via + # -r requirements/test.txt + # kombu asgiref==3.8.1 # via # -r requirements/test.txt @@ -19,10 +23,16 @@ attrs==24.2.0 # referencing backports-tarfile==1.2.0 # via jaraco-context +billiard==4.2.1 + # via + # -r requirements/test.txt + # celery bytecode==0.16.0 # via # -r requirements/test.txt # ddtrace +celery==5.4.0 + # via -r requirements/test.txt certifi==2024.8.30 # via # -r requirements/test.txt @@ -39,18 +49,34 @@ charset-normalizer==3.4.0 click==8.1.7 # via # -r requirements/test.txt + # celery + # click-didyoumean # click-log + # click-plugins + # click-repl # code-annotations # edx-django-utils # edx-lint +click-didyoumean==0.3.1 + # via + # -r requirements/test.txt + # celery click-log==0.4.0 # via edx-lint -code-annotations==1.8.1 +click-plugins==1.1.1 + # via + # -r requirements/test.txt + # celery +click-repl==0.3.0 + # via + # -r requirements/test.txt + # celery +code-annotations==1.8.2 # via # -r requirements/test.txt # edx-lint # edx-toggles -coverage[toml]==7.6.5 +coverage[toml]==7.6.7 # via # -r requirements/test.txt # pytest-cov @@ -58,12 +84,11 @@ cryptography==43.0.3 # via # -r requirements/test.txt # pyjwt - # secretstorage ddt==1.7.2 # via -r requirements/test.txt -ddtrace==2.16.1 +ddtrace==2.16.3 # via -r requirements/test.txt -deprecated==1.2.14 +deprecated==1.2.15 # via # -r requirements/test.txt # opentelemetry-api @@ -85,7 +110,7 @@ django-crum==0.7.9 # -r requirements/test.txt # edx-django-utils # edx-toggles -django-waffle==4.1.0 +django-waffle==4.2.0 # via # -r requirements/test.txt # edx-django-utils @@ -151,10 +176,6 @@ jaraco-context==6.0.1 # via keyring jaraco-functools==4.1.0 # via keyring -jeepney==0.8.0 - # via - # keyring - # secretstorage jinja2==3.1.4 # via # -r requirements/test.txt @@ -167,6 +188,10 @@ jsonschema-specifications==2024.10.1 # jsonschema keyring==25.5.0 # via twine +kombu==5.4.2 + # via + # -r requirements/test.txt + # celery markdown-it-py==3.0.0 # via rich markupsafe==3.0.2 @@ -181,13 +206,13 @@ more-itertools==10.5.0 # via # jaraco-classes # jaraco-functools -newrelic==10.2.0 +newrelic==10.3.0 # via # -r requirements/test.txt # edx-django-utils nh3==0.2.18 # via readme-renderer -opentelemetry-api==1.28.1 +opentelemetry-api==1.28.2 # via # -r requirements/test.txt # ddtrace @@ -207,6 +232,10 @@ pluggy==1.5.0 # via # -r requirements/test.txt # pytest +prompt-toolkit==3.0.48 + # via + # -r requirements/test.txt + # click-repl protobuf==5.28.3 # via # -r requirements/test.txt @@ -227,7 +256,7 @@ pygments==2.18.0 # via # readme-renderer # rich -pyjwt[crypto]==2.9.0 +pyjwt[crypto]==2.10.0 # via # -r requirements/test.txt # drf-jwt @@ -266,6 +295,10 @@ pytest-django==4.9.0 # via -r requirements/test.txt pytest-randomly==3.16.0 # via -r requirements/test.txt +python-dateutil==2.9.0.post0 + # via + # -r requirements/test.txt + # celery python-slugify==8.0.4 # via # -r requirements/test.txt @@ -298,8 +331,6 @@ rpds-py==0.21.0 # -r requirements/test.txt # jsonschema # referencing -secretstorage==3.3.3 - # via keyring semantic-version==2.10.0 # via # -r requirements/test.txt @@ -309,6 +340,7 @@ six==1.16.0 # -r requirements/test.txt # edx-codejail # edx-lint + # python-dateutil snowballstemmer==2.2.0 # via pydocstyle sqlparse==0.5.2 @@ -334,11 +366,26 @@ typing-extensions==4.12.2 # -r requirements/test.txt # ddtrace # edx-opaque-keys +tzdata==2024.2 + # via + # -r requirements/test.txt + # celery + # kombu urllib3==2.2.3 # via # -r requirements/test.txt # requests # twine +vine==5.1.0 + # via + # -r requirements/test.txt + # amqp + # celery + # kombu +wcwidth==0.2.13 + # via + # -r requirements/test.txt + # prompt-toolkit wrapt==1.16.0 # via # -r requirements/test.txt diff --git a/requirements/scripts.txt b/requirements/scripts.txt index 6d4d7dd..b8c51f3 100644 --- a/requirements/scripts.txt +++ b/requirements/scripts.txt @@ -4,6 +4,10 @@ # # make upgrade # +amqp==5.3.1 + # via + # -r requirements/base.txt + # kombu asgiref==3.8.1 # via # -r requirements/base.txt @@ -16,6 +20,12 @@ attrs==24.2.0 # referencing avro==1.12.0 # via confluent-kafka +billiard==4.2.1 + # via + # -r requirements/base.txt + # celery +celery==5.4.0 + # via -r requirements/base.txt certifi==2024.8.30 # via # -r requirements/base.txt @@ -32,13 +42,29 @@ charset-normalizer==3.4.0 click==8.1.7 # via # -r requirements/base.txt + # celery + # click-didyoumean + # click-plugins + # click-repl # code-annotations # edx-django-utils -code-annotations==1.8.1 +click-didyoumean==0.3.1 + # via + # -r requirements/base.txt + # celery +click-plugins==1.1.1 + # via + # -r requirements/base.txt + # celery +click-repl==0.3.0 + # via + # -r requirements/base.txt + # celery +code-annotations==1.8.2 # via # -r requirements/base.txt # edx-toggles -confluent-kafka[avro]==2.6.0 +confluent-kafka[avro]==2.6.1 # via -r requirements/scripts.in cryptography==43.0.3 # via @@ -62,7 +88,7 @@ django-crum==0.7.9 # -r requirements/base.txt # edx-django-utils # edx-toggles -django-waffle==4.1.0 +django-waffle==4.2.0 # via # -r requirements/base.txt # edx-django-utils @@ -124,11 +150,15 @@ jsonschema-specifications==2024.10.1 # via # -r requirements/base.txt # jsonschema +kombu==5.4.2 + # via + # -r requirements/base.txt + # celery markupsafe==3.0.2 # via # -r requirements/base.txt # jinja2 -newrelic==10.2.0 +newrelic==10.3.0 # via # -r requirements/base.txt # edx-django-utils @@ -138,6 +168,10 @@ pbr==6.1.0 # via # -r requirements/base.txt # stevedore +prompt-toolkit==3.0.48 + # via + # -r requirements/base.txt + # click-repl psutil==6.1.0 # via # -r requirements/base.txt @@ -146,7 +180,7 @@ pycparser==2.22 # via # -r requirements/base.txt # cffi -pyjwt[crypto]==2.9.0 +pyjwt[crypto]==2.10.0 # via # -r requirements/base.txt # drf-jwt @@ -159,6 +193,10 @@ pynacl==1.5.0 # via # -r requirements/base.txt # edx-django-utils +python-dateutil==2.9.0.post0 + # via + # -r requirements/base.txt + # celery python-slugify==8.0.4 # via # -r requirements/base.txt @@ -191,6 +229,7 @@ six==1.16.0 # -r requirements/base.txt # edx-ccx-keys # edx-codejail + # python-dateutil sqlparse==0.5.2 # via # -r requirements/base.txt @@ -209,10 +248,25 @@ typing-extensions==4.12.2 # via # -r requirements/base.txt # edx-opaque-keys +tzdata==2024.2 + # via + # -r requirements/base.txt + # celery + # kombu urllib3==2.2.3 # via # -r requirements/base.txt # requests +vine==5.1.0 + # via + # -r requirements/base.txt + # amqp + # celery + # kombu +wcwidth==0.2.13 + # via + # -r requirements/base.txt + # prompt-toolkit # The following packages are considered to be unsafe in a requirements file: setuptools==75.5.0 diff --git a/requirements/test.txt b/requirements/test.txt index 1cecc5c..8c1c3ef 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -4,6 +4,10 @@ # # make upgrade # +amqp==5.3.1 + # via + # -r requirements/base.txt + # kombu asgiref==3.8.1 # via # -r requirements/base.txt @@ -13,8 +17,14 @@ attrs==24.2.0 # -r requirements/base.txt # jsonschema # referencing +billiard==4.2.1 + # via + # -r requirements/base.txt + # celery bytecode==0.16.0 # via ddtrace +celery==5.4.0 + # via -r requirements/base.txt certifi==2024.8.30 # via # -r requirements/base.txt @@ -31,14 +41,30 @@ charset-normalizer==3.4.0 click==8.1.7 # via # -r requirements/base.txt + # celery + # click-didyoumean + # click-plugins + # click-repl # code-annotations # edx-django-utils -code-annotations==1.8.1 +click-didyoumean==0.3.1 + # via + # -r requirements/base.txt + # celery +click-plugins==1.1.1 + # via + # -r requirements/base.txt + # celery +click-repl==0.3.0 + # via + # -r requirements/base.txt + # celery +code-annotations==1.8.2 # via # -r requirements/base.txt # -r requirements/test.in # edx-toggles -coverage[toml]==7.6.5 +coverage[toml]==7.6.7 # via pytest-cov cryptography==43.0.3 # via @@ -46,9 +72,9 @@ cryptography==43.0.3 # pyjwt ddt==1.7.2 # via -r requirements/test.in -ddtrace==2.16.1 +ddtrace==2.16.3 # via -r requirements/test.in -deprecated==1.2.14 +deprecated==1.2.15 # via opentelemetry-api # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt @@ -65,7 +91,7 @@ django-crum==0.7.9 # -r requirements/base.txt # edx-django-utils # edx-toggles -django-waffle==4.1.0 +django-waffle==4.2.0 # via # -r requirements/base.txt # edx-django-utils @@ -119,15 +145,19 @@ jsonschema-specifications==2024.10.1 # via # -r requirements/base.txt # jsonschema +kombu==5.4.2 + # via + # -r requirements/base.txt + # celery markupsafe==3.0.2 # via # -r requirements/base.txt # jinja2 -newrelic==10.2.0 +newrelic==10.3.0 # via # -r requirements/base.txt # edx-django-utils -opentelemetry-api==1.28.1 +opentelemetry-api==1.28.2 # via ddtrace packaging==24.2 # via pytest @@ -137,6 +167,10 @@ pbr==6.1.0 # stevedore pluggy==1.5.0 # via pytest +prompt-toolkit==3.0.48 + # via + # -r requirements/base.txt + # click-repl protobuf==5.28.3 # via ddtrace psutil==6.1.0 @@ -147,7 +181,7 @@ pycparser==2.22 # via # -r requirements/base.txt # cffi -pyjwt[crypto]==2.9.0 +pyjwt[crypto]==2.10.0 # via # -r requirements/base.txt # drf-jwt @@ -171,6 +205,10 @@ pytest-django==4.9.0 # via -r requirements/test.in pytest-randomly==3.16.0 # via -r requirements/test.in +python-dateutil==2.9.0.post0 + # via + # -r requirements/base.txt + # celery python-slugify==8.0.4 # via # -r requirements/base.txt @@ -201,6 +239,7 @@ six==1.16.0 # via # -r requirements/base.txt # edx-codejail + # python-dateutil sqlparse==0.5.2 # via # -r requirements/base.txt @@ -220,10 +259,25 @@ typing-extensions==4.12.2 # -r requirements/base.txt # ddtrace # edx-opaque-keys +tzdata==2024.2 + # via + # -r requirements/base.txt + # celery + # kombu urllib3==2.2.3 # via # -r requirements/base.txt # requests +vine==5.1.0 + # via + # -r requirements/base.txt + # amqp + # celery + # kombu +wcwidth==0.2.13 + # via + # -r requirements/base.txt + # prompt-toolkit wrapt==1.16.0 # via # ddtrace From 96148291afe624b932d1d4684003c4d8406193a5 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Wed, 20 Nov 2024 16:53:15 -0500 Subject: [PATCH 09/19] fixup! add span processor from celery signal --- .../datadog_monitoring/apps.py | 45 ++---------------- .../datadog_monitoring/code_owner/datadog.py | 26 +++++++++++ .../datadog_monitoring/signals/handlers.py | 28 +++++++++++ .../test_datadog.py} | 46 ++++++------------- .../tests/signals/test_handlers.py | 33 +++++++++++++ .../datadog_monitoring/tests/test_apps.py | 22 +++++++++ setup.py | 2 + 7 files changed, 127 insertions(+), 75 deletions(-) create mode 100644 edx_arch_experiments/datadog_monitoring/code_owner/datadog.py create mode 100644 edx_arch_experiments/datadog_monitoring/signals/handlers.py rename edx_arch_experiments/datadog_monitoring/tests/{test_app.py => code_owner/test_datadog.py} (52%) create mode 100644 edx_arch_experiments/datadog_monitoring/tests/signals/test_handlers.py create mode 100644 edx_arch_experiments/datadog_monitoring/tests/test_apps.py diff --git a/edx_arch_experiments/datadog_monitoring/apps.py b/edx_arch_experiments/datadog_monitoring/apps.py index 41ce467..5051e14 100644 --- a/edx_arch_experiments/datadog_monitoring/apps.py +++ b/edx_arch_experiments/datadog_monitoring/apps.py @@ -1,40 +1,8 @@ """ App for 2U-specific edx-platform Datadog monitoring. """ - -import logging - from django.apps import AppConfig -from .code_owner.utils import set_code_owner_attribute_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. - set_code_owner_attribute_from_module(span.resource) - - def on_span_finish(self, span): - pass - - def shutdown(self, _timeout): - pass - class DatadogMonitoring(AppConfig): """ @@ -46,13 +14,6 @@ class DatadogMonitoring(AppConfig): 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." - ) + # Implicitly connect signal handlers decorated with @receiver + # pylint: disable=import-outside-toplevel,unused-import + from edx_arch_experiments.datadog_monitoring.signals import handlers diff --git a/edx_arch_experiments/datadog_monitoring/code_owner/datadog.py b/edx_arch_experiments/datadog_monitoring/code_owner/datadog.py new file mode 100644 index 0000000..9802010 --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/code_owner/datadog.py @@ -0,0 +1,26 @@ +""" +Datadog span processor for celery span code owners. +""" +from .utils import set_code_owner_attribute_from_module + + +class CeleryCodeOwnerSpanProcessor: + """ + Datadog span processor that adds celery code owner span tags. + """ + + def on_span_start(self, span): + """ + Adds code owner span tag for celery run spans at span creation. + """ + if hasattr(span, 'name') and 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. + set_code_owner_attribute_from_module(span.resource) + + def on_span_finish(self, span): + pass + + def shutdown(self, _timeout): + pass diff --git a/edx_arch_experiments/datadog_monitoring/signals/handlers.py b/edx_arch_experiments/datadog_monitoring/signals/handlers.py new file mode 100644 index 0000000..df96a38 --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/signals/handlers.py @@ -0,0 +1,28 @@ +""" +Handlers to listen to celery signals. +""" +import logging + +from celery.signals import worker_process_init +from django.dispatch import receiver + +from edx_arch_experiments.datadog_monitoring.code_owner.datadog import CeleryCodeOwnerSpanProcessor + +log = logging.getLogger(__name__) + + +@receiver(worker_process_init, weak=False) +def init_worker_process(sender, **kwargs): + """ + Adds a Datadog span processor to each worker process. + """ + try: + from ddtrace import tracer # pylint: disable=import-outside-toplevel + + tracer._span_processors.append(CeleryCodeOwnerSpanProcessor()) # pylint: disable=protected-access + log.info("Attached CeleryCodeOwnerSpanProcessor") + except ImportError: + log.warning( + "Unable to attach CeleryCodeOwnerSpanProcessor" + " -- ddtrace module not found." + ) diff --git a/edx_arch_experiments/datadog_monitoring/tests/test_app.py b/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_datadog.py similarity index 52% rename from edx_arch_experiments/datadog_monitoring/tests/test_app.py rename to edx_arch_experiments/datadog_monitoring/tests/code_owner/test_datadog.py index e835b52..4769e0d 100644 --- a/edx_arch_experiments/datadog_monitoring/tests/test_app.py +++ b/edx_arch_experiments/datadog_monitoring/tests/code_owner/test_datadog.py @@ -1,52 +1,32 @@ """ -Tests for plugin app. +Tests for datadog span processor. """ from unittest.mock import patch -from ddtrace import tracer from django.test import TestCase -from .. import apps +from edx_arch_experiments.datadog_monitoring.code_owner.datadog import CeleryCodeOwnerSpanProcessor class FakeSpan: - """A fake Span instance with span name and resource.""" + """ + 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.""" +class TestCeleryCodeOwnerSpanProcessor(TestCase): + """ + Tests for CeleryCodeOwnerSpanProcessor. + """ @patch('edx_arch_experiments.datadog_monitoring.code_owner.utils.set_custom_attribute') def test_celery_span(self, mock_set_custom_attribute): """ Tests processor with a celery span. """ - proc = apps.DatadogMonitoringSpanProcessor() + proc = CeleryCodeOwnerSpanProcessor() celery_span = FakeSpan('celery.run', 'test.module.for.celery.task') proc.on_span_start(celery_span) @@ -56,7 +36,7 @@ def test_celery_span(self, mock_set_custom_attribute): @patch('edx_arch_experiments.datadog_monitoring.code_owner.utils.set_custom_attribute') def test_other_span(self, mock_set_custom_attribute): """ Tests processor with a non-celery span. """ - proc = apps.DatadogMonitoringSpanProcessor() + proc = CeleryCodeOwnerSpanProcessor() celery_span = FakeSpan('other.span', 'test.resource.name') proc.on_span_start(celery_span) @@ -66,7 +46,7 @@ def test_other_span(self, mock_set_custom_attribute): @patch('edx_arch_experiments.datadog_monitoring.code_owner.utils.set_custom_attribute') def test_non_span(self, mock_set_custom_attribute): """ Tests processor with an object that doesn't have span name or resource. """ - proc = apps.DatadogMonitoringSpanProcessor() + proc = CeleryCodeOwnerSpanProcessor() non_span = object() proc.on_span_start(non_span) diff --git a/edx_arch_experiments/datadog_monitoring/tests/signals/test_handlers.py b/edx_arch_experiments/datadog_monitoring/tests/signals/test_handlers.py new file mode 100644 index 0000000..c0816a7 --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/tests/signals/test_handlers.py @@ -0,0 +1,33 @@ +""" +Tests for celery signal handler. +""" +from ddtrace import tracer +from django.test import TestCase + +from edx_arch_experiments.datadog_monitoring.signals.handlers import init_worker_process + + +class TestHandlers(TestCase): + """Tests for signal handlers.""" + + 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__ != 'CeleryCodeOwnerSpanProcessor' + ] + + def test_init_worker_process(self): + def get_processor_list(): + # pylint: disable=protected-access + return [type(sp).__name__ for sp in tracer._span_processors] + + assert sorted(get_processor_list()) == [ + 'EndpointCallCounterProcessor', 'TopLevelSpanProcessor', + ] + + init_worker_process(sender=None) + + assert sorted(get_processor_list()) == [ + 'CeleryCodeOwnerSpanProcessor', 'EndpointCallCounterProcessor', 'TopLevelSpanProcessor', + ] diff --git a/edx_arch_experiments/datadog_monitoring/tests/test_apps.py b/edx_arch_experiments/datadog_monitoring/tests/test_apps.py new file mode 100644 index 0000000..a1f5273 --- /dev/null +++ b/edx_arch_experiments/datadog_monitoring/tests/test_apps.py @@ -0,0 +1,22 @@ +""" +Tests for plugin app. +""" +from celery.signals import worker_process_init +from django.test import TestCase + + +class TestDatadogMonitoringApp(TestCase): + """ + Tests for TestDatadogMonitoringApp. + """ + + def test_signal_has_receiver(self): + """ + Imperfect test to ensure celery signal has the receiver. + + The receiver gets added during DatadogMonitoringApp's ready() call + at load time. An attempt to disconnect the receiver did not allow it + to be re-added during a call to ready, presumably because the signal + was already imported. + """ + assert worker_process_init.receivers[0][1].__name__ == 'init_worker_process' diff --git a/setup.py b/setup.py index 3b29fa9..f6cbc94 100644 --- a/setup.py +++ b/setup.py @@ -164,9 +164,11 @@ def is_requirement(line): "arch_experiments = edx_arch_experiments.apps:EdxArchExperimentsConfig", "config_watcher = edx_arch_experiments.config_watcher.apps:ConfigWatcher", "codejail_service = edx_arch_experiments.codejail_service.apps:CodejailService", + "datadog_monitoring = edx_arch_experiments.datadog_monitoring.apps:DatadogMonitoring", ], "cms.djangoapp": [ "config_watcher = edx_arch_experiments.config_watcher.apps:ConfigWatcher", + "datadog_monitoring = edx_arch_experiments.datadog_monitoring.apps:DatadogMonitoring", ], }, ) From 4c734e40834fb86ff92836deb01dfb6d2f1bb1ac Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Wed, 20 Nov 2024 16:57:42 -0500 Subject: [PATCH 10/19] fixup! minor doc updates --- .../how_tos/add_code_owner_custom_attribute_to_an_ida.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 index 7392e3c..a9872c2 100644 --- 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 @@ -26,13 +26,15 @@ If you want to learn more about custom span tags in general, see `Enhanced Monit 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. +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``. +This is accomplished by receiving signals from celery's worker_process_init for each process, and then adding a custom Datadog span processor to add the span tags as appropriate. + Configuring your app settings ----------------------------- From 07bd5f1ab17adbdc6a54181539167959eac0ba87 Mon Sep 17 00:00:00 2001 From: edX requirements bot <49161187+edx-requirements-bot@users.noreply.github.com> Date: Wed, 20 Nov 2024 17:11:51 -0500 Subject: [PATCH 11/19] chore: Upgrade Python requirements (#860) --- requirements/base.txt | 4 ++-- requirements/dev.txt | 16 +++++++++++++--- requirements/doc.txt | 6 +++--- requirements/pip-tools.txt | 2 +- requirements/pip.txt | 2 +- requirements/quality.txt | 13 ++++++++++--- requirements/scripts.txt | 4 ++-- requirements/test.txt | 6 +++--- 8 files changed, 35 insertions(+), 18 deletions(-) diff --git a/requirements/base.txt b/requirements/base.txt index a569ee4..5985616 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -139,7 +139,7 @@ six==1.16.0 # python-dateutil sqlparse==0.5.2 # via django -stevedore==5.3.0 +stevedore==5.4.0 # via # code-annotations # edx-django-utils @@ -163,5 +163,5 @@ wcwidth==0.2.13 # via prompt-toolkit # The following packages are considered to be unsafe in a requirements file: -setuptools==75.5.0 +setuptools==75.6.0 # via -r requirements/base.in diff --git a/requirements/dev.txt b/requirements/dev.txt index 2a24e01..a595cf2 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -108,9 +108,10 @@ cryptography==43.0.3 # via # -r requirements/quality.txt # pyjwt + # secretstorage ddt==1.7.2 # via -r requirements/quality.txt -ddtrace==2.16.3 +ddtrace==2.16.4 # via -r requirements/quality.txt deprecated==1.2.15 # via @@ -224,6 +225,11 @@ jaraco-functools==4.1.0 # via # -r requirements/quality.txt # keyring +jeepney==0.8.0 + # via + # -r requirements/quality.txt + # keyring + # secretstorage jinja2==3.1.4 # via # -r requirements/quality.txt @@ -444,6 +450,10 @@ rpds-py==0.21.0 # -r requirements/quality.txt # jsonschema # referencing +secretstorage==3.3.3 + # via + # -r requirements/quality.txt + # keyring semantic-version==2.10.0 # via # -r requirements/quality.txt @@ -462,7 +472,7 @@ sqlparse==0.5.2 # via # -r requirements/quality.txt # django -stevedore==5.3.0 +stevedore==5.4.0 # via # -r requirements/quality.txt # code-annotations @@ -533,7 +543,7 @@ pip==24.2 # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # -r requirements/pip-tools.txt # pip-tools -setuptools==75.5.0 +setuptools==75.6.0 # via # -r requirements/pip-tools.txt # -r requirements/quality.txt diff --git a/requirements/doc.txt b/requirements/doc.txt index e239cb8..6126fda 100644 --- a/requirements/doc.txt +++ b/requirements/doc.txt @@ -85,7 +85,7 @@ cryptography==43.0.3 # pyjwt ddt==1.7.2 # via -r requirements/test.txt -ddtrace==2.16.3 +ddtrace==2.16.4 # via -r requirements/test.txt deprecated==1.2.15 # via @@ -327,7 +327,7 @@ sqlparse==0.5.2 # via # -r requirements/test.txt # django -stevedore==5.3.0 +stevedore==5.4.0 # via # -r requirements/test.txt # code-annotations @@ -378,5 +378,5 @@ zipp==3.21.0 # importlib-metadata # The following packages are considered to be unsafe in a requirements file: -setuptools==75.5.0 +setuptools==75.6.0 # via -r requirements/test.txt diff --git a/requirements/pip-tools.txt b/requirements/pip-tools.txt index 1d2a7e5..8d7dfd5 100644 --- a/requirements/pip-tools.txt +++ b/requirements/pip-tools.txt @@ -24,5 +24,5 @@ pip==24.2 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt # pip-tools -setuptools==75.5.0 +setuptools==75.6.0 # via pip-tools diff --git a/requirements/pip.txt b/requirements/pip.txt index 828273e..bdde397 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -10,5 +10,5 @@ wheel==0.45.0 # The following packages are considered to be unsafe in a requirements file: pip==24.3.1 # via -r requirements/pip.in -setuptools==75.5.0 +setuptools==75.6.0 # via -r requirements/pip.in diff --git a/requirements/quality.txt b/requirements/quality.txt index ce5ba05..0ca72ba 100644 --- a/requirements/quality.txt +++ b/requirements/quality.txt @@ -84,9 +84,10 @@ cryptography==43.0.3 # via # -r requirements/test.txt # pyjwt + # secretstorage ddt==1.7.2 # via -r requirements/test.txt -ddtrace==2.16.3 +ddtrace==2.16.4 # via -r requirements/test.txt deprecated==1.2.15 # via @@ -176,6 +177,10 @@ jaraco-context==6.0.1 # via keyring jaraco-functools==4.1.0 # via keyring +jeepney==0.8.0 + # via + # keyring + # secretstorage jinja2==3.1.4 # via # -r requirements/test.txt @@ -331,6 +336,8 @@ rpds-py==0.21.0 # -r requirements/test.txt # jsonschema # referencing +secretstorage==3.3.3 + # via keyring semantic-version==2.10.0 # via # -r requirements/test.txt @@ -347,7 +354,7 @@ sqlparse==0.5.2 # via # -r requirements/test.txt # django -stevedore==5.3.0 +stevedore==5.4.0 # via # -r requirements/test.txt # code-annotations @@ -401,5 +408,5 @@ zipp==3.21.0 # importlib-metadata # The following packages are considered to be unsafe in a requirements file: -setuptools==75.5.0 +setuptools==75.6.0 # via -r requirements/test.txt diff --git a/requirements/scripts.txt b/requirements/scripts.txt index b8c51f3..d7aa512 100644 --- a/requirements/scripts.txt +++ b/requirements/scripts.txt @@ -234,7 +234,7 @@ sqlparse==0.5.2 # via # -r requirements/base.txt # django -stevedore==5.3.0 +stevedore==5.4.0 # via # -r requirements/base.txt # code-annotations @@ -269,5 +269,5 @@ wcwidth==0.2.13 # prompt-toolkit # The following packages are considered to be unsafe in a requirements file: -setuptools==75.5.0 +setuptools==75.6.0 # via -r requirements/base.txt diff --git a/requirements/test.txt b/requirements/test.txt index 8c1c3ef..e0bfda2 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -72,7 +72,7 @@ cryptography==43.0.3 # pyjwt ddt==1.7.2 # via -r requirements/test.in -ddtrace==2.16.3 +ddtrace==2.16.4 # via -r requirements/test.in deprecated==1.2.15 # via opentelemetry-api @@ -244,7 +244,7 @@ sqlparse==0.5.2 # via # -r requirements/base.txt # django -stevedore==5.3.0 +stevedore==5.4.0 # via # -r requirements/base.txt # code-annotations @@ -288,5 +288,5 @@ zipp==3.21.0 # via importlib-metadata # The following packages are considered to be unsafe in a requirements file: -setuptools==75.5.0 +setuptools==75.6.0 # via -r requirements/base.txt From c1a190f2af8a0a6106f1916434cdca1379017f11 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Wed, 20 Nov 2024 17:09:32 -0500 Subject: [PATCH 12/19] fixup! update CHANGELOG date --- CHANGELOG.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index bf5d34a..b82c080 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,7 +14,7 @@ Change Log Unreleased ~~~~~~~~~~ -[5.1.0] - 2024-10-23 +[5.1.0] - 2024-11-21 ~~~~~~~~~~~~~~~~~~~~ Added ----- From 71b0c450f6742159c646d06aeef5067963259445 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Thu, 21 Nov 2024 12:51:01 -0500 Subject: [PATCH 13/19] fixup! update CHANGELOG entry --- CHANGELOG.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b82c080..11445c5 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -21,8 +21,9 @@ 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. + * Adds code owner span tags 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. + * Span tag code_owner_2_module includes the task name, where the original code_owner_module does not. In both cases, the code owner is computed the same, because it is based on a prefix match. [5.0.0] - 2024-10-22 ~~~~~~~~~~~~~~~~~~~~ From 69014901c847f266cb71cd823cd0d6112825a028 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Thu, 21 Nov 2024 12:55:10 -0500 Subject: [PATCH 14/19] fixup! minor update Co-authored-by: Tim McCormack --- edx_arch_experiments/datadog_monitoring/code_owner/datadog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/edx_arch_experiments/datadog_monitoring/code_owner/datadog.py b/edx_arch_experiments/datadog_monitoring/code_owner/datadog.py index 9802010..3b38fcb 100644 --- a/edx_arch_experiments/datadog_monitoring/code_owner/datadog.py +++ b/edx_arch_experiments/datadog_monitoring/code_owner/datadog.py @@ -13,7 +13,7 @@ def on_span_start(self, span): """ Adds code owner span tag for celery run spans at span creation. """ - if hasattr(span, 'name') and span.name == 'celery.run': + if getattr(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. From 678bae3e89a9e631ae8adabff2f07a8d607ad4fe Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Thu, 21 Nov 2024 12:55:23 -0500 Subject: [PATCH 15/19] fixup! minor update Co-authored-by: Tim McCormack --- edx_arch_experiments/datadog_monitoring/signals/handlers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/edx_arch_experiments/datadog_monitoring/signals/handlers.py b/edx_arch_experiments/datadog_monitoring/signals/handlers.py index df96a38..cd706a2 100644 --- a/edx_arch_experiments/datadog_monitoring/signals/handlers.py +++ b/edx_arch_experiments/datadog_monitoring/signals/handlers.py @@ -11,7 +11,7 @@ log = logging.getLogger(__name__) -@receiver(worker_process_init, weak=False) +@receiver(worker_process_init) def init_worker_process(sender, **kwargs): """ Adds a Datadog span processor to each worker process. From 9e14b61af4694db776cd3f69d9a170098bfc9d27 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Thu, 21 Nov 2024 12:55:44 -0500 Subject: [PATCH 16/19] fixup! minor update Co-authored-by: Tim McCormack --- edx_arch_experiments/datadog_monitoring/signals/handlers.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/edx_arch_experiments/datadog_monitoring/signals/handlers.py b/edx_arch_experiments/datadog_monitoring/signals/handlers.py index cd706a2..b6430bf 100644 --- a/edx_arch_experiments/datadog_monitoring/signals/handlers.py +++ b/edx_arch_experiments/datadog_monitoring/signals/handlers.py @@ -15,6 +15,9 @@ def init_worker_process(sender, **kwargs): """ Adds a Datadog span processor to each worker process. + + We have to do this from inside the worker processes because they fork from the + parent process before the plugin app is initialized. """ try: from ddtrace import tracer # pylint: disable=import-outside-toplevel From 4e8ddad2725530a53def571e87e5d078cab266a7 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Thu, 21 Nov 2024 12:59:12 -0500 Subject: [PATCH 17/19] fixup! fix getattr issue --- edx_arch_experiments/datadog_monitoring/code_owner/datadog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/edx_arch_experiments/datadog_monitoring/code_owner/datadog.py b/edx_arch_experiments/datadog_monitoring/code_owner/datadog.py index 3b38fcb..0199907 100644 --- a/edx_arch_experiments/datadog_monitoring/code_owner/datadog.py +++ b/edx_arch_experiments/datadog_monitoring/code_owner/datadog.py @@ -13,7 +13,7 @@ def on_span_start(self, span): """ Adds code owner span tag for celery run spans at span creation. """ - if getattr(span, 'name') == 'celery.run': + if getattr(span, 'name', None) == '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. From 3bcabaf153109c88e7486cae0259c693d42c9cda Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Thu, 21 Nov 2024 13:14:10 -0500 Subject: [PATCH 18/19] fixup! fix failing test --- edx_arch_experiments/datadog_monitoring/tests/test_apps.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/edx_arch_experiments/datadog_monitoring/tests/test_apps.py b/edx_arch_experiments/datadog_monitoring/tests/test_apps.py index a1f5273..cf03b4d 100644 --- a/edx_arch_experiments/datadog_monitoring/tests/test_apps.py +++ b/edx_arch_experiments/datadog_monitoring/tests/test_apps.py @@ -19,4 +19,5 @@ def test_signal_has_receiver(self): to be re-added during a call to ready, presumably because the signal was already imported. """ - assert worker_process_init.receivers[0][1].__name__ == 'init_worker_process' + # the name of the function is in the weakref __repr__ + assert 'init_worker_process' in worker_process_init.receivers[0][1].__repr__() From be8b27e1d31cd3b5c0c85b814a1b5a611a82ed79 Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Thu, 21 Nov 2024 13:19:36 -0500 Subject: [PATCH 19/19] fixup! fix quality --- edx_arch_experiments/datadog_monitoring/signals/handlers.py | 2 +- edx_arch_experiments/datadog_monitoring/tests/test_apps.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/edx_arch_experiments/datadog_monitoring/signals/handlers.py b/edx_arch_experiments/datadog_monitoring/signals/handlers.py index b6430bf..04b5ab3 100644 --- a/edx_arch_experiments/datadog_monitoring/signals/handlers.py +++ b/edx_arch_experiments/datadog_monitoring/signals/handlers.py @@ -15,7 +15,7 @@ def init_worker_process(sender, **kwargs): """ Adds a Datadog span processor to each worker process. - + We have to do this from inside the worker processes because they fork from the parent process before the plugin app is initialized. """ diff --git a/edx_arch_experiments/datadog_monitoring/tests/test_apps.py b/edx_arch_experiments/datadog_monitoring/tests/test_apps.py index cf03b4d..bdea120 100644 --- a/edx_arch_experiments/datadog_monitoring/tests/test_apps.py +++ b/edx_arch_experiments/datadog_monitoring/tests/test_apps.py @@ -20,4 +20,4 @@ def test_signal_has_receiver(self): was already imported. """ # the name of the function is in the weakref __repr__ - assert 'init_worker_process' in worker_process_init.receivers[0][1].__repr__() + assert 'init_worker_process' in repr(worker_process_init.receivers[0][1])