-
Notifications
You must be signed in to change notification settings - Fork 133
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
Added Power Measurement State to DCMI collector #160
Conversation
Ping @bitfehler |
Hi @k0ste, sorry and thanks for the ping. Thanks for the patch! In general, this is a welcome addition. One issue though: I'd much prefer the metric to simply not be there if it is unsupported. Could this be done, or is there a reason why the additional metric is needed? For example, is it possible for the |
Hi, @bitfehler! When I was imagine, how's better to handle this, I'm also thinked to this side, but:
Thanks! |
Absolutely!
I was actually thinking more like this: do perform the test for active/not available that you added, if "active" output metric regardless of value, if not available drop metric. Shouldn't that do? |
The 3rd point ☝️ exactly about this idea 🌚 |
I am not sure I understand. Your 3rd point says
but my suggestion is to only drop it if the new test you added in this PR says not available (i.e. when As I understand it, with your current patch, the decision table would be:
Or am I missing something? My suggestion would simply be to translate this into the following decision table:
Note the absence of an "impossible" state, hence I think it would make for a better solution. Sorry if I am misunderstanding s/t, maybe you can elaborate on your third point? |
You understanding is totally correctly. (But) If we translate "my table" to "your table" - this will be breaking changes in current how |
If the project okay for small breaking changes, I can prepare the release 1.7 with notes to |
@k0ste a breaking change is totally fine. can you update the patch accordingly? thanks a lot! |
@bitfehler updated! The checks: Host with Power Measurement root@host# curl -Ss "http://localhost:9291/ipmi?module=default&target=10.249.0.9" | grep -i power
# HELP ipmi_chassis_power_state Current power state (1=on, 0=off).
# TYPE ipmi_chassis_power_state gauge
ipmi_chassis_power_state 1
# HELP ipmi_dcmi_power_consumption_watts Current power consumption in Watts (no value if Power Measurement is not avail)
# TYPE ipmi_dcmi_power_consumption_watts gauge
ipmi_dcmi_power_consumption_watts 372
ipmi_sensor_state{id="4024",name="PS1 Status",type="Power Supply"} 0
ipmi_sensor_state{id="4091",name="PS2 Status",type="Power Supply"} 0
ipmi_sensor_value{id="4024",name="PS1 Status",type="Power Supply"} NaN
ipmi_sensor_value{id="4091",name="PS2 Status",type="Power Supply"} NaN Host without Power Measurement root@host# curl -Ss "http://localhost:9291/ipmi?module=default&target=10.249.0.6" | grep -i power
# HELP ipmi_chassis_power_state Current power state (1=on, 0=off).
# TYPE ipmi_chassis_power_state gauge
ipmi_chassis_power_state 1
ipmi_sensor_state{id="4158",name="PS1 Status",type="Power Supply"} 0
ipmi_sensor_state{id="4225",name="PS2 Status",type="Power Supply"} 0
ipmi_sensor_value{id="4158",name="PS1 Status",type="Power Supply"} NaN
ipmi_sensor_value{id="4225",name="PS2 Status",type="Power Supply"} NaN |
ping @bitfehler |
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.
Thanks for tackling this! Some details to iron out, but should be easy!
Measurement feature is not present. Before this change - the value was zero Signed-off-by: Konstantin Shalygin <k0ste@k0ste.ru>
Changes maden 🙏 |
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.
Excellent, thanks a bunch!
Added DCMI metric to definitely determine avail of Power Measurement feature
In some cases, for example with cheap Power Supply Units power consumption may be not available at all, to control this added
ipmi_dcmi_power_measurement_state
metric. The value is1
for power measure is available on, and0
otherwise.The main point for this changes is a try to knowledge: the measurements isn't accessed due PSU broken (FW/cable/etc issues) or not avail at all on system level (and system know about that)
The host with "normal PSU":
The host with "strange PSU's"