Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 540274 - sys-process/cronbase: run-crons: add code to display script name for cron.{h,d,w,m}*
Summary: sys-process/cronbase: run-crons: add code to display script name for cron.{h,...
Status: CONFIRMED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal enhancement (vote)
Assignee: Gentoo's Team for Core System packages
URL:
Whiteboard:
Keywords:
: 553294 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-02-16 12:34 UTC by Tobias Klausmann (RETIRED)
Modified: 2023-01-29 05:24 UTC (History)
2 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
Add code to display script name for cron.{h,d,w,m}* (cronbase-scriptnames.patch,1.43 KB, patch)
2015-02-16 12:34 UTC, Tobias Klausmann (RETIRED)
Details | Diff
Use appropriate priority when logging through logger (use-appropriate-priority.patch,983 bytes, patch)
2015-07-22 09:18 UTC, Thomas Deutschmann (RETIRED)
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Klausmann (RETIRED) gentoo-dev 2015-02-16 12:34:39 UTC
Created attachment 396590 [details, diff]
Add code to display script name for cron.{h,d,w,m}*

If there are a lot of scripts in the hourly/daily/weekly/monthly cron
directories, it can become very difficult to hunt down which script a
particular message comes from. This patch prefixes script output with
the name and exit status of the script that was run.

This also changes behavior fro scripts that exit with non-zero status
but no output. Previously, there would be no message indicating the
(possible) failure. To recreate that behaviour (and only send messages
on output being non-empty), those scripts can be amended with an
unconditional "exit 0" or similar.
Comment 1 SpanKY gentoo-dev 2015-07-21 03:44:43 UTC
Comment on attachment 396590 [details, diff]
Add code to display script name for cron.{h,d,w,m}*

this doesn't actually send output to the syslog that i can see, and it relies on the non-standard `tempfile` from debianutils, and the tempfile handling looks racy (security), and the stat call is not needed

if we don't handle the no-output scenario, it makes the code much easier:
  $SCRIPT |& logger -i -p cron.info -t "run-crons.$SCRIPT"
  ret=${PIPESTATUS[0]}
  if [[ ${ret} -ne 0 ]] ; then
    logger -i -p cron.info -t run-crons "(`whoami`) EXIT ${ret} ($SCRIPT)"
  fi
Comment 2 Tobias Klausmann (RETIRED) gentoo-dev 2015-07-21 07:43:22 UTC
(In reply to SpanKY from comment #1)
> Comment on attachment 396590 [details, diff] [details, diff]
> Add code to display script name for cron.{h,d,w,m}*
> 
> this doesn't actually send output to the syslog that i can see, and it
> relies on the non-standard `tempfile` from debianutils, and the tempfile
> handling looks racy (security), and the stat call is not needed

Tempfile itself is not racy. And the &> redirect/truncate is atomic, so I don't see where the race would happen. But the dep on debianutils would be bad, yes.

> if we don't handle the no-output scenario, it makes the code much easier:
>   $SCRIPT |& logger -i -p cron.info -t "run-crons.$SCRIPT"
>   ret=${PIPESTATUS[0]}
>   if [[ ${ret} -ne 0 ]] ; then
>     logger -i -p cron.info -t run-crons "(`whoami`) EXIT ${ret} ($SCRIPT)"
>   fi

This would write to some logfile, whereas usually, failing cron jobs generate mails. I know I don't browse cron log files for failures and I'd rather not have to set up another logwatcher.

Note that there is also the short solution of bug 553294, which may be the best compromise.
Comment 3 SpanKY gentoo-dev 2015-07-21 09:31:45 UTC
(In reply to Tobias Klausmann from comment #2)

one command creates the file while another opens/writes while another opens/reads it.  those are not atomic operations.

i forgot about the e-mail aspect as i focus on logging personally.

i thought of doing a sed on the output directly but didn't feel like getting into handling the escape character correctly (bug 553294 fails when the script contains a | for example).  but if that satisfies your request, we can go that route as it is better as it maintains the streaming aspect and doesn't require any tempfile usage.

we can also carry over the PIPESTATUS check:

  $SCRIPT 2>&1 | sed -e "s/^/${SCRIPT##*/}: /"
  ret=${PIPESTATUS[0]}
  if [[ ${ret} -ne 0 ]] ; then
    logger -i -p cron.info -t run-crons "(`whoami`) EXIT ${ret} ($SCRIPT)"
    echo "run-crons: $SCRIPT exited with ${ret}"
  fi
Comment 4 SpanKY gentoo-dev 2015-07-22 08:19:08 UTC
cronbase-0.3.5 now does a syslog for each failing script, so there's at least some visibility now
Comment 5 Thomas Deutschmann (RETIRED) gentoo-dev 2015-07-22 09:18:01 UTC
Created attachment 407374 [details, diff]
Use appropriate priority when logging through logger

I would suggest to *not* hard code syslog's severity level in the new log() function because a failure should probably use cron.err instead of cron.info which allows the user to use the correct log filter mechanism when looking for failures/problems.
Comment 7 SpanKY gentoo-dev 2015-07-23 03:04:15 UTC
*** Bug 553294 has been marked as a duplicate of this bug. ***
Comment 8 SpanKY gentoo-dev 2015-07-23 03:05:14 UTC
[taken from bug 553294]

it would be nice to just do:

  $SCRIPT 2>&1 | sed "1i${SCRIPT}:"

if $SCRIPT produces no output, then sed won't do anything, but as soon as it produces at least one line, sed will add a header like:
  /etc/cron.daily/foo:

unfortunately though, this now conflicts with bug 491520 where we track the exit status and bug 530416 where we use POSIX shell (so we can't check PIPESTATUS).
Comment 9 Amadeusz Żołnowski (RETIRED) gentoo-dev 2016-02-17 21:53:47 UTC
Can't we provide a useflag depending on bash by which script would be patched?
Comment 10 Amadeusz Żołnowski (RETIRED) gentoo-dev 2016-02-17 21:56:30 UTC
There's also a POSIX solution for the problem: http://stackoverflow.com/a/8925688 although it's ugly.