-
Notifications
You must be signed in to change notification settings - Fork 627
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
feat: flamegraph node time-series #2961
Conversation
// Stack trace of the call site. Root at call_site[0]. | ||
// Only stack traces having the prefix provided will be selected. | ||
// If empty, the filter is ignored. | ||
repeated Location call_site = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed it one more time. As the API it not used anywhere, it should not be an issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @bryanhuhta for the hackathon this is what we're going to use in all API when clicking on a function to reduce size of pprof and zoom on a single function.
// CallSiteValues represents statistics associated with a call tree node. | ||
type CallSiteValues struct { | ||
// Flat is the sum of sample values directly attributed to the node. | ||
Flat uint64 | ||
// Total is the total sum of sample values attributed to the node and | ||
// its descendants. | ||
Total uint64 | ||
// LocationFlat is the sum of sample values directly attributed to the | ||
// node location, irrespectively of the call chain. | ||
LocationFlat uint64 | ||
// LocationTotal is the total sum of sample values attributed to the | ||
// node location and its descendants, irrespectively of the call chain. | ||
LocationTotal uint64 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please note that at the moment only Total
is actually used. We can return all, but we need some toggle in UI that switches between them. Otherwise, the timeline view becomes too busy (especially, if group by
is used)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Resolves #2228