Skip to content

in_node_exporter_metrics: fix incorrect pointer arithmetic in diskstats metric cache - #12255

Open
ngillen wants to merge 1 commit into
fluent:masterfrom
ngillen:fix/in_node_exporter_metrics-diskstats
Open

in_node_exporter_metrics: fix incorrect pointer arithmetic in diskstats metric cache#12255
ngillen wants to merge 1 commit into
fluent:masterfrom
ngillen:fix/in_node_exporter_metrics-diskstats

Conversation

@ngillen

@ngillen ngillen commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

ctx->dt_metrics is a flat array of struct dt_metric, but metric_cache_set()/metric_cache_update() indexed it as an array of pointers, causing every entry past id=0 to read/write at the wrong offset. This corrupts both the scaling factor and the metric pointer, producing wildly incorrect node_disk_* values on Linux and a corrupted counter target on Darwin.

Also extract the repeated .001 ms-to-seconds factor into a named MS_TO_SECONDS constant.

Fixes: #7583

Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

  • [N/A] Example configuration file for the change
  • Debug log output from testing the change
Fluent Bit v5.1.0
* Copyright (C) 2015-2026 The Fluent Bit Authors
* Fluent Bit is a CNCF graduated project under the Fluent organization
* https://fluentbit.io

______ _                  _    ______ _ _           _____  _____ 
|  ___| |                | |   | ___ (_) |         |  ___||  _  |
| |_  | |_   _  ___ _ __ | |_  | |_/ /_| |_  __   _|___ \ | |/' |
|  _| | | | | |/ _ \ '_ \| __| | ___ \ | __| \ \ / /   \ \|  /| |
| |   | | |_| |  __/ | | | |_  | |_/ / | |_   \ V //\__/ /\ |_/ /
\_|   |_|\__,_|\___|_| |_|\__| \____/|_|\__|   \_/ \____(_)\___/


[2026/08/10 11:35:06.217] [ info] Configuration:
[2026/08/10 11:35:06.217] [ info]  flush time     | 1.000000 seconds
[2026/08/10 11:35:06.217] [ info]  grace          | 5 seconds
[2026/08/10 11:35:06.217] [ info]  daemon         | 0
[2026/08/10 11:35:06.217] [ info] ___________
[2026/08/10 11:35:06.217] [ info]  inputs:
[2026/08/10 11:35:06.217] [ info]      node_exporter_metrics
[2026/08/10 11:35:06.217] [ info] ___________
[2026/08/10 11:35:06.217] [ info]  filters:
[2026/08/10 11:35:06.217] [ info] ___________
[2026/08/10 11:35:06.217] [ info]  outputs:
[2026/08/10 11:35:06.217] [ info]      stdout.0
[2026/08/10 11:35:06.217] [ info] ___________
[2026/08/10 11:35:06.217] [ info]  collectors:
[2026/08/10 11:35:06.218] [ info] [fluent bit] version=5.1.0, commit=c16ed84f6f, pid=85399
[2026/08/10 11:35:06.218] [debug] [engine] coroutine stack size: 65536 bytes (64.0K)
[2026/08/10 11:35:06.218] [ info] [storage] ver=1.5.4, type=memory, sync=normal, checksum=off, max_chunks_up=128
[2026/08/10 11:35:06.218] [ info] [simd    ] SSE2
[2026/08/10 11:35:06.218] [ info] [cmetrics] version=2.2.1
[2026/08/10 11:35:06.218] [ info] [ctraces ] version=0.7.1
[2026/08/10 11:35:06.218] [ info] [input:node_exporter_metrics:node_exporter_metrics.0] initializing
[2026/08/10 11:35:06.218] [ info] [input:node_exporter_metrics:node_exporter_metrics.0] storage_strategy='memory' (memory only)
[2026/08/10 11:35:06.218] [ info] [input:node_exporter_metrics:node_exporter_metrics.0] path.rootfs = /
[2026/08/10 11:35:06.218] [ info] [input:node_exporter_metrics:node_exporter_metrics.0] path.procfs = /proc
[2026/08/10 11:35:06.218] [ info] [input:node_exporter_metrics:node_exporter_metrics.0] path.sysfs  = /sys
[2026/08/10 11:35:06.219] [debug] [input:node_exporter_metrics:node_exporter_metrics.0] enabled metrics diskstats
[2026/08/10 11:35:06.219] [debug] [input:node_exporter_metrics:node_exporter_metrics.0] [thread init] initialization OK
[2026/08/10 11:35:06.220] [ info] [input:node_exporter_metrics:node_exporter_metrics.0] thread instance initialized
[2026/08/10 11:35:06.220] [debug] [node_exporter_metrics:node_exporter_metrics.0] created event channels: read=32 write=33
[2026/08/10 11:35:06.220] [debug] [stdout:stdout.0] created event channels: read=36 write=37
[2026/08/10 11:35:06.220] [ info] [sp] stream processor started
[2026/08/10 11:35:06.220] [ info] [engine] Shutdown Grace Period=5, Shutdown Input Grace Period=2
[2026/08/10 11:35:06.220] [ info] [output:stdout:stdout.0] worker #0 started
[2026/08/10 11:35:10.409] [debug] [input:node_exporter_metrics:node_exporter_metrics.0] skip device: loop0
[2026/08/10 11:35:11.408] [debug] [task] created task=0x71c4240361c0 id=0 OK
[2026/08/10 11:35:11.409] [debug] [output:stdout:stdout.0] task_id=0 assigned to thread #0
2026-08-10T09:35:10.409474347Z node_disk_reads_completed_total{device="nvme0n1"} = 6239999
2026-08-10T09:35:10.409577322Z node_disk_reads_completed_total{device="dm-0"} = 9751940
2026-08-10T09:35:10.409600290Z node_disk_reads_completed_total{device="dm-1"} = 9751720
2026-08-10T09:35:10.409948928Z node_disk_reads_completed_total{device="sda"} = 0
2026-08-10T09:35:10.409968370Z node_disk_reads_completed_total{device="sdb"} = 0
2026-08-10T09:35:10.409477126Z node_disk_reads_merged_total{device="nvme0n1"} = 3514577
2026-08-10T09:35:10.409578813Z node_disk_reads_merged_total{device="dm-0"} = 0
2026-08-10T09:35:10.409600987Z node_disk_reads_merged_total{device="dm-1"} = 0
2026-08-10T09:35:10.409950249Z node_disk_reads_merged_total{device="sda"} = 0
2026-08-10T09:35:10.409969201Z node_disk_reads_merged_total{device="sdb"} = 0
2026-08-10T09:35:10.409479159Z node_disk_read_bytes_total{device="nvme0n1"} = 46999789056
2026-08-10T09:35:10.409579715Z node_disk_read_bytes_total{device="dm-0"} = 46983795712
2026-08-10T09:35:10.409601897Z node_disk_read_bytes_total{device="dm-1"} = 46982779904
2026-08-10T09:35:10.409951295Z node_disk_read_bytes_total{device="sda"} = 0
2026-08-10T09:35:10.409969833Z node_disk_read_bytes_total{device="sdb"} = 0
2026-08-10T09:35:10.409480553Z node_disk_read_time_seconds_total{device="nvme0n1"} = 576.54200000000003
2026-08-10T09:35:10.409581134Z node_disk_read_time_seconds_total{device="dm-0"} = 2128.768
2026-08-10T09:35:10.409602748Z node_disk_read_time_seconds_total{device="dm-1"} = 2136.3429999999998
2026-08-10T09:35:10.409952669Z node_disk_read_time_seconds_total{device="sda"} = 0
2026-08-10T09:35:10.409970396Z node_disk_read_time_seconds_total{device="sdb"} = 0
2026-08-10T09:35:10.409482039Z node_disk_writes_completed_total{device="nvme0n1"} = 559118
2026-08-10T09:35:10.409582248Z node_disk_writes_completed_total{device="dm-0"} = 1013319
2026-08-10T09:35:10.409603641Z node_disk_writes_completed_total{device="dm-1"} = 807364
2026-08-10T09:35:10.409953717Z node_disk_writes_completed_total{device="sda"} = 0
2026-08-10T09:35:10.409970952Z node_disk_writes_completed_total{device="sdb"} = 0
2026-08-10T09:35:10.409483535Z node_disk_writes_merged_total{device="nvme0n1"} = 454249
2026-08-10T09:35:10.409582924Z node_disk_writes_merged_total{device="dm-0"} = 0
2026-08-10T09:35:10.409604790Z node_disk_writes_merged_total{device="dm-1"} = 0
2026-08-10T09:35:10.409954531Z node_disk_writes_merged_total{device="sda"} = 0
2026-08-10T09:35:10.409971788Z node_disk_writes_merged_total{device="sdb"} = 0
2026-08-10T09:35:10.409485124Z node_disk_written_bytes_total{device="nvme0n1"} = 33107584000
2026-08-10T09:35:10.409584183Z node_disk_written_bytes_total{device="dm-0"} = 33107464192
2026-08-10T09:35:10.409605584Z node_disk_written_bytes_total{device="dm-1"} = 33107464192
2026-08-10T09:35:10.409955204Z node_disk_written_bytes_total{device="sda"} = 0
2026-08-10T09:35:10.409972569Z node_disk_written_bytes_total{device="sdb"} = 0
2026-08-10T09:35:10.409486654Z node_disk_write_time_seconds_total{device="nvme0n1"} = 1097.135
2026-08-10T09:35:10.409585138Z node_disk_write_time_seconds_total{device="dm-0"} = 48948.391000000003
2026-08-10T09:35:10.409606524Z node_disk_write_time_seconds_total{device="dm-1"} = 4780.1030000000001
2026-08-10T09:35:10.409955982Z node_disk_write_time_seconds_total{device="sda"} = 0
2026-08-10T09:35:10.409973158Z node_disk_write_time_seconds_total{device="sdb"} = 0
2026-08-10T09:35:10.409491425Z node_disk_io_time_seconds_total{device="nvme0n1"} = 475.95600000000002
2026-08-10T09:35:10.409586962Z node_disk_io_time_seconds_total{device="dm-0"} = 613.80700000000002
2026-08-10T09:35:10.409607996Z node_disk_io_time_seconds_total{device="dm-1"} = 672.86000000000001
2026-08-10T09:35:10.409957288Z node_disk_io_time_seconds_total{device="sda"} = 0
2026-08-10T09:35:10.409974395Z node_disk_io_time_seconds_total{device="sdb"} = 0
2026-08-10T09:35:10.409493124Z node_disk_io_time_weighted_seconds_total{device="nvme0n1"} = 1682.298
2026-08-10T09:35:10.409587987Z node_disk_io_time_weighted_seconds_total{device="dm-0"} = 51077.159
2026-08-10T09:35:10.409609024Z node_disk_io_time_weighted_seconds_total{device="dm-1"} = 6916.4459999999999
2026-08-10T09:35:10.409958095Z node_disk_io_time_weighted_seconds_total{device="sda"} = 0
2026-08-10T09:35:10.409974967Z node_disk_io_time_weighted_seconds_total{device="sdb"} = 0
2026-08-10T09:35:10.409494822Z node_disk_discards_completed_total{device="nvme0n1"} = 0
2026-08-10T09:35:10.409588610Z node_disk_discards_completed_total{device="dm-0"} = 0
2026-08-10T09:35:10.409610166Z node_disk_discards_completed_total{device="dm-1"} = 0
2026-08-10T09:35:10.409958808Z node_disk_discards_completed_total{device="sda"} = 0
2026-08-10T09:35:10.409975514Z node_disk_discards_completed_total{device="sdb"} = 0
2026-08-10T09:35:10.409496238Z node_disk_discards_merged_total{device="nvme0n1"} = 0
2026-08-10T09:35:10.409589264Z node_disk_discards_merged_total{device="dm-0"} = 0
2026-08-10T09:35:10.409611371Z node_disk_discards_merged_total{device="dm-1"} = 0
2026-08-10T09:35:10.409959826Z node_disk_discards_merged_total{device="sda"} = 0
2026-08-10T09:35:10.409976023Z node_disk_discards_merged_total{device="sdb"} = 0
2026-08-10T09:35:10.409497606Z node_disk_discarded_sectors_total{device="nvme0n1"} = 0
2026-08-10T09:35:10.409590167Z node_disk_discarded_sectors_total{device="dm-0"} = 0
2026-08-10T09:35:10.409612202Z node_disk_discarded_sectors_total{device="dm-1"} = 0
2026-08-10T09:35:10.409960561Z node_disk_discarded_sectors_total{device="sda"} = 0
2026-08-10T09:35:10.409976515Z node_disk_discarded_sectors_total{device="sdb"} = 0
2026-08-10T09:35:10.409499010Z node_disk_discard_time_seconds_total{device="nvme0n1"} = 0
2026-08-10T09:35:10.409590879Z node_disk_discard_time_seconds_total{device="dm-0"} = 0
2026-08-10T09:35:10.409613008Z node_disk_discard_time_seconds_total{device="dm-1"} = 0
2026-08-10T09:35:10.409961499Z node_disk_discard_time_seconds_total{device="sda"} = 0
2026-08-10T09:35:10.409977015Z node_disk_discard_time_seconds_total{device="sdb"} = 0
2026-08-10T09:35:10.409500134Z node_disk_flush_requests_total{device="nvme0n1"} = 21027
2026-08-10T09:35:10.409591714Z node_disk_flush_requests_total{device="dm-0"} = 0
2026-08-10T09:35:10.409613968Z node_disk_flush_requests_total{device="dm-1"} = 0
2026-08-10T09:35:10.409962410Z node_disk_flush_requests_total{device="sda"} = 0
2026-08-10T09:35:10.409977525Z node_disk_flush_requests_total{device="sdb"} = 0
2026-08-10T09:35:10.409501677Z node_disk_flush_requests_time_seconds_total{device="nvme0n1"} = 8.620000000000001
2026-08-10T09:35:10.409592953Z node_disk_flush_requests_time_seconds_total{device="dm-0"} = 0
2026-08-10T09:35:10.409614611Z node_disk_flush_requests_time_seconds_total{device="dm-1"} = 0
2026-08-10T09:35:10.409963247Z node_disk_flush_requests_time_seconds_total{device="sda"} = 0
2026-08-10T09:35:10.409978061Z node_disk_flush_requests_time_seconds_total{device="sdb"} = 0
2026-08-10T09:35:10.409489627Z node_disk_io_now{device="nvme0n1"} = 0
2026-08-10T09:35:10.409586036Z node_disk_io_now{device="dm-0"} = 0
2026-08-10T09:35:10.409607247Z node_disk_io_now{device="dm-1"} = 0
2026-08-10T09:35:10.409956629Z node_disk_io_now{device="sda"} = 0
2026-08-10T09:35:10.409973787Z node_disk_io_now{device="sdb"} = 0
  • Attached Valgrind output that shows no leaks or memory corruption was found
