Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: allow probers to provide a duration value #898

Merged
merged 1 commit into from
Sep 19, 2024
Merged

Fix: allow probers to provide a duration value #898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions internal/adhoc/adhoc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,13 +472,19 @@ func (r *runner) Run(ctx context.Context, tenantId model.GlobalID, publisher pus
rCtx, cancel := context.WithTimeout(ctx, r.timeout)
defer cancel()

if r.prober.Probe(rCtx, r.target, registry, logger) {
success, duration := r.prober.Probe(rCtx, r.target, registry, logger)

if success {
successGauge.Set(1)
} else {
successGauge.Set(0)
}

durationGauge.Set(float64(time.Since(start).Microseconds()) / 1e6)
if duration != 0 {
durationGauge.Set(duration)
} else {
durationGauge.Set(float64(time.Since(start).Microseconds()) / 1e6)
}

mfs, err := registry.Gather()

Expand Down
4 changes: 2 additions & 2 deletions internal/adhoc/adhoc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,13 +337,13 @@ func (p *testProber) Name() string {
return "test"
}

func (p *testProber) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) bool {
func (p *testProber) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) (bool, float64) {
p.logger.Info().Str("func", "Probe").Caller(0).Send()
g := prometheus.NewGauge(prometheus.GaugeOpts{
Name: "test",
})
g.Set(1)
registry.MustRegister(g)
_ = logger.Log("msg", "test")
return true
return true, 1
}
4 changes: 2 additions & 2 deletions internal/checks/checks_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,8 +441,8 @@ func (testProber) Name() string {
return "test-prober"
}

func (testProber) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) bool {
return false
func (testProber) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) (bool, float64) {
return false, 0
}

type testProbeFactory struct{}
Expand Down
7 changes: 4 additions & 3 deletions internal/prober/browser/browser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,13 @@ func (p Prober) Name() string {
return proberName
}

func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) bool {
func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) (bool, float64) {
success, err := p.processor.Run(ctx, registry, logger, p.logger)
if err != nil {
p.logger.Error().Err(err).Msg("running probe")
return false
return false, 0
}

return success
// TODO(mem): implement custom duration extraction.
return success, 0
}
4 changes: 2 additions & 2 deletions internal/prober/dns/dns.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (p Prober) Name() string {
return "dns"
}

