proposal: add num cpu cores to process.report output

[boneskull]

In the report generated by process.report, we have resourceUsage.cpuConsumptionPercent.

As per the original PR, this is the percentage of CPU in use across all cpu cores. The issue I'm raising here didn't seem to be considered--rather the maintainer decided that granular information about each core was not useful (and they are right).

Now, it's been awhile since my computer architecture classes, but what I think this means is that you cannot just look at the number reported and know what it means. A usage of 100% on system A may be a problem, but a usage of 120% on system B might not be.

(We are not guaranteed to be consuming the report on the machine it was generated on, either.)

So, I propose adding header.osCpus, which would be a number corresponding to the output of os.cpus().length. A consumer can then use both of these to determine the theoretical ceiling of the CPU consumption percentage value (multiply header.osCpus by 100).

If it's a good idea, I could implement. If I can do it in lib/internal/process/report.js that'd be even better. 😝

[sam-github]

Seems pretty reasonable to me, @richardlau, thoughts?

[richardlau]

Yes, sounds reasonable. One question is whether it would be useful to include more information about the CPU's in the report, e.g. the information returned by uv_cpu_info() (i.e. the contents of the returned uv_cpu_info_t's)?

The report is written out in C++ code -- for the header section see: https://github.com/nodejs/node/blob/04633eeeb93c5a86a6635940eb4f34a7839ada11/src/node_report.cc#L150-L236

[boneskull]

@richardlau Isn't the cpu time information in the uv_cpu_info_t put into resourceUsage.userCpuSeconds & resourceUsage.kernelCpuSeconds? So it'd be essentially duplicate information?

At any rate, I don't have a use for anything past the number of cores, and could only speculate on what someone would want to do with the extra info here.

[richardlau]

resourceUsage is obtained from uv_getrusage(): https://github.com/nodejs/node/blob/155caf72bd7972aa46a7b4e8210d13150f8feb43/src/node_report.cc#L423-L452
It may well be that the information is duplicated in the two calls.

The only reason I'm speculating on whether it's worth including the extra information is in terms of the report format (i.e. we could go the simple route and have header.osCpus as the count but then if we wanted to expand the information that would potentially be a format change).

[mhdawson]

+1 on adding the number of cores to the report.

[mmarchini]

@boneskull nodejs/node#28188 landed, so the number of CPUs can be determined by the number of entries in osCpus. Can we close this issue?

[boneskull]

yep!