Record real ICMP RTT as ping latency, not wall-clock

The ping check spaces probes 200ms apart, so with count > 1 the
wall-clock duration of Check() is dominated by the inter-probe
interval, not the round-trip time. Add an optional LatencyReporter
interface consulted by runOnce; the ping check reports AvgRtt from
the run, falling back to wall-clock on failure. Other check types
are unaffected.

Fixes #1

Author: brvier

internal/check/check.go

@@ -24,6 +24,14 @@ type Checker interface {
 	Check(ctx context.Context) (ok bool, message string)
 }
 
+// LatencyReporter is optionally implemented by checkers that measure a truer
+// latency than the wall-clock duration of Check (e.g. ping's ICMP RTT, which
+// excludes the inter-probe interval). A non-positive value means no
+// measurement is available and the wall-clock duration is used instead.
+type LatencyReporter interface {
+	Latency() time.Duration
+}
+
 // New builds a Checker from a validated monitor config.
 func New(m config.Monitor) (Checker, error) {
 	var (
@@ -87,11 +95,17 @@ func runOnce(ctx context.Context, m config.Monitor, c Checker) Result {
 	defer cancel()
 	start := time.Now()
 	ok, msg := c.Check(cctx)
+	latency := time.Since(start)
+	if lr, isLR := c.(LatencyReporter); isLR {
+		if l := lr.Latency(); l > 0 {
+			latency = l
+		}
+	}
 	return Result{
 		Monitor: m.Name,
 		Time:    start,
 		OK:      ok,
-		Latency: time.Since(start),
+		Latency: latency,
 		Message: msg,
 	}
 }

internal/check/check_test.go

@@ -0,0 +1,31 @@
+package check
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"gjallar/internal/config"
+)
+
+type fakeReporter struct {
+	rtt time.Duration
+}
+
+func (f *fakeReporter) Check(ctx context.Context) (bool, string) { return true, "" }
+func (f *fakeReporter) Latency() time.Duration                   { return f.rtt }
+
+func TestRunOnceLatencyReporter(t *testing.T) {
+	m := config.Monitor{Name: "t", Timeout: config.Duration(time.Second)}
+
+	r := runOnce(context.Background(), m, &fakeReporter{rtt: 300 * time.Microsecond})
+	if r.Latency != 300*time.Microsecond {
+		t.Errorf("Latency = %v, want reported 300µs", r.Latency)
+	}
+
+	// A non-positive report falls back to the wall-clock duration.
+	r = runOnce(context.Background(), m, &fakeReporter{rtt: 0})
+	if r.Latency <= 0 {
+		t.Errorf("Latency = %v, want wall-clock fallback > 0", r.Latency)
+	}
+}

internal/check/ping.go

@@ -15,6 +15,7 @@ type pingCheck struct {
 	count      int
 	privileged bool
 	timeout    time.Duration
+	lastRTT    time.Duration
 }
 
 func newPingCheck(m config.Monitor) (*pingCheck, error) {
@@ -27,6 +28,7 @@ func newPingCheck(m config.Monitor) (*pingCheck, error) {
 }
 
 func (c *pingCheck) Check(ctx context.Context) (bool, string) {
+	c.lastRTT = 0
 	pinger, err := probing.NewPinger(c.host)
 	if err != nil {
 		return false, err.Error()
@@ -43,12 +45,18 @@ func (c *pingCheck) Check(ctx context.Context) (bool, string) {
 	if stats.PacketsRecv == 0 {
 		return false, fmt.Sprintf("no reply from %s (%d packets sent)", c.host, stats.PacketsSent)
 	}
+	c.lastRTT = stats.AvgRtt
 	if stats.PacketLoss > 0 {
 		return true, fmt.Sprintf("%.0f%% loss, avg %s", stats.PacketLoss, stats.AvgRtt.Round(time.Millisecond))
 	}
 	return true, ""
 }
 
+// Latency reports the measured ICMP round-trip time instead of the wall-clock
+// duration of Check, which includes the 200ms inter-probe interval. Safe
+// without locking: each monitor's Checker runs sequentially in one goroutine.
+func (c *pingCheck) Latency() time.Duration { return c.lastRTT }
+
 // SelfTestPing pings 127.0.0.1 once so a missing capability or sysctl fails
 // at startup with an actionable message instead of every check failing forever.
 func SelfTestPing(privileged bool) error {

internal/check/ping_test.go

@@ -22,6 +22,10 @@ func TestPingLocalhost(t *testing.T) {
 	if ok, msg := c.Check(context.Background()); !ok {
 		t.Errorf("ping 127.0.0.1 failed: %s", msg)
 	}
+	// Latency must be the ICMP RTT, not the ~200ms inter-probe interval.
+	if rtt := c.Latency(); rtt <= 0 || rtt >= 200*time.Millisecond {
+		t.Errorf("Latency() = %v, want a real localhost RTT", rtt)
+	}
 }
 
 func TestPingUnreachable(t *testing.T) {
@@ -39,4 +43,7 @@ func TestPingUnreachable(t *testing.T) {
 	if ok, msg := c.Check(context.Background()); ok {
 		t.Errorf("expected failure, got ok (%s)", msg)
 	}
+	if rtt := c.Latency(); rtt != 0 {
+		t.Errorf("Latency() = %v after failed check, want 0", rtt)
+	}
 }