prometheus-community/pro-bing

A library for creating continuous probers

pro-bing's Issues

Move CI to GitHub Actions

Currently the CI uses CircleCI. Given that Prometheus itself is soon moving to GitHub Actions exclusively, we should setup this project with GH directly.

Packet Loss is (arguably) calculated incorrectly

Responses to network requests are never instantaneous.

This library calculates packet loss by dividing the number of packets that have been sent and the number of packets that have been received. While I cannot say that the resulting number is "objectively incorrect," this does not take into account the fact that some delay between request and response should be expected.

For example, in the instant after sending our first packet, we have sent 1 packet and received 0 packets. This is technically 100% packet loss at this moment in time, but actually construing this as 100% packet loss probably isn't helpful.

I think this would be more useful if the number of sent packets used for this calculation were only incremented some (user-configurable?) time after the packet has actually been sent.

While it was straightforward to create my own wrapper with this functionality (using the OnSend and OnRecv callbacks), I'm wondering if anyone thinks it would make sense to add this functionality into the library itself.

Investigate using SO_TIMESTAMP for packet timing

Per this thread, we should be able to get more accurate RTT results if we use the Linux kernel to get timestamps (depending on the platform of course).

Resolve with parent context

To have proper cancellation, it would help to do all name resolution using the net.Resolver.LookupIP, which takes a context. It's already in use since version 0.4.0, but it uses context.Background as a base, not the context passed to RunWithContext. It would remove surprises like those in #64.

It would be a behaviour change, but not a very surprising one, and IMHO much less annoying than adding other APIs like ResolveWithContext and RunWithContextAndResolveWithContext.

Would you be open to such a PR?

Include packet data in Packet

Currently, there's no way to access the ICMP data on echo. This makes it impossible to see what the reason for a failed ping was, i'd love to be able to check the type and code of the received message.

J

stop pinger by id

Hi,
Is it possible to have multiple pinger running and stop one of them by something like id?

support add tos

support add tos to ping connection

Allow setting TTL

This in order to implement traceroute-like tools..

Use go-kit/log for logging

This would fit the rest of the Prometheus ecosystem better

linux icmp6 is error

c, e := icmp.ListenPacket("udp6", "")

error is
socket: protocol not supported

### Summary

Summary

A clear and concise description of what the problem is, and what you solution you want.

Changes

What changes this feature would introduce. Any risks these changes involve. Any alternative solutions or features you've considered.

Additional context

Add any other context or screenshots about the feature request here.

Originally posted by @Christopher799 in skills/introduction-to-github#456

download package error

---➤ go get -u github.com/prometheus-community/pro-bing
go: downloading github.com/prometheus-community/pro-bing v0.1.0
go: github.com/prometheus-community/[email protected]: verifying module: github.com/prometheus-community/[email protected]: checking tree#15554181 against tree#15705495: reading https://goproxy.io/sumdb/sum.golang.org/tile/8/1/237: 404 Not Found
        server response: not found

Pinger timeout should return an error

If the pinger reaches timeout, no error is returned from OnFinish() and it makes it seem like the probe finished successfully, even if nothing was returned from the ping job.

pro-bing/ping.go

Lines 538 to 539 in 0999adf

	case <-timeout.C:
	return nil

I believe this should return an error indicating the timeout was reached before the ping job finished fully. Happy to make the PR if this sounds good

Allow stopping a running ping with a Context

It could be helpful if there were a method like *Pinger.Run() that accepted a Context for cancellation. Would this be difficult to add given the current implementation?

Use github.com/campoy/embedmd to embed code snippets in README

Refactoring the README quite interesting. We should make use of embedmd to embed actual .go files into the README and not have them outdated. Also we get static type checking etc when using real Go files.

OnRecvError timeout errors

As soon as I add OnRecvError I get heaps of timeout errors (20+ errs / s )
read ip 0.0.0.0: raw-read ip4 0.0.0.0: i/o timeout
The pings themselves seem to work as expected.

Here's the pro-bing code. Am I doing something wrong?
This happens on macOS 13, 14, amd64/arm64, golang go1.22.2/3

	t.Pinger, err = probing.NewPinger(t.Address)
	if err != nil {
		Errorf("probing.NewPinger error %s", err)
		return
	}
	t.Pinger.SetPrivileged(true)
	t.Pinger.Size = t.Size
	t.Pinger.Interval = time.Second * time.Duration(t.Freq)

	t.Pinger.OnRecvError = func(err error) {
		Errorf("OnRecvError %s", err)
	}

	t.Pinger.OnSendError = func(pkt *probing.Packet, err error) {
		Errorf("OnSendError %s", err)
	}

	t.Pinger.OnDuplicateRecv = func(pkt *probing.Packet) {
		Errorf("OnDuplicateRecv %v", pkt)
	}

	t.Pinger.OnRecv = func(pkt *probing.Packet) {
		...
	}
	err = t.Pinger.Run()

