From 84f31c5a11989031bf2ddcf77f31d35f082a1799 Mon Sep 17 00:00:00 2001
From: David Herberth <david.herberth@sentry.io>
Date: Thu, 8 Aug 2024 10:36:43 +0200
Subject: [PATCH] ref(trimming): Keep frames from both ends of the trace

---
 CHANGELOG.md                              |  6 ++
 relay-event-normalization/src/trimming.rs | 71 +++++++++++++++++++++--
 2 files changed, 71 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c27a26989e..a9c1eff8df 100644
--- a/CHANGELOG.md
+++ b/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**:
diff --git a/relay-event-normalization/src/trimming.rs b/relay-event-normalization/src/trimming.rs
index f3b17ec176..e35c92d5ff 100644
--- a/relay-event-normalization/src/trimming.rs
+++ b/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 {