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

[metrics/system] Remove shared from system.memory.state values #933

Merged
merged 5 commits into from
Apr 30, 2024

Conversation

ChrsMark
Copy link
Member

@ChrsMark ChrsMark commented Apr 18, 2024

Fixes #522

Changes

This PR removes the shared state from the possible values of the system.memory.state metric since it does not contribute to the total. The attribute guidelines mention though:

the sum of usage over all attribute values SHOULD be equal to the limit

Hence, we need to remove this state and make it standalone metric.

Merge requirement checklist

Note: I'm not sure if schema-next.yaml needs to be updated since the PR only touches the values' set 🤔

@ChrsMark ChrsMark requested review from a team April 18, 2024 08:49
@ChrsMark ChrsMark force-pushed the decouple_shared_memory_metric branch from 8f3f66e to fcbab37 Compare April 18, 2024 12:52
Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

LGTM, at a first glance I don't think this kind of change is possible to express in the telemetry schema

@ChrsMark ChrsMark force-pushed the decouple_shared_memory_metric branch from 0b4b92d to b59e2d7 Compare April 29, 2024 08:18
@ChrsMark ChrsMark force-pushed the decouple_shared_memory_metric branch 2 times, most recently from 994c13a to d24df00 Compare April 29, 2024 08:20
@ChrsMark ChrsMark requested a review from lmolkova April 29, 2024 08:20
@ChrsMark
Copy link
Member Author

Thank's @lmolkova ! I have addressed your comments. Please take another look.

Signed-off-by: ChrsMark <[email protected]>
@ChrsMark ChrsMark force-pushed the decouple_shared_memory_metric branch from d24df00 to fa32823 Compare April 29, 2024 08:26
Copy link
Contributor

@lmolkova lmolkova left a comment

Choose a reason for hiding this comment

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

Left a small suggestion.

@lmolkova lmolkova enabled auto-merge (squash) April 30, 2024 15:37
@lmolkova lmolkova merged commit 8590a71 into open-telemetry:main Apr 30, 2024
11 of 12 checks passed
drewby pushed a commit to drewby/otel-semantic-conventions that referenced this pull request May 23, 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.

Remove shared from system.memory.state values?
6 participants