Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
CristianCantoro opened this issue Dec 12, 2018 · 3 comments

Comments

@CristianCantoro
Copy link
Contributor

CristianCantoro commented Dec 12, 2018

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
Copy link

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

  • passes --log ${LOG_FILENAME} to psrecord
  • backgrounds the psrecord process
  • registers a trap on SIGINT and SIGTERM to cleanup the log file
  • tails the output of the log file

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
Copy link
Owner

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

@CristianCantoro
Copy link
Contributor Author

@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.

astrofrog added a commit that referenced this issue Apr 25, 2024
If neither --log nor --plot is passed, log to stdout. Closes #50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants