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

Add system.linux.memory.available metric #257

Closed
ItsLastDay opened this issue Aug 15, 2023 · 6 comments · Fixed by #323
Closed

Add system.linux.memory.available metric #257

ItsLastDay opened this issue Aug 15, 2023 · 6 comments · Fixed by #323
Assignees

Comments

@ItsLastDay
Copy link
Contributor

What are you trying to achieve?

Linux kernel exposes a memory metric called available, which helps to understand how much memory is really available for the userspace apps. See open-telemetry/opentelemetry-collector-contrib#7417.

This value is unique to Linux, hence the system.linux prefix: see open-telemetry/opentelemetry-collector-contrib#19149 (comment).

What did you expect to see?

system.linux.memory.available - Gauge metric of integer type, meaning number of bytes available. No attributes.

Additional context.

open-telemetry/opentelemetry-collector-contrib#7417 (comment)

cc @dmitryax @braydonk

@braydonk
Copy link
Contributor

Based on this comment, I think adding this Linux-only metric for MemAvailable makes sense.

Earlier in discussion on open-telemetry/opentelemetry-collector-contrib#7417 there was a concern about the user-friendliness of there being a separate metric for available, but I think making it Linux specific alleviates this since it's more immediately obvious that we're exposing something unique. Based on this, I don't see any reason we can't move forward with this optional metric. It seems orthogonal to the other issues with the memory.usage states because it has clear reason and explanation for existing on its own.

P.S. It does seem that a similar value is available in the Win32 API so that is the only source of potential confusion. We'll need to ensure the purpose of this metric is very clearly documented as to why it exists solely for Linux.

@dmitryax
Copy link
Member

This was discussed in the System metrics WG, and we agreed to go ahead with introducing system.linux.memory.available

@ItsLastDay
Copy link
Contributor Author

Great! I'll prepare a PR in a week or two, after Braydon merges his #160.

@mx-psi
Copy link
Member

mx-psi commented Sep 12, 2023

Sorry for the spammy messages. I think this is unblocked now after #89? Or would we expect this to be under process metrics?

@braydonk
Copy link
Contributor

braydonk commented Sep 12, 2023

Actually I think it is unblocked. I'm not sure why I suggested we wait until my PR is merged, I think I was mistaken. @ItsLastDay I believe you can move forward with this by adding it to the appropriate spot in the new yaml from #89.

@ItsLastDay
Copy link
Contributor Author

Ack, will do.

codeboten pushed a commit to open-telemetry/opentelemetry-collector-contrib that referenced this issue Nov 23, 2023
…ric (#27247)

Implement a new metric `system.linux.memory.available`,
which I defined in
open-telemetry/semantic-conventions#257.

**Link to tracking Issue:**
#7417

**Testing:** added a check to an existing unit test. I had to refactor
the test a bit: it assumed that a certain metric will be at position 0,
which is not true now.

**Documentation:** Added note in `metadata.yaml`

---------

Co-authored-by: Curtis Robert <[email protected]>
RoryCrispin pushed a commit to ClickHouse/opentelemetry-collector-contrib that referenced this issue Nov 24, 2023
…ric (open-telemetry#27247)

Implement a new metric `system.linux.memory.available`,
which I defined in
open-telemetry/semantic-conventions#257.

**Link to tracking Issue:**
open-telemetry#7417

**Testing:** added a check to an existing unit test. I had to refactor
the test a bit: it assumed that a certain metric will be at position 0,
which is not true now.

**Documentation:** Added note in `metadata.yaml`

---------

Co-authored-by: Curtis Robert <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging a pull request may close this issue.

5 participants