From 58fdb24b0b1b752752bd3aea1d893117a1390778 Mon Sep 17 00:00:00 2001 From: Alexander Weaver Date: Fri, 7 Jun 2024 11:24:06 -0500 Subject: [PATCH] Alerting: Recording rules appear as type=recording in Prometheus API + better abstraction for type (#88805) * Wire status through to prom API * Regenerate swagger --- pkg/services/ngalert/api/api_prometheus.go | 2 +- pkg/services/ngalert/api/api_provisioning.go | 4 +- pkg/services/ngalert/api/tooling/api.json | 34 +++++++++--- .../ngalert/api/tooling/definitions/prom.go | 6 +-- pkg/services/ngalert/api/tooling/post.json | 9 ++-- pkg/services/ngalert/api/tooling/spec.json | 8 +-- pkg/services/ngalert/models/alert_rule.go | 22 ++++++-- pkg/services/ngalert/schedule/alert_rule.go | 2 +- .../ngalert/schedule/recording_rule.go | 2 +- pkg/services/ngalert/store/alert_rule_test.go | 6 +-- public/api-merged.json | 52 +------------------ public/openapi3.json | 52 +------------------ 12 files changed, 65 insertions(+), 134 deletions(-) diff --git a/pkg/services/ngalert/api/api_prometheus.go b/pkg/services/ngalert/api/api_prometheus.go index ad3981e2fe6..ac2bbad3718 100644 --- a/pkg/services/ngalert/api/api_prometheus.go +++ b/pkg/services/ngalert/api/api_prometheus.go @@ -451,7 +451,7 @@ func toRuleGroup(log log.Logger, manager state.AlertInstanceManager, groupKey ng Name: rule.Title, Labels: rule.GetLabels(labelOptions...), Health: "ok", - Type: apiv1.RuleTypeAlerting, + Type: rule.Type().String(), LastEvaluation: time.Time{}, } diff --git a/pkg/services/ngalert/api/api_provisioning.go b/pkg/services/ngalert/api/api_provisioning.go index dbc930aecab..7e44a1893a9 100644 --- a/pkg/services/ngalert/api/api_provisioning.go +++ b/pkg/services/ngalert/api/api_provisioning.go @@ -341,7 +341,7 @@ func (srv *ProvisioningSrv) RoutePostAlertRule(c *contextmodel.ReqContext, ar de return ErrResp(http.StatusBadRequest, err, "") } - if upstreamModel.IsRecordingRule() && !srv.featureManager.IsEnabledGlobally(featuremgmt.FlagGrafanaManagedRecordingRules) { + if upstreamModel.Type() == alerting_models.RuleTypeRecording && !srv.featureManager.IsEnabledGlobally(featuremgmt.FlagGrafanaManagedRecordingRules) { return ErrResp( http.StatusBadRequest, fmt.Errorf("%w: recording rules cannot be created on this instance", alerting_models.ErrAlertRuleFailedValidation), @@ -377,7 +377,7 @@ func (srv *ProvisioningSrv) RoutePutAlertRule(c *contextmodel.ReqContext, ar def ErrResp(http.StatusBadRequest, err, "") } - if updated.IsRecordingRule() && !srv.featureManager.IsEnabledGlobally(featuremgmt.FlagGrafanaManagedRecordingRules) { + if updated.Type() == alerting_models.RuleTypeRecording && !srv.featureManager.IsEnabledGlobally(featuremgmt.FlagGrafanaManagedRecordingRules) { return ErrResp( http.StatusBadRequest, fmt.Errorf("%w: recording rules cannot be created on this instance", alerting_models.ErrAlertRuleFailedValidation), diff --git a/pkg/services/ngalert/api/tooling/api.json b/pkg/services/ngalert/api/tooling/api.json index c48b501935b..5826f28cf70 100644 --- a/pkg/services/ngalert/api/tooling/api.json +++ b/pkg/services/ngalert/api/tooling/api.json @@ -478,7 +478,7 @@ "type": "object" }, "type": { - "$ref": "#/definitions/RuleType" + "type": "string" } }, "required": [ @@ -3479,7 +3479,7 @@ "type": "string" }, "type": { - "$ref": "#/definitions/RuleType" + "type": "string" } }, "required": [ @@ -3597,10 +3597,6 @@ ], "type": "object" }, - "RuleType": { - "title": "RuleType models the type of a rule.", - "type": "string" - }, "SNSConfig": { "properties": { "api_url": { @@ -4668,7 +4664,6 @@ "type": "object" }, "gettableAlerts": { - "description": "GettableAlerts gettable alerts", "items": { "$ref": "#/definitions/gettableAlert", "type": "object" @@ -4743,6 +4738,31 @@ }, "type": "array" }, + "gettableGrafanaSilence": { + "properties": { + "accessControl": { + "additionalProperties": { + "type": "boolean" + }, + "example": { + "create": false, + "read": true, + "write": false + }, + "type": "object" + }, + "metadata": { + "$ref": "#/definitions/SilenceMetadata" + } + }, + "type": "object" + }, + "gettableGrafanaSilences": { + "items": { + "$ref": "#/definitions/gettableGrafanaSilence" + }, + "type": "array" + }, "gettableSilence": { "description": "GettableSilence gettable silence", "properties": { diff --git a/pkg/services/ngalert/api/tooling/definitions/prom.go b/pkg/services/ngalert/api/tooling/definitions/prom.go index 59bc32e041f..cb37533377c 100644 --- a/pkg/services/ngalert/api/tooling/definitions/prom.go +++ b/pkg/services/ngalert/api/tooling/definitions/prom.go @@ -172,9 +172,9 @@ type Rule struct { Health string `json:"health"` LastError string `json:"lastError,omitempty"` // required: true - Type v1.RuleType `json:"type"` - LastEvaluation time.Time `json:"lastEvaluation"` - EvaluationTime float64 `json:"evaluationTime"` + Type string `json:"type"` + LastEvaluation time.Time `json:"lastEvaluation"` + EvaluationTime float64 `json:"evaluationTime"` } // Alert has info for an alert. diff --git a/pkg/services/ngalert/api/tooling/post.json b/pkg/services/ngalert/api/tooling/post.json index 9929f15fbf3..65574070653 100644 --- a/pkg/services/ngalert/api/tooling/post.json +++ b/pkg/services/ngalert/api/tooling/post.json @@ -478,7 +478,7 @@ "type": "object" }, "type": { - "$ref": "#/definitions/RuleType" + "type": "string" } }, "required": [ @@ -3476,7 +3476,7 @@ "type": "string" }, "type": { - "$ref": "#/definitions/RuleType" + "type": "string" } }, "required": [ @@ -3594,10 +3594,6 @@ ], "type": "object" }, - "RuleType": { - "title": "RuleType models the type of a rule.", - "type": "string" - }, "SNSConfig": { "properties": { "api_url": { @@ -4503,6 +4499,7 @@ "type": "object" }, "alertGroups": { + "description": "AlertGroups alert groups", "items": { "$ref": "#/definitions/alertGroup", "type": "object" diff --git a/pkg/services/ngalert/api/tooling/spec.json b/pkg/services/ngalert/api/tooling/spec.json index fb2dec22a71..f8d93d36b8d 100644 --- a/pkg/services/ngalert/api/tooling/spec.json +++ b/pkg/services/ngalert/api/tooling/spec.json @@ -4040,7 +4040,7 @@ } }, "type": { - "$ref": "#/definitions/RuleType" + "type": "string" } } }, @@ -7037,7 +7037,7 @@ "type": "string" }, "type": { - "$ref": "#/definitions/RuleType" + "type": "string" } } }, @@ -7148,10 +7148,6 @@ } } }, - "RuleType": { - "type": "string", - "title": "RuleType models the type of a rule." - }, "SNSConfig": { "type": "object", "properties": { diff --git a/pkg/services/ngalert/models/alert_rule.go b/pkg/services/ngalert/models/alert_rule.go index a2a1680c041..3408b7baa23 100644 --- a/pkg/services/ngalert/models/alert_rule.go +++ b/pkg/services/ngalert/models/alert_rule.go @@ -103,6 +103,17 @@ const ( KeepLastErrState ExecutionErrorState = "KeepLast" ) +type RuleType string + +const ( + RuleTypeAlerting = "alerting" + RuleTypeRecording = "recording" +) + +func (r RuleType) String() string { + return string(r) +} + const ( // Annotations are actually a set of labels, so technically this is the label name of an annotation. DashboardUIDAnnotation = "__dashboardUid__" @@ -344,7 +355,7 @@ func (alertRule *AlertRule) GetLabels(opts ...LabelOption) map[string]string { } func (alertRule *AlertRule) GetEvalCondition() Condition { - if alertRule.IsRecordingRule() { + if alertRule.Type() == RuleTypeRecording { return Condition{ Condition: alertRule.Record.From, Data: alertRule.Data, @@ -512,7 +523,7 @@ func (alertRule *AlertRule) ValidateAlertRule(cfg setting.UnifiedAlertingSetting } var err error - if alertRule.IsRecordingRule() { + if alertRule.Type() == RuleTypeRecording { err = validateRecordingRuleFields(alertRule) } else { err = validateAlertRuleFields(alertRule) @@ -586,8 +597,11 @@ func (alertRule *AlertRule) GetFolderKey() FolderKey { } } -func (alertRule *AlertRule) IsRecordingRule() bool { - return alertRule.Record != nil +func (alertRule *AlertRule) Type() RuleType { + if alertRule.Record != nil { + return RuleTypeRecording + } + return RuleTypeAlerting } // AlertRuleVersion is the model for alert rule versions in unified alerting. diff --git a/pkg/services/ngalert/schedule/alert_rule.go b/pkg/services/ngalert/schedule/alert_rule.go index 4c1a2cf3e00..99757cc222e 100644 --- a/pkg/services/ngalert/schedule/alert_rule.go +++ b/pkg/services/ngalert/schedule/alert_rule.go @@ -62,7 +62,7 @@ func newRuleFactory( stopAppliedHook stopAppliedFunc, ) ruleFactoryFunc { return func(ctx context.Context, rule *ngmodels.AlertRule) Rule { - if rule.IsRecordingRule() { + if rule.Type() == ngmodels.RuleTypeRecording { return newRecordingRule( ctx, maxAttempts, diff --git a/pkg/services/ngalert/schedule/recording_rule.go b/pkg/services/ngalert/schedule/recording_rule.go index 94ff252c6a7..01f4824e778 100644 --- a/pkg/services/ngalert/schedule/recording_rule.go +++ b/pkg/services/ngalert/schedule/recording_rule.go @@ -189,7 +189,7 @@ func (r *recordingRule) tryEvaluation(ctx context.Context, ev *Evaluation, logge return fmt.Errorf("server side expressions pipeline returned an error: %w", err) } - logger.Debug("Alert rule evaluated", "results", result, "duration", evalDur) + logger.Info("Recording rule evaluated", "results", result, "duration", evalDur) span.AddEvent("rule evaluated", trace.WithAttributes( attribute.Int64("results", int64(len(result.Responses))), )) diff --git a/pkg/services/ngalert/store/alert_rule_test.go b/pkg/services/ngalert/store/alert_rule_test.go index a28b2aeef65..c8752e01627 100644 --- a/pkg/services/ngalert/store/alert_rule_test.go +++ b/pkg/services/ngalert/store/alert_rule_test.go @@ -693,7 +693,7 @@ func TestIntegrationInsertAlertRules(t *testing.T) { t.Run("inserted alerting rules should have nil recording rule fields on model", func(t *testing.T) { for _, rule := range dbRules { - if !rule.IsRecordingRule() { + if rule.Type() == models.RuleTypeAlerting { require.Nil(t, rule.Record) } } @@ -701,7 +701,7 @@ func TestIntegrationInsertAlertRules(t *testing.T) { t.Run("inserted recording rules map identical fields when listed", func(t *testing.T) { for _, rule := range dbRules { - if rule.IsRecordingRule() { + if rule.Type() == models.RuleTypeRecording { require.NotNil(t, rule.Record) require.Equal(t, "my_metric", rule.Record.Metric) require.Equal(t, "A", rule.Record.From) @@ -711,7 +711,7 @@ func TestIntegrationInsertAlertRules(t *testing.T) { t.Run("inserted recording rules have empty or default alert-specific settings", func(t *testing.T) { for _, rule := range dbRules { - if rule.IsRecordingRule() { + if rule.Type() == models.RuleTypeRecording { require.Empty(t, rule.Condition) require.Equal(t, models.NoDataState(""), rule.NoDataState) require.Equal(t, models.ExecutionErrorState(""), rule.ExecErrState) diff --git a/public/api-merged.json b/public/api-merged.json index ea090cfaeb3..4dab4ebc0b7 100644 --- a/public/api-merged.json +++ b/public/api-merged.json @@ -12476,7 +12476,7 @@ } }, "type": { - "$ref": "#/definitions/RuleType" + "type": "string" } } }, @@ -19148,7 +19148,7 @@ "type": "string" }, "type": { - "$ref": "#/definitions/RuleType" + "type": "string" } } }, @@ -19259,10 +19259,6 @@ } } }, - "RuleType": { - "type": "string", - "title": "RuleType models the type of a rule." - }, "SNSConfig": { "type": "object", "properties": { @@ -21749,7 +21745,6 @@ } }, "gettableAlerts": { - "description": "GettableAlerts gettable alerts", "type": "array", "items": { "type": "object", @@ -21758,16 +21753,6 @@ }, "gettableGrafanaSilence": { "type": "object", - "required": [ - "comment", - "createdBy", - "endsAt", - "matchers", - "startsAt", - "id", - "status", - "updatedAt" - ], "properties": { "accessControl": { "type": "object", @@ -21780,41 +21765,8 @@ "write": false } }, - "comment": { - "description": "comment", - "type": "string" - }, - "createdBy": { - "description": "created by", - "type": "string" - }, - "endsAt": { - "description": "ends at", - "type": "string", - "format": "date-time" - }, - "id": { - "description": "id", - "type": "string" - }, - "matchers": { - "$ref": "#/definitions/matchers" - }, "metadata": { "$ref": "#/definitions/SilenceMetadata" - }, - "startsAt": { - "description": "starts at", - "type": "string", - "format": "date-time" - }, - "status": { - "$ref": "#/definitions/silenceStatus" - }, - "updatedAt": { - "description": "updated at", - "type": "string", - "format": "date-time" } } }, diff --git a/public/openapi3.json b/public/openapi3.json index 34c05433dd9..615735a2789 100644 --- a/public/openapi3.json +++ b/public/openapi3.json @@ -2845,7 +2845,7 @@ "type": "object" }, "type": { - "$ref": "#/components/schemas/RuleType" + "type": "string" } }, "required": [ @@ -9520,7 +9520,7 @@ "type": "string" }, "type": { - "$ref": "#/components/schemas/RuleType" + "type": "string" } }, "required": [ @@ -9638,10 +9638,6 @@ ], "type": "object" }, - "RuleType": { - "title": "RuleType models the type of a rule.", - "type": "string" - }, "SNSConfig": { "properties": { "api_url": { @@ -12126,7 +12122,6 @@ "type": "object" }, "gettableAlerts": { - "description": "GettableAlerts gettable alerts", "items": { "$ref": "#/components/schemas/gettableAlert" }, @@ -12145,53 +12140,10 @@ }, "type": "object" }, - "comment": { - "description": "comment", - "type": "string" - }, - "createdBy": { - "description": "created by", - "type": "string" - }, - "endsAt": { - "description": "ends at", - "format": "date-time", - "type": "string" - }, - "id": { - "description": "id", - "type": "string" - }, - "matchers": { - "$ref": "#/components/schemas/matchers" - }, "metadata": { "$ref": "#/components/schemas/SilenceMetadata" - }, - "startsAt": { - "description": "starts at", - "format": "date-time", - "type": "string" - }, - "status": { - "$ref": "#/components/schemas/silenceStatus" - }, - "updatedAt": { - "description": "updated at", - "format": "date-time", - "type": "string" } }, - "required": [ - "comment", - "createdBy", - "endsAt", - "matchers", - "startsAt", - "id", - "status", - "updatedAt" - ], "type": "object" }, "gettableGrafanaSilences": {