Add configurable timeout to varnish input (influxdata#5214)

dupondje · Apr 22, 2019 · 9eeabe4 · 9eeabe4
1 parent f8a8a41
commit 9eeabe4
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 12 deletions.
diff --git a/plugins/inputs/activemq/activemq.go b/plugins/inputs/activemq/activemq.go
@@ -91,7 +91,7 @@ var sampleConfig = `
 
   ## Required ActiveMQ port
   # port = 8161
-  
+
   ## Credentials for basic HTTP authentication
   # username = "admin"
   # password = "admin"
@@ -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"

diff --git a/plugins/inputs/varnish/varnish.go b/plugins/inputs/varnish/varnish.go
@@ -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:
@@ -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 {
@@ -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 != "" {
@@ -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)
 	}
@@ -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)
 	}
@@ -170,6 +176,7 @@ func init() {
 			Binary:       defaultBinary,
 			UseSudo:      false,
 			InstanceName: "",
+			Timeout:      defaultTimeout,
 		}
 	})
 }
diff --git a/plugins/inputs/varnish/varnish_test.go b/plugins/inputs/varnish/varnish_test.go
@@ -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)
@@ -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)
@@ -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)
@@ -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)