Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*: Flamegraph data trimming #2299

Merged
merged 10 commits into from
Dec 22, 2022
576 changes: 301 additions & 275 deletions gen/proto/go/parca/query/v1alpha1/query.pb.go

Large diffs are not rendered by default.

50 changes: 50 additions & 0 deletions gen/proto/go/parca/query/v1alpha1/query_vtproto.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions gen/proto/swagger/parca/query/v1alpha1/query.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,14 @@
"in": "query",
"required": false,
"type": "string"
},
{
"name": "nodeTrimThreshold",
"description": "node_trim_threshold is the threshold % where the nodes with Value less than this will be removed from the report",
"in": "query",
"required": false,
"type": "number",
"format": "float"
}
],
"tags": [
Expand Down Expand Up @@ -699,6 +707,11 @@
"filterQuery": {
"type": "string",
"title": "filter_query is the query string to filter the profile samples"
},
"nodeTrimThreshold": {
"type": "number",
"format": "float",
"title": "node_trim_threshold is the threshold % where the nodes with Value less than this will be removed from the report"
}
},
"title": "QueryRequest is a request for a profile query"
Expand Down Expand Up @@ -941,6 +954,11 @@
"$ref": "#/definitions/metastorev1alpha1Function"
},
"description": "function deduplicated by their ID to be referenced by nodes."
},
"untrimmedTotal": {
"type": "string",
"format": "int64",
"title": "untrimmed_total is the total weight of the flame graph before trimming"
}
},
"title": "Flamegraph is the flame graph report type"
Expand Down
5 changes: 5 additions & 0 deletions gen/proto/swagger/parca/share/v1alpha1/share.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,11 @@
"$ref": "#/definitions/metastorev1alpha1Function"
},
"description": "function deduplicated by their ID to be referenced by nodes."
},
"untrimmedTotal": {
"type": "string",
"format": "int64",
"title": "untrimmed_total is the total weight of the flame graph before trimming"
}
},
"title": "Flamegraph is the flame graph report type"
Expand Down
4 changes: 2 additions & 2 deletions pkg/query/callgraph.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
)

const (
NodeCutOffFraction = 0.005
NodeCutOffFraction = float32(0.005)
)

func GenerateCallgraph(ctx context.Context, p *profile.Profile) (*querypb.Callgraph, error) {
Expand Down Expand Up @@ -182,7 +182,7 @@ func prunableNodes(nodes []*querypb.CallgraphNode, c int64) []*querypb.Callgraph
return nodes[i].Cumulative > nodes[j].Cumulative
})
i := 0
cutoffValue := (float64(c) * NodeCutOffFraction)
cutoffValue := (float64(c) * float64(NodeCutOffFraction))
for ; i < len(nodes); i++ {
if float64(nodes[i].Cumulative) < cutoffValue {
break
Expand Down
11 changes: 8 additions & 3 deletions pkg/query/columnquery.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func (q *ColumnQueryAPI) Query(ctx context.Context, req *pb.QueryRequest) (*pb.Q
p = q.filterProfileData(ctx, p, *req.FilterQuery)
}

return q.renderReport(ctx, p, req.GetReportType())
return q.renderReport(ctx, p, req.GetReportType(), req.GetNodeTrimThreshold())
}

func keepSample(s *profile.SymbolizedSample, filterQuery string) bool {
Expand Down Expand Up @@ -177,11 +177,16 @@ func (q *ColumnQueryAPI) filterProfileData(ctx context.Context, p *profile.Profi
}
}

func (q *ColumnQueryAPI) renderReport(ctx context.Context, p *profile.Profile, typ pb.QueryRequest_ReportType) (*pb.QueryResponse, error) {
func (q *ColumnQueryAPI) renderReport(ctx context.Context, p *profile.Profile, typ pb.QueryRequest_ReportType, nodeTrimThreshold float32) (*pb.QueryResponse, error) {
ctx, span := q.tracer.Start(ctx, "renderReport")
span.SetAttributes(attribute.String("reportType", typ.String()))
defer span.End()

nodeTrimFraction := float32(0)
if nodeTrimThreshold != 0 {
nodeTrimFraction = nodeTrimThreshold / 100
}

switch typ {
//nolint:staticcheck // SA1019: Fow now we want to support these APIs
case pb.QueryRequest_REPORT_TYPE_FLAMEGRAPH_UNSPECIFIED:
Expand All @@ -195,7 +200,7 @@ func (q *ColumnQueryAPI) renderReport(ctx context.Context, p *profile.Profile, t
},
}, nil
case pb.QueryRequest_REPORT_TYPE_FLAMEGRAPH_TABLE:
fg, err := GenerateFlamegraphTable(ctx, q.tracer, p)
fg, err := GenerateFlamegraphTable(ctx, q.tracer, p, nodeTrimFraction)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to generate flamegraph: %v", err.Error())
}
Expand Down
97 changes: 92 additions & 5 deletions pkg/query/flamegraph_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"github.com/parca-dev/parca/pkg/profile"
)

func GenerateFlamegraphTable(ctx context.Context, tracer trace.Tracer, p *profile.Profile) (*querypb.Flamegraph, error) {
func GenerateFlamegraphTable(ctx context.Context, tracer trace.Tracer, p *profile.Profile, nodeTrimFraction float32) (*querypb.Flamegraph, error) {
rootNode := &querypb.FlamegraphNode{}
current := rootNode

Expand Down Expand Up @@ -104,9 +104,10 @@ func GenerateFlamegraphTable(ctx context.Context, tracer trace.Tracer, p *profil
Diff: rootNode.Diff,
Children: rootNode.Children,
},
Total: rootNode.Cumulative,
Unit: p.Meta.SampleType.Unit,
Height: height + 1, // add one for the root
Total: rootNode.Cumulative,
UntrimmedTotal: rootNode.Cumulative,
Unit: p.Meta.SampleType.Unit,
Height: height + 1, // add one for the root

StringTable: tables.Strings(),
Mapping: tables.Mappings(),
Expand All @@ -120,7 +121,13 @@ func GenerateFlamegraphTable(ctx context.Context, tracer trace.Tracer, p *profil
f.Id = ""
}

return aggregateByFunctionTable(tables, flamegraph), nil
aggregatedFlamegraph := aggregateByFunctionTable(tables, flamegraph)

if nodeTrimFraction == 0 {
return aggregatedFlamegraph, nil
}

return TrimFlamegraph(ctx, tracer, aggregatedFlamegraph, nodeTrimFraction), nil
}

type tableConverter struct {
Expand Down Expand Up @@ -475,3 +482,83 @@ func equalsByNameTable(tables TableGetter, a, b *querypb.FlamegraphNode) bool {
strings := tables.Strings()
return strings[aFunction.NameStringIndex] == strings[bFunction.NameStringIndex]
}

type FlamegraphChildren []*querypb.FlamegraphNode

func (n FlamegraphChildren) Cumulative() int64 {
cumulative := int64(0)
for _, child := range n {
cumulative += child.Cumulative
}
return cumulative
}

func (n FlamegraphChildren) Diff() int64 {
diff := int64(0)
for _, child := range n {
diff += child.Diff
}
return diff
}

func TrimFlamegraph(ctx context.Context, tracer trace.Tracer, graph *querypb.Flamegraph, thresholdRate float32) *querypb.Flamegraph {
ctx, span := tracer.Start(ctx, "trimFlamegraph")
defer span.End()
if graph == nil {
return nil
}
total := graph.Total

threshold := int64(float64(thresholdRate) * float64(total))
var children FlamegraphChildren = trimFlamegraphNodes(ctx, tracer, graph.Root.Children, threshold)
newTotal := int64(0)
newDiff := int64(0)
if len(graph.Root.Children) > 0 {
newTotal = children.Cumulative()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should include the difference in cumulative due to node trimming in the proto response. I think it would be very confusing to click on a sample on a graph with let's say 100 CPU samples, and then only see 85 cumulative in the root node.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, how do we account for the difference in the Child nodes? As the sum of child nodes would still only be 85 right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I meant ultimately the UI should show something like "Showing 85 out of 100 samples"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So actually I meant that we should include the previous and trimmed total value in the response

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is samples the right terminology for all profile types?
Also would it be the right place to show this message or should we hide it under some i icon?

Screenshot 2022-12-21 at 12 01 21 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is done.

newDiff = children.Diff()
}

trimmedGraph := &querypb.Flamegraph{
Root: &querypb.FlamegraphRootNode{
Children: children,
Cumulative: newTotal,
Diff: newDiff,
},
Total: newTotal,
UntrimmedTotal: graph.Total,
Unit: graph.Unit,
Height: graph.Height,
StringTable: graph.StringTable,
Locations: graph.Locations,
Mapping: graph.Mapping,
Function: graph.Function,
}

return trimmedGraph
}

func trimFlamegraphNodes(ctx context.Context, tracer trace.Tracer, nodes []*querypb.FlamegraphNode, threshold int64) []*querypb.FlamegraphNode {
var trimmedNodes []*querypb.FlamegraphNode
for _, node := range nodes {
if node.Cumulative < threshold {
continue
}
var children FlamegraphChildren = trimFlamegraphNodes(ctx, tracer, node.Children, threshold)
newCum := int64(0)
newDiff := int64(0)
if len(node.Children) > 0 {
newCum = children.Cumulative()
newDiff = children.Diff()
} else {
newCum = node.Cumulative
newDiff = node.Diff
}
trimmedNodes = append(trimmedNodes, &querypb.FlamegraphNode{
Meta: node.Meta,
Cumulative: newCum,
Diff: newDiff,
Children: children,
})
}
return trimmedNodes
}
Loading