From 1d3af1ca69baa2d4cb37271fa3b065c017c8dd8f Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Thu, 24 Oct 2024 10:11:31 -0400 Subject: [PATCH] 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()