Skip to content

feat: add rss_bytes memory metric for processes#66

Open
gtchaos wants to merge 2 commits intotigrisdata:mainfrom
gtchaos:main
Open

feat: add rss_bytes memory metric for processes#66
gtchaos wants to merge 2 commits intotigrisdata:mainfrom
gtchaos:main

Conversation

@gtchaos
Copy link
Copy Markdown

@gtchaos gtchaos commented Apr 10, 2026

Summary

  • Add rss_bytes field to ProcessMemory struct to capture resident memory from FoundationDB 7.1+ process status
  • Export mem_rss_bytes metric in the processes metric group

Test plan

  • go test ./models/... passes

Note

Low Risk
Low risk: adds a new optional rss_bytes field to status parsing and exposes it as an additional gauge, without altering existing metric names or collection flow.

Overview
Adds resident-set-size memory reporting for processes by extending ProcessMemory with RssBytes (mapped from JSON rss_bytes) and emitting a new mem_rss_bytes metric alongside existing per-process memory gauges.

Reviewed by Cursor Bugbot for commit c08d62c. Bugbot is set up for automated code reviews on this repo. Configure here.

Adds the mem_rss_bytes metric from FoundationDB 7.1+ process memory status.
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 10, 2026

Greptile Summary

This PR adds rss_bytes (resident set size) as a new field in the ProcessMemory model and exports it as the mem_rss_bytes gauge metric in the processes metric group. The change is minimal, correctly gated behind the existing process.Memory != nil guard, and consistent with how other memory metrics are handled.

Confidence Score: 5/5

Safe to merge — only P2 style findings, no correctness or data-integrity issues.

Both changes are trivial and correct: the struct field uses the right type and JSON tag, and the metric is emitted inside the existing null guard. The only findings are a one-space alignment gap and a missing test assertion, neither of which affects runtime behavior.

No files require special attention.

Important Files Changed

Filename Overview
models/process.go Adds RssBytes int field with correct json:"rss_bytes" tag to ProcessMemory struct; minor alignment inconsistency with adjacent fields.
metrics/group_processes.go Correctly exports the new mem_rss_bytes metric from process.Memory.RssBytes inside the existing Memory != nil guard.

Comments Outside Diff (1)

  1. models/process_test.go, line 74-81 (link)

    P2 Missing assertion for new field

    TestProcessMemoryWritesSingleBasic doesn't assert the new RssBytes field. Adding an assertion (and the corresponding value in the fixture JSON if it isn't there yet) would confirm the JSON deserialization path is correct end-to-end.

Reviews (1): Last reviewed commit: "feat: add rss_bytes memory metric for pr..." | Re-trigger Greptile

Comment thread models/process.go Outdated
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant