Merge pull request #458 from openedx/diana/refactor-monitoring

feat: Remove unusued background_task transaction.

Author: dianakhuang

CHANGELOG.rst

@@ -11,6 +11,13 @@ Change Log
 
 .. There should always be an "Unreleased" section for changes pending release.
 
+7.0.0 - 2024-10-16
+------------------
+Removed
+~~~~~~~
+* Remove unused ``background_task`` monitoring function.
+* Remove ``get_current_transaction`` (used internally only) from the public API.
+
 [6.1.0] - 2024-10-15
 ---------------------
 Changed

edx_django_utils/__init__.py

@@ -2,7 +2,7 @@
 EdX utilities for Django Application development..
 """
 
-__version__ = "6.1.0"
+__version__ = "7.0.0"
 
 default_app_config = (
     "edx_django_utils.apps.EdxDjangoUtilsConfig"

edx_django_utils/monitoring/README.rst

@@ -36,18 +36,14 @@ Feature support matrix for built-in telemetry backends:
      - ✅
      - ❌
      - ✅
-   * - Retrieve and manipulate spans (``get_current_transaction``, ``ignore_transaction``)
+   * - Manipulate spans (``ignore_transaction``)
      - ✅
      - ❌
      - ❌
    * - Record exceptions (``record_exception``)
      - ✅
      - ✅
      - ✅
-   * - Instrument non-web tasks (``background_task``)
-     - ✅
-     - ❌
-     - ❌
 
 Additional requirements for using these backends:
 

edx_django_utils/monitoring/__init__.py

@@ -18,10 +18,9 @@
     MonitoringMemoryMiddleware,
     MonitoringSupportMiddleware
 )
-from .internal.transactions import get_current_transaction, ignore_transaction
+from .internal.transactions import ignore_transaction
 from .internal.utils import (
     accumulate,
-    background_task,
     function_trace,
     increment,
     record_exception,

edx_django_utils/monitoring/internal/code_owner/middleware.py

@@ -6,7 +6,6 @@
 from django.urls import resolve
 from django.urls.exceptions import Resolver404
 
-from ..transactions import get_current_transaction
 from ..utils import set_custom_attribute
 from .utils import (
     _get_catch_all_code_owner,
@@ -15,9 +14,48 @@
     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.

edx_django_utils/monitoring/internal/transactions.py

@@ -28,35 +28,3 @@ def ignore_transaction():
     # an equivalent for Datadog. For Datadog, use filter/ignore rules.
     if newrelic:  # pragma: no cover
         newrelic.agent.ignore_transaction()
-
-
-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.
-    """
-    current_transaction = None
-    if newrelic:
-        current_transaction = newrelic.agent.current_transaction()
-
-    return MonitoringTransaction(current_transaction)

edx_django_utils/monitoring/tests/test_custom_monitoring.py

@@ -9,7 +9,7 @@
 from django.test import TestCase, override_settings
 
 from edx_django_utils.cache import RequestCache
-from edx_django_utils.monitoring import MonitoringSupportMiddleware, accumulate, get_current_transaction, increment
+from edx_django_utils.monitoring import MonitoringSupportMiddleware, accumulate, increment
 
 from ..middleware import CachedCustomMonitoringMiddleware as DeprecatedCachedCustomMonitoringMiddleware
 from ..middleware import MonitoringCustomMetricsMiddleware as DeprecatedMonitoringCustomMetricsMiddleware
@@ -128,16 +128,6 @@ def test_error_tagging(self, mock_get_root_span):
                 type(fake_exception), fake_exception, fake_exception.__traceback__
             )
 
-    @patch('newrelic.agent')
-    def test_get_current_transaction(self, mock_newrelic_agent):
-        mock_newrelic_agent.current_transaction().name = 'fake-transaction'
-        current_transaction = get_current_transaction()
-        self.assertEqual(current_transaction.name, 'fake-transaction')
-
-    def test_get_current_transaction_without_newrelic(self):
-        current_transaction = get_current_transaction()
-        self.assertEqual(current_transaction.name, None)
-
     @patch('edx_django_utils.monitoring.utils.internal_accumulate')
     def test_deprecated_accumulate(self, mock_accumulate):
         deprecated_accumulate('key', 1)