in_node_exporter_metrics: add Linux hwmon collector #10719
Conversation
edsiper
requested review from
fujimotos,
koleini and
leonardo-albertovich
as code owners
WalkthroughA hardware monitoring (hwmon) collector has been added to the Node Exporter Metrics plugin for Fluent Bit. This includes new source and header files implementing Linux-specific sensor collection, configuration options for filtering and scrape intervals, and updates to the plugin's build configuration and internal data structures to support hwmon metrics. Changes
Sequence Diagram(s)sequenceDiagram
participant InputPlugin as Node Exporter Metrics Plugin
participant HwmonCollector as Hwmon Collector
participant Sysfs as Linux Sysfs
InputPlugin->>HwmonCollector: Initialize (cb_init)
HwmonCollector->>HwmonCollector: Compile regex, create metrics
loop On scrape interval
InputPlugin->>HwmonCollector: Update (cb_update)
HwmonCollector->>Sysfs: Scan /sys/class/hwmon/hwmon*
loop For each chip
HwmonCollector->>HwmonCollector: Apply chip include/exclude regex
loop For each sensor in chip
HwmonCollector->>HwmonCollector: Apply sensor include/exclude regex
HwmonCollector->>Sysfs: Read sensor value
HwmonCollector->>HwmonCollector: Convert & set metric gauge
end
end
end
InputPlugin->>HwmonCollector: Exit (cb_exit)
HwmonCollector->>HwmonCollector: Destroy regex, cleanup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
plugins/in_node_exporter_metrics/ne_hwmon.h (1)
20-27: Header interface is minimal and correctGuard name is consistent and the extern declaration is sufficient for linking the collector. Consider a brief comment documenting the intent of the symbol for discoverability.
#include "ne.h" +/* Global collector instance for hwmon; defined in ne_hwmon.c (stub) or ne_hwmon_linux.c (Linux) */ extern struct flb_ne_collector hwmon_collector;plugins/in_node_exporter_metrics/ne_hwmon.c (1)
20-33: Optional: prefer build-time selection over including a .c fileIncluding ne_hwmon_linux.c from another .c works, but it increases compile time and can surprise tooling. A cleaner approach is:
- Compile ne_hwmon_linux.c only on Linux.
- Compile a small ne_hwmon_stub.c (your current else-branch) on non-Linux.
- Keep ne_hwmon.h as the shared declaration.
This avoids .c inclusion and will align with typical CMake conditional compilation patterns.
For example (CMake sketch):
set(src ne_cpu.c ... - ne_hwmon.c + ne_hwmon_stub.c ... ) +if(CMAKE_SYSTEM_NAME STREQUAL "Linux") + list(APPEND src ne_hwmon_linux.c) +endif()And update ne_hwmon.c -> ne_hwmon_stub.c to just define the stub collector.
plugins/in_node_exporter_metrics/ne.h (1)
215-229: Struct additions for hwmon are appropriate and completeGauge handles and regex fields cover temp/max/crit, voltage, fan, and power with include/exclude filters. Ensure these fields are initialized to NULL/0 during ctx creation so exit paths can safely destroy them.
plugins/in_node_exporter_metrics/ne_hwmon_linux.c (1)
194-201: Use full buffer size with snprintf for clarity and consistencyPassing sizeof(buf) is standard and avoids off-by-one headroom confusion. Current code is safe but inconsistent.
- snprintf(label_name, sizeof(label_name) - 1, "%s_label", sensor_name); + snprintf(label_name, sizeof(label_name), "%s_label", sensor_name); - snprintf(file_tmp, sizeof(file_tmp) - 1, "%s_max", sensor_name); + snprintf(file_tmp, sizeof(file_tmp), "%s_max", sensor_name); - snprintf(file_tmp, sizeof(file_tmp) - 1, "%s_crit", sensor_name); + snprintf(file_tmp, sizeof(file_tmp), "%s_crit", sensor_name);Also applies to: 208-214, 216-222
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
plugins/in_node_exporter_metrics/CMakeLists.txt(1 hunks)plugins/in_node_exporter_metrics/ne.c(4 hunks)plugins/in_node_exporter_metrics/ne.h(2 hunks)plugins/in_node_exporter_metrics/ne_hwmon.c(1 hunks)plugins/in_node_exporter_metrics/ne_hwmon.h(1 hunks)plugins/in_node_exporter_metrics/ne_hwmon_linux.c(1 hunks)plugins/in_node_exporter_metrics/ne_hwmon_linux.h(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: PR - fuzzing test
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-centos-7
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
🔇 Additional comments (11)
plugins/in_node_exporter_metrics/CMakeLists.txt (1)
16-16: hwmon collector integration verified – no duplicate ne_hwmon_linux.c compilation
ne_hwmon_linux.cis only pulled in via#includewithinne_hwmon.c(lines 20–22)- It is not added separately to CMakeLists.txt, so there’s no risk of duplicate symbols
plugins/in_node_exporter_metrics/ne.c (4)
47-47: Include is correct and minimalIncluding ne_hwmon.h is required for the global collector symbol; no issues.
425-429: Per-collector scrape interval wired correctlyZero defaults to global scrape_interval via get_interval_property, consistent with other collectors.
202-203: Collector registration and default metrics: ConfirmedThe default metrics list in plugins/in_node_exporter_metrics/ne.h includes “hwmon” only under
__linux__(line 36) and excludes it under__APPLE__(line 38), matching the intended platform-specific defaults. No further changes required.
507-530: Regex filter lifecycle verifiedConfirmed in
plugins/in_node_exporter_metrics/ne_hwmon_linux.cthat eachhwmon_*_regex_*string is compiled withflb_regex_create()(lines 109–142) and destroyed inne_hwmon_exit()viaflb_regex_destroy()(lines 325–336). Compile failures are logged withflb_plg_error()and cause the collector init to return-1.If you’d prefer invalid patterns to be skipped (with a warning) rather than failing init, adjust the error handling in the block around
flb_regex_create(). Otherwise, no memory‐leak or lifecycle issues remain.plugins/in_node_exporter_metrics/ne.h (1)
36-36: Default-enable hwmon on Linux looks goodAppending hwmon to NE_DEFAULT_ENABLED_METRICS under linux is appropriate and aligns with the new collector.
plugins/in_node_exporter_metrics/ne_hwmon_linux.c (5)
30-50: Filter helper implements include-then-exclude correctlySemantics are sensible: require include match when provided and deny on exclude match. LGTM.
52-107: Metric initialization and help text are solidMetrics follow node_hwmon_* naming and label schema ("chip", "sensor") with proper unit conversions planned downstream. Error handling on creation is consistent with the rest of the plugin.
323-338: Regex cleanup is correct; cmetrics objects don’t require explicit freeCompiled regexes are destroyed; cmetrics gauges are owned by ctx->cmt and will be released with the cmt context. LGTM.
340-345: Collector registration looks correct.name = "hwmon" and callbacks wired. Ensure ne.c adds this collector to the initialization list and that NE_DEFAULT_ENABLED_METRICS includes "hwmon" (it does).
284-301: Incorrect base-relative assumption; no patch neededThe
entry->strvalues fromne_utils_path_scanare always full (absolute) paths returned byglob(). Stripping offctx->path_sysfsvia pointer arithmetic yields the leading‐slash relative segment (e.g."/class/…") and appending/*_inputproduces the correct pattern when re‐prefixed withctx->path_sysfs. The existing logic handles both trailing-slash and non-trailing-slash cases correctly. No changes are required.Likely an incorrect or invalid review comment.
| ret = ne_utils_path_scan(ctx, ctx->path_sysfs, HWMON_PATTERN, | ||
| NE_SCAN_DIR, &hwmons); | ||
| if (ret != 0) { | ||
| return -1; | ||
| } | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider graceful behavior when hwmon directory is absent
If ne_utils_path_scan returns a non-zero code when no hwmon entries exist (e.g., directory missing in containers), returning -1 will mark the update as an error every scrape. Prefer treating “no matches” as success with no samples; only return -1 on genuine IO/parse errors.
- if (ret != 0) {
- return -1;
- }
+ if (ret != 0) {
+ /* If ret indicates 'no matches', treat as success to avoid noisy errors.
+ * Only return -1 on actual scan errors. Adjust once ne_utils_path_scan
+ * semantics are confirmed. */
+ return 0;
+ }Please confirm ne_utils_path_scan return semantics for “no matches” vs “error” so we can finalize the exact condition here.
🏁 Script executed:
#!/bin/bash
# Locate the definition and usages of ne_utils_path_scan to inspect its return semantics
rg -n -C5 "ne_utils_path_scan" -R .Length of output: 440
🏁 Script executed:
#!/bin/bash
# Search for the definition and all call sites of ne_utils_path_scan to inspect its return codes
rg -n "ne_utils_path_scan" -C2 .Length of output: 6054
#!/bin/bash
# Display the implementation of ne_utils_path_scan to inspect return semantics
sed -n '250,350p' plugins/in_node_exporter_metrics/ne_utils.c🏁 Script executed:
#!/bin/bash
# Display the implementation of ne_utils_path_scan to inspect return semantics
sed -n '270,350p' plugins/in_node_exporter_metrics/ne_utils.cLength of output: 2271
Distinguish “no matches” from real errors in ne_utils_path_scan
Currently, ne_utils_path_scan() returns ‑1 for both IO/glob errors and “no matches” (GLOB_NOMATCH or zero paths), so every caller treats an empty directory as a scrape failure. To avoid noisy errors when hwmon (or any other probe) simply has nothing to scan, split out “no matches” as a distinct return code and handle it at each call site.
– In plugins/in_node_exporter_metrics/ne_utils.c, change the GLOB_NOMATCH and zero-count paths to return e.g. 1 instead of -1:
--- a/plugins/in_node_exporter_metrics/ne_utils.c
+++ b/plugins/in_node_exporter_metrics/ne_utils.c
@@ case GLOB_NOMATCH:
- return -1;
+ globfree(&globbuf);
+ /* no entries matched */
+ return 1;
@@
- if (globbuf.gl_pathc <= 0) {
- globfree(&globbuf);
- return -1;
- }
+ if (globbuf.gl_pathc <= 0) {
+ globfree(&globbuf);
+ /* no entries found */
+ return 1;
+ }– In each consumer (e.g. ne_hwmon_linux.c, lines 263–268), treat <0 as a real error, ==1 as “no samples,” and ==0 as success with data:
ret = ne_utils_path_scan(ctx, ctx->path_sysfs, HWMON_PATTERN,
NE_SCAN_DIR, &hwmons);
-if (ret != 0) {
- return -1;
-}
+if (ret < 0) {
+ /* genuine IO/parse error – fail the plugin */
+ return -1;
+}
+if (ret == 1) {
+ /* no hwmon entries – skip gracefully */
+ return 0;
+}
/* ret == 0: we have hwmon dirs to iterate */Repeat the same pattern for the other probes (ne_textfile_linux.c, ne_thermalzone_linux.c, etc.). This lets containers without a hwmon (or thermal/CPU, NVMe, etc.) path succeed with zero metrics rather than error on every scrape.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ret = ne_utils_path_scan(ctx, ctx->path_sysfs, HWMON_PATTERN, | |
| NE_SCAN_DIR, &hwmons); | |
| if (ret != 0) { | |
| return -1; | |
| } | |
| ret = ne_utils_path_scan(ctx, ctx->path_sysfs, HWMON_PATTERN, | |
| NE_SCAN_DIR, &hwmons); | |
| if (ret < 0) { | |
| /* genuine IO/parse error – fail the plugin */ | |
| return -1; | |
| } | |
| if (ret == 1) { | |
| /* no hwmon entries – skip gracefully */ | |
| return 0; | |
| } | |
| /* ret == 0: we have hwmon dirs to iterate */ |
🤖 Prompt for AI Agents
In plugins/in_node_exporter_metrics/ne_hwmon_linux.c around lines 263 to 268,
update the handling of the return value from ne_utils_path_scan to distinguish
between real errors and "no matches." Modify ne_utils_path_scan in ne_utils.c to
return 1 for no matches instead of -1. Then, in this file, treat any return
value less than 0 as a real error, return 1 as no samples found (not an error),
and 0 as success with data. Apply this pattern consistently to avoid treating
empty directories as scrape failures.
| #define HWMON_BASE "/class/hwmon/hwmon" | ||
| #define HWMON_PATTERN HWMON_BASE"*" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm sysfs path join semantics and globbing behavior
Macros look fine; they’ll typically be joined with ctx->path_sysfs. Watch for potential double slashes when users set path.sysfs to “/sys/”. Most syscalls tolerate that, but if you canonicalize paths elsewhere, it could matter. Also, ensure the code actually uses globbing or an equivalent to consume the HWMON_PATTERN string.
- Nit: add a space for readability in concatenated string macros.
-#define HWMON_PATTERN HWMON_BASE"*"
+#define HWMON_PATTERN HWMON_BASE "*"Run a quick scan to verify how these macros are used and how path.sysfs is joined:
🏁 Script executed:
#!/bin/bash
rg -n '(HWMON_BASE|HWMON_PATTERN|path_sysfs)' -A3 -B3 plugins/in_node_exporter_metricsLength of output: 27779
Verify HWMON_PATTERN Usage and Sysfs Path Joining
- Trailing slashes in
ctx->path_sysfsare trimmed inne_hwmon_linux.c(around lines 285–288), so joining withHWMON_PATTERNwon’t produce double-slashes. HWMON_PATTERNis passed directly tone_utils_path_scanon line 263 ofne_hwmon_linux.c, confirming that globbing is actually used.
Stylistic nit for readability (optional):
-#define HWMON_PATTERN HWMON_BASE"*"
+#define HWMON_PATTERN HWMON_BASE "*"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #define HWMON_BASE "/class/hwmon/hwmon" | |
| #define HWMON_PATTERN HWMON_BASE"*" | |
| #define HWMON_BASE "/class/hwmon/hwmon" | |
| #define HWMON_PATTERN HWMON_BASE "*" |
🤖 Prompt for AI Agents
In plugins/in_node_exporter_metrics/ne_hwmon_linux.h around lines 25 to 26, the
macro HWMON_PATTERN is defined as HWMON_BASE followed directly by an asterisk
without a slash, which is correct given that trailing slashes are trimmed in
ctx->path_sysfs before joining. Confirm that this usage aligns with the globbing
in ne_utils_path_scan at line 263 in ne_hwmon_linux.c. For improved readability,
consider adding a comment explaining the pattern or slightly adjusting the macro
definition for clarity, but no functional change is required.
Signed-off-by: Eduardo Silva <[email protected]>
edsiper
force-pushed
the
node_exporter-hwmon
branch
from
3a8ef28 to
b74faf1
Compare
This PR adds hardware monitoring (hwmon) support to the in_node_exporter_metrics input plugin, enabling collection of hardware sensor data from Linux systems.
New metrics
Features
Configuration Options
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.