-
Notifications
You must be signed in to change notification settings - Fork 903
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(analysis): Adds timeout property to NewRelic metrics provider. Resolves: #3741 #3742
feat(analysis): Adds timeout property to NewRelic metrics provider. Resolves: #3741 #3742
Conversation
@@ -41,13 +66,30 @@ type NewRelicClient struct { | |||
} | |||
|
|||
// Query executes a NRQL query against the given New Relic account | |||
func (n *NewRelicClient) Query(query string) ([]nrdb.NRDBResult, error) { | |||
results, err := n.Nrdb.Query(n.AccountID, nrdb.NRQL(query)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an equivalent function that supports passing timeout in the nrdb
package but it pulls way more data than the current function which by itself can impact latency.
That's why I switched to the NerdGraph package to send the timeout in the query without incurring in additional data over the wire. I actually slimmed down the query compared to the used in the Query
function to just what we need for this metric provider so the GQL API has to do less work to respond (and even less data transmitted back)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3742 +/- ##
===========================================
+ Coverage 0 83.99% +83.99%
===========================================
Files 0 162 +162
Lines 0 18517 +18517
===========================================
+ Hits 0 15554 +15554
- Misses 0 2101 +2101
- Partials 0 862 +862 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Orlando Valdez <[email protected]>
Signed-off-by: Orlando Valdez <[email protected]>
Signed-off-by: Orlando Valdez <[email protected]>
Signed-off-by: Orlando Valdez <[email protected]>
Signed-off-by: Orlando Valdez <[email protected]>
54cd686
to
1a28ef1
Compare
Signed-off-by: Orlando Valdez <[email protected]>
Signed-off-by: Orlando Valdez <[email protected]>
Published E2E Test Results 4 files 4 suites 3h 24m 59s ⏱️ For more details on these failures, see this check. Results for commit e781f32. ♻️ This comment has been updated with latest results. |
Published Unit Test Results2 263 tests 2 263 ✅ 2m 58s ⏱️ Results for commit e781f32. ♻️ This comment has been updated with latest results. |
Can I have you update with master and fix conflicts, that should re-run the tests now that I have them fixed. |
Signed-off-by: Orlando Valdez <[email protected]>
Signed-off-by: Orlando Valdez <[email protected]>
Signed-off-by: Orlando Valdez <[email protected]>
Noticed something during my last round of local testing. Changed to draft while I figure this out. |
Signed-off-by: Orlando Valdez <[email protected]>
Ready for review now |
Signed-off-by: Orlando Valdez <[email protected]>
Signed-off-by: Orlando Valdez <[email protected]>
|
Resolves: #3741
Adds a timeout property to the New Relic metrics provider spec to be sent to the NerdGraph API for queries that take longer than the default 5 seconds. I bumped up the newrelic-go-client library while working on this.
Checklist:
"fix(controller): Updates such and such. Fixes #1234"
.