feat: refactor code_owner code from edx-dajango-utils

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

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'

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.

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."
+            )

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)

edx_arch_experiments/datadog_monitoring/code_owner/utils.py

@@ -1,13 +1,13 @@
 """
-Utilities for monitoring code_owner
+Utilities for monitoring code_owner_2
 """
 import logging
 import re
 from functools import wraps
 
 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,73 +112,54 @@ 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)
 
 
 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():
             ...