Alerting: Support recording rule struct in provisioning API #87849
Conversation
rwwiv
added
area/alerting
rwwiv
requested review from
jtheory,
JacobsonMT,
yuri-tceretian,
grobinson-grafana and
inf0rmer
and removed request for
a team
rwwiv
force-pushed
the
rwwiv/recording-rule-provis-api
branch
from
685cfd5
to
5bb8638
Compare
pkg/services/ngalert/api/compat.go
Outdated
| @@ -456,7 +456,17 @@ func NotificationSettingsFromAlertRuleNotificationSettings(ns *definitions.Alert | |||
| } | |||
| } | |||
|
|
|||
| func ApiRecordFromModelRecord(r *models.Record) *definitions.Record { | |||
| func AlertRuleRecordFromRecord(r *definitions.Record) *models.Record { | |||
There was a problem hiding this comment.
Looks like we both wrote the same conversion func :D
I think this can be dropped (or can replace) ModelRecordFromApiRecord below
There was a problem hiding this comment.
Just dropped this, prefer your naming scheme there 👍
| @@ -477,6 +477,18 @@ type AlertRuleNotificationSettings struct { | |||
| MuteTimeIntervals []string `json:"mute_time_intervals,omitempty"` | |||
| } | |||
|
|
|||
| // swagger:model | |||
| type Record struct { | |||
There was a problem hiding this comment.
I think my PR also introduced this type in this file - but your version is better because of the doc comments and examples. I think you'll need to delete mine? It's called the same thing.
| "", | ||
| ) | ||
| } | ||
|
|
||
| provenance := determineProvenance(c) | ||
| createdAlertRule, err := srv.alertRules.CreateAlertRule(c.Req.Context(), c.SignedInUser, upstreamModel, alerting_models.Provenance(provenance)) |
There was a problem hiding this comment.
Sadly I think we'll need to duplicate much of the validation code, inside the AlertRuleService too. The main API validation can't be reused by provisioning because of how that whole validation flow is architected (it mixes type conversions, that are specific to the main API, alongside validation - this is true for validation of all provisioning fields).
There was a problem hiding this comment.
This can ofc be saved for a future PR - but if we decide to do so, let's make sure we add a checkbox to the tasklist to remember it. We'll at least need to copy some validation logic, at most need to separate validation in a way that's reusable.
What is this feature?
Adds support for creating recording rules in the main API.
Recording rules have a new field,
Record, and lack several of the other "statefulness" fields that only make sense for alerting rules.Why do we need this feature?
Allows users to provision recording rules.
Which issue(s) does this PR fix?:
N/A
Special notes for your reviewer:
Please check that: