Change hddtemp to always put temperature in temperature field (influx…

…data#1905)

Added unit tests for the changes

Fixes influxdata#1904

CHANGELOG.md

@@ -69,6 +69,7 @@ plugins, not just statsd.
 - [#1730](https://github.com/influxdata/telegraf/issues/1730): Fix win_perf_counters not gathering non-English counters.
 - [#2061](https://github.com/influxdata/telegraf/issues/2061): Fix panic when file stat info cannot be collected due to permissions or other issue(s).
 - [#2045](https://github.com/influxdata/telegraf/issues/2045): Graylog output should set short_message field.
+- [#1904](https://github.com/influxdata/telegraf/issues/1904): Hddtemp always put the value in the field temperature.
 
 ## v1.1.2 [2016-12-12]
 

plugins/inputs/hddtemp/README.md

@@ -8,7 +8,7 @@ Hddtemp should be installed and its daemon running
 
 ## Configuration
 
-```
+```toml
 [[inputs.hddtemp]]
 ## By default, telegraf gathers temps data from all disks detected by the
 ## hddtemp.
@@ -20,3 +20,24 @@ Hddtemp should be installed and its daemon running
 # address = "127.0.0.1:7634"
 # devices = ["sda", "*"]
 ```
+
+## Measurements
+
+- hddtemp
+  - temperature
+
+Tags:
+- device
+- model
+- unit
+- status
+
+
+
+## Example output
+
+```
+> hddtemp,unit=C,status=,host=server1,device=sdb,model=WDC\ WD740GD-00FLA1 temperature=43i 1481655647000000000
+> hddtemp,device=sdc,model=SAMSUNG\ HD103UI,unit=C,status=,host=server1 temperature=38i 148165564700000000
+> hddtemp,device=sdd,model=SAMSUNG\ HD103UI,unit=C,status=,host=server1 temperature=36i 1481655647000000000
+```

plugins/inputs/hddtemp/go-hddtemp/hddtemp.go

@@ -8,20 +8,27 @@ import (
 	"strings"
 )
 
-type disk struct {
+type Disk struct {
 	DeviceName  string
 	Model       string
 	Temperature int32
 	Unit        string
 	Status      string
 }
 
-func Fetch(address string) ([]disk, error) {
+type hddtemp struct {
+}
+
+func New() *hddtemp {
+	return &hddtemp{}
+}
+
+func (h *hddtemp) Fetch(address string) ([]Disk, error) {
 	var (
 		err    error
 		conn   net.Conn
 		buffer bytes.Buffer
-		disks  []disk
+		disks  []Disk
 	)
 
 	if conn, err = net.Dial("tcp", address); err != nil {
@@ -48,7 +55,7 @@ func Fetch(address string) ([]disk, error) {
 			status = temperatureField
 		}
 
-		disks = append(disks, disk{
+		disks = append(disks, Disk{
 			DeviceName:  device,
 			Model:       fields[offset+2],
 			Temperature: int32(temperature),

plugins/inputs/hddtemp/go-hddtemp/hddtemp_test.go

@@ -10,13 +10,13 @@ func TestFetch(t *testing.T) {
 	l := serve(t, []byte("|/dev/sda|foobar|36|C|"))
 	defer l.Close()
 
-	disks, err := Fetch(l.Addr().String())
+	disks, err := New().Fetch(l.Addr().String())
 
 	if err != nil {
 		t.Error("expecting err to be nil")
 	}
 
-	expected := []disk{
+	expected := []Disk{
 		{
 			DeviceName:  "sda",
 			Model:       "foobar",
@@ -31,7 +31,7 @@ func TestFetch(t *testing.T) {
 }
 
 func TestFetchWrongAddress(t *testing.T) {
-	_, err := Fetch("127.0.0.1:1")
+	_, err := New().Fetch("127.0.0.1:1")
 
 	if err == nil {
 		t.Error("expecting err to be non-nil")
@@ -42,13 +42,13 @@ func TestFetchStatus(t *testing.T) {
 	l := serve(t, []byte("|/dev/sda|foobar|SLP|C|"))
 	defer l.Close()
 
-	disks, err := Fetch(l.Addr().String())
+	disks, err := New().Fetch(l.Addr().String())
 
 	if err != nil {
 		t.Error("expecting err to be nil")
 	}
 
-	expected := []disk{
+	expected := []Disk{
 		{
 			DeviceName:  "sda",
 			Model:       "foobar",
@@ -67,13 +67,13 @@ func TestFetchTwoDisks(t *testing.T) {
 	l := serve(t, []byte("|/dev/hda|ST380011A|46|C||/dev/hdd|ST340016A|SLP|*|"))
 	defer l.Close()
 
-	disks, err := Fetch(l.Addr().String())
+	disks, err := New().Fetch(l.Addr().String())
 
 	if err != nil {
 		t.Error("expecting err to be nil")
 	}
 
-	expected := []disk{
+	expected := []Disk{
 		{
 			DeviceName:  "hda",
 			Model:       "ST380011A",

plugins/inputs/hddtemp/hddtemp.go

@@ -13,6 +13,11 @@ const defaultAddress = "127.0.0.1:7634"
 type HDDTemp struct {
 	Address string
 	Devices []string
+	fetcher Fetcher
+}
+
+type Fetcher interface {
+	Fetch(address string) ([]gohddtemp.Disk, error)
 }
 
 func (_ *HDDTemp) Description() string {
@@ -36,7 +41,10 @@ func (_ *HDDTemp) SampleConfig() string {
 }
 
 func (h *HDDTemp) Gather(acc telegraf.Accumulator) error {
-	disks, err := gohddtemp.Fetch(h.Address)
+	if h.fetcher == nil {
+		h.fetcher = gohddtemp.New()
+	}
+	disks, err := h.fetcher.Fetch(h.Address)
 
 	if err != nil {
 		return err
@@ -53,7 +61,7 @@ func (h *HDDTemp) Gather(acc telegraf.Accumulator) error {
 				}
 
 				fields := map[string]interface{}{
-					disk.DeviceName: disk.Temperature,
+					"temperature": disk.Temperature,
 				}
 
 				acc.AddFields("hddtemp", fields, tags)

plugins/inputs/hddtemp/hddtemp_test.go

@@ -0,0 +1,80 @@
+package hddtemp
+
+import (
+	"testing"
+
+	hddtemp "github.com/influxdata/telegraf/plugins/inputs/hddtemp/go-hddtemp"
+	"github.com/influxdata/telegraf/testutil"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+type mockFetcher struct {
+}
+
+func (h *mockFetcher) Fetch(address string) ([]hddtemp.Disk, error) {
+	return []hddtemp.Disk{
+		hddtemp.Disk{
+			DeviceName:  "Disk1",
+			Model:       "Model1",
+			Temperature: 13,
+			Unit:        "C",
+		},
+		hddtemp.Disk{
+			DeviceName:  "Disk2",
+			Model:       "Model2",
+			Temperature: 14,
+			Unit:        "C",
+		},
+	}, nil
+
+}
+func newMockFetcher() *mockFetcher {
+	return &mockFetcher{}
+}
+
+func TestFetch(t *testing.T) {
+	hddtemp := &HDDTemp{
+		fetcher: newMockFetcher(),
+		Devices: []string{"*"},
+	}
+
+	acc := &testutil.Accumulator{}
+	err := hddtemp.Gather(acc)
+
+	require.NoError(t, err)
+	assert.Equal(t, acc.NFields(), 2)
+
+	var tests = []struct {
+		fields map[string]interface{}
+		tags   map[string]string
+	}{
+		{
+			map[string]interface{}{
+				"temperature": int32(13),
+			},
+			map[string]string{
+				"device": "Disk1",
+				"model":  "Model1",
+				"unit":   "C",
+				"status": "",
+			},
+		},
+		{
+			map[string]interface{}{
+				"temperature": int32(14),
+			},
+			map[string]string{
+				"device": "Disk2",
+				"model":  "Model2",
+				"unit":   "C",
+				"status": "",
+			},
+		},
+	}
+
+	for _, test := range tests {
+		acc.AssertContainsTaggedFields(t, "hddtemp", test.fields, test.tags)
+	}
+
+}