in_node_exporter_metrics: add netstat linux collector#11052
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds netstat collector to in_node_exporter_metrics plugin for TCP and UDP statistics. Includes build configuration update, metric definitions, configuration option, and Linux-specific implementation reading from /proc/net/snmp. Changes
Sequence DiagramsequenceDiagram
participant Plugin as in_node_exporter_metrics
participant Collector as netstat_collector
participant Config as ne_netstat_configure
participant Update as ne_netstat_update
participant FS as /proc/net/snmp
Plugin->>Collector: Initialize collector
Collector->>Config: ne_netstat_init → netstat_configure
Config->>Config: Create 7 metrics (TCP/UDP)
Config->>Plugin: Register metrics
loop On scrape interval
Plugin->>Update: Trigger ne_netstat_update
Update->>FS: Read /proc/net/snmp
Update->>Update: Parse TCP/UDP blocks
Update->>Update: Extract & convert metric values
Update->>Plugin: Update gauge/counter metrics
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Extend node_exporter_metrics by exposing netstat metrics by parsing /proc/net/snmp data and aligning the output with Prometheus node_exporter conventions. Example parsed values from a live system: node_netstat_Tcp_ActiveOpens = 26581 node_netstat_Tcp_PassiveOpens = 129 node_netstat_Tcp_RetransSegs = 9254 node_netstat_Tcp_CurrEstab = 6 node_netstat_Udp_InDatagrams = 1292553 node_netstat_Udp_OutDatagrams = 1601161 node_netstat_Udp_NoPorts = 2047 node_netstat_Udp_InErrors = 0 Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ne_vmstat.c | ||
| ne_netdev.c | ||
| ne_netstat.c | ||
| ne_sockstat.c |
There was a problem hiding this comment.
Reference netstat collector without adding implementation
The new build sources list ne_netstat.c and ne.c now includes ne_netstat.h/netstat_collector, but there is no ne_netstat.* file anywhere in the repository. During configuration CMake will fail with “Cannot find source file "ne_netstat.c"”, preventing the node_exporter_metrics plugin from compiling. The collector implementation needs to be added (or the references removed) for the build to succeed.
Useful? React with 👍 / 👎.
ca5980d to
c0aa9f9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
plugins/in_node_exporter_metrics/CMakeLists.txt (1)
10-10: Build wiring for netstat is now present.ne_netstat.c is added to sources; this resolves earlier “missing source” feedback.
🧹 Nitpick comments (4)
plugins/in_node_exporter_metrics/ne_netstat.c (1)
20-33: Platform split via including the Linux .c is acceptable, but consider build-time selection.Optional: instead of including a .c, add ne_netstat_linux.c as a separate source guarded by platform condition in CMake for clearer compilation boundaries.
plugins/in_node_exporter_metrics/ne_netstat_linux.c (3)
113-137: Tighten loops once all target keys are found.Micro-opt: break early after matching all desired keys to avoid extra strcmp work.
Also applies to: 150-174
207-260: Surface read failures as debug once per scrape.ne_utils_file_read_lines() failure returns -1; netstat_update propagates -1 but the caller ignores it. Log at debug level to aid troubleshooting.
- flb_slist_destroy(&list); - return 0; + flb_slist_destroy(&list); + return 0;And in the caller (see below), log non-zero returns.
267-274: Use parameters and log update failures.Mark unused parameters to silence warnings, and log if netstat_update() fails.
static int ne_netstat_update(struct flb_input_instance *ins, struct flb_config *config, void *in_context) { struct flb_ne *ctx = (struct flb_ne *) in_context; - netstat_update(ctx); - return 0; + (void) ins; + (void) config; + + if (netstat_update(ctx) != 0) { + flb_plg_debug(ctx->ins, "netstat: failed to read /proc/net/snmp"); + } + return 0; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
plugins/in_node_exporter_metrics/CMakeLists.txt(1 hunks)plugins/in_node_exporter_metrics/ne.c(3 hunks)plugins/in_node_exporter_metrics/ne.h(2 hunks)plugins/in_node_exporter_metrics/ne_netstat.c(1 hunks)plugins/in_node_exporter_metrics/ne_netstat.h(1 hunks)plugins/in_node_exporter_metrics/ne_netstat_linux.c(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugins/in_node_exporter_metrics/ne_netstat_linux.c (4)
lib/cmetrics/src/cmt_gauge.c (2)
cmt_gauge_create(27-81)cmt_gauge_set(94-109)lib/cmetrics/src/cmt_counter.c (2)
cmt_counter_create(26-81)cmt_counter_set(138-161)src/flb_slist.c (3)
flb_slist_entry_get(342-361)flb_slist_split_string(223-313)flb_slist_destroy(327-339)plugins/in_node_exporter_metrics/ne_utils.c (2)
ne_utils_str_to_double(33-45)ne_utils_file_read_lines(151-194)
⏰ 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). (30)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- 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 (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=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_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=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_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=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_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- 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, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 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-22.04, clang-12)
- 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)
- GitHub Check: pr-compile-without-cxx (3.31.6)
- 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-centos-7
🔇 Additional comments (7)
plugins/in_node_exporter_metrics/ne.c (3)
42-42: Wire-up header looks good.Header inclusion for the new collector is correct and consistent with other collectors.
394-399: Config knob added — consistent with existing pattern.The per-collector scrape_interval entry aligns with other collectors.
198-198: Netstat correctly included in default enabled metrics.Verified:
NE_DEFAULT_ENABLED_METRICSatplugins/in_node_exporter_metrics/ne.h:36includes "netstat" in its comma-separated list of enabled collectors. The placement of the collector addition is correct.plugins/in_node_exporter_metrics/ne_netstat.h (1)
20-27: Public declaration is fine.Extern symbol matches usage in ne.c; guard is correct.
plugins/in_node_exporter_metrics/ne_netstat_linux.c (1)
20-41: Nice: Metric names/descriptions match node_exporter semantics.Gauge vs counter choices and help text align with expectations.
plugins/in_node_exporter_metrics/ne.h (2)
36-36: LGTM! Netstat added to default enabled metrics.The addition of "netstat" to the default metrics list is correctly placed within the Linux-specific block and logically positioned between "netdev" and "sockstat" (network-related collectors).
166-175: LGTM! Netstat metric fields properly declared.All 8 requested metrics from issue #11044 are correctly declared:
- Metric types are appropriate: gauge for
CurrEstab(current state), counters for cumulative statistics- Naming convention matches Prometheus node_exporter standards
- Logical placement after the sockstat_linux section
Fixes #11044
Extend node_exporter_metrics by exposing netstat metrics by parsing /proc/net/snmp data and aligning the output with Prometheus node_exporter conventions.
Example parsed values from a live system:
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.
Summary by CodeRabbit
collector.netstat.scrape_intervalconfiguration option to control collection frequency.