Skip to content

Commit

Permalink
feat: add monitoring signals for plugins (#467)
Browse files Browse the repository at this point in the history
Adds signals monitoring_support_process_request,
monitoring_support_process_response, and
monitoring_support_process_exception to the
MonitoringSupportMiddleware to enable plugging in
monitoring capabilities.
  • Loading branch information
robrap authored Dec 4, 2024
1 parent ac488e6 commit a2ab5d6
Show file tree
Hide file tree
Showing 7 changed files with 131 additions and 19 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ Change Log

.. There should always be an "Unreleased" section for changes pending release.
7.1.0 - 2024-12-05
------------------
Added
~~~~~
* Added signals monitoring_support_process_request, monitoring_support_process_response, and monitoring_support_process_exception to the MonitoringSupportMiddleware to enable plugging in monitoring capabilities.

7.0.1 - 2024-11-21
------------------
Fixed
Expand Down
2 changes: 1 addition & 1 deletion edx_django_utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
EdX utilities for Django Application development..
"""

__version__ = "7.0.1"
__version__ = "7.1.0"

default_app_config = (
"edx_django_utils.apps.EdxDjangoUtilsConfig"
Expand Down
16 changes: 12 additions & 4 deletions edx_django_utils/monitoring/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -78,18 +78,26 @@ Here is how you add the middleware:
'edx_django_utils.cache.middleware.RequestCacheMiddleware',
# Add monitoring middleware immediately after RequestCacheMiddleware
'edx_django_utils.monitoring.MonitoringSupportMiddleware',
'edx_django_utils.monitoring.DeploymentMonitoringMiddleware',
'edx_django_utils.monitoring.CookieMonitoringMiddleware',
'edx_django_utils.monitoring.CodeOwnerMonitoringMiddleware',
'edx_django_utils.monitoring.CachedCustomMonitoringMiddleware',
'edx_django_utils.monitoring.FrontendMonitoringMiddleware',
'edx_django_utils.monitoring.MonitoringMemoryMiddleware',
)
Cached Custom Monitoring Middleware
-----------------------------------
Monitoring Support Middleware and Monitoring Plugins
----------------------------------------------------

The middleware ``CachedCustomMonitoringMiddleware`` is required to allow certain utility methods, like ``accumulate`` and ``increment``, to work appropriately.
The middleware ``MonitoringSupportMiddleware`` provides a number of monitoring capabilities:

* It enables plugging in outside monitoring capabilities, by sending the signals ``monitoring_support_process_request``, ``monitoring_support_process_response``, and ``monitoring_support_process_exception``. These can be useful because this middleware should be available in all Open edX services, and should appear early enough in the list of middleware to monitor most requests, even those that respond early from another middleware.
* It allows certain utility methods, like ``accumulate`` and ``increment``, to work appropriately.
* It adds error span tags to the root span.

In order to use the monitoring signals, import them as follows::

from edx_django_utils.monitoring.signals import monitoring_support_process_response

Code Owner Custom Attribute
---------------------------
Expand Down
5 changes: 3 additions & 2 deletions edx_django_utils/monitoring/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
"""
Metrics utilities public api
Monitoring utilities public api
See README.rst for details.
Does not include signals.py, which is also part of the public api.
See README.rst for additional details.
"""
from .internal.backends import DatadogBackend, NewRelicBackend, OpenTelemetryBackend, TelemetryBackend
from .internal.code_owner.middleware import CodeOwnerMonitoringMiddleware
Expand Down
30 changes: 26 additions & 4 deletions edx_django_utils/monitoring/internal/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@

from edx_django_utils.cache import RequestCache
from edx_django_utils.logging import encrypt_for_log
from edx_django_utils.monitoring.signals import (
monitoring_support_process_exception,
monitoring_support_process_request,
monitoring_support_process_response
)

from .backends import configured_backends

Expand Down Expand Up @@ -74,10 +79,13 @@ class MonitoringSupportMiddleware(MiddlewareMixin):
"""
Middleware to support monitoring.
1. Middleware batch reports cached custom attributes at the end of a request.
2. Middleware adds error span tags to the root span.
1. Send process request, response, and exception signals to enable
plugins to add custom monitoring.
2. Middleware batch reports cached custom attributes at the end of a request.
3. Middleware adds error span tags to the root span.
Make sure to add below the request cache in MIDDLEWARE.
Make sure to add below the request cache in MIDDLEWARE, but as early
in the stack of middleware as possible.
This middleware will only call on the telemetry collector if there are any attributes
to report for this request, so it will not incur any processing overhead for
Expand Down Expand Up @@ -140,20 +148,34 @@ def _tag_root_span_with_error(self, exception):
for backend in configured_backends():
backend.tag_root_span_with_error(exception)

# Whether or not there was an exception, report any custom NR attributes that
# Whether or not there was an exception, report any custom attributes that
# may have been collected.

def process_request(self, request):
"""
Django middleware handler to process a request
"""
monitoring_support_process_request.send_robust(
sender=self.__class__, request=request
)

def process_response(self, request, response):
"""
Django middleware handler to process a response
"""
monitoring_support_process_response.send_robust(
sender=self.__class__, request=request, response=response
)
self._batch_report()
return response

def process_exception(self, request, exception):
"""
Django middleware handler to process an exception
"""
monitoring_support_process_exception.send_robust(
sender=self.__class__, request=request, exception=exception
)
self._batch_report()
self._tag_root_span_with_error(exception)

Expand Down
17 changes: 17 additions & 0 deletions edx_django_utils/monitoring/signals.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
"""
Signals defined to support monitoring plugins.
Placing signals in this file (vs alternatives like /internal, or
signals/signals.py) provides a good public api for these signals.
"""
import django.dispatch

# The MonitoringSupportMiddleware sends these signals to enable plugging in
# outside monitoring capabilities. This is a useful capability because the
# MonitoringSupportMiddleware should be available in all Open edX services,
# and should be one of the first middleware (after RequestCacheMiddleware),
# allowing it access to monitor most requests (even those that fail or
# respond early in other middleware).
monitoring_support_process_request = django.dispatch.Signal()
monitoring_support_process_response = django.dispatch.Signal()
monitoring_support_process_exception = django.dispatch.Signal()
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
"""
Tests for CachedCustomMonitoringMiddleware and associated utilities.
Tests for MonitoringSupportMiddleware and associated utilities.
Note: See test_middleware.py for the rest of the middleware tests.
"""
from unittest.mock import Mock, call, patch
from contextlib import contextmanager
from unittest.mock import ANY, Mock, call, patch

import ddt
from django.test import TestCase, override_settings

from edx_django_utils.cache import RequestCache
from edx_django_utils.monitoring import MonitoringSupportMiddleware, accumulate, increment
from edx_django_utils.monitoring.signals import (
monitoring_support_process_exception,
monitoring_support_process_request,
monitoring_support_process_response
)

from ..middleware import CachedCustomMonitoringMiddleware as DeprecatedCachedCustomMonitoringMiddleware
from ..middleware import MonitoringCustomMetricsMiddleware as DeprecatedMonitoringCustomMetricsMiddleware
Expand All @@ -20,13 +26,13 @@


@ddt.ddt
class TestCustomMonitoringMiddleware(TestCase):
class TestMonitoringSupportMiddleware(TestCase):
"""
Test the monitoring_utils middleware and helpers
"""
def setUp(self):
super().setUp()
self.mock_response = Mock()
self.mock_get_response = Mock()
RequestCache.clear_all_namespaces()

@patch('newrelic.agent')
Expand Down Expand Up @@ -57,7 +63,7 @@ def test_accumulate_and_increment(
]

# fake a response to trigger attributes reporting
middleware_method = getattr(cached_monitoring_middleware_class(self.mock_response), middleware_method_name)
middleware_method = getattr(cached_monitoring_middleware_class(self.mock_get_response), middleware_method_name)
middleware_method(
'fake request',
'fake response',
Expand Down Expand Up @@ -96,9 +102,9 @@ def test_accumulate_with_illegal_value(
call('error_adding_accumulated_metric', 'name=hello, new_value=10, cached_value=None'),
]

self.mock_response = Mock()
self.mock_get_response = Mock()
# fake a response to trigger metrics reporting
cached_monitoring_middleware_class(self.mock_response).process_response(
cached_monitoring_middleware_class(self.mock_get_response).process_response(
'fake request',
'fake response',
)
Expand All @@ -113,6 +119,58 @@ def test_accumulate_with_illegal_value(
# Assert call args to newrelic.agent.add_custom_parameter().
mock_newrelic_agent.add_custom_parameter.assert_has_calls(nr_agent_calls_expected, any_order=True)

@contextmanager
def catch_signal(self, signal):
"""
Catch django signal and return the mocked handler.
"""
handler = Mock()
signal.connect(handler)
yield handler
signal.disconnect(handler)

def test_process_request_signal(self):
"""
Test middleware sends process request signal.
"""
with self.catch_signal(monitoring_support_process_request) as handler:
MonitoringSupportMiddleware(self.mock_get_response).process_request(
'fake request'
)

handler.assert_called_once_with(
signal=ANY, sender=MonitoringSupportMiddleware, request='fake request'
)

def test_process_response_signal(self):
"""
Test middleware sends process response signal.
"""
with self.catch_signal(monitoring_support_process_response) as handler:
MonitoringSupportMiddleware(self.mock_get_response).process_response(
'fake request', 'fake response'
)

handler.assert_called_once_with(
signal=ANY, sender=MonitoringSupportMiddleware,
request='fake request', response='fake response'
)

def test_process_exception_signal(self):
"""
Test middleware sends process exception signal.
"""
fake_exception = Exception()
with self.catch_signal(monitoring_support_process_exception) as handler:
MonitoringSupportMiddleware(self.mock_get_response).process_exception(
'fake request', fake_exception
)

handler.assert_called_once_with(
signal=ANY, sender=MonitoringSupportMiddleware,
request='fake request', exception=fake_exception
)

@patch('ddtrace.Tracer.current_root_span')
def test_error_tagging(self, mock_get_root_span):
# Ensure that we continue to support tagging exceptions in MonitoringSupportMiddleware.
Expand All @@ -121,7 +179,7 @@ def test_error_tagging(self, mock_get_root_span):
mock_root_span = Mock()
mock_get_root_span.return_value = mock_root_span
with override_settings(OPENEDX_TELEMETRY=['edx_django_utils.monitoring.DatadogBackend']):
MonitoringSupportMiddleware(self.mock_response).process_exception(
MonitoringSupportMiddleware(self.mock_get_response).process_exception(
'fake request', fake_exception
)
mock_root_span.set_exc_info.assert_called_with(
Expand Down

0 comments on commit a2ab5d6

Please sign in to comment.