prometheus-community / ipmi_exporter

Remote IPMI exporter for Prometheus
MIT License
473 stars 133 forks source link

Failed to parse ipmimonitoring data from *.*.*.* #62

Closed huangzhangke closed 3 years ago

huangzhangke commented 3 years ago

there is a problem where scap data from som bmc ip. and I found the error is caused by csv parsing. In the line start with 63, there is one more comma(,), which causes the failure of CSV parsing.

part of data:

55,PSU1_Iin,Current,Nominal,0.00,A,'OK' 56,PSU1_Iout,Current,Nominal,0.00,A,'OK' 57,PSU1_Vout,Voltage,Critical,0.20,V,'At or Below (<=) Lower Non-Recoverable Threshold' 58,PSU1_Vin,Voltage,Critical,0.00,V,'At or Below (<=) Lower Non-Recoverable Threshold' 59,PSU1_Pin,Power Supply,Nominal,0.00,W,'OK' 60,PSU1_Pout,Power Supply,Nominal,0.00,W,'OK' 61,PSU1_Hs_Temp,Temperature,Nominal,20.00,C,'OK' 62,PSU1_Amb_Temp,Temperature,Nominal,23.00,C,'OK' 63,PSU1_Status,Power Supply,Critical,N/A,N/A,'Presence detected' 'Power Supply Failure detected' 'Power Supply input lost (AC/DC)' 'Power Supply input out-of-range, but present'

flazoon commented 3 years ago

I'm having the exact same issue. Any fix/workaround would be welcome.

bitfehler commented 3 years ago

Hi, thanks a lot for the report. The output is mighty strange, give me some time to look into this. I think there was some way to prevent multiple events from showing up (which seems to be the case here).

bitfehler commented 3 years ago

Actually, I think I have a reasonable solution for this, as we actually don't really use the event string anyways. Could you maybe run the command that produced your sample output again, but with --output-event-bitmask added (I can't find a sensor right now that outputs multiple events)?

flazoon commented 3 years ago

Hi Conrad,

Thanks for looking into this.

So this is the output line that's originally causing the issue:

The comma in the last column (before ", but present") makes the csv parser think there are more columns.

Running with --output-event-bitmask might do the trick. Here's the output: 64,PSU1_Status,Power Supply,Critical,N/A,N/A,0031h

I fixed it locally by applying the following patch:

diff --git a/collector.go b/collector.go index c6e2c51..b6982d5 100644 --- a/collector.go +++ b/collector.go @@ -304,6 +304,7 @@ func splitMonitoringOutput(impiOutput []byte, excludeSensorIds []int64) ([]senso var result []sensorData

r := csv.NewReader(bytes.NewReader(impiOutput))

But I think running with output-event-bitmask is a better solution.

Thanks, Flaviu

On Fri, Feb 12, 2021 at 8:00 AM Conrad Hoffmann notifications@github.com wrote:

Actually, I think I have a reasonable solution for this, as we actually don't really use the event string anyways. Could you maybe run the command that produced your sample output again, but with --output-event-bitmask added (I can't find a sensor right now that outputs multiple events)?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/soundcloud/ipmi_exporter/issues/62#issuecomment-778281151, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF4LAUI7DUK37SHGZIGRDCTS6VGDNANCNFSM4V63PDHQ .

bitfehler commented 3 years ago

Thanks for looking into that! I like the stricter validation, so I'll go with bitmask solution. Fix should land soon.