Fix netstats rollover (#181)

* Fix network counters for roll over

Change the way that network counters function
so that if any counters roll over
(current value < previous value) the
changes are ignored for that monitoring
cycle, with a warning printed.

* Fix counting bug for network tests

If the --blocks parameter was specified the
test itself failed to take this into account
(default of 1000 was always assumed)

* Fix unused local variable issue

Flake8: Local variable name is assigned
to but never used (F841)

* Remove unnecessary header

package/src/netmon.cpp

@@ -27,8 +27,10 @@ netmon::netmon(std::vector<std::string> netdevs)
   open_interface_streams();
 
   // Ensure internal stat counters are initialised properly
-  read_raw_network_stats(network_stats_start);
-  read_raw_network_stats(network_stats);
+  read_raw_network_stats(network_stats_last);
+  for (const auto& i : network_stats_last) {
+    network_net_counters[i.first] = 0;
+  }
 }
 
 // Get all available network devices
@@ -85,14 +87,26 @@ void netmon::read_raw_network_stats(
   }
 }
 
+// Update statistics
+void netmon::update_stats(const std::vector<pid_t>& pids) {
+  read_raw_network_stats(network_stats);
+  for (const auto& i : network_stats) {
+    if (i.second >= network_stats_last[i.first]) {
+      network_net_counters[i.first] += i.second - network_stats_last[i.first];
+    } else {
+      std::clog
+          << "prmon: network statistics error, counter values dropped for "
+          << i.first << " (" << i.second << " < " << network_stats_last[i.first]
+          << ")" << std::endl;
+    }
+    network_stats_last[i.first] = i.second;
+  }
+}
+
 // Relative counter statistics for text file
 std::map<std::string, unsigned long long> const netmon::get_text_stats() {
   std::map<std::string, unsigned long long> text_stats{};
-  for (const auto& if_param : interface_params) {
-    text_stats[if_param] =
-        network_stats[if_param] - network_stats_start[if_param];
-  }
-  return text_stats;
+  return network_net_counters;
 }
 
 // Also relative counters for JSON totals
@@ -103,9 +117,8 @@ std::map<std::string, unsigned long long> const netmon::get_json_total_stats() {
 // For JSON averages, divide by elapsed time
 std::map<std::string, double> const netmon::get_json_average_stats(
     unsigned long long elapsed_clock_ticks) {
-  std::map<std::string, unsigned long long> text_stats = get_text_stats();
   std::map<std::string, double> json_average_stats{};
-  for (auto& stat : text_stats) {
+  for (auto& stat : network_net_counters) {
     json_average_stats[stat.first] =
         double(stat.second * sysconf(_SC_CLK_TCK)) / elapsed_clock_ticks;
   }

package/src/netmon.h

@@ -42,7 +42,8 @@ class netmon final : public Imonitor {
       network_if_streams;
 
   // Container for stats, initial and current
-  std::map<std::string, unsigned long long> network_stats_start, network_stats;
+  std::map<std::string, unsigned long long> network_stats, network_stats_last,
+      network_net_counters;
 
   // Find all network interfaces on the system
   std::vector<std::string> const get_all_network_devs();
@@ -65,9 +66,7 @@ class netmon final : public Imonitor {
   netmon(std::vector<std::string> netdevs);
   netmon() : netmon(std::vector<std::string>{}){};
 
-  void update_stats(const std::vector<pid_t>& pids) {
-    read_raw_network_stats(network_stats);
-  }
+  void update_stats(const std::vector<pid_t>& pids);
 
   // These are the stat getter methods which retrieve current statistics
   std::map<std::string, unsigned long long> const get_text_stats();

package/tests/test_count.py

@@ -39,7 +39,7 @@ def test_run_test_with_params(self):
                 prmon_cmd.extend(["--pid", str(burn_p.pid)])
                 prmon_p = subprocess.Popen(prmon_cmd, shell=False)
 
-                _ = burn_p.wait()
+                burn_p.wait()
                 prmon_rc = prmon_p.wait()
 
             self.assertEqual(prmon_rc, 0, "Non-zero return code from prmon")

package/tests/test_cpu.py

@@ -52,7 +52,7 @@ def test_run_test_with_params(self):
                 prmon_cmd.extend(["--pid", str(burn_p.pid)])
                 prmon_p = subprocess.Popen(prmon_cmd, shell=False)
 
-                _ = burn_p.wait()
+                burn_p.wait()
                 prmon_rc = prmon_p.wait()
 
             self.assertEqual(prmon_rc, 0, "Non-zero return code from prmon")

package/tests/test_io.py

@@ -38,7 +38,7 @@ def test_run_test_with_params(self):
             ]
             prmon_p = subprocess.Popen(prmon_cmd, shell=False)
 
-            _ = burn_p.wait()
+            burn_p.wait()
             prmon_rc = prmon_p.wait()
 
             self.assertEqual(prmon_rc, 0, "Non-zero return code from prmon")

package/tests/test_net.py

@@ -32,6 +32,7 @@ def setUp(self):
         def tearDown(self):
             """Kill http server"""
             os.kill(self.http_server.pid, signal.SIGTERM)
+            self.http_server.wait()
 
         def test_run_test_with_params(self):
             """Test class for a specific set of parameters"""
@@ -54,7 +55,7 @@ def test_run_test_with_params(self):
             ]
             prmon_p = subprocess.Popen(prmon_cmd, shell=False)
 
-            _ = burn_p.wait()
+            burn_p.wait()
             prmon_rc = prmon_p.wait()
 
             self.assertEqual(prmon_rc, 0, "Non-zero return code from prmon")
@@ -63,7 +64,7 @@ def test_run_test_with_params(self):
                 prmon_json = json.load(infile)
 
                 # Network tests
-                expected_bytes = 1025000 * requests * slack
+                expected_bytes = (blocks if blocks else 1000) * 1024 * requests * slack
                 self.assertGreaterEqual(
                     prmon_json["Max"]["rx_bytes"],
                     expected_bytes,

package/tests/test_net2.py

@@ -31,6 +31,7 @@ def setUp(self):
         def tearDown(self):
             """Kill http server"""
             os.kill(self.http_server.pid, signal.SIGTERM)
+            self.http_server.wait()
 
         def test_run_test_with_params(self):
             """Test class for a specific set of parameters"""
@@ -53,7 +54,7 @@ def test_run_test_with_params(self):
             ]
             prmon_p = subprocess.Popen(prmon_cmd, shell=False)
 
-            _ = burn_p.wait()
+            burn_p.wait()
             prmon_rc = prmon_p.wait()
 
             self.assertEqual(prmon_rc, 0, "Non-zero return code from prmon")
@@ -62,7 +63,7 @@ def test_run_test_with_params(self):
                 prmon_json = json.load(infile)
 
                 # Network tests
-                expected_bytes = 1025000 * requests * slack
+                expected_bytes = (blocks if blocks else 1000) * 1024 * requests * slack
                 self.assertGreaterEqual(
                     prmon_json["Max"]["rx_bytes"],
                     expected_bytes,