-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
collector/diskstats: add block device rotational #3022
base: master
Are you sure you want to change the base?
Conversation
queueDescs: []typedFactorDesc{ | ||
{ | ||
desc: prometheus.NewDesc( | ||
prometheus.BuildFQName(namespace, diskSubsystem, "rotational"), |
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.
Wondering if we should put that into the info metric as label instead. @SuperQ wdyt?
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 don't have a strong opinion, but I noticed an instance where a device
attribute was defined as a metric and left out of the info
metric labels.
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.
Good question, it depends a bit on how this is to be used. I kinda feel like this would be more useful as an info metric, as you will want to use it on a join with other metrics.
Having it be a separate metric requires some more awkward PromQL.
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.
@SuperQ Just to be clear, do you mean exposing this as an info
metric rather than a gauge
, as it is done currently?
2a22d79
to
bac46b7
Compare
Tests keep passing locally for me, I'll need to dig a bit more into this. |
@rexagod Any updates on the test failures? |
8d1195c
to
c01750c
Compare
Rebased on #3131, tests passing. |
Sync dependencies and log using the machinery introduced in prometheus#3097. Signed-off-by: Pranshu Srivastava <[email protected]>
Add metric to indicate if a device is rotational or not. Fixes: prometheus#2956 Signed-off-by: Pranshu Srivastava <[email protected]>
Signed-off-by: Pranshu Srivastava <[email protected]>
Add metric to indicate if a device is rotational or not.
Fixes: #2956