Skip to content

Commit

Permalink
Fix SMART plugin to recognize all devices from config (#8374)
Browse files Browse the repository at this point in the history 
(cherry picked from commit ff0a8c2)
  • Loading branch information
@p-zak @ssoroka
p-zak authored and ssoroka committed Nov 13, 2020
1 parent de4d70f commit 0c797b6
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 57 deletions.
71 changes: 24 additions & 47 deletions plugins/inputs/smart/smart.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,13 @@ import (
"sync"
"syscall"
"time"
"unicode"

"github.com/influxdata/telegraf"
"github.com/influxdata/telegraf/internal"
"github.com/influxdata/telegraf/plugins/inputs"
)

const IntelVID = "0x8086"
const intelVID = "0x8086"

var (
// Device Model: APPLE SSD SM256E
Expand Down Expand Up @@ -55,7 +54,7 @@ var (

// vid : 0x8086
// sn : CFGT53260XSP8011P
nvmeIdCtrlExpressionPattern = regexp.MustCompile(`^([\w\s]+):([\s\w]+)`)
nvmeIDCtrlExpressionPattern = regexp.MustCompile(`^([\w\s]+):([\s\w]+)`)

deviceFieldIds = map[string]string{
"1": "read_error_rate",
Expand Down Expand Up @@ -267,13 +266,7 @@ var (
}
)

type NVMeDevice struct {
name string
vendorID string
model string
serialNumber string
}

// Smart plugin reads metrics from storage devices supporting S.M.A.R.T.
type Smart struct {
Path string `toml:"path"` //deprecated - to keep backward compatibility
PathSmartctl string `toml:"path_smartctl"`
Expand All @@ -288,6 +281,13 @@ type Smart struct {
Log telegraf.Logger `toml:"-"`
}

type nvmeDevice struct {
name string
vendorID string
model string
serialNumber string
}

var sampleConfig = `
## Optionally specify the path to the smartctl executable
# path_smartctl = "/usr/bin/smartctl"
Expand Down Expand Up @@ -330,20 +330,23 @@ var sampleConfig = `
# timeout = "30s"
`

func NewSmart() *Smart {
func newSmart() *Smart {
return &Smart{
Timeout: internal.Duration{Duration: time.Second * 30},
}
}

// SampleConfig returns sample configuration for this plugin.
func (m *Smart) SampleConfig() string {
return sampleConfig
}

// Description returns the plugin description.
func (m *Smart) Description() string {
return "Read metrics from storage devices supporting S.M.A.R.T."
}

// Init performs one time setup of the plugin and returns an error if the configuration is invalid.
func (m *Smart) Init() error {
//if deprecated `path` (to smartctl binary) is provided in config and `path_smartctl` override does not exist
if len(m.Path) > 0 && len(m.PathSmartctl) == 0 {
Expand Down Expand Up @@ -377,6 +380,7 @@ func (m *Smart) Init() error {
return nil
}

// Gather takes in an accumulator and adds the metrics that the SMART tools gather.
func (m *Smart) Gather(acc telegraf.Accumulator) error {
var err error
var scannedNVMeDevices []string
Expand All @@ -387,8 +391,6 @@ func (m *Smart) Gather(acc telegraf.Accumulator) error {
isVendorExtension := len(m.EnableExtensions) != 0

if len(m.Devices) != 0 {
devicesFromConfig = excludeWrongDeviceNames(devicesFromConfig)

m.getAttributes(acc, devicesFromConfig)

// if nvme-cli is present, vendor specific attributes can be gathered
Expand Down Expand Up @@ -418,31 +420,6 @@ func (m *Smart) Gather(acc telegraf.Accumulator) error {
return nil
}

// validate and exclude not correct config device names to avoid unwanted behaviours
func excludeWrongDeviceNames(devices []string) []string {
validSigns := map[string]struct{}{
" ": {},
"/": {},
"\\": {},
"-": {},
",": {},
}
var wrongDevices []string

for _, device := range devices {
for _, char := range device {
if unicode.IsLetter(char) || unicode.IsNumber(char) {
continue
}
if _, exist := validSigns[string(char)]; exist {
continue
}
wrongDevices = append(wrongDevices, device)
}
}
return difference(devices, wrongDevices)
}

func (m *Smart) scanAllDevices(ignoreExcludes bool) ([]string, []string, error) {
// this will return all devices (including NVMe devices) for smartctl version >= 7.0
// for older versions this will return non NVMe devices
Expand Down Expand Up @@ -540,28 +517,28 @@ func (m *Smart) getVendorNVMeAttributes(acc telegraf.Accumulator, devices []stri
for _, device := range NVMeDevices {
if contains(m.EnableExtensions, "auto-on") {
switch device.vendorID {
case IntelVID:
case intelVID:
wg.Add(1)
go gatherIntelNVMeDisk(acc, m.Timeout, m.UseSudo, m.PathNVMe, device, &wg)
}
} else if contains(m.EnableExtensions, "Intel") && device.vendorID == IntelVID {
} else if contains(m.EnableExtensions, "Intel") && device.vendorID == intelVID {
wg.Add(1)
go gatherIntelNVMeDisk(acc, m.Timeout, m.UseSudo, m.PathNVMe, device, &wg)
}
}
wg.Wait()
}

func getDeviceInfoForNVMeDisks(acc telegraf.Accumulator, devices []string, nvme string, timeout internal.Duration, useSudo bool) []NVMeDevice {
var NVMeDevices []NVMeDevice
func getDeviceInfoForNVMeDisks(acc telegraf.Accumulator, devices []string, nvme string, timeout internal.Duration, useSudo bool) []nvmeDevice {
var NVMeDevices []nvmeDevice

for _, device := range devices {
vid, sn, mn, err := gatherNVMeDeviceInfo(nvme, device, timeout, useSudo)
if err != nil {
acc.AddError(fmt.Errorf("cannot find device info for %s device", device))
continue
}
newDevice := NVMeDevice{
newDevice := nvmeDevice{
name: device,
vendorID: vid,
model: mn,
Expand Down Expand Up @@ -593,7 +570,7 @@ func findNVMeDeviceInfo(output string) (string, string, string, error) {
for scanner.Scan() {
line := scanner.Text()

if matches := nvmeIdCtrlExpressionPattern.FindStringSubmatch(line); len(matches) > 2 {
if matches := nvmeIDCtrlExpressionPattern.FindStringSubmatch(line); len(matches) > 2 {
matches[1] = strings.TrimSpace(matches[1])
matches[2] = strings.TrimSpace(matches[2])
if matches[1] == "vid" {
Expand All @@ -612,7 +589,7 @@ func findNVMeDeviceInfo(output string) (string, string, string, error) {
return vid, sn, mn, nil
}

func gatherIntelNVMeDisk(acc telegraf.Accumulator, timeout internal.Duration, usesudo bool, nvme string, device NVMeDevice, wg *sync.WaitGroup) {
func gatherIntelNVMeDisk(acc telegraf.Accumulator, timeout internal.Duration, usesudo bool, nvme string, device nvmeDevice, wg *sync.WaitGroup) {
defer wg.Done()

args := []string{"intel", "smart-log-add"}
Expand Down Expand Up @@ -966,7 +943,7 @@ func parseTemperature(fields, deviceFields map[string]interface{}, str string) e
return nil
}

func parseTemperatureSensor(fields, deviceFields map[string]interface{}, str string) error {
func parseTemperatureSensor(fields, _ map[string]interface{}, str string) error {
var temp int64
if _, err := fmt.Sscanf(str, "%d C", &temp); err != nil {
return err
Expand All @@ -993,7 +970,7 @@ func init() {
_ = os.Setenv("LC_NUMERIC", "en_US.UTF-8")

inputs.Add("smart", func() telegraf.Input {
m := NewSmart()
m := newSmart()
m.Nocheck = "standby"
return m
})
Expand Down
13 changes: 3 additions & 10 deletions plugins/inputs/smart/smart_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
)

func TestGatherAttributes(t *testing.T) {
s := NewSmart()
s := newSmart()
s.Attributes = true

assert.Equal(t, time.Second*30, s.Timeout.Duration)
Expand Down Expand Up @@ -78,7 +78,7 @@ func TestGatherAttributes(t *testing.T) {
}

func TestGatherNoAttributes(t *testing.T) {
s := NewSmart()
s := newSmart()
s.Attributes = false

assert.Equal(t, time.Second*30, s.Timeout.Duration)
Expand Down Expand Up @@ -244,7 +244,7 @@ func TestGatherIntelNvme(t *testing.T) {
var (
acc = &testutil.Accumulator{}
wg = &sync.WaitGroup{}
device = NVMeDevice{
device = nvmeDevice{
name: "nvme0",
model: mockModel,
serialNumber: mockSerial,
Expand Down Expand Up @@ -275,13 +275,6 @@ func Test_checkForNVMeDevices(t *testing.T) {
assert.Equal(t, expectedNVMeDevices, resultNVMeDevices)
}

func Test_excludeWrongDeviceNames(t *testing.T) {
devices := []string{"/dev/sda", "/dev/nvme -d nvme", "/dev/sda1 -d megaraid,1", "/dev/sda ; ./suspicious_script.sh"}
validDevices := []string{"/dev/sda", "/dev/nvme -d nvme", "/dev/sda1 -d megaraid,1"}
result := excludeWrongDeviceNames(devices)
assert.Equal(t, validDevices, result)
}

func Test_contains(t *testing.T) {
devices := []string{"/dev/sda", "/dev/nvme1"}
device := "/dev/nvme1"
Expand Down

0 comments on commit 0c797b6

Please sign in to comment.