Skip to content

Commit a065ba9

Browse files
fix: modify error tracking exceptions [backport 3.18] (#15168)
Backport 84db0e6 from #15148 to 3.18. ## Description <!-- Provide an overview of the change and motivation for the change --> For [APMS-17683](https://datadoghq.atlassian.net/browse/APMS-17683) customer was having an issue where their custom exception object was unhashable, resulting in a `TypeError` when trying to use the object as a key. This fix changes so that we store the exception object id, not the exception object. ## Testing <!-- Describe your testing strategy or note what tests are included --> ## Risks <!-- Note any risks associated with this change, or "None" if no risks --> ## Additional Notes <!-- Any other information that would be helpful for reviewers --> [APMS-17683]: https://datadoghq.atlassian.net/browse/APMS-17683?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Co-authored-by: Quinna Halim <quinna.halim@datadoghq.com>
1 parent 6ba96f2 commit a065ba9

File tree

4 files changed

+55
-8
lines changed

4 files changed

+55
-8
lines changed

ddtrace/errortracking/_handled_exceptions/collector.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ def _add_span_events(span: Span) -> None:
2121
a span event for every handled exceptions, we store them in the span
2222
and add them when the span finishes.
2323
"""
24-
span_exc_events = list(HandledExceptionCollector.get_exception_events(span.span_id).values())
24+
exception_data = HandledExceptionCollector.get_exception_events(span.span_id).values()
25+
span_exc_events = [event for _exc, event in exception_data]
2526
if span_exc_events:
2627
span._set_tag_str(SPAN_EVENTS_HAS_EXCEPTION, "true")
2728
span._events.extend(span_exc_events)
@@ -30,13 +31,14 @@ def _add_span_events(span: Span) -> None:
3031

3132
def _on_span_exception(span, _exc_msg, exc_val, _exc_tb):
3233
exception_events = HandledExceptionCollector.get_exception_events(span.span_id)
33-
if exception_events and exc_val in exception_events:
34-
del exception_events[exc_val]
34+
exc_id = id(exc_val)
35+
if exception_events and exc_id in exception_events:
36+
del exception_events[exc_id]
3537

3638

3739
class HandledExceptionCollector(Service):
3840
_instance: t.Optional["HandledExceptionCollector"] = None
39-
_span_exception_events: t.Dict[int, t.Dict[Exception, SpanEvent]] = {}
41+
_span_exception_events: t.Dict[int, t.Dict[int, t.Tuple[Exception, SpanEvent]]] = {}
4042

4143
def __init__(self) -> None:
4244
super(HandledExceptionCollector, self).__init__()
@@ -111,8 +113,10 @@ def capture_exception_event(cls, span: Span, exc: Exception, event: SpanEvent):
111113
events_dict = cls._span_exception_events.setdefault(span_id, {})
112114
if not events_dict:
113115
span._add_on_finish_exception_callback(_add_span_events)
114-
if exc in events_dict or len(events_dict) < COLLECTOR_MAX_SIZE_PER_SPAN:
115-
events_dict[exc] = event
116+
exc_id = id(exc)
117+
if exc_id in events_dict or len(events_dict) < COLLECTOR_MAX_SIZE_PER_SPAN:
118+
# Store both exception and event to keep exception alive and prevent ID reuse
119+
events_dict[exc_id] = (exc, event)
116120

117121
@classmethod
118122
def get_exception_events(cls, span_id: int):
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
Error Tracking: Modifies the way exception events are stored such that the exception id is stored instead of the exception object, to prevent TypeErrors with custom exception objects.

tests/errortracking/_test_functions.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,24 @@
11
import asyncio
22

33

4+
class UnhashableException(Exception):
5+
def __init__(self, message, mutable_data):
6+
super().__init__(message)
7+
self.mutable_data = mutable_data
8+
9+
def __eq__(self, other):
10+
# This makes the exception unhashable if __hash__ is not defined
11+
return isinstance(other, UnhashableException) and str(self) == str(other)
12+
13+
14+
def test_unhashable_exception_f(value):
15+
try:
16+
raise UnhashableException("unhashable error", {"key": "value"})
17+
except UnhashableException:
18+
value = 10
19+
return value
20+
21+
422
def test_basic_try_except_f(value):
523
try:
624
raise ValueError("auto caught error")

tests/errortracking/test_handled_exceptions.py

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,9 @@ def test_handle_same_error_multiple_times(self):
9191
)
9292
# This asserts that the reported span event contained the info
9393
# of the last time the error is handled
94-
assert "line 30" in self.spans[0]._events[0].attributes["exception.stacktrace"]
94+
assert "line 48" in self.spans[0]._events[0].attributes["exception.stacktrace"], (
95+
self.spans[0]._events[0].attributes["exception.stacktrace"]
96+
)
9597

9698
@run_in_subprocess(env_overrides=dict(DD_ERROR_TRACKING_HANDLED_ERRORS="all"))
9799
def test_handled_same_error_different_type(self):
@@ -159,11 +161,30 @@ def test_handled_in_parent_span(self):
159161
self.spans[0].assert_span_event_attributes(
160162
0, {"exception.type": "builtins.ValueError", "exception.message": "auto caught error"}
161163
)
162-
assert "line 72" in self.spans[0]._events[0].attributes["exception.stacktrace"]
164+
assert "line 100" in self.spans[0]._events[0].attributes["exception.stacktrace"], (
165+
self.spans[0]._events[0].attributes["exception.stacktrace"]
166+
)
163167

164168
assert self.spans[1].name == "child_span"
165169
assert len(self.spans[1]._events) == 0
166170

171+
@run_in_subprocess(env_overrides=dict(DD_ERROR_TRACKING_HANDLED_ERRORS="all"))
172+
def test_unhashable_exception(self):
173+
"""Test that unhashable exceptions (e.g., with mutable attributes) are handled correctly."""
174+
self._run_error_test(
175+
"test_unhashable_exception_f",
176+
initial_value=0,
177+
expected_value=10,
178+
expected_events=[
179+
[
180+
{
181+
"exception.type": "tests.errortracking._test_functions.UnhashableException",
182+
"exception.message": "unhashable error",
183+
}
184+
]
185+
],
186+
)
187+
167188

168189
@skipif_errortracking_not_supported
169190
class UserCodeErrorTestCases(TracerTestCase):

0 commit comments

Comments
 (0)