808040 - no active alerting on error conditions from vcs2vcs system

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Hal Wine [:hwine] use NI!]

see bug 807702 for user notification. Found errors in logs, but no notification was given.

Comment 1

[Hal Wine [:hwine] use NI!]

Attachment: cron script to monitor job files for serious errors

New file - processes job log files in standard locations looking for failures of 'hg gexport' as run by do_update

First of the vcs2vcs files to be reviewed, so there may be a lot of context missing.

Comment 2

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 678138 [details]
cron script to monitor job files for serious errors

>USAGE="usage: ${##*/} [options] [log_file...]

Missing the 0 in ${0##*/}.

>    if test "$iso_time" = "${iso_time#20}"; then
>        # we have 'path/to/file:20' (assumes no ':' in path)
>        iso_time="20${iso_time#*:20}"
>    fi

egrep -h may avoid needing this, might need to pass it through sort depending on the ordering you want.

>while test $# -gt 0; do
>    case "$1" in
>    -h|--help) usage ;;
>    --email) addr+=("$2") ; shift ;;
>    --last) cutoff_time="$2" ; shift ;;
>    -*) usage "Unknown option '$1'" ;;
>    *) break ;;
>    esac

Some additional indentation would help readability in the case block.

># just egrep hack for now
>egrep -v ';0;[^0-9]' "${log_file_list[@]}" |
># only process failing gexport calls for now
>egrep ';hg .* gexport' |
># but only those as recent as specified
>toss_before $cutoff_time &>$tmp_file

Is this going to be easily extensible if you want to include other classes of errors ? Or need more context?

Comment 3

[Hal Wine [:hwine] use NI!]

All code/formatting changes made - following up on approach.

(In reply to Nick Thomas [:nthomas] from comment #2)
> Comment on attachment 678138 [details]

> 
> >    if test "$iso_time" = "${iso_time#20}"; then
> >        # we have 'path/to/file:20' (assumes no ':' in path)
> >        iso_time="20${iso_time#*:20}"
> >    fi
> 
> egrep -h may avoid needing this, might need to pass it through sort
> depending on the ordering you want.

I debated this - including the file name in the log output saves one lookup step in trouble shooting. But, there are "boot strap" issues on a new machine only runing one job for the first month (only 1 file will match, so file names are dropped by grep.

The current line ordering will help make obvious whether there error condition is persisting, or self correcting.

> ># just egrep hack for now
> >egrep -v ';0;[^0-9]' "${log_file_list[@]}" |
> ># only process failing gexport calls for now
> >egrep ';hg .* gexport' |
> ># but only those as recent as specified
> >toss_before $cutoff_time &>$tmp_file
> 
> Is this going to be easily extensible if you want to include other classes
> of errors ? Or need more context?

Actually, this is a copy/paste/modify from another utility script, so it is "extensible" ;) I can't project if the next show stopper we need to alert for will easily fit this pattern.

I decided to not over engineer at this point. We don't know all the error conditions yet, nor the way IT may require notifications and logging to be done. So this feels very temporary at the moment.

If it does need a longer life or much more complexity, then it needs to be rewritten in python.

Comment 4

[Hal Wine [:hwine] use NI!]

Attachment: cron script to monitor job files for serious errors

Incorporated :nthomas feedback

Comment 5

[Rail Aliiev [:rail]]

Comment on attachment 678385 [details]
cron script to monitor job files for serious errors

Sorry for the delayed review.

Thanks to Nick's feedback, it's much easier to review the logic skipping the (ba)shizm where I'm not the best reviewer. :)

In overall it looks good.