From 59f9ab6e6ec6354718f3771ab506842ba0d9924d Mon Sep 17 00:00:00 2001
From: Pierre Massat <pierre.massat@sentry.io>
Date: Tue, 26 Sep 2023 09:18:40 -0400
Subject: [PATCH] fix: Compare the duration of the frame to the minimum
 duration (#319)

---
 CHANGELOG.md                           |  1 +
 internal/occurrence/frame_drop.go      |  4 +-
 internal/occurrence/frame_drop_test.go | 64 ++++++++++++++++++++------
 3 files changed, 54 insertions(+), 15 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b7e5657..b932561 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,7 @@
 **Bug Fixes**
 
 - Close remaining open events in Android profiles. ([#316](https://github.com/getsentry/vroom/pull/316))
+- Enforce minimum frame duration for frame drop issue. ([#319](https://github.com/getsentry/vroom/pull/319))
 
 **Internal**
 
diff --git a/internal/occurrence/frame_drop.go b/internal/occurrence/frame_drop.go
index a955e21..257b35b 100644
--- a/internal/occurrence/frame_drop.go
+++ b/internal/occurrence/frame_drop.go
@@ -42,9 +42,9 @@ func newFrozenFrameStats(endNS uint64, durationNS float64) frozenFrameStats {
 // nodeStackIfValid returns the nodeStack if we consider it valid as
 // a frame drop cause.
 func (s *frozenFrameStats) IsNodeStackValid(ns *nodeStack) bool {
-	return s.startNS <= ns.n.StartNS &&
+	return ns.n.StartNS >= s.startNS &&
 		ns.n.EndNS <= s.endNS &&
-		s.minDurationNS <= s.durationNS &&
+		ns.n.DurationNS >= s.minDurationNS &&
 		ns.n.StartNS <= s.startLimitNS &&
 		ns.n.IsApplication
 }
diff --git a/internal/occurrence/frame_drop_test.go b/internal/occurrence/frame_drop_test.go
index 3bb8c1c..d1d0a00 100644
--- a/internal/occurrence/frame_drop_test.go
+++ b/internal/occurrence/frame_drop_test.go
@@ -34,8 +34,8 @@ func TestFindFrameDrop(t *testing.T) {
 							Unit: "nanosecond",
 							Values: []measurements.MeasurementValue{
 								{
-									ElapsedSinceStartNs: uint64(500 * time.Millisecond),
-									Value:               float64(300 * time.Millisecond),
+									ElapsedSinceStartNs: uint64(400 * time.Millisecond),
+									Value:               float64(200 * time.Millisecond),
 								},
 							},
 						},
@@ -178,8 +178,8 @@ func TestFindFrameDrop(t *testing.T) {
 							Unit: "nanosecond",
 							Values: []measurements.MeasurementValue{
 								{
-									ElapsedSinceStartNs: uint64(500 * time.Millisecond),
-									Value:               float64(300 * time.Millisecond),
+									ElapsedSinceStartNs: uint64(400 * time.Millisecond),
+									Value:               float64(200 * time.Millisecond),
 								},
 							},
 						},
@@ -400,8 +400,8 @@ func TestFindFrameDrop(t *testing.T) {
 							Unit: "nanosecond",
 							Values: []measurements.MeasurementValue{
 								{
-									ElapsedSinceStartNs: uint64(500 * time.Millisecond),
-									Value:               float64(450 * time.Millisecond),
+									ElapsedSinceStartNs: uint64(400 * time.Millisecond),
+									Value:               float64(300 * time.Millisecond),
 								},
 							},
 						},
@@ -628,8 +628,8 @@ func TestFindFrameDrop(t *testing.T) {
 			callTrees: map[uint64][]*nodetree.Node{
 				1: {
 					{
-						DurationNS:    uint64(500 * time.Millisecond),
-						EndNS:         uint64(500 * time.Millisecond),
+						DurationNS:    uint64(1000 * time.Millisecond),
+						EndNS:         uint64(1000 * time.Millisecond),
 						IsApplication: false,
 						Name:          "root",
 						Package:       "package",
@@ -671,8 +671,8 @@ func TestFindFrameDrop(t *testing.T) {
 								},
 								Children: []*nodetree.Node{
 									{
-										DurationNS:    uint64(200 * time.Millisecond),
-										EndNS:         uint64(300 * time.Millisecond),
+										DurationNS:    uint64(250 * time.Millisecond),
+										EndNS:         uint64(350 * time.Millisecond),
 										IsApplication: true,
 										Name:          "child2-1",
 										Package:       "package",
@@ -688,13 +688,13 @@ func TestFindFrameDrop(t *testing.T) {
 								},
 							},
 							{
-								DurationNS:    uint64(200 * time.Millisecond),
+								DurationNS:    uint64(250 * time.Millisecond),
 								EndNS:         uint64(500 * time.Millisecond),
 								IsApplication: true,
 								Name:          "child3",
 								Package:       "package",
 								Path:          "path",
-								StartNS:       uint64(300 * time.Millisecond),
+								StartNS:       uint64(250 * time.Millisecond),
 								Frame: frame.Frame{
 									Function: "child3",
 									InApp:    &testutil.True,
@@ -734,7 +734,7 @@ func TestFindFrameDrop(t *testing.T) {
 						Tags: map[string]string{},
 					},
 					EvidenceData: map[string]interface{}{
-						"frame_duration_ns":   uint64(200000000),
+						"frame_duration_ns":   uint64(250000000),
 						"frame_module":        "",
 						"frame_name":          "child2-1",
 						"frame_package":       "package",
@@ -777,3 +777,41 @@ func TestFindFrameDrop(t *testing.T) {
 		})
 	}
 }
+
+func TestIsNodeStackValid(t *testing.T) {
+	tests := []struct {
+		name      string
+		stats     frozenFrameStats
+		nodestack *nodeStack
+		valid     bool
+	}{
+		{
+			name: "frame too short",
+			stats: frozenFrameStats{
+				startLimitNS:  0,
+				durationNS:    1000,
+				minDurationNS: 500,
+				startNS:       0,
+				endNS:         1000,
+			},
+			nodestack: &nodeStack{
+				n: &nodetree.Node{
+					StartNS:       100,
+					EndNS:         200,
+					IsApplication: true,
+					DurationNS:    100,
+				},
+			},
+			valid: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			output := tt.stats.IsNodeStackValid(tt.nodestack)
+			if output != tt.valid {
+				t.Fatalf("Didn't expect this value: %v", output)
+			}
+		})
+	}
+}