-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[receiver/hostmetricsreceiver] Add system.memory.linux.shared metric #45194
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
base: main
Are you sure you want to change the base?
Changes from 11 commits
bfc4ea7
18ba18d
7fdac29
7a13db0
b372a60
0615d67
a36b732
0a6c8f3
e8a7069
fdb6318
c2f0004
13f2a9a
958b857
3d982a9
b8d2201
1f0302c
b7ab719
96a23fe
1709177
451af87
831d228
0029f1b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| # Use this changelog template to create an entry for release notes. | ||
|
|
||
| # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
| change_type: 'enhancement' | ||
|
|
||
| # The name of the component, or a single word describing the area of concern, (e.g. receiver/filelog) | ||
| component: receiver/hostmetrics | ||
|
|
||
| # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | ||
| note: Add optional `system.memory.shared` metric (currently Linux only) | ||
|
|
||
| # Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. | ||
| issues: [32712] | ||
|
|
||
| # (Optional) One or more lines of additional information to render under the primary note. | ||
| # These lines will be padded with 2 spaces and then inserted directly into the document. | ||
| # Use pipe (|) for multiline entries. | ||
| subtext: | | ||
| This metric reports shared memory usage, including tmpfs filesystems, | ||
| System V shared memory, and POSIX shared memory. Currently only available | ||
| on Linux systems due to platform-specific data availability. | ||
| This corresponds to the `Shmem` field in `/proc/meminfo`. | ||
|
|
||
| Support for other platforms may be added in the future when the underlying | ||
| gopsutil library provides the necessary data. | ||
|
|
||
| # If your change doesn't affect end users or the exported elements of any package, | ||
| # you should instead start your pull request title with [chore] or use the "Skip Changelog" label. | ||
| # Optional: The change log or logs in which this entry should be included. | ||
| # e.g. '[user]' or '[user, api]' | ||
| # Include 'user' if the change is relevant to end users. | ||
| # Include 'api' if there is a change to a library API. | ||
| # Default: '[user]' | ||
| change_logs: [user] | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will only ever be available on Linux to my knowledge. To my knowledge, the other target platforms for the collector do not provide singular shared memory usage as a programatically readable statistic. I would recommend removing the suggestion that this will be supported by other platforms.
(We may also need to rename this metric in our semantic conventions to reflect that, CC @rogercoll)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense, I have removed the suggestion that other platforms may support this in the future.
Regarding renaming, we could go with something like
system.linux.memory.shared: That would be more accurate and consistent withsystem.linux.memory.availableandsystem.linux.memory.dirty.Should I wait for the semantic conventions to be updated first, or would you prefer I rename it in this PR and we update semconv afterward?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're having kind of a growing-pain with the OS name in semconv right now. Our written guidance suggests that the OS name goes after the "representative subsystem" of a metric namespace, and we would generally consider
system.memoryto be a "subsystem". So we would use the namesystem.memory.linux.sharedin that case. The tension is that this name is kind of awkward; the "memory" is what's shared, not the "linux".It's on my task list this week to get that sorted out once and for all. It's gonna come down to whether Semantic Conventions generally values clearer individual names or clearer organization within a namespace.
In the meantime, this PR can stay open since the functionality is all correct, and it's just down to the name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I have created an issue in the semconv repo to tackle this, see here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have opened a PR in semconv for this name change, see per here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The semconv PR has been merged with the rename to system.memory.linux.shared, with that this work has been updated to match that semconv.