From cedcdc56f2390d8991db361fb8c197e58b3d8755 Mon Sep 17 00:00:00 2001 From: quinna-h Date: Wed, 5 Nov 2025 11:27:24 -0500 Subject: [PATCH 1/5] modify error tracking --- .../_handled_exceptions/collector.py | 12 ++++--- .../errortracking/test_handled_exceptions.py | 36 +++++++++++++++++++ 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/ddtrace/errortracking/_handled_exceptions/collector.py b/ddtrace/errortracking/_handled_exceptions/collector.py index 20e0545c76e..1d9df469fbb 100644 --- a/ddtrace/errortracking/_handled_exceptions/collector.py +++ b/ddtrace/errortracking/_handled_exceptions/collector.py @@ -30,13 +30,14 @@ def _add_span_events(span: Span) -> None: def _on_span_exception(span, _exc_msg, exc_val, _exc_tb): exception_events = HandledExceptionCollector.get_exception_events(span.span_id) - if exception_events and exc_val in exception_events: - del exception_events[exc_val] + exc_id = id(exc_val) + if exception_events and exc_id in exception_events: + del exception_events[exc_id] class HandledExceptionCollector(Service): _instance: t.Optional["HandledExceptionCollector"] = None - _span_exception_events: t.Dict[int, t.Dict[Exception, SpanEvent]] = {} + _span_exception_events: t.Dict[int, t.Dict[int, SpanEvent]] = {} def __init__(self) -> None: super(HandledExceptionCollector, self).__init__() @@ -111,8 +112,9 @@ def capture_exception_event(cls, span: Span, exc: Exception, event: SpanEvent): events_dict = cls._span_exception_events.setdefault(span_id, {}) if not events_dict: span._add_on_finish_exception_callback(_add_span_events) - if exc in events_dict or len(events_dict) < COLLECTOR_MAX_SIZE_PER_SPAN: - events_dict[exc] = event + exc_id = id(exc) + if exc_id in events_dict or len(events_dict) < COLLECTOR_MAX_SIZE_PER_SPAN: + events_dict[exc_id] = event @classmethod def get_exception_events(cls, span_id: int): diff --git a/tests/errortracking/test_handled_exceptions.py b/tests/errortracking/test_handled_exceptions.py index ad32b447bab..d17d728d1fc 100644 --- a/tests/errortracking/test_handled_exceptions.py +++ b/tests/errortracking/test_handled_exceptions.py @@ -164,6 +164,42 @@ def test_handled_in_parent_span(self): assert self.spans[1].name == "child_span" assert len(self.spans[1]._events) == 0 + @run_in_subprocess(env_overrides=dict(DD_ERROR_TRACKING_HANDLED_ERRORS="all")) + def test_unhashable_exception(self): + """Test that unhashable exceptions (e.g., with mutable attributes) are handled correctly.""" + from ddtrace.errortracking._handled_exceptions.collector import HandledExceptionCollector + + class UnhashableException(Exception): + def __init__(self, message, mutable_data): + super().__init__(message) + self.mutable_data = mutable_data + + def __eq__(self, other): + # This makes the exception unhashable if __hash__ is not defined + return isinstance(other, UnhashableException) and self.message == other.message + + HandledExceptionCollector.enable() + + value = 0 + + @self.tracer.wrap() + def f(): + nonlocal value + try: + raise UnhashableException("unhashable error", {"key": "value"}) + except UnhashableException: + value = 10 + + f() + HandledExceptionCollector.disable() + + assert value == 10 + self.assert_span_count(1) + assert len(self.spans[0]._events) == 1 + # The exception type should include the test class path + assert "UnhashableException" in self.spans[0]._events[0].attributes["exception.type"] + assert self.spans[0]._events[0].attributes["exception.message"] == "unhashable error" + @skipif_errortracking_not_supported class UserCodeErrorTestCases(TracerTestCase): From 98e783ac3ab65328badf8c5900c564fd0694c754 Mon Sep 17 00:00:00 2001 From: quinna-h Date: Wed, 5 Nov 2025 11:30:32 -0500 Subject: [PATCH 2/5] wip --- tests/errortracking/test_handled_exceptions.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/errortracking/test_handled_exceptions.py b/tests/errortracking/test_handled_exceptions.py index d17d728d1fc..ef2b0882a89 100644 --- a/tests/errortracking/test_handled_exceptions.py +++ b/tests/errortracking/test_handled_exceptions.py @@ -196,7 +196,6 @@ def f(): assert value == 10 self.assert_span_count(1) assert len(self.spans[0]._events) == 1 - # The exception type should include the test class path assert "UnhashableException" in self.spans[0]._events[0].attributes["exception.type"] assert self.spans[0]._events[0].attributes["exception.message"] == "unhashable error" From d3bf3e6fb31203d2fa9ca385fd196452c2abb4cc Mon Sep 17 00:00:00 2001 From: quinna-h Date: Wed, 5 Nov 2025 13:02:30 -0500 Subject: [PATCH 3/5] update tests --- .../_handled_exceptions/collector.py | 9 ++-- tests/errortracking/_test_functions.py | 18 +++++++ .../errortracking/test_handled_exceptions.py | 52 +++++++------------ 3 files changed, 43 insertions(+), 36 deletions(-) diff --git a/ddtrace/errortracking/_handled_exceptions/collector.py b/ddtrace/errortracking/_handled_exceptions/collector.py index 1d9df469fbb..20008792597 100644 --- a/ddtrace/errortracking/_handled_exceptions/collector.py +++ b/ddtrace/errortracking/_handled_exceptions/collector.py @@ -21,7 +21,9 @@ def _add_span_events(span: Span) -> None: a span event for every handled exceptions, we store them in the span and add them when the span finishes. """ - span_exc_events = list(HandledExceptionCollector.get_exception_events(span.span_id).values()) + # Extract just the SpanEvent objects from the stored (exception, event) tuples + exception_data = HandledExceptionCollector.get_exception_events(span.span_id).values() + span_exc_events = [event for _exc, event in exception_data] if span_exc_events: span._set_tag_str(SPAN_EVENTS_HAS_EXCEPTION, "true") span._events.extend(span_exc_events) @@ -37,7 +39,7 @@ def _on_span_exception(span, _exc_msg, exc_val, _exc_tb): class HandledExceptionCollector(Service): _instance: t.Optional["HandledExceptionCollector"] = None - _span_exception_events: t.Dict[int, t.Dict[int, SpanEvent]] = {} + _span_exception_events: t.Dict[int, t.Dict[int, t.Tuple[Exception, SpanEvent]]] = {} def __init__(self) -> None: super(HandledExceptionCollector, self).__init__() @@ -114,7 +116,8 @@ def capture_exception_event(cls, span: Span, exc: Exception, event: SpanEvent): span._add_on_finish_exception_callback(_add_span_events) exc_id = id(exc) if exc_id in events_dict or len(events_dict) < COLLECTOR_MAX_SIZE_PER_SPAN: - events_dict[exc_id] = event + # Store both exception and event to keep exception alive and prevent ID reuse + events_dict[exc_id] = (exc, event) @classmethod def get_exception_events(cls, span_id: int): diff --git a/tests/errortracking/_test_functions.py b/tests/errortracking/_test_functions.py index 8ddcc53d61f..de800ab3fa9 100644 --- a/tests/errortracking/_test_functions.py +++ b/tests/errortracking/_test_functions.py @@ -1,6 +1,24 @@ import asyncio +class UnhashableException(Exception): + def __init__(self, message, mutable_data): + super().__init__(message) + self.mutable_data = mutable_data + + def __eq__(self, other): + # This makes the exception unhashable if __hash__ is not defined + return isinstance(other, UnhashableException) and str(self) == str(other) + + +def test_unhashable_exception_f(value): + try: + raise UnhashableException("unhashable error", {"key": "value"}) + except UnhashableException: + value = 10 + return value + + def test_basic_try_except_f(value): try: raise ValueError("auto caught error") diff --git a/tests/errortracking/test_handled_exceptions.py b/tests/errortracking/test_handled_exceptions.py index ef2b0882a89..c3d9f4081c3 100644 --- a/tests/errortracking/test_handled_exceptions.py +++ b/tests/errortracking/test_handled_exceptions.py @@ -91,7 +91,9 @@ def test_handle_same_error_multiple_times(self): ) # This asserts that the reported span event contained the info # of the last time the error is handled - assert "line 30" in self.spans[0]._events[0].attributes["exception.stacktrace"] + assert "line 48" in self.spans[0]._events[0].attributes["exception.stacktrace"], ( + self.spans[0]._events[0].attributes["exception.stacktrace"] + ) @run_in_subprocess(env_overrides=dict(DD_ERROR_TRACKING_HANDLED_ERRORS="all")) def test_handled_same_error_different_type(self): @@ -159,7 +161,9 @@ def test_handled_in_parent_span(self): self.spans[0].assert_span_event_attributes( 0, {"exception.type": "builtins.ValueError", "exception.message": "auto caught error"} ) - assert "line 72" in self.spans[0]._events[0].attributes["exception.stacktrace"] + assert "line 100" in self.spans[0]._events[0].attributes["exception.stacktrace"], ( + self.spans[0]._events[0].attributes["exception.stacktrace"] + ) assert self.spans[1].name == "child_span" assert len(self.spans[1]._events) == 0 @@ -167,37 +171,19 @@ def test_handled_in_parent_span(self): @run_in_subprocess(env_overrides=dict(DD_ERROR_TRACKING_HANDLED_ERRORS="all")) def test_unhashable_exception(self): """Test that unhashable exceptions (e.g., with mutable attributes) are handled correctly.""" - from ddtrace.errortracking._handled_exceptions.collector import HandledExceptionCollector - - class UnhashableException(Exception): - def __init__(self, message, mutable_data): - super().__init__(message) - self.mutable_data = mutable_data - - def __eq__(self, other): - # This makes the exception unhashable if __hash__ is not defined - return isinstance(other, UnhashableException) and self.message == other.message - - HandledExceptionCollector.enable() - - value = 0 - - @self.tracer.wrap() - def f(): - nonlocal value - try: - raise UnhashableException("unhashable error", {"key": "value"}) - except UnhashableException: - value = 10 - - f() - HandledExceptionCollector.disable() - - assert value == 10 - self.assert_span_count(1) - assert len(self.spans[0]._events) == 1 - assert "UnhashableException" in self.spans[0]._events[0].attributes["exception.type"] - assert self.spans[0]._events[0].attributes["exception.message"] == "unhashable error" + self._run_error_test( + "test_unhashable_exception_f", + initial_value=0, + expected_value=10, + expected_events=[ + [ + { + "exception.type": "tests.errortracking._test_functions.UnhashableException", + "exception.message": "unhashable error", + } + ] + ], + ) @skipif_errortracking_not_supported From d06dcb1b363f58fae3bcc8b61b48c721af42e209 Mon Sep 17 00:00:00 2001 From: quinna-h Date: Wed, 5 Nov 2025 13:34:37 -0500 Subject: [PATCH 4/5] wip - remove comment --- ddtrace/errortracking/_handled_exceptions/collector.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ddtrace/errortracking/_handled_exceptions/collector.py b/ddtrace/errortracking/_handled_exceptions/collector.py index 20008792597..fe6241b1be7 100644 --- a/ddtrace/errortracking/_handled_exceptions/collector.py +++ b/ddtrace/errortracking/_handled_exceptions/collector.py @@ -21,7 +21,6 @@ def _add_span_events(span: Span) -> None: a span event for every handled exceptions, we store them in the span and add them when the span finishes. """ - # Extract just the SpanEvent objects from the stored (exception, event) tuples exception_data = HandledExceptionCollector.get_exception_events(span.span_id).values() span_exc_events = [event for _exc, event in exception_data] if span_exc_events: From db42015d231f515d2a28d2f63710b1b34a3cc147 Mon Sep 17 00:00:00 2001 From: quinna-h Date: Thu, 6 Nov 2025 09:59:01 -0500 Subject: [PATCH 5/5] add release note --- .../modify-error-tracking-exceptions-6be5aa66cb4cd2ef.yaml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 releasenotes/notes/modify-error-tracking-exceptions-6be5aa66cb4cd2ef.yaml diff --git a/releasenotes/notes/modify-error-tracking-exceptions-6be5aa66cb4cd2ef.yaml b/releasenotes/notes/modify-error-tracking-exceptions-6be5aa66cb4cd2ef.yaml new file mode 100644 index 00000000000..ec9b90a0884 --- /dev/null +++ b/releasenotes/notes/modify-error-tracking-exceptions-6be5aa66cb4cd2ef.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - | + 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.