in_process_exporter_metrics: Fix the reported RSS ram#12139
Conversation
The RSS value exported through process_memory_bytes was taken directly from /proc/<pid>/stat, which is reported in pages rather than bytes. So before exposing the metric we have to convert to bytes, by using the size of pages Signed-off-by: Vasileios Anagnostopoulos <anagnwstopoulos@hotmail.com>
📝 WalkthroughWalkthroughThe process exporter now caches the system page size during configuration and uses it to convert procfs RSS page counts to bytes when collecting memory metrics. ChangesProcess exporter memory metrics
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
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_process_exporter_metrics/pe_config.c`:
- Around line 43-45: Update the page-size initialization in the pe_config setup
to detect when sysconf(_SC_PAGESIZE) returns a non-positive value, then emit a
warning or apply an explicit fallback before RSS conversion proceeds. Preserve
the existing page_size value and downstream RSS-skipping behavior when the
lookup fails.
🪄 Autofix (Beta)
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
Run ID: 6b5a0f53-cfe2-4dba-8d7f-3b66d43f0c21
📒 Files selected for processing (3)
plugins/in_process_exporter_metrics/pe.hplugins/in_process_exporter_metrics/pe_config.cplugins/in_process_exporter_metrics/pe_process.c
cosmo0920
left a comment
There was a problem hiding this comment.
Good catch! Thanks for the fix.
|
@cosmo0920 Am I the reason for the failing test or is it flaky? |
The failing test is one of the flaky tests. So, it's okey to merge. |
The RSS value exported through process_memory_bytes was taken directly from /proc/pid/stat, which is reported in pages rather than bytes.
So before exposing the metric we have to convert to bytes, by using the size of pages
I tested it with my devices and now the results match the ones from htop
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:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
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