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(analysis): Adds timeout property to NewRelic metrics provider. Resolves: #3741 #3742

Merged
merged 18 commits into from
Aug 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/analysis/newrelic.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ spec:
profile: my-newrelic-secret # optional, defaults to 'newrelic'
query: |
FROM Transaction SELECT percentage(count(*), WHERE httpResponseCode != 500) as successRate where appName = '{{ args.application-name }}'
timeout: 10 # NRQL query timeout in seconds. Optional, defaults to 5
```

The `result` evaluated for the condition will always be map or list of maps. The name will follow the pattern of either `function` or `function.field`, e.g. `SELECT average(duration) from Transaction` will yield `average.duration`. In this case the field result cannot be accessed with dot notation and instead should be accessed like `result['average.duration']`. Query results can be renamed using the [NRQL clause `AS`](https://docs.newrelic.com/docs/query-your-data/nrql-new-relic-query-language/get-started/nrql-syntax-clauses-functions#sel-as) as seen above.
Expand Down
12 changes: 12 additions & 0 deletions docs/features/kustomize/rollout_cr_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -4606,6 +4606,10 @@
},
"query": {
"type": "string"
},
"timeout": {
"format": "int64",
"type": "integer"
}
},
"required": [
Expand Down Expand Up @@ -9457,6 +9461,10 @@
},
"query": {
"type": "string"
},
"timeout": {
"format": "int64",
"type": "integer"
}
},
"required": [
Expand Down Expand Up @@ -14321,6 +14329,10 @@
},
"query": {
"type": "string"
},
"timeout": {
"format": "int64",
"type": "integer"
}
},
"required": [
Expand Down
6 changes: 3 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ require (
github.com/juju/ansiterm v1.0.0
github.com/machinebox/graphql v0.2.2
github.com/mitchellh/mapstructure v1.5.0
github.com/newrelic/newrelic-client-go v1.1.0
github.com/newrelic/newrelic-client-go/v2 v2.41.2
github.com/pkg/errors v0.9.1
github.com/prometheus/client_golang v1.18.0
github.com/prometheus/client_model v0.6.1
Expand Down Expand Up @@ -141,7 +141,7 @@ require (
github.com/hashicorp/go-retryablehttp v0.7.1 // indirect
github.com/hashicorp/yamux v0.1.1 // indirect
github.com/huandu/xstrings v1.3.3 // indirect
github.com/imdario/mergo v0.3.13 // indirect
github.com/imdario/mergo v0.3.16 // indirect
github.com/inconshreveable/mousetrap v1.1.0 // indirect
github.com/influxdata/line-protocol v0.0.0-20210922203350-b1ad95c89adf // indirect
github.com/jmespath/go-jmespath v0.4.0 // indirect
Expand Down Expand Up @@ -181,7 +181,7 @@ require (
github.com/stretchr/objx v0.5.2 // indirect
github.com/tomnomnom/linkheader v0.0.0-20180905144013-02ca5825eb80 // indirect
github.com/valyala/bytebufferpool v1.0.0 // indirect
github.com/valyala/fastjson v1.6.3 // indirect
github.com/valyala/fastjson v1.6.4 // indirect
github.com/whilp/git-urls v0.0.0-20191001220047-6db9661140c0 // indirect
github.com/xlab/treeprint v1.2.0 // indirect
go.etcd.io/etcd/api/v3 v3.5.10 // indirect
Expand Down
17 changes: 8 additions & 9 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,8 @@ github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkp
github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto=
github.com/elazarl/goproxy v0.0.0-20170405201442-c4fc26588b6e/go.mod h1:/Zj4wYkgs4iZTTu3o/KG3Itv/qCCa8VVMlb3i9OVuzc=
github.com/elazarl/goproxy v0.0.0-20180725130230-947c36da3153/go.mod h1:/Zj4wYkgs4iZTTu3o/KG3Itv/qCCa8VVMlb3i9OVuzc=
github.com/elazarl/goproxy v0.0.0-20220417044921-416226498f94 h1:VIy7cdK7ufs7ctpTFkXJHm1uP3dJSnCGSPysEICB1so=
github.com/elazarl/goproxy v0.0.0-20220417044921-416226498f94/go.mod h1:Ro8st/ElPeALwNFlcTpWmkr6IoMFfkjXAvTHpevnDsM=
github.com/elazarl/goproxy v0.0.0-20231117061959-7cc037d33fb5 h1:m62nsMU279qRD9PQSWD1l66kmkXzuYcnVJqL4XLeV2M=
github.com/elazarl/goproxy v0.0.0-20231117061959-7cc037d33fb5/go.mod h1:Ro8st/ElPeALwNFlcTpWmkr6IoMFfkjXAvTHpevnDsM=
github.com/emicklei/go-restful v0.0.0-20170410110728-ff4f55a20633/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs=
github.com/emicklei/go-restful v2.9.5+incompatible/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs=
github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxERmMY4rD+g=
Expand Down Expand Up @@ -430,8 +430,8 @@ github.com/huandu/xstrings v1.3.3/go.mod h1:y5/lhBue+AyNmUVz9RLU9xbLR0o4KIIExikq
github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
github.com/imdario/mergo v0.3.5/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
github.com/imdario/mergo v0.3.11/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA=
github.com/imdario/mergo v0.3.13 h1:lFzP57bqS/wsqKssCGmtLAb8A0wKjLGrve2q3PPVcBk=
github.com/imdario/mergo v0.3.13/go.mod h1:4lJ1jqUDcsbIECGy0RUJAXNIhg+6ocWgb1ALK2O4oXg=
github.com/imdario/mergo v0.3.16 h1:wwQJbIsHYGMUyLSPrEq1CT16AhnhNJQ51+4fdHUnCl4=
github.com/imdario/mergo v0.3.16/go.mod h1:WBLT9ZmE3lPoWsEzCh9LPo3TiwVN+ZKEjmz+hD27ysY=
github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
Expand Down Expand Up @@ -556,8 +556,8 @@ github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f h1:KUppIJq7/+
github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f h1:y5//uYreIhSUg3J1GEMiLbxo1LJaP8RfCpH6pymGZus=
github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+o7JKHSa8/e818NopupXU1YMK5fe1lsApnBw=
github.com/newrelic/newrelic-client-go v1.1.0 h1:aflNjzQ21c+2GwBVh+UbAf9lznkRfCcVABoc5UM4IXw=
github.com/newrelic/newrelic-client-go v1.1.0/go.mod h1:RYMXt7hgYw7nzuXIGd2BH0F1AivgWw7WrBhNBQZEB4k=
github.com/newrelic/newrelic-client-go/v2 v2.41.2 h1:5YkFRSU19a9CoOkF8qgWoNRXVbL6qh0nZYaWbQjFppA=
github.com/newrelic/newrelic-client-go/v2 v2.41.2/go.mod h1:pDFY24/6iIMEbPIdowTRrRn9YYwkXc3j+B+XpTb4oF4=
github.com/nlopes/slack v0.5.0/go.mod h1:jVI4BBK3lSktibKahxBF74txcK2vyvkza1z/+rRnVAM=
github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A=
github.com/oapi-codegen/runtime v1.0.0 h1:P4rqFX5fMFWqRzY9M/3YF9+aPSPPB06IzP2P7oOxrWo=
Expand Down Expand Up @@ -696,8 +696,8 @@ github.com/tomnomnom/linkheader v0.0.0-20180905144013-02ca5825eb80 h1:nrZ3ySNYwJ
github.com/tomnomnom/linkheader v0.0.0-20180905144013-02ca5825eb80/go.mod h1:iFyPdL66DjUD96XmzVL3ZntbzcflLnznH0fr99w5VqE=
github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw=
github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc=
github.com/valyala/fastjson v1.6.3 h1:tAKFnnwmeMGPbwJ7IwxcTPCNr3uIzoIj3/Fh90ra4xc=
github.com/valyala/fastjson v1.6.3/go.mod h1:CLCAqky6SMuOcxStkYQvblddUtoRxhYMGLrsQns1aXY=
github.com/valyala/fastjson v1.6.4 h1:uAUNq9Z6ymTgGhcm0UynUAB6tlbakBrz6CQFax3BXVQ=
github.com/valyala/fastjson v1.6.4/go.mod h1:CLCAqky6SMuOcxStkYQvblddUtoRxhYMGLrsQns1aXY=
github.com/valyala/fasttemplate v1.2.2 h1:lxLXG0uE3Qnshl9QyaK6XJxMXlQZELvChBOCmQD0Loo=
github.com/valyala/fasttemplate v1.2.2/go.mod h1:KHLXt3tVN2HBp8eijSv/kGJopbvo7S+qRAEEKiv+SiQ=
github.com/whilp/git-urls v0.0.0-20191001220047-6db9661140c0 h1:qqllXPzXh+So+mmANlX/gCJrgo+1kQyshMoQ+NASzm0=
Expand Down Expand Up @@ -1177,7 +1177,6 @@ gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.0-20200605160147-a5ece683394c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.0/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
Expand Down
3 changes: 3 additions & 0 deletions manifests/crds/analysis-run-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3015,6 +3015,9 @@ spec:
type: string
query:
type: string
timeout:
format: int64
type: integer
required:
- query
type: object
Expand Down
3 changes: 3 additions & 0 deletions manifests/crds/analysis-template-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3011,6 +3011,9 @@ spec:
type: string
query:
type: string
timeout:
format: int64
type: integer
required:
- query
type: object
Expand Down
3 changes: 3 additions & 0 deletions manifests/crds/cluster-analysis-template-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3011,6 +3011,9 @@ spec:
type: string
query:
type: string
timeout:
format: int64
type: integer
required:
- query
type: object
Expand Down
9 changes: 9 additions & 0 deletions manifests/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3016,6 +3016,9 @@ spec:
type: string
query:
type: string
timeout:
format: int64
type: integer
required:
- query
type: object
Expand Down Expand Up @@ -6314,6 +6317,9 @@ spec:
type: string
query:
type: string
timeout:
format: int64
type: integer
required:
- query
type: object
Expand Down Expand Up @@ -9490,6 +9496,9 @@ spec:
type: string
query:
type: string
timeout:
format: int64
type: integer
required:
- query
type: object
Expand Down
53 changes: 51 additions & 2 deletions metricproviders/newrelic/mock_test.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,64 @@
package newrelic

import "github.com/newrelic/newrelic-client-go/pkg/nrdb"
import (
"reflect"

"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
"github.com/newrelic/newrelic-client-go/v2/pkg/nrdb"
)

type mockAPI struct {
response []nrdb.NRDBResult
err error
}

func (m *mockAPI) Query(query string) ([]nrdb.NRDBResult, error) {
func (m *mockAPI) Query(metric v1alpha1.Metric) ([]nrdb.NRDBResult, error) {
if m.err != nil {
return nil, m.err
}
return m.response, nil
}

type mockNerdGraphClient struct {
response []nrdb.NRDBResult
lastArgs map[string]interface{}
err error
}

func (m *mockNerdGraphClient) QueryWithResponse(query string, variables map[string]interface{}, respBody interface{}) error {
m.lastArgs = variables

if m.err != nil {
return m.err
}

r := gqlNrglQueryResponse{}
r.Actor.Account.NRQL.Results = m.response

rVal := reflect.ValueOf(r)
reflect.ValueOf(respBody).Elem().Set(rVal)

return nil
}

// Response sets the response the mock client will return
func (m *mockNerdGraphClient) Response(response []nrdb.NRDBResult) {
m.response = response
}

// LastArgs returns the variables used when calling the NerdGraph API
func (m *mockNerdGraphClient) LastArgs() map[string]any {
return m.lastArgs
}

// Err sets the error to be returned when calling the NerdGraph API
func (m *mockNerdGraphClient) Err(err error) {
m.err = err
}

// Clear removes all configured mock behavior
func (m *mockNerdGraphClient) Clear() {
m.err = nil
m.response = nil
m.lastArgs = nil
}
72 changes: 61 additions & 11 deletions metricproviders/newrelic/newrelic.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import (
"fmt"
"strconv"

"github.com/newrelic/newrelic-client-go/newrelic"
"github.com/newrelic/newrelic-client-go/pkg/nrdb"
"github.com/newrelic/newrelic-client-go/v2/newrelic"
"github.com/newrelic/newrelic-client-go/v2/pkg/nrdb"
log "github.com/sirupsen/logrus"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
Expand All @@ -27,27 +27,77 @@ const (
DefaultNewRelicProfileSecretName = "newrelic"
repoURL = "https://github.com/argoproj/argo-rollouts"
resolvedNewRelicQuery = "ResolvedNewRelicQuery"
defaultNrqlTimeout = 5
)

var (
ErrNegativeTimeout = errors.New("timeout value needs to be a positive value")
)

type gqlNrglQueryResponse struct {
Actor struct {
Account struct {
NRQL nrdb.NRDBResultContainer
}
}
}

const gqlNrqlQuery = `query (
$query: Nrql!,
$accountId: Int!,
$timeout: Seconds!
)
{
actor {
account(id: $accountId) {
nrql(query: $query, timeout: $timeout) {
results
}
}
}
}
`

var userAgent = fmt.Sprintf("argo-rollouts/%s (%s)", version.GetVersion(), repoURL)

type NewRelicClientAPI interface {
Query(query string) ([]nrdb.NRDBResult, error)
Query(metric v1alpha1.Metric) ([]nrdb.NRDBResult, error)
}

type nerdGraphClient interface {
QueryWithResponse(query string, variables map[string]interface{}, respBody interface{}) error
}

type NewRelicClient struct {
*newrelic.NewRelic
AccountID int
NerdGraphClient nerdGraphClient
AccountID int
}

// 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))
Copy link
Contributor Author

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)

if err != nil {
func (n *NewRelicClient) Query(metric v1alpha1.Metric) ([]nrdb.NRDBResult, error) {
var timeout int64 = defaultNrqlTimeout
respBody := gqlNrglQueryResponse{}

if metric.Provider.NewRelic.Timeout != nil {
timeout = *metric.Provider.NewRelic.Timeout
}

if timeout < 0 {
return nil, ErrNegativeTimeout
}

args := map[string]any{
"accountId": n.AccountID,
"query": metric.Provider.NewRelic.Query,
"timeout": timeout,
}

if err := n.NerdGraphClient.QueryWithResponse(gqlNrqlQuery, args, &respBody); err != nil {
return nil, err
}

// TODO(jwelch) return metadata from NRDBResultContainer to include on the measurement
return results.Results, nil
return respBody.Actor.Account.NRQL.Results, nil
}

type Provider struct {
Expand All @@ -62,7 +112,7 @@ func (p *Provider) Run(run *v1alpha1.AnalysisRun, metric v1alpha1.Metric) v1alph
StartedAt: &startTime,
}

results, err := p.api.Query(metric.Provider.NewRelic.Query)
results, err := p.api.Query(metric)
if err != nil {
return metricutil.MarkMeasurementError(newMeasurement, err)
}
Expand Down Expand Up @@ -191,7 +241,7 @@ func NewNewRelicAPIClient(metric v1alpha1.Metric, kubeclientset kubernetes.Inter
if err != nil {
return nil, fmt.Errorf("could not parse account ID: %w", err)
}
return &NewRelicClient{NewRelic: nrClient, AccountID: accID}, nil
return &NewRelicClient{NerdGraphClient: &nrClient.NerdGraph, AccountID: accID}, nil
} else {
return nil, errors.New("account ID or personal API key not found")
}
Expand Down
Loading
Loading