diff --git a/agent/smart.go b/agent/smart.go index b4f79d4c..fb6563c1 100644 --- a/agent/smart.go +++ b/agent/smart.go @@ -426,7 +426,7 @@ func (sm *SmartManager) smartctlArgs(deviceInfo *DeviceInfo, includeStandby bool } } - args = append(args, "-aj") + args = append(args, "-a", "--json=c") if includeStandby { args = append(args, "-n", "standby") @@ -688,13 +688,17 @@ func (sm *SmartManager) parseSmartForSata(output []byte) (bool, int) { // update SmartAttributes smartData.Attributes = make([]*smart.SmartAttribute, 0, len(data.AtaSmartAttributes.Table)) for _, attr := range data.AtaSmartAttributes.Table { + rawValue := uint64(attr.Raw.Value) + if parsed, ok := smart.ParseSmartRawValueString(attr.Raw.String); ok { + rawValue = parsed + } smartAttr := &smart.SmartAttribute{ ID: attr.ID, Name: attr.Name, Value: attr.Value, Worst: attr.Worst, Threshold: attr.Thresh, - RawValue: uint64(attr.Raw.Value), + RawValue: rawValue, RawString: attr.Raw.String, WhenFailed: attr.WhenFailed, } diff --git a/agent/smart_test.go b/agent/smart_test.go index 2fa969e2..98f6a926 100644 --- a/agent/smart_test.go +++ b/agent/smart_test.go @@ -89,6 +89,49 @@ func TestParseSmartForSata(t *testing.T) { } } +func TestParseSmartForSataParentheticalRawValue(t *testing.T) { + jsonPayload := []byte(`{ + "smartctl": {"exit_status": 0}, + "device": {"name": "/dev/sdz", "type": "sat"}, + "model_name": "Example", + "serial_number": "PARENTHESES123", + "firmware_version": "1.0", + "user_capacity": {"bytes": 1024}, + "smart_status": {"passed": true}, + "temperature": {"current": 25}, + "ata_smart_attributes": { + "table": [ + { + "id": 9, + "name": "Power_On_Hours", + "value": 93, + "worst": 55, + "thresh": 0, + "when_failed": "", + "raw": { + "value": 57891864217128, + "string": "39925 (212 206 0)" + } + } + ] + } + }`) + + sm := &SmartManager{SmartDataMap: make(map[string]*smart.SmartData)} + + hasData, exitStatus := sm.parseSmartForSata(jsonPayload) + require.True(t, hasData) + assert.Equal(t, 0, exitStatus) + + data, ok := sm.SmartDataMap["PARENTHESES123"] + require.True(t, ok) + require.Len(t, data.Attributes, 1) + + attr := data.Attributes[0] + assert.Equal(t, uint64(39925), attr.RawValue) + assert.Equal(t, "39925 (212 206 0)", attr.RawString) +} + func TestParseSmartForNvme(t *testing.T) { fixturePath := filepath.Join("test-data", "smart", "nvme0.json") data, err := os.ReadFile(fixturePath) @@ -198,7 +241,7 @@ func TestSmartctlArgsWithoutType(t *testing.T) { sm := &SmartManager{} args := sm.smartctlArgs(device, true) - assert.Equal(t, []string{"-aj", "-n", "standby", "/dev/sda"}, args) + assert.Equal(t, []string{"-a", "--json=c", "-n", "standby", "/dev/sda"}, args) } func TestSmartctlArgs(t *testing.T) { @@ -206,17 +249,17 @@ func TestSmartctlArgs(t *testing.T) { sataDevice := &DeviceInfo{Name: "/dev/sda", Type: "sat"} assert.Equal(t, - []string{"-d", "sat", "-aj", "-n", "standby", "/dev/sda"}, + []string{"-d", "sat", "-a", "--json=c", "-n", "standby", "/dev/sda"}, sm.smartctlArgs(sataDevice, true), ) assert.Equal(t, - []string{"-d", "sat", "-aj", "/dev/sda"}, + []string{"-d", "sat", "-a", "--json=c", "/dev/sda"}, sm.smartctlArgs(sataDevice, false), ) assert.Equal(t, - []string{"-aj", "-n", "standby"}, + []string{"-a", "--json=c", "-n", "standby"}, sm.smartctlArgs(nil, true), ) } diff --git a/internal/entities/smart/smart.go b/internal/entities/smart/smart.go index a40b85f8..48c8f1f4 100644 --- a/internal/entities/smart/smart.go +++ b/internal/entities/smart/smart.go @@ -1,6 +1,7 @@ package smart import ( + "encoding/json" "strconv" "strings" ) @@ -160,6 +161,33 @@ type RawValue struct { String string `json:"string"` } +func (r *RawValue) UnmarshalJSON(data []byte) error { + var tmp struct { + Value json.RawMessage `json:"value"` + String string `json:"string"` + } + + if err := json.Unmarshal(data, &tmp); err != nil { + return err + } + + if len(tmp.Value) > 0 { + if err := r.Value.UnmarshalJSON(tmp.Value); err != nil { + return err + } + } else { + r.Value = 0 + } + + r.String = tmp.String + + if parsed, ok := ParseSmartRawValueString(tmp.String); ok { + r.Value = SmartRawValue(parsed) + } + + return nil +} + type SmartRawValue uint64 // handles when drives report strings like "0h+0m+0.000s" or "7344 (253d 8h)" for power on hours @@ -170,61 +198,73 @@ func (v *SmartRawValue) UnmarshalJSON(data []byte) error { return nil } - if trimmed[0] != '"' { - parsed, err := strconv.ParseUint(trimmed, 0, 64) + if trimmed[0] == '"' { + valueStr, err := strconv.Unquote(trimmed) if err != nil { return err } - *v = SmartRawValue(parsed) - return nil - } - - valueStr, err := strconv.Unquote(trimmed) - if err != nil { - return err - } - if valueStr == "" { + parsed, ok := ParseSmartRawValueString(valueStr) + if ok { + *v = SmartRawValue(parsed) + return nil + } *v = 0 return nil } - if parsed, err := strconv.ParseUint(valueStr, 0, 64); err == nil { + if parsed, err := strconv.ParseUint(trimmed, 0, 64); err == nil { *v = SmartRawValue(parsed) return nil } - if idx := strings.IndexRune(valueStr, 'h'); idx >= 0 { - hoursPart := strings.TrimSpace(valueStr[:idx]) - if hoursPart == "" { - *v = 0 - return nil - } - if parsed, err := strconv.ParseFloat(hoursPart, 64); err == nil { - *v = SmartRawValue(uint64(parsed)) - return nil - } - } - - if digits := leadingDigitPrefix(valueStr); digits != "" { - if parsed, err := strconv.ParseUint(digits, 0, 64); err == nil { - *v = SmartRawValue(parsed) - return nil - } + if parsed, ok := ParseSmartRawValueString(trimmed); ok { + *v = SmartRawValue(parsed) + return nil } *v = 0 return nil } -func leadingDigitPrefix(value string) string { - var builder strings.Builder - for _, r := range value { - if r < '0' || r > '9' { - break - } - builder.WriteRune(r) +// ParseSmartRawValueString attempts to extract a numeric value from the raw value +// strings emitted by smartctl, which sometimes include human-friendly annotations +// like "7344 (253d 8h)" or "0h+0m+0.000s". It returns the parsed value and a +// boolean indicating success. +func ParseSmartRawValueString(value string) (uint64, bool) { + value = strings.TrimSpace(value) + if value == "" { + return 0, false } - return builder.String() + + if parsed, err := strconv.ParseUint(value, 0, 64); err == nil { + return parsed, true + } + + if idx := strings.IndexRune(value, 'h'); idx > 0 { + hoursPart := strings.TrimSpace(value[:idx]) + if hoursPart != "" { + if parsed, err := strconv.ParseFloat(hoursPart, 64); err == nil { + return uint64(parsed), true + } + } + } + + for i := 0; i < len(value); i++ { + if value[i] < '0' || value[i] > '9' { + continue + } + end := i + 1 + for end < len(value) && value[end] >= '0' && value[end] <= '9' { + end++ + } + digits := value[i:end] + if parsed, err := strconv.ParseUint(digits, 10, 64); err == nil { + return parsed, true + } + i = end + } + + return 0, false } // type PowerOnTimeInfo struct { diff --git a/internal/entities/smart/smart_test.go b/internal/entities/smart/smart_test.go index 67be4b61..f1b19bb8 100644 --- a/internal/entities/smart/smart_test.go +++ b/internal/entities/smart/smart_test.go @@ -3,28 +3,60 @@ package smart import ( "encoding/json" "testing" + + "github.com/stretchr/testify/assert" ) func TestSmartRawValueUnmarshalDuration(t *testing.T) { input := []byte(`{"value":"62312h+33m+50.907s","string":"62312h+33m+50.907s"}`) var raw RawValue - if err := json.Unmarshal(input, &raw); err != nil { - t.Fatalf("unexpected error unmarshalling raw value: %v", err) - } + err := json.Unmarshal(input, &raw) + assert.NoError(t, err) - if uint64(raw.Value) != 62312 { - t.Fatalf("expected hours to be 62312, got %d", raw.Value) - } + assert.EqualValues(t, 62312, raw.Value) } func TestSmartRawValueUnmarshalNumericString(t *testing.T) { input := []byte(`{"value":"7344","string":"7344"}`) var raw RawValue - if err := json.Unmarshal(input, &raw); err != nil { - t.Fatalf("unexpected error unmarshalling numeric string: %v", err) - } + err := json.Unmarshal(input, &raw) + assert.NoError(t, err) - if uint64(raw.Value) != 7344 { - t.Fatalf("expected hours to be 7344, got %d", raw.Value) - } + assert.EqualValues(t, 7344, raw.Value) +} + +func TestSmartRawValueUnmarshalParenthetical(t *testing.T) { + input := []byte(`{"value":"39925 (212 206 0)","string":"39925 (212 206 0)"}`) + var raw RawValue + err := json.Unmarshal(input, &raw) + assert.NoError(t, err) + + assert.EqualValues(t, 39925, raw.Value) +} + +func TestSmartRawValueUnmarshalDurationWithFractions(t *testing.T) { + input := []byte(`{"value":"2748h+31m+49.560s","string":"2748h+31m+49.560s"}`) + var raw RawValue + err := json.Unmarshal(input, &raw) + assert.NoError(t, err) + + assert.EqualValues(t, 2748, raw.Value) +} + +func TestSmartRawValueUnmarshalParentheticalRawValue(t *testing.T) { + input := []byte(`{"value":57891864217128,"string":"39925 (212 206 0)"}`) + var raw RawValue + err := json.Unmarshal(input, &raw) + assert.NoError(t, err) + + assert.EqualValues(t, 39925, raw.Value) +} + +func TestSmartRawValueUnmarshalDurationRawValue(t *testing.T) { + input := []byte(`{"value":57891864217128,"string":"2748h+31m+49.560s"}`) + var raw RawValue + err := json.Unmarshal(input, &raw) + assert.NoError(t, err) + + assert.EqualValues(t, 2748, raw.Value) } diff --git a/supplemental/CHANGELOG.md b/supplemental/CHANGELOG.md index 50fa5cd9..5953cf57 100644 --- a/supplemental/CHANGELOG.md +++ b/supplemental/CHANGELOG.md @@ -1,3 +1,11 @@ +## 0.15.3 + +- Improve parsing of edge case S.M.A.R.T. power on times. (#1347) + +## 0.15.2 + +- Improve S.M.A.R.T. device detection logic (fix regression in 0.15.1) (#1345) + ## 0.15.1 - Add `SMART_DEVICES` environment variable to specify devices and types. (#373, #1335)