feat: client side pprof merging for cumulative types #24
Conversation
korniltsev
force-pushed
the
feat/clientside_pprof_merging
branch
from
38bde26 to
b9385f9
Compare
|
hmm, pprof module requires Go 1.18 |
|
I think that vendoring/forking pprof package exclusively to support 1.15 (which is not supported officially already) might be redundant |
internal/cumulative/cumulative.go
Outdated
|
|
||
| // todo should we disableClientSideMerge if we fail to merge or keep trying? | ||
| // todo should we filter by enabled ps.profileTypes to reduce profile size ? maybe add a separate option ? | ||
| func (m *ProfileMerger) Merge(j *upstream.UploadJob) error { |
There was a problem hiding this comment.
One potential issue with this code is that it is using the prev field of the ProfileMerger struct to store the previous profile. However, it is not clear from the code provided how this field is being initialized or whether it is being reset after each call to the Merge function. This could potentially cause issues if the prev field is not being properly initialized or reset, as it could lead to unexpected behavior when merging profiles.
Additionally, the code is using the defer keyword to print a message to the console after the Merge function has completed. This is indicated by the comment //todo remove before merge. It is generally a good idea to remove any debug code (such as this defer statement) before merging your code, as it can cause unexpected behavior and make it difficult to understand and debug your code.
Finally, the code is using the fmt.Printf function to print a message to the console. This function should not be used in production code, as it is intended for debugging purposes only. In production code, it is generally a better idea to use a logging library (such as the standard log package) to log messages. This allows you to easily control the log level and output location, which can be useful for debugging and monitoring your application.
(I didn't write it, ChatGPT did)
There was a problem hiding this comment.
not resolving for lulz
No description provided.