feat: add an "ipfs diag profile" command #8291
Conversation
Stebalien
force-pushed
the
feat/diag-profile
branch
2 times, most recently
from
01188ce
to
4117aed
Compare
This comment has been minimized.
This comment has been minimized.
Stebalien
force-pushed
the
feat/diag-profile
branch
2 times, most recently
from
5df2f5e
to
9270680
Compare
This should replace the "collect-profiles.sh" script and allow users to easily collect profiles. At the moment, it just dumps all profiles into a single zip file. It does this server-side so it's easy fetch them with curl. In the future, it would be nice to add: 1. Progress indicators (cpu profiles take 30 seconds). 2. An option to specify which profiles to collect. But we can handle that later. Unfortunately, this command doesn't produce symbolized svgs as I didn't want to depend on having a local go compiler.
Stebalien
force-pushed
the
feat/diag-profile
branch
from
9270680
to
d52d183
Compare
sys diag includes private information (like the interface addresses)
Stebalien
force-pushed
the
feat/diag-profile
branch
from
cd64cf3
to
77f3fed
Compare
| // Collect binary | ||
| if fi, err := openIPFSBinary(); err == nil { |
There was a problem hiding this comment.
I don't know if this is worth doing by default. We have the version info and it bloats the size of the zip. WDYT?
There was a problem hiding this comment.
We need it for the symbols in case the user deployed a custom binary. I guess we could include it iff the version is "dirty"?
Note: the zip is ~26MiB. So it's not terrible.
There was a problem hiding this comment.
It's not, although I think I've seen maximum sizes for emails and Slack attachments from external parties be lower. I'm not sure what the maximum GitHub issue attachment size is (I think I saw 25MB at one point, but I have no idea if that's still true)
Even if it's not the default we should probably have a flag here just in case
Co-authored-by: Adin Schmahmann <adin.schmahmann@gmail.com>
There was a problem hiding this comment.
Awesome!
As mentioned above I'm a little concerned about the zip file sizes containing binaries causing problems for us. Let's figure out what we want to do there (maybe it's just having a flag defaulted to including the binaries and then switching the defaults later if we're running into problems). Aside from that though LGTM.
Co-authored-by: Adin Schmahmann <adin.schmahmann@gmail.com>
The current "collect-profiles.sh" script does the same thing. Unfortunately, without the binary, we don't get the symbols. And I couldn't find a way to extract just the symbols. Well, a quick google turns up https://stackoverflow.com/questions/42554900/how-to-extract-own-symbol-table. But that seems like it'll be unreliable, especially on other operating systems. |
|
Fair enough. It's certainly not worse than the current script and if it's a problem we can deal with it later. Merge when ready.
You could in theory clone most of the code used by |
Well, there's probably a way to strip out everything except the symbol table, and make pprof think we're passing it the real binary. |
This should replace the "collect-profiles.sh" script and allow users to easily collect profiles.
Usage:
ipfs diag profileoripfs diag profile -o my-profile.zip.At the moment, it just dumps all profiles into a single zip file. It does this server-side so it's easy fetch them with curl.
In the future, it would be nice to add:
But we can handle that later.
Unfortunately, this command doesn't produce symbolized svgs as I didn't want to depend on having a local go compiler.