Skip to content

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

Merged
lmolkova merged 5 commits intoopen-telemetry:mainfrom
ChrsMark:decouple_shared_memory_metric
Apr 30, 2024
Merged

[metrics/system] Remove shared from system.memory.state values#933
lmolkova merged 5 commits intoopen-telemetry:mainfrom
ChrsMark:decouple_shared_memory_metric

Conversation

@ChrsMark
Copy link
Copy Markdown
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
Copy Markdown
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
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@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
Copy Markdown
Member Author

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

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark ChrsMark force-pushed the decouple_shared_memory_metric branch from d24df00 to fa32823 Compare April 29, 2024 08:26
Copy link
Copy Markdown
Member

@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
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