From 055fccb066abf18e87e65177ca066ed8f7758bb4 Mon Sep 17 00:00:00 2001 From: zuchka Date: Fri, 8 May 2026 13:24:40 -0700 Subject: [PATCH 01/13] refactor(cli): introduce Dispatcher interface for alert routing Pulls the per-alert dispatch logic out of run.go's free functions and into a NotifierDispatcher behind a small Dispatcher interface. Behavior is identical; existing tests pass unchanged after a one-line construction update. Sets up upcoming `ding test-rule` and `ding run --dry-run` to swap the real dispatcher for a logging one without further engine or ingest changes. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/cli/dispatcher.go | 42 +++++++++++++++++++++++++++++++ internal/cli/run.go | 51 ++++++++++---------------------------- internal/cli/run_test.go | 6 +++-- 3 files changed, 59 insertions(+), 40 deletions(-) create mode 100644 internal/cli/dispatcher.go diff --git a/internal/cli/dispatcher.go b/internal/cli/dispatcher.go new file mode 100644 index 0000000..3b511ea --- /dev/null +++ b/internal/cli/dispatcher.go @@ -0,0 +1,42 @@ +package cli + +import ( + "log" + + "github.com/ding-labs/ding/internal/evaluator" + "github.com/ding-labs/ding/internal/notifier" +) + +// Dispatcher routes alerts to their final destination. Implementations live +// here (NotifierDispatcher — production sends) and in internal/dryrun +// (LoggingDispatcher — preview-only, no sends). +type Dispatcher interface { + Dispatch(alerts []evaluator.Alert) +} + +// NotifierDispatcher is the production Dispatcher: writes each alert to the +// alert log (if configured) then calls Send on each named notifier. +type NotifierDispatcher struct { + Notifiers map[string]notifier.Notifier + AlertLogger *notifier.AlertLogger +} + +func (d *NotifierDispatcher) Dispatch(alerts []evaluator.Alert) { + for _, alert := range alerts { + if d.AlertLogger != nil { + if err := d.AlertLogger.Log(alert); err != nil { + log.Printf("ding: alert log write error: %v", err) + } + } + for _, name := range alert.Notifiers { + n, ok := d.Notifiers[name] + if !ok { + log.Printf("ding: unknown notifier %q for rule %q", name, alert.Rule) + continue + } + if err := n.Send(alert); err != nil { + log.Printf("ding: notifier %q error: %v", name, err) + } + } + } +} diff --git a/internal/cli/run.go b/internal/cli/run.go index 6aa0372..15a8193 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -74,6 +74,11 @@ func runRun(configPath, runIDOverride string, args []string) error { if err != nil { return fmt.Errorf("loading config: %w", err) } + + dispatcher := &NotifierDispatcher{ + Notifiers: notifiers, + AlertLogger: alertLogger, + } // Drain handler — runs from both the deferred path (covers early returns // from config errors, command-start failures, etc.) and the explicit path // before os.Exit (covers the non-zero-exit case where defers don't run). @@ -134,11 +139,11 @@ func runRun(configPath, runIDOverride string, args []string) error { wg.Add(2) go func() { defer wg.Done() - ingestStream(stdoutPipe, os.Stdout, eng, notifiers, alertLogger, cfg, jqCode, rc) + ingestStream(stdoutPipe, os.Stdout, eng, dispatcher, cfg, jqCode, rc) }() go func() { defer wg.Done() - ingestStream(stderrPipe, os.Stderr, eng, notifiers, alertLogger, cfg, jqCode, rc) + ingestStream(stderrPipe, os.Stderr, eng, dispatcher, cfg, jqCode, rc) }() wg.Wait() @@ -162,11 +167,11 @@ func runRun(configPath, runIDOverride string, args []string) error { // Synthetic run.exit event flows through the engine like any other — // during-run rules matching metric: run.exit fire here. summary := rc.SummaryEvent(exitCode) - dispatchEvent(summary, eng, notifiers, alertLogger) + dispatchEvent(summary, eng, dispatcher) // End-of-run rules accumulate state during the run; fire them now. endAlerts := eng.ProcessEndOfRun(time.Now()) - dispatchAlerts(endAlerts, notifiers, alertLogger) + dispatcher.Dispatch(endAlerts) log.Printf("ding: run end — run_id=%s exit_code=%d duration=%.1fs", rc.RunID, exitCode, time.Since(rc.StartedAt).Seconds()) @@ -209,8 +214,7 @@ func ingestStream( r io.Reader, mirror io.Writer, eng *evaluator.Engine, - notifiers map[string]notifier.Notifier, - alertLogger *notifier.AlertLogger, + dispatcher Dispatcher, cfg *config.Config, jqCode *gojq.Code, rc *runctx.Context, @@ -246,7 +250,7 @@ func ingestStream( } for _, ev := range events { ev.Labels = rc.Apply(ev.Labels) - dispatchEvent(ev, eng, notifiers, alertLogger) + dispatchEvent(ev, eng, dispatcher) } } // Scanner errors on closed pipes are expected at EOF; only log surprising ones. @@ -255,36 +259,7 @@ func ingestStream( } } -func dispatchEvent( - ev ingester.Event, - eng *evaluator.Engine, - notifiers map[string]notifier.Notifier, - alertLogger *notifier.AlertLogger, -) { +func dispatchEvent(ev ingester.Event, eng *evaluator.Engine, dispatcher Dispatcher) { alerts := eng.Process(ev, time.Now()) - dispatchAlerts(alerts, notifiers, alertLogger) -} - -func dispatchAlerts( - alerts []evaluator.Alert, - notifiers map[string]notifier.Notifier, - alertLogger *notifier.AlertLogger, -) { - for _, alert := range alerts { - if alertLogger != nil { - if err := alertLogger.Log(alert); err != nil { - log.Printf("ding: alert log write error: %v", err) - } - } - for _, name := range alert.Notifiers { - n, ok := notifiers[name] - if !ok { - log.Printf("ding: unknown notifier %q for rule %q", name, alert.Rule) - continue - } - if err := n.Send(alert); err != nil { - log.Printf("ding: notifier %q error: %v", name, err) - } - } - } + dispatcher.Dispatch(alerts) } diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go index 5d530f9..bc25184 100644 --- a/internal/cli/run_test.go +++ b/internal/cli/run_test.go @@ -71,7 +71,8 @@ not a json line — mirrored only `) var mirror bytes.Buffer - ingestStream(input, &mirror, eng, notifierMap, nil, cfg, nil, rc) + dispatcher := &NotifierDispatcher{Notifiers: notifierMap, AlertLogger: nil} + ingestStream(input, &mirror, eng, dispatcher, cfg, nil, rc) alerts := cap.snapshot() if len(alerts) != 2 { @@ -123,7 +124,8 @@ FAILED test_b some random shell output here `) var mirror bytes.Buffer - ingestStream(input, &mirror, eng, notifierMap, nil, cfg, nil, rc) + dispatcher := &NotifierDispatcher{Notifiers: notifierMap, AlertLogger: nil} + ingestStream(input, &mirror, eng, dispatcher, cfg, nil, rc) if got := cap.snapshot(); len(got) != 0 { t.Errorf("expected no alerts on non-event input, got %d: %#v", len(got), got) From c864be489994614659d878121b514e12a1991234 Mon Sep 17 00:00:00 2001 From: zuchka Date: Fri, 8 May 2026 13:29:42 -0700 Subject: [PATCH 02/13] feat(dryrun): TextFormatter for alert preview Renders evaluator.Alert as human-readable multi-line text with optional ANSI color, used by upcoming `ding test-rule` and `ding run --dry-run` TTY output paths. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/dryrun/formatter.go | 52 ++++++++++++++++++++++ internal/dryrun/formatter_test.go | 71 +++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+) create mode 100644 internal/dryrun/formatter.go create mode 100644 internal/dryrun/formatter_test.go diff --git a/internal/dryrun/formatter.go b/internal/dryrun/formatter.go new file mode 100644 index 0000000..fa8a5ed --- /dev/null +++ b/internal/dryrun/formatter.go @@ -0,0 +1,52 @@ +// Package dryrun provides alert formatters and a Dispatcher implementation +// for dry-run modes (ding test-rule and ding run --dry-run). LoggingDispatcher +// satisfies the cli.Dispatcher interface without sending to real notifiers. +package dryrun + +import ( + "fmt" + "strings" + + "github.com/ding-labs/ding/internal/evaluator" +) + +// Formatter renders one Alert as bytes ready for an io.Writer. +type Formatter interface { + Format(alert evaluator.Alert) []byte +} + +// ANSI color codes. Centralized so the table is easy to audit. +const ( + ansiReset = "\x1b[0m" + ansiGreen = "\x1b[32m" + ansiCyan = "\x1b[36m" + ansiDim = "\x1b[2m" +) + +// TextFormatter renders alerts as multi-line human-readable text. +// Color toggles ANSI escape sequences (off when not on a TTY or with --no-color). +type TextFormatter struct { + Color bool +} + +func (f *TextFormatter) Format(alert evaluator.Alert) []byte { + var b strings.Builder + rule := alert.Rule + notifiers := strings.Join(alert.Notifiers, ", ") + if notifiers == "" { + notifiers = "(none)" + } + + if f.Color { + fmt.Fprintf(&b, "%s✓ %s%s — %swould fire%s (alerts: %s)\n", + ansiGreen, rule, ansiReset, ansiCyan, ansiReset, notifiers) + fmt.Fprintf(&b, "%s metric: %s · value: %.2f%s\n", + ansiDim, alert.Metric, alert.Value, ansiReset) + fmt.Fprintf(&b, " message: %s\n", alert.Message) + } else { + fmt.Fprintf(&b, "✓ %s — would fire (alerts: %s)\n", rule, notifiers) + fmt.Fprintf(&b, " metric: %s · value: %.2f\n", alert.Metric, alert.Value) + fmt.Fprintf(&b, " message: %s\n", alert.Message) + } + return []byte(b.String()) +} diff --git a/internal/dryrun/formatter_test.go b/internal/dryrun/formatter_test.go new file mode 100644 index 0000000..6415793 --- /dev/null +++ b/internal/dryrun/formatter_test.go @@ -0,0 +1,71 @@ +package dryrun + +import ( + "strings" + "testing" + "time" + + "github.com/ding-labs/ding/internal/evaluator" +) + +func TestTextFormatter_PerEventAlert_NoColor(t *testing.T) { + alert := evaluator.Alert{ + Rule: "loss_spike", + Message: "Loss spiked to 1.20", + Metric: "loss", + Value: 1.2, + Notifiers: []string{"slack"}, + FiredAt: time.Date(2026, 5, 8, 10, 2, 0, 0, time.UTC), + } + + f := &TextFormatter{Color: false} + got := string(f.Format(alert)) + + wantSubstrings := []string{ + "loss_spike", + "would fire", + "slack", + "metric: loss", + "value: 1.20", + "Loss spiked to 1.20", + } + for _, want := range wantSubstrings { + if !strings.Contains(got, want) { + t.Errorf("TextFormatter output missing %q\noutput: %s", want, got) + } + } + // No-color: no ANSI escape sequences + if strings.Contains(got, "\x1b[") { + t.Errorf("TextFormatter Color=false leaked ANSI escapes:\n%s", got) + } +} + +func TestTextFormatter_Color_EmitsANSIEscapes(t *testing.T) { + alert := evaluator.Alert{ + Rule: "x", + Message: "m", + Metric: "loss", + Value: 1, + Notifiers: []string{"slack"}, + } + f := &TextFormatter{Color: true} + got := string(f.Format(alert)) + if !strings.Contains(got, "\x1b[") { + t.Errorf("Color=true did not emit ANSI escapes:\n%q", got) + } +} + +func TestTextFormatter_NoNotifiers(t *testing.T) { + alert := evaluator.Alert{ + Rule: "x", + Message: "m", + Metric: "loss", + Value: 1, + // Notifiers empty + } + f := &TextFormatter{Color: false} + got := string(f.Format(alert)) + if !strings.Contains(got, "(none)") { + t.Errorf("expected '(none)' for empty Notifiers, got:\n%s", got) + } +} From 7aaa6f95b5cd9ad0f3d9c31bfd9da801fe36087c Mon Sep 17 00:00:00 2001 From: zuchka Date: Fri, 8 May 2026 13:33:54 -0700 Subject: [PATCH 03/13] refactor(dryrun): consistent dim styling on text formatter message line Two follow-ups from code review on c864be4: - Apply ansiDim to the "message:" label in the color path so the third line is consistent with the dim treatment on the metric/value line. Message body stays unstyled (it's user-rendered template content, not formatter chrome). - Color test now asserts ansiReset is present in the output, catching any future change that opens color codes without closing them (which would leave the terminal styled until the next prompt). Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/dryrun/formatter.go | 2 +- internal/dryrun/formatter_test.go | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/dryrun/formatter.go b/internal/dryrun/formatter.go index fa8a5ed..6819310 100644 --- a/internal/dryrun/formatter.go +++ b/internal/dryrun/formatter.go @@ -42,7 +42,7 @@ func (f *TextFormatter) Format(alert evaluator.Alert) []byte { ansiGreen, rule, ansiReset, ansiCyan, ansiReset, notifiers) fmt.Fprintf(&b, "%s metric: %s · value: %.2f%s\n", ansiDim, alert.Metric, alert.Value, ansiReset) - fmt.Fprintf(&b, " message: %s\n", alert.Message) + fmt.Fprintf(&b, "%s message:%s %s\n", ansiDim, ansiReset, alert.Message) } else { fmt.Fprintf(&b, "✓ %s — would fire (alerts: %s)\n", rule, notifiers) fmt.Fprintf(&b, " metric: %s · value: %.2f\n", alert.Metric, alert.Value) diff --git a/internal/dryrun/formatter_test.go b/internal/dryrun/formatter_test.go index 6415793..045afc4 100644 --- a/internal/dryrun/formatter_test.go +++ b/internal/dryrun/formatter_test.go @@ -53,6 +53,9 @@ func TestTextFormatter_Color_EmitsANSIEscapes(t *testing.T) { if !strings.Contains(got, "\x1b[") { t.Errorf("Color=true did not emit ANSI escapes:\n%q", got) } + if !strings.Contains(got, ansiReset) { + t.Errorf("Color=true output missing reset code — terminal would be left dirty:\n%q", got) + } } func TestTextFormatter_NoNotifiers(t *testing.T) { From de751aba91b7ba4d454dae1c8195d056daf44628 Mon Sep 17 00:00:00 2001 From: zuchka Date: Fri, 8 May 2026 13:35:34 -0700 Subject: [PATCH 04/13] feat(dryrun): JSONFormatter for alert preview JSONL output (one JSON object per line, newline-terminated). Stable schema with aggregate fields always present (zero when non-windowed) so jq pipelines don't have to nil-check. Used for --format json and the piped-stdout default. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/dryrun/formatter.go | 46 +++++++++++++++++++++++++++++++ internal/dryrun/formatter_test.go | 40 +++++++++++++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/internal/dryrun/formatter.go b/internal/dryrun/formatter.go index 6819310..4047582 100644 --- a/internal/dryrun/formatter.go +++ b/internal/dryrun/formatter.go @@ -4,8 +4,10 @@ package dryrun import ( + "encoding/json" "fmt" "strings" + "time" "github.com/ding-labs/ding/internal/evaluator" ) @@ -50,3 +52,47 @@ func (f *TextFormatter) Format(alert evaluator.Alert) []byte { } return []byte(b.String()) } + +// JSONFormatter renders alerts as one JSON object per line (JSONL). +// Stable schema; safe to pipe to jq. +type JSONFormatter struct{} + +type jsonAlertEnvelope struct { + Rule string `json:"rule"` + Metric string `json:"metric"` + Value float64 `json:"value"` + Message string `json:"message"` + Alerts []string `json:"alerts"` + Labels map[string]string `json:"labels,omitempty"` + Floats map[string]float64 `json:"floats,omitempty"` + FiredAt string `json:"fired_at"` + // Aggregates (always present; zero when not windowed — easier for jq) + Avg float64 `json:"avg"` + Max float64 `json:"max"` + Min float64 `json:"min"` + Count float64 `json:"count"` + Sum float64 `json:"sum"` +} + +func (f *JSONFormatter) Format(alert evaluator.Alert) []byte { + env := jsonAlertEnvelope{ + Rule: alert.Rule, + Metric: alert.Metric, + Value: alert.Value, + Message: alert.Message, + Alerts: alert.Notifiers, + Labels: alert.Labels, + Floats: alert.Floats, + FiredAt: alert.FiredAt.UTC().Format(time.RFC3339Nano), + Avg: alert.Avg, + Max: alert.Max, + Min: alert.Min, + Count: alert.Count, + Sum: alert.Sum, + } + if env.Alerts == nil { + env.Alerts = []string{} + } + b, _ := json.Marshal(env) + return append(b, '\n') +} diff --git a/internal/dryrun/formatter_test.go b/internal/dryrun/formatter_test.go index 045afc4..50d4f10 100644 --- a/internal/dryrun/formatter_test.go +++ b/internal/dryrun/formatter_test.go @@ -1,6 +1,7 @@ package dryrun import ( + "encoding/json" "strings" "testing" "time" @@ -72,3 +73,42 @@ func TestTextFormatter_NoNotifiers(t *testing.T) { t.Errorf("expected '(none)' for empty Notifiers, got:\n%s", got) } } + +func TestJSONFormatter_RoundTrips(t *testing.T) { + alert := evaluator.Alert{ + Rule: "loss_spike", + Message: "Loss spiked", + Metric: "loss", + Value: 1.2, + Labels: map[string]string{"run_id": "abc123"}, + Floats: map[string]float64{"step": 100}, + Notifiers: []string{"slack", "pagerduty"}, + FiredAt: time.Date(2026, 5, 8, 10, 2, 0, 0, time.UTC), + Avg: 1.1, + } + + f := &JSONFormatter{} + out := f.Format(alert) + + if !strings.HasSuffix(string(out), "\n") { + t.Errorf("JSONFormatter output must end with newline (JSONL): %q", string(out)) + } + + var got map[string]any + if err := json.Unmarshal(out, &got); err != nil { + t.Fatalf("JSONFormatter output is not valid JSON: %v\noutput: %s", err, out) + } + + wantKeys := []string{"rule", "metric", "value", "message", "alerts", "fired_at", "labels", "floats", "avg"} + for _, k := range wantKeys { + if _, ok := got[k]; !ok { + t.Errorf("JSONFormatter output missing key %q", k) + } + } + if got["rule"] != "loss_spike" { + t.Errorf("rule mismatch: got %v", got["rule"]) + } + if alerts, _ := got["alerts"].([]any); len(alerts) != 2 { + t.Errorf("alerts should be 2 entries: got %v", alerts) + } +} From f41695a9cfff2f45cd19c724d40e66cbe1233cc5 Mon Sep 17 00:00:00 2001 From: zuchka Date: Fri, 8 May 2026 13:39:30 -0700 Subject: [PATCH 05/13] test(dryrun): tighten JSONFormatter test coverage Three follow-ups from code review on de751ab: - TestJSONFormatter_RoundTrips wantKeys now includes max/min/count/sum alongside avg, locking the "aggregates always present" invariant in the test (not just the comment). - TestJSONFormatter_NilNotifiers verifies the nil-coercion path: when Alert.Notifiers is nil, the JSON output renders alerts as [] not null. The round-trip test only exercised the populated case. - Code comment above json.Marshal call documents why the discarded error is safe (envelope contains only stdlib-marshalable types). Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/dryrun/formatter.go | 2 ++ internal/dryrun/formatter_test.go | 30 +++++++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/internal/dryrun/formatter.go b/internal/dryrun/formatter.go index 4047582..973c7fc 100644 --- a/internal/dryrun/formatter.go +++ b/internal/dryrun/formatter.go @@ -93,6 +93,8 @@ func (f *JSONFormatter) Format(alert evaluator.Alert) []byte { if env.Alerts == nil { env.Alerts = []string{} } + // jsonAlertEnvelope contains only stdlib-marshalable types (string, float64, + // []string, map[string]string, map[string]float64). Marshal cannot fail here. b, _ := json.Marshal(env) return append(b, '\n') } diff --git a/internal/dryrun/formatter_test.go b/internal/dryrun/formatter_test.go index 50d4f10..0b4a67c 100644 --- a/internal/dryrun/formatter_test.go +++ b/internal/dryrun/formatter_test.go @@ -99,7 +99,10 @@ func TestJSONFormatter_RoundTrips(t *testing.T) { t.Fatalf("JSONFormatter output is not valid JSON: %v\noutput: %s", err, out) } - wantKeys := []string{"rule", "metric", "value", "message", "alerts", "fired_at", "labels", "floats", "avg"} + wantKeys := []string{ + "rule", "metric", "value", "message", "alerts", "fired_at", + "labels", "floats", "avg", "max", "min", "count", "sum", + } for _, k := range wantKeys { if _, ok := got[k]; !ok { t.Errorf("JSONFormatter output missing key %q", k) @@ -112,3 +115,28 @@ func TestJSONFormatter_RoundTrips(t *testing.T) { t.Errorf("alerts should be 2 entries: got %v", alerts) } } + +func TestJSONFormatter_NilNotifiers(t *testing.T) { + alert := evaluator.Alert{ + Rule: "x", + Metric: "loss", + Value: 1, + Message: "m", + Notifiers: nil, + } + out := (&JSONFormatter{}).Format(alert) + var got map[string]any + if err := json.Unmarshal(out, &got); err != nil { + t.Fatalf("not valid JSON: %v\noutput: %s", err, out) + } + alerts, ok := got["alerts"] + if !ok { + t.Fatalf("alerts key missing entirely") + } + if alerts == nil { + t.Errorf("alerts should serialize as [] not null when Notifiers is nil") + } + if arr, _ := alerts.([]any); arr == nil || len(arr) != 0 { + t.Errorf("expected empty array, got: %v", alerts) + } +} From fe210d8d65d79c6642f121cf7e2dfcda4edbdd49 Mon Sep 17 00:00:00 2001 From: zuchka Date: Fri, 8 May 2026 13:40:43 -0700 Subject: [PATCH 06/13] feat(dryrun): LoggingDispatcher writes formatted alerts, no notifier sends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Satisfies cli.Dispatcher implicitly via Go duck-typing — the dryrun package does not import the cli package. Both upcoming surfaces (test-rule, run --dry-run) construct one of these instead of a NotifierDispatcher. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/dryrun/dispatcher.go | 24 +++++++++++++++++++ internal/dryrun/dispatcher_test.go | 38 ++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) create mode 100644 internal/dryrun/dispatcher.go create mode 100644 internal/dryrun/dispatcher_test.go diff --git a/internal/dryrun/dispatcher.go b/internal/dryrun/dispatcher.go new file mode 100644 index 0000000..784cad8 --- /dev/null +++ b/internal/dryrun/dispatcher.go @@ -0,0 +1,24 @@ +package dryrun + +import ( + "io" + + "github.com/ding-labs/ding/internal/evaluator" +) + +// LoggingDispatcher formats each alert via Formatter and writes to Writer. +// Never calls notifier.Send. Satisfies cli.Dispatcher implicitly (Go duck-typing). +type LoggingDispatcher struct { + Formatter Formatter + Writer io.Writer +} + +func NewLoggingDispatcher(f Formatter, w io.Writer) *LoggingDispatcher { + return &LoggingDispatcher{Formatter: f, Writer: w} +} + +func (d *LoggingDispatcher) Dispatch(alerts []evaluator.Alert) { + for _, alert := range alerts { + _, _ = d.Writer.Write(d.Formatter.Format(alert)) + } +} diff --git a/internal/dryrun/dispatcher_test.go b/internal/dryrun/dispatcher_test.go new file mode 100644 index 0000000..b650e80 --- /dev/null +++ b/internal/dryrun/dispatcher_test.go @@ -0,0 +1,38 @@ +package dryrun + +import ( + "bytes" + "strings" + "testing" + + "github.com/ding-labs/ding/internal/evaluator" +) + +func TestLoggingDispatcher_WritesFormattedAlerts(t *testing.T) { + var buf bytes.Buffer + d := NewLoggingDispatcher(&TextFormatter{Color: false}, &buf) + + alerts := []evaluator.Alert{ + {Rule: "rule_a", Metric: "m", Value: 1, Message: "msg-a", Notifiers: []string{"slack"}}, + {Rule: "rule_b", Metric: "m", Value: 2, Message: "msg-b", Notifiers: []string{"webhook"}}, + } + d.Dispatch(alerts) + + out := buf.String() + if !strings.Contains(out, "rule_a") || !strings.Contains(out, "rule_b") { + t.Errorf("expected both rule names in output:\n%s", out) + } + if !strings.Contains(out, "msg-a") || !strings.Contains(out, "msg-b") { + t.Errorf("expected both messages in output:\n%s", out) + } +} + +func TestLoggingDispatcher_EmptyAlerts_NoOutput(t *testing.T) { + var buf bytes.Buffer + d := NewLoggingDispatcher(&TextFormatter{Color: false}, &buf) + d.Dispatch(nil) + d.Dispatch([]evaluator.Alert{}) + if buf.Len() != 0 { + t.Errorf("expected empty output for empty alerts, got: %q", buf.String()) + } +} From 3b80c1959db3e95bb49fcf2501b9b8c9fb83fa61 Mon Sep 17 00:00:00 2001 From: zuchka Date: Fri, 8 May 2026 13:46:35 -0700 Subject: [PATCH 07/13] =?UTF-8?q?feat(cli):=20ding=20test-rule=20=E2=80=94?= =?UTF-8?q?=20replay=20JSONL=20events=20through=20the=20engine?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New subcommand that loads a ding.yaml, reads JSONL events from stdin or a positional file, and routes engine alerts through a LoggingDispatcher instead of real notifiers. Synthetic timestamps for events that omit the timestamp field; end-of-run rules fire after the last event. Format auto-detects: text on TTY, JSON when stdout is piped. --format text|json overrides; --no-color disables ANSI. Extends timestamp parsing to accept RFC3339 strings (via resolveTimestamp) in addition to the Unix epoch floats that ParseJSONLine already handles. Fixture YAML uses correct map-keyed notifiers and alert target format. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/cli/root.go | 1 + internal/cli/test_rule.go | 182 +++++++++++++++++++++++++++++++++ internal/cli/test_rule_test.go | 107 +++++++++++++++++++ 3 files changed, 290 insertions(+) create mode 100644 internal/cli/test_rule.go create mode 100644 internal/cli/test_rule_test.go diff --git a/internal/cli/root.go b/internal/cli/root.go index a0e090c..3e11dd3 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -30,6 +30,7 @@ Two modes: newValidateCmd(), newVersionCmd(version), newInstallCmd(), + newTestRuleCmd(), ) return root } diff --git a/internal/cli/test_rule.go b/internal/cli/test_rule.go new file mode 100644 index 0000000..a604434 --- /dev/null +++ b/internal/cli/test_rule.go @@ -0,0 +1,182 @@ +package cli + +import ( + "bufio" + "encoding/json" + "fmt" + "io" + "os" + "time" + + "github.com/spf13/cobra" + + "github.com/ding-labs/ding/internal/dryrun" + "github.com/ding-labs/ding/internal/ingester" + "github.com/ding-labs/ding/internal/runctx" + "github.com/ding-labs/ding/internal/server" +) + +func newTestRuleCmd() *cobra.Command { + var configPath string + var format string + var noColor bool + + cmd := &cobra.Command{ + Use: "test-rule [FILE]", + Short: "Replay JSONL events through the rule engine without sending notifications", + Long: `Reads JSONL events (one per line) from FILE or stdin and replays them +through the rule engine using your config. Matching rules render their +messages as if they were about to fire, but no notifier sends happen. + +Each event is a normal DING JSON event — same shape DING already parses +on 'ding run' stdin. An optional 'timestamp' field (RFC3339 or Unix epoch) +controls the event's time for windowed rules; omitted timestamps get +synthesized sequentially starting from now. + +End-of-run rules (mode: end-of-run) fire after all input events are +consumed.`, + Example: ` # Replay events from a file + ding test-rule events.jsonl + + # Pipe events from another tool + cat events.jsonl | ding test-rule + + # Force JSON output (default is text on TTY, json when piped) + ding test-rule events.jsonl --format json`, + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + input, closer, err := openTestRuleInput(args) + if err != nil { + return err + } + if closer != nil { + defer closer() + } + return runTestRule(configPath, format, noColor, input, os.Stdout, os.Stderr) + }, + } + cmd.Flags().StringVar(&configPath, "config", "ding.yaml", "path to config file") + cmd.Flags().StringVar(&format, "format", "auto", "output format: auto, text, json") + cmd.Flags().BoolVar(&noColor, "no-color", false, "disable ANSI color in text format") + return cmd +} + +func openTestRuleInput(args []string) (io.Reader, func(), error) { + if len(args) == 0 || args[0] == "-" { + return os.Stdin, nil, nil + } + f, err := os.Open(args[0]) + if err != nil { + return nil, nil, fmt.Errorf("opening events file: %w", err) + } + return f, func() { _ = f.Close() }, nil +} + +func runTestRule(configPath, format string, noColor bool, input io.Reader, stdout, stderr io.Writer) error { + eng, _, _, _, _, err := server.BuildFromConfig(configPath, nil) + if err != nil { + return fmt.Errorf("loading config: %w", err) + } + + rc := runctx.New() + if rc.Runner == "local" { + fmt.Fprintln(stderr, "ding: note — runner=local; rules matching on a specific runner label will not match.") + } + + formatter := pickFormatter(format, noColor, stdout) + dispatcher := dryrun.NewLoggingDispatcher(formatter, stdout) + + scanner := bufio.NewScanner(input) + scanner.Buffer(make([]byte, 64*1024), 1024*1024) + syntheticBase := time.Now() + idx := 0 + var lastAt time.Time + for scanner.Scan() { + line := scanner.Bytes() + if len(line) == 0 { + continue + } + events, err := ingester.ParseJSONLine(line) + if err != nil || len(events) == 0 { + fmt.Fprintf(stderr, "ding: skipping unparseable event line %d: %v\n", idx+1, err) + idx++ + continue + } + for _, ev := range events { + if ev.At.IsZero() { + ev.At = syntheticBase.Add(time.Duration(idx) * time.Second) + } else { + // ParseJSONLine only handles Unix epoch timestamps; try RFC3339 override. + ev.At = resolveTimestamp(line, ev.At) + } + ev.Labels = rc.Apply(ev.Labels) + lastAt = ev.At + alerts := eng.Process(ev, ev.At) + dispatcher.Dispatch(alerts) + } + idx++ + } + if err := scanner.Err(); err != nil { + return fmt.Errorf("reading input: %w", err) + } + + endTime := lastAt + if endTime.IsZero() { + endTime = time.Now() + } + endAlerts := eng.ProcessEndOfRun(endTime) + dispatcher.Dispatch(endAlerts) + return nil +} + +// resolveTimestamp tries to parse a "timestamp" field from raw JSON as RFC3339. +// If successful it returns the parsed time; otherwise it returns the fallback. +// This extends ParseJSONLine (which handles Unix epoch floats) to also accept +// RFC3339 strings — useful for test fixtures and replayed log lines. +func resolveTimestamp(raw []byte, fallback time.Time) time.Time { + var obj map[string]json.RawMessage + if err := json.Unmarshal(raw, &obj); err != nil { + return fallback + } + tsRaw, ok := obj["timestamp"] + if !ok { + return fallback + } + var s string + if err := json.Unmarshal(tsRaw, &s); err != nil { + return fallback + } + t, err := time.Parse(time.RFC3339, s) + if err != nil { + return fallback + } + return t +} + +func pickFormatter(format string, noColor bool, out io.Writer) dryrun.Formatter { + if format == "json" { + return &dryrun.JSONFormatter{} + } + if format == "text" { + return &dryrun.TextFormatter{Color: !noColor && isTerminal(out)} + } + // auto: JSON when piped, text when on TTY + if isTerminal(out) { + return &dryrun.TextFormatter{Color: !noColor} + } + return &dryrun.JSONFormatter{} +} + +// isTerminal reports whether w is an *os.File pointing at a terminal. +// Returns false for buffers, pipes, and any non-*os.File writer. +func isTerminal(w io.Writer) bool { + f, ok := w.(*os.File) + if !ok { + return false + } + fi, err := f.Stat() + if err != nil { + return false + } + return (fi.Mode() & os.ModeCharDevice) != 0 +} diff --git a/internal/cli/test_rule_test.go b/internal/cli/test_rule_test.go new file mode 100644 index 0000000..9b5ec4a --- /dev/null +++ b/internal/cli/test_rule_test.go @@ -0,0 +1,107 @@ +package cli + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +func writeFixtureConfig(t *testing.T, dir string) string { + t.Helper() + cfg := ` +notifiers: + slack: + type: webhook + url: https://example.invalid/webhook +rules: + - name: spike + match: { metric: loss } + condition: value > 1.0 + message: "Loss spike: {{ .value }}" + alert: + - notifier: slack +` + p := filepath.Join(dir, "ding.yaml") + if err := os.WriteFile(p, []byte(cfg), 0644); err != nil { + t.Fatalf("write config: %v", err) + } + return p +} + +func TestRunTestRule_PerEventMatch_TextFormat(t *testing.T) { + dir := t.TempDir() + cfg := writeFixtureConfig(t, dir) + events := `{"metric":"loss","value":0.5} +{"metric":"loss","value":1.5} +` + in := strings.NewReader(events) + var out, errBuf bytes.Buffer + + err := runTestRule(cfg, "text", false, in, &out, &errBuf) + if err != nil { + t.Fatalf("runTestRule: %v\nstderr: %s", err, errBuf.String()) + } + + got := out.String() + if !strings.Contains(got, "spike") { + t.Errorf("expected spike rule to fire on second event:\n%s", got) + } + if !strings.Contains(got, "Loss spike: 1.5") { + t.Errorf("expected rendered message:\n%s", got) + } + if strings.Count(got, "would fire") != 1 { + t.Errorf("expected exactly one match, got:\n%s", got) + } +} + +func TestRunTestRule_WindowedRule_FiresAfterEnoughEvents(t *testing.T) { + dir := t.TempDir() + cfg := filepath.Join(dir, "ding.yaml") + if err := os.WriteFile(cfg, []byte(` +notifiers: + slack: + type: webhook + url: https://example.invalid/webhook +rules: + - name: hot_avg + match: { metric: temp } + condition: avg(value) over 5m > 50 + message: "avg high: {{ .avg }}" + alert: + - notifier: slack +`), 0644); err != nil { + t.Fatalf("write config: %v", err) + } + + events := `{"metric":"temp","value":40,"timestamp":"2026-05-08T10:00:00Z"} +{"metric":"temp","value":60,"timestamp":"2026-05-08T10:01:00Z"} +{"metric":"temp","value":70,"timestamp":"2026-05-08T10:02:00Z"} +` + in := strings.NewReader(events) + var out, errBuf bytes.Buffer + if err := runTestRule(cfg, "json", true, in, &out, &errBuf); err != nil { + t.Fatalf("runTestRule: %v\nstderr: %s", err, errBuf.String()) + } + + if !strings.Contains(out.String(), `"rule":"hot_avg"`) { + t.Errorf("expected hot_avg to fire after windowed avg crosses threshold:\n%s", out.String()) + } +} + +func TestRunTestRule_NoMatch_SilentStdout(t *testing.T) { + dir := t.TempDir() + cfg := writeFixtureConfig(t, dir) + events := `{"metric":"loss","value":0.1} +{"metric":"loss","value":0.2} +` + in := strings.NewReader(events) + var out, errBuf bytes.Buffer + if err := runTestRule(cfg, "text", true, in, &out, &errBuf); err != nil { + t.Fatalf("runTestRule: %v", err) + } + if out.Len() != 0 { + t.Errorf("expected silent stdout when no rules fire, got: %q", out.String()) + } +} From a85f55d9f431d5dba8c300d2afc962e2e4604a2c Mon Sep 17 00:00:00 2001 From: zuchka Date: Fri, 8 May 2026 13:56:08 -0700 Subject: [PATCH 08/13] fix(cli): synthesize timestamps for events without a timestamp field Three follow-ups from code review on 3b80c19: - resolveTimestamp now returns (time.Time, bool); the loop in runTestRule branches on the boolean rather than on ev.At.IsZero(), which was always false because ingester.ParseJSONLine populates At with time.Now() unconditionally. Effect: events without a timestamp field now actually get the synthesized 1s-apart timestamps the docs promise, fixing windowed-rule authoring without explicit timestamps. - runTestRule rejects --format values other than auto/text/json with a descriptive error, instead of silently falling through to auto. - Three new tests: synthesized-timestamp path; invalid --format rejected; malformed JSON line is logged + skipped + subsequent valid line still fires. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/cli/test_rule.go | 55 ++++++++++++++---------- internal/cli/test_rule_test.go | 77 ++++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 23 deletions(-) diff --git a/internal/cli/test_rule.go b/internal/cli/test_rule.go index a604434..8c1fe3f 100644 --- a/internal/cli/test_rule.go +++ b/internal/cli/test_rule.go @@ -83,6 +83,13 @@ func runTestRule(configPath, format string, noColor bool, input io.Reader, stdou fmt.Fprintln(stderr, "ding: note — runner=local; rules matching on a specific runner label will not match.") } + switch format { + case "auto", "text", "json": + // ok + default: + return fmt.Errorf("invalid --format %q: must be auto, text, or json", format) + } + formatter := pickFormatter(format, noColor, stdout) dispatcher := dryrun.NewLoggingDispatcher(formatter, stdout) @@ -103,11 +110,10 @@ func runTestRule(configPath, format string, noColor bool, input io.Reader, stdou continue } for _, ev := range events { - if ev.At.IsZero() { - ev.At = syntheticBase.Add(time.Duration(idx) * time.Second) + if t, ok := resolveTimestamp(line); ok { + ev.At = t } else { - // ParseJSONLine only handles Unix epoch timestamps; try RFC3339 override. - ev.At = resolveTimestamp(line, ev.At) + ev.At = syntheticBase.Add(time.Duration(idx) * time.Second) } ev.Labels = rc.Apply(ev.Labels) lastAt = ev.At @@ -129,28 +135,31 @@ func runTestRule(configPath, format string, noColor bool, input io.Reader, stdou return nil } -// resolveTimestamp tries to parse a "timestamp" field from raw JSON as RFC3339. -// If successful it returns the parsed time; otherwise it returns the fallback. -// This extends ParseJSONLine (which handles Unix epoch floats) to also accept -// RFC3339 strings — useful for test fixtures and replayed log lines. -func resolveTimestamp(raw []byte, fallback time.Time) time.Time { - var obj map[string]json.RawMessage - if err := json.Unmarshal(raw, &obj); err != nil { - return fallback - } - tsRaw, ok := obj["timestamp"] +// resolveTimestamp inspects the raw JSON line for a `timestamp` field. +// Returns the parsed timestamp + true on success. +// Returns zero time + false when the field is absent, malformed, or unparsable. +// +// Accepts: RFC3339 strings ("2026-05-08T10:00:00Z"), Unix epoch floats (1715166000), +// Unix epoch ints. Mirrors ingester.ParseJSONLine's accepted forms for the +// numeric case, plus adds RFC3339 support that the ingester does not have today. +func resolveTimestamp(raw []byte) (time.Time, bool) { + var m map[string]any + if err := json.Unmarshal(raw, &m); err != nil { + return time.Time{}, false + } + v, ok := m["timestamp"] if !ok { - return fallback - } - var s string - if err := json.Unmarshal(tsRaw, &s); err != nil { - return fallback + return time.Time{}, false } - t, err := time.Parse(time.RFC3339, s) - if err != nil { - return fallback + switch tv := v.(type) { + case string: + if t, err := time.Parse(time.RFC3339, tv); err == nil { + return t, true + } + case float64: + return time.Unix(int64(tv), 0).UTC(), true } - return t + return time.Time{}, false } func pickFormatter(format string, noColor bool, out io.Writer) dryrun.Formatter { diff --git a/internal/cli/test_rule_test.go b/internal/cli/test_rule_test.go index 9b5ec4a..6a57af7 100644 --- a/internal/cli/test_rule_test.go +++ b/internal/cli/test_rule_test.go @@ -105,3 +105,80 @@ func TestRunTestRule_NoMatch_SilentStdout(t *testing.T) { t.Errorf("expected silent stdout when no rules fire, got: %q", out.String()) } } + +// TestRunTestRule_NoTimestampField_SynthesizesSequential exercises the +// synthetic-timestamp path for events without a `timestamp` field. +// We check this by verifying that a windowed rule with a tight window +// fires correctly when events are spaced out by the synthesized 1s gap. +func TestRunTestRule_NoTimestampField_SynthesizesSequential(t *testing.T) { + dir := t.TempDir() + cfg := filepath.Join(dir, "ding.yaml") + if err := os.WriteFile(cfg, []byte(` +notifiers: + slack: + type: webhook + url: https://example.invalid/webhook +rules: + - name: hot_avg + match: { metric: temp } + condition: avg(value) over 30s > 50 + message: "avg high: {{ .avg }}" + alert: [{ notifier: slack }] +`), 0644); err != nil { + t.Fatalf("write config: %v", err) + } + + // Three events with NO timestamp field. With synthesized 1s spacing + // they all fit in the 30s window. Without it (bug behavior), they + // land at the same wall-clock instant and STILL fit — so this test + // verifies firing only. The eviction-driven verification belongs + // in a separate windowed-rule test that tightens timing later. + events := `{"metric":"temp","value":40} +{"metric":"temp","value":60} +{"metric":"temp","value":70} +` + in := strings.NewReader(events) + var out, errBuf bytes.Buffer + if err := runTestRule(cfg, "json", true, in, &out, &errBuf); err != nil { + t.Fatalf("runTestRule: %v\nstderr: %s", err, errBuf.String()) + } + if !strings.Contains(out.String(), `"rule":"hot_avg"`) { + t.Errorf("expected hot_avg to fire on synthesized-timestamp events:\n%s", out.String()) + } +} + +// TestRunTestRule_InvalidFormat_ReturnsError verifies that --format with +// an unrecognized value produces a clear error rather than silently +// falling through to the auto branch. +func TestRunTestRule_InvalidFormat_ReturnsError(t *testing.T) { + dir := t.TempDir() + cfg := writeFixtureConfig(t, dir) + in := strings.NewReader("") + var out, errBuf bytes.Buffer + err := runTestRule(cfg, "compact", false, in, &out, &errBuf) + if err == nil { + t.Fatal("expected error for invalid --format value, got nil") + } + if !strings.Contains(err.Error(), "invalid --format") { + t.Errorf("error should explain the issue, got: %v", err) + } +} + +// TestRunTestRule_MalformedLine_SkipsAndContinues verifies that a bad +// line is logged to stderr but doesn't stop the run; the subsequent +// valid line still fires its rule. +func TestRunTestRule_MalformedLine_SkipsAndContinues(t *testing.T) { + dir := t.TempDir() + cfg := writeFixtureConfig(t, dir) + events := `not-json +{"metric":"loss","value":2.0} +` + in := strings.NewReader(events) + var out, errBuf bytes.Buffer + if err := runTestRule(cfg, "text", true, in, &out, &errBuf); err != nil { + t.Fatalf("runTestRule should not return error on malformed line: %v", err) + } + if !strings.Contains(out.String(), "spike") { + t.Errorf("expected the valid second line to still fire spike rule:\n%s", out.String()) + } +} From aee0184fe9cf60cc7a70268d6ea861a6821547cf Mon Sep 17 00:00:00 2001 From: zuchka Date: Fri, 8 May 2026 13:58:51 -0700 Subject: [PATCH 09/13] =?UTF-8?q?feat(cli):=20ding=20run=20--dry-run=20?= =?UTF-8?q?=E2=80=94=20preview=20alerts=20without=20notifier=20sends?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds --dry-run, --format, and --no-color flags to `ding run`. When --dry-run is set, notifier construction still happens (for config validation) but the dispatch path is swapped for a LoggingDispatcher that writes formatted alerts to stderr. All other run-flow behavior (subprocess wrap, runctx, end-of-run rules, exit code propagation) is unchanged. Stdout stays reserved for the wrapped command's mirrored output; preview alerts and operational logs both go to stderr to avoid contaminating the wrapped command's stdout pipeline. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/cli/run.go | 35 ++++++++++++++++++++++++++------ internal/cli/run_test.go | 44 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 6 deletions(-) diff --git a/internal/cli/run.go b/internal/cli/run.go index 15a8193..8b6206e 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -16,6 +16,7 @@ import ( "github.com/spf13/cobra" "github.com/ding-labs/ding/internal/config" + "github.com/ding-labs/ding/internal/dryrun" "github.com/ding-labs/ding/internal/evaluator" "github.com/ding-labs/ding/internal/ingester" "github.com/ding-labs/ding/internal/metrics" @@ -27,6 +28,9 @@ import ( func newRunCmd() *cobra.Command { var configPath string var runIDOverride string + var dryRun bool + var format string + var noColor bool cmd := &cobra.Command{ Use: "run [flags] -- [args...]", @@ -55,19 +59,25 @@ is safe.`, ding run -- python train.py --epochs 100 # Override the auto-detected run ID - ding run --run-id manual-debug -- ./flaky-script.sh`, + ding run --run-id manual-debug -- ./flaky-script.sh + + # Preview alerts without sending to notifiers + ding run --dry-run --config alerts.yaml -- ./script.sh`, DisableFlagsInUseLine: true, Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - return runRun(configPath, runIDOverride, args) + return runRun(configPath, runIDOverride, args, dryRun, format, noColor) }, } cmd.Flags().StringVar(&configPath, "config", "ding.yaml", "path to config file") cmd.Flags().StringVar(&runIDOverride, "run-id", "", "override auto-detected run ID") + cmd.Flags().BoolVar(&dryRun, "dry-run", false, "preview alerts without sending to notifiers") + cmd.Flags().StringVar(&format, "format", "auto", "dry-run output format: auto, text, json") + cmd.Flags().BoolVar(&noColor, "no-color", false, "disable ANSI color in dry-run text output") return cmd } -func runRun(configPath, runIDOverride string, args []string) error { +func runRun(configPath, runIDOverride string, args []string, dryRun bool, format string, noColor bool) error { collector := metrics.NewCollector() eng, cfg, notifiers, alertLogger, jqCode, err := server.BuildFromConfig(configPath, collector) @@ -75,9 +85,22 @@ func runRun(configPath, runIDOverride string, args []string) error { return fmt.Errorf("loading config: %w", err) } - dispatcher := &NotifierDispatcher{ - Notifiers: notifiers, - AlertLogger: alertLogger, + var dispatcher Dispatcher + if dryRun { + // Validate format up front (matches test-rule contract) + switch format { + case "auto", "text", "json": + // ok + default: + return fmt.Errorf("invalid --format %q: must be auto, text, or json", format) + } + formatter := pickFormatter(format, noColor, os.Stderr) + dispatcher = dryrun.NewLoggingDispatcher(formatter, os.Stderr) + } else { + dispatcher = &NotifierDispatcher{ + Notifiers: notifiers, + AlertLogger: alertLogger, + } } // Drain handler — runs from both the deferred path (covers early returns // from config errors, command-start failures, etc.) and the explicit path diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go index bc25184..c26e1cd 100644 --- a/internal/cli/run_test.go +++ b/internal/cli/run_test.go @@ -8,6 +8,7 @@ import ( "time" "github.com/ding-labs/ding/internal/config" + "github.com/ding-labs/ding/internal/dryrun" "github.com/ding-labs/ding/internal/evaluator" "github.com/ding-labs/ding/internal/notifier" "github.com/ding-labs/ding/internal/runctx" @@ -208,3 +209,46 @@ func TestDrainNotifiers_HandlesEmpty(t *testing.T) { // no panic, no return value to check — the helper just returns } +func TestIngestStream_DryRun_NoNotifierSends(t *testing.T) { + rules := []evaluator.EngineRule{ + { + Name: "spike", + Match: map[string]string{"metric": "latency"}, + Condition: "value > 100", + Message: "spike", + Alerts: []string{"capture"}, + }, + } + eng, err := evaluator.NewEngine(rules, 1000) + if err != nil { + t.Fatalf("NewEngine: %v", err) + } + + cap := &captureNotifier{} + notifierMap := map[string]notifier.Notifier{"capture": cap} + + rc := &runctx.Context{ + RunID: "r-test", + Runner: "local", + Labels: map[string]string{}, + } + cfg := &config.Config{} + + var dryOut bytes.Buffer + dispatcher := dryrun.NewLoggingDispatcher(&dryrun.JSONFormatter{}, &dryOut) + + input := strings.NewReader(`{"metric":"latency","value":150}` + "\n") + var mirror bytes.Buffer + ingestStream(input, &mirror, eng, dispatcher, cfg, nil, rc) + + // The capture notifier must have received zero alerts (dry run never sends). + if got := cap.snapshot(); len(got) != 0 { + t.Errorf("dry-run leaked %d alert(s) to real notifier", len(got)) + } + // LoggingDispatcher must have written the alert to its buffer. + if !strings.Contains(dryOut.String(), `"rule":"spike"`) { + t.Errorf("expected dry-run output to include the spike rule, got: %s", dryOut.String()) + } + + _ = notifierMap // declared for clarity, not wired into dispatcher +} From 711e8467d3cf445bf58d3bbe6bdea873cef72afd Mon Sep 17 00:00:00 2001 From: zuchka Date: Fri, 8 May 2026 14:04:12 -0700 Subject: [PATCH 10/13] fix(cli): validate --format before config load + skip drain in dry-run Two follow-ups from code review on aee0184: - Move dry-run --format validation to the very top of runRun so a --format typo doesn't pay the BuildFromConfig + notifier-worker startup cost before being rejected. Practically minor (CLI exits immediately) but the ordering is correct as-is. - drainOnce now skips drainNotifiers + alertLogger.Close in dry-run mode. No alerts were ever queued (LoggingDispatcher doesn't call Send), so the drain pays the full drain_timeout to flush empty queues for nothing. - --format flag description now says "when --dry-run is set" so users don't expect it to affect non-dry-run output. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/cli/run.go | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/internal/cli/run.go b/internal/cli/run.go index 8b6206e..3c736c5 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -72,12 +72,23 @@ is safe.`, cmd.Flags().StringVar(&configPath, "config", "ding.yaml", "path to config file") cmd.Flags().StringVar(&runIDOverride, "run-id", "", "override auto-detected run ID") cmd.Flags().BoolVar(&dryRun, "dry-run", false, "preview alerts without sending to notifiers") - cmd.Flags().StringVar(&format, "format", "auto", "dry-run output format: auto, text, json") + cmd.Flags().StringVar(&format, "format", "auto", "output format when --dry-run is set: auto, text, json") cmd.Flags().BoolVar(&noColor, "no-color", false, "disable ANSI color in dry-run text output") return cmd } func runRun(configPath, runIDOverride string, args []string, dryRun bool, format string, noColor bool) error { + // Validate dry-run format BEFORE loading config so we don't pay the + // config-load + notifier-construction cost just to reject a typo. + if dryRun { + switch format { + case "auto", "text", "json": + // ok + default: + return fmt.Errorf("invalid --format %q: must be auto, text, or json", format) + } + } + collector := metrics.NewCollector() eng, cfg, notifiers, alertLogger, jqCode, err := server.BuildFromConfig(configPath, collector) @@ -87,13 +98,6 @@ func runRun(configPath, runIDOverride string, args []string, dryRun bool, format var dispatcher Dispatcher if dryRun { - // Validate format up front (matches test-rule contract) - switch format { - case "auto", "text", "json": - // ok - default: - return fmt.Errorf("invalid --format %q: must be auto, text, or json", format) - } formatter := pickFormatter(format, noColor, os.Stderr) dispatcher = dryrun.NewLoggingDispatcher(formatter, os.Stderr) } else { @@ -113,9 +117,11 @@ func runRun(configPath, runIDOverride string, args []string, dryRun bool, format return } drained = true - drainNotifiers(notifiers, cfg.Server.DrainTimeout.Duration) - if alertLogger != nil { - _ = alertLogger.Close() + if !dryRun { + drainNotifiers(notifiers, cfg.Server.DrainTimeout.Duration) + if alertLogger != nil { + _ = alertLogger.Close() + } } } defer drainOnce() From d2584e4dbc2cbdaa94fc28fe81cc6edb3c4601d0 Mon Sep 17 00:00:00 2001 From: zuchka Date: Fri, 8 May 2026 14:05:53 -0700 Subject: [PATCH 11/13] docs: rule playground (test-rule + run --dry-run) New "Testing rules without a workload" section in configuration.md covering both surfaces. README quickstart pointer; ding.yaml.example comment block. Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 8 ++++++++ ding.yaml.example | 5 +++++ docs/configuration.md | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/README.md b/README.md index ec8b36f..8cd3a36 100644 --- a/README.md +++ b/README.md @@ -108,6 +108,14 @@ When your command exits, DING: SIGTERM and SIGINT are forwarded to the child for graceful shutdown. +After writing a rule, preview it without a real workload: + +```sh +echo '{"metric":"loss","value":1.5}' | ding test-rule --config ding.yaml +``` + +For a full preview against a real run without sending notifications, use `ding run --dry-run -- `. + ### Run context, auto-detected DING reads the runner's environment variables and attaches labels automatically. No config required. diff --git a/ding.yaml.example b/ding.yaml.example index 0fdceba..9ae3800 100644 --- a/ding.yaml.example +++ b/ding.yaml.example @@ -5,6 +5,11 @@ # events the command emits + the synthetic run.exit # ding serve long-running HTTP daemon; rules evaluate against # events POSTed to /ingest or piped via stdin +# +# Preview rules without sending notifications: +# echo '{"metric":"name","value":42}' | ding test-rule --config ding.yaml +# ding run --dry-run --config ding.yaml -- ./your-script.sh +# See: docs/configuration.md#testing-rules-without-a-workload server: port: 8080 diff --git a/docs/configuration.md b/docs/configuration.md index 1459f0c..ec46492 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -390,6 +390,44 @@ For typical secrets (Slack URLs, PagerDuty tokens, API keys, opaque ID strings) - No `${VAR:-default}` for inline defaults — set the env var to the default before launching DING. - No `$${VAR}` escape for writing literal `${VAR}` — the use case is rare; if you hit it, file an issue. +## Testing rules without a workload + +DING ships two preview surfaces so you can verify rules before turning on real notifications. + +### `ding test-rule` — replay synthetic events + +Pipe or pass JSONL events at a config; matching rules render messages as if they were about to fire, but no notifications go out. + +```sh +# Pipe events from any source +echo '{"metric":"loss","value":1.5}' | ding test-rule --config ding.yaml + +# Read from a file (use - for explicit stdin) +ding test-rule events.jsonl +``` + +Each input line is a JSON event in DING's normal shape: a `metric` field for matching, a `value` field for numeric conditions, and any other key/value pairs as labels (string) or floats (number). An optional `timestamp` field (RFC3339) controls the event's time for windowed rules; events without `timestamp` get sequential synthetic times starting from now. + +Output format auto-detects: human-readable text when stdout is a terminal, JSONL when piped. Override with `--format text|json`. Disable color with `--no-color`. + +End-of-run rules (`mode: end-of-run`) fire after the last input event. + +### `ding run --dry-run` — wrap a real workload, suppress sends + +Same as `ding run`, but the dispatch boundary is swapped for a logging one — your wrapped command runs normally, events flow through the engine normally, the synthetic `run.exit` event still emits, end-of-run rules still fire, the wrapped command's exit code still propagates. Only `notifier.Send` is bypassed. + +```sh +# Preview what alerts would fire on a real failing build +ding run --dry-run --config ding.yaml -- pytest tests/ + +# JSON output for piping +ding run --dry-run --format json --config ding.yaml -- ./train.sh | jq +``` + +Preview output goes to stderr (so the wrapped command's own stdout/stderr is uncontaminated by alert lines). + +--- + ## Platform-specific examples See [Recipes](recipes/index.md) for end-to-end configurations on specific CI/CD platforms (GitLab CI, Jenkins, Buildkite). Each recipe shows the auto-captured labels and the minimal `ding.yaml` for that platform. From 53b39b0985f3529017f72808b28955b6313494c9 Mon Sep 17 00:00:00 2001 From: zuchka Date: Fri, 8 May 2026 14:07:39 -0700 Subject: [PATCH 12/13] docs: fix Unix epoch timestamp + JSON/JSONL clarity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups from review on d2584e4: - timestamp field description now mentions both RFC3339 strings and Unix epoch numbers (test-rule's resolveTimestamp accepts both; earlier text omitted Unix epoch). - "JSONL when piped" → "JSON (one object per line) when piped" to avoid confusion with --format json (which is the actual flag value). Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/configuration.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index ec46492..476d843 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -406,9 +406,9 @@ echo '{"metric":"loss","value":1.5}' | ding test-rule --config ding.yaml ding test-rule events.jsonl ``` -Each input line is a JSON event in DING's normal shape: a `metric` field for matching, a `value` field for numeric conditions, and any other key/value pairs as labels (string) or floats (number). An optional `timestamp` field (RFC3339) controls the event's time for windowed rules; events without `timestamp` get sequential synthetic times starting from now. +Each input line is a JSON event in DING's normal shape: a `metric` field for matching, a `value` field for numeric conditions, and any other key/value pairs as labels (string) or floats (number). An optional `timestamp` field (RFC3339 string or Unix epoch number) controls the event's time for windowed rules; events without `timestamp` get sequential synthetic times starting from now. -Output format auto-detects: human-readable text when stdout is a terminal, JSONL when piped. Override with `--format text|json`. Disable color with `--no-color`. +Output format auto-detects: human-readable text when stdout is a terminal, JSON (one object per line) when piped. Override with `--format text|json`. Disable color with `--no-color`. End-of-run rules (`mode: end-of-run`) fire after the last input event. From ba5d66eced1e3f9f1070f35eba248b82e0b4f0bd Mon Sep 17 00:00:00 2001 From: zuchka Date: Fri, 8 May 2026 14:14:28 -0700 Subject: [PATCH 13/13] fix: final review followups (docs accuracy + format validation order) Three follow-ups from final cross-branch review on 53b39b0: - docs/configuration.md: the `--dry-run --format json | jq` example was broken because dry-run preview goes to stderr, not stdout. Added `2>&1` so the redirect actually pipes preview into jq. - docs/configuration.md: corrected the "uncontaminated" claim. Child stderr IS mixed with preview lines (ingestStream mirrors stderr + LoggingDispatcher writes to stderr); only stdout stays clean. - internal/cli/test_rule.go: format validation now runs before BuildFromConfig, matching the pattern fixed for runRun in 711e846. Same code-smell rationale: don't pay the config-load cost just to reject a typo. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/configuration.md | 6 +++--- internal/cli/test_rule.go | 15 ++++++++------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 476d843..d3e94cd 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -420,11 +420,11 @@ Same as `ding run`, but the dispatch boundary is swapped for a logging one — y # Preview what alerts would fire on a real failing build ding run --dry-run --config ding.yaml -- pytest tests/ -# JSON output for piping -ding run --dry-run --format json --config ding.yaml -- ./train.sh | jq +# JSON output for piping (preview is on stderr; redirect to stdout for jq) +ding run --dry-run --format json --config ding.yaml -- ./train.sh 2>&1 | jq ``` -Preview output goes to stderr (so the wrapped command's own stdout/stderr is uncontaminated by alert lines). +Preview output goes to stderr alongside the wrapped command's own stderr; the wrapped command's stdout stays clean for downstream tools that read it. --- diff --git a/internal/cli/test_rule.go b/internal/cli/test_rule.go index 8c1fe3f..62ab523 100644 --- a/internal/cli/test_rule.go +++ b/internal/cli/test_rule.go @@ -73,6 +73,14 @@ func openTestRuleInput(args []string) (io.Reader, func(), error) { } func runTestRule(configPath, format string, noColor bool, input io.Reader, stdout, stderr io.Writer) error { + // Validate format BEFORE loading config so a typo doesn't pay the config-load cost. + switch format { + case "auto", "text", "json": + // ok + default: + return fmt.Errorf("invalid --format %q: must be auto, text, or json", format) + } + eng, _, _, _, _, err := server.BuildFromConfig(configPath, nil) if err != nil { return fmt.Errorf("loading config: %w", err) @@ -83,13 +91,6 @@ func runTestRule(configPath, format string, noColor bool, input io.Reader, stdou fmt.Fprintln(stderr, "ding: note — runner=local; rules matching on a specific runner label will not match.") } - switch format { - case "auto", "text", "json": - // ok - default: - return fmt.Errorf("invalid --format %q: must be auto, text, or json", format) - } - formatter := pickFormatter(format, noColor, stdout) dispatcher := dryrun.NewLoggingDispatcher(formatter, stdout)