Simplify go-torch: use raw pprof output, and use go-flags

[prashantv]

Simplify go-torch:

• use go-flags instead of cli since we only need flags, we don't have subcommands etc. This simplifies the code and lets us use structs to access the command line flags.
• Use raw output from pprof rather, which contains every single sample that we can convert to call stacks for flame graph scripts
• reduce use of interfaces, test the real concrete implementations by having tests invoke "go tool pprof" and creating fake flamegraph scripts in our PATH for tests etc
• removed logrus to reduce dependencies since we don't really need a smart logger.

A lot of logic has been moved around into 2 packages:
pprof: for invoking pprof and parsing the raw output from it
renderer: for invoking flamegraph scripts

r: @sectioneight

[ascandella]

this is a hoog change. i will try to get to this tomorrow, but more likely it will be this weekend

[ascandella]

is this syntax preferable over new(options)?

[prashantv]

I don't think there's any strong preferences - new was kind of the predecessor to the &T{} syntax. There's a few online discussions about wanting to remove new but it'll stay because of backwards compatibility.

I tend to prefer &T{}

[prashantv]

Yeah, it's a huge change, I got a little carried away with the refactoring. Don't need to review immediately - early next week is fine, thanks!

[sandlerben]

@prashantv thank you for putting in a lot of work/time to make these improvements! If you don't mind, I'd like to review these changes, too. I'm pretty busy with classes and clubs, but I can definitely have it reviewed by early next week.

[prashantv]

@sandlerben sure, if you can review by early next week that'd be great!

[sandlerben]

@prashantv just to keep you updated, I plan to review it tomorrow.

[sandlerben]

When I call go-torch --help I see:

Usage:
  go-torch [OPTIONS]

Application Options:
  -u, --url=         Base URL of your Go program (http://localhost:8080)
  -s, --suffix=      URL path of pprof profile (/debug/pprof/profile)
  -b, --binaryinput= file path of previously saved binary profile. (binary profile is anything accepted by https://golang.org/cmd/pprof)
      --binaryname=  file path of the binary that the binaryinput is for, used for pprof inputs
  -t, --time=        Duration to profile for (30)
  -f, --file=        Output file name (must be .svg) (torch.svg)
  -p, --print        Print the generated svg to stdout instead of writing to file
  -r, --raw          Print the raw call graph output to stdout instead of creating a flame graph; use with Brendan Gregg's flame graph perl script (see
                     https://github.com/brendangregg/FlameGraph)

Help Options:
  -h, --help         Show this help message

2015/09/15 14:49:26 Failed: could not parse options: Usage:
  go-torch [OPTIONS]

Application Options:
  -u, --url=         Base URL of your Go program (http://localhost:8080)
  -s, --suffix=      URL path of pprof profile (/debug/pprof/profile)
  -b, --binaryinput= file path of previously saved binary profile. (binary profile is anything accepted by https://golang.org/cmd/pprof)
      --binaryname=  file path of the binary that the binaryinput is for, used for pprof inputs
  -t, --time=        Duration to profile for (30)
  -f, --file=        Output file name (must be .svg) (torch.svg)
  -p, --print        Print the generated svg to stdout instead of writing to file
  -r, --raw          Print the raw call graph output to stdout instead of creating a flame graph; use with Brendan Gregg's flame graph perl script (see
                     https://github.com/brendangregg/FlameGraph)

Help Options:
  -h, --help         Show this help message

It looks like "application options" are getting printed twice. Also, what does "Failed: could not parse options: Usage:" mean?

[sandlerben]

flamegraph.pl is in my path but for some reason I am getting the no perl script error. Do you know what might be going on?

$ which flamegraph.pl
/usr/local/bin/flamegraph.pl
$ go-torch --time=10
2015/09/15 15:00:07 Failed: could not collapse stacks: Cannot find flamegraph scripts in the PATH or current directory. You can download the script at https://github.com/brendangregg/FlameGraph. These scripts should be added to your PATH or in the directory where go-torch is executed. Alternatively, you can run go-torch with the --raw flag.

[sandlerben]

It also doesn't work even when flamegraph.pl is in the current directory. Very weird.

[prashantv]

Do you have stackcollapse.pl - it's failing on collapsing stacks which uses stackcollapse? I'll make the error message more descriptive about which script failed.

[sandlerben]

Oh, no I don't. So we need both of those now?

[prashantv]

Looked into what stack collapsing does, it was pretty trivial so just implemented it. No longer need it.

[sandlerben]

@prashantv I made a few small comments. Once you make those, I'll look it over one more time and then it should be good to merge.

On an unrelated note, it's sad to see most of my summer's work being deleted but awesome to see how much better go-torch will work now. 😢 😀

[prashantv]

@sandlerben Thank you for the review! I've moved stackcollapse.pl logic into go-torch so that the raw output works as before. I've fixed the help error message problem too.

As for the code - I wish we'd known about the raw output before, since it really simplified it a lot. Your work was still useful though -- productionizing it, and figuring out how to put the pieces together, since the overall flow of data is still the same :)

@sectioneight did you want to review this change as well?

[ascandella]

Still haven't had time to take a real look. Go ahead and merge.

[sandlerben]

Maybe I can try to find a (lightweight) way to add color back later.

[prashantv]

Thanks for reviewing @sandlerben

We can definitely add colour back as a separate PR - we can use the color package, or there's also colog:
https://github.com/comail/colog

I think we really need a minimal amount of support though, so we could just get a couple of escape codes (cyan for info prints, red for errors) rather than adding a new dependency.