From f2b41cedf7afb02467563e72c208502b04a02d14 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Tue, 23 Sep 2025 15:03:59 -0400 Subject: [PATCH 1/4] fix(flamegraph): More accurage average computation for flamegraph metrics This is a bigger revamp of how the metrics are computed for flamegraphs to ensure the same averages are reported everywhere. --- cmd/vroom/flamegraph.go | 2 +- internal/flamegraph/flamegraph.go | 40 +++-- internal/flamegraph/flamegraph_test.go | 170 ++++++++++++-------- internal/metrics/metrics.go | 214 +++++++++++++++++++------ internal/metrics/metrics_test.go | 164 ------------------- internal/nodetree/nodetree.go | 64 +++++++- internal/nodetree/nodetree_test.go | 181 +++++++++++++++++++++ internal/profile/profile.go | 10 +- 8 files changed, 534 insertions(+), 311 deletions(-) delete mode 100644 internal/metrics/metrics_test.go diff --git a/cmd/vroom/flamegraph.go b/cmd/vroom/flamegraph.go index f128c62b..47f98d59 100644 --- a/cmd/vroom/flamegraph.go +++ b/cmd/vroom/flamegraph.go @@ -57,7 +57,7 @@ func (env *environment) postFlamegraph(w http.ResponseWriter, r *http.Request) { s = sentry.StartSpan(ctx, "processing") var ma *metrics.Aggregator if body.GenerateMetrics { - agg := metrics.NewAggregator(maxUniqueFunctionsPerProfile, 5, minDepth) + agg := metrics.NewAggregator(maxUniqueFunctionsPerProfile, 5, int(minDepth), false) ma = &agg } speedscope, err := flamegraph.GetFlamegraphFromCandidates( diff --git a/internal/flamegraph/flamegraph.go b/internal/flamegraph/flamegraph.go index 4e97440a..e60d2a02 100644 --- a/internal/flamegraph/flamegraph.go +++ b/internal/flamegraph/flamegraph.go @@ -49,13 +49,17 @@ func sumNodesSampleCount(nodes []*nodetree.Node) int { return c } -func annotateWithProfileExample(example examples.ExampleMetadata) func(n *nodetree.Node) { - return func(n *nodetree.Node) { +func annotateWithProfileExample(example examples.ExampleMetadata) func(n, m *nodetree.Node) { + return func(n, m *nodetree.Node) { n.Profiles[example] = void + if n.WorstSelfTime < m.SelfTimeNS { + n.WorstSelfTime = m.SelfTimeNS + n.WorstProfile = example + } } } -func addCallTreeToFlamegraph(flamegraphTree *[]*nodetree.Node, callTree []*nodetree.Node, annotate func(n *nodetree.Node)) { +func addCallTreeToFlamegraph(flamegraphTree *[]*nodetree.Node, callTree []*nodetree.Node, annotate func(n, m *nodetree.Node)) { for _, node := range callTree { var currentNode *nodetree.Node if existingNode := getMatchingNode(flamegraphTree, node); existingNode != nil { @@ -63,13 +67,16 @@ func addCallTreeToFlamegraph(flamegraphTree *[]*nodetree.Node, callTree []*nodet currentNode.Occurrence += node.Occurrence currentNode.SampleCount += node.SampleCount currentNode.DurationNS += node.DurationNS + currentNode.SelfTimeNS += node.SelfTimeNS + currentNode.DurationsNS = append(currentNode.DurationsNS, node.DurationNS) } else { currentNode = node.ShallowCopyWithoutChildren() + currentNode.DurationsNS = []uint64{node.DurationNS} *flamegraphTree = append(*flamegraphTree, currentNode) } addCallTreeToFlamegraph(¤tNode.Children, node.Children, annotate) if node.SampleCount > sumNodesSampleCount(node.Children) { - annotate(currentNode) + annotate(currentNode, node) } } } @@ -436,12 +443,6 @@ func GetFlamegraphFromCandidates( for _, callTree := range result.CallTrees { addCallTreeToFlamegraph(&flamegraphTree, callTree, annotate) } - // if metrics aggregator is not null, while we're at it, - // compute the metrics as well - if ma != nil { - functions := metrics.CapAndFilterFunctions(metrics.ExtractFunctionsFromCallTrees(result.CallTrees, ma.MinDepth), int(ma.MaxUniqueFunctions), true) - ma.AddFunctions(functions, example) - } transactionProfileSpan.Finish() } else if result, ok := res.(chunk.CallTreesReadJobResult); ok { @@ -469,13 +470,6 @@ func GetFlamegraphFromCandidates( annotate := annotateWithProfileExample(example) addCallTreeToFlamegraph(&flamegraphTree, callTree, annotate) - - // if metrics aggregator is not null, while we're at it, - // compute the metrics as well - if ma != nil { - functions := metrics.CapAndFilterFunctions(metrics.ExtractFunctionsFromCallTreesForThread(callTree, ma.MinDepth), int(ma.MaxUniqueFunctions), true) - ma.AddFunctions(functions, example) - } } chunkProfileSpan.Finish() } else { @@ -489,10 +483,22 @@ func GetFlamegraphFromCandidates( serializeSpan := span.StartChild("serialize") defer serializeSpan.Finish() + speedscopeSpan := span.StartChild("processing speedscope") sp := toSpeedscope(ctx, flamegraphTree, 1000, 0) + speedscopeSpan.Finish() + + // if metrics aggregator is not null, while we're at it, + // compute the metrics as well if ma != nil { + metricsSpan := span.StartChild("processing metrics") + for _, tree := range flamegraphTree { + // tree.RecursiveComputeSelfTime() + tree.Visit(ma.AddFunction) + } fm := ma.ToMetrics() sp.Metrics = &fm + metricsSpan.Finish() } + return sp, nil } diff --git a/internal/flamegraph/flamegraph_test.go b/internal/flamegraph/flamegraph_test.go index cc2a7d42..fbf71c93 100644 --- a/internal/flamegraph/flamegraph_test.go +++ b/internal/flamegraph/flamegraph_test.go @@ -127,6 +127,10 @@ func TestFlamegraphAggregation(t *testing.T) { ElapsedSinceStartNS: 20, StackID: 2, }, + { + ElapsedSinceStartNS: 30, + StackID: 2, + }, }, // end Samples }, // end Trace }, @@ -140,7 +144,7 @@ func TestFlamegraphAggregation(t *testing.T) { }, Profiles: []interface{}{ speedscope.SampledProfile{ - EndValue: 6, + EndValue: 7, IsMainThread: true, Samples: [][]int{ {2}, @@ -152,13 +156,13 @@ func TestFlamegraphAggregation(t *testing.T) { {0}, {1}, {1}, - {0}, + {0, 1}, }, Type: "sampled", Unit: "count", - Weights: []uint64{1, 1, 1, 3}, - SampleCounts: []uint64{1, 1, 1, 3}, - SampleDurationsNs: []uint64{10, 10, 10, 30}, + Weights: []uint64{1, 1, 1, 4}, + SampleCounts: []uint64{1, 1, 1, 4}, + SampleDurationsNs: []uint64{10, 10, 10, 40}, }, }, Shared: speedscope.SharedData{ @@ -169,8 +173,8 @@ func TestFlamegraphAggregation(t *testing.T) { {Image: "test.package", Name: "e", Fingerprint: 2430275448}, }, FrameInfos: []speedscope.FrameInfo{ + {Count: 4, Weight: 50}, {Count: 3, Weight: 40}, - {Count: 2, Weight: 30}, {Count: 2, Weight: 20}, {Count: 1, Weight: 10}, }, @@ -181,6 +185,20 @@ func TestFlamegraphAggregation(t *testing.T) { }, }, metrics: []examples.FunctionMetrics{ + { + Name: "a", + Package: "test.package", + Fingerprint: 2430275452, + P75: 10, + P95: 20, + P99: 20, + Avg: float64(50) / float64(4), + Sum: 50, + SumSelfTime: 10, + Count: 5, + Worst: examples.ExampleMetadata{ProfileID: "cd2"}, + Examples: []examples.ExampleMetadata{{ProfileID: "cd2"}}, + }, { Name: "b", Package: "test.package", @@ -188,10 +206,41 @@ func TestFlamegraphAggregation(t *testing.T) { P75: 20, P95: 20, P99: 20, - Avg: 15, - Sum: 30, - SumSelfTime: 30, - Count: 3, + Avg: float64(40) / float64(3), + Sum: 40, + SumSelfTime: 40, + Count: 4, + Worst: examples.ExampleMetadata{ProfileID: "ab1"}, + Examples: []examples.ExampleMetadata{{ProfileID: "ab1"}, {ProfileID: "cd2"}}, + }, + { + Name: "c", + Package: "test.package", + Fingerprint: 2430275454, + InApp: true, + P75: 10, + P95: 10, + P99: 10, + Avg: 10, + Sum: 20, + SumSelfTime: 20, + Count: 2, + Worst: examples.ExampleMetadata{ProfileID: "ab1"}, + Examples: []examples.ExampleMetadata{{ProfileID: "ab1"}}, + }, + { + Name: "e", + Package: "test.package", + Fingerprint: 2430275448, + P75: 10, + P95: 10, + P99: 10, + Avg: 10, + Sum: 10, + SumSelfTime: 10, + Count: 1, + Worst: examples.ExampleMetadata{ProfileID: "cd2"}, + Examples: []examples.ExampleMetadata{{ProfileID: "cd2"}}, }, }, }, @@ -316,19 +365,6 @@ func TestFlamegraphAggregation(t *testing.T) { }, }, metrics: []examples.FunctionMetrics{ - { - Name: "c", - Package: "test.package", - Fingerprint: 2430275454, - InApp: true, - P75: 30, - P95: 30, - P99: 30, - Avg: 25, - Sum: 50, - SumSelfTime: 50, - Count: 5, - }, { Name: "a", Package: "test.package", @@ -341,6 +377,8 @@ func TestFlamegraphAggregation(t *testing.T) { Sum: 90, SumSelfTime: 20, Count: 9, + Worst: examples.ExampleMetadata{ProfileID: "ab1"}, + Examples: []examples.ExampleMetadata{{ProfileID: "ab1"}}, }, { Name: "b", @@ -354,6 +392,23 @@ func TestFlamegraphAggregation(t *testing.T) { Sum: 70, SumSelfTime: 20, Count: 7, + Worst: examples.ExampleMetadata{ProfileID: "ab1"}, + Examples: []examples.ExampleMetadata{{ProfileID: "ab1"}}, + }, + { + Name: "c", + Package: "test.package", + Fingerprint: 2430275454, + InApp: true, + P75: 30, + P95: 30, + P99: 30, + Avg: 25, + Sum: 50, + SumSelfTime: 50, + Count: 5, + Worst: examples.ExampleMetadata{ProfileID: "ab1"}, + Examples: []examples.ExampleMetadata{{ProfileID: "ab1"}}, }, }, }, @@ -460,30 +515,34 @@ func TestFlamegraphAggregation(t *testing.T) { }, metrics: []examples.FunctionMetrics{ { - Name: "c", + Name: "b", Package: "test.package", - Fingerprint: 2430275454, + Fingerprint: 2430275455, InApp: true, P75: 30, P95: 30, P99: 30, - Avg: 30, - Sum: 30, - SumSelfTime: 30, - Count: 3, + Avg: 25, + Sum: 50, + SumSelfTime: 20, + Count: 5, + Worst: examples.ExampleMetadata{ProfileID: "ab1"}, + Examples: []examples.ExampleMetadata{{ProfileID: "ab1"}}, }, { - Name: "b", + Name: "c", Package: "test.package", - Fingerprint: 2430275455, + Fingerprint: 2430275454, InApp: true, P75: 30, P95: 30, P99: 30, - Avg: 25, - Sum: 50, - SumSelfTime: 20, - Count: 5, + Avg: 30, + Sum: 30, + SumSelfTime: 30, + Count: 3, + Worst: examples.ExampleMetadata{ProfileID: "ab1"}, + Examples: []examples.ExampleMetadata{{ProfileID: "ab1"}}, }, }, }, @@ -492,6 +551,7 @@ func TestFlamegraphAggregation(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { var ft []*nodetree.Node + for _, sp := range test.profiles { p := profile.New(&sp) callTrees, err := p.CallTrees() @@ -508,44 +568,14 @@ func TestFlamegraphAggregation(t *testing.T) { t.Fatalf("Result mismatch: got - want +\n%s", diff) } - ma := metrics.NewAggregator( - 100, - 5, - 0, - ) - - for _, sp := range test.profiles { - p := profile.New(&sp) - callTrees, err := p.CallTrees() - if err != nil { - t.Fatalf("error when generating calltrees: %v", err) - } - - start, end := p.StartAndEndEpoch() - example := examples.NewExampleFromProfileID( - p.ProjectID(), - p.ID(), - start, - end, - ) - - functions := metrics.CapAndFilterFunctions( - metrics.ExtractFunctionsFromCallTrees( - callTrees, - ma.MinDepth, - ), - int(ma.MaxUniqueFunctions), - false, - ) - ma.AddFunctions(functions, example) + ma := metrics.NewAggregator(100, 5, 0, false) + for _, tree := range ft { + // tree.RecursiveComputeSelfTime() + tree.Visit(ma.AddFunction) } m := ma.ToMetrics() - options := cmp.Options{ - cmpopts.IgnoreFields(examples.FunctionMetrics{}, "Worst"), - cmpopts.IgnoreFields(examples.FunctionMetrics{}, "Examples"), - } - if diff := testutil.Diff(m, test.metrics, options); diff != "" { + if diff := testutil.Diff(m, test.metrics); diff != "" { t.Fatalf("Result mismatch: got - want +\n%s", diff) } }) diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index aa17955e..bed8c007 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -1,6 +1,7 @@ package metrics import ( + "container/heap" "errors" "math" "sort" @@ -18,68 +19,176 @@ type ( } Aggregator struct { - MaxUniqueFunctions uint - // For a frame to be aggregated it has to have a depth >= MinDepth. - // So, if MinDepth is set to 1, it means all the root frames will not be part of the aggregation. - MinDepth uint - MaxNumOfExamples uint - CallTreeFunctions map[uint32]nodetree.CallTreeFunction - FunctionsMetadata map[uint32]FunctionsMetadata + callTreeFunctionsMap map[uint32]*FunctionMetric + callTreeFunctions []*FunctionMetric + maxUniqueFunctions int + maxNumberOfExamples int + // For a frame to be aggregated it has to have a depth >= minDepth. So, if minDepth + // is set to 1, it means all the root frames will not be part of the aggregation. + minDepth int + filterSystemFrames bool + } + + FunctionMetric struct { + index int + Fingerprint uint32 + Function string + Package string + InApp bool + DurationsNS []uint64 + SumDurationNS uint64 + SumSelfTimeNS uint64 + SampleCount uint64 + Examples []examples.ExampleMetadata + WorstSelfTime uint64 + WorstExample examples.ExampleMetadata } ) -func NewAggregator(MaxUniqueFunctions uint, MaxNumOfExamples uint, MinDepth uint) Aggregator { +func NewAggregator( + maxUniqueFunctions int, + maxNumberOfExamples int, + minDepth int, + filterSystemFrames bool, +) Aggregator { return Aggregator{ - MaxUniqueFunctions: MaxUniqueFunctions, - MinDepth: MinDepth, - MaxNumOfExamples: MaxNumOfExamples, - CallTreeFunctions: make(map[uint32]nodetree.CallTreeFunction), - FunctionsMetadata: make(map[uint32]FunctionsMetadata), + callTreeFunctionsMap: make(map[uint32]*FunctionMetric), + callTreeFunctions: make([]*FunctionMetric, 0, maxUniqueFunctions+1), + maxUniqueFunctions: maxUniqueFunctions, + maxNumberOfExamples: maxNumberOfExamples, + minDepth: minDepth, + filterSystemFrames: filterSystemFrames, } } -func (ma *Aggregator) AddFunctions(functions []nodetree.CallTreeFunction, resultMetadata examples.ExampleMetadata) { - for _, f := range functions { - if fn, ok := ma.CallTreeFunctions[f.Fingerprint]; ok { - fn.SampleCount += f.SampleCount - fn.DurationsNS = append(fn.DurationsNS, f.DurationsNS...) - fn.SumDurationNS += f.SumDurationNS - fn.SumSelfTimeNS += f.SumSelfTimeNS - funcMetadata := ma.FunctionsMetadata[f.Fingerprint] - if f.SumSelfTimeNS > funcMetadata.MaxVal { - funcMetadata.MaxVal = f.SumSelfTimeNS - funcMetadata.Worst = resultMetadata - } - if len(funcMetadata.Examples) < int(ma.MaxNumOfExamples) { - funcMetadata.Examples = append(funcMetadata.Examples, resultMetadata) - } - ma.FunctionsMetadata[f.Fingerprint] = funcMetadata - ma.CallTreeFunctions[f.Fingerprint] = fn - } else { - ma.CallTreeFunctions[f.Fingerprint] = f - ma.FunctionsMetadata[f.Fingerprint] = FunctionsMetadata{ - MaxVal: f.SumSelfTimeNS, - Worst: resultMetadata, - Examples: []examples.ExampleMetadata{resultMetadata}, - } +func (a Aggregator) Len() int { + return len(a.callTreeFunctions) +} + +func (a Aggregator) Less(i, j int) bool { + fi := a.callTreeFunctions[i] + fj := a.callTreeFunctions[j] + + if fi.SumSelfTimeNS != fj.SumSelfTimeNS { + return fi.SumSelfTimeNS < fj.SumSelfTimeNS + } + + if fi.SumDurationNS != fj.SumDurationNS { + return fi.SumDurationNS < fj.SumDurationNS + } + + return i < j +} + +func (a *Aggregator) Swap(i, j int) { + a.callTreeFunctions[i], a.callTreeFunctions[j] = a.callTreeFunctions[j], a.callTreeFunctions[i] + + // make sure to update the index so it can be easily found later + a.callTreeFunctions[i].index = i + a.callTreeFunctions[j].index = j +} + +func (a *Aggregator) Push(item any) { + n := a.Len() + metric := item.(*FunctionMetric) + metric.index = n + a.callTreeFunctions = append(a.callTreeFunctions, metric) + a.callTreeFunctionsMap[metric.Fingerprint] = metric +} + +func (a *Aggregator) Pop() any { + n := a.Len() + item := a.callTreeFunctions[n-1] + item.index = -1 // for safety + a.callTreeFunctions = a.callTreeFunctions[0 : n-1] + delete(a.callTreeFunctionsMap, item.Fingerprint) + return item +} + +func (a *Aggregator) AddFunction(n *nodetree.Node, depth int) { + if a.filterSystemFrames && !n.IsApplication { + return + } + + if depth < a.minDepth { + return + } + + if !nodetree.ShouldAggregateFrame(n.Frame) { + return + } + + var function *FunctionMetric + var ok bool + fingerprint := n.Frame.Fingerprint() + if function, ok = a.callTreeFunctionsMap[fingerprint]; ok { + function.DurationsNS = append(function.DurationsNS, n.DurationsNS...) + function.SumDurationNS += n.DurationNS + function.SumSelfTimeNS += n.SelfTimeNS + function.SampleCount += uint64(n.SampleCount) + heap.Fix(a, function.index) + } else { + f := FunctionMetric{ + Fingerprint: fingerprint, + Function: n.Name, + Package: n.Package, + InApp: n.IsApplication, + DurationsNS: n.DurationsNS, + SumDurationNS: n.DurationNS, + SumSelfTimeNS: n.SelfTimeNS, + SampleCount: uint64(n.SampleCount), } + function = &f + heap.Push(a, function) + } + + for example := range n.Profiles { + function.Examples = append(function.Examples, example) + } + + if function.WorstSelfTime < n.WorstSelfTime { + function.WorstSelfTime = n.WorstSelfTime + function.WorstExample = n.WorstProfile + } + + for a.Len() > a.maxUniqueFunctions { + heap.Pop(a) } } -func (ma *Aggregator) ToMetrics() []examples.FunctionMetrics { - metrics := make([]examples.FunctionMetrics, 0, len(ma.CallTreeFunctions)) +func (a Aggregator) ToMetrics() []examples.FunctionMetrics { + metrics := make([]examples.FunctionMetrics, 0, len(a.callTreeFunctions)) + + for _, f := range a.callTreeFunctions { + if f.SumSelfTimeNS <= 0 { + continue + } + sort.Slice(f.Examples, func(i, j int) bool { + example1 := f.Examples[i] + example2 := f.Examples[j] + + if example1.ProfileID != "" { + return example1.ProfileID < example2.ProfileID + } + + if example2.ProfileID != "" { + return true + } - for _, f := range ma.CallTreeFunctions { + return example1.ProfilerID < example2.ProfilerID + }) sort.Slice(f.DurationsNS, func(i, j int) bool { return f.DurationsNS[i] < f.DurationsNS[j] }) + p75, _ := quantile(f.DurationsNS, 0.75) p95, _ := quantile(f.DurationsNS, 0.95) p99, _ := quantile(f.DurationsNS, 0.99) - metrics = append(metrics, examples.FunctionMetrics{ + + m := examples.FunctionMetrics{ + Fingerprint: f.Fingerprint, Name: f.Function, Package: f.Package, - Fingerprint: f.Fingerprint, InApp: f.InApp, P75: p75, P95: p95, @@ -87,20 +196,25 @@ func (ma *Aggregator) ToMetrics() []examples.FunctionMetrics { Avg: float64(f.SumDurationNS) / float64(len(f.DurationsNS)), Sum: f.SumDurationNS, SumSelfTime: f.SumSelfTimeNS, - Count: uint64(f.SampleCount), - Worst: ma.FunctionsMetadata[f.Fingerprint].Worst, - Examples: ma.FunctionsMetadata[f.Fingerprint].Examples, - }) + Count: f.SampleCount, + Worst: f.WorstExample, + Examples: f.Examples, + } + metrics = append(metrics, m) } + sort.Slice(metrics, func(i, j int) bool { + if metrics[i].Sum != metrics[j].Sum { + return metrics[i].Sum > metrics[j].Sum + } + if metrics[i].SumSelfTime != metrics[j].SumSelfTime { return metrics[i].SumSelfTime > metrics[j].SumSelfTime } - return metrics[i].Sum > metrics[j].Sum + + return i < j }) - if len(metrics) > int(ma.MaxUniqueFunctions) { - metrics = metrics[:ma.MaxUniqueFunctions] - } + return metrics } diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go deleted file mode 100644 index 02651c62..00000000 --- a/internal/metrics/metrics_test.go +++ /dev/null @@ -1,164 +0,0 @@ -package metrics - -import ( - "sort" - "testing" - - "github.com/getsentry/vroom/internal/examples" - "github.com/getsentry/vroom/internal/nodetree" - "github.com/getsentry/vroom/internal/testutil" -) - -func TestAggregatorAddFunctions(t *testing.T) { - tests := []struct { - name string - calltreeFunctions []nodetree.CallTreeFunction - want Aggregator - }{ - { - name: "addFunctions", - calltreeFunctions: []nodetree.CallTreeFunction{ - { - Function: "a", - Fingerprint: 0, - SumSelfTimeNS: 40, - DurationsNS: []uint64{10, 5, 25}, - SumDurationNS: 40, - }, - { - Function: "b", - Fingerprint: 1, - SumSelfTimeNS: 105, - DurationsNS: []uint64{45, 60}, - SumDurationNS: 105, - }, - }, - want: Aggregator{ - MaxUniqueFunctions: 100, - MaxNumOfExamples: 5, - CallTreeFunctions: map[uint32]nodetree.CallTreeFunction{ - 0: { - Function: "a", - Fingerprint: 0, - SumSelfTimeNS: 80, - DurationsNS: []uint64{10, 5, 25, 10, 5, 25}, - SumDurationNS: 80, - }, - 1: { - Function: "b", - Fingerprint: 1, - SumSelfTimeNS: 210, - DurationsNS: []uint64{45, 60, 45, 60}, - SumDurationNS: 210, - }, - }, - FunctionsMetadata: map[uint32]FunctionsMetadata{ - 0: { - MaxVal: 40, - Worst: examples.ExampleMetadata{ProfileID: "1"}, - Examples: []examples.ExampleMetadata{{ProfileID: "1"}, {ProfileID: "2"}}, - }, - 1: { - MaxVal: 105, - Worst: examples.ExampleMetadata{ProfileID: "1"}, - Examples: []examples.ExampleMetadata{{ProfileID: "1"}, {ProfileID: "2"}}, - }, - }, // end want - }, - }, // end first test - } // end tests list - - ma := NewAggregator(100, 5, 0) - for _, test := range tests { - // add the same calltreeFunctions twice: once coming from a profile/chunk with - // ID 1 and the second one with ID 2 - ma.AddFunctions(test.calltreeFunctions, examples.ExampleMetadata{ProfileID: "1"}) - ma.AddFunctions(test.calltreeFunctions, examples.ExampleMetadata{ProfileID: "2"}) - if diff := testutil.Diff(ma, test.want); diff != "" { - t.Fatalf("Result mismatch: got - want +\n%s", diff) - } - } -} - -func TestAggregatorToMetrics(t *testing.T) { - tests := []struct { - name string - Aggregator Aggregator - want []examples.FunctionMetrics - }{ - { - name: "toMetrics", - Aggregator: Aggregator{ - MaxUniqueFunctions: 100, - CallTreeFunctions: map[uint32]nodetree.CallTreeFunction{ - 0: { - Function: "a", - Fingerprint: 0, - DurationsNS: []uint64{1, 2, 3, 4, 10, 8, 7, 11, 20}, - SumDurationNS: 66, - SumSelfTimeNS: 66, - SampleCount: 2, - }, - 1: { - Function: "b", - Fingerprint: 1, - DurationsNS: []uint64{1, 2, 3, 4, 10, 8, 7, 11, 20}, - SumDurationNS: 66, - SumSelfTimeNS: 66, - SampleCount: 2, - }, - }, // end callTreeFunctions - FunctionsMetadata: map[uint32]FunctionsMetadata{ - 0: { - MaxVal: 66, - Worst: examples.ExampleMetadata{ProfileID: "1"}, - Examples: []examples.ExampleMetadata{{ProfileID: "1"}, {ProfileID: "2"}}, - }, - 1: { - MaxVal: 66, - Worst: examples.ExampleMetadata{ProfileID: "3"}, - Examples: []examples.ExampleMetadata{{ProfileID: "1"}, {ProfileID: "3"}}, - }, - }, // end functionsMetadata - }, // end Aggregator - want: []examples.FunctionMetrics{ - { - Name: "a", - Fingerprint: 0, - P75: 10, - P95: 20, - P99: 20, - Count: 2, - Sum: 66, - SumSelfTime: 66, - Avg: float64(66) / float64(9), - Worst: examples.ExampleMetadata{ProfileID: "1"}, - Examples: []examples.ExampleMetadata{{ProfileID: "1"}, {ProfileID: "2"}}, - }, - { - Name: "b", - Fingerprint: 1, - P75: 10, - P95: 20, - P99: 20, - Count: 2, - Sum: 66, - SumSelfTime: 66, - Avg: float64(66) / float64(9), - Worst: examples.ExampleMetadata{ProfileID: "3"}, - Examples: []examples.ExampleMetadata{{ProfileID: "1"}, {ProfileID: "3"}}, - }, - }, // want - }, - } - - for _, test := range tests { - metrics := test.Aggregator.ToMetrics() - sort.Slice(metrics, func(i, j int) bool { - return metrics[i].Name < metrics[j].Name - }) - if diff := testutil.Diff(metrics, test.want); diff != "" { - t.Fatalf("Result mismatch: got - want +\n%s", diff) - } - } -} diff --git a/internal/nodetree/nodetree.go b/internal/nodetree/nodetree.go index f7184f98..42805a25 100644 --- a/internal/nodetree/nodetree.go +++ b/internal/nodetree/nodetree.go @@ -39,12 +39,16 @@ type ( Package string `json:"package"` Path string `json:"path,omitempty"` - EndNS uint64 `json:"-"` - Frame frame.Frame `json:"-"` - Occurrence uint32 `json:"-"` - SampleCount int `json:"-"` - StartNS uint64 `json:"-"` - Profiles map[examples.ExampleMetadata]struct{} `json:"profiles,omitempty"` + DurationsNS []uint64 `json:"-"` + EndNS uint64 `json:"-"` + Frame frame.Frame `json:"-"` + Occurrence uint32 `json:"-"` + SampleCount int `json:"-"` + SelfTimeNS uint64 `json:"-"` + StartNS uint64 `json:"-"` + Profiles map[examples.ExampleMetadata]struct{} `json:"profiles,omitempty"` + WorstSelfTime uint64 `json:"-"` + WorstProfile examples.ExampleMetadata `json:"-"` } ) @@ -73,6 +77,49 @@ func NodeFromFrame(f frame.Frame, start, end, fingerprint uint64) *Node { return &n } +func (n *Node) RecursiveComputeSelfTime() { + for _, c := range n.Children { + c.RecursiveComputeSelfTime() + } + + var childrenApplicationDurationNS uint64 + var childrenSystemDurationNS uint64 + + for _, c := range n.Children { + if c.IsApplication { + childrenApplicationDurationNS += c.DurationNS + } else { + childrenSystemDurationNS += c.DurationNS + } + } + + if n.IsApplication { + if n.DurationNS > childrenApplicationDurationNS { + n.SelfTimeNS = n.DurationNS - childrenApplicationDurationNS + } else { + n.SelfTimeNS = 0 + } + } else { + sum := childrenApplicationDurationNS + childrenSystemDurationNS + if n.DurationNS > sum { + n.SelfTimeNS = n.DurationNS - sum + } else { + n.SelfTimeNS = 0 + } + } +} + +func (n *Node) Visit(visitor func(*Node, int)) { + n.visit(visitor, 0) +} + +func (n *Node) visit(visitor func(*Node, int), depth int) { + visitor(n, depth) + for _, c := range n.Children { + c.visit(visitor, depth+1) + } +} + func (n *Node) ShallowCopyWithoutChildren() *Node { clone := Node{ EndNS: n.EndNS, @@ -88,6 +135,7 @@ func (n *Node) ShallowCopyWithoutChildren() *Node { StartNS: n.StartNS, Profiles: n.Profiles, DurationNS: n.DurationNS, + SelfTimeNS: n.SelfTimeNS, } return &clone @@ -172,7 +220,7 @@ func (n *Node) CollectFunctions( applicationDurationNS = n.DurationNS } - if nodeDepth >= minDepth && shouldAggregateFrame(n.Frame) { + if nodeDepth >= minDepth && ShouldAggregateFrame(n.Frame) { var selfTimeNS uint64 if n.IsApplication { @@ -238,7 +286,7 @@ func (n *Node) CollectFunctions( return applicationDurationNS, n.DurationNS - applicationDurationNS } -func shouldAggregateFrame(frame frame.Frame) bool { +func ShouldAggregateFrame(frame frame.Frame) bool { frameFunction := frame.Function // frames with no name are not valuable for aggregation diff --git a/internal/nodetree/nodetree_test.go b/internal/nodetree/nodetree_test.go index 41028952..a286c40c 100644 --- a/internal/nodetree/nodetree_test.go +++ b/internal/nodetree/nodetree_test.go @@ -500,3 +500,184 @@ func TestIsSymbolicated(t *testing.T) { }) } } + +func TestComputeSelfTime(t *testing.T) { + tests := []struct { + name string + node Node + selfTime uint64 + }{ + { + name: "single system frame", + node: Node{ + Children: []*Node{}, + DurationNS: 1000, + IsApplication: false, + }, + selfTime: 1000, + }, + { + name: "single application frame", + node: Node{ + Children: []*Node{}, + DurationNS: 1000, + IsApplication: true, + }, + selfTime: 1000, + }, + { + name: "system frame with single system frame child full duration", + node: Node{ + Children: []*Node{ + { + Children: []*Node{}, + DurationNS: 1000, + IsApplication: false, + }, + }, + DurationNS: 1000, + IsApplication: false, + }, + selfTime: 0, + }, + { + name: "system frame with single system frame child partial duration", + node: Node{ + Children: []*Node{ + { + Children: []*Node{}, + DurationNS: 500, + IsApplication: false, + }, + }, + DurationNS: 1000, + IsApplication: false, + }, + selfTime: 500, + }, + { + name: "system frame with single system frame child partial duration", + node: Node{ + Children: []*Node{ + { + Children: []*Node{}, + DurationNS: 500, + IsApplication: false, + }, + }, + DurationNS: 1000, + IsApplication: false, + }, + selfTime: 500, + }, + { + name: "system frame with single application frame child full duration", + node: Node{ + Children: []*Node{ + { + Children: []*Node{}, + DurationNS: 1000, + IsApplication: true, + }, + }, + DurationNS: 1000, + IsApplication: false, + }, + selfTime: 0, + }, + { + name: "system frame with single application frame child partial duration", + node: Node{ + Children: []*Node{ + { + Children: []*Node{}, + DurationNS: 500, + IsApplication: true, + }, + }, + DurationNS: 1000, + IsApplication: false, + }, + selfTime: 500, + }, + { + name: "system frame with multiple child frames", + node: Node{ + Children: []*Node{ + { + Children: []*Node{}, + DurationNS: 500, + IsApplication: true, + }, + { + Children: []*Node{}, + DurationNS: 250, + IsApplication: false, + }, + }, + DurationNS: 1000, + IsApplication: false, + }, + selfTime: 250, + }, + { + name: "application frame with single application frame child full duration", + node: Node{ + Children: []*Node{ + { + Children: []*Node{}, + DurationNS: 1000, + IsApplication: true, + }, + }, + DurationNS: 1000, + IsApplication: true, + }, + selfTime: 0, + }, + { + name: "application frame with single application frame child partial duration", + node: Node{ + Children: []*Node{ + { + Children: []*Node{}, + DurationNS: 500, + IsApplication: true, + }, + }, + DurationNS: 1000, + IsApplication: true, + }, + selfTime: 500, + }, + { + name: "application frame with multiple child frames", + node: Node{ + Children: []*Node{ + { + Children: []*Node{}, + DurationNS: 500, + IsApplication: true, + }, + { + Children: []*Node{}, + DurationNS: 250, + IsApplication: false, + }, + }, + DurationNS: 1000, + IsApplication: true, + }, + selfTime: 500, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(*testing.T) { + tt.node.RecursiveComputeSelfTime() + if diff := testutil.Diff(tt.node.SelfTimeNS, tt.selfTime); diff != "" { + t.Fatalf("Result mismatch: got - want +\n%s", diff) + } + }) + } +} diff --git a/internal/profile/profile.go b/internal/profile/profile.go index 1d177cba..38b3f3d7 100644 --- a/internal/profile/profile.go +++ b/internal/profile/profile.go @@ -81,7 +81,15 @@ func (p Profile) MarshalJSON() ([]byte, error) { } func (p *Profile) CallTrees() (map[uint64][]*nodetree.Node, error) { - return p.profile.CallTrees() + callTrees, err := p.profile.CallTrees() + + for _, callTreesForThread := range callTrees { + for _, callTree := range callTreesForThread { + callTree.RecursiveComputeSelfTime() + } + } + + return callTrees, err } func (p *Profile) DebugMeta() debugmeta.DebugMeta { From 60178ead767cc840b37f17aab6fe2a72835f313c Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Tue, 23 Sep 2025 15:08:02 -0400 Subject: [PATCH 2/4] remove commented out code --- internal/flamegraph/flamegraph.go | 1 - internal/flamegraph/flamegraph_test.go | 1 - 2 files changed, 2 deletions(-) diff --git a/internal/flamegraph/flamegraph.go b/internal/flamegraph/flamegraph.go index e60d2a02..5a3eb275 100644 --- a/internal/flamegraph/flamegraph.go +++ b/internal/flamegraph/flamegraph.go @@ -492,7 +492,6 @@ func GetFlamegraphFromCandidates( if ma != nil { metricsSpan := span.StartChild("processing metrics") for _, tree := range flamegraphTree { - // tree.RecursiveComputeSelfTime() tree.Visit(ma.AddFunction) } fm := ma.ToMetrics() diff --git a/internal/flamegraph/flamegraph_test.go b/internal/flamegraph/flamegraph_test.go index fbf71c93..176bf854 100644 --- a/internal/flamegraph/flamegraph_test.go +++ b/internal/flamegraph/flamegraph_test.go @@ -570,7 +570,6 @@ func TestFlamegraphAggregation(t *testing.T) { ma := metrics.NewAggregator(100, 5, 0, false) for _, tree := range ft { - // tree.RecursiveComputeSelfTime() tree.Visit(ma.AddFunction) } m := ma.ToMetrics() From 8bd7728ba21a294af7cef8dadadec2f69b9e9839 Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Tue, 23 Sep 2025 15:11:36 -0400 Subject: [PATCH 3/4] limit unique functions --- internal/metrics/metrics.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index bed8c007..c4b2aaf6 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -143,7 +143,9 @@ func (a *Aggregator) AddFunction(n *nodetree.Node, depth int) { } for example := range n.Profiles { - function.Examples = append(function.Examples, example) + if len(function.Examples) < a.maxUniqueFunctions { + function.Examples = append(function.Examples, example) + } } if function.WorstSelfTime < n.WorstSelfTime { From 3d76f1fed81237c656204e997be934847c17584c Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Tue, 23 Sep 2025 15:20:01 -0400 Subject: [PATCH 4/4] update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b43eca95..79ddcf61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,7 +35,7 @@ - Add default non-root user in the Docker image. ([#593](https://github.com/getsentry/vroom/pull/593)) - Classify macOS frames from an application as application frames. ([#604](https://github.com/getsentry/vroom/pull/604)) - Return number of occurrences in flamegraph. ([#622](https://github.com/getsentry/vroom/pull/622), [#625](https://github.com/getsentry/vroom/pull/625)) -- Use function duration when computing metrics ([#627](https://github.com/getsentry/vroom/pull/627), [#628](https://github.com/getsentry/vroom/pull/628), [#629](https://github.com/getsentry/vroom/pull/629), [#630](https://github.com/getsentry/vroom/pull/630)) +- Use function duration when computing metrics ([#627](https://github.com/getsentry/vroom/pull/627), [#628](https://github.com/getsentry/vroom/pull/628), [#629](https://github.com/getsentry/vroom/pull/629), [#630](https://github.com/getsentry/vroom/pull/630), [#635](https://github.com/getsentry/vroom/pull/635)) - Remove Unused metrics endpoint ([#633](https://github.com/getsentry/vroom/pull/633)) **Bug Fixes**: