astrofrog / psrecord

Record the CPU and memory activity of a process :chart_with_upwards_trend:
BSD 2-Clause "Simplified" License
591 stars 76 forks source link

if neither --log nor --plot is passed nothing happens, by default log to stdout #50

Closed CristianCantoro closed 4 months ago

CristianCantoro commented 5 years ago

Hi,

this both a bug report and a proposal.

From reading the code what I get is that if neither --log LOG nor --plot PLOTis passed psrecord still work, but no data gets printed, saved or plotted anywhere.

$ psrecord 3170  
Attaching to process 3170

Is this behavior intended?

I cannot imagine why this should not raise an error saying "You need to specify at least one between --log LOG and --plot PLOT". In my opinion, one sensible default would be writing to standard output, i.e. like if psrecord was called like this:

$ psrecord --log /dev/stdout 3170  
Attaching to process 3170

In this way --log and --plot are truly options, and their usage reflects the usage string of psrecord.

Instead, if you want to make either one of the two options mandatory it's a little bit more tricky. Unfortunately argparse is not able to handle this use case, that is having two options of whom at least one is required (also, they are not mutually exclusive).

One simple way would be to add a check like this:

if not args.log and not args.plot:
    raise ValueError(`"You need to specify at least one between --log LOG and --plot PLOT"`)

With docopt - well, POSIX - this would be represented like this:

usage:
  psrecord [options] --log LOG process_id_or_command
  psrecord [options] --plot PLOT process_id_or_command
  psrecord [options] --log LOG --plot PLOT process_id_or_command

Record CPU and memory usage for a process

positional arguments:
  process_id_or_command
                        the process id or command

optional arguments:
  -h, --help            show this help message and exit
  --log LOG             output the statistics to a file
  --plot PLOT           output the statistics to a plot
  --duration DURATION   how long to record for (in seconds). If not specified,
                        the recording is continuous until the job exits.
  --interval INTERVAL   how long to wait between each sample (in seconds). By
                        default the process is sampled as often as possible.
  --include-children    include sub-processes in statistics (results in a
                        slower maximum sampling rate).
trevorboydsmith commented 2 years ago

to get psrecord to print to the console what i want/need, i wrote a simple 30 line shell script:

i would prefer the default behavior would be to print to console the same thing that is output to the log file --> then i could get rid of my 30 line shell script.

astrofrog commented 2 years ago

I agree that printing out to stdout would make sense by default and would be happy to review a PR enabling this!

CristianCantoro commented 2 years ago

@astrofrog wrote:

I agree that printing out to stdout would make sense by default and would be happy to review a PR enabling this!

Done!

Note that I implemented it as explained above: if neither --log nor --plot is passed then psrecord will print to stdfout. However, if you pass --plot, it won't print anything to stdout. This means that strictly speaking, this behavior is not a default for a missing --log.

It would be trivial to make stdout the default argument for --log and independent from --plot, which may be a more sensible choice.