Skip to content

Commit

Permalink
feat: Add root span tagging for exceptions.
Browse files Browse the repository at this point in the history
Datadog has issues with tagging the root span
with error information. This change adds the functionality
to tag the root span on exceptions.

This also renames the CachedCustomMonitoringMiddleware into
MonitoringSupportMiddleware so that it can be a central place
for most monitoring middleware changes instead of adding a new
one every time. At some point, CachedCustomMonitoringMiddleware
will be removed.

edx/edx-arch-experiments#647
  • Loading branch information
dianakhuang committed Sep 27, 2024
1 parent 51b2d4a commit f1393a2
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 6 deletions.
3 changes: 2 additions & 1 deletion edx_django_utils/monitoring/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
CookieMonitoringMiddleware,
DeploymentMonitoringMiddleware,
FrontendMonitoringMiddleware,
MonitoringMemoryMiddleware
MonitoringMemoryMiddleware,
MonitoringSupportMiddleware
)
from .internal.transactions import get_current_transaction, ignore_transaction, set_monitoring_transaction_name
from .internal.utils import (
Expand Down
20 changes: 20 additions & 0 deletions edx_django_utils/monitoring/internal/backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,14 @@ def create_span(self, name):
or create a new root span if not currently in a span.
"""

@abstractmethod
def tag_root_span(self, exception):
"""
Tags the root span with the given exception. This is primarily useful for
Datadog as New Relic handles this behavior correctly. Unclear if this is also needs to
be implemented for OTEL.
"""


class NewRelicBackend(TelemetryBackend):
"""
Expand Down Expand Up @@ -96,6 +104,10 @@ def create_span(self, name):
nr_transaction = newrelic.agent.current_transaction()
return newrelic.agent.FunctionTrace(nr_transaction, name)

def tag_root_span(self, exception):
# Does not need to be implemented for NewRelic, because it is handled automatically.
pass


class OpenTelemetryBackend(TelemetryBackend):
"""
Expand All @@ -121,6 +133,10 @@ def create_span(self, name):
# Currently, this is not implemented.
pass

def tag_root_span(self, exception):
# Currently, this is not implemented for OTel
pass


class DatadogBackend(TelemetryBackend):
"""
Expand All @@ -145,6 +161,10 @@ def record_exception(self):
def create_span(self, name):
return self.dd_tracer.trace(name)

def tag_root_span(self, exception):
root_span = self.dd_tracer.current_root_span()
root_span.set_exc_info(type(exception), exception, exception.__traceback__)


# We're using an lru_cache instead of assigning the result to a variable on
# module load. With the default settings (pointing to a TelemetryBackend
Expand Down
21 changes: 20 additions & 1 deletion edx_django_utils/monitoring/internal/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,10 @@ def record_python_version():
_set_custom_attribute('python_version', platform.python_version())


class CachedCustomMonitoringMiddleware(MiddlewareMixin):
class MonitoringSupportMiddleware(MiddlewareMixin):
"""
Middleware to support monitoring, including cached custom attributes
Middleware batch reports cached custom attributes at the end of a request.
Make sure to add below the request cache in MIDDLEWARE.
Expand Down Expand Up @@ -130,6 +132,13 @@ def _batch_report(cls):
for key, value in attributes_cache.data.items():
_set_custom_attribute(key, value)

def _tag_root_span(self, exception):
"""
Tags the root span with the exception information for all configured backends.
"""
for backend in configured_backends():
backend.tag_root_span(exception)

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

Expand All @@ -145,6 +154,16 @@ def process_exception(self, request, exception): # pylint: disable=W0613
Django middleware handler to process an exception
"""
self._batch_report()
self._tag_root_span(exception)


class CachedCustomMonitoringMiddleware(MonitoringSupportMiddleware):
"""
DEPRECATED: Use MonitoringSupportMiddleware instead.
This is the old name for the MonitoringSupportMiddleware. We are keeping it
around for backwards compatibility until it can be fully deprecated and removed.
"""


def _set_custom_attribute(key, value):
Expand Down
8 changes: 4 additions & 4 deletions edx_django_utils/monitoring/tests/test_custom_monitoring.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from django.test import TestCase

from edx_django_utils.cache import RequestCache
from edx_django_utils.monitoring import CachedCustomMonitoringMiddleware, accumulate, get_current_transaction, increment
from edx_django_utils.monitoring import MonitoringSupportMiddleware, accumulate, get_current_transaction, increment

from ..middleware import CachedCustomMonitoringMiddleware as DeprecatedCachedCustomMonitoringMiddleware
from ..middleware import MonitoringCustomMetricsMiddleware as DeprecatedMonitoringCustomMetricsMiddleware
Expand All @@ -31,8 +31,8 @@ def setUp(self):

@patch('newrelic.agent')
@ddt.data(
(CachedCustomMonitoringMiddleware, False, 'process_response'),
(CachedCustomMonitoringMiddleware, False, 'process_exception'),
(MonitoringSupportMiddleware, False, 'process_response'),
(MonitoringSupportMiddleware, False, 'process_exception'),
(DeprecatedCachedCustomMonitoringMiddleware, True, 'process_response'),
(DeprecatedMonitoringCustomMetricsMiddleware, True, 'process_response'),
)
Expand Down Expand Up @@ -76,7 +76,7 @@ def test_accumulate_and_increment(

@patch('newrelic.agent')
@ddt.data(
(CachedCustomMonitoringMiddleware, False),
(MonitoringSupportMiddleware, False),
(DeprecatedCachedCustomMonitoringMiddleware, True),
(DeprecatedMonitoringCustomMetricsMiddleware, True),
)
Expand Down

0 comments on commit f1393a2

Please sign in to comment.