Skip to content

Commit

Permalink
Change hddtemp to always put temperature in temperature field (#1905)
Browse files Browse the repository at this point in the history
Added unit tests for the changes

Fixes #1904
  • Loading branch information
jonaz authored and sparrc committed Dec 13, 2016
1 parent c4c13c4 commit 74d8aef
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
- [#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]

Expand Down
23 changes: 22 additions & 1 deletion plugins/inputs/hddtemp/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
```
15 changes: 11 additions & 4 deletions plugins/inputs/hddtemp/go-hddtemp/hddtemp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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),
Expand Down
14 changes: 7 additions & 7 deletions plugins/inputs/hddtemp/go-hddtemp/hddtemp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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")
Expand All @@ -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",
Expand All @@ -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",
Expand Down
12 changes: 10 additions & 2 deletions plugins/inputs/hddtemp/hddtemp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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)
Expand Down
80 changes: 80 additions & 0 deletions plugins/inputs/hddtemp/hddtemp_test.go
Original file line number Diff line number Diff line change
@@ -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)
}

}

0 comments on commit 74d8aef

Please sign in to comment.