Add System Load collector #2231
Conversation
aeeef02 to
d606f34
Compare
| struct MetricDef { | ||
| const char* name; | ||
| double* value; | ||
| } metrics[] = { |
There was a problem hiding this comment.
CPU这样写是用constexpr + 数组定义一个schema。这里如果是存的具体的指标数据的话，直接用vector就行。下面有个18可以改成 metrics.size()
There was a problem hiding this comment.
CPU这样写是用constexpr + 数组定义一个schema。这里如果是存的具体的指标数据的话，直接用vector就行。下面有个18可以改成 metrics.size()
已修改
| Init(); | ||
| } | ||
| int SystemCollector::Init(int totalCount) { | ||
| mTotalCount = totalCount; |
There was a problem hiding this comment.
这个mTotalCount其实是指要发送的周期？这个名字表示的含义不太清楚
There was a problem hiding this comment.
这个mTotalCount其实是指要发送的周期？这个名字表示的含义不太清楚
是的，采集周期和发送周期不一定一致，mTotalCount=发送周期/采集周期，意思是采集多少轮以后发送一次。
There was a problem hiding this comment.
改成mCountPerReport或mCountPerStat是不是更好
There was a problem hiding this comment.
改成mCountPerReport或mCountPerStat是不是更好
已修改
| std::vector<std::string> loadMetric; | ||
| boost::split(loadMetric, loadLines[0], boost::is_any_of(" "), boost::token_compress_on); | ||
|
|
||
| systemload.load1 = std::atof(loadMetric[0].c_str()); |
There was a problem hiding this comment.
外部输入做好保护，下标访问合法性判断
已修改
ea2a55c to
07b2c1b
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new system load collector that reads /proc/loadavg to capture system load metrics and introduces corresponding unit tests. Key changes include:
- Adding a new collector (SystemCollector) and its implementation.
- Creating new unit tests for GetHostSystemStatWithPath and the collector’s logic.
- Updating CMakeLists.txt and related files to include the new collector.
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| core/unittest/host_monitor/SystemInformationToolsUnittest.cpp | Updated unit tests to use the new GetHostSystemStatWithPath function; legacy test code is commented out. |
| core/unittest/host_monitor/SystemCollectorUnittest.cpp | Added tests for system load statistic collection and metric event validation. |
| core/host_monitor/collector/SystemCollector.h / SystemCollector.cpp | Added the SystemCollector implementation for reading load data and calculating per-core metrics. |
| core/host_monitor/Constants.h / Constants.cpp | Introduced PROCESS_LOADAVG constant to support file path changes. |
| Other files (CMakeLists, InputHostMonitor.cpp, etc.) | Integrated the new collector into the existing test and plugin infrastructure. |
Comments suppressed due to low confidence (1)
core/host_monitor/collector/SystemCollector.cpp:112
- Verify that the argument passed to SetValue is correct; passing the metric event itself may not be intended and could lead to unexpected behavior.
metricEvent->SetValue<UntypedMultiDoubleValues>(metricEvent);
| @@ -26,23 +26,36 @@ namespace logtail { | |||
|
|
|||
| class SystemInformationToolsUnittest : public testing::Test { | |||
| public: | |||
| void TestGetHostSystemStat() const; | |||
| // void TestGetHostSystemStat() const; | |||
There was a problem hiding this comment.
[nitpick] If the legacy test is no longer necessary, consider removing the commented-out TestGetHostSystemStat code to reduce clutter.
| // void TestGetHostSystemStat() const; |
Copilot uses AI. Check for mistakes.
| } | ||
| }; | ||
|
|
||
| void SystemInformationToolsUnittest::TestGetHostSystemStat() const { | ||
| // void SystemInformationToolsUnittest::TestGetHostSystemStat() const { |
| systemload.load5 = std::stod(loadMetric[1]); | ||
| systemload.load15 = std::stod(loadMetric[2]); | ||
|
|
||
| auto cpuCoreCount = static_cast<double>(std::thread::hardware_concurrency()); |
There was a problem hiding this comment.
auto cpuCoreCount = static_cast<double>(std::thread::hardware_concurrency());
cpuCoreCount = cpuCoreCount < 1 ? 1.0 : cpuCoreCount;
这两行放到init里吧
087e975 to
fe2597c
Compare
| private: | ||
| int mTotalCount = 0; | ||
| int mCount = 0; | ||
| double cpuCoreCount = 1.0; |
|
|
||
| namespace logtail { | ||
|
9E81
|
||
| extern const uint32_t kMinInterval; |
There was a problem hiding this comment.
这个到底是哪个kMinInterval，代码里有两个，一个在InputHostMeta，一个在InputHostMonitor。
请用constexpr定义在头文件中，不要用隐式依赖。
原来的文件如已有问题，请帮忙一并改正。
下面量变量同理
There was a problem hiding this comment.
这个到底是哪个kMinInterval，代码里有两个，一个在InputHostMeta，一个在InputHostMonitor。 请用constexpr定义在头文件中，不要用隐式依赖。 原来的文件如已有问题，请帮忙一并改正。 下面量变量同理
已修改
|
|
||
| extern const uint32_t kHostMonitorMinInterval; | ||
| extern const uint32_t kHostMonitorDefaultInterval; | ||
| extern std::filesystem::path PROC_LOADAVG; |
There was a problem hiding this comment.
这个好像没用到
最开始用了，后面修改后忘记删除了。已删除。
c0bb150 to
b8a494d
Compare
新增采集系统负载的collector，通过读取/proc/loadavg的内容采集系统负载，和对应的ut
新增一个函数GetHostSystemStatWithPath，读取指定文件的内容，和对应ut