pkg/query: Create flamegraph_arrow #3069
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
asubiotto
reviewed
May 4, 2023
pkg/query/flamegraph_arrow.go
Outdated
| ) | ||
|
|
||
| const ( | ||
| flamegraphFieldMappingStart = "mapping_start" |
There was a problem hiding this comment.
I wonder if we can make this code cleaner by having a map from field name to some struct that encapsulates the arrow type and holds a reference to the builder.
metalmatze
force-pushed
the
flamegraph-arrow
branch
from
4d2f122
to
001fede
Compare
|
Next week, I'll bring this PR into a mergeable state. So we can make progress on smaller follow-up PRs. The follow ups on the Go side should be:
|
metalmatze
force-pushed
the
flamegraph-arrow
branch
from
060cabc
to
375ee8c
Compare
metalmatze
force-pushed
the
flamegraph-arrow
branch
from
7a8f26b
to
a2a06fe
Compare
|
There's one more fix I need to do in upstream Arrow (due to which the CI is going to fail here) |
brancz
approved these changes
Jun 20, 2023
| int32 height = 3; | ||
|
|
||
| // trimmed is the amount of cumulative value trimmed from the flame graph. | ||
| int64 trimmed = 4; |
There was a problem hiding this comment.
This is mostly used for debugging, otherwise, we don't really need it.
|
The new changes break the tests again. I'll fix those tomorrow. |
Additionally, we now update the root's row cumulative value and the end and correctly build the children lists.
…regating This has huge performance improvements due to not allocating strings when converting the bytes to string.
…prof labels This will allow users to specify if pprof labels should be ignored and aggregated away, or if the flame graph should consider their values and group by the once that are equal.
Also improve ergonomics by introducing isRoot, isLeaf and parent helper structs and funcs.
metalmatze
force-pushed
the
flamegraph-arrow
branch
from
3829fde
to
9efa875
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This creates a new flame graph representation. Instead of creating the flame graph with Go structs directly, we utilize Apache Arrow to create a record including the rows of each location.
The way the arrow schema looks can be viewed in
pkg/query/flamegraph_arrow.goandpkg/query/flamegraph_arrow_test.gocontains an example of what we currently have.The API currently doesn't expose this type just yet. Only the enum type was added to proto to be able to call
RenderReportwithQueryRequest_REPORT_TYPE_FLAMEGRAPH_ARROW.