Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cgroup v2 CPU & memory stats for correct chart generation #65

Merged
merged 2 commits into from
Oct 29, 2024

Conversation

gkhandake
Copy link
Contributor

cgroup v2 CPU & Memory stats for correct chart generation

  • The CPU set is located at "/cpuset.cpus.effective" for cgroup v2
  • Added 'file' and 'oom_kill' stats for memory chart to generate correctly

@f18m
Copy link
Owner

f18m commented Oct 24, 2024

hey @gkhandake ,
thanks for the PR! Hope you're doing well!!
I'm going to look at this right now! :)

@f18m
Copy link
Owner

f18m commented Oct 24, 2024

PR looks good but I think the CI pipeline is red because we need to update the expected values . IIRC when you run the tests locally you will get some "result JSON" and git contains the "expected JSON". You can basically copy-paste from the "result JSON" into the "expected JSON" whatever new fields are there... they should match the new fields you added in this PR btw :)

@gkhandake
Copy link
Contributor Author

hey @gkhandake , thanks for the PR! Hope you're doing well!! I'm going to look at this right now! :)

Hi @f18m, thank you, I am doing good. appreciate taking this up on priority.

@gkhandake
Copy link
Contributor Author

PR looks good but I think the CI pipeline is red because we need to update the expected values . IIRC when you run the tests locally you will get some "result JSON" and git contains the "expected JSON". You can basically copy-paste from the "result JSON" into the "expected JSON" whatever new fields are there... they should match the new fields you added in this PR btw :)

Fixed the failing unit test. thank you!

@f18m f18m merged commit 51c5901 into f18m:master Oct 29, 2024
2 checks passed
@f18m
Copy link
Owner

f18m commented Oct 29, 2024

@gkhandake I merged this PR.
I don't remember if you need a new cmonitor version released in COPR or not...

@gkhandake
Copy link
Contributor Author

@gkhandake I merged this PR. I don't remember if you need a new cmonitor version released in COPR or not...

Hi @f18m , Yes I need a new cmonitor version release in CORP. Can you please help with that? Appreciate your assistance.

@f18m
Copy link
Owner

f18m commented Nov 14, 2024

@gkhandake ,
I started looking into a new release but I managed to get a successful build only for EPEL/RHEL8 and EPEL/RHEL9. Fedora versions don't build due to some issues with legacy c++11 version.
I think the best thing to do is to drop support for c+11 and switch to c++14, which means cmonitor will not support Centos/RHEL7 anymore.
I believe we're in 2024 and that makes sense.
I'm working on some other PR to get the project to compile doing a bunch of upgrades (prometheus-cpp to v 1.2.4, c++14, conan 2). I'll see how far I can get...

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.

2 participants