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

Update Alert class to handle nrql validation #2196

Merged
merged 7 commits into from
Jan 10, 2024

Conversation

josephgregoryii
Copy link
Contributor

@josephgregoryii josephgregoryii commented Jan 9, 2024

Summary

This PR stems from handling alert policy error while installing alerts in our UI.

We found that alert conditions that contain a nrql query with TIMESERIES fail an installation after it
hits our database.

This is a supplementary PR #2190 so we can have a more automated process when verifying alert conditions in PRs.

Testing

Our workflows that validate quickstarts will fail during checks.

Testing PR: #2197

  • This PR should fail

Link to validation failure + logs: https://github.com/newrelic/newrelic-quickstarts/actions/runs/7468258398/job/20323336084

Note: as of right now, we have 3 other alerts containing invalid queries, so it is a little noisy in the logs

@josephgregoryii josephgregoryii requested review from a team, d3caf and mickeyryan42 and removed request for a team January 9, 2024 23:34
Copy link
Contributor

@zstix zstix left a comment

Choose a reason for hiding this comment

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

A bunch of nitpicky style-based things. Nothing I would block the PR on, but if you do decide to incorporate those fixes let me know and I'll re-review.

Copy link
Contributor

@mickeyryan42 mickeyryan42 left a comment

Choose a reason for hiding this comment

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

This lgtm!

@aswanson-nr aswanson-nr merged commit 75157dc into release Jan 10, 2024
@aswanson-nr aswanson-nr deleted the joe/update-alert-validation branch January 10, 2024 18:15
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.

4 participants