Skip to content
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

Alerting: Use new endpoints to fetch single GMA rule on view and edit pages #87625

Merged
merged 23 commits into from
May 30, 2024

Conversation

konrad147
Copy link
Contributor

@konrad147 konrad147 commented May 10, 2024

What is this feature?

In #86845 we added the ability to retrieve the alert rule definition for Grafana-managed rules by a UID.

This PR refactors rule fetching for Grafana-managed rules to use this new endpoint and significantly increase the speed at which we can render the detail view for a single alert rule.

Depends on #88295

@konrad147 konrad147 changed the title Update Ruler response to include rule_group field Alerting: Use new endpoints to fetch single GMA rule on view and edit pages May 10, 2024
@konrad147
Copy link
Contributor Author

/deploy-to-hg

@ephemeral-instances-bot
Copy link

  • Preparing your instance. A comment containing your instance's url will be added to this PR when the instance is ready.
  • Your instance will be ready in ~10 minutes. Follow the workflow progress
  • Slack channel: #proj-ephemeral-hg-instances
  • Building instance with alerting/gma-view-edit-performance oss branch and main enterprise branch. How to choose a branch

@ephemeral-instances-bot
Copy link

@tomratcliffe tomratcliffe added the area/alerting Grafana Alerting label May 14, 2024
@konrad147
Copy link
Contributor Author

/deploy-to-hg

@ephemeral-instances-bot
Copy link

  • Preparing your instance. A comment containing your instance's url will be added to this PR when the instance is ready.
  • Your instance will be ready in ~10 minutes. Follow the workflow progress
  • Slack channel: #proj-ephemeral-hg-instances
  • Building instance with alerting/gma-view-edit-performance oss branch and main enterprise branch. How to choose a branch

@ephemeral-instances-bot
Copy link

@konrad147 konrad147 marked this pull request as ready for review May 16, 2024 10:19
@konrad147 konrad147 requested review from a team as code owners May 16, 2024 10:19
@konrad147 konrad147 requested review from jtheory and removed request for a team May 16, 2024 10:19
@stevesg
Copy link
Contributor

stevesg commented May 23, 2024

This is looking really nice. I discussed with @konrad147 already but repeating here:

  • Because GMA returns the folder path in the "file" field, not the folder UID. There we should:
    • Add a separate folder_uid= filter.
    • And/Or we could make the "file" filter work on folder path, to be consistent with Prometheus
  • We'll pull the backend changes out into a separate PR.

@@ -43,6 +43,7 @@ export const alertingApi = createApi({
'DataSourceSettings',
'GrafanaLabels',
'CombinedAlertRule',
'GrafanaRulerRule',
Copy link
Member

Choose a reason for hiding this comment

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

👌

@gillesdemey gillesdemey added the no-changelog Skip including change in changelog/release notes label May 24, 2024
@gillesdemey
Copy link
Member

/deploy-to-hg

@grafana grafana deleted a comment from ephemeral-instances-bot bot May 24, 2024
@grafana grafana deleted a comment from ephemeral-instances-bot bot May 24, 2024
@gillesdemey
Copy link
Member

/deploy-to-hg

@grafana grafana deleted a comment from ephemeral-instances-bot bot May 24, 2024
@grafana grafana deleted a comment from ephemeral-instances-bot bot May 24, 2024
@ephemeral-instances-bot
Copy link

  • Preparing your instance. A comment containing your instance's url will be added to this PR when the instance is ready.
  • Your instance will be ready in ~10 minutes. Follow the workflow progress
  • Slack channel: #proj-ephemeral-hg-instances
  • Building instance with alerting/gma-view-edit-performance oss branch and main enterprise branch. How to choose a branch

@ephemeral-instances-bot
Copy link

}: {
ruleIdentifier: RuleIdentifier;
}): RequestState<RuleWithLocation> {
const ruleSource = getRulesSourceFromIdentifier(ruleIdentifier);
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need this ruleSource?
We are only using the ruleSource name that it's already included in the ruleIdentifier.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not the original author of this piece but it looks to be to resolve the rulesSourceName (which is a string) to either grafana or an instance of DataSourceInstanceSettings which we don't have in the rule identifier if I'm not mistaken.

(I do have some experiments set aside that pass the entire data source settings with the rule identifier so we don't have to resolve ad-hoc but this seems like a separate piece of work)

Copy link
Member

Choose a reason for hiding this comment

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

What I mean is that if you look to this function this ruleSource is not used for anything. It only appears in a check

Copy link
Member

@soniaAguilarPeiron soniaAguilarPeiron left a comment

Choose a reason for hiding this comment

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

LGTM!

@gillesdemey
Copy link
Member

/deploy-to-hg

@ephemeral-instances-bot
Copy link

  • Preparing your instance. A comment containing your instance's url will be added to this PR when the instance is ready.
  • Your instance will be ready in ~10 minutes. Follow the workflow progress
  • Slack channel: #proj-ephemeral-hg-instances
  • Building instance with alerting/gma-view-edit-performance oss branch and main enterprise branch. How to choose a branch

@ephemeral-instances-bot
Copy link

@gillesdemey gillesdemey merged commit 93870c1 into main May 30, 2024
14 checks passed
@gillesdemey gillesdemey deleted the alerting/gma-view-edit-performance branch May 30, 2024 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/alerting Grafana Alerting area/backend area/frontend no-changelog Skip including change in changelog/release notes
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

6 participants