-
Notifications
You must be signed in to change notification settings - Fork 890
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 process.disk.operations
metric to semantic conventions
#2845
Add process.disk.operations
metric to semantic conventions
#2845
Conversation
5280b95
to
ed52505
Compare
ed52505
to
84d7490
Compare
f6ba2ae
to
5bcb0c7
Compare
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
What needs to happen for this pull request to be merged? (Other than me updating the branch again) |
5bcb0c7
to
f94bda0
Compare
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
f94bda0
to
257228b
Compare
257228b
to
68e1464
Compare
@@ -38,6 +38,7 @@ Below is a table of Process metric instruments. | |||
| `process.memory.usage` | UpDownCounter | By | The amount of physical memory in use. | | | |||
| `process.memory.virtual` | UpDownCounter | By | The amount of committed virtual memory. | | | |||
| `process.disk.io` | Counter | By | Disk bytes transferred. | `direction` SHOULD be one of: `read`, `write` | | |||
| `process.disk.operations` | Counter | {operations} | Number of disk operations performed by the process | `direction` SHOULD be one of: `read`, `write` | |
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.
What about "control" (e.g. seek, S.M.A.R.T. signal)?
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 like how this is inline with process.disk.io
, w/ direction being either read
or write
. I suspect SMART capabilities would need their own metric(s). I don't how to address seek.
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.
There are lots and back-and-forth, e.g. #2617.
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.
@reyang I do recognize that this has been a much-discussed topic:
- the "direction" attribute was removed in favor of separate metric names Is direction dimension a mistake in semantic conventions? #2589
- the change was then reverted Mistake while applying the logic in #2617 from #2589: process.network.io and system.network.* should not have been split #2726
The current state is that metrics very similar to the proposed process.disk.operations
metric exist in the semantic conventions in the same shape as the proposed metric (with the "direction" attribute of either "read" or "write"):
- system.disk.io has a system.disk.operations counterpart
- process.disk.io does not have such counterpart
This proposal adds the process.disk.operations
metric that is related to process.disk.io
in the same was that the system.disk.operations
is related to system.disk.io
metric. I believe this is not a controversial change and there's no reason to not add this metric in the current shape.
If you want the discuss the shape of these metrics, I'd say it's better to open a separate issue.
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 believe this is not a controversial change and there's no reason to not add this metric in the current shape.
I have different understanding - we already know the existing ones are wrong and will take breaking changes before they can be marked stable, it'll be better to avoid adding more things that require a fix.
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.
68e1464
to
6230413
Compare
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Fixes #2812
Changes
Adds a new metric
process.disk.operations
to the semantic conventions.Context
Here's a related proposal to implement this metric in the OT collector's
hostmetrics
receiver: open-telemetry/opentelemetry-collector-contrib#14084.