From 45d7a1c6ff089b3f59184df04b0ceb986a570fce Mon Sep 17 00:00:00 2001 From: Sebastian Spaink Date: Tue, 12 Jan 2021 15:19:20 -0600 Subject: [PATCH 1/7] Use go-ping for "native" execution in Ping plugin --- docs/LICENSE_OF_DEPENDENCIES.md | 2 +- go.mod | 2 +- go.sum | 5 +- plugins/inputs/ping/README.md | 2 +- plugins/inputs/ping/ping.go | 350 +++++++------------------------ plugins/inputs/ping/ping_test.go | 60 ++++-- 6 files changed, 120 insertions(+), 301 deletions(-) diff --git a/docs/LICENSE_OF_DEPENDENCIES.md b/docs/LICENSE_OF_DEPENDENCIES.md index 642c79673b18c..14c46448c3b4a 100644 --- a/docs/LICENSE_OF_DEPENDENCIES.md +++ b/docs/LICENSE_OF_DEPENDENCIES.md @@ -51,9 +51,9 @@ following works: - github.com/eclipse/paho.mqtt.golang [Eclipse Public License - v 1.0](https://github.com/eclipse/paho.mqtt.golang/blob/master/LICENSE) - github.com/ericchiang/k8s [Apache License 2.0](https://github.com/ericchiang/k8s/blob/master/LICENSE) - github.com/ghodss/yaml [MIT License](https://github.com/ghodss/yaml/blob/master/LICENSE) -- github.com/glinton/ping [MIT License](https://github.com/glinton/ping/blob/master/LICENSE) - github.com/go-logfmt/logfmt [MIT License](https://github.com/go-logfmt/logfmt/blob/master/LICENSE) - github.com/go-ole/go-ole [MIT License](https://github.com/go-ole/go-ole/blob/master/LICENSE) +- github.com/go-ping/ping [MIT License](https://github.com/go-ping/ping/blob/master/LICENSE) - github.com/go-redis/redis [BSD 2-Clause "Simplified" License](https://github.com/go-redis/redis/blob/master/LICENSE) - github.com/go-sql-driver/mysql [Mozilla Public License 2.0](https://github.com/go-sql-driver/mysql/blob/master/LICENSE) - github.com/goburrow/modbus [BSD 3-Clause "New" or "Revised" License](https://github.com/goburrow/modbus/blob/master/LICENSE) diff --git a/go.mod b/go.mod index bd0ec9345ffbc..45a9a48ba618e 100644 --- a/go.mod +++ b/go.mod @@ -49,9 +49,9 @@ require ( github.com/eclipse/paho.mqtt.golang v1.2.0 github.com/ericchiang/k8s v1.2.0 github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32 - github.com/glinton/ping v0.1.4-0.20200311211934-5ac87da8cd96 github.com/go-logfmt/logfmt v0.4.0 github.com/go-ole/go-ole v1.2.1 // indirect + github.com/go-ping/ping v0.0.0-20201115131931-3300c582a663 github.com/go-redis/redis v6.15.9+incompatible github.com/go-sql-driver/mysql v1.5.0 github.com/goburrow/modbus v0.1.0 diff --git a/go.sum b/go.sum index 79588d467c4f9..18fc73ab9df13 100644 --- a/go.sum +++ b/go.sum @@ -208,8 +208,6 @@ github.com/ghodss/yaml v0.0.0-20150909031657-73d445a93680/go.mod h1:4dBDuWmgqj2H github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32 h1:Mn26/9ZMNWSw9C9ERFA1PUxfmGpolnw2v0bKOREu5ew= github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32/go.mod h1:GIjDIg/heH5DOkXY3YJ/wNhfHsQHoXGjl8G8amsYQ1I= -github.com/glinton/ping v0.1.4-0.20200311211934-5ac87da8cd96 h1:YpooqMW354GG47PXNBiaCv6yCQizyP3MXD9NUPrCEQ8= -github.com/glinton/ping v0.1.4-0.20200311211934-5ac87da8cd96/go.mod h1:uY+1eqFUyotrQxF1wYFNtMeHp/swbYRsoGzfcPZ8x3o= github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= @@ -225,6 +223,8 @@ github.com/go-openapi/jsonpointer v0.0.0-20160704185906-46af16f9f7b1/go.mod h1:+ github.com/go-openapi/jsonreference v0.0.0-20160704190145-13c6e3589ad9/go.mod h1:W3Z9FmVs9qj+KR4zFKmDPGiLdk1D9Rlm7cyMvf57TTg= github.com/go-openapi/spec v0.0.0-20160808142527-6aced65f8501/go.mod h1:J8+jY1nAiCcj+friV/PDoE1/3eeccG9LYBs0tYvLOWc= github.com/go-openapi/swag v0.0.0-20160704191624-1d0bd113de87/go.mod h1:DXUve3Dpr1UfpPtxFw+EFuQ41HhCWZfha5jSVRG7C7I= +github.com/go-ping/ping v0.0.0-20201115131931-3300c582a663 h1:jI2GiiRh+pPbey52EVmbU6kuLiXqwy4CXZ4gwUBj8Y0= +github.com/go-ping/ping v0.0.0-20201115131931-3300c582a663/go.mod h1:35JbSyV/BYqHwwRA6Zr1uVDm1637YlNOU61wI797NPI= github.com/go-redis/redis v6.15.9+incompatible h1:K0pv1D7EQUjfyoMql+r/jZqCLizCGKFlFgcHWWmHQjg= github.com/go-redis/redis v6.15.9+incompatible/go.mod h1:NAIEuMOZ/fxfXJIrKDQDz8wamY7mA7PouImQ2Jvg6kA= github.com/go-sql-driver/mysql v1.5.0 h1:ozyZYNQW3x3HtqT1jira07DN2PArx2v7/mN66gGcHOs= @@ -698,7 +698,6 @@ golang.org/x/net v0.0.0-20190503192946-f4e77d36d62c/go.mod h1:t9HGtf8HONx5eT2rtn golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= golang.org/x/net v0.0.0-20190613194153-d28f0bde5980/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= -golang.org/x/net v0.0.0-20190628185345-da137c7871d7/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20190724013045-ca1201d0de80/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20190827160401-ba9fcec4b297/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20191003171128-d98b1b443823/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= diff --git a/plugins/inputs/ping/README.md b/plugins/inputs/ping/README.md index 83a91a2eeb96d..b3f412943a42b 100644 --- a/plugins/inputs/ping/README.md +++ b/plugins/inputs/ping/README.md @@ -19,7 +19,7 @@ apt-get install iputils-ping When using `method = "native"` a ping is sent and the results are reported in native Go by the Telegraf process, eliminating the need to execute the system -`ping` command. +`ping` command. This is dependent on the [go-ping](https://github.com/go-ping/ping) package. ### Configuration: diff --git a/plugins/inputs/ping/ping.go b/plugins/inputs/ping/ping.go index 87f7af8e7489f..8065670cb19e9 100644 --- a/plugins/inputs/ping/ping.go +++ b/plugins/inputs/ping/ping.go @@ -1,23 +1,15 @@ package ping import ( - "context" "errors" "fmt" - "log" "math" - "net" - "os/exec" "runtime" - "sort" - "strings" "sync" - "sync/atomic" "time" - "github.com/glinton/ping" + "github.com/go-ping/ping" "github.com/influxdata/telegraf" - "github.com/influxdata/telegraf/internal" "github.com/influxdata/telegraf/plugins/inputs" ) @@ -26,12 +18,14 @@ import ( // for unit test purposes (see ping_test.go) type HostPinger func(binary string, timeout float64, args ...string) (string, error) -type HostResolver func(ctx context.Context, ipv6 bool, host string) (*net.IPAddr, error) +type Ping struct { + // wg is used to wait for ping with multiple URLs + wg sync.WaitGroup `toml:"-"` -type IsCorrectNetwork func(ip net.IPAddr) bool + // ttl stands for "Time to live" and is a gathered value on non-windows machines + ttl int `toml:"-"` -type Ping struct { - wg sync.WaitGroup + Log telegraf.Logger `toml:"-"` // Interval at which to ping (ping -i ) PingInterval float64 `toml:"ping_interval"` @@ -67,12 +61,6 @@ type Ping struct { // host ping function pingHost HostPinger - // resolve host function - resolveHost HostResolver - - // listenAddr is the address associated with the interface defined. - listenAddr string - // Calculate the given percentiles when using native method Percentiles []int } @@ -134,10 +122,6 @@ func (*Ping) SampleConfig() string { } func (p *Ping) Gather(acc telegraf.Accumulator) error { - if p.Interface != "" && p.listenAddr == "" { - p.listenAddr = getAddr(p.Interface) - } - for _, host := range p.Urls { p.wg.Add(1) go func(host string) { @@ -157,204 +141,103 @@ func (p *Ping) Gather(acc telegraf.Accumulator) error { return nil } -func getAddr(iface string) string { - if addr := net.ParseIP(iface); addr != nil { - return addr.String() - } - - ifaces, err := net.Interfaces() - if err != nil { - return "" - } - - var ip net.IP - for i := range ifaces { - if ifaces[i].Name == iface { - addrs, err := ifaces[i].Addrs() - if err != nil { - return "" - } - if len(addrs) > 0 { - switch v := addrs[0].(type) { - case *net.IPNet: - ip = v.IP - case *net.IPAddr: - ip = v.IP - } - if len(ip) == 0 { - return "" - } - return ip.String() - } - } - } - - return "" -} - -func hostPinger(binary string, timeout float64, args ...string) (string, error) { - bin, err := exec.LookPath(binary) +func (p *Ping) pingToURLNative(destination string, acc telegraf.Accumulator) { + pinger, err := ping.NewPinger(destination) if err != nil { - return "", err - } - c := exec.Command(bin, args...) - out, err := internal.CombinedOutputTimeout(c, - time.Second*time.Duration(timeout+5)) - return string(out), err -} - -func filterIPs(addrs []net.IPAddr, filterFunc IsCorrectNetwork) []net.IPAddr { - n := 0 - for _, x := range addrs { - if filterFunc(x) { - addrs[n] = x - n++ - } + p.Log.Errorf("Failed to ping: %v", err) + acc.AddError(err) + return } - return addrs[:n] -} - -func hostResolver(ctx context.Context, ipv6 bool, destination string) (*net.IPAddr, error) { - resolver := &net.Resolver{} - ips, err := resolver.LookupIPAddr(ctx, destination) - if err != nil { - return nil, err + // Required for windows. Despite the method name, this should work without the need to elevate privileges and has been tested on Windows 10 + if runtime.GOOS == "windows" { + pinger.SetPrivileged(true) } - if ipv6 { - ips = filterIPs(ips, isV6) + // The interval cannot be below 0.2 seconds, matching ping implementation: https://linux.die.net/man/8/ping + if p.PingInterval < 0.2 { + pinger.Interval = time.Duration(.2 * float64(time.Second)) } else { - ips = filterIPs(ips, isV4) - } - - if len(ips) == 0 { - return nil, errors.New("Cannot resolve ip address") - } - return &ips[0], err -} - -func isV4(ip net.IPAddr) bool { - return ip.IP.To4() != nil -} - -func isV6(ip net.IPAddr) bool { - return !isV4(ip) -} - -func (p *Ping) pingToURLNative(destination string, acc telegraf.Accumulator) { - ctx := context.Background() - interval := p.PingInterval - if interval < 0.2 { - interval = 0.2 + pinger.Interval = time.Duration(p.PingInterval * float64(time.Second)) } - timeout := p.Timeout - if timeout == 0 { - timeout = 5 + // If no timeout is given default to 5 seconds, matching original implementation + if p.Timeout == 0 { + pinger.Timeout = time.Duration(5) * time.Second + } else { + pinger.Timeout = time.Duration(p.Timeout) * time.Second } - tick := time.NewTicker(time.Duration(interval * float64(time.Second))) - defer tick.Stop() - - if p.Deadline > 0 { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, time.Duration(p.Deadline)*time.Second) - defer cancel() - } + go func() { + if p.Deadline <= 0 { + return + } + // If deadline is set ping exits regardless of how many packets have been sent or received + timer := time.AfterFunc(time.Duration(p.Deadline)*time.Second, func() { + pinger.Stop() + }) + defer timer.Stop() + }() - host, err := p.resolveHost(ctx, p.IPv6, destination) + pinger.Count = p.Count + err = pinger.Run() if err != nil { - acc.AddFields( - "ping", - map[string]interface{}{"result_code": 1}, - map[string]string{"url": destination}, - ) + p.Log.Errorf("Failed to ping: %v", err) acc.AddError(err) return } - resps := make(chan *ping.Response) - rsps := []*ping.Response{} + stats := pinger.Statistics() - r := &sync.WaitGroup{} - r.Add(1) - go func() { - for res := range resps { - rsps = append(rsps, res) + // Get Time to live (TTL) of first response, matching original implementation + var firstTTL bool + pinger.OnRecv = func(pkt *ping.Packet) { + if !firstTTL { + p.ttl = pkt.Ttl + firstTTL = true } - r.Done() - }() - - wg := &sync.WaitGroup{} - c := ping.Client{} - - var doErr error - var packetsSent int32 + } - type sentReq struct { - err error - sent bool + tags := map[string]string{"url": destination} + fields := map[string]interface{}{ + "result_code": 0, + "packets_transmitted": stats.PacketsSent, + "packets_received": stats.PacketsRecv, } - sents := make(chan sentReq) - r.Add(1) - go func() { - for sent := range sents { - if sent.err != nil { - doErr = sent.err - } - if sent.sent { - atomic.AddInt32(&packetsSent, 1) - } + if stats.PacketsSent == 0 { + if err != nil { + fields["result_code"] = 2 } - r.Done() - }() + return + } - for i := 0; i < p.Count; i++ { - select { - case <-ctx.Done(): - goto finish - case <-tick.C: - ctx, cancel := context.WithTimeout(ctx, time.Duration(timeout*float64(time.Second))) - defer cancel() - - wg.Add(1) - go func(seq int) { - defer wg.Done() - resp, err := c.Do(ctx, &ping.Request{ - Dst: net.ParseIP(host.String()), - Src: net.ParseIP(p.listenAddr), - Seq: seq, - }) - - sent := sentReq{err: err, sent: true} - if err != nil { - if strings.Contains(err.Error(), "not permitted") { - sent.sent = false - } - sents <- sent - return - } - - resps <- resp - sents <- sent - }(i + 1) + if stats.PacketsRecv == 0 { + if err != nil { + fields["result_code"] = 1 } + fields["percent_packet_loss"] = float64(100) + return } -finish: - wg.Wait() - close(resps) - close(sents) - - r.Wait() + for _, perc := range p.Percentiles { + var value = percentile(durationSlice(stats.Rtts), perc) + var field = fmt.Sprintf("percentile%v_ms", perc) + fields[field] = float64(value.Nanoseconds()) / float64(time.Millisecond) + } - if doErr != nil && strings.Contains(doErr.Error(), "not permitted") { - log.Printf("D! [inputs.ping] %s", doErr.Error()) + // Set TTL only on supported platform. See golang.org/x/net/ipv4/payload_cmsg.go + switch runtime.GOOS { + case "aix", "darwin", "dragonfly", "freebsd", "linux", "netbsd", "openbsd", "solaris": + fields["ttl"] = p.ttl } - tags, fields := onFin(packetsSent, rsps, doErr, destination, p.Percentiles) + fields["percent_packet_loss"] = float64(stats.PacketLoss) + fields["minimum_response_ms"] = float64(stats.MinRtt) / float64(time.Millisecond) + fields["average_response_ms"] = float64(stats.AvgRtt) / float64(time.Millisecond) + fields["maximum_response_ms"] = float64(stats.MaxRtt) / float64(time.Millisecond) + fields["standard_deviation_ms"] = float64(stats.StdDevRtt) / float64(time.Millisecond) + acc.AddFields("ping", fields, tags) } @@ -388,87 +271,6 @@ func percentile(values durationSlice, perc int) time.Duration { } } -func onFin(packetsSent int32, resps []*ping.Response, err error, destination string, percentiles []int) (map[string]string, map[string]interface{}) { - packetsRcvd := len(resps) - - tags := map[string]string{"url": destination} - fields := map[string]interface{}{ - "result_code": 0, - "packets_transmitted": packetsSent, - "packets_received": packetsRcvd, - } - - if packetsSent == 0 { - if err != nil { - fields["result_code"] = 2 - } - return tags, fields - } - - if packetsRcvd == 0 { - if err != nil { - fields["result_code"] = 1 - } - fields["percent_packet_loss"] = float64(100) - return tags, fields - } - - fields["percent_packet_loss"] = float64(int(packetsSent)-packetsRcvd) / float64(packetsSent) * 100 - ttl := resps[0].TTL - - var min, max, avg, total time.Duration - - if len(percentiles) > 0 { - var rtt []time.Duration - for _, resp := range resps { - rtt = append(rtt, resp.RTT) - total += resp.RTT - } - sort.Sort(durationSlice(rtt)) - min = rtt[0] - max = rtt[len(rtt)-1] - - for _, perc := range percentiles { - var value = percentile(durationSlice(rtt), perc) - var field = fmt.Sprintf("percentile%v_ms", perc) - fields[field] = float64(value.Nanoseconds()) / float64(time.Millisecond) - } - } else { - min = resps[0].RTT - max = resps[0].RTT - - for _, res := range resps { - if res.RTT < min { - min = res.RTT - } - if res.RTT > max { - max = res.RTT - } - total += res.RTT - } - } - - avg = total / time.Duration(packetsRcvd) - var sumsquares time.Duration - for _, res := range resps { - sumsquares += (res.RTT - avg) * (res.RTT - avg) - } - stdDev := time.Duration(math.Sqrt(float64(sumsquares / time.Duration(packetsRcvd)))) - - // Set TTL only on supported platform. See golang.org/x/net/ipv4/payload_cmsg.go - switch runtime.GOOS { - case "aix", "darwin", "dragonfly", "freebsd", "linux", "netbsd", "openbsd", "solaris": - fields["ttl"] = ttl - } - - fields["minimum_response_ms"] = float64(min.Nanoseconds()) / float64(time.Millisecond) - fields["average_response_ms"] = float64(avg.Nanoseconds()) / float64(time.Millisecond) - fields["maximum_response_ms"] = float64(max.Nanoseconds()) / float64(time.Millisecond) - fields["standard_deviation_ms"] = float64(stdDev.Nanoseconds()) / float64(time.Millisecond) - - return tags, fields -} - // Init ensures the plugin is configured correctly. func (p *Ping) Init() error { if p.Count < 1 { @@ -481,8 +283,6 @@ func (p *Ping) Init() error { func init() { inputs.Add("ping", func() telegraf.Input { return &Ping{ - pingHost: hostPinger, - resolveHost: hostResolver, PingInterval: 1.0, Count: 1, Timeout: 1.0, diff --git a/plugins/inputs/ping/ping_test.go b/plugins/inputs/ping/ping_test.go index 7aadba223e224..fd5c727225857 100644 --- a/plugins/inputs/ping/ping_test.go +++ b/plugins/inputs/ping/ping_test.go @@ -405,25 +405,46 @@ func mockHostResolver(ctx context.Context, ipv6 bool, host string) (*net.IPAddr, func TestPingGatherNative(t *testing.T) { t.Skip("Skipping test due to permission requirements.") - var acc testutil.Accumulator - p := Ping{ - Urls: []string{"localhost", "127.0.0.2"}, - Method: "native", - Count: 5, - resolveHost: mockHostResolver, - Percentiles: []int{50, 95, 99}, + type test struct { + P *Ping } - assert.NoError(t, acc.GatherError(p.Gather)) - assert.True(t, acc.HasPoint("ping", map[string]string{"url": "localhost"}, "packets_transmitted", 5)) - assert.True(t, acc.HasPoint("ping", map[string]string{"url": "localhost"}, "packets_received", 5)) - assert.True(t, acc.HasField("ping", "percentile50_ms")) - assert.True(t, acc.HasField("ping", "percentile95_ms")) - assert.True(t, acc.HasField("ping", "percentile99_ms")) -} + tests := []test{ + { + P: &Ping{ + Urls: []string{"localhost", "127.0.0.2"}, + Method: "native", + Count: 5, + Percentiles: []int{50, 95, 99}, + }, + }, + { + P: &Ping{ + Urls: []string{"localhost", "127.0.0.2"}, + Method: "native", + Count: 5, + PingInterval: 1, + Percentiles: []int{50, 95, 99}, + }, + }, + } + + for _, tc := range tests { + var acc testutil.Accumulator + + require.NoError(t, acc.GatherError(tc.P.Gather)) + assert.True(t, acc.HasPoint("ping", map[string]string{"url": "localhost"}, "packets_transmitted", 5)) + assert.True(t, acc.HasPoint("ping", map[string]string{"url": "localhost"}, "packets_received", 5)) + assert.True(t, acc.HasField("ping", "percentile50_ms")) + assert.True(t, acc.HasField("ping", "percentile95_ms")) + assert.True(t, acc.HasField("ping", "percentile99_ms")) + assert.True(t, acc.HasField("ping", "percent_packet_loss")) + assert.True(t, acc.HasField("ping", "minimum_response_ms")) + assert.True(t, acc.HasField("ping", "average_response_ms")) + assert.True(t, acc.HasField("ping", "maximum_response_ms")) + assert.True(t, acc.HasField("ping", "standard_deviation_ms")) + } -func mockHostResolverError(ctx context.Context, ipv6 bool, host string) (*net.IPAddr, error) { - return nil, errors.New("myMock error") } // Test failed DNS resolutions @@ -434,10 +455,9 @@ func TestDNSLookupError(t *testing.T) { var acc testutil.Accumulator p := Ping{ - Urls: []string{"localhost"}, - Method: "native", - IPv6: false, - resolveHost: mockHostResolverError, + Urls: []string{"localhost"}, + Method: "native", + IPv6: false, } acc.GatherError(p.Gather) From 8b7a901de643f887e64b2afda277405b8194b75d Mon Sep 17 00:00:00 2001 From: Sebastian Spaink Date: Wed, 13 Jan 2021 15:32:35 -0600 Subject: [PATCH 2/7] Check for ipv6 and deadline out of go func --- plugins/inputs/ping/README.md | 2 +- plugins/inputs/ping/ping.go | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/plugins/inputs/ping/README.md b/plugins/inputs/ping/README.md index b3f412943a42b..83a91a2eeb96d 100644 --- a/plugins/inputs/ping/README.md +++ b/plugins/inputs/ping/README.md @@ -19,7 +19,7 @@ apt-get install iputils-ping When using `method = "native"` a ping is sent and the results are reported in native Go by the Telegraf process, eliminating the need to execute the system -`ping` command. This is dependent on the [go-ping](https://github.com/go-ping/ping) package. +`ping` command. ### Configuration: diff --git a/plugins/inputs/ping/ping.go b/plugins/inputs/ping/ping.go index 8065670cb19e9..c7b598d60f371 100644 --- a/plugins/inputs/ping/ping.go +++ b/plugins/inputs/ping/ping.go @@ -154,6 +154,10 @@ func (p *Ping) pingToURLNative(destination string, acc telegraf.Accumulator) { pinger.SetPrivileged(true) } + if p.IPv6 == true { + pinger.SetNetwork("ip6") + } + // The interval cannot be below 0.2 seconds, matching ping implementation: https://linux.die.net/man/8/ping if p.PingInterval < 0.2 { pinger.Interval = time.Duration(.2 * float64(time.Second)) @@ -168,16 +172,13 @@ func (p *Ping) pingToURLNative(destination string, acc telegraf.Accumulator) { pinger.Timeout = time.Duration(p.Timeout) * time.Second } - go func() { - if p.Deadline <= 0 { - return - } + if p.Deadline > 0 { // If deadline is set ping exits regardless of how many packets have been sent or received timer := time.AfterFunc(time.Duration(p.Deadline)*time.Second, func() { pinger.Stop() }) defer timer.Stop() - }() + } pinger.Count = p.Count err = pinger.Run() From edda43240d4d67d501e174062414636d84828d9b Mon Sep 17 00:00:00 2001 From: Sebastian Spaink Date: Thu, 21 Jan 2021 15:34:21 -0600 Subject: [PATCH 3/7] ensure dns failure --- plugins/inputs/ping/ping_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/inputs/ping/ping_test.go b/plugins/inputs/ping/ping_test.go index fd5c727225857..f660e7b70a89e 100644 --- a/plugins/inputs/ping/ping_test.go +++ b/plugins/inputs/ping/ping_test.go @@ -455,7 +455,8 @@ func TestDNSLookupError(t *testing.T) { var acc testutil.Accumulator p := Ping{ - Urls: []string{"localhost"}, + Log: testutil.Logger{}, + Urls: []string{"fakehost"}, Method: "native", IPv6: false, } From 574d15c405d024e1c4251075347121aab884ab59 Mon Sep 17 00:00:00 2001 From: Sebastian Spaink Date: Mon, 25 Jan 2021 11:10:39 -0600 Subject: [PATCH 4/7] Move interval and timeout calc to init Removed dns failure check, 3rd parties libary responsibility --- plugins/inputs/ping/ping.go | 39 +++++++++++++++++++------------- plugins/inputs/ping/ping_test.go | 21 ++--------------- 2 files changed, 25 insertions(+), 35 deletions(-) diff --git a/plugins/inputs/ping/ping.go b/plugins/inputs/ping/ping.go index c7b598d60f371..6ba77cf1e9249 100644 --- a/plugins/inputs/ping/ping.go +++ b/plugins/inputs/ping/ping.go @@ -20,10 +20,14 @@ type HostPinger func(binary string, timeout float64, args ...string) (string, er type Ping struct { // wg is used to wait for ping with multiple URLs - wg sync.WaitGroup `toml:"-"` + wg sync.WaitGroup // ttl stands for "Time to live" and is a gathered value on non-windows machines - ttl int `toml:"-"` + ttl int + + interval time.Duration + + timeout time.Duration Log telegraf.Logger `toml:"-"` @@ -154,23 +158,12 @@ func (p *Ping) pingToURLNative(destination string, acc telegraf.Accumulator) { pinger.SetPrivileged(true) } - if p.IPv6 == true { + if p.IPv6 { pinger.SetNetwork("ip6") } - // The interval cannot be below 0.2 seconds, matching ping implementation: https://linux.die.net/man/8/ping - if p.PingInterval < 0.2 { - pinger.Interval = time.Duration(.2 * float64(time.Second)) - } else { - pinger.Interval = time.Duration(p.PingInterval * float64(time.Second)) - } - - // If no timeout is given default to 5 seconds, matching original implementation - if p.Timeout == 0 { - pinger.Timeout = time.Duration(5) * time.Second - } else { - pinger.Timeout = time.Duration(p.Timeout) * time.Second - } + pinger.Interval = p.interval + pinger.Timeout = p.timeout if p.Deadline > 0 { // If deadline is set ping exits regardless of how many packets have been sent or received @@ -278,6 +271,20 @@ func (p *Ping) Init() error { return errors.New("bad number of packets to transmit") } + // The interval cannot be below 0.2 seconds, matching ping implementation: https://linux.die.net/man/8/ping + if p.PingInterval < 0.2 { + p.interval = time.Duration(.2 * float64(time.Second)) + } else { + p.interval = time.Duration(p.PingInterval * float64(time.Second)) + } + + // If no timeout is given default to 5 seconds, matching original implementation + if p.Timeout == 0 { + p.timeout = time.Duration(5) * time.Second + } else { + p.timeout = time.Duration(p.Timeout) * time.Second + } + return nil } diff --git a/plugins/inputs/ping/ping_test.go b/plugins/inputs/ping/ping_test.go index f660e7b70a89e..f6a2601f7cb8a 100644 --- a/plugins/inputs/ping/ping_test.go +++ b/plugins/inputs/ping/ping_test.go @@ -431,7 +431,8 @@ func TestPingGatherNative(t *testing.T) { for _, tc := range tests { var acc testutil.Accumulator - + err := tc.P.Init() + require.NoError(t, err) require.NoError(t, acc.GatherError(tc.P.Gather)) assert.True(t, acc.HasPoint("ping", map[string]string{"url": "localhost"}, "packets_transmitted", 5)) assert.True(t, acc.HasPoint("ping", map[string]string{"url": "localhost"}, "packets_received", 5)) @@ -446,21 +447,3 @@ func TestPingGatherNative(t *testing.T) { } } - -// Test failed DNS resolutions -func TestDNSLookupError(t *testing.T) { - if testing.Short() { - t.Skip("Skipping test due to permission requirements.") - } - - var acc testutil.Accumulator - p := Ping{ - Log: testutil.Logger{}, - Urls: []string{"fakehost"}, - Method: "native", - IPv6: false, - } - - acc.GatherError(p.Gather) - assert.True(t, len(acc.Errors) > 0) -} From f63e1698b32e272af3e21e64d6554bcbec3dd483 Mon Sep 17 00:00:00 2001 From: Sebastian Spaink Date: Mon, 25 Jan 2021 11:23:38 -0600 Subject: [PATCH 5/7] Rename timeout to avoid conflict --- plugins/inputs/ping/ping.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/plugins/inputs/ping/ping.go b/plugins/inputs/ping/ping.go index 6ba77cf1e9249..1db1dde4cd497 100644 --- a/plugins/inputs/ping/ping.go +++ b/plugins/inputs/ping/ping.go @@ -25,9 +25,9 @@ type Ping struct { // ttl stands for "Time to live" and is a gathered value on non-windows machines ttl int - interval time.Duration - - timeout time.Duration + // Pre-calculated interval and timeout + calcInterval time.Duration + calcTimeout time.Duration Log telegraf.Logger `toml:"-"` @@ -162,8 +162,8 @@ func (p *Ping) pingToURLNative(destination string, acc telegraf.Accumulator) { pinger.SetNetwork("ip6") } - pinger.Interval = p.interval - pinger.Timeout = p.timeout + pinger.Interval = p.calcInterval + pinger.Timeout = p.calcTimeout if p.Deadline > 0 { // If deadline is set ping exits regardless of how many packets have been sent or received @@ -273,16 +273,16 @@ func (p *Ping) Init() error { // The interval cannot be below 0.2 seconds, matching ping implementation: https://linux.die.net/man/8/ping if p.PingInterval < 0.2 { - p.interval = time.Duration(.2 * float64(time.Second)) + p.calcInterval = time.Duration(.2 * float64(time.Second)) } else { - p.interval = time.Duration(p.PingInterval * float64(time.Second)) + p.calcInterval = time.Duration(p.PingInterval * float64(time.Second)) } // If no timeout is given default to 5 seconds, matching original implementation if p.Timeout == 0 { - p.timeout = time.Duration(5) * time.Second + p.calcTimeout = time.Duration(5) * time.Second } else { - p.timeout = time.Duration(p.Timeout) * time.Second + p.calcTimeout = time.Duration(p.Timeout) * time.Second } return nil From 22af2ef818e69a07ecd07f0dc516c6c13ce06a99 Mon Sep 17 00:00:00 2001 From: Sebastian Spaink Date: Tue, 26 Jan 2021 14:48:04 -0600 Subject: [PATCH 6/7] Move native ping to interface Update tests --- plugins/inputs/ping/ping.go | 79 +++++++++++++++++---------- plugins/inputs/ping/ping_test.go | 93 ++++++++++++++++++++++++++++---- 2 files changed, 134 insertions(+), 38 deletions(-) diff --git a/plugins/inputs/ping/ping.go b/plugins/inputs/ping/ping.go index 1db1dde4cd497..f4c0f304004c0 100644 --- a/plugins/inputs/ping/ping.go +++ b/plugins/inputs/ping/ping.go @@ -5,6 +5,7 @@ import ( "fmt" "math" "runtime" + "strings" "sync" "time" @@ -22,9 +23,6 @@ type Ping struct { // wg is used to wait for ping with multiple URLs wg sync.WaitGroup - // ttl stands for "Time to live" and is a gathered value on non-windows machines - ttl int - // Pre-calculated interval and timeout calcInterval time.Duration calcTimeout time.Duration @@ -65,6 +63,8 @@ type Ping struct { // host ping function pingHost HostPinger + nativePingFunc NativePingFunc + // Calculate the given percentiles when using native method Percentiles []int } @@ -145,12 +145,19 @@ func (p *Ping) Gather(acc telegraf.Accumulator) error { return nil } -func (p *Ping) pingToURLNative(destination string, acc telegraf.Accumulator) { +type pingStats struct { + ping.Statistics + ttl int +} + +type NativePingFunc func(destination string) (*pingStats, error) + +func (p *Ping) nativePing(destination string) (*pingStats, error) { + ps := &pingStats{} + pinger, err := ping.NewPinger(destination) if err != nil { - p.Log.Errorf("Failed to ping: %v", err) - acc.AddError(err) - return + return nil, fmt.Errorf("Failed to create new pinger: %w", err) } // Required for windows. Despite the method name, this should work without the need to elevate privileges and has been tested on Windows 10 @@ -173,44 +180,57 @@ func (p *Ping) pingToURLNative(destination string, acc telegraf.Accumulator) { defer timer.Stop() } + // Get Time to live (TTL) of first response, matching original implementation + once := &sync.Once{} + pinger.OnRecv = func(pkt *ping.Packet) { + once.Do(func() { + ps.ttl = pkt.Ttl + }) + } + pinger.Count = p.Count err = pinger.Run() if err != nil { - p.Log.Errorf("Failed to ping: %v", err) - acc.AddError(err) - return + return nil, fmt.Errorf("Failed to run pinger: %w", err) } - stats := pinger.Statistics() + ps.Statistics = *pinger.Statistics() - // Get Time to live (TTL) of first response, matching original implementation - var firstTTL bool - pinger.OnRecv = func(pkt *ping.Packet) { - if !firstTTL { - p.ttl = pkt.Ttl - firstTTL = true + return ps, nil +} + +func (p *Ping) pingToURLNative(destination string, acc telegraf.Accumulator) { + + tags := map[string]string{"url": destination} + fields := map[string]interface{}{} + + stats, err := p.nativePingFunc(destination) + if err != nil { + if strings.Contains(err.Error(), "unknown") { + fields["result_code"] = 1 + } else { + fields["result_code"] = 2 } + acc.AddFields("ping", fields, tags) + return } - tags := map[string]string{"url": destination} - fields := map[string]interface{}{ + fields = map[string]interface{}{ "result_code": 0, "packets_transmitted": stats.PacketsSent, "packets_received": stats.PacketsRecv, } if stats.PacketsSent == 0 { - if err != nil { - fields["result_code"] = 2 - } + fields["result_code"] = 2 + acc.AddFields("ping", fields, tags) return } if stats.PacketsRecv == 0 { - if err != nil { - fields["result_code"] = 1 - } + fields["result_code"] = 1 fields["percent_packet_loss"] = float64(100) + acc.AddFields("ping", fields, tags) return } @@ -223,7 +243,7 @@ func (p *Ping) pingToURLNative(destination string, acc telegraf.Accumulator) { // Set TTL only on supported platform. See golang.org/x/net/ipv4/payload_cmsg.go switch runtime.GOOS { case "aix", "darwin", "dragonfly", "freebsd", "linux", "netbsd", "openbsd", "solaris": - fields["ttl"] = p.ttl + fields["ttl"] = stats.ttl } fields["percent_packet_loss"] = float64(stats.PacketLoss) @@ -243,6 +263,9 @@ func (p durationSlice) Swap(i, j int) { p[i], p[j] = p[j], p[i] } // R7 from Hyndman and Fan (1996), which matches Excel func percentile(values durationSlice, perc int) time.Duration { + if len(values) < 0 { + return 0 + } if perc < 0 { perc = 0 } @@ -290,7 +313,7 @@ func (p *Ping) Init() error { func init() { inputs.Add("ping", func() telegraf.Input { - return &Ping{ + p := &Ping{ PingInterval: 1.0, Count: 1, Timeout: 1.0, @@ -300,5 +323,7 @@ func init() { Arguments: []string{}, Percentiles: []int{}, } + p.nativePingFunc = p.nativePing + return p }) } diff --git a/plugins/inputs/ping/ping_test.go b/plugins/inputs/ping/ping_test.go index f6a2601f7cb8a..0afa53706ab5d 100644 --- a/plugins/inputs/ping/ping_test.go +++ b/plugins/inputs/ping/ping_test.go @@ -5,11 +5,14 @@ package ping import ( "context" "errors" + "fmt" "net" "reflect" "sort" "testing" + "time" + "github.com/go-ping/ping" "github.com/influxdata/telegraf/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -403,28 +406,47 @@ func mockHostResolver(ctx context.Context, ipv6 bool, host string) (*net.IPAddr, // Test that Gather function works using native ping func TestPingGatherNative(t *testing.T) { - t.Skip("Skipping test due to permission requirements.") - type test struct { P *Ping } + fakePingFunc := func(destination string) (*pingStats, error) { + s := &pingStats{ + Statistics: ping.Statistics{ + PacketsSent: 5, + PacketsRecv: 5, + Rtts: []time.Duration{ + 1 * time.Millisecond, + 2 * time.Millisecond, + 3 * time.Millisecond, + 4 * time.Millisecond, + 5 * time.Millisecond, + }, + }, + ttl: 1, + } + + return s, nil + } + tests := []test{ { P: &Ping{ - Urls: []string{"localhost", "127.0.0.2"}, - Method: "native", - Count: 5, - Percentiles: []int{50, 95, 99}, + Urls: []string{"localhost", "127.0.0.2"}, + Method: "native", + Count: 5, + Percentiles: []int{50, 95, 99}, + nativePingFunc: fakePingFunc, }, }, { P: &Ping{ - Urls: []string{"localhost", "127.0.0.2"}, - Method: "native", - Count: 5, - PingInterval: 1, - Percentiles: []int{50, 95, 99}, + Urls: []string{"localhost", "127.0.0.2"}, + Method: "native", + Count: 5, + PingInterval: 1, + Percentiles: []int{50, 95, 99}, + nativePingFunc: fakePingFunc, }, }, } @@ -447,3 +469,52 @@ func TestPingGatherNative(t *testing.T) { } } + +func TestNoPacketsSent(t *testing.T) { + p := &Ping{ + Urls: []string{"localhost", "127.0.0.2"}, + Method: "native", + Count: 5, + Percentiles: []int{50, 95, 99}, + nativePingFunc: func(destination string) (*pingStats, error) { + s := &pingStats{ + Statistics: ping.Statistics{ + PacketsSent: 0, + PacketsRecv: 0, + }, + } + + return s, nil + }, + } + + var testAcc testutil.Accumulator + err := p.Init() + require.NoError(t, err) + p.pingToURLNative("localhost", &testAcc) + require.Zero(t, testAcc.Errors) + require.True(t, testAcc.HasField("ping", "result_code")) + require.Equal(t, 2, testAcc.Metrics[0].Fields["result_code"]) +} + +// Test failed DNS resolutions +func TestDNSLookupError(t *testing.T) { + p := &Ping{ + Count: 1, + Log: testutil.Logger{}, + Urls: []string{"localhost"}, + Method: "native", + IPv6: false, + nativePingFunc: func(destination string) (*pingStats, error) { + return nil, fmt.Errorf("unknown") + }, + } + + var testAcc testutil.Accumulator + err := p.Init() + require.NoError(t, err) + p.pingToURLNative("localhost", &testAcc) + require.Zero(t, testAcc.Errors) + require.True(t, testAcc.HasField("ping", "result_code")) + require.Equal(t, 1, testAcc.Metrics[0].Fields["result_code"]) +} From fd5e2c320a16b67263dadcd822f35c15e41134f7 Mon Sep 17 00:00:00 2001 From: Sebastian Spaink Date: Tue, 26 Jan 2021 15:16:04 -0600 Subject: [PATCH 7/7] Check for zero length --- plugins/inputs/ping/ping.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/inputs/ping/ping.go b/plugins/inputs/ping/ping.go index f4c0f304004c0..f242a80b85400 100644 --- a/plugins/inputs/ping/ping.go +++ b/plugins/inputs/ping/ping.go @@ -263,7 +263,7 @@ func (p durationSlice) Swap(i, j int) { p[i], p[j] = p[j], p[i] } // R7 from Hyndman and Fan (1996), which matches Excel func percentile(values durationSlice, perc int) time.Duration { - if len(values) < 0 { + if len(values) == 0 { return 0 } if perc < 0 {