pprof icon indicating copy to clipboard operation
pprof copied to clipboard

web: flamegraph: show filename in tooltip

Open mhansen opened this issue 3 years ago • 11 comments

Forked off from https://github.com/google/pprof/pull/649#issuecomment-930796262

What version of pprof are you using?

7fe48b4c820be13151ae35ce5a5e3f54f1b53eef

What did you do?

Open the flamegraph view in the web browser:

image

What did you expect to see?

Filename in the tooltip -- in this case I used -roottag thread to add synthetic stack frames to the root of the trace, with the Filename = "thread", so help label the node.

What did you see instead?

No filename.

Fixing this might require moving to a DOM-element for the tooltip, I'm not sure if browsers natively support multiline tooltips?

mhansen avatar Oct 01 '21 07:10 mhansen

Looks like there's options for an HTML-based tooltip in the latest d3-flamegraph: https://github.com/spiermar/d3-flame-graph#tooltip

mhansen avatar Oct 09 '21 23:10 mhansen

The filename isn't passed as JSON to the FlameGraph yet, we'll want to add it to the {{.FlameGraph}} template variable to access it in the client.

mhansen avatar Oct 09 '21 23:10 mhansen

Can't just pass node.Info.File to {{.FlameGraph}} as .File doesn't seem to be filled in yet, it seems empty in my test profiles.

mhansen avatar Oct 09 '21 23:10 mhansen

@mhansen RE empty filenames: can you try passing -filefunctions flag to pprof?

aalexand avatar Oct 10 '21 19:10 aalexand

Thanks Alexey, -filefunctions works pretty well! Works on the synthetic tag filenames too.

image

mhansen avatar Oct 15 '21 20:10 mhansen

I'm still wondering if we could put the filename in the tooltip by default, but I could go either way on it (this at least solves my immediate problem of how to demo tagroot in a blog post).

mhansen avatar Oct 15 '21 20:10 mhansen

I think we went back and forth on having -functions vs. -filefunctions as the default granularity in the past, so I'm not too enthusiastic about flipping the default. I could be convinced, but this might break some users.

aalexand avatar Oct 15 '21 21:10 aalexand

https://github.com/google/pprof/issues/106 is when we changed this.

We could force a different default in the flame graph generation perhaps, though we'll need to make sure this only changes the defaults and choosing a different granularity on the command line is still possible (e.g. -addresses etc).

aalexand avatar Oct 15 '21 21:10 aalexand

@mhansen For the blog post, you could simply have the instructions there as "Run pprof with -filefunctions flag"?

aalexand avatar Oct 15 '21 21:10 aalexand

I don't think we should flip the defaults. I'm just wondering about maybe sending the filename and the function name down in the JSON, and making a richer tooltip that shows the filename+function.

On Sat, 16 Oct 2021 at 08:28, Alexey Alexandrov @.***> wrote:

#106 https://github.com/google/pprof/issues/106 is when we changed this.

We could force a different default in the flame graph generation perhaps, though we'll need to make sure this only changes the defaults and choosing a different granularity on the command line is still possible (e.g. -addresses etc).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/pprof/issues/655#issuecomment-944685414, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZYOLK3DV5CFPZRYJA53TUHCMGFANCNFSM5FECQC5A .

mhansen avatar Oct 15 '21 21:10 mhansen

The picture in my mind is something like what Firefox Profiler does: a baby step towards a richer tooltip that shows the filename (and doesn't mean the flamegraph has to also show the filename). Could start with the filename then grow that tooltip into more information like self & total time.

image

mhansen avatar Oct 15 '21 21:10 mhansen