Bugfix and extension of http_loadtime.in #191
Conversation
steveschnepp
commented
Mar 1, 2014
steveschnepp
commented
- fixes http://munin-monitoring.org/ticket/1350
- introduces multi-url support (backwards compatible to single url)
- reintroduces using a tmp dir (necessary for requisites
- reintroduces a specific user agent
- allows auto config again
|
thx! :) |
|
Hi, what's the status of this PR ? I can see the label Here is the diff between branch diff --git a/plugins/node.d/http_loadtime b/plugins/node.d/http_loadtime
old mode 100755
new mode 100644
index 4f7e2c8..37d990b
--- a/plugins/node.d/http_loadtime
+++ b/plugins/node.d/http_loadtime
@@ -1,4 +1,5 @@
-#!/bin/bash
+#!@@BASH@@
+# -*- sh -*-
: << =cut
@@ -35,13 +36,14 @@ GPLv2
=cut
+MUNIN_LIBDIR=${MUNIN_LIBDIR:-"/usr/share/munin"}
. $MUNIN_LIBDIR/plugins/plugin.sh
target=${target:-"http://localhost/"}
requisites=${requisites:-"false"}
urls=`echo $target | tr "," "\n"`
-wget_opt="--user-agent munin/http_loadtime --no-cache -q"
+wget_opt="--user-agent \"Munin - http_loadtime\" --no-cache -q"
if [ "$requisites" == "true" ]; then
wget_opt="$wget_opt --page-requisites"
fi
@@ -54,24 +56,21 @@ if [ "$1" = "autoconf" ]; then
command -v tr 2>&1 >/dev/null || result=1
command -v wget 2>&1 >/dev/null || result=1
if [ "$result" != "yes" ]; then
- echo "no (programs time, wget and tr required)"
- exit 0
+ echo "no (programs time, wget and tr required)"
+ exit 0
fi
- # if $target contains more than one url
- if ! wget -q -O /dev/null $target; then
-
- # check if urls respond
- #
- for uri in $urls
- do
- wget --spider $uri $wget_opt
- if [ "$?" != "0" ]; then
- echo "no (Cannot run wget against \"$uri\")"
- exit 0
- fi
- done
- fi
+ # check if urls respond
+ #
+ for uri in $urls
+ do
+ wget --spider $uri $wget_opt
+ if [ "$?" != "0" ]; then
+ echo "no (Cannot run wget against \"$uri\")"
+ exit 0
+ fi
+ done
+
echo yes
exit 0
fi
@@ -101,14 +100,10 @@ cd $TEMPO_DIR || exit 1
for uri in $urls
do
- loadtime=`$time_bin -f "%e" wget $wget_opt --header='Accept-Encoding: gzip,deflate' $uri 2>&1`
- if [ "$?" == "0" ]; then
- esc_uri="$(clean_fieldname "$uri")"
- echo $esc_uri".value $loadtime"
- else
- echo "Download error during $uri" 1>&2
- exit 1
- fi
+ loadtime=`$time_bin --quiet -f "%e" wget $wget_opt $uri 2>&1`
+
+ esc_uri="$(clean_fieldname "$uri")"
+ echo $esc_uri".value $loadtime"
done
-exit 0
+cd -I think this PR can be closed without merge, it seems that relevant changes have already been applied. I am not sure though about theses:
|
|
the file was renamed (removed the |
|
oh, what happened? |
|
Steve made a mistake in the last IRC meeting
Full chat protocol: http://meetbot.debian.net/munin/2016/munin.2016-11-09-20.21.log.html |
|
;D |
| @@ -1,5 +1,4 @@ | |||
| #!@@BASH@@ | |||
| # -*- sh -*- | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
I don't like to use /usr/bin/envin the source from official plugins.
We just use /bin/bash or /bin/sh. This is much cleaner and let package maintainers handle the eventual change in PATH.
There was a problem hiding this comment.
it's less general, but I will change that to /bin/bash
| esc_uri="$(clean_fieldname "$uri")" | ||
| echo $esc_uri".label $uri_short" | ||
| echo $esc_uri".info page load time" | ||
| echo `escapeUri $uri`".label $uri_short" |
There was a problem hiding this comment.
I also quite like the $( ... ) syntax more. Specially if you use twice the same var.
5 similar comments
not used in similar files any more
Thanks, updated! :)
ax3l
force-pushed
the
loadtime_cherry
branch
from
bf59f72
to
c42fda8
Compare
|
@steveschnepp thank you for the review! all inline comments addressed and rebased on top of the latest |
