give HostStatus analyzer more info: details on ping success/failure and ping duration

preparing HostStatus to decide other ping timeouts/frequencies/numbers and DNS timeout

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 10b4f12..2b6b7e3 100644 (file)
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -69,6 +69,7 @@ set(SOURCES
     dns/dnsresolver.cpp
     dns/dnsmaster.cpp
     host/hoststatus.cpp
+    host/pingstatus.cpp
     host/loglevel.cpp
     host/logoutput.cpp
     host/messagepayload.cpp

diff --git a/src/host/hoststatus.cpp b/src/host/hoststatus.cpp
index d620e58..b5d5bdb 100644 (file)
--- a/src/host/hoststatus.cpp
+++ b/src/host/hoststatus.cpp
@@ -86,14 +86,20 @@ bool HostStatus::exceeded_ping_failed_limit() const
 }
 
 /**
- * Adds a ping status (success or failure).
- * @param ping_success
+ * Tells the status analyzer how the last ping went
+ *
+ * @param result: status of ping specifying success/failure and reason of fail
+ * @param ping_duration_us duration of ping in micro seconds
  */
-void HostStatus::update_ping_statistics( bool ping_success )
+void HostStatus::update_ping_statistics( const PingStatus &result,
+                                         const long ping_duration_us )
 {
+    float ping_duration_ms = static_cast<float>(ping_duration_us) / 1000.;
+
     GlobalLogger.debug() << "Stat(" << HostAddress << "): "
         << PingsFailedCount << " fail/" << PingsPerformedCount << " pings/"
-        << ResolvedIpCount << " IPs: add ping with success=" << ping_success;
+        << ResolvedIpCount << " IPs: add ping with result "
+        << to_string(result) << " which took " << ping_duration_ms << " ms";
 
     BOOST_ASSERT( 1 <= ResolvedIpCount );
     BOOST_ASSERT( 0 <= PingsPerformedCount );
@@ -101,7 +107,7 @@ void HostStatus::update_ping_statistics( bool ping_success )
 
     increase_ping_performed_count();
 
-    if ( !ping_success )
+    if ( result != PingStatus_SuccessReply )
     {
         increase_ping_failed_count();
     }

diff --git a/src/host/hoststatus.h b/src/host/hoststatus.h
index c5f558d..a167afe 100644 (file)
--- a/src/host/hoststatus.h
+++ b/src/host/hoststatus.h
@@ -22,6 +22,7 @@ on this file might be covered by the GNU General Public License.
 
 #include <string>
 
+#include "host/pingstatus.h"
 #include "link/linkstatus.h"
 
 //-----------------------------------------------------------------------------
@@ -45,7 +46,8 @@ public:
 
     void set_resolved_ip_count( const int resolved_ip_count );
     bool exceeded_ping_failed_limit() const;
-    void update_ping_statistics( bool ping_success );
+    void update_ping_statistics( const PingStatus &ping_success,
+                                 const long ping_duration_us );
 
 private:
     bool tried_all_resolved_ip() const;

diff --git a/src/host/pinger.h b/src/host/pinger.h
index 47d3e21..fa3a28e 100644 (file)
--- a/src/host/pinger.h
+++ b/src/host/pinger.h
@@ -32,6 +32,8 @@
 #include <boost/asio/ip/address.hpp>
 #include <boost/weak_ptr.hpp>
 
+#include "host/pingstatus.h"
+
 //-----------------------------------------------------------------------------
 // IoServiceItem
 //-----------------------------------------------------------------------------
@@ -57,7 +59,7 @@ public:
     virtual void ping(
             const boost::asio::ip::address &ip,
             const uint16_t destination_port,
-            boost::function<void(bool)> ping_done_callback
+            boost::function<void(PingStatus)> ping_done_callback
     ) = 0;
 
     virtual void stop_pinging() = 0;

diff --git a/src/host/pingscheduler.cpp b/src/host/pingscheduler.cpp
index abeb02d..0bf1777 100644 (file)
--- a/src/host/pingscheduler.cpp
+++ b/src/host/pingscheduler.cpp
@@ -207,7 +207,7 @@ void PingScheduler::ping_when_ready()
         // skip the ping and directly call ping_done_handler
         HostAnalyzer.set_resolved_ip_count(1);   // must have been 0 --> failed
              // ping would create failed assumption (nPings > nIPs)
-        ping_done_handler(false);
+        ping_done_handler(PingStatus_FailureNoIP);
         HostAnalyzer.set_resolved_ip_count(0);   // set back
     }
     else
@@ -228,6 +228,7 @@ void PingScheduler::ping_when_ready()
         Ping->ping( ip.get_ip(),
                     DestinationPort,
                     boost::bind(&PingScheduler::ping_done_handler, this, _1) );
+        TimeSentLastPing = microsec_clock::universal_time();
     }
 }
 
@@ -241,25 +242,30 @@ void PingScheduler::ping_when_ready()
  *   statistics, ping interval and elapsed time;
  *   schedules a call to ping, thereby closing the loop
  */
-void PingScheduler::ping_done_handler( const bool ping_success )
+void PingScheduler::ping_done_handler( const PingStatus &result )
 {
-    GlobalLogger.info() << LogPrefix << "Ping done with success = "
-                                     << ping_success;
+    PingStatus edited_result = result;
+    if (result == PingStatus_SuccessReply && ContinueOnOutdatedIps)
+    {
+        edited_result = PingStatus_SuccessOutdatedIP;
+
+        // reset ContinueOnOutdatedIps
+        ContinueOnOutdatedIps = false;
+        update_log_prefix();
+    }
+
+    GlobalLogger.info() << LogPrefix << "Ping done with result "
+                                     << to_string(edited_result);
 
     // post-processing
     // You must call these 3 methods exactly in this order
     // TODO Fix this method, once it has a semantic dependency with the
     // update_ping_statistics method, because it depends on the PingAnalyzer
     // statistics to update the exceeded_ping_failed_limit
-    HostAnalyzer.update_ping_statistics( ping_success );
+    ptime now = microsec_clock::universal_time();
+    HostAnalyzer.update_ping_statistics( edited_result,
+                                 (now - TimeSentLastPing).total_microseconds());
     update_ping_interval();
-    update_ping_elapsed_time();
-
-    if (ping_success)
-    {   // reset ContinueOnOutdatedIps
-        ContinueOnOutdatedIps = false;
-        update_log_prefix();
-    }
 
     // get next protocol, possibly start resolving IPs
     update_ping_protocol();
@@ -289,18 +295,6 @@ void PingScheduler::update_ping_interval()
     }
 }
 
-void PingScheduler::update_ping_elapsed_time()
-{
-    ptime now = microsec_clock::universal_time();
-    time_resolution_traits_adapted64_impl::int_type elapsed_time_in_sec =
-            (now - TimeSentLastPing).total_seconds();
-    GlobalLogger.debug() << LogPrefix << "- Time elapsed since last ping: "
-                                      << elapsed_time_in_sec << "s";
-
-    TimeSentLastPing = microsec_clock::universal_time();
-}
-
-
 //------------------------------------------------------------------------------
 // Ping Protocol Rotation
 //------------------------------------------------------------------------------

diff --git a/src/host/pingscheduler.h b/src/host/pingscheduler.h
index 106f69c..37d94a1 100644 (file)
--- a/src/host/pingscheduler.h
+++ b/src/host/pingscheduler.h
@@ -30,6 +30,7 @@ on this file might be covered by the GNU General Public License.
 
 #include "link/linkstatus.h"
 #include "host/hoststatus.h"
+#include "host/pingstatus.h"
 #include "host/pinger.h"
 #include "host/pinginterval.h"
 #include "host/pingprotocol.h"
@@ -70,9 +71,8 @@ private:
     //
 
     void ping(const boost::system::error_code &error);
-    void ping_done_handler(const bool ping_success);
+    void ping_done_handler(const PingStatus &result);
     void update_ping_interval();
-    void update_ping_elapsed_time();
 
     void init_ping_protocol();
     void update_ping_protocol();

diff --git a/src/host/pingstatus.cpp b/src/host/pingstatus.cpp
new file mode 100644 (file)
index 0000000..1b255aa
--- /dev/null
+++ b/src/host/pingstatus.cpp
@@ -0,0 +1,46 @@
+/*
+ The software in this package is distributed under the GNU General
+ Public License version 2 (with a special exception described below).
+
+ A copy of GNU General Public License (GPL) is included in this distribution,
+ in the file COPYING.GPL.
+
+ As a special exception, if other files instantiate templates or use macros
+ or inline functions from this file, or you compile this file and link it
+ with other works to produce a work based on this file, this file
+ does not by itself cause the resulting work to be covered
+ by the GNU General Public License.
+
+ However the source code for this file must still be made available
+ in accordance with section (3) of the GNU General Public License.
+
+ This exception does not invalidate any other reasons why a work based
+ on this file might be covered by the GNU General Public License.
+
+ Christian Herdtweck, Intra2net AG 2015
+ */
+
+#include "host/pingstatus.h"
+
+std::string to_string( const PingStatus &status )
+{
+    switch (status)
+    {
+        case PingStatus_NotSent: return "PingNotSent"; break;
+        case PingStatus_SuccessReply: return "PingSucceeded"; break;
+        case PingStatus_FailureOther: return "PingFailed"; break;
+        case PingStatus_FailureTimeout: return "PingTimedOut"; break;
+        case PingStatus_FailureDestinationUnreachable:
+                                         return "PingFailed(DestUnreachable)";
+                                         break;
+        case PingStatus_FailureNoIP: return "PingFailed(NoIP)"; break;
+        case PingStatus_SuccessOutdatedIP: return "PingSucceeded(OutdatedIP)";
+                                           break;
+        case PingStatus_FailureAsyncCancel: return "PingFailed(AsyncCancel)";
+                                            break;
+        case PingStatus_FailureAsyncError: return "PingFailed(AsyncError)";
+                                           break;
+        default: return "PingStatusUnknown"; break;
+    }
+}
+

diff --git a/src/host/pingstatus.h b/src/host/pingstatus.h
index 30f412a..f62dc88 100644 (file)
--- a/src/host/pingstatus.h
+++ b/src/host/pingstatus.h
@@ -21,12 +21,21 @@ on this file might be covered by the GNU General Public License.
 #ifndef PING_STATUS_H
 #define PING_STATUS_H
 
+#include <string>
+
 enum PingStatus
 {
     PingStatus_NotSent,
     PingStatus_SuccessReply,
+    PingStatus_FailureOther,
     PingStatus_FailureTimeout,
-    PingStatus_FailureDestinationUnreachable
+    PingStatus_FailureDestinationUnreachable,
+    PingStatus_FailureNoIP,
+    PingStatus_SuccessOutdatedIP,
+    PingStatus_FailureAsyncCancel,
+    PingStatus_FailureAsyncError
 };
 
+std::string to_string( const PingStatus &status );
+
 #endif // PING_STATUS_H

diff --git a/src/icmp/icmppinger.cpp b/src/icmp/icmppinger.cpp
index 93a071d..12832d6 100644 (file)
--- a/src/icmp/icmppinger.cpp
+++ b/src/icmp/icmppinger.cpp
@@ -127,7 +127,7 @@ IcmpPinger::~IcmpPinger()
 void IcmpPinger::ping(
         const address &destination_ip,
         const uint16_t /*destination_port*/, // the ICMP protocol does not use ports
-        function<void(bool)> ping_done_callback
+        function<void(PingStatus)> ping_done_callback
 )
 {
     PingDoneCallback = ping_done_callback;
@@ -238,7 +238,7 @@ void IcmpPinger::schedule_timeout_echo_reply()
 /**
  * @brief Gets called when the ping is finished: Either on timeout or on ping reply
  *
- * @return void
+ * @return void (but calls PingDoneCallback)
  **/
 void IcmpPinger::handle_timeout(const boost::system::error_code& error)
 {
@@ -247,32 +247,34 @@ void IcmpPinger::handle_timeout(const boost::system::error_code& error)
         if ( error ==  boost::asio::error::operation_aborted )
         {
             if (! ReplyReceived)
+            {
                 GlobalLogger.notice() << LogPrefix
                     << "Timer waiting for ICMP echo reply was cancelled!"
                     << endl;
+                set_ping_status( PingStatus_FailureAsyncCancel );
+            }
             // otherwise probably called by IcmpPacketReceiveTimer.cancel in
             // handle_receive_icmp_packet!
         }
         else
+        {
             GlobalLogger.notice() << LogPrefix << "Error " << error
                 << " waiting for ICMP echo reply!" << endl;
+            set_ping_status( PingStatus_FailureAsyncError );
+        }
 
         // Still continue with rest of function, so PingStatus is updated and Callback executed
         //   when timer was cancelled
     }
-
-    // Check ReplyReceived as the timer handler
-    // is also called by Timer.cancel();
-    if ( !ReplyReceived )
-    {
+    else if ( !ReplyReceived )
+    {    // Check ReplyReceived since the timer handler is also called by Timer.cancel();
         GlobalLogger.info() << LogPrefix << "Request timed out" << endl;
 
         set_ping_status( PingStatus_FailureTimeout );
     }
 
     // Call ping-done handler
-    bool ping_success = (PingerStatus == PingStatus_SuccessReply);
-    PingDoneCallback( ping_success );
+    PingDoneCallback( PingerStatus );
 }
 
 

diff --git a/src/icmp/icmppinger.h b/src/icmp/icmppinger.h
index f6ce523..e9fb98a 100644 (file)
--- a/src/icmp/icmppinger.h
+++ b/src/icmp/icmppinger.h
@@ -127,7 +127,7 @@ public:
     virtual void ping(
             const boost::asio::ip::address &destination_ip,
             const uint16_t destination_port,
-            boost::function<void(bool)> ping_done_callback
+            boost::function<void(PingStatus)> ping_done_callback
     );
 
     virtual void stop_pinging();
@@ -176,7 +176,7 @@ private:
     /// The status of the pinger
     PingStatus PingerStatus;
     /// Callback to notify when the ping is done (got reply/timeout)
-    boost::function< void(bool) > PingDoneCallback;
+    boost::function< void(PingStatus) > PingDoneCallback;
     /// prefix to logging output lines
     std::string LogPrefix;
 };

diff --git a/src/tcp/tcppinger.cpp b/src/tcp/tcppinger.cpp
index 1cd857d..8bf6a7a 100644 (file)
--- a/src/tcp/tcppinger.cpp
+++ b/src/tcp/tcppinger.cpp
@@ -115,7 +115,7 @@ TcpPinger::~TcpPinger()
 void TcpPinger::ping(
         const address &destination_ip,
         const uint16_t destination_port,
-        function<void(bool)> ping_done_callback
+        function<void(PingStatus)> ping_done_callback
 )
 {
     BOOST_ASSERT( ( 0 < destination_port ) && ( destination_port < numeric_limits<uint16_t>::max() ) );
@@ -252,8 +252,7 @@ void TcpPinger::handle_ping_done()
     }
 
     // Call ping-done handler
-    bool ping_success = (PingerStatus == PingStatus_SuccessReply);
-    PingDoneCallback( ping_success );
+    PingDoneCallback( PingerStatus );
 }
 
 void TcpPinger::start_receive()

diff --git a/src/tcp/tcppinger.h b/src/tcp/tcppinger.h
index 19effee..05e9586 100644 (file)
--- a/src/tcp/tcppinger.h
+++ b/src/tcp/tcppinger.h
@@ -49,7 +49,7 @@ public:
     virtual void ping(
             const boost::asio::ip::address &destination_ip,
             const uint16_t destination_port,
-            boost::function<void(bool)> ping_done_callback
+            boost::function<void(PingStatus)> ping_done_callback
     );
 
     virtual void stop_pinging();
@@ -117,7 +117,7 @@ private:
     /// The status of the pinger
     PingStatus PingerStatus;
     /// Callback to notify when the ping is done (got reply/timeout)
-    boost::function< void(bool) > PingDoneCallback;
+    boost::function< void(PingStatus) > PingDoneCallback;
 };
 
 #endif // TCP_PINGER_H

diff --git a/test/CMakeLists.test_hoststatus.txt b/test/CMakeLists.test_hoststatus.txt
index 097a23e..bf1bc29 100644 (file)
--- a/test/CMakeLists.test_hoststatus.txt
+++ b/test/CMakeLists.test_hoststatus.txt
@@ -3,6 +3,7 @@ add_executable(test_hoststatus
     test_hoststatus.cpp
     ${CMAKE_SOURCE_DIR}/src/boost_assert_handler.cpp
     ${CMAKE_SOURCE_DIR}/src/host/hoststatus.cpp
+    ${CMAKE_SOURCE_DIR}/src/host/pingstatus.cpp
 )
 
 # linker: link the program against the libraries

diff --git a/test/test_hoststatus.cpp b/test/test_hoststatus.cpp
index a97f92c..752822f 100644 (file)
--- a/test/test_hoststatus.cpp
+++ b/test/test_hoststatus.cpp
@@ -29,6 +29,7 @@ on this file might be covered by the GNU General Public License.
 #include "mock_linkstatus.h"
 
 #include "host/hoststatus.h"
+#include "host/pingstatus.h"
 
 BOOST_AUTO_TEST_SUITE( TestHostStatus )
 
@@ -41,34 +42,34 @@ BOOST_AUTO_TEST_CASE( fail_percentage_10 )
     HostStatus host_status( "localhost", ping_fail_percentage_limit, link_status );
     host_status.set_resolved_ip_count( resolved_ip_count );
 
-    host_status.update_ping_statistics( true );
+    host_status.update_ping_statistics( PingStatus_SuccessReply, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false );
 
-    host_status.update_ping_statistics( false );
+    host_status.update_ping_statistics( PingStatus_FailureOther, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false );
 
-    host_status.update_ping_statistics( false );
+    host_status.update_ping_statistics( PingStatus_FailureOther, 1);
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true );
 
-    host_status.update_ping_statistics( true );
+    host_status.update_ping_statistics( PingStatus_SuccessReply, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true );
 
-    host_status.update_ping_statistics( true );
+    host_status.update_ping_statistics( PingStatus_SuccessReply, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true );
 
-    host_status.update_ping_statistics( true );
+    host_status.update_ping_statistics( PingStatus_SuccessReply, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true );
 
-    host_status.update_ping_statistics( true );
+    host_status.update_ping_statistics( PingStatus_SuccessReply, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true );
 
-    host_status.update_ping_statistics( true );
+    host_status.update_ping_statistics( PingStatus_SuccessReply, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true );
 
-    host_status.update_ping_statistics( true );
+    host_status.update_ping_statistics( PingStatus_SuccessReply, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true );
 
-    host_status.update_ping_statistics( true );
+    host_status.update_ping_statistics( PingStatus_SuccessReply, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true );
 }
 
@@ -81,34 +82,34 @@ BOOST_AUTO_TEST_CASE( fail_percentage_50 )
     HostStatus host_status( "localhost", ping_fail_percentage_limit, link_status );
     host_status.set_resolved_ip_count( resolved_ip_count );
 
-    host_status.update_ping_statistics( true );
+    host_status.update_ping_statistics( PingStatus_SuccessReply, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false );
 
-    host_status.update_ping_statistics( false );
+    host_status.update_ping_statistics( PingStatus_FailureOther, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false );
 
-    host_status.update_ping_statistics( false );
+    host_status.update_ping_statistics( PingStatus_FailureOther, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false );
 
-    host_status.update_ping_statistics( false );
+    host_status.update_ping_statistics( PingStatus_FailureOther, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false );
 
-    host_status.update_ping_statistics( false );
+    host_status.update_ping_statistics( PingStatus_FailureOther, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false );
 
-    host_status.update_ping_statistics( true );
+    host_status.update_ping_statistics( PingStatus_SuccessReply, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false );
 
-    host_status.update_ping_statistics( false );
+    host_status.update_ping_statistics( PingStatus_FailureOther, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false );
 
-    host_status.update_ping_statistics( true );
+    host_status.update_ping_statistics( PingStatus_SuccessReply, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false );
 
-    host_status.update_ping_statistics( false );
+    host_status.update_ping_statistics( PingStatus_FailureOther, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true );
 
-    host_status.update_ping_statistics( true );
+    host_status.update_ping_statistics( PingStatus_SuccessReply, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true );
 }
 
@@ -121,34 +122,34 @@ BOOST_AUTO_TEST_CASE( fail_percentage_80 )
     HostStatus host_status( "localhost", ping_fail_percentage_limit, link_status );
     host_status.set_resolved_ip_count( resolved_ip_count );
 
-    host_status.update_ping_statistics( false );
+    host_status.update_ping_statistics( PingStatus_FailureOther, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false );
 
-    host_status.update_ping_statistics( false );
+    host_status.update_ping_statistics( PingStatus_FailureOther, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false );
 
-    host_status.update_ping_statistics( false );
+    host_status.update_ping_statistics( PingStatus_FailureOther, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false );
 
-    host_status.update_ping_statistics( false );
+    host_status.update_ping_statistics( PingStatus_FailureOther, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false );
 
-    host_status.update_ping_statistics( false );
+    host_status.update_ping_statistics( PingStatus_FailureOther, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false );
 
-    host_status.update_ping_statistics( false );
+    host_status.update_ping_statistics( PingStatus_FailureOther, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false );
 
-    host_status.update_ping_statistics( false );
+    host_status.update_ping_statistics( PingStatus_FailureOther, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false );
 
-    host_status.update_ping_statistics( false );
+    host_status.update_ping_statistics( PingStatus_FailureOther, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), false );
 
-    host_status.update_ping_statistics( false );
+    host_status.update_ping_statistics( PingStatus_FailureOther, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true );
 
-    host_status.update_ping_statistics( true );
+    host_status.update_ping_statistics( PingStatus_SuccessReply, 1 );
     BOOST_CHECK_EQUAL( host_status.exceeded_ping_failed_limit(), true );
 }