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

Report total physical system memory on Linux #855

Merged
merged 3 commits into from
Dec 15, 2021

Conversation

nr-ahemsath
Copy link
Member

@nr-ahemsath nr-ahemsath commented Dec 15, 2021

Description

Resolves #36.

Total system memory will now be reported by .NET core agents running on Linux systems. The agent reads /proc/meminfo and pulls out the MemTotal value.

Regarding testing: I considered adding a unit test for this, but the only real logic is the regex parsing of the MemTotal: 12345 kB line, and so a unit test using mock/fake /proc/meminfo data would just be testing that the regex works or doesn't. I can still add a unit test like this if desired; some refactoring would be required to make the method in question testable. We don't have any existing integration tests for this feature of the agent, and an integration test seems too heavyweight for this.

I did test this change manually on Ubuntu 20.04 and verified that the total physical memory was being sent in the utilization payload.

Author Checklist

  • Unit tests, Integration tests, and Unbounded tests completed - see above statement
  • Performance testing completed with satisfactory results (if required) - N/A
  • Agent Changelog or Lambda Agent changelog updated

Reviewer Checklist

  • Perform code review
  • Pull request was adequately tested (new/existing tests, performance tests)
  • Review Changelog

@nr-ahemsath nr-ahemsath added this to the FY22Q3 - Firefly milestone Dec 15, 2021
Copy link
Contributor

@tehbio tehbio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Don't forget to update the changelog.

if (match.Success)
{
// Despite the text of meminfo showing everything as 'kB' which should be
// 'kilobytes', i.e. 1000 bytes, in fact it is reporting 'kibibytes' (KiB, 1024 bytes)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great comment!

@nr-ahemsath nr-ahemsath merged commit 00cc6dd into main Dec 15, 2021
@nr-ahemsath nr-ahemsath deleted the read-physical-memory-linux branch December 15, 2021 21:24
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.

Linux systems do not report total physical memory on agent connect
3 participants