From 1c2c98f9ba5cabde1face7f997d7c80f65031fab Mon Sep 17 00:00:00 2001 From: Bartek Tofel Date: Tue, 29 Sep 2026 14:15:26 +0200 Subject: [PATCH 1/3] feat: allow to fetch alerts by labels --- grafana-alertcheck/.changeset/v0.1.4.md | 1 + grafana-alertcheck/README.md | 10 +- grafana-alertcheck/cmd/check.go | 19 ++- grafana-alertcheck/cmd/check_test.go | 15 +++ grafana-alertcheck/cmd/common.go | 45 ++++++- grafana-alertcheck/cmd/common_test.go | 44 +++++++ grafana-alertcheck/cmd/watch.go | 40 +++++-- grafana-alertcheck/cmd/watch_test.go | 12 ++ grafana-alertcheck/docs/architecture.md | 2 +- grafana-alertcheck/docs/index.md | 4 +- grafana-alertcheck/docs/reference/cli.md | 27 ++++- grafana-alertcheck/internal/gate/check.go | 22 ++-- .../internal/gate/check_test.go | 85 ++++++++++++++ grafana-alertcheck/internal/gate/labels.go | 111 ++++++++++++++++++ .../internal/gate/labels_test.go | 103 ++++++++++++++++ .../internal/gate/parse_ruler.go | 16 ++- .../internal/gate/parse_ruler_test.go | 16 +++ grafana-alertcheck/internal/gate/watch.go | 12 +- .../internal/gate/watch_test.go | 48 ++++++++ 19 files changed, 590 insertions(+), 42 deletions(-) create mode 100644 grafana-alertcheck/.changeset/v0.1.4.md create mode 100644 grafana-alertcheck/cmd/common_test.go create mode 100644 grafana-alertcheck/internal/gate/labels.go create mode 100644 grafana-alertcheck/internal/gate/labels_test.go diff --git a/grafana-alertcheck/.changeset/v0.1.4.md b/grafana-alertcheck/.changeset/v0.1.4.md new file mode 100644 index 000000000..b6bfdebe8 --- /dev/null +++ b/grafana-alertcheck/.changeset/v0.1.4.md @@ -0,0 +1 @@ +- `watch` and single-step `check` can now select alerts by labels instead of names: `--include-labels team=bcm,env=stage`, optionally refined with `--exclude-labels severity=info`. Matches are exact; label selection cannot be combined with `--alerts` or `--folder`, and is refused with `--in`. A selection that matches no rules exits `2`. diff --git a/grafana-alertcheck/README.md b/grafana-alertcheck/README.md index 6dd5e038e..19d68ee26 100644 --- a/grafana-alertcheck/README.md +++ b/grafana-alertcheck/README.md @@ -7,9 +7,10 @@ A CD quality gate for Grafana alerts. It bookends a release with two commands watch → your work → check ``` -`watch` starts a background recorder that polls each named alert into a JSONL log. After the work emits a -`from`/`to` pair, `check` proves continuous coverage of that window, classifies each alert's state -timeline, and exits `0`, `1`, or `2`. If the work fails first, `stop` reaps the recorder. +`watch` starts a background recorder that polls each watched alert into a JSONL log. Alerts are selected by +name or by labels. After the work emits a `from`/`to` pair, `check` proves continuous coverage of that +window, classifies each alert's state timeline, and exits `0`, `1`, or `2`. If the work fails first, `stop` +reaps the recorder. It **fails closed**: if it cannot get an answer, it stops the release — never a pass on an unproven window. @@ -25,6 +26,9 @@ grafana-alertcheck watch --out /tmp/run.jsonl --alerts alerts.txt grafana-alertcheck check --in /tmp/run.jsonl --from "$deployed_at" --to "$finished_at" ``` +Or select alerts by label instead of a file: `--include-labels team=bcm,env=stage` (optionally +`--exclude-labels`). See the [CLI reference](./docs/reference/cli.md#selecting-alerts-by-labels). + Requires Grafana >= 13.0.0 and < 14.0.0. Connection details come from the environment only — the token is never a flag. diff --git a/grafana-alertcheck/cmd/check.go b/grafana-alertcheck/cmd/check.go index ab063562a..74c368bd9 100644 --- a/grafana-alertcheck/cmd/check.go +++ b/grafana-alertcheck/cmd/check.go @@ -15,7 +15,8 @@ import ( ) const checkUsage = "usage: grafana-alertcheck check [--in ] [--pidfile F] --from RFC3339 --to RFC3339 " + - "[--alerts ...] [--folder F] [--states ...] [--preexisting ...] [--min-observed N] [--allow-paused] " + + "[--alerts ... | --include-labels k=v,...] [--exclude-labels k=v,...] [--folder F] " + + "[--states ...] [--preexisting ...] [--min-observed N] [--allow-paused] " + "[--nodata-is-unobservable] [--no-fail-fast] [--concurrency N] [--output json]" // runCheck is the classify step's CLI surface: parse flags into a gate.Config, @@ -55,6 +56,20 @@ func runCheck(args []string, stdin io.Reader, stdout, stderr io.Writer) int { fmt.Fprintf(stderr, "--output: unknown value %q (only \"json\" is supported)\n", *output) return 2 } + if *common.alerts != "" && (*common.includeLabels != "" || *common.excludeLabels != "") { + fmt.Fprintln(stderr, "check: --alerts cannot be combined with label selection") + return 2 + } + includeLabels, err := parseLabelPairs("--include-labels", *common.includeLabels) + if err != nil { + fmt.Fprintln(stderr, err) + return 2 + } + excludeLabels, err := parseLabelPairs("--exclude-labels", *common.excludeLabels) + if err != nil { + fmt.Fprintln(stderr, err) + return 2 + } url, token, err := grafanaEnv() if err != nil { @@ -82,6 +97,8 @@ func runCheck(args []string, stdin io.Reader, stdout, stderr io.Writer) int { Token: token, Alerts: alerts, Folder: *common.folder, + IncludeLabels: includeLabels, + ExcludeLabels: excludeLabels, States: stateList, Preexisting: preexistingPolicy, MinObserved: *minObserved, diff --git a/grafana-alertcheck/cmd/check_test.go b/grafana-alertcheck/cmd/check_test.go index 1bcffc245..9172f98f4 100644 --- a/grafana-alertcheck/cmd/check_test.go +++ b/grafana-alertcheck/cmd/check_test.go @@ -84,6 +84,21 @@ func TestRunCheck_FlagValidation(t *testing.T) { {"no alerts no in", true, func(t *testing.T) []string { return []string{"--to", "2026-01-01T00:00:00Z"} }, "no alert names"}, + {"alerts and labels", true, func(t *testing.T) []string { + return []string{"--to", "2026-01-01T00:00:00Z", "--alerts", writeTempAlerts(t), "--include-labels", "team=bcm"} + }, "cannot be combined with label selection"}, + {"bad label pair", true, func(t *testing.T) []string { + return []string{"--to", "2026-01-01T00:00:00Z", "--include-labels", "team"} + }, "--include-labels"}, + {"exclude without include", true, func(t *testing.T) []string { + return []string{"--to", "2026-01-01T00:00:00Z", "--exclude-labels", "severity=info"} + }, "requires --include-labels"}, + {"labels with in", true, func(t *testing.T) []string { + return []string{"--to", "2026-01-01T00:00:00Z", "--in", "some.jsonl", "--include-labels", "team=bcm"} + }, "refused with a recorded log"}, + {"labels with folder", true, func(t *testing.T) []string { + return []string{"--to", "2026-01-01T00:00:00Z", "--include-labels", "team=bcm", "--folder", "F"} + }, "--folder"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/grafana-alertcheck/cmd/common.go b/grafana-alertcheck/cmd/common.go index 3c56cd63f..2dc4f6a52 100644 --- a/grafana-alertcheck/cmd/common.go +++ b/grafana-alertcheck/cmd/common.go @@ -11,16 +11,18 @@ import ( "github.com/smartcontractkit/chainlink-testing-framework/grafana-alertcheck/internal/gate" ) -// commonFlags is registerCommon's result: the exactly three flags watch and -// check share. Connection details are never flags, and states / poll-interval -// are deliberately NOT here — states is check-only because recording is +// commonFlags is registerCommon's result: the flags watch and check share. +// Connection details are never flags, and states / poll-interval are +// deliberately NOT here — states is check-only because recording is // unfiltered, and poll-interval is watch-only because check reads the cadence // from the log header. Putting either here would give both commands an opinion // about a value only one of them may set. type commonFlags struct { - folder *string - concurrency *int - alerts *string + folder *string + concurrency *int + alerts *string + includeLabels *string + excludeLabels *string } func registerCommon(fs *flag.FlagSet) *commonFlags { @@ -28,9 +30,40 @@ func registerCommon(fs *flag.FlagSet) *commonFlags { folder: fs.String("folder", "", "default folder to scope an unqualified alert name to"), concurrency: fs.Int("concurrency", 1, "maximum concurrent requests to Grafana"), alerts: fs.String("alerts", "", "path to a file of alert names, one per line, or - for stdin"), + includeLabels: fs.String("include-labels", "", + "comma-separated key=value pairs selecting rules by label, e.g. team=bcm,env=stage (cannot be combined with --alerts)"), + excludeLabels: fs.String("exclude-labels", "", + "comma-separated key=value pairs; rules carrying any of them are dropped (requires --include-labels)"), } } +// parseLabelPairs parses a comma-separated list of exact-match key=value label +// pairs. An empty string means the flag was not given. +func parseLabelPairs(flagName, s string) ([]gate.LabelMatcher, error) { + if strings.TrimSpace(s) == "" { + return nil, nil + } + seen := make(map[string]bool) + var out []gate.LabelMatcher + for _, part := range strings.Split(s, ",") { + part = strings.TrimSpace(part) + if part == "" { + return nil, fmt.Errorf("%s: empty label pair in %q", flagName, s) + } + key, value, ok := strings.Cut(part, "=") + key, value = strings.TrimSpace(key), strings.TrimSpace(value) + if !ok || key == "" || value == "" { + return nil, fmt.Errorf("%s: %q is not a non-empty key=value pair", flagName, part) + } + if seen[key] { + return nil, fmt.Errorf("%s: duplicate label %q", flagName, key) + } + seen[key] = true + out = append(out, gate.LabelMatcher{Key: key, Value: value}) + } + return out, nil +} + // readAlerts reads alert names, one per line, from a file or from // stdin when path is "-". An empty path is not an error here — watch and // check each decide for themselves whether an empty list is allowed diff --git a/grafana-alertcheck/cmd/common_test.go b/grafana-alertcheck/cmd/common_test.go new file mode 100644 index 000000000..756f657c7 --- /dev/null +++ b/grafana-alertcheck/cmd/common_test.go @@ -0,0 +1,44 @@ +package main + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/smartcontractkit/chainlink-testing-framework/grafana-alertcheck/internal/gate" +) + +func TestParseLabelPairs(t *testing.T) { + tests := []struct { + name string + in string + want []gate.LabelMatcher + wantErr string + }{ + {"empty means not given", "", nil, ""}, + {"whitespace means not given", " ", nil, ""}, + {"single pair", "team=bcm", []gate.LabelMatcher{{Key: "team", Value: "bcm"}}, ""}, + {"spaces are trimmed", " team = bcm , env = stage ", []gate.LabelMatcher{ + {Key: "team", Value: "bcm"}, + {Key: "env", Value: "stage"}, + }, ""}, + {"value may contain equals", "query=a=b", []gate.LabelMatcher{{Key: "query", Value: "a=b"}}, ""}, + {"missing equals", "team", nil, "not a non-empty key=value pair"}, + {"empty key", "=bcm", nil, "not a non-empty key=value pair"}, + {"empty value", "team=", nil, "not a non-empty key=value pair"}, + {"empty segment", "team=bcm,", nil, "empty label pair"}, + {"duplicate key", "team=a,team=b", nil, "duplicate label"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseLabelPairs("--include-labels", tt.in) + if tt.wantErr != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tt.wantErr) + return + } + require.NoError(t, err) + require.Equal(t, tt.want, got) + }) + } +} diff --git a/grafana-alertcheck/cmd/watch.go b/grafana-alertcheck/cmd/watch.go index 69b585298..b13c0b3d2 100644 --- a/grafana-alertcheck/cmd/watch.go +++ b/grafana-alertcheck/cmd/watch.go @@ -12,7 +12,8 @@ import ( ) const watchUsage = "usage: grafana-alertcheck watch --out [--pidfile F] [--daemon-log F] " + - "--alerts [--folder F] [--poll-interval D] [--concurrency N] [--until RFC3339]" + "(--alerts | --include-labels k=v,...) [--exclude-labels k=v,...] [--folder F] " + + "[--poll-interval D] [--concurrency N] [--until RFC3339]" // runWatch is the record step's entire CLI surface, split in two by one flag // set — gate.DaemonChildFlag ("--daemon-child") and gate.ReadyFDFlag @@ -64,6 +65,21 @@ func runWatch(args []string, stdin io.Reader, stdout, stderr io.Writer) int { return runDaemonChild(*out, *until, *common.concurrency, *readyFD, stderr) } + if *common.alerts != "" && (*common.includeLabels != "" || *common.excludeLabels != "") { + fmt.Fprintln(stderr, "watch: --alerts cannot be combined with label selection") + return 2 + } + includeLabels, err := parseLabelPairs("--include-labels", *common.includeLabels) + if err != nil { + fmt.Fprintln(stderr, err) + return 2 + } + excludeLabels, err := parseLabelPairs("--exclude-labels", *common.excludeLabels) + if err != nil { + fmt.Fprintln(stderr, err) + return 2 + } + url, token, err := grafanaEnv() if err != nil { fmt.Fprintln(stderr, err) @@ -77,16 +93,18 @@ func runWatch(args []string, stdin io.Reader, stdout, stderr io.Writer) int { } cfg := gate.WatchConfig{ - URL: url, - Token: token, - Alerts: alerts, - Folder: *common.folder, - Out: *out, - PidFile: *pidfile, - DaemonLog: *daemonLog, - Concurrency: *common.concurrency, - Clock: gate.SystemClock{}, - Notes: newNoteStyler(stderr), + URL: url, + Token: token, + Alerts: alerts, + Folder: *common.folder, + IncludeLabels: includeLabels, + ExcludeLabels: excludeLabels, + Out: *out, + PidFile: *pidfile, + DaemonLog: *daemonLog, + Concurrency: *common.concurrency, + Clock: gate.SystemClock{}, + Notes: newNoteStyler(stderr), } if *until != "" { t, err := time.Parse(time.RFC3339, *until) diff --git a/grafana-alertcheck/cmd/watch_test.go b/grafana-alertcheck/cmd/watch_test.go index 5c8f53c55..861f0c4af 100644 --- a/grafana-alertcheck/cmd/watch_test.go +++ b/grafana-alertcheck/cmd/watch_test.go @@ -35,6 +35,18 @@ func TestRunWatch_FlagValidation(t *testing.T) { {"bad poll-interval", true, func(t *testing.T) []string { return []string{"--out", t.TempDir() + "/log.jsonl", "--alerts", writeTempAlerts(t), "--poll-interval", "not-a-duration"} }, "--poll-interval"}, + {"alerts and labels", true, func(t *testing.T) []string { + return []string{"--out", t.TempDir() + "/log.jsonl", "--alerts", writeTempAlerts(t), "--include-labels", "team=bcm"} + }, "cannot be combined with label selection"}, + {"bad label pair", true, func(t *testing.T) []string { + return []string{"--out", t.TempDir() + "/log.jsonl", "--include-labels", "team"} + }, "--include-labels"}, + {"exclude without include", true, func(t *testing.T) []string { + return []string{"--out", t.TempDir() + "/log.jsonl", "--exclude-labels", "severity=info"} + }, "requires --include-labels"}, + {"labels with folder", true, func(t *testing.T) []string { + return []string{"--out", t.TempDir() + "/log.jsonl", "--include-labels", "team=bcm", "--folder", "F"} + }, "--folder"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/grafana-alertcheck/docs/architecture.md b/grafana-alertcheck/docs/architecture.md index fa2971178..092d68c62 100644 --- a/grafana-alertcheck/docs/architecture.md +++ b/grafana-alertcheck/docs/architecture.md @@ -55,7 +55,7 @@ This, plus the declared supported range (Grafana >= 13.0.0, < 14.0.0), is how a `watch` detaches a background recorder so observation survives the step boundary: -1. Parent resolves names, writes the header, observes every non-paused rule once, checks the budget. +1. Parent resolves the alert set (names or labels), writes the header, observes every non-paused rule once, checks the budget. 2. Parent re-execs itself as the child (`--daemon-child`) under a new session/process group, stdout/stderr to the daemon log. 3. Child re-reads the header, reopens the log `O_APPEND`, takes the exclusive `flock`, and writes one readiness byte on `--ready-fd`. 4. Parent writes the pidfile **after** the readiness report, then returns. diff --git a/grafana-alertcheck/docs/index.md b/grafana-alertcheck/docs/index.md index f9d8cac0a..c7cd58c71 100644 --- a/grafana-alertcheck/docs/index.md +++ b/grafana-alertcheck/docs/index.md @@ -18,7 +18,7 @@ It **fails closed**: if it cannot get an answer, it stops the release. It never ## How it works, in one paragraph -`watch` starts a background recorder that polls each named alert and appends snapshots to a JSONL log. Your work then emits two RFC3339 timestamps — `from` (when the change landed) and `to` (when the work ended). `check` proves continuous coverage of `[from, to]`, builds a state timeline per alert, classifies it, and exits `0`, `1`, or `2`. +`watch` starts a background recorder that polls each watched alert and appends snapshots to a JSONL log. Your work then emits two RFC3339 timestamps — `from` (when the change landed) and `to` (when the work ended). `check` proves continuous coverage of `[from, to]`, builds a state timeline per alert, classifies it, and exits `0`, `1`, or `2`. ## Install @@ -46,7 +46,7 @@ grafana-alertcheck watch --out /tmp/run.jsonl --alerts alerts.txt grafana-alertcheck check --in /tmp/run.jsonl --from "$deployed_at" --to "$finished_at" ``` -`alerts.txt` holds one alert name per line. See [Naming alerts](./reference/cli#naming-alerts). +`alerts.txt` holds one alert name per line. See [Naming alerts](./reference/cli#naming-alerts). Alerts can also be selected by label instead of by name: `--include-labels team=bcm,env=stage` (optionally `--exclude-labels`). `watch` returns only after the recorder has observed every named, non-paused alert once and reported ready — so auth, name-resolution, and parse failures surface **before** your deploy runs. diff --git a/grafana-alertcheck/docs/reference/cli.md b/grafana-alertcheck/docs/reference/cli.md index 312ea4860..ff28b05b4 100644 --- a/grafana-alertcheck/docs/reference/cli.md +++ b/grafana-alertcheck/docs/reference/cli.md @@ -26,7 +26,8 @@ grafana-alertcheck list ```bash grafana-alertcheck watch --out [--pidfile F] [--daemon-log F] \ - --alerts [--folder F] [--poll-interval D] [--concurrency N] [--until RFC3339] + (--alerts | --include-labels k=v,...) [--exclude-labels k=v,...] \ + [--folder F] [--poll-interval D] [--concurrency N] [--until RFC3339] ``` | Flag | Default | Meaning | @@ -34,7 +35,9 @@ grafana-alertcheck watch --out [--pidfile F] [--daemon-log F] \ | `--out` | — | JSONL log path (required) | | `--pidfile` | `.pid` | Where the recorder's pid is written | | `--daemon-log` | `.daemon.log` | stdout/stderr sink for the detached recorder | -| `--alerts` | — | File of alert names, one per line, or `-` for stdin (required) | +| `--alerts` | — | File of alert names, one per line, or `-` for stdin (required unless `--include-labels`) | +| `--include-labels` | — | Comma-separated exact-match `key=value` pairs selecting rules by label (cannot be combined with `--alerts`) | +| `--exclude-labels` | — | Comma-separated exact-match `key=value` pairs; a rule carrying any of them is dropped (requires `--include-labels`) | | `--folder` | — | Default folder to scope unqualified names | | `--poll-interval` | half the rule's interval | Override every rule's cadence (never clamped) | | `--concurrency` | `1` | Max concurrent requests to Grafana | @@ -63,7 +66,8 @@ Use it when the work failed and the alert verdict no longer matters, but the rec ```bash grafana-alertcheck check [--in ] [--pidfile F] --from RFC3339 --to RFC3339 \ - [--alerts ...] [--folder F] [--states ...] [--preexisting ...] [--min-observed N] \ + [--alerts ... | --include-labels k=v,...] [--exclude-labels k=v,...] [--folder F] \ + [--states ...] [--preexisting ...] [--min-observed N] \ [--allow-paused] [--nodata-is-unobservable] [--no-fail-fast] [--concurrency N] [--output json] ``` @@ -73,7 +77,9 @@ grafana-alertcheck check [--in ] [--pidfile F] --from RFC3339 --to RFC3339 | `--pidfile` | `.pid` | Recorder to stop before reading `--in` | | `--from` | see below | Moment the deploy finished | | `--to` | — | End of the window (required) | -| `--alerts` | — | Required **without** `--in`; refused **with** `--in` | +| `--alerts` | — | Required **without** `--in` (unless `--include-labels`); refused **with** `--in` | +| `--include-labels` | — | Comma-separated exact-match `key=value` pairs selecting rules by label (cannot be combined with `--alerts`) | +| `--exclude-labels` | — | Comma-separated exact-match `key=value` pairs; a rule carrying any of them is dropped (requires `--include-labels`) | | `--states` | `firing` | Comma-separated bad states: `firing,pending,nodata,error` | | `--preexisting` | `fail-unless-recovered` | `fail-unless-recovered` \| `fail` \| `ignore` | | `--min-observed` | every resolved rule | Minimum rules that must be observed | @@ -100,6 +106,19 @@ Alert names take one of four forms: Datasource-managed and recording rules are refused with a specific error. A name matching multiple rules errors listing every candidate with the copyable `Folder/Group/Title` and its `uid:` form. A no-match errors with case-insensitive substring suggestions and points at `list`. Duplicate names that resolve to the same uid collapse to one (a note, not an error). +## Selecting alerts by labels + +Instead of naming alerts, `watch` and single-step `check` accept a label selection: + +```bash +grafana-alertcheck watch --out /tmp/run.jsonl --include-labels team=bcm,env=stage +grafana-alertcheck check --to "$finished_at" --include-labels team=bcm --exclude-labels severity=info +``` + +`--include-labels` takes comma-separated exact-match `key=value` pairs; a rule must carry **all** of them. `--exclude-labels` is optional and drops any rule carrying **one** of its pairs. A rule that does not carry the label is never dropped, only never included. Values cannot contain commas. + +The label flags cannot be combined with `--alerts` or `--folder`, and they are refused with `--in` — the recorded log names its own alert set. A selection that matches no rules, whose matches are all excluded, or that matches a datasource-managed or recording rule exits `2`: an empty watch set must never pass. + ## Output and exit codes The human table goes to **stderr**: `RESULTS` (one row per rule, with the verdict, time broken, check cadence and whether the window was observed), `VIOLATIONS` (one per distinct rule/verdict/state/health/note signature, with an `INSTANCES` count of the instances it stands for — instance identity is only in the JSON), and `LIMITS USED` (each rule's observation limits in plain words, explained by a legend under the table, plus the extra observation time, the evaluation wait and the largest measured clock difference). The JSON outcome values are `healthy`, `new_failure`, `still_failing`, `recovered`, `unstable`, `paused`, `not_verified` and the synthetic `not_counted`. `--output json` writes the result to stdout. diff --git a/grafana-alertcheck/internal/gate/check.go b/grafana-alertcheck/internal/gate/check.go index 9faa8f1f1..f04fa0f25 100644 --- a/grafana-alertcheck/internal/gate/check.go +++ b/grafana-alertcheck/internal/gate/check.go @@ -37,6 +37,11 @@ type Config struct { Alerts []string Folder string + // IncludeLabels selects the watched rules by exact-match labels instead of + // names; ExcludeLabels drops matching rules from that set. Both are refused + // in log mode, where the header IS the alert set. + IncludeLabels, ExcludeLabels []LabelMatcher + States []State Preexisting PreexistingPolicy MinObserved int @@ -133,16 +138,19 @@ func (cfg Config) validate() error { } named := cfg.namedAlerts() + labeled := len(cfg.IncludeLabels) > 0 || len(cfg.ExcludeLabels) > 0 if cfg.Log == "" { - // An empty Alerts is an error — but only without a log. - if len(named) == 0 { - return errors.New("check: no alert names given and no recorded log to take them from") + if err := validateSelection("check", len(named), cfg.IncludeLabels, cfg.ExcludeLabels, cfg.Folder, + "check: no alert names given, no --include-labels, and no recorded log to take them from"); err != nil { + return err } } else if len(named) > 0 { - // The other direction: with a log, the alert set comes from the log. - // Accepting both would mean reconciling two sets, which the log being - // the one source removes entirely. + // With a log, the alert set comes from the log. Accepting both would + // mean reconciling two sets, which the log being the one source + // removes entirely. return fmt.Errorf("check: --alerts is refused with a recorded log: %s already names the alert set it recorded", cfg.Log) + } else if labeled { + return fmt.Errorf("check: label selection is refused with a recorded log: %s already names the alert set it recorded", cfg.Log) } now := cfg.Clock.Now() @@ -239,7 +247,7 @@ func check(ctx context.Context, cfg Config, src Source) (Result, error) { } } } else { - resolved, notes, err = Resolve(allDefs, cfg.namedAlerts(), cfg.Folder) + resolved, notes, err = resolveAlertSet(allDefs, cfg.namedAlerts(), cfg.IncludeLabels, cfg.ExcludeLabels, cfg.Folder) } if err != nil { return Result{}, err diff --git a/grafana-alertcheck/internal/gate/check_test.go b/grafana-alertcheck/internal/gate/check_test.go index 2dc6cb1a6..d14ccf139 100644 --- a/grafana-alertcheck/internal/gate/check_test.go +++ b/grafana-alertcheck/internal/gate/check_test.go @@ -192,6 +192,35 @@ func TestCheckValidateRejectsBadConfigurations(t *testing.T) { mutate: func(c *Config) { c.Log = "log.jsonl"; c.Alerts = []string{"A"} }, wantErr: "--alerts is refused with a recorded log", }, + { + name: "log mode with labels", + mutate: func(c *Config) { + c.Log = "log.jsonl" + c.IncludeLabels = []LabelMatcher{{Key: "team", Value: "bcm"}} + }, + wantErr: "refused with a recorded log", + }, + { + name: "alerts and labels", + mutate: func(c *Config) { + c.Alerts = []string{"A"} + c.IncludeLabels = []LabelMatcher{{Key: "team", Value: "bcm"}} + }, + wantErr: "cannot be combined", + }, + { + name: "exclude without include", + mutate: func(c *Config) { c.ExcludeLabels = []LabelMatcher{{Key: "team", Value: "bcm"}} }, + wantErr: "requires --include-labels", + }, + { + name: "labels with folder", + mutate: func(c *Config) { + c.Folder = "F" + c.IncludeLabels = []LabelMatcher{{Key: "team", Value: "bcm"}} + }, + wantErr: "--folder", + }, { // Never a warning-and-continue. name: "log mode without from", @@ -252,6 +281,19 @@ func TestCheckValidateAcceptsAPastToWithALog(t *testing.T) { require.Equal(t, "log.jsonl.pid", cfg.PidFile) } +func TestCheckValidateAcceptsLabelSelection(t *testing.T) { + cfg := Config{ + URL: "https://grafana.example.com", + From: testNow, + To: testNow.Add(5 * time.Minute), + Clock: newFakeClock(testNow), + IncludeLabels: []LabelMatcher{{Key: "team", Value: "bcm"}}, + ExcludeLabels: []LabelMatcher{{Key: "env", Value: "stage"}}, + }.withDefaults() + + require.NoError(t, cfg.validate()) +} + // --------------------------------------------------------------------------- // Single-step mode // --------------------------------------------------------------------------- @@ -304,6 +346,49 @@ func TestCheckSingleStepDuplicateAlertNamesCollapseWithNote(t *testing.T) { require.Contains(t, notesOf(cfg), "counted once") } +// Label selection replaces the enumerated alert set end to end: only the rule +// matching include minus exclude is watched, and the run passes over it. +func TestCheckSingleStepSelectsByLabels(t *testing.T) { + clock := newVirtualClock(testNow) + cfg := baseConfig(t, clock) + cfg.Alerts = nil + cfg.IncludeLabels = []LabelMatcher{{Key: "team", Value: "bcm"}} + cfg.ExcludeLabels = []LabelMatcher{{Key: "env", Value: "stage"}} + + matching := checkDef() + matching.Labels = map[string]string{"team": "bcm", "env": "prod"} + excluded := checkDef() + excluded.UID, excluded.Title = "other", "Other" + excluded.Labels = map[string]string{"team": "bcm", "env": "stage"} + + src := newCheckSource(func(_ string, _ int) (Observation, error) { + return healthyObservation(clock.Now()), nil + }) + src.defs = []Definition{matching, excluded} + + res, err := check(context.Background(), cfg, src) + require.NoError(t, err) + require.Len(t, res.Verdicts, 1) + require.Equal(t, checkUID, res.Verdicts[0].RuleUID) + require.Empty(t, res.Violations) +} + +// A label selection matching nothing is exit 2, before any poll — never an +// empty watch set that would pass an unproven window. +func TestCheckSingleStepLabelSelectionWithNoMatchFails(t *testing.T) { + clock := newVirtualClock(testNow) + cfg := baseConfig(t, clock) + cfg.Alerts = nil + cfg.IncludeLabels = []LabelMatcher{{Key: "team", Value: "does-not-exist"}} + + src := newCheckSource(nil) // nil responder: any poll fails the test + src.defs = rulerDefs(t) + + _, err := check(context.Background(), cfg, src) + require.Error(t, err) + require.Contains(t, err.Error(), "no rule matches the include labels") +} + // A rule with health=error for the whole window is unobservable, exit 2 — // driven from the real "[JD] No Job Proposals" capture (testdata/README.md), // not a synthetic Poll table, so a change in how the real payload shapes diff --git a/grafana-alertcheck/internal/gate/labels.go b/grafana-alertcheck/internal/gate/labels.go new file mode 100644 index 000000000..15f2038d8 --- /dev/null +++ b/grafana-alertcheck/internal/gate/labels.go @@ -0,0 +1,111 @@ +package gate + +import ( + "errors" + "fmt" + "strings" +) + +// LabelMatcher is one exact-match label requirement. +type LabelMatcher struct { + Key, Value string +} + +// SelectByLabels resolves a label selection: a rule is selected when it +// carries every include pair, and dropped when it carries any exclude pair. +// A missing label never triggers an exclude (unlike a Prometheus != matcher); +// it only fails an include. Zero matches and a matched unsupported kind are +// errors, never a smaller watch set. +func SelectByLabels(defs []Definition, include, exclude []LabelMatcher) ([]Definition, error) { + matchedInclude := 0 + selected := make([]Definition, 0, len(defs)) + for _, d := range defs { + if !matchesAll(d.Labels, include) { + continue + } + matchedInclude++ + if matchesAny(d.Labels, exclude) { + continue + } + if d.Kind != KindGrafanaManaged { + return nil, fmt.Errorf("label selection matches %q, a %s, which is not supported", d.Title, kindName(d.Kind)) + } + selected = append(selected, d) + } + if matchedInclude == 0 { + return nil, fmt.Errorf("no rule matches the include labels %s (%d rules visible)", + formatLabelMatchers(include), len(defs)) + } + if len(selected) == 0 { + return nil, fmt.Errorf("all %d rule(s) matching the include labels %s are dropped by the exclude labels %s", + matchedInclude, formatLabelMatchers(include), formatLabelMatchers(exclude)) + } + return selected, nil +} + +// resolveAlertSet picks the alert set for a run. Validation guarantees the two +// modes are never mixed; an empty include list means enumerated names. +func resolveAlertSet(defs []Definition, names []string, include, exclude []LabelMatcher, folder string) ([]Definition, []string, error) { + if len(include) > 0 { + selected, err := SelectByLabels(defs, include, exclude) + return selected, nil, err + } + return Resolve(defs, names, folder) +} + +func matchesAll(labels map[string]string, matchers []LabelMatcher) bool { + for _, m := range matchers { + // The key check is load-bearing: without it an empty matcher value + // would match every rule missing the key. + if v, ok := labels[m.Key]; !ok || v != m.Value { + return false + } + } + return true +} + +// validateSelection enforces the alert-set rules shared by watch and check: +// names and labels are mutually exclusive, exclusions need inclusions, and a +// label selection cannot be scoped by --folder. emptySelection phrases the +// "neither mode named anything" error, which differs per command. +func validateSelection(cmd string, named int, include, exclude []LabelMatcher, folder, emptySelection string) error { + switch { + case named > 0 && (len(include) > 0 || len(exclude) > 0): + return fmt.Errorf("%s: --alerts cannot be combined with label selection", cmd) + case len(exclude) > 0 && len(include) == 0: + return fmt.Errorf("%s: --exclude-labels requires --include-labels; refusing to watch every rule", cmd) + case named == 0 && len(include) == 0: + return errors.New(emptySelection) + case (len(include) > 0 || len(exclude) > 0) && folder != "": + return fmt.Errorf("%s: --folder scopes alert names and cannot be combined with label selection", cmd) + } + return nil +} + +func matchesAny(labels map[string]string, matchers []LabelMatcher) bool { + for _, m := range matchers { + if v, ok := labels[m.Key]; ok && v == m.Value { + return true + } + } + return false +} + +func formatLabelMatchers(ms []LabelMatcher) string { + parts := make([]string, len(ms)) + for i, m := range ms { + parts[i] = m.Key + "=" + m.Value + } + return strings.Join(parts, ",") +} + +func kindName(k RuleKind) string { + switch k { + case KindDatasourceManaged: + return "datasource-managed rule" + case KindRecording: + return "recording rule" + default: + return "grafana-managed rule" + } +} diff --git a/grafana-alertcheck/internal/gate/labels_test.go b/grafana-alertcheck/internal/gate/labels_test.go new file mode 100644 index 000000000..b97f15503 --- /dev/null +++ b/grafana-alertcheck/internal/gate/labels_test.go @@ -0,0 +1,103 @@ +package gate + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func selectedUIDs(defs []Definition) []string { + out := make([]string, len(defs)) + for i, d := range defs { + out[i] = d.UID + } + return out +} + +func TestSelectByLabels_IncludeIsAnExactAND(t *testing.T) { + defs := rulerDefs(t) + + selected, err := SelectByLabels(defs, []LabelMatcher{{Key: "team", Value: "example-team"}}, nil) + require.NoError(t, err) + require.Equal(t, []string{"rule0000006a", "rule0000006b", "rule0000009", "rule0000010"}, selectedUIDs(selected)) + + selected, err = SelectByLabels(defs, []LabelMatcher{ + {Key: "team", Value: "example-team"}, + {Key: "severity", Value: "warning"}, + }, nil) + require.NoError(t, err) + require.Equal(t, []string{"rule0000009", "rule0000010"}, selectedUIDs(selected)) +} + +// A rule that does not carry an include label never matches, even when the +// label is missing rather than different. +func TestSelectByLabels_MissingIncludeLabelDoesNotMatch(t *testing.T) { + defs := rulerDefs(t) + + selected, err := SelectByLabels(defs, []LabelMatcher{{Key: "zone", Value: "zone-a"}}, nil) + require.NoError(t, err) + require.Equal(t, []string{"rule0000006a", "rule0000006b"}, selectedUIDs(selected)) + + _, err = SelectByLabels(defs, []LabelMatcher{{Key: "zone", Value: "zone-c"}}, nil) + require.Error(t, err) + require.Contains(t, err.Error(), "no rule matches the include labels") +} + +// Exclude drops rules that carry the pair; a missing label is never a reason +// to drop (unlike a Prometheus != matcher). +func TestSelectByLabels_ExcludeDropsCarriersOnly(t *testing.T) { + defs := rulerDefs(t) + + selected, err := SelectByLabels(defs, + []LabelMatcher{{Key: "env", Value: "production"}}, + []LabelMatcher{{Key: "severity", Value: "critical"}}) + require.NoError(t, err) + // rule0000007 has env=production and no severity: the missing label must + // keep it, not exclude it. Selection preserves the definitions' order. + require.Equal(t, []string{"rule0000009", "rule0000010", "rule0000007"}, selectedUIDs(selected)) +} + +func TestSelectByLabels_AllExcludedIsAnError(t *testing.T) { + defs := rulerDefs(t) + + _, err := SelectByLabels(defs, + []LabelMatcher{{Key: "team", Value: "example-team"}}, + []LabelMatcher{{Key: "team", Value: "example-team"}}) + require.Error(t, err) + require.Contains(t, err.Error(), "dropped by the exclude labels") +} + +func TestSelectByLabels_RefusesMatchedUnsupportedKinds(t *testing.T) { + dsDefs, err := ParseDefinitions(readFixture(t, "ruler_datasource_managed.json")) + require.NoError(t, err) + _, err = SelectByLabels(dsDefs, []LabelMatcher{{Key: "severity", Value: "warning"}}, nil) + require.Error(t, err) + require.Contains(t, err.Error(), "datasource-managed") + + recDefs := []Definition{{UID: "r1", Title: "recorded", Kind: KindRecording, Labels: map[string]string{"env": "production"}}} + _, err = SelectByLabels(recDefs, []LabelMatcher{{Key: "env", Value: "production"}}, nil) + require.Error(t, err) + require.Contains(t, err.Error(), "recording rule") +} + +// A caller-supplied empty matcher value must still require the key: a missing +// label is absent, not equal to the empty string. +func TestSelectByLabels_EmptyValueStillRequiresTheKey(t *testing.T) { + defs := []Definition{ + {UID: "absent", Title: "absent", Kind: KindGrafanaManaged, Labels: map[string]string{}}, + {UID: "empty", Title: "empty", Kind: KindGrafanaManaged, Labels: map[string]string{"env": ""}}, + } + selected, err := SelectByLabels(defs, []LabelMatcher{{Key: "env", Value: ""}}, nil) + require.NoError(t, err) + require.Equal(t, []string{"empty"}, selectedUIDs(selected)) +} + +// An empty include list matches every rule; the exclude list is then the only +// filter. Validation refuses this mode (it would watch the whole fleet), so +// this pins the pure function's behaviour only. +func TestSelectByLabels_EmptyIncludeIsNotAnError(t *testing.T) { + defs := rulerDefs(t) + selected, err := SelectByLabels(defs, nil, []LabelMatcher{{Key: "env", Value: "production"}}) + require.NoError(t, err) + require.Equal(t, []string{"rule0000002"}, selectedUIDs(selected)) +} diff --git a/grafana-alertcheck/internal/gate/parse_ruler.go b/grafana-alertcheck/internal/gate/parse_ruler.go index 5517fcce2..4a6b1bbbd 100644 --- a/grafana-alertcheck/internal/gate/parse_ruler.go +++ b/grafana-alertcheck/internal/gate/parse_ruler.go @@ -22,8 +22,10 @@ const ( // (/api/ruler/grafana/api/v1/rules). IntervalSeconds, NoDataState and // ExecErrState live inside the grafana_alert block and are only populated for // KindGrafanaManaged — a datasource-managed rule has no such block by -// definition. relativeTimeRange and keep_firing_for are deliberately not -// parsed: nothing in the gate reads them. +// definition. Labels is the rule's own label set, optional (the 13.1 fleet +// capture has three unlabeled rules) and the only input to label selection. +// relativeTimeRange and keep_firing_for are deliberately not parsed: nothing in +// the gate reads them. type Definition struct { UID, Title, Folder, FolderUID, Group string For time.Duration @@ -32,6 +34,7 @@ type Definition struct { ExecErrState string IsPaused bool Kind RuleKind + Labels map[string]string } // ParseDefinitions strictly parses a ruler-endpoint response body @@ -96,6 +99,11 @@ func parseDefinition(raw json.RawMessage, folder, group string) (Definition, err return Definition{}, fmt.Errorf("for: %w", err) } + var labels map[string]string + if err := opt(m, "labels", &labels); err != nil { + return Definition{}, fmt.Errorf("labels: %w", err) + } + var gaRaw json.RawMessage if err := opt(m, "grafana_alert", &gaRaw); err != nil { return Definition{}, err @@ -106,7 +114,7 @@ func parseDefinition(raw json.RawMessage, folder, group string) (Definition, err // name — "alert" for an alerting rule, "record" for a recording // one — never a synthetic UID (Grafana's ruler API gives this // shape no uid at all; inventing one would be inventing shape). - def := Definition{Folder: folder, Group: group, For: forDur, Kind: KindDatasourceManaged} + def := Definition{Folder: folder, Group: group, For: forDur, Kind: KindDatasourceManaged, Labels: labels} if err := opt(m, "alert", &def.Title); err != nil { return Definition{}, err } @@ -132,7 +140,7 @@ func parseDefinition(raw json.RawMessage, folder, group string) (Definition, err if err := req(ga, "uid", &uid); err != nil { return Definition{}, fmt.Errorf("grafana_alert: %w", err) } - def := Definition{Folder: folder, Group: group, For: forDur, UID: uid} + def := Definition{Folder: folder, Group: group, For: forDur, UID: uid, Labels: labels} // Classify by the presence of "record" before requiring anything else. // no_data_state/exec_err_state/is_paused/intervalSeconds are alerting-only diff --git a/grafana-alertcheck/internal/gate/parse_ruler_test.go b/grafana-alertcheck/internal/gate/parse_ruler_test.go index 280dc9910..64c5cf35f 100644 --- a/grafana-alertcheck/internal/gate/parse_ruler_test.go +++ b/grafana-alertcheck/internal/gate/parse_ruler_test.go @@ -47,6 +47,22 @@ func TestParseDefinitions_RulerRules(t *testing.T) { // Identity shared with testdata/state_paused.json. shared := byUID["rule0000002"] require.Equal(t, "folder0000002", shared.FolderUID) + + // Labels are parsed verbatim; an unlabeled rule stays empty. + require.Equal(t, map[string]string{ + "env": "production", "severity": "critical", "team": "example-team", "zone": "zone-a", + }, byUID["rule0000006a"].Labels) + require.Empty(t, byUID["rule0000002"].Labels) +} + +// The real 13.1 fleet capture has three rules with no labels key at all; that +// absence is legal and must not fail the whole parse. +func TestParseDefinitions_LabelsAreOptional(t *testing.T) { + body := []byte(`{"Example":[{"name":"G","rules":[{"expr":"","for":"1m","grafana_alert":{"title":"T","uid":"rule0000001","namespace_uid":"f","intervalSeconds":60,"no_data_state":"OK","exec_err_state":"OK","is_paused":false}}]}]}`) + defs, err := ParseDefinitions(body) + require.NoError(t, err) + require.Len(t, defs, 1) + require.Empty(t, defs[0].Labels) } func TestParseDefinitions_DatasourceManaged(t *testing.T) { diff --git a/grafana-alertcheck/internal/gate/watch.go b/grafana-alertcheck/internal/gate/watch.go index b4cfe1913..4598bcad0 100644 --- a/grafana-alertcheck/internal/gate/watch.go +++ b/grafana-alertcheck/internal/gate/watch.go @@ -52,6 +52,11 @@ type WatchConfig struct { Alerts []string Folder string + // IncludeLabels selects the watched rules by exact-match labels instead of + // names; ExcludeLabels drops matching rules from that set. The two + // selection modes cannot be combined (validate refuses it). + IncludeLabels, ExcludeLabels []LabelMatcher + // Out is the JSONL log path. PidFile and DaemonLog default to // .pid and .daemon.log — the same convention check uses to find // the recorder it must stop, so nothing has to be wired by hand. @@ -113,8 +118,9 @@ func (cfg WatchConfig) validate() error { named++ } } - if named == 0 { - return fmt.Errorf("watch: no alert names given; there is nothing to record") + if err := validateSelection("watch", named, cfg.IncludeLabels, cfg.ExcludeLabels, cfg.Folder, + "watch: no alert names given and no --include-labels; there is nothing to record"); err != nil { + return err } // An --until already in the past would make the child stop before it ever // polled, and the parent would then report a child that never reported @@ -288,7 +294,7 @@ func prepareWatch(ctx context.Context, cfg WatchConfig, src Source) (*preparedWa if err != nil { return nil, fmt.Errorf("read rule definitions: %w", err) } - resolved, notes, err := Resolve(defs, cfg.Alerts, cfg.Folder) + resolved, notes, err := resolveAlertSet(defs, cfg.Alerts, cfg.IncludeLabels, cfg.ExcludeLabels, cfg.Folder) if err != nil { return nil, err } diff --git a/grafana-alertcheck/internal/gate/watch_test.go b/grafana-alertcheck/internal/gate/watch_test.go index 7f877dc43..8342b7f14 100644 --- a/grafana-alertcheck/internal/gate/watch_test.go +++ b/grafana-alertcheck/internal/gate/watch_test.go @@ -482,6 +482,13 @@ func TestWatchConfigValidation(t *testing.T) { {"no log path", func(c *WatchConfig) { c.Out = "" }, "no log path"}, {"no alerts", func(c *WatchConfig) { c.Alerts = nil }, "no alert names"}, {"blank alerts only", func(c *WatchConfig) { c.Alerts = []string{"", " "} }, "no alert names"}, + {"alerts and labels", func(c *WatchConfig) { c.IncludeLabels = []LabelMatcher{{Key: "team", Value: "bcm"}} }, "cannot be combined"}, + {"exclude without include", func(c *WatchConfig) { c.Alerts = nil; c.ExcludeLabels = []LabelMatcher{{Key: "team", Value: "bcm"}} }, "requires --include-labels"}, + {"labels with folder", func(c *WatchConfig) { + c.Alerts = nil + c.Folder = "F" + c.IncludeLabels = []LabelMatcher{{Key: "team", Value: "bcm"}} + }, "--folder"}, {"until in the past", func(c *WatchConfig) { c.Until = testNow.Add(-time.Second) }, "not in the future"}, } { t.Run(tc.name, func(t *testing.T) { @@ -499,6 +506,47 @@ func TestWatchConfigValidation(t *testing.T) { require.NotEmpty(t, cfg.DaemonLog, "a detached child would have nowhere to explain a failure") require.NoError(t, cfg.validate()) }) + + t.Run("label selection alone is a valid alert set", func(t *testing.T) { + cfg := base() + cfg.Alerts = nil + cfg.IncludeLabels = []LabelMatcher{{Key: "team", Value: "bcm"}} + require.NoError(t, cfg.withDefaults().validate()) + }) +} + +// A label selection replaces Resolve: the header must name every matched rule, +// not the fixture's whole fleet. +func TestPrepareWatchSelectsByLabels(t *testing.T) { + var notes strings.Builder + cfg := watchTestConfig(t, ¬es) + cfg.IncludeLabels = []LabelMatcher{{Key: "severity", Value: "warning"}} + + src := watchTestSource(t, liveObservation(testNow)) + const weeklyUID, weeklyTitle = "rule0000010", "Example Failure Ratio Above 10 Percent Weekly" + src.script(weeklyTitle, observation(testNow, + testStateRule(weeklyUID, weeklyTitle, time.Minute, testNow, testInstance(StateNormal, "", "a"))), nil) + + prep, err := prepareWatch(context.Background(), cfg, src) + require.NoError(t, err) + require.NoError(t, prep.writer.Close()) + + header, polls, _, err := ReadLog(cfg.Out) + require.NoError(t, err) + require.Len(t, header.Rules, 2) + require.Equal(t, []string{watchActiveUID, weeklyUID}, + []string{header.Rules[0].UID, header.Rules[1].UID}) + require.Len(t, polls, 2) +} + +func TestPrepareWatchLabelSelectionWithNoMatchFails(t *testing.T) { + var notes strings.Builder + cfg := watchTestConfig(t, ¬es) + cfg.IncludeLabels = []LabelMatcher{{Key: "team", Value: "does-not-exist"}} + + _, err := prepareWatch(context.Background(), cfg, watchTestSource(t, liveObservation(testNow))) + require.Error(t, err) + require.Contains(t, err.Error(), "no rule matches the include labels") } // The fail-open direction, checked on the child's side: a log recorded at 5s on From 56cb4b1396261c82de4ad449db63156dd84a66de Mon Sep 17 00:00:00 2001 From: Bartek Tofel Date: Tue, 29 Sep 2026 14:52:21 +0200 Subject: [PATCH 2/3] chore: address code review comments --- grafana-alertcheck/cmd/common.go | 6 ++++-- grafana-alertcheck/cmd/common_test.go | 7 ++++--- grafana-alertcheck/docs/index.md | 2 +- grafana-alertcheck/docs/reference/cli.md | 2 +- grafana-alertcheck/internal/gate/labels.go | 6 +++--- 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/grafana-alertcheck/cmd/common.go b/grafana-alertcheck/cmd/common.go index 2dc4f6a52..a9d8feae2 100644 --- a/grafana-alertcheck/cmd/common.go +++ b/grafana-alertcheck/cmd/common.go @@ -50,10 +50,12 @@ func parseLabelPairs(flagName, s string) ([]gate.LabelMatcher, error) { if part == "" { return nil, fmt.Errorf("%s: empty label pair in %q", flagName, s) } + // An empty value is legal: it selects rules that carry the label with + // an empty value (a missing label never matches). key, value, ok := strings.Cut(part, "=") key, value = strings.TrimSpace(key), strings.TrimSpace(value) - if !ok || key == "" || value == "" { - return nil, fmt.Errorf("%s: %q is not a non-empty key=value pair", flagName, part) + if !ok || key == "" { + return nil, fmt.Errorf("%s: %q is not a key=value pair with a non-empty key", flagName, part) } if seen[key] { return nil, fmt.Errorf("%s: duplicate label %q", flagName, key) diff --git a/grafana-alertcheck/cmd/common_test.go b/grafana-alertcheck/cmd/common_test.go index 756f657c7..228d59125 100644 --- a/grafana-alertcheck/cmd/common_test.go +++ b/grafana-alertcheck/cmd/common_test.go @@ -23,9 +23,10 @@ func TestParseLabelPairs(t *testing.T) { {Key: "env", Value: "stage"}, }, ""}, {"value may contain equals", "query=a=b", []gate.LabelMatcher{{Key: "query", Value: "a=b"}}, ""}, - {"missing equals", "team", nil, "not a non-empty key=value pair"}, - {"empty key", "=bcm", nil, "not a non-empty key=value pair"}, - {"empty value", "team=", nil, "not a non-empty key=value pair"}, + {"empty value selects the empty value", "team=", []gate.LabelMatcher{{Key: "team"}}, ""}, + {"empty and set values mix", "env=,team=bcm", []gate.LabelMatcher{{Key: "env"}, {Key: "team", Value: "bcm"}}, ""}, + {"missing equals", "team", nil, "not a key=value pair with a non-empty key"}, + {"empty key", "=bcm", nil, "not a key=value pair with a non-empty key"}, {"empty segment", "team=bcm,", nil, "empty label pair"}, {"duplicate key", "team=a,team=b", nil, "duplicate label"}, } diff --git a/grafana-alertcheck/docs/index.md b/grafana-alertcheck/docs/index.md index c7cd58c71..ce3313edb 100644 --- a/grafana-alertcheck/docs/index.md +++ b/grafana-alertcheck/docs/index.md @@ -48,7 +48,7 @@ grafana-alertcheck check --in /tmp/run.jsonl --from "$deployed_at" --to "$finish `alerts.txt` holds one alert name per line. See [Naming alerts](./reference/cli#naming-alerts). Alerts can also be selected by label instead of by name: `--include-labels team=bcm,env=stage` (optionally `--exclude-labels`). -`watch` returns only after the recorder has observed every named, non-paused alert once and reported ready — so auth, name-resolution, and parse failures surface **before** your deploy runs. + `watch` returns only after the recorder has observed every selected, non-paused alert once and reported ready — so auth, alert-selection, and parse failures surface **before** your deploy runs. If your work fails before `check` runs and the alert verdict no longer matters, reap the recorder with `grafana-alertcheck stop --out /tmp/run.jsonl`. It is idempotent, so it is safe as an `if: always()` step: after `check` has already stopped the recorder it is a no-op. diff --git a/grafana-alertcheck/docs/reference/cli.md b/grafana-alertcheck/docs/reference/cli.md index ff28b05b4..6d025535f 100644 --- a/grafana-alertcheck/docs/reference/cli.md +++ b/grafana-alertcheck/docs/reference/cli.md @@ -115,7 +115,7 @@ grafana-alertcheck watch --out /tmp/run.jsonl --include-labels team=bcm,env=stag grafana-alertcheck check --to "$finished_at" --include-labels team=bcm --exclude-labels severity=info ``` -`--include-labels` takes comma-separated exact-match `key=value` pairs; a rule must carry **all** of them. `--exclude-labels` is optional and drops any rule carrying **one** of its pairs. A rule that does not carry the label is never dropped, only never included. Values cannot contain commas. +`--include-labels` takes comma-separated exact-match `key=value` pairs; a rule must carry **all** of them. `--exclude-labels` is optional and drops any rule carrying **one** of its pairs. A rule that does not carry the label is never dropped, only never included. Values cannot contain commas; `key=` matches only rules that carry the label with an empty value. The label flags cannot be combined with `--alerts` or `--folder`, and they are refused with `--in` — the recorded log names its own alert set. A selection that matches no rules, whose matches are all excluded, or that matches a datasource-managed or recording rule exits `2`: an empty watch set must never pass. diff --git a/grafana-alertcheck/internal/gate/labels.go b/grafana-alertcheck/internal/gate/labels.go index 15f2038d8..d913f46c4 100644 --- a/grafana-alertcheck/internal/gate/labels.go +++ b/grafana-alertcheck/internal/gate/labels.go @@ -24,12 +24,12 @@ func SelectByLabels(defs []Definition, include, exclude []LabelMatcher) ([]Defin continue } matchedInclude++ - if matchesAny(d.Labels, exclude) { - continue - } if d.Kind != KindGrafanaManaged { return nil, fmt.Errorf("label selection matches %q, a %s, which is not supported", d.Title, kindName(d.Kind)) } + if matchesAny(d.Labels, exclude) { + continue + } selected = append(selected, d) } if matchedInclude == 0 { From b7d968239aa8f7dfb8fd3f69cdeba6af25bbf6b6 Mon Sep 17 00:00:00 2001 From: Bartek Tofel Date: Tue, 29 Sep 2026 15:10:21 +0200 Subject: [PATCH 3/3] chore: fix help msg --- grafana-alertcheck/cmd/check.go | 2 +- grafana-alertcheck/cmd/watch.go | 2 +- grafana-alertcheck/docs/index.md | 2 +- grafana-alertcheck/docs/reference/cli.md | 9 +++++---- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/grafana-alertcheck/cmd/check.go b/grafana-alertcheck/cmd/check.go index 74c368bd9..7afcd7214 100644 --- a/grafana-alertcheck/cmd/check.go +++ b/grafana-alertcheck/cmd/check.go @@ -15,7 +15,7 @@ import ( ) const checkUsage = "usage: grafana-alertcheck check [--in ] [--pidfile F] --from RFC3339 --to RFC3339 " + - "[--alerts ... | --include-labels k=v,...] [--exclude-labels k=v,...] [--folder F] " + + "[--alerts ... [--folder F] | --include-labels k=v,... [--exclude-labels k=v,...]] " + "[--states ...] [--preexisting ...] [--min-observed N] [--allow-paused] " + "[--nodata-is-unobservable] [--no-fail-fast] [--concurrency N] [--output json]" diff --git a/grafana-alertcheck/cmd/watch.go b/grafana-alertcheck/cmd/watch.go index b13c0b3d2..f97362738 100644 --- a/grafana-alertcheck/cmd/watch.go +++ b/grafana-alertcheck/cmd/watch.go @@ -12,7 +12,7 @@ import ( ) const watchUsage = "usage: grafana-alertcheck watch --out [--pidfile F] [--daemon-log F] " + - "(--alerts | --include-labels k=v,...) [--exclude-labels k=v,...] [--folder F] " + + "(--alerts [--folder F] | --include-labels k=v,... [--exclude-labels k=v,...]) " + "[--poll-interval D] [--concurrency N] [--until RFC3339]" // runWatch is the record step's entire CLI surface, split in two by one flag diff --git a/grafana-alertcheck/docs/index.md b/grafana-alertcheck/docs/index.md index ce3313edb..d93630cbb 100644 --- a/grafana-alertcheck/docs/index.md +++ b/grafana-alertcheck/docs/index.md @@ -48,7 +48,7 @@ grafana-alertcheck check --in /tmp/run.jsonl --from "$deployed_at" --to "$finish `alerts.txt` holds one alert name per line. See [Naming alerts](./reference/cli#naming-alerts). Alerts can also be selected by label instead of by name: `--include-labels team=bcm,env=stage` (optionally `--exclude-labels`). - `watch` returns only after the recorder has observed every selected, non-paused alert once and reported ready — so auth, alert-selection, and parse failures surface **before** your deploy runs. +`watch` returns only after the recorder has observed every selected, non-paused alert once and reported ready — so auth, alert-selection, and parse failures surface **before** your deploy runs. If your work fails before `check` runs and the alert verdict no longer matters, reap the recorder with `grafana-alertcheck stop --out /tmp/run.jsonl`. It is idempotent, so it is safe as an `if: always()` step: after `check` has already stopped the recorder it is a no-op. diff --git a/grafana-alertcheck/docs/reference/cli.md b/grafana-alertcheck/docs/reference/cli.md index 6d025535f..6cd78d49b 100644 --- a/grafana-alertcheck/docs/reference/cli.md +++ b/grafana-alertcheck/docs/reference/cli.md @@ -26,8 +26,8 @@ grafana-alertcheck list ```bash grafana-alertcheck watch --out [--pidfile F] [--daemon-log F] \ - (--alerts | --include-labels k=v,...) [--exclude-labels k=v,...] \ - [--folder F] [--poll-interval D] [--concurrency N] [--until RFC3339] + (--alerts [--folder F] | --include-labels k=v,... [--exclude-labels k=v,...]) \ + [--poll-interval D] [--concurrency N] [--until RFC3339] ``` | Flag | Default | Meaning | @@ -36,9 +36,9 @@ grafana-alertcheck watch --out [--pidfile F] [--daemon-log F] \ | `--pidfile` | `.pid` | Where the recorder's pid is written | | `--daemon-log` | `.daemon.log` | stdout/stderr sink for the detached recorder | | `--alerts` | — | File of alert names, one per line, or `-` for stdin (required unless `--include-labels`) | +| `--folder` | — | Default folder to scope unqualified names (with `--alerts` only) | | `--include-labels` | — | Comma-separated exact-match `key=value` pairs selecting rules by label (cannot be combined with `--alerts`) | | `--exclude-labels` | — | Comma-separated exact-match `key=value` pairs; a rule carrying any of them is dropped (requires `--include-labels`) | -| `--folder` | — | Default folder to scope unqualified names | | `--poll-interval` | half the rule's interval | Override every rule's cadence (never clamped) | | `--concurrency` | `1` | Max concurrent requests to Grafana | | `--until` | run until signalled | Optional hard stop | @@ -66,7 +66,7 @@ Use it when the work failed and the alert verdict no longer matters, but the rec ```bash grafana-alertcheck check [--in ] [--pidfile F] --from RFC3339 --to RFC3339 \ - [--alerts ... | --include-labels k=v,...] [--exclude-labels k=v,...] [--folder F] \ + [--alerts ... [--folder F] | --include-labels k=v,... [--exclude-labels k=v,...]] \ [--states ...] [--preexisting ...] [--min-observed N] \ [--allow-paused] [--nodata-is-unobservable] [--no-fail-fast] [--concurrency N] [--output json] ``` @@ -78,6 +78,7 @@ grafana-alertcheck check [--in ] [--pidfile F] --from RFC3339 --to RFC3339 | `--from` | see below | Moment the deploy finished | | `--to` | — | End of the window (required) | | `--alerts` | — | Required **without** `--in` (unless `--include-labels`); refused **with** `--in` | +| `--folder` | — | Default folder to scope unqualified names (with `--alerts` only) | | `--include-labels` | — | Comma-separated exact-match `key=value` pairs selecting rules by label (cannot be combined with `--alerts`) | | `--exclude-labels` | — | Comma-separated exact-match `key=value` pairs; a rule carrying any of them is dropped (requires `--include-labels`) | | `--states` | `firing` | Comma-separated bad states: `firing,pending,nodata,error` |