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()