in_node_exporter_metrics: Add a capability to adjust log level w/ error paths#12057
in_node_exporter_metrics: Add a capability to adjust log level w/ error paths#12057cosmo0920 wants to merge 2 commits into
Conversation
…or paths Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds level-aware file-read helpers in ChangesLevel-aware file read logging
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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
♻️ Duplicate comments (1)
plugins/in_node_exporter_metrics/ne_cpu_linux.c (1)
133-138: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftUnconditional
FLB_LOG_DEBUGalso masks genuine read errors here.This is the concrete call site affected by the uniform-downgrade concern raised in
ne_utils.c: any failure readingcore_throttle_count/package_throttle_count(not just an absent file) will now log at debug only, viane_utils_file_read_uint64_at_level(..., FLB_LOG_DEBUG). See the corresponding comment inne_utils.cfor the suggested fix.Also applies to: 155-160
🤖 Prompt for 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. In `@plugins/in_node_exporter_metrics/ne_cpu_linux.c` around lines 133 - 138, The `ne_utils_file_read_uint64_at_level()` calls in `ne_cpu_linux.c` are forcing `FLB_LOG_DEBUG` for `core_throttle_count` and `package_throttle_count`, which hides real read failures. Update these call sites to only downgrade the log level for the specific expected “missing file” case, and keep genuine errors at their normal severity. Use the existing `ne_utils_file_read_uint64_at_level` flow and the throttle counter reads in this file to apply the same fix to both sections.
🤖 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_utils.c`:
- Around line 101-112: The logging in ne_utils_file_log() and
ne_utils_file_read_uint64_at_level() is applying the caller’s debug level to
every failure, not just the expected missing-file case. Update the error
handling so ENOENT stays at the caller-supplied level, but any other errno is
logged as FLB_LOG_ERROR, preserving the path-in-message behavior without hiding
real failures. Make the same distinction wherever the helper is used, including
the cpu_thermal_update callers for core_throttle_count and
package_throttle_count.
---
Duplicate comments:
In `@plugins/in_node_exporter_metrics/ne_cpu_linux.c`:
- Around line 133-138: The `ne_utils_file_read_uint64_at_level()` calls in
`ne_cpu_linux.c` are forcing `FLB_LOG_DEBUG` for `core_throttle_count` and
`package_throttle_count`, which hides real read failures. Update these call
sites to only downgrade the log level for the specific expected “missing file”
case, and keep genuine errors at their normal severity. Use the existing
`ne_utils_file_read_uint64_at_level` flow and the throttle counter reads in this
file to apply the same fix to both sections.
🪄 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: e90921b6-99fb-4b3d-9093-2628bd326e94
📒 Files selected for processing (3)
plugins/in_node_exporter_metrics/ne_cpu_linux.cplugins/in_node_exporter_metrics/ne_utils.cplugins/in_node_exporter_metrics/ne_utils.h
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Closes #12033.
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