Merge pull request #308 from getsentry/pierre/additional-frame-drop-d…

…etectionfilters

CHANGELOG.md

@@ -6,6 +6,7 @@
 
 - Improve frame drop detection algorithm. ([#304](https://github.com/getsentry/vroom/pull/304))
 - Accept and return the time a profile started at in a timestamp field on Android. ([#306](https://github.com/getsentry/vroom/pull/306))
+- Filter frame drop candidates based on thresholds. ([#308](https://github.com/getsentry/vroom/pull/308))
 
 **Internal**
 

internal/occurrence/frame_drop.go

@@ -1,6 +1,9 @@
 package occurrence
 
 import (
+	"math"
+	"time"
+
 	"github.com/getsentry/vroom/internal/frame"
 	"github.com/getsentry/vroom/internal/nodetree"
 	"github.com/getsentry/vroom/internal/profile"
@@ -12,10 +15,46 @@ type (
 		n     *nodetree.Node
 		st    []*nodetree.Node
 	}
+
+	frozenFrameStats struct {
+		durationNS    uint64
+		endNS         uint64
+		minDurationNS uint64
+		startLimitNS  uint64
+		startNS       uint64
+	}
 )
 
+func newFrozenFrameStats(endNS uint64, durationNS float64) frozenFrameStats {
+	margin := uint64(math.Max(durationNS*marginPercent, float64(10*time.Millisecond)))
+	s := frozenFrameStats{
+		endNS:         endNS + margin,
+		durationNS:    uint64(durationNS),
+		minDurationNS: uint64(durationNS * minFrameDurationPercent),
+	}
+	if endNS >= (s.durationNS + margin) {
+		s.startNS = endNS - s.durationNS - margin
+	}
+	s.startLimitNS = s.startNS + uint64(durationNS*0.20)
+	return s
+}
+
+// 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 &&
+		ns.n.EndNS <= s.endNS &&
+		s.minDurationNS <= s.durationNS &&
+		ns.n.StartNS <= s.startLimitNS &&
+		ns.n.IsApplication
+}
+
 const (
 	FrameDrop Category = "frame_drop"
+
+	marginPercent           float64 = 0.05
+	minFrameDurationPercent float64 = 0.5
+	startLimitPercent       float64 = 0.2
 )
 
 func findFrameDropCause(
@@ -32,12 +71,12 @@ func findFrameDropCause(
 		return
 	}
 	for _, mv := range frameDrops.Values {
+		stats := newFrozenFrameStats(mv.ElapsedSinceStartNs, mv.Value)
 		for _, root := range callTrees {
 			st := make([]*nodetree.Node, 0, 128)
 			cause := findFrameDropCauseFrame(
 				root,
-				mv.ElapsedSinceStartNs-uint64(mv.Value),
-				mv.ElapsedSinceStartNs,
+				stats,
 				&st,
 				0,
 			)
@@ -64,7 +103,7 @@ func findFrameDropCause(
 
 func findFrameDropCauseFrame(
 	n *nodetree.Node,
-	frozenFrameStartNS, frozenFrameEndNS uint64,
+	stats frozenFrameStats,
 	st *[]*nodetree.Node,
 	depth int,
 ) *nodeStack {
@@ -78,8 +117,7 @@ func findFrameDropCauseFrame(
 	for _, c := range n.Children {
 		cause := findFrameDropCauseFrame(
 			c,
-			frozenFrameStartNS,
-			frozenFrameEndNS,
+			stats,
 			st,
 			depth+1,
 		)
@@ -98,19 +136,23 @@ func findFrameDropCauseFrame(
 		}
 	}
 
+	var current *nodeStack
+
 	// Create a nodeStack of the current node
 	ns := &nodeStack{depth, n, nil}
 	// Check if current node if valid.
-	current := nodeStackIfValid(
-		ns,
-		frozenFrameStartNS,
-		frozenFrameEndNS,
-	)
+	if stats.IsNodeStackValid(ns) {
+		current = ns
+	}
+
+	if longest == nil && current == nil {
+		return nil
+	}
 
 	// If we didn't find any valid node downstream, we return the current.
 	if longest == nil {
-		ns.st = make([]*nodetree.Node, len(*st))
-		copy(ns.st, *st)
+		current.st = make([]*nodetree.Node, len(*st))
+		copy(current.st, *st)
 		return current
 	}
 
@@ -120,21 +162,7 @@ func findFrameDropCauseFrame(
 		return longest
 	}
 
-	ns.st = make([]*nodetree.Node, len(*st))
-	copy(ns.st, *st)
+	current.st = make([]*nodetree.Node, len(*st))
+	copy(current.st, *st)
 	return current
 }
-
-// nodeStackIfValid returns the nodeStack if we consider it valid as
-// a frame drop cause.
-func nodeStackIfValid(
-	ns *nodeStack,
-	frozenFrameStartNS, frozenFrameEndNS uint64,
-) *nodeStack {
-	if ns.n.StartNS >= frozenFrameStartNS &&
-		ns.n.EndNS <= frozenFrameEndNS &&
-		ns.n.IsApplication {
-		return ns
-	}
-	return nil
-}

internal/occurrence/frame_drop_test.go

@@ -490,13 +490,13 @@ func TestFindFrameDrop(t *testing.T) {
 								},
 								Children: []*nodetree.Node{
 									{
-										DurationNS:    uint64(100 * time.Millisecond),
+										DurationNS:    uint64(150 * time.Millisecond),
 										EndNS:         uint64(250 * time.Millisecond),
 										IsApplication: true,
 										Name:          "child2-1",
 										Package:       "package",
 										Path:          "path",
-										StartNS:       uint64(150 * time.Millisecond),
+										StartNS:       uint64(100 * time.Millisecond),
 										Frame: frame.Frame{
 											Function: "child2-1",
 											InApp:    &testutil.True,
@@ -570,7 +570,7 @@ func TestFindFrameDrop(t *testing.T) {
 						Tags: map[string]string{},
 					},
 					EvidenceData: map[string]interface{}{
-						"frame_duration_ns":   uint64(100000000),
+						"frame_duration_ns":   uint64(150000000),
 						"frame_module":        "",
 						"frame_name":          "child2-1",
 						"frame_package":       "package",