Skip to content

Commit

Permalink
Add SortBy and GroupBy actions for Icicle Graphs (#3479)
Browse files Browse the repository at this point in the history
* ui/package/profile/ProfileIcicleGraph: Add more actions

We want to add sorting to the icicle graph itself. This allows to not only sort by function name (the default and what's currently supported) but also sort by cumulative values (such that the highest functions are always left) and in compare mode by diff value (meaning the functions that got worse are left and better right).

This can make finding specific things even easier.

* proto/parca: Add optional groupBy field strings to Query

* ui/shared/profile: Add GroupBy dropdown to Arrow IcicleGraph

* ui/shared/profile: Fix small issues

* pkg/query: Fix BenchmarkQuery using RenderReport

* proto: Add missing GroupBy comment

* Re-rendering fix

---------

Co-authored-by: Manoj Vivek <p.manoj.vivek@gmail.com>
  • Loading branch information
metalmatze and manojVivek authored Jul 20, 2023
1 parent f922df8 commit c57c71f
Show file tree
Hide file tree
Showing 12 changed files with 1,157 additions and 524 deletions.
1,070 changes: 575 additions & 495 deletions gen/proto/go/parca/query/v1alpha1/query.pb.go

Large diffs are not rendered by default.

191 changes: 191 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.

28 changes: 28 additions & 0 deletions gen/proto/swagger/parca/query/v1alpha1/query.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,17 @@
"required": false,
"type": "number",
"format": "float"
},
{
"name": "groupBy.fields",
"description": "the names of the fields to group by.\nspecial fields are the ones prefixed with \"labels.\" which are grouping by pprof labels.",
"in": "query",
"required": false,
"type": "array",
"items": {
"type": "string"
},
"collectionFormat": "multi"
}
],
"tags": [
Expand Down Expand Up @@ -723,6 +734,10 @@
"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"
},
"groupBy": {
"$ref": "#/definitions/v1alpha1GroupBy",
"title": "group_by indicates the fields to group by"
}
},
"title": "QueryRequest is a request for a profile query"
Expand Down Expand Up @@ -1108,6 +1123,19 @@
},
"title": "FlamegraphRootNode is a root node of a flame graph"
},
"v1alpha1GroupBy": {
"type": "object",
"properties": {
"fields": {
"type": "array",
"items": {
"type": "string"
},
"description": "the names of the fields to group by.\nspecial fields are the ones prefixed with \"labels.\" which are grouping by pprof labels."
}
},
"title": "GroupBy encapsulates the repeated fields to group by"
},
"v1alpha1LabelSet": {
"type": "object",
"properties": {
Expand Down
18 changes: 15 additions & 3 deletions pkg/query/columnquery.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ func (q *ColumnQueryAPI) Query(ctx context.Context, req *pb.QueryRequest) (*pb.Q
req.GetReportType(),
req.GetNodeTrimThreshold(),
filtered,
req.GetGroupBy().GetFields(),
)
}

Expand Down Expand Up @@ -240,8 +241,9 @@ func (q *ColumnQueryAPI) renderReport(
typ pb.QueryRequest_ReportType,
nodeTrimThreshold float32,
filtered int64,
groupBy []string,
) (*pb.QueryResponse, error) {
return RenderReport(ctx, q.tracer, p, typ, nodeTrimThreshold, filtered, q.tableConverterPool, q.mem)
return RenderReport(ctx, q.tracer, p, typ, nodeTrimThreshold, filtered, groupBy, q.tableConverterPool, q.mem)
}

func RenderReport(
Expand All @@ -251,6 +253,7 @@ func RenderReport(
typ pb.QueryRequest_ReportType,
nodeTrimThreshold float32,
filtered int64,
groupBy []string,
pool *sync.Pool,
mem memory.Allocator,
) (*pb.QueryResponse, error) {
Expand Down Expand Up @@ -291,8 +294,17 @@ func RenderReport(
},
}, nil
case pb.QueryRequest_REPORT_TYPE_FLAMEGRAPH_ARROW:
// TODO: Make the fields to aggregate by configurable via the API.
fa, total, err := GenerateFlamegraphArrow(ctx, mem, tracer, p, []string{FlamegraphFieldFunctionName}, nodeTrimFraction)
allowedGroupBy := map[string]struct{}{
FlamegraphFieldFunctionName: {},
FlamegraphFieldLabels: {},
}
for _, f := range groupBy {
if _, allowed := allowedGroupBy[f]; !allowed {
return nil, status.Errorf(codes.InvalidArgument, "invalid group by field: %s", f)
}
}

fa, total, err := GenerateFlamegraphArrow(ctx, mem, tracer, p, groupBy, nodeTrimFraction)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to generate arrow flamegraph: %v", err.Error())
}
Expand Down
12 changes: 11 additions & 1 deletion pkg/query/columnquery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1145,7 +1145,17 @@ func BenchmarkQuery(b *testing.B) {
b.ResetTimer()

for i := 0; i < b.N; i++ {
_, _ = RenderReport(ctx, tracer, sp, pb.QueryRequest_REPORT_TYPE_FLAMEGRAPH_ARROW, 0, 0, NewTableConverterPool(), memory.DefaultAllocator)
_, _ = RenderReport(
ctx,
tracer,
sp,
pb.QueryRequest_REPORT_TYPE_FLAMEGRAPH_ARROW,
0,
0,
[]string{FlamegraphFieldFunctionName},
NewTableConverterPool(),
memory.DefaultAllocator,
)
}
}

Expand Down
10 changes: 10 additions & 0 deletions proto/parca/query/v1alpha1/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,16 @@ message QueryRequest {

// node_trim_threshold is the threshold % where the nodes with Value less than this will be removed from the report
optional float node_trim_threshold = 7;

// group_by indicates the fields to group by
optional GroupBy group_by = 8;
}

// GroupBy encapsulates the repeated fields to group by
message GroupBy {
// the names of the fields to group by.
// special fields are the ones prefixed with "labels." which are grouping by pprof labels.
repeated string fields = 1;
}

// Top is the top report type
Expand Down
Loading

0 comments on commit c57c71f

Please sign in to comment.