Merge pull request #899 from mickem/debugging_core_issue

Debugging core issue t3

Author: mickem

docs/docs/reference/windows/CheckSystem.md

@@ -113,6 +113,7 @@ CPU Load ok|'total 5m'=16%;80;90 'total 1m'=13%;80;90 'total 5s'=13%;80;90
 <a name="check_cpu_show-default"/>
 <a name="check_cpu_help-short"/>
 <a name="check_cpu_time"/>
+<a name="check_cpu_cores"/>
 <a name="check_cpu_options"/>
 #### Command-line Arguments
 
@@ -140,6 +141,7 @@ CPU Load ok|'total 5m'=16%;80;90 'total 1m'=13%;80;90 'total 5s'=13%;80;90
 | [detail-syntax](#check_cpu_detail-syntax) | ${time}: ${load}%          | Detail level syntax.                                                                                             |
 | [perf-syntax](#check_cpu_perf-syntax)     | ${core} ${time}            | Performance alias syntax.                                                                                        |
 | time                                      |                            | The time to check                                                                                                |
+| cores                                     | N/A                        | This will remove the filter to  include the cores, if you use filter dont use this as well.                      |
 
 
 
@@ -2273,6 +2275,7 @@ Section for system checks and system settings
 |-----------------------------------------------|---------------|--------------------------|
 | [default buffer length](#default-buffer-time) | 1h            | Default buffer time      |
 | [disable](#disable-automatic-checks)          |               | Disable automatic checks |
+| [fetch core loads](#fetch-core-load)          | true          | Fetch core load          |
 | [subsystem](#pdh-subsystem)                   | default       | PDH subsystem            |
 
 
@@ -2281,6 +2284,7 @@ Section for system checks and system settings
 # Section for system checks and system settings
 [/settings/system/windows]
 default buffer length=1h
+fetch core loads=true
 subsystem=default
 
 ```
@@ -2343,6 +2347,33 @@ disable=
 
 
 
+#### Fetch core load <a id="/settings/system/windows/fetch core loads"></a>
+
+Set to false to use a different API for fetching CPU load (will not provide core load, and will not show exact same values as task manager).
+
+
+
+
+
+| Key            | Description                                           |
+|----------------|-------------------------------------------------------|
+| Path:          | [/settings/system/windows](#/settings/system/windows) |
+| Key:           | fetch core loads                                      |
+| Advanced:      | Yes (means it is not commonly used)                   |
+| Default value: | `true`                                                |
+| Used by:       | CheckSystem                                           |
+
+
+**Sample:**
+
+```
+[/settings/system/windows]
+# Fetch core load
+fetch core loads=true
+```
+
+
+
 #### PDH subsystem <a id="/settings/system/windows/subsystem"></a>
 
 Set which pdh subsystem to use.

include/rrd_buffer.hpp

@@ -63,23 +63,23 @@ struct rrd_buffer {
     value_type ret;
     if (time < 0) return ret;
     if (time <= seconds.size()) {
-      for (list_type::const_iterator cit = seconds.end() - time; cit != seconds.end(); ++cit) {
+      for (typename list_type::const_iterator cit = seconds.end() - time; cit != seconds.end(); ++cit) {
         ret.add(*cit);
       }
       ret.normalize(time);
       return ret;
     }
     time /= 60;
     if (time <= minutes.size()) {
-      for (list_type::const_iterator cit = minutes.end() - time; cit != minutes.end(); ++cit) {
+      for (typename list_type::const_iterator cit = minutes.end() - time; cit != minutes.end(); ++cit) {
         ret.add(*cit);
       }
       ret.normalize(time);
       return ret;
     }
     time /= 60;
     if (time >= hours.size()) throw nsclient::nsclient_exception("Size larger than buffer");
-    for (list_type::const_iterator cit = hours.end() - time; cit != hours.end(); ++cit) {
+    for (typename list_type::const_iterator cit = hours.end() - time; cit != hours.end(); ++cit) {
       ret.add(*cit);
     }
     ret.normalize(time);

include/win_sysinfo/win_sysinfo.cpp

@@ -17,7 +17,8 @@
  * along with NSClient++.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <win/windows.hpp>
+#define WIN32_LEAN_AND_MEAN  // Exclude rarely-used stuff from Windows headers
+#include <windows.h>
 
 #include <win_sysinfo/win_defines.hpp>
 #include <win_sysinfo/win_sysinfo.hpp>
@@ -225,7 +226,7 @@ bool g_hasVersion = false;
 bool g_hasBasicInfo = false;
 
 boost::scoped_array<unsigned long long> g_CPUIdleTimeOld;
-boost::scoped_array<unsigned long long> g_CPUTotalTimeOld;
+boost::scoped_array<unsigned long long> g_CPUUserTimeOld;
 boost::scoped_array<unsigned long long> g_CPUKernelTimeOld;
 
 void init_old_buffer(boost::scoped_array<unsigned long long> &array, const std::size_t size) {
@@ -341,16 +342,22 @@ hlp::buffer<BYTE, winapi::SYSTEM_PROCESS_INFORMATION *> system_info::get_system_
   throw nsclient::nsclient_exception("Failed to enumerate processes: unknown error");
 }
 
-system_info::cpu_load system_info::get_cpu_load() {
+double get_rate(unsigned long long part, unsigned long long total) {
+  auto part_d = static_cast<double>(part);
+  auto total_d = static_cast<double>(total);
+  return (part_d * 100.0) / total_d;
+}
+
+system_info::cpu_load system_info::get_cpu_load_per_core() {
   int cores = get_numberOfProcessorscores();
   init_old_buffer(g_CPUIdleTimeOld, cores);
-  init_old_buffer(g_CPUTotalTimeOld, cores);
+  init_old_buffer(g_CPUUserTimeOld, cores);
   init_old_buffer(g_CPUKernelTimeOld, cores);
 
   boost::scoped_array<winapi::SYSTEM_PROCESSOR_PERFORMANCE_INFORMATION> buffer(new winapi::SYSTEM_PROCESSOR_PERFORMANCE_INFORMATION[cores]);
   if (winapi::NtQuerySystemInformation(winapi::SystemProcessorPerformanceInformation, &buffer[0],
                                        sizeof(winapi::SYSTEM_PROCESSOR_PERFORMANCE_INFORMATION) * cores, NULL) != 0) {
-    throw nsclient::nsclient_exception("Whoops");
+    throw nsclient::nsclient_exception("Failed to fetch cpu load");
   }
 
   cpu_load result;
@@ -360,18 +367,20 @@ system_info::cpu_load system_info::get_cpu_load() {
 
   for (int i = 0; i < cores; i++) {
     unsigned long long CPUIdleTime = buffer[i].IdleTime.QuadPart;
-    unsigned long long CPUKernelTime = buffer[i].KernelTime.QuadPart - buffer[i].IdleTime.QuadPart;
-    unsigned long long CPUTotalTime = buffer[i].KernelTime.QuadPart + buffer[i].UserTime.QuadPart;
+    unsigned long long CPUKernelTime = buffer[i].KernelTime.QuadPart;
+    unsigned long long CPUUserTime = buffer[i].UserTime.QuadPart;
 
     unsigned long long CPUIdleTimeDiff = CPUIdleTime - g_CPUIdleTimeOld[i];
     unsigned long long CPUKernelTimeDiff = CPUKernelTime - g_CPUKernelTimeOld[i];
-    unsigned long long CPUTotalTimeDiff = CPUTotalTime - g_CPUTotalTimeOld[i];
+    unsigned long long CPUUserTimeDiff = CPUUserTime - g_CPUUserTimeOld[i];
 
-    if (CPUTotalTimeDiff != 0) {
+    unsigned long long kernel_time_diff = CPUKernelTimeDiff - CPUIdleTimeDiff;
+    unsigned long long total_time_diff = CPUKernelTimeDiff + CPUUserTimeDiff;
+    if (total_time_diff != 0) {
       result.core[i].core = i;
-      result.core[i].idle = static_cast<double>(((CPUIdleTimeDiff * 100) / CPUTotalTimeDiff));
-      result.core[i].kernel = static_cast<double>(((CPUKernelTimeDiff * 100) / CPUTotalTimeDiff));
-      result.core[i].total = 100.0 - result.core[i].idle;
+      result.core[i].idle = get_rate(CPUIdleTimeDiff, total_time_diff);
+      result.core[i].kernel = get_rate(kernel_time_diff, total_time_diff);
+      result.core[i].total = 100 - result.core[i].idle;
       result.total.idle += result.core[i].idle;
       result.total.kernel += result.core[i].kernel;
       result.total.total += result.core[i].total;
@@ -380,16 +389,56 @@ system_info::cpu_load system_info::get_cpu_load() {
       result.core[i].kernel = 0;
       result.core[i].total = 0;
     }
-    g_CPUTotalTimeOld[i] = CPUTotalTime;
     g_CPUIdleTimeOld[i] = CPUIdleTime;
     g_CPUKernelTimeOld[i] = CPUKernelTime;
+    g_CPUUserTimeOld[i] = CPUUserTime;
   }
   result.total.idle /= result.cores;
   result.total.kernel /= result.cores;
   result.total.total /= result.cores;
   return result;
 }
 
+system_info::cpu_load system_info::get_cpu_load_total() {
+  init_old_buffer(g_CPUIdleTimeOld, 1);
+  init_old_buffer(g_CPUUserTimeOld, 1);
+  init_old_buffer(g_CPUKernelTimeOld, 1);
+
+  FILETIME lpIdleTime;
+  FILETIME lpKernelTime;
+  FILETIME lpUserTime;
+
+  if (GetSystemTimes(&lpIdleTime, &lpKernelTime, &lpUserTime) == 0) {
+    throw nsclient::nsclient_exception("Failed to fetch cpu load");
+  }
+  unsigned long long CPUIdleTime = (static_cast<unsigned long long>(lpIdleTime.dwHighDateTime) << 32) | lpIdleTime.dwLowDateTime;
+  unsigned long long CPUKernelTime = (static_cast<unsigned long long>(lpKernelTime.dwHighDateTime) << 32) | lpKernelTime.dwLowDateTime;
+  unsigned long long CPUUserTime = (static_cast<unsigned long long>(lpUserTime.dwHighDateTime) << 32) | lpUserTime.dwLowDateTime;
+
+  cpu_load result;
+  result.cores = 0;
+  result.core.resize(0);
+  result.total.idle = result.total.kernel = result.total.total = 0.0;
+
+  unsigned long long CPUIdleTimeDiff = CPUIdleTime - g_CPUIdleTimeOld[0];
+  // Kernel also includes idle time so we need to subtract that
+  unsigned long long CPUKernelTimeDiff = CPUKernelTime - g_CPUKernelTimeOld[0];
+  unsigned long long CPUUserTimeDiff = CPUUserTime - g_CPUUserTimeOld[0];
+
+  unsigned long long kernel_time_diff = CPUKernelTimeDiff - CPUIdleTimeDiff;
+  unsigned long long used_time_diff = kernel_time_diff + CPUUserTimeDiff;
+  unsigned long long total_time_diff = CPUKernelTimeDiff + CPUUserTimeDiff;
+
+  result.total.idle = get_rate(CPUIdleTimeDiff, total_time_diff);
+  result.total.kernel = get_rate(kernel_time_diff, total_time_diff);
+  result.total.total = get_rate(used_time_diff, total_time_diff);
+
+  g_CPUIdleTimeOld[0] = CPUIdleTime;
+  g_CPUKernelTimeOld[0] = CPUKernelTime;
+  g_CPUUserTimeOld[0] = CPUUserTime;
+  return result;
+}
+
 class CheckMemory {
  public:
   CheckMemory() : hKernel32(NULL), FEGlobalMemoryStatusEx(NULL), FEGlobalMemoryStatus(NULL) {

include/win_sysinfo/win_sysinfo.hpp

@@ -54,7 +54,15 @@ struct system_info {
     double kernel;
     int core;
     load_entry() : idle(0.0), total(0.0), kernel(0.0), core(-1) {}
-    void add(const load_entry &other) {
+    load_entry(const load_entry& obj) : idle(obj.idle), total(obj.total), kernel(obj.kernel), core(obj.core) {}
+    load_entry& operator=(const load_entry& obj) {
+      idle = obj.idle;
+      total = obj.total;
+      kernel = obj.kernel;
+      core = obj.core;
+      return *this;
+    }
+    void add(const load_entry& other) {
       idle += other.idle;
       total += other.total;
       kernel += other.kernel;
@@ -71,7 +79,15 @@ struct system_info {
     std::vector<load_entry> core;
     load_entry total;
     cpu_load() : cores(0) {}
-    void add(const cpu_load &n) {
+    cpu_load(const cpu_load& obj) : cores(obj.cores), core(obj.core), total(obj.total) {}
+    cpu_load& operator=(const cpu_load& obj) {
+      cores = obj.cores;
+      core = obj.core;
+      total = obj.total;
+      return *this;
+    }
+
+    void add(const cpu_load& n) {
       total.add(n.total);
       cores = max(cores, n.cores);
       core.resize(cores);
@@ -81,7 +97,7 @@ struct system_info {
     }
     void normalize(double value) {
       total.normalize(value);
-      for (load_entry &c : core) {
+      for (load_entry& c : core) {
         c.normalize(value);
       }
     }
@@ -103,7 +119,7 @@ struct system_info {
     long long peak_usage;
     std::string name;
     pagefile_info(const std::string name = "") : size(0), usage(0), peak_usage(0), name(name) {}
-    void add(const pagefile_info &other) {
+    void add(const pagefile_info& other) {
       size += other.size;
       usage += other.usage;
       peak_usage += other.peak_usage;
@@ -114,14 +130,16 @@ struct system_info {
 
   static std::string get_version_string();
   static unsigned long get_version();
-  static OSVERSIONINFOEX *get_versioninfo();
+  static OSVERSIONINFOEX* get_versioninfo();
   static long get_numberOfProcessorscores();
   static std::vector<std::string> get_suite_list();
   static long long get_suite_i();
 
-  static cpu_load get_cpu_load();
+  static cpu_load get_cpu_load_per_core();
+  static cpu_load get_cpu_load_total();
+
   static memory_usage get_memory();
-  static hlp::buffer<BYTE, windows::winapi::SYSTEM_PROCESS_INFORMATION *> get_system_process_information(int size = 0x32000);
+  static hlp::buffer<BYTE, windows::winapi::SYSTEM_PROCESS_INFORMATION*> get_system_process_information(int size = 0x32000);
 };
 
 namespace winapi {

modules/CheckSystem/CheckSystem.cpp

@@ -177,7 +177,11 @@ bool CheckSystem::loadModuleEx(std::string alias, NSCAPI::moduleLoadMode mode) {
 		("subsystem", sh::string_key(&collector->subsystem, "default"),
 			"PDH subsystem", "Set which pdh subsystem to use.\nCurrently default and thread-safe are supported where thread-safe is slower but required if you have some problematic counters.", true)
 
-		("disable", sh::string_key(&collector->disable_, ""),
+    ("fetch core loads", sh::bool_key(&collector->read_core_load, "true"),
+     "Fetch core load", "Set to false to use a different API for fetching CPU load (will not provide core load, and will not show exact same values as task manager).", true)
+
+
+      ("disable", sh::string_key(&collector->disable_, ""),
 		"Disable automatic checks", "A comma separated list of checks to disable in the collector: cpu,handles,network,metrics,pdh. Please note disabling these will mean part of NSClient++ will no longer function as expected.", true)
 		;
 	;
@@ -559,17 +563,24 @@ void CheckSystem::check_cpu(const PB::Commands::QueryRequestMessage::Request &re
 	modern_filter::data_container data;
 	modern_filter::cli_helper<filter_type> filter_helper(request, response, data);
 	std::vector<std::string> times;
+  bool show_all_cores = false;
 
 	filter_type filter;
 	filter_helper.add_options("load > 80", "load > 90", "core = 'total'", filter.get_filter_syntax(), "ignored");
-	filter_helper.add_syntax("${status}: ${problem_list}", "${time}: ${load}%", "${core} ${time}", "", "%(status): CPU load is ok.");
+  filter_helper.add_syntax("${status}: ${problem_list}", "${time}: ${load}%", "${core} ${time}", "", "%(status): CPU load is ok.");
 	filter_helper.get_desc().add_options()
-		("time", po::value<std::vector<std::string>>(&times), "The time to check")
+      ("time", po::value<std::vector<std::string>>(&times), "The time to check")
+              ("cores", boost::program_options::bool_switch(&show_all_cores),
+               "This will remove the filter to  include the cores, if you use filter dont use this as well.")
 		;
 
 	if (!filter_helper.parse_options())
 		return;
 
+  if (show_all_cores && filter_helper.data.filter_string.size() == 1) {
+    filter_helper.data.filter_string.clear();
+  }
+
 	if (times.empty()) {
 		times.push_back("5m");
 		times.push_back("1m");

modules/CheckSystem/pdh_thread.cpp

@@ -282,7 +282,11 @@ void pdh_thread::thread_proc() {
 			windows::system_info::cpu_load load;
 			if (!disable_cpu) {
 				try {
-					load = windows::system_info::get_cpu_load();
+          if (read_core_load) {
+            load = windows::system_info::get_cpu_load_per_core();
+          } else {
+            load = windows::system_info::get_cpu_load_total();
+          }
 				} catch (...) {
 					errors.push_back("Failed to get cpu load");
 				}

modules/CheckSystem/pdh_thread.hpp

@@ -69,13 +69,14 @@ class pdh_thread {
 	network_check::network_data network;
 public:
 
+  bool read_core_load;
 	std::string subsystem;
 	std::string disable_;
 	std::string default_buffer_size;
 
 public:
 
-	pdh_thread(nscapi::core_wrapper *core, int plugin_id) : core(core), plugin_id(plugin_id) {
+	pdh_thread(nscapi::core_wrapper *core, int plugin_id) : core(core), plugin_id(plugin_id), read_core_load(true) {
 		mutex_.lock();
 	}
 	void add_counter(const PDH::pdh_object &counter);