VScode:could not import github.com/prometheus-community/pro-bing

import(
probing "github.com/prometheus-community/pro-bing"
)

Hit:
could not import github.com/prometheus-community/pro-bing (no required module provides package "github.com/prometheus-community/pro-bing")compilerBrokenImport

go.mod:
Go version: go 1.17
require(
....
github.com/prometheus-community/pro-bing v0.1.0 // indirect
....
)

I don't know why but anything is fine.

Not working on FreeBSD systems

My test steps are:

mkdir test
cd test
wget https://raw.githubusercontent.com/prometheus-community/pro-bing/main/cmd/ping/ping.go
go mod init test
go mod tidy

On my FreeBSD system, I get "protocol not supported" error:

root@FreeBSD # ~/test> go run ping.go 8.8.8.8
go run ping.go 8.8.8.8
PING 8.8.8.8 (8.8.8.8):
Failed to ping target host: socket: protocol not supported

The same steps on a Linux system:

root@ubuntu-cloud:~/test# go run ping.go 8.8.8.8
PING 8.8.8.8 (8.8.8.8):
32 bytes from 8.8.8.8: icmp_seq=0 time=15.968962ms ttl=118
32 bytes from 8.8.8.8: icmp_seq=1 time=16.025836ms ttl=118
32 bytes from 8.8.8.8: icmp_seq=2 time=16.281375ms ttl=118
^C
--- 8.8.8.8 ping statistics ---
4 packets transmitted, 3 packets received, 0 duplicates, 25% packet loss
round-trip min/avg/max/stddev = 15.968962ms/16.092057ms/16.281375ms/135.866µs

My FreeBSD system details are as follows:

freebsd-version -kru
13.2-RELEASE-p4
13.2-RELEASE-p4
13.2-RELEASE-p7

go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/kr0m/.cache/go-build"
GOENV="/home/kr0m/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="freebsd"
GOINSECURE=""
GOMODCACHE="/home/kr0m/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="freebsd"
GOPATH="/home/kr0m/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go120"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go120/pkg/tool/freebsd_amd64"
GOVCS=""
GOVERSION="go1.20.8"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="cc"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2645866918=/tmp/go-build -gno-record-gcc-switches"

If you require any further information feel free to contact me.

Memory bloat when sending pings to something which does not respond

When looking through the code I noticed the awaitingSequences map can grow infinitely if ICMP requests are never responded to. This causes increased memory bloat over time and is something I'd like to get rid of.

I know its a fairly slow bloat and most users probably use pro-bing in short lived contexts, but I'd like to have many pingers (100s) with very fast ping intervals (100 ms) and will be long lived (months). So far I haven't run into any issues but I don't want to wait for issues to pop up related to this in the future.

The original idea I had was to use timeouts per packet like what #19 wants. Once it times out, remove from awaitingSequences. This doesn't seem that good as we lose visibility into timed out duplicated ICMP responses while what we care about is the amount of resources a ping uses.

Instead I've looked into limiting the total number of in flight packets pro-bing tracks instead. This means we only lose visibility into duplicated and timed out packets when we're past a certain number of them.

I have a very early draft for this in https://github.com/prometheus-community/pro-bing/compare/main...TheRushingWookie:pro-bing:remove_bloat?expand=1

Its not clean or fully done, but it does limit the resources by using what is basically an evicting cache which generates the UUID/sequence number for pro-bing to send.

What do you guys think about this?

func `RunWithContext` didn't exit as expected

// RunWithContext runs the pinger with a context. This is a blocking function that will exit when it's
// done or if the context is canceled. If Count or Interval are not specified, it will run continuously until
// it is interrupted.
func (p *Pinger) RunWithContext(ctx context.Context) error {
...
	if p.ipaddr == nil {
		err = p.Resolve()
	}
...
	return p.run(ctx, conn)
}

function RunWithContext didn't exit as expected as it will block in p.Resolve(), it didn't exit even when ctx is canceled before it was passed in

Blocked while running on macOS.

Although this project does not support macOS, I tried it on macOS and found that if pinger.Run() is called for a long time, it will be blocked and the process needs to be manually killed. What is the reason for this?

Ping broadcast address, the original address is displayed instead of the actual reply address.

When ping is performed to broadcast address, the original destination address is provided in the callback.
However, in this case, the actual reply is sent from different address.
I want to provide the actual reply address in the callback, not the original address.

The following PR seems to fix the problem.
go-ping/ping#187.

Per Probe/Packet Timeout Option

On go-ping this PR existed:

go-ping/ping#202

It would be very usefull to have a packet timeout option on the Pinger in this package

Allow setting the fragmentation of packets option when sending a ping

Similar to how you can do:

ping -M do -s 1472 google.com

We should be able to tell pinger to not fragment packets if too large (due to MTU size).

The option seems to be available as https://pkg.go.dev/golang.org/x/net/ipv4#HeaderFlags (I think).

Decrease the number of digits after the decimal point

I noticed that it depends on the system. Linux has 6 digits (e.g. 120.125368 ms), MacOS has 3 digits. In my opinion this is excessive precision. Could you round it down to one digit?

feature request: bound interface

is it possible to provide some method to bind interface like ping do?

ping [-AaCDdfnoQqRrv] [-b boundif] 
     -b boundif
             Bind the socket to interface boundif for sending.  This option is an Apple addition.

use control function in ListenConfig is one option, but may need change x/icmp to net.ListenConfig

[Windows 10] A message sent on a datagram socket was larger than the internal message buffer

I get same error in windows 10 by use go-ping. can pro-bing fix this?

go-ping/ping#168

Feature request: count out of order packets received

How would you feel about adding a count of out-of-order responses received? In some wacky routing scenarios this would be useful.

pro-bing/ping.go

Line 703 in 6bee245

func (p *Pinger) processPacket(recv *packet) error {

Feature request: add HTTP support

I'd love to work on the implementation of HTTP support. I will try to maintain the same approach and interface (where possible) as the existing ICMP one.

CC: @SuperQ @davinci26 @shadialtarsha

High concurrency situation, returning packet loss,

High concurrency situation, returning packet loss,
2024/05/14 18:44:32 ping.go:201: D! no packets received, stats: &{{0 5 0 100 10.10.25.117 10.10.25.117 [] 0s 0s 0s 0s} 0}
but tcpdump has packet return
18:44:27.903699 IP 10.4.3.20 > 10.10.25.117: ICMP echo request, id 22102, seq 0, length 64
18:44:27.925773 IP 10.10.25.117> 10.4.3.20: ICMP echo reply, id 22102, seq 0, length 64
18:44:28.904025 IP 10.4.3.20 > 10.10.25.117: ICMP echo request, id 22102, seq 1, length 64
18:44:28.926564 IP 10.10.25.117 > 10.4.3.20: ICMP echo reply, id 22102, seq 1, length 64
18:44:29.903706 IP 10.4.3.20 > 10.10.25.117: ICMP echo request, id 22102, seq 2, length 64
18:44:29.928764 IP 10.10.25.117 > 10.4.3.20: ICMP echo reply, id 22102, seq 2, length 64
18:44:30.903703 IP 10.4.3.20 > 10.10.25.117: ICMP echo request, id 22102, seq 3, length 64
18:44:30.927299 IP 10.10.25.117 > 10.4.3.20: ICMP echo reply, id 22102, seq 3, length 64
18:44:31.903708 IP 10.4.3.20 > 10.10.25.117: ICMP echo request, id 22102, seq 4, length 64
18:44:31.926030 IP 10.10.25.117 > 10.4.3.20: ICMP echo reply, id 22102, seq 4, length 64

How to optimize

Why is the ping failing on Debian 12?

The following code works fine on Windows, Ubuntu 22.04, MacOS, and some others. But it fails on Debian 12.

I've tried running it as root vs a user with the same result.

I've also tried SetPrivileges and the same result.

How to resolve this?

Thanks

ping -c 1 8.8.8.8
PING 8.8.8.8 (8.8.8.8) 56(84) bytes of data.
64 bytes from 8.8.8.8: icmp_seq=1 ttl=115 time=4.19 ms

--- 8.8.8.8 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 4.190/4.190/4.190/0.000 ms
$ ./testping 
socket: permission denied
false
$ sudo ./testping 
socket: permission denied
false
$ 
$ cat /etc/os-release 
PRETTY_NAME="Debian GNU/Linux 12 (bookworm)"
NAME="Debian GNU/Linux"
VERSION_ID="12"
VERSION="12 (bookworm)"
VERSION_CODENAME=bookworm
ID=debian
HOME_URL="https://www.debian.org/"
SUPPORT_URL="https://www.debian.org/support"
BUG_REPORT_URL="https://bugs.debian.org/"
$ 

package main

import (
	"fmt"
	"runtime"
	"time"

	probing "github.com/prometheus-community/pro-bing"
)

func main() {

	isup := Ping("8.8.8.8")

	fmt.Println(isup)

}

func Ping(ip string) bool {
	pinger, err := probing.NewPinger(ip)
	if err != nil {
		return false
	}
	pinger.Count = 1
	pinger.Timeout = 250 * time.Millisecond
	if runtime.GOOS == "windows" {
		pinger.SetPrivileged(true)
	}
	err = pinger.Run()
	if err != nil {
		fmt.Println(err)
		return false
	}

	stats := pinger.Statistics()

	return stats.PacketsRecv > 0
}