Sign up ×
Unix & Linux Stack Exchange is a question and answer site for users of Linux, FreeBSD and other Un*x-like operating systems. It's 100% free, no registration required.

I have this bash script:

gunzip -c /var/log/cisco/cisco.log-$(date +%Y%m%d).gz | awk '/ath_bstuck_tasklet/ { print $4 }' | sort | uniq -c > /tmp/netgear_beacon.txt
echo "There are  `wc -l /tmp/netgear_beacon.txt | awk '{print $1}'` Stuck beacon; resetting" >> /tmp/netgear_beacon.txt

gunzip -c /var/log/cisco/cisco.log-`date +%Y%m%d`.gz | awk '/Virtual device ath0 asks to queue packet/ { print $4 }' | sort | uniq -c > /tmp/netgear_buffer_queue.txt
echo "There are  `wc -l /tmp/netgear_buffer_queue.txt | awk '{print $1}'`  routers with 'Virtual device ath0 asks to queue packet' errors" >> /tmp/netgear_buffer_queue.txt

gunzip -c /var/log/cisco/cisco.log-`date +%Y%m%d`.gz | awk '/CMS_MSG_DNSPROXY_RELOAD/ { print $4 }' | sort | uniq -c > /tmp/netgear_dns.txt
echo "There are  `wc -l /tmp/netgear_dns.txt | awk '{print $1}'`  routers with 'DNS Proxy Issue' errors" >> /tmp/netgear_dns.txt

gunzip -c /var/log/cisco/cisco.log-$(date +%Y%m%d).gz | awk '/beacon/ { print $4 }' | sort | uniq -c > /tmp/netgear_beacon_frame.txt
echo "There are  `wc -l /tmp/netgear_beacon_frame.txt | awk '{print $1}'` routers with beacon frame errors" >> /tmp/netgear_beacon_frame.txt

gunzip -c /var/log/cisco/cisco.log-$(date +%Y%m%d).gz | awk '/ACK/ { print $4 }' | sort | uniq -c | awk -v x=50 '$1 >= x' > /tmp/netgear_ACK.txt
echo "There are  `wc -l /tmp/netgear_ACK.txt | awk '{print $1}'` routers with more than 50 ACK" >> /tmp/netgear_ACK.txt

I would try to not repeat the gunzip command every time. I would run it just once and use it for all steps. I was thinking a variable, but is it the best practice?

share|improve this question
2  
(1) Why do you use `…` sometimes and $(…) others?  You know they're equivalent, right?  (2) If you run this script at 11:59PM, and it takes more than a minute to run, the date will change, and the last commands will get a different result for the $(date +%Y%m%d) than the first ones.  You might want to assign it to a variable at the beginning of the script. – G-Man yesterday

2 Answers 2

up vote 11 down vote accepted

There are no "best practices". Only things that make sense and make things easier.

Extracting the common parts and parameterizing the rest is one such thing:

lines="`gunzip -c /var/log/cisco/cisco.log-$(date +%Y%m%d).gz`"
#gunzip would always output the same thing on the same day, so 
#just run it once and store the results in a variable
grepAndLog(){
  local regex="$1" file="$2" msg="$3" filter="${4:-cat}"
  #^names for positional parameters

  printf "%s\n" "$lines" | grep "$regex" | cut -d' ' -f4 | sort | uniq -c | eval "$filter"  > "/tmp/$file"
  local count=`wc -l < "/tmp/$file"`   
  echo "There are $count "" $msg" >> "/tmp/$file"
}
grepAndLog ath_bstuck_tasklet netgear_bacon.txt \
 'Stuck beacon; resetting'
grepAndLog netgear_buffer_queue netgear_buffer_queue.txt \
 "routers with 'Virtual device ath0 asks to queue packet' errors"
grepAndLog CMS_MSG_DNSPROXY_RELOAD netgear_dns.txt \
 " routers with 'DNS Proxy Issue' errors"
grepAndLog ath_bstuck_tasklet netgear_bacon.txt \
 " routers with beacon frame errors"
grepAndLog ACK netgear_ACK.txt \
 " routers with more than 50 ACK" 'awk -v x=50 "\$1 >= x"'

It's still a mainly-shell solution. But IMO more readable and over 40% shorter.

About the code:

I'm using grep "$regex" | cut -d' ' -f4 instead of the awk expression. Other than that the grepAndLog function is a generalization of what you do in each line of your script: You have some input (the output of gunzip), you grep that for an expression (the $regex parameter), and output the resulting lines, sorted and prefixed with count into a $file. Then you append the line count (I do wc -l < "$file" instead of wc -l "$file" | awk ...) wrapped in a message whose beginning is constant and whose end varies ($msg).

In your last line you don't simply grep, but you use another filter on top of that. Instead of creating an if branch for that in the function, I simply use cat as an implicit default additional filter in the normal cases where no fourth parameter exists (local filter="${4:-cat}" means create a function-local variable filter whose contents is the fourth parameter given to the function, or cat if no fourth parameter is provided). cat gets overriden if a fourth parameter is given to grepAndLog.

share|improve this answer
1  
Upvoted, not just for the solution but for the first two sentences. – Jenny D yesterday
    
Hi, Thanks for that. Just one question. Whit this solution the "gunzip" process is it going to be repeated every time? – Federi yesterday
1  
@Federi Yeah, that didn't make much sense. Fixed it. – PSkocik yesterday
    
@PSkocik Thank you! Would be veeery nice if you spend a minutes to explain me what it means :D I guess that grepAndLog is a kind of variable repeated for what I need but I'm struggling to understand the remaining script. Thanks in advanced – Federi yesterday
    
@Federi An attempt at an explanation added. – PSkocik yesterday

The best thing to do here would be to perform all the processing in a single awk. Something similar to this:

gunzip -c /var/log/cisco/cisco.log-$(date +%Y%m%d).gz | awk '
/ath_bstuck_tasklet/ { netgear_beakon[$4] = 1 }
/Virtual device ath0 asks to queue packet/ { netgear_buffer_queue[$4] = 1 }
...
/ACK/ { netgear_ACK[$4] ++ }
END {
  n=0; for(k in netgear_beakon) n++; print n,"Stuck beacon; resetting";
  n=0; for(k in netgear_buffer_queue) n++; print n,"routers with Virtual device ath0 asks to queue packet";
  ...
  n=0; for(k in netgear_ACK) n+=(netgear_ACK[k]>=50); print n,"routers with more than 50 ACK"
}'

In addition to eliminating reading the file more than once, this also eliminates the need to execute sort and uniq multiple times. This stores (or counts) each unique item in an array and then computes the number of items by iterating over the keys of each array.

share|improve this answer
    
It sounds interesting. If you have time, Can you please give me a brief explanation about this script? I'm not confident yet with the language Thanks! – Federi yesterday
3  
@Federi - with the missing bits added (redirect to distinct files etc) this solution will be miles ahead the other in terms of speed & efficiency. – don_crissti yesterday
    
proposed edit: 'split' doesn't do what is wanted here, and the last case needs to be handled differently by counting and testing the counts – dave_thompson_085 yesterday

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.