From e91c4f075824981010f8eed6ad64ec823a982b70 Mon Sep 17 00:00:00 2001 From: Kapil Bansal Date: Thu, 14 Oct 2021 03:19:42 +0530 Subject: [PATCH 1/4] [enhancement] Move logger calls in subroutine #57 Closes #57 --- openwisp-monitoring/files/monitoring.agent | 74 ++++++++++++---------- openwisp-monitoring/files/monitoring.init | 18 ++---- 2 files changed, 43 insertions(+), 49 deletions(-) diff --git a/openwisp-monitoring/files/monitoring.agent b/openwisp-monitoring/files/monitoring.agent index fed7d49..fedd3b7 100755 --- a/openwisp-monitoring/files/monitoring.agent +++ b/openwisp-monitoring/files/monitoring.agent @@ -34,6 +34,27 @@ show_version() { echoerr() { echo "$@" 1>&2 && exit 1; } +log() { + level="$1" + type="$2" + shift 2 + + if [ "$type" = "-v" ] && [ "$VERBOSE_MODE" -ne "1" ]; then + return 0 + fi + + case "$level" in + -i) level=daemon.info ;; + -w) level=daemon.warn ;; + -e) level=daemon.err ;; + -*) + echoerr "Invalid message level : $level" + ;; + esac + + logger -s "$@" -p "$level" -t openwisp-monitoring +} + check_available_memory() { while true; do total=$(ubus call system info | jsonfilter -e '@.memory.total') @@ -50,9 +71,7 @@ check_available_memory() { if [ -f "$file" ]; then rm "$file" else - [ "$VERBOSE_MODE" -eq "1" ] \ - && logger -s "Not enough memory available, skipping collect data." \ - -p daemon.warn + log -w -v "Not enough memory available, skipping collect data." return 1 fi fi @@ -61,14 +80,11 @@ check_available_memory() { collect_data() { n=0 - [ "$VERBOSE_MODE" -eq "1" ] && logger -s "Collecting NetJSON Monitoring data." \ - -p daemon.info + log -i -v "Collecting NetJSON Monitoring data." until [ "$n" -ge 5 ]; do /usr/sbin/netjson-monitoring --dump "$MONITORED_INTERFACES" && break - if [ "$n" -eq 5 ]; then - [ "$VERBOSE_MODE" -eq "1" ] && logger -s "Collecting data failed!" -p daemon.err - fi + [ "$n" -eq 5 ] && log -e -v "Collecting data failed!" n=$((n + 1)) sleep 5 done @@ -101,8 +117,7 @@ save_data() { echo "$data" >"$TMP_DIR/$filename" # compress data gzip "$TMP_DIR/$filename" - [ "$VERBOSE_MODE" -eq "1" ] && logger -s "Data saved temporarily." \ - -p daemon.info + log -i -v "Data saved temporarily." fi # get process id of the process sending data pid=$(pgrep -f "openwisp-monitoring.*--mode send") @@ -112,8 +127,7 @@ save_data() { } handle_sigusr1() { - [ "$VERBOSE_MODE" -eq "1" ] && logger -s "SIGUSR1 received! Sending data." \ - -p daemon.info + log -i -v "SIGUSR1 received! Sending data." return 0 } @@ -121,8 +135,7 @@ send_data() { while true; do for file in "$TMP_DIR"/*; do if [ ! -f "$file" ]; then - [ "$VERBOSE_MODE" -eq "1" ] && logger -s "No data file found to send." \ - -p daemon.info + log -i -v "No data file found to send. Checking after $INTERVAL seconds" trap handle_sigusr1 USR1 # SIGUSR1 signal received, interrupt sleep and continue sending data sleep "$INTERVAL" & @@ -149,32 +162,25 @@ send_data() { while true; do if [ "$failures" -eq "$MAX_RETRIES" ]; then [ -f "$RESPONSE_FILE" ] && error_message="\"$(cat "$RESPONSE_FILE")\"" || error_message='"".' - if [ "$VERBOSE_MODE" -eq "1" ]; then - logger -s "Data not sent successfully. Response code is \"$response_code\"." \ - "Error message is $error_message" \ - -p daemon.err - elif [ "$FAILING" -eq "0" ]; then + log -e -v "Data not sent successfully. Response code is \"$response_code\"." \ + "Error message is $error_message" + # check if agent was already passing or not to avoid repeating log messages + if [ "$FAILING" -eq "0" ]; then FAILING=1 - logger -s "Data not sent successfully. Response code is \"$response_code\"." \ - "Error message is $error_message" \ - "Run with verbose mode to find more." \ - -t openwisp-monitoring \ - -p daemon.err + [ "$VERBOSE_MODE" -ne "1" ] && log -e -n "Data not sent successfully. Response code is \"$response_code\"." \ + "Run with verbose mode to find more." fi break fi # send data response_code=$($CURL_COMMAND -H "Content-Type: application/json" -d "$data" "$url") if [ "$response_code" = "200" ]; then - if [ "$VERBOSE_MODE" -eq "1" ]; then - logger -s "Data sent successfully." \ - -p daemon.info - elif [ "$FAILING" -eq "1" ]; then - logger -s "Data sent successfully." \ - -t openwisp-monitoring \ - -p daemon.info + log -i -v "Data sent successfully." + # check if agent was already failing or not to avoid repeating log messages + if [ "$FAILING" -eq "1" ]; then FAILING=0 rm -f "$RESPONSE_FILE" + [ "$VERBOSE_MODE" -ne "1" ] && log -i -n "Data sent successfully." fi # remove saved data rm -f "$filename" @@ -187,8 +193,7 @@ send_data() { break else timeout=$(/usr/sbin/openwisp-get-random-number 2 15) - [ "$VERBOSE_MODE" -eq "1" ] && logger -s "Data not sent successfully. Retrying in $timeout seconds." \ - -p daemon.warn + log -w -v "Data not sent successfully. Retrying in $timeout seconds." failures=$((failures + 1)) sleep "$timeout" fi @@ -288,8 +293,7 @@ main() { shift ;; -*) - echo "Invalid option: $1" - exit 1 + echoerr "Invalid option: $1" ;; *) break ;; esac diff --git a/openwisp-monitoring/files/monitoring.init b/openwisp-monitoring/files/monitoring.init index 43580a4..7a2b941 100755 --- a/openwisp-monitoring/files/monitoring.init +++ b/openwisp-monitoring/files/monitoring.init @@ -56,12 +56,8 @@ start_service() { config_get bootup_delay monitoring bootup_delay "10" interval="$(time_to_seconds "$interval")" - if [ "$interval" -lt 1 ]; then - logger -s "Interval is invalid. Use time value(eg: '10', '2m', '3h', '1d')" \ - -t openwisp-monitoring \ - -p daemon.err - exit 1 - fi + [ -z "$interval" ] && { echo "Interval is invalid. Use time value(eg: '10', '2m', '3h', '1d')" 1>&2 && exit 1; } + interval="--interval $interval" verbose="--verbose_mode ${verbose_mode:-0}" required_memory="--required_memory $required_memory" @@ -72,25 +68,19 @@ start_service() { # shellcheck disable=SC2086,SC2154 procd_set_param command $PROG $interval $verbose $required_memory --mode collect --monitored_interfaces "$monitored_interfaces" procd_set_param respawn "${respawn_threshold:-3600}" "${respawn_timeout:-5}" "${respawn_retry:-5}" - [ "$verbose_mode" -eq "1" ] && procd_set_param stdout 1 && procd_set_param stderr 1 procd_close_instance procd_open_instance "openwisp-monitoring_send_data" # shellcheck disable=SC2086 procd_set_param command $PROG $base_url $uuid $key $verify_ssl $interval $verbose $max_retries $bootup_delay --mode send procd_set_param respawn "${respawn_threshold:-3600}" "${respawn_timeout:-5}" "${respawn_retry:-5}" - [ "$verbose_mode" -eq "1" ] && procd_set_param stdout 1 && procd_set_param stderr 1 procd_close_instance - logger -s "$PROG_NAME started" \ - -t openwisp-monitoring \ - -p daemon.info + logger -s "$PROG_NAME started" -t openwisp-monitoring -p daemon.info } stop_service() { - logger -s "$PROG_NAME stopping" \ - -t openwisp-monitoring \ - -p daemon.info + logger -s "$PROG_NAME stopping" -t openwisp-monitoring -p daemon.info } service_triggers() { From b47e2a552ec9677340ec86da0ab87a6192b5faa2 Mon Sep 17 00:00:00 2001 From: Kapil Bansal Date: Thu, 18 Nov 2021 03:26:59 +0530 Subject: [PATCH 2/4] [refactor] Refactor init script --- openwisp-monitoring/files/monitoring.agent | 16 ++++ openwisp-monitoring/files/monitoring.init | 93 +++++++++------------- 2 files changed, 55 insertions(+), 54 deletions(-) diff --git a/openwisp-monitoring/files/monitoring.agent b/openwisp-monitoring/files/monitoring.agent index fedd3b7..6a3d7a0 100755 --- a/openwisp-monitoring/files/monitoring.agent +++ b/openwisp-monitoring/files/monitoring.agent @@ -55,6 +55,20 @@ log() { logger -s "$@" -p "$level" -t openwisp-monitoring } +time_to_seconds() { + time=$1 + + { [ "$time" -ge 1 ] 2>/dev/null && seconds="$time"; } \ + || { [ "${time%s}" -ge 1 ] 2>/dev/null && seconds="${time%s}"; } \ + || { [ "${time%m}" -ge 1 ] 2>/dev/null && seconds=$((${time%m} * 60)); } \ + || { [ "${time%h}" -ge 1 ] 2>/dev/null && seconds=$((${time%h} * 3600)); } \ + || { [ "${time%d}" -ge 1 ] 2>/dev/null && seconds=$((${time%d} * 86400)); } + + echo $seconds + unset seconds + unset time +} + check_available_memory() { while true; do total=$(ubus call system info | jsonfilter -e '@.memory.total') @@ -302,6 +316,8 @@ main() { INTERVAL=${INTERVAL:-300} REGISTRATION_INTERVAL=$((INTERVAL / 10)) + INTERVAL="$(time_to_seconds "$INTERVAL")" + [ -z "$INTERVAL" ] && echoerr "Interval is invalid. Use time value(eg: '10', '2m', '3h', '1d')" VERBOSE_MODE=${VERBOSE_MODE:-0} BOOTUP_DELAY=${BOOTUP_DELAY:-10} TMP_DIR="/tmp/openwisp/monitoring" diff --git a/openwisp-monitoring/files/monitoring.init b/openwisp-monitoring/files/monitoring.init index 7a2b941..b5d20f3 100755 --- a/openwisp-monitoring/files/monitoring.init +++ b/openwisp-monitoring/files/monitoring.init @@ -4,75 +4,60 @@ # shellcheck disable=SC2034 START=99 -STOP=15 + USE_PROCD=1 PROG="/usr/sbin/openwisp-monitoring" PROG_NAME="OpenWISP monitoring daemon" -time_to_seconds() { - time=$1 - - { [ "$time" -ge 1 ] 2>/dev/null && seconds="$time"; } \ - || { [ "${time%s}" -ge 1 ] 2>/dev/null && seconds="${time%s}"; } \ - || { [ "${time%m}" -ge 1 ] 2>/dev/null && seconds=$((${time%m} * 60)); } \ - || { [ "${time%h}" -ge 1 ] 2>/dev/null && seconds=$((${time%h} * 3600)); } \ - || { [ "${time%d}" -ge 1 ] 2>/dev/null && seconds=$((${time%d} * 86400)); } +add_option() { + # shellcheck disable=SC3043 + { + local cfg="$1" + local flag="$2" + local option="$3" + local default="$4" + local value + } - echo $seconds - unset seconds - unset time + config_get value "$cfg" "$option" "$default" + [ -n "$value" ] && procd_append_param command "$flag" "$value" } start_service() { # for openwisp-config config_load openwisp - config_get base_url http url - config_get uuid http uuid - config_get key http key - config_get_bool verify_ssl http verify_ssl "1" - config_get respawn_threshold http respawn_threshold - config_get respawn_timeout http respawn_timeout - config_get respawn_retry http respawn_retry - - [ -n "$base_url" ] && base_url="--url $base_url" - [ -n "$uuid" ] && uuid="--uuid $uuid" - [ -n "$key" ] && key="--key $key" - [ -n "$verify_ssl" ] && verify_ssl="--verify_ssl $verify_ssl" - - if [ -z "$base_url" ]; then - logger -s "url is not set, please add it to /etc/config/openwisp" \ - -t openwisp-monitoring \ - -p daemon.err - exit 1 - fi - - # for openwisp-monitoring + + respawn_threshold=$(config_get http respawn_threshold) + respawn_timeout=$(config_get http respawn_timeout) + respawn_retry=$(config_get http respawn_retry) + + procd_open_instance "openwisp-monitoring_send_data" + procd_set_param command $PROG + + add_option "http" "--url" url + add_option "http" "--uuid" uuid + add_option "http" "--key" key + add_option "http" "--verify_ssl" verify_ssl "1" + config_load openwisp-monitoring - config_get monitored_interfaces monitoring monitored_interfaces "*" - config_get interval monitoring interval "300" - config_get_bool verbose_mode monitoring verbose_mode "0" - config_get required_memory monitoring required_memory "0.05" - config_get max_retries monitoring max_retries "5" - config_get bootup_delay monitoring bootup_delay "10" - - interval="$(time_to_seconds "$interval")" - [ -z "$interval" ] && { echo "Interval is invalid. Use time value(eg: '10', '2m', '3h', '1d')" 1>&2 && exit 1; } - - interval="--interval $interval" - verbose="--verbose_mode ${verbose_mode:-0}" - required_memory="--required_memory $required_memory" - max_retries="--max_retries $max_retries" - bootup_delay="--bootup_delay $bootup_delay" - procd_open_instance "openwisp-monitoring_collect_data" - # shellcheck disable=SC2086,SC2154 - procd_set_param command $PROG $interval $verbose $required_memory --mode collect --monitored_interfaces "$monitored_interfaces" + add_option "monitoring" "--verbose_mode" verbose_mode "0" + add_option "monitoring" "--max_retries" max_retries "5" + add_option "monitoring" "--interval" interval "300" + procd_append_param command "--mode" "send" + procd_set_param respawn "${respawn_threshold:-3600}" "${respawn_timeout:-5}" "${respawn_retry:-5}" procd_close_instance - procd_open_instance "openwisp-monitoring_send_data" - # shellcheck disable=SC2086 - procd_set_param command $PROG $base_url $uuid $key $verify_ssl $interval $verbose $max_retries $bootup_delay --mode send + procd_open_instance "openwisp-monitoring_collect_data" + procd_set_param command $PROG + + add_option "monitoring" "--monitored_interfaces" monitored_interfaces "*" + add_option "monitoring" "--required_memory" required_memory "0.05" + add_option "monitoring" "--verbose_mode" verbose_mode "0" + add_option "monitoring" "--interval" interval "300" + procd_append_param command "--mode" "collect" + procd_set_param respawn "${respawn_threshold:-3600}" "${respawn_timeout:-5}" "${respawn_retry:-5}" procd_close_instance From 88a27f620b1d38ccebdfb3b2e6825b3fa759dd96 Mon Sep 17 00:00:00 2001 From: Kapil Bansal Date: Fri, 13 Jan 2023 18:53:59 +0000 Subject: [PATCH 3/4] [chores] Improve log messages and use log util in all agent logs --- openwisp-monitoring/files/monitoring.agent | 24 ++++++++-------------- openwisp-monitoring/files/monitoring.init | 4 ++-- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/openwisp-monitoring/files/monitoring.agent b/openwisp-monitoring/files/monitoring.agent index 6a3d7a0..519c7c1 100755 --- a/openwisp-monitoring/files/monitoring.agent +++ b/openwisp-monitoring/files/monitoring.agent @@ -98,7 +98,7 @@ collect_data() { until [ "$n" -ge 5 ]; do /usr/sbin/netjson-monitoring --dump "$MONITORED_INTERFACES" && break - [ "$n" -eq 5 ] && log -e -v "Collecting data failed!" + [ "$n" -eq 5 ] && log -e -v "Collecting data failed!." n=$((n + 1)) sleep 5 done @@ -149,7 +149,7 @@ send_data() { while true; do for file in "$TMP_DIR"/*; do if [ ! -f "$file" ]; then - log -i -v "No data file found to send. Checking after $INTERVAL seconds" + log -i -v "No data file found to send. Checking after $INTERVAL seconds." trap handle_sigusr1 USR1 # SIGUSR1 signal received, interrupt sleep and continue sending data sleep "$INTERVAL" & @@ -200,9 +200,7 @@ send_data() { rm -f "$filename" break elif [ "$response_code" = "400" ]; then - logger -s "Data not sent successfully (HTTP status $response_code), discarding data." \ - -t openwisp-monitoring \ - -p daemon.err + log -i -n "Data not sent successfully (HTTP status $response_code), discarding data." rm -f "$filename" break else @@ -222,17 +220,13 @@ wait_until_registered() { if [ -n "$UUID" ] && [ -n "$KEY" ]; then return 0 fi - logger -s "Waiting for device to register." \ - -t openwisp-monitoring \ - -p daemon.info + log -i -n "Waiting for device to register." UUID=$(uci get openwisp.http.uuid 2>/dev/null) KEY=$(uci get openwisp.http.key 2>/dev/null) if [ -z "$UUID" ] || [ -z "$KEY" ]; then return 1 fi - logger -s "Setting uuid and key." \ - -t openwisp-monitoring \ - -p daemon.info + log -i -n "Setting uuid and key." export UUID KEY } @@ -242,9 +236,7 @@ bootup_delay() { if [ "$BOOTUP_DELAY" -ne "0" ]; then # get a random number between zero and $BOOTUP_DELAY DELAY=$(/usr/sbin/openwisp-get-random-number 0 "$BOOTUP_DELAY") - logger "Delaying initialization of the monitoring agent for $DELAY seconds." \ - -t openwisp-monitoring \ - -p daemon.info + log -i -n "Delaying initialization of the monitoring agent for $DELAY seconds." sleep "$DELAY" fi # send bootup hotplug event @@ -307,7 +299,7 @@ main() { shift ;; -*) - echoerr "Invalid option: $1" + echoerr "Invalid option: $1." ;; *) break ;; esac @@ -345,7 +337,7 @@ main() { RESPONSE_FILE="$TMP_DIR"/response.txt set_url_and_curl && send_data else - echoerr "The supplied mode is invalid. Only send and collect are allowed" + echoerr "The supplied mode is invalid. Only send and collect are allowed." fi } diff --git a/openwisp-monitoring/files/monitoring.init b/openwisp-monitoring/files/monitoring.init index b5d20f3..dc52898 100755 --- a/openwisp-monitoring/files/monitoring.init +++ b/openwisp-monitoring/files/monitoring.init @@ -61,11 +61,11 @@ start_service() { procd_set_param respawn "${respawn_threshold:-3600}" "${respawn_timeout:-5}" "${respawn_retry:-5}" procd_close_instance - logger -s "$PROG_NAME started" -t openwisp-monitoring -p daemon.info + logger -s "$PROG_NAME started." -t openwisp-monitoring -p daemon.info } stop_service() { - logger -s "$PROG_NAME stopping" -t openwisp-monitoring -p daemon.info + logger -s "$PROG_NAME stopping." -t openwisp-monitoring -p daemon.info } service_triggers() { From bb5a19fbdd374cbac845a6456560bc6dd222aefe Mon Sep 17 00:00:00 2001 From: Kapil Bansal Date: Sun, 29 Jan 2023 16:58:08 +0000 Subject: [PATCH 4/4] [refactor] Minor changes --- openwisp-monitoring/files/monitoring.agent | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/openwisp-monitoring/files/monitoring.agent b/openwisp-monitoring/files/monitoring.agent index 519c7c1..6f66e5a 100755 --- a/openwisp-monitoring/files/monitoring.agent +++ b/openwisp-monitoring/files/monitoring.agent @@ -85,7 +85,7 @@ check_available_memory() { if [ -f "$file" ]; then rm "$file" else - log -w -v "Not enough memory available, skipping collect data." + log -w -n "Not enough memory available, skipping collect data." return 1 fi fi @@ -98,7 +98,7 @@ collect_data() { until [ "$n" -ge 5 ]; do /usr/sbin/netjson-monitoring --dump "$MONITORED_INTERFACES" && break - [ "$n" -eq 5 ] && log -e -v "Collecting data failed!." + [ "$n" -eq 4 ] && log -e -v "Collecting data failed!" n=$((n + 1)) sleep 5 done @@ -149,7 +149,7 @@ send_data() { while true; do for file in "$TMP_DIR"/*; do if [ ! -f "$file" ]; then - log -i -v "No data file found to send. Checking after $INTERVAL seconds." + log -i -v "No data file found to send. Checking again in $INTERVAL seconds." trap handle_sigusr1 USR1 # SIGUSR1 signal received, interrupt sleep and continue sending data sleep "$INTERVAL" & @@ -307,9 +307,9 @@ main() { done INTERVAL=${INTERVAL:-300} - REGISTRATION_INTERVAL=$((INTERVAL / 10)) INTERVAL="$(time_to_seconds "$INTERVAL")" [ -z "$INTERVAL" ] && echoerr "Interval is invalid. Use time value(eg: '10', '2m', '3h', '1d')" + REGISTRATION_INTERVAL=$((INTERVAL / 10)) VERBOSE_MODE=${VERBOSE_MODE:-0} BOOTUP_DELAY=${BOOTUP_DELAY:-10} TMP_DIR="/tmp/openwisp/monitoring"