Skip to content

Commit

Permalink
Add configurable timeout to varnish input (influxdata#5214)
Browse files Browse the repository at this point in the history
  • Loading branch information
kamsz authored and Mathieu Lecarme committed Apr 17, 2020
1 parent ec83bcf commit 87d1804
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 12 deletions.
4 changes: 2 additions & 2 deletions plugins/inputs/activemq/activemq.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ var sampleConfig = `
## Required ActiveMQ port
# port = 8161
## Credentials for basic HTTP authentication
# username = "admin"
# password = "admin"
Expand All @@ -101,7 +101,7 @@ var sampleConfig = `
## Maximum time to receive response.
# response_timeout = "5s"
## Optional TLS Config
# tls_ca = "/etc/telegraf/ca.pem"
# tls_cert = "/etc/telegraf/cert.pem"
Expand Down
15 changes: 11 additions & 4 deletions plugins/inputs/varnish/varnish.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,23 @@ import (
"github.com/influxdata/telegraf/plugins/inputs"
)

type runner func(cmdName string, UseSudo bool, InstanceName string) (*bytes.Buffer, error)
type runner func(cmdName string, UseSudo bool, InstanceName string, Timeout internal.Duration) (*bytes.Buffer, error)

// Varnish is used to store configuration values
type Varnish struct {
Stats []string
Binary string
UseSudo bool
InstanceName string
Timeout internal.Duration

filter filter.Filter
run runner
}

var defaultStats = []string{"MAIN.cache_hit", "MAIN.cache_miss", "MAIN.uptime"}
var defaultBinary = "/usr/bin/varnishstat"
var defaultTimeout = internal.Duration{Duration: time.Second}

var sampleConfig = `
## If running as a restricted user you can prepend sudo for additional access:
Expand All @@ -49,6 +51,9 @@ var sampleConfig = `
## Optional name for the varnish instance (or working directory) to query
## Usually appened after -n in varnish cli
# instance_name = instanceName
## Timeout for varnishstat command
# timeout = "1s"
`

func (s *Varnish) Description() string {
Expand All @@ -61,7 +66,7 @@ func (s *Varnish) SampleConfig() string {
}

// Shell out to varnish_stat and return the output
func varnishRunner(cmdName string, UseSudo bool, InstanceName string) (*bytes.Buffer, error) {
func varnishRunner(cmdName string, UseSudo bool, InstanceName string, Timeout internal.Duration) (*bytes.Buffer, error) {
cmdArgs := []string{"-1"}

if InstanceName != "" {
Expand All @@ -78,7 +83,8 @@ func varnishRunner(cmdName string, UseSudo bool, InstanceName string) (*bytes.Bu

var out bytes.Buffer
cmd.Stdout = &out
err := internal.RunTimeout(cmd, time.Millisecond*500)

err := internal.RunTimeout(cmd, Timeout.Duration)
if err != nil {
return &out, fmt.Errorf("error running varnishstat: %s", err)
}
Expand Down Expand Up @@ -109,7 +115,7 @@ func (s *Varnish) Gather(acc telegraf.Accumulator) error {
}
}

out, err := s.run(s.Binary, s.UseSudo, s.InstanceName)
out, err := s.run(s.Binary, s.UseSudo, s.InstanceName, s.Timeout)
if err != nil {
return fmt.Errorf("error gathering metrics: %s", err)
}
Expand Down Expand Up @@ -170,6 +176,7 @@ func init() {
Binary: defaultBinary,
UseSudo: false,
InstanceName: "",
Timeout: defaultTimeout,
}
})
}
14 changes: 8 additions & 6 deletions plugins/inputs/varnish/varnish_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,23 @@ import (
"fmt"
"strings"
"testing"
"time"

"github.com/influxdata/telegraf/internal"
"github.com/influxdata/telegraf/testutil"
"github.com/stretchr/testify/assert"
)

func fakeVarnishStat(output string, useSudo bool, InstanceName string) func(string, bool, string) (*bytes.Buffer, error) {
return func(string, bool, string) (*bytes.Buffer, error) {
func fakeVarnishStat(output string, useSudo bool, InstanceName string, Timeout internal.Duration) func(string, bool, string, internal.Duration) (*bytes.Buffer, error) {
return func(string, bool, string, internal.Duration) (*bytes.Buffer, error) {
return bytes.NewBuffer([]byte(output)), nil
}
}

func TestGather(t *testing.T) {
acc := &testutil.Accumulator{}
v := &Varnish{
run: fakeVarnishStat(smOutput, false, ""),
run: fakeVarnishStat(smOutput, false, "", internal.Duration{Duration: time.Second}),
Stats: []string{"*"},
}
v.Gather(acc)
Expand All @@ -37,7 +39,7 @@ func TestGather(t *testing.T) {
func TestParseFullOutput(t *testing.T) {
acc := &testutil.Accumulator{}
v := &Varnish{
run: fakeVarnishStat(fullOutput, true, ""),
run: fakeVarnishStat(fullOutput, true, "", internal.Duration{Duration: time.Second}),
Stats: []string{"*"},
}
err := v.Gather(acc)
Expand All @@ -52,7 +54,7 @@ func TestParseFullOutput(t *testing.T) {
func TestFilterSomeStats(t *testing.T) {
acc := &testutil.Accumulator{}
v := &Varnish{
run: fakeVarnishStat(fullOutput, false, ""),
run: fakeVarnishStat(fullOutput, false, "", internal.Duration{Duration: time.Second}),
Stats: []string{"MGT.*", "VBE.*"},
}
err := v.Gather(acc)
Expand All @@ -75,7 +77,7 @@ func TestFieldConfig(t *testing.T) {
for fieldCfg, expected := range expect {
acc := &testutil.Accumulator{}
v := &Varnish{
run: fakeVarnishStat(fullOutput, true, ""),
run: fakeVarnishStat(fullOutput, true, "", internal.Duration{Duration: time.Second}),
Stats: strings.Split(fieldCfg, ","),
}
err := v.Gather(acc)
Expand Down

0 comments on commit 87d1804

Please sign in to comment.