ref(trimming): Keep frames from both ends of the trace

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+**Bug Fixes**:
+
+- Keep frames from both ends of the stacktrace when trimming frames. ([#3905](https://github.com/getsentry/relay/pull/3905))
+
 ## 24.8.0
 
 **Bug Fixes**:

relay-event-normalization/src/trimming.rs

@@ -283,7 +283,7 @@ impl Processor for TrimmingProcessor {
  }
 
  processor::apply(&mut stacktrace.frames, |frames, meta| {
- enforce_frame_hard_limit(frames, meta, 250);
+ enforce_frame_hard_limit(frames, meta, 200, 50);
  Ok(())
  })?;
 
@@ -359,12 +359,26 @@ fn trim_string(value: &mut String, meta: &mut Meta, max_chars: usize, max_chars_
  });
 }
 
-fn enforce_frame_hard_limit(frames: &mut Array<Frame>, meta: &mut Meta, limit: usize) {
- // Trim down the frame list to a hard limit. Prioritize the last frames.
+/// Trim down the frame list to a hard limit.
+///
+/// The total limit is `recent_frames` + `old_frames`.
+/// `recent_frames` is the number of frames to keep from the beginning of the list,
+/// the most recent stack frames, `old_frames` is the last at the end of the list of frames,
+/// the oldest frames up the stack.
+///
+/// It makes sense to keep some of the old frames in recursion cases to see what actually caused
+/// the recursion.
+fn enforce_frame_hard_limit(
+ frames: &mut Array<Frame>,
+ meta: &mut Meta,
+ recent_frames: usize,
+ old_frames: usize,
+) {
  let original_length = frames.len();
+ let limit = recent_frames + old_frames;
  if original_length > limit {
  meta.set_original_length(Some(original_length));
- let _ = frames.drain(0..original_length - limit);
+ let _ = frames.drain(old_frames..original_length - recent_frames);
  }
 }
 
@@ -823,7 +837,13 @@ mod tests {
  ]);
 
  processor::apply(&mut frames, |f, m| {
- enforce_frame_hard_limit(f, m, 3);
+ enforce_frame_hard_limit(f, m, 3, 0);
+ Ok(())
+ })
+ .unwrap();
+
+ processor::apply(&mut frames, |f, m| {
+ enforce_frame_hard_limit(f, m, 1, 2);
  Ok(())
  })
  .unwrap();
@@ -850,7 +870,7 @@ mod tests {
  ]);
 
  processor::apply(&mut frames, |f, m| {
- enforce_frame_hard_limit(f, m, 3);
+ enforce_frame_hard_limit(f, m, 3, 0);
  Ok(())
  })
  .unwrap();
@@ -871,6 +891,45 @@ mod tests {
  );
  }
 
+ #[test]
+ fn test_frame_hard_limit_recent_old() {
+ fn create_frame(filename: &str) -> Annotated<Frame> {
+ Annotated::new(Frame {
+ filename: Annotated::new(filename.into()),
+ ..Default::default()
+ })
+ }
+
+ let mut frames = Annotated::new(vec![
+ create_frame("foo1.py"),
+ create_frame("foo2.py"),
+ create_frame("foo3.py"),
+ create_frame("foo4.py"),
+ create_frame("foo5.py"),
+ ]);
+
+ processor::apply(&mut frames, |f, m| {
+ enforce_frame_hard_limit(f, m, 2, 1);
+ Ok(())
+ })
+ .unwrap();
+
+ let mut expected_meta = Meta::default();
+ expected_meta.set_original_length(Some(5));
+
+ assert_eq!(
+ frames,
+ Annotated(
+ Some(vec![
+ create_frame("foo1.py"),
+ create_frame("foo4.py"),
+ create_frame("foo5.py"),
+ ]),
+ expected_meta
+ )
+ );
+ }
+
  #[test]
  fn test_slim_frame_data_under_max() {
  let mut frames = vec![Annotated::new(Frame {