diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 61ee87c2..091e24d1 100644 --- a/CHANGELOG.rst +++ b/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 diff --git a/edx_django_utils/__init__.py b/edx_django_utils/__init__.py index 0c900198..512bddfd 100644 --- a/edx_django_utils/__init__.py +++ b/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" diff --git a/edx_django_utils/monitoring/README.rst b/edx_django_utils/monitoring/README.rst index 90125270..ae4dc036 100644 --- a/edx_django_utils/monitoring/README.rst +++ b/edx_django_utils/monitoring/README.rst @@ -36,7 +36,7 @@ Feature support matrix for built-in telemetry backends: - ✅ - ❌ - ✅ - * - Retrieve and manipulate spans (``get_current_transaction``, ``ignore_transaction``) + * - Manipulate spans (``ignore_transaction``) - ✅ - ❌ - ❌ @@ -44,10 +44,6 @@ Feature support matrix for built-in telemetry backends: - ✅ - ✅ - ✅ - * - Instrument non-web tasks (``background_task``) - - ✅ - - ❌ - - ❌ Additional requirements for using these backends: diff --git a/edx_django_utils/monitoring/__init__.py b/edx_django_utils/monitoring/__init__.py index fd4069a9..9537725a 100644 --- a/edx_django_utils/monitoring/__init__.py +++ b/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, diff --git a/edx_django_utils/monitoring/internal/code_owner/middleware.py b/edx_django_utils/monitoring/internal/code_owner/middleware.py index a08cf277..1af5583a 100644 --- a/edx_django_utils/monitoring/internal/code_owner/middleware.py +++ b/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. diff --git a/edx_django_utils/monitoring/internal/transactions.py b/edx_django_utils/monitoring/internal/transactions.py index 739ba4e8..ff32248a 100644 --- a/edx_django_utils/monitoring/internal/transactions.py +++ b/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) diff --git a/edx_django_utils/monitoring/tests/test_custom_monitoring.py b/edx_django_utils/monitoring/tests/test_custom_monitoring.py index 44594104..9af5ef99 100644 --- a/edx_django_utils/monitoring/tests/test_custom_monitoring.py +++ b/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,15 +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): diff --git a/edx_django_utils/monitoring/tests/test_utils.py b/edx_django_utils/monitoring/tests/test_utils.py deleted file mode 100644 index cfb095c1..00000000 --- a/edx_django_utils/monitoring/tests/test_utils.py +++ /dev/null @@ -1,25 +0,0 @@ -""" -Tests for utilities in monitoring. -""" - -from unittest.mock import patch - -from edx_django_utils.monitoring import background_task - - -@patch('edx_django_utils.monitoring.internal.utils.newrelic') -def test_background_task_wrapper(wrapped_nr): - # We are verifying that this returns the correct decorated function - # in the two cases we care about. - returned_func = background_task() - - assert returned_func == wrapped_nr.agent.background_task() - - -@patch('edx_django_utils.monitoring.internal.utils.newrelic', None) -def test_background_task_wrapper_no_new_relic(): - # Test that the decorator behaves as a no-op when newrelic is not set. - returned_func = background_task() - wrapped_value = returned_func('a') - - assert wrapped_value == 'a'