==85866== 
==85866== HEAP SUMMARY:
==85866==     in use at exit: 0 bytes in 0 blocks
==85866==   total heap usage: 19,084 allocs, 19,084 frees, 11,953,461 bytes allocated
==85866== 
==85866== All heap blocks were freed -- no leaks are possible
==85866== 
==85866== For lists of detected and suppressed errors, rerun with: -s
==85866== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • Run local packaging test showing all targets (including any new ones) build.
  • Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • [N/A] Documentation required for this feature

Backporting

  • Backport to latest stable release.

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

Refactor

  • Improved consistency and maintainability of disk metric processing across supported platforms.
  • Standardized disk-time unit conversions.

Bug Fixes

  • Preserved existing disk metric behavior and error handling.
  • No changes to metric names, values, or collection behavior.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The diskstats plugin now uses contiguous metric caches on Linux and Darwin. Linux disk time metrics use a shared millisecond-to-second conversion constant.

Changes

Diskstats metrics

Layer / File(s) Summary
Contiguous metric cache
plugins/in_node_exporter_metrics/ne_diskstats_linux.c, plugins/in_node_exporter_metrics/ne_diskstats_darwin.c
The metric caches store struct dt_metric entries in directly indexed arrays. Cache lookup no longer uses pointer indirection.
Shared time conversion
plugins/in_node_exporter_metrics/ne_diskstats_linux.c
The plugin defines MS_TO_SECONDS and uses it for read, write, I/O, discard, and flush-request time metrics.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: edsiper

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The cache fixes address issue #7583 by correcting diskstats values derived from the flat metric array.
Out of Scope Changes check ✅ Passed All changes are limited to the Linux and Darwin diskstats cache fixes and the related millisecond conversion constant.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the incorrect pointer arithmetic fix in the diskstats metric cache, which is the main change.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@plugins/in_node_exporter_metrics/ne_diskstats_linux.c`:
- Around line 85-91: Update metric_cache_set and metric_cache_update in
ne_diskstats_darwin.c to cast ctx->dt_metrics to struct dt_metric * rather than
struct dt_metric **, and access entries directly by cache[id] without deriving m
from &cache[id]. Preserve the existing metric and factor updates while using the
flat struct-sized layout.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 789ba633-26ec-47c6-a4bc-b1e19ea71284

📥 Commits

Reviewing files that changed from the base of the PR and between a1d6fb1 and c16ed84.

📒 Files selected for processing (1)
  • plugins/in_node_exporter_metrics/ne_diskstats_linux.c

Comment thread plugins/in_node_exporter_metrics/ne_diskstats_linux.c
ctx->dt_metrics is a flat array of 'struct dt_metric', but
metric_cache_set()/metric_cache_update() indexed it as an array of
pointers, causing every entry past id=0 to read/write at the wrong
offset. This corrupts both the scaling factor and the metric
pointer, producing wildly incorrect node_disk_* values on Linux
and a corrupted counter target on Darwin.

Also extract the repeated .001 ms-to-seconds factor into a named
MS_TO_SECONDS constant.

Signed-off-by: Nicolas Gillen <nicolas.gillen@agregio-solutions.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

node_exporter_metrics diskstats reports incorrect, large float values

1 participant