func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) bool {
func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) (bool, float64) {
cfg := p.config

if p.experimental {
Expand Down Expand Up @@ -78,7 +78,7 @@ func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.R
// pass the server as the target parameter, and ignore the
// _target_ paramater that is passed to this function.

return bbeprober.ProbeDNS(ctx, p.target, cfg, registry, logger)
return bbeprober.ProbeDNS(ctx, p.target, cfg, registry, logger), 0
}

func settingsToModule(settings *sm.DnsSettings, target string) config.Module {
Expand Down
3 changes: 2 additions & 1 deletion internal/prober/dns/dns_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,9 +252,10 @@ func TestProberRetries(t *testing.T) {
logger := log.NewLogfmtLogger(&buf)

t0 := time.Now()
success := p.Probe(ctx, p.target, registry, logger)
success, duration := p.Probe(ctx, p.target, registry, logger)
t.Log(success, time.Since(t0))
require.True(t, success)
require.Equal(t, 0, duration)

mfs, err := registry.Gather()
require.NoError(t, err)
Expand Down
4 changes: 2 additions & 2 deletions internal/prober/grpc/grpc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ func (p Prober) Name() string {
return "grpc"
}

func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) bool {
return bbeprober.ProbeGRPC(ctx, target, p.config, registry, logger)
func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) (bool, float64) {
return bbeprober.ProbeGRPC(ctx, target, p.config, registry, logger), 0
}

func settingsToModule(ctx context.Context, settings *sm.GrpcSettings, logger zerolog.Logger) (config.Module, error) {
Expand Down
4 changes: 2 additions & 2 deletions internal/prober/http/http.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,13 @@ func (p Prober) Name() string {
return "http"
}

func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) bool {
func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) (bool, float64) {
if p.cacheBustingQueryParamName != "" {
// FIXME(mem): the second target argument should be the probe's name
target = addCacheBustParam(target, p.cacheBustingQueryParamName, target)
}

return bbeprober.ProbeHTTP(ctx, target, p.config, registry, logger)
return bbeprober.ProbeHTTP(ctx, target, p.config, registry, logger), 0
}

func settingsToModule(ctx context.Context, settings *sm.HttpSettings, logger zerolog.Logger) (config.Module, error) {
Expand Down
5 changes: 4 additions & 1 deletion internal/prober/http/http_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,10 @@ func TestProbe(t *testing.T) {

prober, err := NewProber(ctx, check, zl, http.Header{})
require.NoError(t, err)
require.Equal(t, tc.expectFailure, !prober.Probe(ctx, check.Target, registry, kl))

success, duration := prober.Probe(ctx, check.Target, registry, kl)
require.Equal(t, tc.expectFailure, !success)
require.Equal(t, float64(0), duration)

mfs, err := registry.Gather()
require.NoError(t, err)
Expand Down
4 changes: 2 additions & 2 deletions internal/prober/icmp/icmp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (p Prober) Name() string {
return "ping"
}

func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) bool {
func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) (bool, float64) {
return probeICMP(ctx, target, p.config, registry, logger)
}

Expand Down Expand Up @@ -108,7 +108,7 @@ func isPrivilegedRequired() bool {
}
)

success := probeICMP(ctx, target, config, registry, logger)
success, _ := probeICMP(ctx, target, config, registry, logger)

privilegedRequired = !success
privilegedCheckDone = true
Expand Down
16 changes: 10 additions & 6 deletions internal/prober/icmp/icmp_impl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"github.com/prometheus/client_golang/prometheus"
)

func probeICMP(ctx context.Context, target string, module Module, registry *prometheus.Registry, logger log.Logger) (success bool) {
func probeICMP(ctx context.Context, target string, module Module, registry *prometheus.Registry, logger log.Logger) (success bool, duration float64) {
var (
durationGaugeVec = prometheus.NewGaugeVec(prometheus.GaugeOpts{
Name: "probe_icmp_duration_seconds",
Expand Down Expand Up @@ -64,10 +64,11 @@ func probeICMP(ctx context.Context, target string, module Module, registry *prom
dstIPAddr, lookupTime, err := chooseProtocol(ctx, module.ICMP.IPProtocol, module.ICMP.IPProtocolFallback, target, int(module.MaxResolveRetries), registry, logger)
if err != nil {
_ = level.Error(logger).Log("msg", "Error resolving address", "err", err)
return false
return false, 0
}

durationGaugeVec.WithLabelValues("resolve").Add(lookupTime)
duration += lookupTime

pinger := ping.New(dstIPAddr.String())

Expand All @@ -76,7 +77,7 @@ func probeICMP(ctx context.Context, target string, module Module, registry *prom
if err := pinger.Resolve(); err != nil {
// This should never happen, the address is already resolved.
_ = level.Error(logger).Log("msg", "Error resolving address", "err", err)
return false
return false, 0
}

pinger.Timeout = module.Timeout
Expand All @@ -90,7 +91,9 @@ func probeICMP(ctx context.Context, target string, module Module, registry *prom

pinger.OnSetup = func() {
if !setupDone {
durationGaugeVec.WithLabelValues("setup").Add(time.Since(setupStart).Seconds())
setupDuration := time.Since(setupStart).Seconds()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the actual change I need. :-P

durationGaugeVec.WithLabelValues("setup").Add(setupDuration)
duration += setupDuration
setupDone = true
}
_ = level.Info(logger).Log("msg", "Using source address", "srcIP", pinger.Source)
Expand All @@ -116,6 +119,7 @@ func probeICMP(ctx context.Context, target string, module Module, registry *prom

pinger.OnFinish = func(stats *ping.Statistics) {
durationGaugeVec.WithLabelValues("rtt").Set(stats.AvgRtt.Seconds())
duration += stats.AvgRtt.Seconds()
durationMaxGauge.Set(stats.MaxRtt.Seconds())
durationMinGauge.Set(stats.MinRtt.Seconds())
durationStddevGauge.Set(stats.StdDevRtt.Seconds())
Expand Down Expand Up @@ -148,10 +152,10 @@ func probeICMP(ctx context.Context, target string, module Module, registry *prom

if err := pinger.Run(); err != nil {
_ = level.Info(logger).Log("msg", "failed to run ping", "err", err.Error())
return false
return false, 0
}

return pinger.PacketsSent >= int(module.ReqSuccessCount) && pinger.PacketsRecv >= int(module.ReqSuccessCount)
return pinger.PacketsSent >= int(module.ReqSuccessCount) && pinger.PacketsRecv >= int(module.ReqSuccessCount), duration
}

type icmpLogger struct {
Expand Down
3 changes: 2 additions & 1 deletion internal/prober/icmp/icmp_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,9 @@ func TestProber(t *testing.T) {
logger := log.NewLogfmtLogger(os.Stdout)
require.NotNil(t, logger)

success := prober.Probe(ctx, "127.0.0.1", registry, logger)
success, duration := prober.Probe(ctx, "127.0.0.1", registry, logger)
require.True(t, success)
require.Greater(t, duration, float64(0))
}

func TestBBEProber(t *testing.T) {
Expand Down
7 changes: 4 additions & 3 deletions internal/prober/multihttp/multihttp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,15 @@ func (p Prober) Name() string {
return proberName
}

func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) bool {
func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) (bool, float64) {
success, err := p.processor.Run(ctx, registry, logger, p.logger)
if err != nil {
p.logger.Error().Err(err).Msg("running probe")
return false
return false, 0
}

return success
// TODO(mem): implement custom duration extraction.
return success, 0
}

// Overrides any user-provided headers with our own augmented values
Expand Down
3 changes: 2 additions & 1 deletion internal/prober/multihttp/script_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -862,11 +862,12 @@ func TestSettingsToScript(t *testing.T) {
userLogger := level.NewFilter(kitlog.NewLogfmtLogger(&buf), level.AllowInfo(), level.SquelchNoLevel(false))
require.NotNil(t, userLogger)

success := prober.Probe(ctx, check.Target, reg, userLogger)
success, duration := prober.Probe(ctx, check.Target, reg, userLogger)

t.Log("Log entries:\n" + buf.String())

require.True(t, success)
require.Equal(t, float64(0), duration)
}

func TestReplaceVariablesInString(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions internal/prober/prober.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ const unsupportedCheckType = error_types.BasicError("unsupported check type")

type Prober interface {
Name() string
Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) bool
Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) (bool, float64)
}

func Run(ctx context.Context, p Prober, target string, registry *prometheus.Registry, logger logger.Logger) bool {
func Run(ctx context.Context, p Prober, target string, registry *prometheus.Registry, logger logger.Logger) (bool, float64) {
return p.Probe(ctx, target, registry, logger)
}

Expand Down
7 changes: 4 additions & 3 deletions internal/prober/scripted/scripted.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,13 @@ func (p Prober) Name() string {
return proberName
}

func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) bool {
func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) (bool, float64) {
success, err := p.processor.Run(ctx, registry, logger, p.logger)
if err != nil {
p.logger.Error().Err(err).Msg("running probe")
return false
return false, 0
}

return success
// TODO(mem): implement custom duration extraction.
return success, 0
}
4 changes: 2 additions & 2 deletions internal/prober/tcp/tcp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ func (p Prober) Name() string {
return "tcp"
}

func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) bool {
return bbeprober.ProbeTCP(ctx, target, p.config, registry, logger)
func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) (bool, float64) {
return bbeprober.ProbeTCP(ctx, target, p.config, registry, logger), 0
}

func settingsToModule(ctx context.Context, settings *sm.TcpSettings, logger zerolog.Logger) (config.Module, error) {
Expand Down
10 changes: 5 additions & 5 deletions internal/prober/traceroute/traceroute.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (p Prober) Name() string {
return "traceroute"
}

func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) bool {
func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.Registry, logger logger.Logger) (bool, float64) {
m, ch, err := mtr.NewMTR(
target,
p.config.srcAddr,
Expand All @@ -70,9 +70,9 @@ func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.R
logErr := logger.Log(err)
if logErr != nil {
p.logger.Error().Err(logErr).Msg("logging error")
return false
return false, 0
}
return false
return false, 0
}

go func(ch <-chan struct{}) {
Expand Down Expand Up @@ -130,7 +130,7 @@ func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.R
_, err = traceHash.Write([]byte(hostsString))
if err != nil {
p.logger.Error().Err(err).Msg("computing trace hash")
return false
return false, 0
}

var traceHashGauge = prometheus.NewGauge(prometheus.GaugeOpts{
Expand Down Expand Up @@ -158,7 +158,7 @@ func (p Prober) Probe(ctx context.Context, target string, registry *prometheus.R
overallPacketLoss := totalPacketsLost / totalPacketsSent
overallPacketLossGauge.Set(overallPacketLoss)

return success
return success, 0
}

func settingsToModule(settings *sm.TracerouteSettings) Module {
Expand Down
12 changes: 8 additions & 4 deletions internal/scraper/scraper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -668,9 +668,13 @@ func runProber(
checkCtx, cancel := context.WithTimeout(ctx, timeout)
defer cancel()

success := prober.Probe(checkCtx, target, registry, logger)
success, duration := prober.Probe(checkCtx, target, registry, logger)

duration := time.Since(start).Seconds()
probeDuration := time.Since(start).Seconds()

if duration == 0 { // If the prober did not provide their own duration, fallback to probeDuration.
duration = probeDuration
}

probeSuccessGauge := prometheus.NewGauge(prometheus.GaugeOpts{
Name: ProbeSuccessMetricName,
Expand All @@ -694,10 +698,10 @@ func runProber(

if success {
probeSuccessGauge.Set(1)
_ = level.Info(logger).Log("msg", "Check succeeded", "duration_seconds", duration)
_ = level.Info(logger).Log("msg", "Check succeeded", "duration_seconds", probeDuration)
} else {
probeSuccessGauge.Set(0)
_ = level.Error(logger).Log("msg", "Check failed", "duration_seconds", duration)
_ = level.Error(logger).Log("msg", "Check failed", "duration_seconds", probeDuration)
}

smCheckInfo.Set(1)
Expand Down
Loading