Skip to content

Feat: add new alerting rule fields #2120

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

moustafab
Copy link
Contributor

New fields for keep_firing_for and missing_series_evals_to_resolve have been added to the alerting rule group resource.

Added in grafana/grafana#102150

@moustafab moustafab requested review from a team as code owners April 9, 2025 16:33
Copy link
Contributor

github-actions bot commented Apr 9, 2025

In order to lower resource usage and have a faster runtime, PRs will not run Cloud tests automatically.
To do so, a Grafana Labs employee must trigger the cloud acceptance tests workflow manually.

@moustafab
Copy link
Contributor Author

This depends on fixes for grafana grafana/grafana#103736 and maybe grafana/grafana#103735

Copy link
Contributor

@alexander-akhmetov alexander-akhmetov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@moustafab moustafab force-pushed the moustafab/add-new-rule-fields branch 2 times, most recently from 9edb74c to 2d39b0e Compare April 10, 2025 21:07
New fields for `keep_firing_for` and `missing_series_evals_to_resolve` have been added to the alerting rule group resource.

Added in grafana/grafana#102150
@moustafab moustafab force-pushed the moustafab/add-new-rule-fields branch from 2d39b0e to 96f5843 Compare April 14, 2025 14:37
Updates to use grafana/grafana-openapi-client-go#113

Not working with the MuteTimings resource, will need to sort a solution for that
@moustafab
Copy link
Contributor Author

This became a bit more involved, so converting back to draft while I get the solution for the mute timings.

@moustafab moustafab marked this pull request as draft April 21, 2025 16:49
@moustafab moustafab self-assigned this Apr 21, 2025
@@ -519,6 +542,15 @@ func packAlertRule(r *models.ProvisionedAlertRule) (interface{}, error) {
json["record"] = record
}

// FIXME: open api needs to be a reference to the duration
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexander-akhmetov I forgot to about this discovery. The For in the spec is a *Duration and the KeepFiringFor is a Duration. I think effectively they're the same thing but not 100% sure, can you have another look at this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why it's not a *Duration as well, they are similarly defined. I'll check.

Copy link
Contributor

@alexander-akhmetov alexander-akhmetov Apr 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably because the For field is required, it's a pointer: go-swagger/go-swagger#1386 (comment), and KeepFiringFor is not required.

@Duologic Duologic removed the request for review from a team April 23, 2025 09:08
@alexander-akhmetov alexander-akhmetov force-pushed the moustafab/add-new-rule-fields branch 4 times, most recently from 7b05bf3 to af63b06 Compare April 25, 2025 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants