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

Clarify how to report the total amount of memory #409

Merged
merged 10 commits into from
Oct 27, 2023

Conversation

mx-psi
Copy link
Member

@mx-psi mx-psi commented Oct 16, 2023

Updates open-telemetry/build-tools#127

Changes

  • Removes total from list of well-known values for system.memory.state
  • Adds note to system.memory.usage clarifying that adding over system.memory.state SHOULD equal the total amount of memory available in the system.
  • Adds opt-in system.memory.limit metric that reports the total amount of memory available in the system.

This tries to follow the guidance on https://github.com/open-telemetry/semantic-conventions/blob/main/docs/general/metrics.md#instrument-naming:

an instrument that measures the constant, known total amount of something should be called entity.limit. For example, system.memory.limit for the total amount of memory on a system.

and

Where appropriate, the sum of usage over all attribute values SHOULD be equal to the limit.

The total value was added on open-telemetry/build-tools#89 but was not listed before the refactoring. This rewording avoids an inconsistency with the general guidelines and adds a metric that may be useful in some implementations given that total is reported separately by certain OSs (e.g. Linux).

Merge requirement checklist

@mx-psi mx-psi force-pushed the mx-psi/system-total-memory branch 2 times, most recently from f0d46c3 to b862a39 Compare October 16, 2023 15:26
@mx-psi mx-psi force-pushed the mx-psi/system-total-memory branch from b862a39 to aac8c7e Compare October 16, 2023 15:29
@mx-psi mx-psi marked this pull request as ready for review October 16, 2023 15:30
@mx-psi mx-psi requested review from a team and joaopgrassi October 16, 2023 15:30
Copy link
Contributor

@jsuereth jsuereth left a comment

Choose a reason for hiding this comment

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

Can you open a bug to provide a mechanism in Schema-Files to denote an attribute being dropped?

Yes, it's a breaking change, we're actually going to try to encode breaking changes in SCHEMA over time.

@mx-psi
Copy link
Member Author

mx-psi commented Oct 19, 2023

Can you open a bug to provide a mechanism in Schema-Files to denote an attribute being dropped?

#1054 is related (although it refers to a different instance where the values were renamed, not dropped). Does that work? Or should we file a new one?

@tigrannajaryan
Copy link
Member

I think this can be a non-breaking change:

  • system.memory.usage{system.memory.state=used|free|shared|buffers|cached} -> system.memory.usage{system.memory.state=used|free|shared|buffers|cached} // i.e. remain as is
  • system.memory.usage{system.memory.state=total} -> system.memory.limit

We can define a rewite transform which will be a more generalized version of the existing split transform.

@mx-psi mx-psi requested a review from joaopgrassi October 26, 2023 10:33
@AlexanderWert AlexanderWert merged commit 131637a into open-telemetry:main Oct 27, 2023
8 checks passed
dmitryax pushed a commit to open-telemetry/opentelemetry-collector-contrib that referenced this pull request Jan 5, 2024
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this pull request Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants