prometheus-community / pro-bing

A library for creating continuous probers
MIT License
309 stars 52 forks source link

Add context based Run API #29

Closed TheRushingWookie closed 1 year ago

TheRushingWookie commented 1 year ago

Description

This adds a context based API. This is useful for code which has an existing context and wants to rely on that instead of using a timeout or a count of packets to stop the pinger. When the context is canceled, the ping object is stopped.

I've also added a mutex to the tests as there is a data race detected in testPacketConnOK See

make test                                                                                                
>> running all tests
GO111MODULE= go test -race -cover  ./...
==================
WARNING: DATA RACE
Write at 0x00c00018f7a8 by goroutine 17:
  github.com/prometheus-community/pro-bing.(*testPacketConnOK).WriteTo()
      /Users/qjarrell/repos/pro-bing/ping_test.go:720 +0x75
  github.com/prometheus-community/pro-bing.(*Pinger).sendICMP()
      /Users/qjarrell/repos/pro-bing/ping.go:752 +0x92a
  github.com/prometheus-community/pro-bing.(*Pinger).runLoop()
      /Users/qjarrell/repos/pro-bing/ping.go:509 +0x4c4
  github.com/prometheus-community/pro-bing.(*Pinger).run.func3()
      /Users/qjarrell/repos/pro-bing/ping.go:463 +0xcf
  golang.org/x/sync/errgroup.(*Group).Go.func1()
      /Users/qjarrell/git/go/1.18.2/pkg/mod/golang.org/x/sync@v0.1.0/errgroup/errgroup.go:75 +0x86

Previous read at 0x00c00018f7a8 by goroutine 14:
  github.com/prometheus-community/pro-bing.(*testPacketConnOK).ReadFrom()
      /Users/qjarrell/repos/pro-bing/ping_test.go:731 +0x55
  github.com/prometheus-community/pro-bing.(*Pinger).recvICMP()
      /Users/qjarrell/repos/pro-bing/ping.go:605 +0x1cd
  github.com/prometheus-community/pro-bing.(*Pinger).run.func2()
      /Users/qjarrell/repos/pro-bing/ping.go:458 +0xcf
  golang.org/x/sync/errgroup.(*Group).Go.func1()
      /Users/qjarrell/git/go/1.18.2/pkg/mod/golang.org/x/sync@v0.1.0/errgroup/errgroup.go:75 +0x86
==================

Compatibility

This change is backwards compatible for the current API but brings in a new dependency, context, which was added in Go 1.7. If a user is on an older than 1.7 version then they will not be able to move to the new version of pro-bing without upgrading their go version.

Testing

Added two unit tests

SuperQ commented 1 year ago

We only support the last 2 Go releases, so there's no need to worry about Go older than 1.18.

TheRushingWookie commented 1 year ago

Fixed lint issues