From a0953c03a73c64e6847f71cf05f299c717bed547 Mon Sep 17 00:00:00 2001 From: Kakuya Ando Date: Wed, 23 Sep 2026 16:15:06 +0900 Subject: [PATCH 1/2] feat(batch): add json/csv output, max-loss exit code, and RTT jitter - Add -o/--output (table, json, csv) for machine-readable batch results - Add --max-loss to exit with status 2 when any target exceeds the threshold - Add -I/--interface to batch for parity with interactive mode - Track RTT jitter (stddev, equivalent to ping's mdev) and show it in host details and json/csv output - Write progress to stderr only when it is a terminal and disable the failure beep in batch mode so stdout stays clean for pipes - Fix --count sending count+1 probes by stopping half an interval after the last round instead of racing the next tick Co-Authored-By: Claude Opus 5.5 --- README.md | 14 +++ cmd/mping/main.go | 5 + internal/command/batch.go | 108 ++++++++++++++++++---- internal/command/batch_test.go | 82 +++++++++++++++++ internal/command/errors.go | 20 ++++ internal/stats/interface.go | 1 + internal/stats/metric.go | 19 ++++ internal/stats/metric_test.go | 28 ++++++ internal/ui/shared/export.go | 148 ++++++++++++++++++++++++++++++ internal/ui/shared/export_test.go | 95 +++++++++++++++++++ internal/ui/shared/formatters.go | 2 + 11 files changed, 506 insertions(+), 16 deletions(-) create mode 100644 internal/command/batch_test.go create mode 100644 internal/command/errors.go create mode 100644 internal/ui/shared/export.go create mode 100644 internal/ui/shared/export_test.go diff --git a/README.md b/README.md index c6f21ed..c51946e 100644 --- a/README.md +++ b/README.md @@ -92,8 +92,22 @@ mping batch --count 5 192.168.1.0/29 # Use with external host lists mping batch -f hosts.txt --count 5 + +# Machine-readable output (json / csv) for scripts and dashboards +mping batch -o json --count 5 10.0.0.0/29 | jq '.[] | select(.loss_percent > 0) | .host' +mping batch -o csv --count 10 -f hosts.txt > result.csv + +# Health check: exit with status 2 if any target's loss exceeds 20% +mping batch --max-loss 20 -f hosts.txt || alert "some targets are unhealthy" ``` +Batch mode details: +- `--count N` sends exactly N probes per target. +- `-o, --output` selects `table` (default), `json`, or `csv`. RTT fields are in milliseconds and include `jitter_ms` (standard deviation of RTT, same as `mdev` in iputils ping). +- `--max-loss PCT` turns batch into a health check. Exit codes: `0` = OK, `1` = usage/config error, `2` = at least one target exceeded the threshold (violating targets are listed on stderr). +- Progress dots are written to stderr only when it is a terminal, so stdout stays clean for pipes. The failure beep is disabled in batch mode. +- `-I, --interface` selects the source interface, same as the interactive mode. + ## Target Expansion mping supports powerful target expansion syntax for monitoring large numbers of hosts efficiently: diff --git a/cmd/mping/main.go b/cmd/mping/main.go index 2ef4d4a..05dea10 100644 --- a/cmd/mping/main.go +++ b/cmd/mping/main.go @@ -1,6 +1,7 @@ package main import ( + "errors" "fmt" "os" "runtime" @@ -31,6 +32,10 @@ func Execute() { cmd.SetOut(os.Stderr) cmd.SetErr(os.Stderr) cmd.Println(err) + var exitErr *command.ExitError + if errors.As(err, &exitErr) { + os.Exit(exitErr.Code) + } os.Exit(1) } } diff --git a/internal/command/batch.go b/internal/command/batch.go index e08c0d7..ea32c8b 100644 --- a/internal/command/batch.go +++ b/internal/command/batch.go @@ -4,9 +4,12 @@ import ( "context" "errors" "fmt" + "io" + "os" + "slices" + "strings" "time" - "github.com/jedib0t/go-pretty/v6/table" "github.com/spf13/cobra" "github.com/servak/mping/internal/config" @@ -23,13 +26,18 @@ func NewPingBatchCmd() *cobra.Command { Example: `mping batch 1.1.1.1 8.8.8.8 mping batch icmpv6:google.com mping batch http://google.com -mping batch dns://8.8.8.8/google.com`, +mping batch dns://8.8.8.8/google.com +mping batch -o json 10.0.0.0/29 | jq '.[] | select(.loss_percent > 0)' +mping batch --max-loss 20 -f hosts.txt || echo "some targets are unhealthy"`, RunE: func(cmd *cobra.Command, args []string) error { flags := cmd.Flags() counter, err := flags.GetInt("count") if err != nil { return err } + if counter <= 0 { + return errors.New("count must be greater than zero") + } interval, err := flags.GetInt("interval") if err != nil { return err @@ -53,6 +61,26 @@ mping batch dns://8.8.8.8/google.com`, if err != nil { return err } + sourceInterface, err := flags.GetString("interface") + if err != nil { + return err + } + output, err := flags.GetString("output") + if err != nil { + return err + } + if !slices.Contains(shared.OutputFormats, output) { + return fmt.Errorf("unsupported output format %q (supported: %s)", output, strings.Join(shared.OutputFormats, ", ")) + } + maxLoss := -1.0 // disabled unless explicitly set + if flags.Changed("max-loss") { + if maxLoss, err = flags.GetFloat64("max-loss"); err != nil { + return err + } + if maxLoss < 0 || maxLoss > 100 { + return fmt.Errorf("max-loss must be between 0 and 100, got %v", maxLoss) + } + } hosts := ExpandTargets(args, filename) if len(hosts) == 0 { @@ -65,12 +93,18 @@ mping batch dns://8.8.8.8/google.com`, if cfg == nil { return fmt.Errorf("failed to load config %q: %w", path, err) } + cfg.SetSourceInterface(sourceInterface) _interval := time.Duration(interval) * time.Millisecond _timeout := time.Duration(timeout) * time.Millisecond // Create ProbeManager and MetricsManager probeManager := prober.NewProbeManager(cfg.Prober, cfg.Default) metricsManager := stats.NewMetricsManager() + // The terminal bell would corrupt machine-readable output and is + // pointless in non-interactive runs. + if metricsManager.IsBeepEnabled() { + metricsManager.ToggleBeep() + } // Add targets err = probeManager.AddTargets(hosts...) @@ -81,43 +115,85 @@ mping batch dns://8.8.8.8/google.com`, // Subscribe to events for metrics collection done := metricsManager.Subscribe(probeManager.Events()) - // Start probing with timeout context - ctx, cancel := context.WithTimeout(context.Background(), time.Duration(counter)*_interval) + ctx, cancel := context.WithCancel(context.Background()) defer cancel() - cmd.Print("probe") + // Progress goes to stderr (only when it is a terminal) so that + // stdout stays clean for piping into other tools. + progress := progressWriter() + fmt.Fprint(progress, "probe") go func() { if err := probeManager.Run(ctx, _interval, _timeout); err != nil { - fmt.Printf("ProbeManager error: %v\n", err) + fmt.Fprintf(os.Stderr, "ProbeManager error: %v\n", err) } }() - // Wait for specified duration - for counter > 0 { - counter-- - cmd.Print(".") + // Probers send the first round immediately and then one round per + // interval, so round N goes out at (N-1)*interval. Stopping half an + // interval after the last round yields exactly `count` probes without + // racing the next tick. Stop waits for in-flight probes to finish. + fmt.Fprint(progress, ".") + for i := 1; i < counter; i++ { time.Sleep(_interval) + fmt.Fprint(progress, ".") } + time.Sleep(_interval / 2) // Stop probing + cancel() probeManager.Stop() <-done // wait for in-flight metric updates to settle - cmd.Print("\r") + fmt.Fprint(progress, "\r\033[K") + metrics := metricsManager.SortBy(stats.Success, true) - tableData := shared.NewTableData(metrics, stats.Success, true) - t := tableData.ToGoPrettyTable() - t.SetStyle(table.StyleLight) - cmd.Println(t.Render()) - return nil + if err := shared.WriteReport(cmd.OutOrStdout(), output, metrics, stats.Success, true); err != nil { + return err + } + return checkMaxLoss(metrics, maxLoss) }, } flags := cmd.Flags() flags.StringP("filename", "f", "", "use contents of file") flags.StringP("config", "c", "~/.mping.yml", "config path") + flags.StringP("interface", "I", "", "source interface (name or IP address)") flags.IntP("interval", "i", 1000, "interval(ms)") flags.IntP("timeout", "t", 1000, "timeout(ms)") flags.IntP("count", "", 10, "repeat count") + flags.StringP("output", "o", shared.FormatTable, "output format ("+strings.Join(shared.OutputFormats, ", ")+")") + flags.Float64("max-loss", 0, fmt.Sprintf("exit with status %d if any target's loss(%%) exceeds this value", ExitCodeThresholdExceeded)) return cmd } + +// checkMaxLoss returns an ExitError listing targets whose loss exceeds maxLoss. +// A negative maxLoss disables the check. +func checkMaxLoss(metrics []stats.Metrics, maxLoss float64) error { + if maxLoss < 0 { + return nil + } + var violated []string + for _, m := range metrics { + if m.GetLoss() > maxLoss { + violated = append(violated, fmt.Sprintf("%s(%.1f%%)", m.GetName(), m.GetLoss())) + } + } + if len(violated) == 0 { + return nil + } + return &ExitError{ + Code: ExitCodeThresholdExceeded, + Err: fmt.Errorf("%d target(s) exceeded max loss %.1f%%: %s", + len(violated), maxLoss, strings.Join(violated, ", ")), + } +} + +// progressWriter returns stderr when it is attached to a terminal, +// otherwise a writer that discards progress output. +func progressWriter() io.Writer { + fi, err := os.Stderr.Stat() + if err != nil || fi.Mode()&os.ModeCharDevice == 0 { + return io.Discard + } + return os.Stderr +} diff --git a/internal/command/batch_test.go b/internal/command/batch_test.go new file mode 100644 index 0000000..b71f51f --- /dev/null +++ b/internal/command/batch_test.go @@ -0,0 +1,82 @@ +package command + +import ( + "errors" + "strings" + "testing" + "time" + + "github.com/servak/mping/internal/stats" +) + +func TestCheckMaxLoss(t *testing.T) { + now := time.Now() + metrics := []stats.Metrics{ + stats.NewMetricsForTest("ok", 1, 10, 10, 0, 0, 0, 0, 0, 0, 0, now, time.Time{}, ""), + stats.NewMetricsForTest("lossy", 1, 10, 7, 3, 30, 0, 0, 0, 0, 0, now, now, "timeout"), + stats.NewMetricsForTest("down", 1, 10, 0, 10, 100, 0, 0, 0, 0, 0, time.Time{}, now, "timeout"), + } + + tests := []struct { + name string + maxLoss float64 + wantErr bool + wantHosts []string + }{ + {"disabled", -1, false, nil}, + {"zero tolerance", 0, true, []string{"lossy", "down"}}, + {"boundary is inclusive", 30, true, []string{"down"}}, + {"all within limit", 100, false, nil}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := checkMaxLoss(metrics, tt.maxLoss) + if !tt.wantErr { + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + return + } + var exitErr *ExitError + if !errors.As(err, &exitErr) { + t.Fatalf("expected *ExitError, got %v", err) + } + if exitErr.Code != ExitCodeThresholdExceeded { + t.Errorf("exit code = %d, want %d", exitErr.Code, ExitCodeThresholdExceeded) + } + for _, h := range tt.wantHosts { + if !strings.Contains(err.Error(), h+"(") { + t.Errorf("error %q should mention %s", err, h) + } + } + if strings.Contains(err.Error(), "ok(") { + t.Errorf("error %q should not mention healthy host", err) + } + }) + } +} + +func TestBatchCmdRejectsInvalidFlags(t *testing.T) { + tests := []struct { + name string + args []string + want string + }{ + {"bad output", []string{"-o", "xml", "127.0.0.1"}, "unsupported output format"}, + {"bad max-loss", []string{"--max-loss", "150", "127.0.0.1"}, "max-loss must be between"}, + {"bad count", []string{"--count", "0", "127.0.0.1"}, "count must be greater than zero"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cmd := NewPingBatchCmd() + cmd.SetArgs(tt.args) + cmd.SilenceUsage = true + cmd.SilenceErrors = true + err := cmd.Execute() + if err == nil || !strings.Contains(err.Error(), tt.want) { + t.Errorf("Execute() error = %v, want containing %q", err, tt.want) + } + }) + } +} diff --git a/internal/command/errors.go b/internal/command/errors.go new file mode 100644 index 0000000..d591612 --- /dev/null +++ b/internal/command/errors.go @@ -0,0 +1,20 @@ +package command + +// ExitError carries a specific process exit code alongside an error. +// It lets automation distinguish "probing worked but targets were unhealthy" +// from usage or configuration errors (which exit with status 1). +type ExitError struct { + Code int + Err error +} + +func (e *ExitError) Error() string { + return e.Err.Error() +} + +func (e *ExitError) Unwrap() error { + return e.Err +} + +// ExitCodeThresholdExceeded is returned when targets violate a health threshold +const ExitCodeThresholdExceeded = 2 diff --git a/internal/stats/interface.go b/internal/stats/interface.go index 4866d7a..b8bad46 100644 --- a/internal/stats/interface.go +++ b/internal/stats/interface.go @@ -17,6 +17,7 @@ type Metrics interface { GetAverageRTT() time.Duration GetMinimumRTT() time.Duration GetMaximumRTT() time.Duration + GetJitter() time.Duration GetLastSuccTime() time.Time GetLastFailTime() time.Time GetLastFailDetail() string diff --git a/internal/stats/metric.go b/internal/stats/metric.go index 9108969..7720c21 100644 --- a/internal/stats/metric.go +++ b/internal/stats/metric.go @@ -1,6 +1,7 @@ package stats import ( + "math" "time" ) @@ -41,6 +42,7 @@ type metrics struct { MinimumRTT time.Duration MaximumRTT time.Duration LastRTT time.Duration + sumSquaredRTT float64 // sum of rtt^2 in ns^2, used to derive jitter LastFailTime time.Time LastSuccTime time.Time LastFailDetail string @@ -52,6 +54,7 @@ func (m *metrics) Success(rtt time.Duration, sentTime time.Time) { m.LastSuccTime = sentTime m.LastRTT = rtt m.TotalRTT += rtt + m.sumSquaredRTT += float64(rtt) * float64(rtt) m.AverageRTT = m.TotalRTT / time.Duration(m.Successful) if m.MinimumRTT == 0 || rtt < m.MinimumRTT { m.MinimumRTT = rtt @@ -87,6 +90,7 @@ func (m *metrics) Reset() { m.MinimumRTT = time.Duration(0) m.MaximumRTT = time.Duration(0) m.LastRTT = time.Duration(0) + m.sumSquaredRTT = 0 m.LastFailTime = time.Time{} m.LastSuccTime = time.Time{} m.LastFailDetail = "" @@ -133,6 +137,21 @@ func (m *metrics) GetMaximumRTT() time.Duration { return m.MaximumRTT } +// GetJitter returns the standard deviation of successful RTTs +// (equivalent to "mdev" reported by iputils ping). +func (m *metrics) GetJitter() time.Duration { + if m.Successful < 2 { + return 0 + } + n := float64(m.Successful) + mean := float64(m.TotalRTT) / n + variance := m.sumSquaredRTT/n - mean*mean + if variance <= 0 { + return 0 + } + return time.Duration(math.Sqrt(variance)) +} + func (m *metrics) GetLastSuccTime() time.Time { return m.LastSuccTime } diff --git a/internal/stats/metric_test.go b/internal/stats/metric_test.go index eb4c655..8b2bd2d 100644 --- a/internal/stats/metric_test.go +++ b/internal/stats/metric_test.go @@ -49,3 +49,31 @@ func TestMetrics(t *testing.T) { t.Errorf("Invalid loss calculation: Loss = %f", m.GetLoss()) } } + +func TestMetricsJitter(t *testing.T) { + m := NewMetrics("", 1) + impl := m.(*metrics) + now := time.Now() + + impl.Success(10*time.Millisecond, now) + if got := m.GetJitter(); got != 0 { + t.Errorf("jitter with a single sample should be 0, got %v", got) + } + + // Samples: 10, 20, 30, 40ms -> population stddev = sqrt(125)ms ≈ 11.18ms + impl.Success(20*time.Millisecond, now) + impl.Success(30*time.Millisecond, now) + impl.Success(40*time.Millisecond, now) + // Failures must not affect jitter + impl.Fail(now, "timeout") + + want := 11180339 * time.Nanosecond + if diff := m.GetJitter() - want; diff < -time.Microsecond || diff > time.Microsecond { + t.Errorf("GetJitter() = %v, want ~%v", m.GetJitter(), want) + } + + impl.Reset() + if got := m.GetJitter(); got != 0 { + t.Errorf("jitter after reset should be 0, got %v", got) + } +} diff --git a/internal/ui/shared/export.go b/internal/ui/shared/export.go new file mode 100644 index 0000000..b0f30d0 --- /dev/null +++ b/internal/ui/shared/export.go @@ -0,0 +1,148 @@ +package shared + +import ( + "encoding/csv" + "encoding/json" + "fmt" + "io" + "math" + "strconv" + "time" + + "github.com/jedib0t/go-pretty/v6/table" + + "github.com/servak/mping/internal/stats" +) + +// Output formats supported by WriteReport +const ( + FormatTable = "table" + FormatJSON = "json" + FormatCSV = "csv" +) + +// OutputFormats lists all supported output formats +var OutputFormats = []string{FormatTable, FormatJSON, FormatCSV} + +// TargetReport is the machine-readable representation of a target's statistics. +// RTT values are expressed in milliseconds so they can be consumed directly by +// tools such as jq or spreadsheet software. +type TargetReport struct { + Host string `json:"host"` + Sent int `json:"sent"` + Success int `json:"success"` + Fail int `json:"fail"` + LossPercent float64 `json:"loss_percent"` + LastRTTMs float64 `json:"last_rtt_ms"` + AvgRTTMs float64 `json:"avg_rtt_ms"` + MinRTTMs float64 `json:"min_rtt_ms"` + MaxRTTMs float64 `json:"max_rtt_ms"` + JitterMs float64 `json:"jitter_ms"` + LastSuccessAt *time.Time `json:"last_success_at,omitempty"` + LastFailAt *time.Time `json:"last_fail_at,omitempty"` + LastFailReason string `json:"last_fail_reason,omitempty"` +} + +// NewTargetReport converts metrics into a TargetReport +func NewTargetReport(m stats.Metrics) TargetReport { + return TargetReport{ + Host: m.GetName(), + Sent: m.GetTotal(), + Success: m.GetSuccessful(), + Fail: m.GetFailed(), + LossPercent: math.Round(m.GetLoss()*100) / 100, + LastRTTMs: durationToMs(m.GetLastRTT()), + AvgRTTMs: durationToMs(m.GetAverageRTT()), + MinRTTMs: durationToMs(m.GetMinimumRTT()), + MaxRTTMs: durationToMs(m.GetMaximumRTT()), + JitterMs: durationToMs(m.GetJitter()), + LastSuccessAt: timePtr(m.GetLastSuccTime()), + LastFailAt: timePtr(m.GetLastFailTime()), + LastFailReason: m.GetLastFailDetail(), + } +} + +// WriteReport renders metrics to w in the given format +func WriteReport(w io.Writer, format string, metrics []stats.Metrics, sortKey stats.Key, ascending bool) error { + switch format { + case FormatTable, "": + t := NewTableData(metrics, sortKey, ascending).ToGoPrettyTable() + t.SetStyle(table.StyleLight) + _, err := fmt.Fprintln(w, t.Render()) + return err + case FormatJSON: + return writeJSON(w, metrics) + case FormatCSV: + return writeCSV(w, metrics) + default: + return fmt.Errorf("unsupported output format %q (supported: %v)", format, OutputFormats) + } +} + +func writeJSON(w io.Writer, metrics []stats.Metrics) error { + reports := make([]TargetReport, 0, len(metrics)) + for _, m := range metrics { + reports = append(reports, NewTargetReport(m)) + } + enc := json.NewEncoder(w) + enc.SetIndent("", " ") + return enc.Encode(reports) +} + +func writeCSV(w io.Writer, metrics []stats.Metrics) error { + cw := csv.NewWriter(w) + header := []string{ + "host", "sent", "success", "fail", "loss_percent", + "last_rtt_ms", "avg_rtt_ms", "min_rtt_ms", "max_rtt_ms", "jitter_ms", + "last_success_at", "last_fail_at", "last_fail_reason", + } + if err := cw.Write(header); err != nil { + return err + } + for _, m := range metrics { + r := NewTargetReport(m) + record := []string{ + r.Host, + strconv.Itoa(r.Sent), + strconv.Itoa(r.Success), + strconv.Itoa(r.Fail), + formatFloat(r.LossPercent), + formatFloat(r.LastRTTMs), + formatFloat(r.AvgRTTMs), + formatFloat(r.MinRTTMs), + formatFloat(r.MaxRTTMs), + formatFloat(r.JitterMs), + formatRFC3339(r.LastSuccessAt), + formatRFC3339(r.LastFailAt), + r.LastFailReason, + } + if err := cw.Write(record); err != nil { + return err + } + } + cw.Flush() + return cw.Error() +} + +func durationToMs(d time.Duration) float64 { + // Round to microsecond precision to keep output readable + return float64(d.Microseconds()) / 1000 +} + +func timePtr(t time.Time) *time.Time { + if t.IsZero() { + return nil + } + return &t +} + +func formatFloat(f float64) string { + return strconv.FormatFloat(f, 'f', -1, 64) +} + +func formatRFC3339(t *time.Time) string { + if t == nil { + return "" + } + return t.Format(time.RFC3339Nano) +} diff --git a/internal/ui/shared/export_test.go b/internal/ui/shared/export_test.go new file mode 100644 index 0000000..f8237ae --- /dev/null +++ b/internal/ui/shared/export_test.go @@ -0,0 +1,95 @@ +package shared + +import ( + "bytes" + "encoding/csv" + "encoding/json" + "strings" + "testing" + "time" + + "github.com/servak/mping/internal/stats" +) + +func exportTestMetrics() []stats.Metrics { + succ := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) + return []stats.Metrics{ + stats.NewMetricsForTest("healthy.example.com", 10, 3, 3, 0, 0, + 30*time.Millisecond, 10*time.Millisecond, 5*time.Millisecond, 15*time.Millisecond, 12500*time.Microsecond, + succ, time.Time{}, ""), + stats.NewMetricsForTest("down.example.com", 10, 3, 0, 3, 100, + 0, 0, 0, 0, 0, + time.Time{}, succ, "timeout, no reply"), + } +} + +func TestWriteReportJSON(t *testing.T) { + var buf bytes.Buffer + if err := WriteReport(&buf, FormatJSON, exportTestMetrics(), stats.Host, true); err != nil { + t.Fatalf("WriteReport() error = %v", err) + } + + var got []TargetReport + if err := json.Unmarshal(buf.Bytes(), &got); err != nil { + t.Fatalf("output is not valid JSON: %v\n%s", err, buf.String()) + } + if len(got) != 2 { + t.Fatalf("expected 2 reports, got %d", len(got)) + } + h := got[0] + if h.Host != "healthy.example.com" || h.Sent != 3 || h.Success != 3 || h.AvgRTTMs != 10 || h.LastRTTMs != 12.5 { + t.Errorf("unexpected healthy report: %+v", h) + } + if h.LastSuccessAt == nil || h.LastFailAt != nil { + t.Errorf("timestamps: want last_success_at set and last_fail_at nil, got %+v", h) + } + d := got[1] + if d.LossPercent != 100 || d.LastFailReason != "timeout, no reply" || d.LastSuccessAt != nil { + t.Errorf("unexpected down report: %+v", d) + } + // Zero timestamps must be omitted, not rendered as 0001-01-01 + if strings.Contains(buf.String(), "0001-01-01") { + t.Errorf("zero time leaked into JSON output:\n%s", buf.String()) + } +} + +func TestWriteReportCSV(t *testing.T) { + var buf bytes.Buffer + if err := WriteReport(&buf, FormatCSV, exportTestMetrics(), stats.Host, true); err != nil { + t.Fatalf("WriteReport() error = %v", err) + } + + records, err := csv.NewReader(&buf).ReadAll() + if err != nil { + t.Fatalf("output is not valid CSV: %v", err) + } + if len(records) != 3 { + t.Fatalf("expected header + 2 rows, got %d", len(records)) + } + if records[0][0] != "host" || records[0][9] != "jitter_ms" { + t.Errorf("unexpected header: %v", records[0]) + } + if records[1][6] != "10" || records[1][10] != "2026-01-02T03:04:05Z" || records[1][11] != "" { + t.Errorf("unexpected healthy row: %v", records[1]) + } + // Fail reason containing a comma must round-trip intact + if records[2][12] != "timeout, no reply" { + t.Errorf("fail reason = %q", records[2][12]) + } +} + +func TestWriteReportTable(t *testing.T) { + var buf bytes.Buffer + if err := WriteReport(&buf, FormatTable, exportTestMetrics(), stats.Host, true); err != nil { + t.Fatalf("WriteReport() error = %v", err) + } + if !strings.Contains(buf.String(), "healthy.example.com") { + t.Errorf("table output missing host:\n%s", buf.String()) + } +} + +func TestWriteReportUnknownFormat(t *testing.T) { + if err := WriteReport(&bytes.Buffer{}, "xml", nil, stats.Host, true); err == nil { + t.Error("expected error for unsupported format") + } +} diff --git a/internal/ui/shared/formatters.go b/internal/ui/shared/formatters.go index 241b0d2..ec05cd6 100644 --- a/internal/ui/shared/formatters.go +++ b/internal/ui/shared/formatters.go @@ -57,6 +57,7 @@ func FormatHostDetail(metric stats.Metrics, theme *Theme) string { [%s]Average RTT:[%s] %s [%s]Minimum RTT:[%s] %s [%s]Maximum RTT:[%s] %s +[%s]Jitter (stddev):[%s] %s [%s]Last Success:[%s] %s [%s]Last Failure:[%s] %s [%s]Last Error:[%s] %s`, @@ -68,6 +69,7 @@ func FormatHostDetail(metric stats.Metrics, theme *Theme) string { theme.Accent, theme.Primary, DurationFormater(metric.GetAverageRTT()), theme.Accent, theme.Primary, DurationFormater(metric.GetMinimumRTT()), theme.Accent, theme.Primary, DurationFormater(metric.GetMaximumRTT()), + theme.Accent, theme.Primary, DurationFormater(metric.GetJitter()), theme.Accent, theme.Primary, TimeFormater(metric.GetLastSuccTime()), theme.Accent, theme.Primary, TimeFormater(metric.GetLastFailTime()), theme.Accent, theme.Primary, metric.GetLastFailDetail(), From 46ec77f6ac36f6a300045244f6f5218746473e26 Mon Sep 17 00:00:00 2001 From: Kakuya Ando Date: Wed, 23 Sep 2026 16:27:40 +0900 Subject: [PATCH 2/2] refactor(batch): fix count drift and consolidate report helpers - Measure batch wait deadlines from a fixed start so sleep overshoot does not accumulate and exceed --count on long runs - Add shared.ValidateOutputFormat and use it from batch and WriteReport - Reuse WriteReport for the interactive mode's final table - Keep CSV header and row together via csvHeader/csvRecord, with a test - Add MetricsManager.SetBeepEnabled instead of check-and-toggle - Drop redundant cancel() and unreachable "" format case --- internal/command/batch.go | 17 ++++---- internal/command/mping.go | 7 +--- internal/stats/interface.go | 1 + internal/stats/manager.go | 12 ++++++ internal/ui/shared/export.go | 65 +++++++++++++++++++------------ internal/ui/shared/export_test.go | 6 +++ 6 files changed, 68 insertions(+), 40 deletions(-) diff --git a/internal/command/batch.go b/internal/command/batch.go index ea32c8b..94aa3ef 100644 --- a/internal/command/batch.go +++ b/internal/command/batch.go @@ -6,7 +6,6 @@ import ( "fmt" "io" "os" - "slices" "strings" "time" @@ -69,8 +68,8 @@ mping batch --max-loss 20 -f hosts.txt || echo "some targets are unhealthy"`, if err != nil { return err } - if !slices.Contains(shared.OutputFormats, output) { - return fmt.Errorf("unsupported output format %q (supported: %s)", output, strings.Join(shared.OutputFormats, ", ")) + if err := shared.ValidateOutputFormat(output); err != nil { + return err } maxLoss := -1.0 // disabled unless explicitly set if flags.Changed("max-loss") { @@ -102,9 +101,7 @@ mping batch --max-loss 20 -f hosts.txt || echo "some targets are unhealthy"`, metricsManager := stats.NewMetricsManager() // The terminal bell would corrupt machine-readable output and is // pointless in non-interactive runs. - if metricsManager.IsBeepEnabled() { - metricsManager.ToggleBeep() - } + metricsManager.SetBeepEnabled(false) // Add targets err = probeManager.AddTargets(hosts...) @@ -132,15 +129,17 @@ mping batch --max-loss 20 -f hosts.txt || echo "some targets are unhealthy"`, // interval, so round N goes out at (N-1)*interval. Stopping half an // interval after the last round yields exactly `count` probes without // racing the next tick. Stop waits for in-flight probes to finish. + // Deadlines are measured from a fixed start so sleep overshoot does + // not accumulate across rounds (the prober's ticker does not drift). + start := time.Now() fmt.Fprint(progress, ".") for i := 1; i < counter; i++ { - time.Sleep(_interval) + time.Sleep(time.Until(start.Add(time.Duration(i) * _interval))) fmt.Fprint(progress, ".") } - time.Sleep(_interval / 2) + time.Sleep(time.Until(start.Add(time.Duration(counter-1)*_interval + _interval/2))) // Stop probing - cancel() probeManager.Stop() <-done // wait for in-flight metric updates to settle fmt.Fprint(progress, "\r\033[K") diff --git a/internal/command/mping.go b/internal/command/mping.go index 538f7ae..3fdbb95 100644 --- a/internal/command/mping.go +++ b/internal/command/mping.go @@ -6,7 +6,6 @@ import ( "fmt" "time" - "github.com/jedib0t/go-pretty/v6/table" "github.com/spf13/cobra" "github.com/servak/mping/internal/config" @@ -109,11 +108,7 @@ mping dns://8.8.8.8/google.com`, // Final results metrics := metricsManager.SortBy(stats.Fail, false) - tableData := shared.NewTableData(metrics, stats.Fail, false) - t := tableData.ToGoPrettyTable() - t.SetStyle(table.StyleLight) - cmd.Println(t.Render()) - return nil + return shared.WriteReport(cmd.OutOrStderr(), shared.FormatTable, metrics, stats.Fail, false) }, } diff --git a/internal/stats/interface.go b/internal/stats/interface.go index b8bad46..269dbeb 100644 --- a/internal/stats/interface.go +++ b/internal/stats/interface.go @@ -37,6 +37,7 @@ type MetricsProvider interface { type MetricsSystemManager interface { ResetAllMetrics() ToggleBeep() + SetBeepEnabled(enabled bool) IsBeepEnabled() bool } diff --git a/internal/stats/manager.go b/internal/stats/manager.go index 7905107..8061b5c 100644 --- a/internal/stats/manager.go +++ b/internal/stats/manager.go @@ -231,6 +231,18 @@ func (mm *metricsManager) ToggleBeep() { } } +// SetBeepEnabled turns beep sound on or off +func (mm *metricsManager) SetBeepEnabled(enabled bool) { + if mm.beeper == nil { + return + } + if enabled { + mm.beeper.Enable() + } else { + mm.beeper.Disable() + } +} + // IsBeepEnabled returns current beep sound state func (mm *metricsManager) IsBeepEnabled() bool { if mm.beeper != nil { diff --git a/internal/ui/shared/export.go b/internal/ui/shared/export.go index b0f30d0..b735dc0 100644 --- a/internal/ui/shared/export.go +++ b/internal/ui/shared/export.go @@ -6,7 +6,9 @@ import ( "fmt" "io" "math" + "slices" "strconv" + "strings" "time" "github.com/jedib0t/go-pretty/v6/table" @@ -24,6 +26,14 @@ const ( // OutputFormats lists all supported output formats var OutputFormats = []string{FormatTable, FormatJSON, FormatCSV} +// ValidateOutputFormat reports an error if format is not one of OutputFormats +func ValidateOutputFormat(format string) error { + if slices.Contains(OutputFormats, format) { + return nil + } + return fmt.Errorf("unsupported output format %q (supported: %s)", format, strings.Join(OutputFormats, ", ")) +} + // TargetReport is the machine-readable representation of a target's statistics. // RTT values are expressed in milliseconds so they can be consumed directly by // tools such as jq or spreadsheet software. @@ -43,6 +53,32 @@ type TargetReport struct { LastFailReason string `json:"last_fail_reason,omitempty"` } +// csvHeader lists CSV columns; it must stay in sync with TargetReport.csvRecord +var csvHeader = []string{ + "host", "sent", "success", "fail", "loss_percent", + "last_rtt_ms", "avg_rtt_ms", "min_rtt_ms", "max_rtt_ms", "jitter_ms", + "last_success_at", "last_fail_at", "last_fail_reason", +} + +// csvRecord renders the report as a CSV row matching csvHeader +func (r TargetReport) csvRecord() []string { + return []string{ + r.Host, + strconv.Itoa(r.Sent), + strconv.Itoa(r.Success), + strconv.Itoa(r.Fail), + formatFloat(r.LossPercent), + formatFloat(r.LastRTTMs), + formatFloat(r.AvgRTTMs), + formatFloat(r.MinRTTMs), + formatFloat(r.MaxRTTMs), + formatFloat(r.JitterMs), + formatRFC3339(r.LastSuccessAt), + formatRFC3339(r.LastFailAt), + r.LastFailReason, + } +} + // NewTargetReport converts metrics into a TargetReport func NewTargetReport(m stats.Metrics) TargetReport { return TargetReport{ @@ -65,7 +101,7 @@ func NewTargetReport(m stats.Metrics) TargetReport { // WriteReport renders metrics to w in the given format func WriteReport(w io.Writer, format string, metrics []stats.Metrics, sortKey stats.Key, ascending bool) error { switch format { - case FormatTable, "": + case FormatTable: t := NewTableData(metrics, sortKey, ascending).ToGoPrettyTable() t.SetStyle(table.StyleLight) _, err := fmt.Fprintln(w, t.Render()) @@ -75,7 +111,7 @@ func WriteReport(w io.Writer, format string, metrics []stats.Metrics, sortKey st case FormatCSV: return writeCSV(w, metrics) default: - return fmt.Errorf("unsupported output format %q (supported: %v)", format, OutputFormats) + return ValidateOutputFormat(format) } } @@ -91,32 +127,11 @@ func writeJSON(w io.Writer, metrics []stats.Metrics) error { func writeCSV(w io.Writer, metrics []stats.Metrics) error { cw := csv.NewWriter(w) - header := []string{ - "host", "sent", "success", "fail", "loss_percent", - "last_rtt_ms", "avg_rtt_ms", "min_rtt_ms", "max_rtt_ms", "jitter_ms", - "last_success_at", "last_fail_at", "last_fail_reason", - } - if err := cw.Write(header); err != nil { + if err := cw.Write(csvHeader); err != nil { return err } for _, m := range metrics { - r := NewTargetReport(m) - record := []string{ - r.Host, - strconv.Itoa(r.Sent), - strconv.Itoa(r.Success), - strconv.Itoa(r.Fail), - formatFloat(r.LossPercent), - formatFloat(r.LastRTTMs), - formatFloat(r.AvgRTTMs), - formatFloat(r.MinRTTMs), - formatFloat(r.MaxRTTMs), - formatFloat(r.JitterMs), - formatRFC3339(r.LastSuccessAt), - formatRFC3339(r.LastFailAt), - r.LastFailReason, - } - if err := cw.Write(record); err != nil { + if err := cw.Write(NewTargetReport(m).csvRecord()); err != nil { return err } } diff --git a/internal/ui/shared/export_test.go b/internal/ui/shared/export_test.go index f8237ae..da6b62a 100644 --- a/internal/ui/shared/export_test.go +++ b/internal/ui/shared/export_test.go @@ -93,3 +93,9 @@ func TestWriteReportUnknownFormat(t *testing.T) { t.Error("expected error for unsupported format") } } + +func TestCSVHeaderMatchesRecord(t *testing.T) { + if got, want := len(TargetReport{}.csvRecord()), len(csvHeader); got != want { + t.Errorf("csvRecord has %d columns, header has %d", got, want) + } +}