Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cpufreq plugin #4215

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions plugins/inputs/all/all.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
_ "github.com/influxdata/telegraf/plugins/inputs/couchbase"
_ "github.com/influxdata/telegraf/plugins/inputs/couchdb"
_ "github.com/influxdata/telegraf/plugins/inputs/cpu"
_ "github.com/influxdata/telegraf/plugins/inputs/cpufreq"
_ "github.com/influxdata/telegraf/plugins/inputs/dcos"
_ "github.com/influxdata/telegraf/plugins/inputs/disk"
_ "github.com/influxdata/telegraf/plugins/inputs/diskio"
Expand Down
29 changes: 29 additions & 0 deletions plugins/inputs/cpufreq/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# CPUFreq Input Plugin For Telegraf Agent

The CPUFreq plugin collects the current CPU's frequency. This plugin work only with linux.

## Configuration

```toml
[[inputs.cpufreq]]
## Path for sysfs filesystem.
## See https://www.kernel.org/doc/Documentation/filesystems/sysfs.txt
## Defaults:
# host_sys = "/sys"
## Gather CPU throttles per core
## Defaults:
# gather_throttles = false
```

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a Metric section describing the fields similar to #8342!?

## Example output

```
> cpufreq,cpu=0,host=server01 cur_freq=3756293000i,max_freq=3900000000i,min_freq=800000000i,throttle_count=0i 1527789803000000000
> cpufreq,cpu=1,host=server01 cur_freq=3735119000i,max_freq=3900000000i,min_freq=800000000i,throttle_count=0i 1527789803000000000
> cpufreq,cpu=2,host=server01 cur_freq=3786381000i,max_freq=3900000000i,min_freq=800000000i,throttle_count=0i 1527789803000000000
> cpufreq,cpu=3,host=server01 cur_freq=3823190000i,max_freq=3900000000i,min_freq=800000000i,throttle_count=0i 1527789803000000000
> cpufreq,cpu=4,host=server01 cur_freq=3780804000i,max_freq=3900000000i,min_freq=800000000i,throttle_count=0i 1527789803000000000
> cpufreq,cpu=5,host=server01 cur_freq=3801758000i,max_freq=3900000000i,min_freq=800000000i,throttle_count=0i 1527789803000000000
> cpufreq,cpu=6,host=server01 cur_freq=3839194000i,max_freq=3900000000i,min_freq=800000000i,throttle_count=0i 1527789803000000000
> cpufreq,cpu=7,host=server01 cur_freq=3877989000i,max_freq=3900000000i,min_freq=800000000i,throttle_count=0i 1527789803000000000
Comment on lines +21 to +28
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about renaming cur_freq, max_freq, min_freq to current, max, min? The freq_ part seems superfluous given that the metric is named cpufreq.

```
116 changes: 116 additions & 0 deletions plugins/inputs/cpufreq/cpufreq.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
package cpufreq
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this only works on linux, please make sure it only builds on linux.

Suggested change
package cpufreq
// +build linux
package cpufreq

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@R4scal
Moreover, you will need to add cpufreq_notlinux.go file with following content:

// +build !linux

package cpufreq

To build (but not include in binary) your plugin succesfully for non-Linux systems.

Look at systtat plugin: https://github.com/influxdata/telegraf/tree/f7d94430d29d15c42d8d186c99bfd065b9a21fa0/plugins/inputs/sysstat


import (
"io/ioutil"
"os"
"path"
"path/filepath"
"strconv"
"strings"

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

const defaultHostSys = "/sys"

type CPUFreq struct {
PathSysfs string `toml:"host_sys"`
GatherThrottles bool `toml:"gather_throttles"`
}

var sampleConfig = `
## Path for sysfs filesystem.
## See https://www.kernel.org/doc/Documentation/filesystems/sysfs.txt
## Defaults:
# host_sys = "/sys"
## Gather CPU throttles per core
## Defaults:
# gather_throttles = false
`

func (g *CPUFreq) SampleConfig() string {
return sampleConfig
}

func (g *CPUFreq) Description() string {
return "Read specific statistics per cgroup"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably

Suggested change
return "Read specific statistics per cgroup"
return "Collect current CPU's frequencies"

would be better, wouldn't it?

}

func (g *CPUFreq) Gather(acc telegraf.Accumulator) error {

if g.PathSysfs == "" {
if os.Getenv("HOST_SYS") != "" {
g.PathSysfs = os.Getenv("HOST_SYS")
Comment on lines +43 to +44
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is HOST_SYS a standard environment variable? I didn't find any documentation about it. Can you point me to a reference?
If it is not, remove this section as an alternative prefix can still be specified in the config.

} else {
g.PathSysfs = defaultHostSys
}
}
Comment on lines +42 to +48
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this to an Init() function that is called once when the plugin is constructed. This will avoid doing it over and over again.


cpus, err := filepath.Glob(path.Join(g.PathSysfs, "devices/system/cpu/cpu[0-9]*"))
if err != nil {
acc.AddError(err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filepath.Glob will only return an error for invalid patterns according to its documentation. Therefore, and because you won't have cpus anyway, we should simply return err here.

}

var value uint64
// cpu loop
for _, cpu := range cpus {
fileds := make(map[string]interface{})
tags := make(map[string]string)

_, cpuName := filepath.Split(cpu)
cpuNum := strings.TrimPrefix(cpuName, "cpu")

tags["cpu"] = cpuNum

// sysfs cpufreq values are kHz, thus multiply by 1000 to export base units (hz).
if _, err := os.Stat(filepath.Join(cpu, "cpufreq")); os.IsNotExist(err) {
acc.AddError(err)
} else {
Comment on lines +68 to +69
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put a `continue here and remove the else saving one indentation level.

if value, err = readUintFromFile(filepath.Join(cpu, "cpufreq", "scaling_cur_freq")); err != nil {
acc.AddError(err)
continue
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for an else here.

fileds["cur_freq"] = uint64(value) * 1000
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo fields instead of fileds.

}
if value, err = readUintFromFile(filepath.Join(cpu, "cpufreq", "scaling_min_freq")); err != nil {
acc.AddError(err)
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use continue as with the previous condition. Maybe it would be nice to avoid the code duplication at all and construct a for loop running over {"min", "max", "cur"}.

fileds["min_freq"] = uint64(value) * 1000
}
if value, err = readUintFromFile(filepath.Join(cpu, "cpufreq", "scaling_max_freq")); err != nil {
acc.AddError(err)
} else {
fileds["max_freq"] = uint64(value) * 1000
}
}

if g.GatherThrottles {
if value, err := readUintFromFile(filepath.Join(cpu, "thermal_throttle", "core_throttle_count")); err != nil {
acc.AddError(err)
} else {
fileds["throttle_count"] = uint64(value)
}
}

acc.AddFields("cpufreq", fileds, tags)
}

return nil
}

func init() {
inputs.Add("cpufreq", func() telegraf.Input { return &CPUFreq{} })
}

func readUintFromFile(path string) (uint64, error) {
data, err := ioutil.ReadFile(path)
if err != nil {
return 0, err
}
value, err := strconv.ParseUint(strings.TrimSpace(string(data)), 10, 64)
if err != nil {
return 0, err
}
return value, nil
}
131 changes: 131 additions & 0 deletions plugins/inputs/cpufreq/cpufreq_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
package cpufreq

import (
"testing"

"github.com/influxdata/telegraf/testutil"
"github.com/stretchr/testify/require"
)

func TestCPUFreq_NoThrottles(t *testing.T) {
var acc testutil.Accumulator
var cpufreq = &CPUFreq{
PathSysfs: "testdata",
}

err := acc.GatherError(cpufreq.Gather)
require.NoError(t, err)

// CPU 0 Core 0
acc.AssertContainsTaggedFields(t,
"cpufreq",
map[string]interface{}{
"cur_freq": uint64(2597101000),
"max_freq": uint64(3400000000),
"min_freq": uint64(1200000000),
},
map[string]string{
"cpu": "0",
},
)
// CPU 1 Core 0
acc.AssertContainsTaggedFields(t,
"cpufreq",
map[string]interface{}{
"cur_freq": uint64(2597027000),
"max_freq": uint64(3400000000),
"min_freq": uint64(1200000000),
},
map[string]string{
"cpu": "1",
},
)
// CPU 0 Core 1
acc.AssertContainsTaggedFields(t,
"cpufreq",
map[string]interface{}{
"cur_freq": uint64(2597328000),
"max_freq": uint64(3400000000),
"min_freq": uint64(1200000000),
},
map[string]string{
"cpu": "2",
},
)
// CPU 1 Core 1
acc.AssertContainsTaggedFields(t,
"cpufreq",
map[string]interface{}{
"cur_freq": uint64(2597176000),
"max_freq": uint64(3400000000),
"min_freq": uint64(1200000000),
},
map[string]string{
"cpu": "3",
},
)
}

func TestCPUFreq_WithThrottles(t *testing.T) {
var acc testutil.Accumulator
var cpufreq = &CPUFreq{
PathSysfs: "testdata",
GatherThrottles: true,
}

err := acc.GatherError(cpufreq.Gather)
require.NoError(t, err)

// CPU 0 Core 0
acc.AssertContainsTaggedFields(t,
"cpufreq",
map[string]interface{}{
"cur_freq": uint64(2597101000),
"max_freq": uint64(3400000000),
"min_freq": uint64(1200000000),
"throttle_count": uint64(0),
},
map[string]string{
"cpu": "0",
},
)
// CPU 1 Core 0
acc.AssertContainsTaggedFields(t,
"cpufreq",
map[string]interface{}{
"cur_freq": uint64(2597027000),
"max_freq": uint64(3400000000),
"min_freq": uint64(1200000000),
"throttle_count": uint64(0),
},
map[string]string{
"cpu": "1",
},
)
// CPU 0 Core 1
acc.AssertContainsTaggedFields(t,
"cpufreq",
map[string]interface{}{
"cur_freq": uint64(2597328000),
"max_freq": uint64(3400000000),
"min_freq": uint64(1200000000),
"throttle_count": uint64(0),
},
map[string]string{
"cpu": "2",
},
)
// CPU 1 Core 1
acc.AssertContainsTaggedFields(t,
"cpufreq",
map[string]interface{}{
"cur_freq": uint64(2597176000),
"max_freq": uint64(3400000000),
"min_freq": uint64(1200000000),
"throttle_count": uint64(100),
},
map[string]string{
"cpu": "3",
},
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
2597101
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
3400000
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1200000
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
2597027
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
3400000
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1200000
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
2597328
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
3400000
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1200000
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
2597176
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
3400000
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1200000
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
100
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1