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

feat(alerts): Add prediction support to NRQL alert conditions #2816

Conversation

founddrama
Copy link
Contributor

Description

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Checklist:

  • My commit message follows conventional commits
  • My code is formatted to Go standards
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes. Go here for instructions on running tests locally.

How to test this change?

For an account that is current added to the feature flag:

  • Create a static NRQL alert condition that uses the prediction field
  • Should also be able to update that condition
  • Should not interfere with the CRUD operations of NRQL alert conditions that do NOT have the prediction field
  • (FWIW: we've been testing this internally since December)

@pranav-new-relic
Copy link
Member

A note for the reviewer: compile is failing on this PR, since it depends on changes in the New Relic Go Client PR referenced in the description.

@pranav-new-relic
Copy link
Member

👀 👀

@codecov-commenter
Copy link

codecov-commenter commented Feb 21, 2025

Codecov Report

Attention: Patch coverage is 47.27273% with 29 lines in your changes missing coverage. Please review.

Project coverage is 31.06%. Comparing base (92361de) to head (60f31d5).
Report is 132 commits behind head on main.

Files with missing lines Patch % Lines
...wrelic/structures_newrelic_nrql_alert_condition.go 47.27% 25 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2816      +/-   ##
==========================================
- Coverage   32.82%   31.06%   -1.76%     
==========================================
  Files          98      102       +4     
  Lines       26884    27602     +718     
==========================================
- Hits         8824     8574     -250     
- Misses      17902    18858     +956     
- Partials      158      170      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

chore: revert test on forks, inconsistent behaviour
@pranav-new-relic pranav-new-relic force-pushed the NR-336017/add_forecast_to_nrql_condition branch from 2ac68ad to 60f31d5 Compare February 21, 2025 11:50
@pranav-new-relic
Copy link
Member

Tested multiple cases of Baseline and Static NRQL Alert Conditions, with and without this attribute. All should be good. The test cases in the NRQL Alert Condition resource are also running fine.

@nr-developer-toolkit nr-developer-toolkit merged commit 79eec78 into newrelic:main Feb 21, 2025
9 checks passed
@founddrama founddrama deleted the NR-336017/add_forecast_to_nrql_condition branch February 21, 2025 16:44
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