prometheus-community / ipmi_exporter

Remote IPMI exporter for Prometheus
MIT License
457 stars 130 forks source link

bmcInfoFirmwareRevisionRegex is missing the patch part of semantic versionning #197

Open fbouynot opened 2 months ago

fbouynot commented 2 months ago

Hi, we're working with the following supermicro BMC output:

$ sudo ipmitool bmc info 
Device ID                 : 32
Device Revision           : 1
Firmware Revision         : 3.10
IPMI Version              : 2.0
Manufacturer ID           : 10876
Manufacturer Name         : Supermicro
Product ID                : 6970 (0x1b3a)
Product Name              : Unknown (0x1B3A)
Device Available          : yes
Provides Device SDRs      : no
Additional Device Support :
    Sensor Device
    SDR Repository Device
    SEL Device
    FRU Inventory Device
    IPMB Event Receiver
    IPMB Event Generator
    Chassis Device
Aux Firmware Rev Info     : 
    0x38
    0x01
    0x00
    0x00

We can see it is matched like by the following regex: https://github.com/prometheus-community/ipmi_exporter/blob/master/freeipmi/freeipmi.go#L45C2-L45C30

It does match only major.minor: image

Although I would like to get major.minor.patch: 3.10.38 here We can see the patch version is the first number printed after the Aux Firmware Rev Info line, we tried on multiples servers with different versions.

I could build an updated regex that would match the patch part if it exists, like this: image

^Firmware Revision\s*:\s*(?P<value>[0-9.]*).*$(?:\n(?:^(?!Aux Firmware Rev Info).*$\n)*Aux Firmware Rev Info.*\n\s*0x(?P<patch>[0-9]*))?

Although I'm not sure how to change the file https://github.com/prometheus-community/ipmi_exporter/blob/master/freeipmi/freeipmi.go to do a PR myself, because it's not only changing the regex since there is a new var, it would require to change at least GetBMCInfoFirmwareRevision too.

Could someone work on this feature request, or maybe help me to understand how to work on this by myself if you don't have time to do this?

bitfehler commented 1 month ago

Hello, and thank you for your suggestion. I am open to making this data available, though I have some reservations about the exact approach.

First, this may be besides the point, but I'd like to point out:

Although I would like to get major.minor.patch: 3.10.38 here

The first byte of the Aux Firmware Rev Info is 0x38, a hex value that would translate to version 3.10.56. Just as a precaution, if this is really the value you are looking for.

Next, I am not sure what the semantics of those values really are, and I suspect it differs from vendor to vendor? At any rate, if anything, these values should be exported as a new metric, but not be mixed with the existing Firmware Revision value. I suppose one could export each byte as separate metric or such, to make using the values easy.

fbouynot commented 1 month ago

Hello,

The first byte of the Aux Firmware Rev Info is 0x38, a hex value that would translate to version 3.10.56. Just as a precaution, if this is really the value you are looking for.

I think you are right, but supermicro is doing supermicro things : the hexadecimal value here is appended as a decimal patch number. Capture d’écran du 2024-07-09 17-48-41 Capture d’écran du 2024-07-09 17-48-58

these values should be exported as a new metric, but not be mixed with the existing Firmware Revision value

I agree with you, your approach looks safer.

Is this something you can do? Or do you have time to give me something that could help me to start? (doc or similar PR)

Thank you