fix(monitors): drop notify_by=["*"] when query has no by-group clause#625
Merged
Conversation
riyazsh
force-pushed
the
riyaz/HAMR-392-monitor-notify-by
branch
from
July 13, 2026 20:30
1de8187 to
51159c4
Compare
Some environments enable a validator that rejects a non-empty notify_by
when the monitor query lacks a `by {...}` group clause. The ["*"] value
is a documented "simple monitor" sentinel that is effectively a no-op
on an ungrouped monitor.
Drop options.notify_by only when it is exactly ["*"] and the query has
no `by {...}` clause. Any concrete group-key list (e.g. ["host"]) on an
ungrouped query is a real misconfiguration and is left in place so the
400 still surfaces it.
Runs in pre_resource_action_hook alongside the existing custom_schedule
and warning_recovery migrations. Null-safe on options and query.
7 new unit tests.
riyazsh
force-pushed
the
riyaz/HAMR-392-monitor-notify-by
branch
from
July 13, 2026 20:35
51159c4 to
5b5aa18
Compare
michael-richey
approved these changes
Jul 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
options.notify_by = ["*"]is a documented "simple monitor" sentinel that is effectively a no-op on an ungrouped monitor. Some destinations enable a validator that rejects any non-emptynotify_bywhen the query has no grouping clause. Drop the sentinel only when it is exactly["*"]AND the query has no grouping. Any concrete list (e.g.["host"]) on an ungrouped query is a real misconfiguration and stays in place so the 400 still surfaces.Grouping detection covers:
by {host, service}(query / metric / service check alerts).by(...)/.BY(...)(log / event-v2 / process alerts, case-insensitive)options.variables[*].group_by(formula/function monitors, invisible to a query-string regex)Runs in
pre_resource_action_hookalongside existingcustom_schedule/warning_recoverymigrations. Null-safe onoptions,query, andvariables.Test plan
14 new unit tests in
TestMonitorsSchemaMigrations— drop on ungrouped queries acrossquery alert/event-v2 alert/log alert; preserve on grouped queries in every syntax form (braces,.by(...), formula variables, uppercaseBY); preserve named group keys and multi-entry lists; no-op whennotify_byabsent or empty; null-safety on missing query. Fulltests/unit/suite passes (0 regressions).