Skip to content

Commit 53c2cfa

Browse files
authored
fix(er): chained exception frame ordering (#15397)
## Description We fix the order in which frames are collected from an exception chain. We make sure that the frames closer to the point where the initial exception is thrown are captured first. This ensures that we propagate the tags on the relevant spans as the exception chain bubbles up and unwinds.
1 parent 17570f8 commit 53c2cfa

File tree

3 files changed

+41
-16
lines changed

3 files changed

+41
-16
lines changed

ddtrace/debugging/_exception/replay.py

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,11 @@ def limit_exception(exc_ident: int) -> bool:
113113
def unwind_exception_chain(
114114
exc: t.Optional[BaseException], tb: t.Optional[TracebackType]
115115
) -> t.Tuple[ExceptionChain, t.Optional[uuid.UUID]]:
116-
"""Unwind the exception chain and assign it an ID."""
116+
"""Unwind the exception chain and assign it an ID.
117+
118+
The chain goes from "cause to effect", meaning that every cause for an
119+
exception is put first.
120+
"""
117121
chain: ExceptionChain = deque()
118122

119123
while exc is not None:
@@ -141,9 +145,13 @@ def unwind_exception_chain(
141145
return chain, exc_id
142146

143147

144-
def get_tb_frames_from_exception_chain(chain: ExceptionChain) -> t.List[TracebackType]:
145-
"""Get the frames from the exception chain."""
146-
frames: t.List[TracebackType] = []
148+
def get_tb_frames_from_exception_chain(chain: ExceptionChain) -> t.Generator[t.Tuple[int, TracebackType], None, None]:
149+
"""Get the frames from the exception chain.
150+
151+
For each exception in the chain we collect the maximum number of frames
152+
configured, starting from the point where the exception was thrown.
153+
"""
154+
frame_count = 0
147155

148156
for _, tb in chain:
149157
local_frames = []
@@ -157,9 +165,21 @@ def get_tb_frames_from_exception_chain(chain: ExceptionChain) -> t.List[Tracebac
157165

158166
# Get only the last N frames from the traceback, where N is the
159167
# configured max size for span tracebacks.
160-
frames.extend(local_frames[-global_config._span_traceback_max_size :])
168+
# local_frames = local_frames[-global_config._span_traceback_max_size :]
169+
local_frames[: -global_config._span_traceback_max_size] = []
170+
171+
# Update the frame count to allow computing the sequence number
172+
frame_count += len(local_frames)
173+
174+
# Set the initial sequence number. This will decrease as we yield the
175+
# new frames
176+
frame_index = frame_count
161177

162-
return frames
178+
# Pop and yield the frames from this traceback in reverse order
179+
while local_frames:
180+
frame = local_frames.pop()
181+
yield frame_index, frame
182+
frame_index -= 1
163183

164184

165185
class SpanExceptionProbe(LogLineProbe):
@@ -262,8 +282,7 @@ def _attach_tb_frame_snapshot_to_span(
262282
return False
263283

264284
snapshot = None
265-
snapshot_id = frame.f_locals.get(SNAPSHOT_KEY, None)
266-
if snapshot_id is None:
285+
if (snapshot_id := frame.f_locals.get(SNAPSHOT_KEY, None)) is None:
267286
# We don't have a snapshot for the frame so we create one
268287
if cached_only:
269288
# If we only want a cached snapshot we return True as a signal
@@ -328,7 +347,7 @@ def on_span_exception(
328347

329348
# Capture more frames if we have budget left, otherwise set just the
330349
# tags on the span for those frames that we have already captured.
331-
for seq_nr, _tb in reversed(list(enumerate(get_tb_frames_from_exception_chain(chain), 1))):
350+
for seq_nr, _tb in get_tb_frames_from_exception_chain(chain):
332351
has_snapshot_budget = frames_captured < config.max_frames
333352
has_captured = self._attach_tb_frame_snapshot_to_span(
334353
span, _tb, exc_id, seq_nr, cached_only=not has_snapshot_budget
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
fixes:
3+
- |
4+
exception replay: fixed the order in which frames are captured to ensure
5+
that the values of frames close to the point where the initial exception
6+
was thrown are always attached to the relevant spans.

tests/debugging/exception/test_replay.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,19 +34,19 @@ def c(foo=42):
3434
c()
3535
except Exception as e:
3636
chain, _ = replay.unwind_exception_chain(e, e.__traceback__)
37-
frames = replay.get_tb_frames_from_exception_chain(chain)
37+
frames = list(replay.get_tb_frames_from_exception_chain(chain))
3838
# There are two tracebacks in the chain: one for KeyError and one for
3939
# ValueError. The innermost goes from the call to a in b up to the point
4040
# where the exception is raised in a. The outermost goes from the call
4141
# in this test function up to the point in b where the exception from a
4242
# is caught and the the KeyError is raised.
4343
assert len(frames) == 2 + 3
44-
assert [f.tb_frame.f_code.co_name for f in frames] == [
45-
"b",
46-
"a",
47-
"test_tb_frames_from_exception_chain",
48-
"c",
49-
"b",
44+
assert [(n, f.tb_frame.f_code.co_name) for n, f in frames] == [
45+
(2, "a"),
46+
(1, "b"),
47+
(5, "b"),
48+
(4, "c"),
49+
(3, "test_tb_frames_from_exception_chain"),
5050
]
5151

5252

0 commit comments

Comments
 (0)