feat: expose averaged process CPU and memory per cluster node - #1743
Open
praveen5959 wants to merge 1 commit into
Open
feat: expose averaged process CPU and memory per cluster node#1743praveen5959 wants to merge 1 commit into
praveen5959 wants to merge 1 commit into
Conversation
Contributor
WalkthroughThe resource monitor now samples process CPU and memory usage every 10 seconds. The metrics module averages samples and exposes Prometheus gauges. Prometheus metric conversion includes the new process metrics. ChangesProcess resource metrics
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ResourceMonitor
participant SYS_INFO
participant CurrentProcess
participant ProcessMetrics
participant PrometheusRegistry
ResourceMonitor->>SYS_INFO: Refresh system information
ResourceMonitor->>CurrentProcess: Resolve current process
CurrentProcess-->>ResourceMonitor: CPU usage and memory
ResourceMonitor->>ProcessMetrics: Record sample
ProcessMetrics->>PrometheusRegistry: Update averaged gauges
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@src/metrics/mod.rs`:
- Around line 179-237: Rename PROCESS_CPU_USAGE_PERCENT and PROCESS_MEMORY_BYTES
to clearly indicate lifetime averages, updating both metric names and help
strings to use “average” terminology. Apply the same renamed metric identifiers
in the matching definitions or references in prom_utils.rs, while leaving the
accumulator and recording behavior unchanged.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 654732fb-7118-4d92-a09b-0c5654821b6e
📒 Files selected for processing (3)
src/handlers/http/resource_check.rssrc/metrics/mod.rssrc/metrics/prom_utils.rs
Comment on lines
+179
to
+237
| pub static PROCESS_CPU_USAGE_PERCENT: Lazy<Gauge> = Lazy::new(|| { | ||
| Gauge::with_opts( | ||
| Opts::new( | ||
| "process_cpu_usage_percent", | ||
| "Current CPU usage percent for this Parseable process", | ||
| ) | ||
| .namespace(METRICS_NAMESPACE), | ||
| ) | ||
| .expect("metric can be created") | ||
| }); | ||
|
|
||
| pub static PROCESS_MEMORY_BYTES: Lazy<Gauge> = Lazy::new(|| { | ||
| Gauge::with_opts( | ||
| Opts::new( | ||
| "process_memory_bytes", | ||
| "Current resident memory used by this Parseable process in bytes", | ||
| ) | ||
| .namespace(METRICS_NAMESPACE), | ||
| ) | ||
| .expect("metric can be created") | ||
| }); | ||
|
|
||
| const CPU_USAGE_PRECISION: f64 = 1_000.0; | ||
|
|
||
| #[derive(Default)] | ||
| struct ProcessMetricsAccumulator { | ||
| cpu_usage_sum: AtomicU64, | ||
| memory_bytes_sum: AtomicU64, | ||
| sample_count: AtomicU64, | ||
| } | ||
|
|
||
| impl ProcessMetricsAccumulator { | ||
| fn record(&self, cpu_usage_percent: f64, memory_bytes: u64) -> (f64, f64) { | ||
| self.cpu_usage_sum.fetch_add( | ||
| (cpu_usage_percent * CPU_USAGE_PRECISION).round() as u64, | ||
| Ordering::Relaxed, | ||
| ); | ||
| self.memory_bytes_sum | ||
| .fetch_add(memory_bytes, Ordering::Relaxed); | ||
| let sample_count = self.sample_count.fetch_add(1, Ordering::Relaxed) + 1; | ||
|
|
||
| ( | ||
| self.cpu_usage_sum.load(Ordering::Relaxed) as f64 | ||
| / sample_count as f64 | ||
| / CPU_USAGE_PRECISION, | ||
| self.memory_bytes_sum.load(Ordering::Relaxed) as f64 / sample_count as f64, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| static PROCESS_METRICS_ACCUMULATOR: Lazy<ProcessMetricsAccumulator> = | ||
| Lazy::new(ProcessMetricsAccumulator::default); | ||
|
|
||
| pub fn record_process_metrics_sample(cpu_usage_percent: f64, memory_bytes: u64) { | ||
| let (average_cpu_usage, average_memory_bytes) = | ||
| PROCESS_METRICS_ACCUMULATOR.record(cpu_usage_percent, memory_bytes); | ||
| PROCESS_CPU_USAGE_PERCENT.set(average_cpu_usage); | ||
| PROCESS_MEMORY_BYTES.set(average_memory_bytes); | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Name these gauges as averages.
record_process_metrics_sample publishes a lifetime running average. The metric names and help text describe a current value. Dashboards and alerts can interpret these values as 10-second samples.
Rename the gauges to include average, or state Lifetime average in both help strings. Update the matching metric names in src/metrics/prom_utils.rs in the same change.
Proposed fix
- "process_cpu_usage_percent",
- "Current CPU usage percent for this Parseable process",
+ "process_cpu_usage_percent_average",
+ "Lifetime average CPU usage percent for this Parseable process",
- "process_memory_bytes",
- "Current resident memory used by this Parseable process in bytes",
+ "process_memory_bytes_average",
+ "Lifetime average resident memory used by this Parseable process in bytes",🤖 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 `@src/metrics/mod.rs` around lines 179 - 237, Rename PROCESS_CPU_USAGE_PERCENT
and PROCESS_MEMORY_BYTES to clearly indicate lifetime averages, updating both
metric names and help strings to use “average” terminology. Apply the same
renamed metric identifiers in the matching definitions or references in
prom_utils.rs, while leaving the accumulator and recording behavior unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit