Command line error messages could be more helpful

[aalexand]

The command line for pprof is not as helpful as it could be. For example:

(pprof) sample_index
(pprof)

It would have been helpful if the command without any parameters was not accepted silently and instead gave some helpful output.

(pprof) sample_index 1
Unrecognized command: "sample_index"

The command with an invalid parameter tells me that the command is not recognised, not that the parameter is invalid. Again it would be helpful to provide me with more feedback.

(pprof) help
   sample_index     Sample value to report (0-based index or name)

Notice that the help output does not make it clear that the command is actually expecting an =N parameter. It could be useful to accept the space delimited syntax, or update the help to say sample_index=N

(pprof) sample_index=foo
sample_index "foo" must be one of: [allocated_objects allocated_space deallocated_objects deallocated_space]

Having determined that =N is necessary, the output is short and helpful. This would be a good candidate for the return value if no appropriate parameter is provided, or if the command doesn't make sense.

The help on sample_index is also useful. So the information is there, but we don't surface it very readily. :)

(pprof) help sample_index
Sample value to report (0-based index or name)
Profiles contain multiple values per sample.
Use sample_index=i to select the ith value (starting at 0).
Or use sample_index=name, with name in [allocated_objects allocated_space deallocated_objects deallocated_space].

[wyk9787]

It seems like if there is no index passed to "sample_index", it is still selecting the last sample value.

pprof/profile/index.go

Line 36 in 7d83b28

return len(p.SampleType) - 1, nil

Is this the intended behavior? If so, I assume that's what we need to clarify in the cli, right?

[nolanmar511]

Using the last index as the sample_index is is intended behavior -- it's documented here (a lot of intentional pprof behavior is not documented, but this bit is at least).

If that could be made clearer, that would be okay, but probably not as important as the cases aalexand outlined.

These kind of issues likely impact options other than sample_index (like focus and other filtering options), so it would be awesome to address those, too.

[aalexand]

At least some other options do fail when no argument:

(pprof) nodecount
strconv.Atoi: parsing "": invalid syntax

So this may be indeed the case that we should make sample_index as the UI option not to accept an empty string (we likely want to do this outside of the function previously mentioned as that function is not at the UI level), plus error messages should be improved like the issue asks.