From 2c249e2a922296a53a54db0a5a53f131c8575764 Mon Sep 17 00:00:00 2001 From: Filip Petkovski Date: Wed, 9 Sep 2020 11:46:11 +0200 Subject: [PATCH 1/5] Add New Relic as a metrics provider --- pkg/metrics/providers/factory.go | 2 + pkg/metrics/providers/newrelic.go | 159 +++++++++++++++++++++++++ pkg/metrics/providers/newrelic_test.go | 126 ++++++++++++++++++++ 3 files changed, 287 insertions(+) create mode 100644 pkg/metrics/providers/newrelic.go create mode 100644 pkg/metrics/providers/newrelic_test.go diff --git a/pkg/metrics/providers/factory.go b/pkg/metrics/providers/factory.go index 55e87d912..a1c32650e 100644 --- a/pkg/metrics/providers/factory.go +++ b/pkg/metrics/providers/factory.go @@ -18,6 +18,8 @@ func (factory Factory) Provider( return NewDatadogProvider(metricInterval, provider, credentials) case "cloudwatch": return NewCloudWatchProvider(metricInterval, provider) + case "newrelic": + return NewNewRelicProvider(metricInterval, provider, credentials) default: return NewPrometheusProvider(provider, credentials) } diff --git a/pkg/metrics/providers/newrelic.go b/pkg/metrics/providers/newrelic.go new file mode 100644 index 000000000..daa19977c --- /dev/null +++ b/pkg/metrics/providers/newrelic.go @@ -0,0 +1,159 @@ +package providers + +import ( + "context" + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + "time" + + flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1" +) + +const ( + newrelicInsightsDefaultHost = "https://insights-api.newrelic.com" + + newrelicQueryKeySecretKey = "newrelic_query_key" + newrelicAccountIdSecretKey = "newrelic_account_id" + + newrelicQueryKeyHeaderKey = "X-Query-Key" +) + +// NewRelicProvider executes newrelic queries +type NewRelicProvider struct { + insightsQueryEndpoint string + + timeout time.Duration + queryKey string + fromDelta int64 +} + +type newRelicResponse struct { + Results []struct { + Result *float64 `json:"result"` + } `json:"results"` +} + +// NewNewRelicProvider takes a canary spec, a provider spec and the credentials map, and +// returns a NewRelic client ready to execute queries against the Insights API +func NewNewRelicProvider( + metricInterval string, + provider flaggerv1.MetricTemplateProvider, + credentials map[string][]byte, +) (*NewRelicProvider, error) { + address := provider.Address + if address == "" { + address = newrelicInsightsDefaultHost + } + + accountId, ok := credentials[newrelicAccountIdSecretKey] + if !ok { + return nil, fmt.Errorf("newrelic credentials does not contain " + newrelicAccountIdSecretKey) + } + + queryEndpoint := fmt.Sprintf("%s/v1/accounts/%s/query", address, accountId) + nr := NewRelicProvider{ + timeout: 5 * time.Second, + insightsQueryEndpoint: queryEndpoint, + } + + if b, ok := credentials[newrelicQueryKeySecretKey]; ok { + nr.queryKey = string(b) + } else { + return nil, fmt.Errorf("newrelic credentials does not contain " + newrelicQueryKeySecretKey) + } + + md, err := time.ParseDuration(metricInterval) + if err != nil { + return nil, fmt.Errorf("error parsing metric interval: %w", err) + } + + nr.fromDelta = int64(md.Seconds()) + return &nr, nil +} + +// RunQuery executes the new relic query against the New Relic Insights API +// and returns the the first result +func (p *NewRelicProvider) RunQuery(query string) (float64, error) { + req, err := p.newInsightsRequest(query) + if err != nil { + return 0, err + } + + ctx, cancel := context.WithTimeout(req.Context(), p.timeout) + defer cancel() + r, err := http.DefaultClient.Do(req.WithContext(ctx)) + if err != nil { + return 0, fmt.Errorf("request failed: %w", err) + } + + defer r.Body.Close() + b, err := ioutil.ReadAll(r.Body) + if err != nil { + return 0, fmt.Errorf("error reading body: %w", err) + } + + if r.StatusCode != http.StatusOK { + return 0, fmt.Errorf("error response: %s: %w", string(b), err) + } + + var res newRelicResponse + if err := json.Unmarshal(b, &res); err != nil { + return 0, fmt.Errorf("error unmarshaling result: %w, '%s'", err, string(b)) + } + + if len(res.Results) != 1 { + return 0, fmt.Errorf("invalid response: %s: %w", string(b), ErrNoValuesFound) + } + + if res.Results[0].Result == nil { + return 0, fmt.Errorf("invalid response: %s: %w", string(b), ErrNoValuesFound) + } + + return *res.Results[0].Result, nil +} + +// IsOnline calls the NewRelic's insights API with +// and returns an error if the request is rejected +func (p *NewRelicProvider) IsOnline() (bool, error) { + req, err := p.newInsightsRequest("SELECT * FROM Metric") + if err != nil { + return false, fmt.Errorf("error http.NewRequest: %w", err) + } + + ctx, cancel := context.WithTimeout(req.Context(), p.timeout) + defer cancel() + r, err := http.DefaultClient.Do(req.WithContext(ctx)) + if err != nil { + return false, fmt.Errorf("request failed: %w", err) + } + + defer r.Body.Close() + + b, err := ioutil.ReadAll(r.Body) + if err != nil { + return false, fmt.Errorf("error reading body: %w", err) + } + + if r.StatusCode != http.StatusOK { + return false, fmt.Errorf("error response: %s", string(b)) + } + + return true, nil +} + +func (p *NewRelicProvider) newInsightsRequest(query string) (*http.Request, error) { + req, err := http.NewRequest("GET", p.insightsQueryEndpoint, nil) + if err != nil { + return nil, fmt.Errorf("error http.NewRequest: %w", err) + } + + req.Header.Set(newrelicQueryKeyHeaderKey, p.queryKey) + + q := req.URL.Query() + q.Add("nrql", fmt.Sprintf("%s SINCE %d seconds ago", query, p.fromDelta)) + req.URL.RawQuery = q.Encode() + + return req, nil +} diff --git a/pkg/metrics/providers/newrelic_test.go b/pkg/metrics/providers/newrelic_test.go new file mode 100644 index 000000000..5552665b5 --- /dev/null +++ b/pkg/metrics/providers/newrelic_test.go @@ -0,0 +1,126 @@ +package providers + +import ( + "errors" + "fmt" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + flaggerv1 "github.com/weaveworks/flagger/pkg/apis/flagger/v1beta1" +) + +func TestNewNewRelicProvider(t *testing.T) { + queryKey := "query-key" + accountId := "51312" + cs := map[string][]byte{ + "newrelic_query_key": []byte(queryKey), + "newrelic_account_id": []byte(accountId), + } + + duration := "100s" + secondsDuration, err := time.ParseDuration(duration) + require.NoError(t, err) + + nr, err := NewNewRelicProvider("100s", flaggerv1.MetricTemplateProvider{}, cs) + require.NoError(t, err) + assert.Equal(t, "https://insights-api.newrelic.com/v1/accounts/51312/query", nr.insightsQueryEndpoint) + assert.Equal(t, int64(secondsDuration.Seconds()), nr.fromDelta) + assert.Equal(t, queryKey, nr.queryKey) +} + +func TestNewRelicProvider_RunQuery(t *testing.T) { + queryKey := "query-key" + accountId := "51312" + t.Run("ok", func(t *testing.T) { + q := `SELECT sum(nginx_ingress_controller_requests) / 1 FROM Metric WHERE status = '200'` + eq := `SELECT sum(nginx_ingress_controller_requests) / 1 FROM Metric WHERE status = '200' SINCE 60 seconds ago` + er := 1.11111 + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + aq := r.URL.Query().Get("nrql") + assert.Equal(t, eq, aq) + assert.Equal(t, queryKey, r.Header.Get(newrelicQueryKeyHeaderKey)) + + json := fmt.Sprintf(`{"results":[{"result": %f}]}`, er) + w.Write([]byte(json)) + })) + defer ts.Close() + + nr, err := NewNewRelicProvider("1m", + flaggerv1.MetricTemplateProvider{ + Address: ts.URL, + }, + map[string][]byte{ + "newrelic_query_key": []byte(queryKey), + "newrelic_account_id": []byte(accountId), + }, + ) + require.NoError(t, err) + + f, err := nr.RunQuery(q) + assert.NoError(t, err) + assert.Equal(t, er, f) + }) + + t.Run("no values", func(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + json := fmt.Sprintf(`{"results": []}`) + w.Write([]byte(json)) + })) + defer ts.Close() + + dp, err := NewNewRelicProvider( + "1m", + flaggerv1.MetricTemplateProvider{Address: ts.URL}, + map[string][]byte{ + "newrelic_query_key": []byte(queryKey), + "newrelic_account_id": []byte(accountId)}, + ) + require.NoError(t, err) + _, err = dp.RunQuery("") + require.True(t, errors.Is(err, ErrNoValuesFound)) + }) +} + +func TestNewReelicProvider_IsOnline(t *testing.T) { + for _, c := range []struct { + code int + errExpected bool + }{ + {code: http.StatusOK, errExpected: false}, + {code: http.StatusUnauthorized, errExpected: true}, + } { + t.Run(fmt.Sprintf("%d", c.code), func(t *testing.T) { + queryKey := "query-key" + accountId := "51312" + query := `SELECT * FROM Metric SINCE 60 seconds ago` + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, queryKey, r.Header.Get(newrelicQueryKeyHeaderKey)) + assert.Equal(t, query, r.URL.Query().Get("nrql")) + w.WriteHeader(c.code) + })) + defer ts.Close() + + dp, err := NewNewRelicProvider( + "1m", + flaggerv1.MetricTemplateProvider{Address: ts.URL}, + map[string][]byte{ + "newrelic_query_key": []byte(queryKey), + "newrelic_account_id": []byte(accountId), + }, + ) + require.NoError(t, err) + + _, err = dp.IsOnline() + if c.errExpected { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} From 68e4e1cc68bf6077a884298c611dac762b1a7294 Mon Sep 17 00:00:00 2001 From: Filip Petkovski Date: Wed, 9 Sep 2020 13:51:27 +0200 Subject: [PATCH 2/5] Apply suggestions from code review Co-authored-by: Stefan Prodan --- pkg/metrics/providers/newrelic.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/metrics/providers/newrelic.go b/pkg/metrics/providers/newrelic.go index daa19977c..6480adb83 100644 --- a/pkg/metrics/providers/newrelic.go +++ b/pkg/metrics/providers/newrelic.go @@ -49,7 +49,7 @@ func NewNewRelicProvider( accountId, ok := credentials[newrelicAccountIdSecretKey] if !ok { - return nil, fmt.Errorf("newrelic credentials does not contain " + newrelicAccountIdSecretKey) + return nil, fmt.Errorf("newrelic credentials does not contain the key '%s'", newrelicAccountIdSecretKey) } queryEndpoint := fmt.Sprintf("%s/v1/accounts/%s/query", address, accountId) @@ -61,7 +61,7 @@ func NewNewRelicProvider( if b, ok := credentials[newrelicQueryKeySecretKey]; ok { nr.queryKey = string(b) } else { - return nil, fmt.Errorf("newrelic credentials does not contain " + newrelicQueryKeySecretKey) + return nil, fmt.Errorf("newrelic credentials does not contain the key ''%s", newrelicQueryKeySecretKey) } md, err := time.ParseDuration(metricInterval) From c81e19c48a7007ff4caa356da7b529242e8a87bc Mon Sep 17 00:00:00 2001 From: Filip Petkovski Date: Wed, 9 Sep 2020 18:10:58 +0200 Subject: [PATCH 3/5] Add newrelic as to the provider type enum --- artifacts/flagger/crd.yaml | 1 + charts/flagger/crds/crd.yaml | 1 + kustomize/base/flagger/crd.yaml | 1 + 3 files changed, 3 insertions(+) diff --git a/artifacts/flagger/crd.yaml b/artifacts/flagger/crd.yaml index a56b9e1a7..37311bab1 100644 --- a/artifacts/flagger/crd.yaml +++ b/artifacts/flagger/crd.yaml @@ -796,6 +796,7 @@ spec: - influxdb - datadog - cloudwatch + - newrelic address: description: API address of this provider type: string diff --git a/charts/flagger/crds/crd.yaml b/charts/flagger/crds/crd.yaml index a56b9e1a7..37311bab1 100644 --- a/charts/flagger/crds/crd.yaml +++ b/charts/flagger/crds/crd.yaml @@ -796,6 +796,7 @@ spec: - influxdb - datadog - cloudwatch + - newrelic address: description: API address of this provider type: string diff --git a/kustomize/base/flagger/crd.yaml b/kustomize/base/flagger/crd.yaml index a56b9e1a7..37311bab1 100644 --- a/kustomize/base/flagger/crd.yaml +++ b/kustomize/base/flagger/crd.yaml @@ -796,6 +796,7 @@ spec: - influxdb - datadog - cloudwatch + - newrelic address: description: API address of this provider type: string From 563b1cd88d4a76ec95e5b7d7bd4e6c43726d0e1c Mon Sep 17 00:00:00 2001 From: Filip Petkovski Date: Thu, 10 Sep 2020 08:48:10 +0200 Subject: [PATCH 4/5] Add New Relic provider to the documentation --- docs/gitbook/usage/metrics.md | 53 +++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/docs/gitbook/usage/metrics.md b/docs/gitbook/usage/metrics.md index 8ba9c8e77..0dc7ac21c 100644 --- a/docs/gitbook/usage/metrics.md +++ b/docs/gitbook/usage/metrics.md @@ -314,3 +314,56 @@ Reference the template in the canary analysis: ``` **Note** that Flagger need AWS IAM permission to perform `cloudwatch:GetMetricData` to use this provider. + +### New Relic + +You can create custom metric checks using the New Relic provider. + +Create a secret with your New Relic Insights credentials: + +```yaml +apiVersion: v1 +kind: Secret +metadata: + name: newrelic + namespace: istio-system +data: + newrelic_account_id: your-account-id + newrelic_query_key: your-insights-query-key +``` + +New Relic template example: + +```yaml +apiVersion: flagger.app/v1beta1 +kind: MetricTemplate +metadata: + name: newrelic-error-rate + namespace: istio-system +spec: + provider: + type: newrelic + secretRef: + name: newrelic + query: | + SELECT + filter(sum(nginx_ingress_controller_requests), WHERE status >= '500') / + sum(nginx_ingress_controller_requests) * 100 + FROM Metric + WHERE metricName = 'nginx_ingress_controller_requests' + AND ingress = '{{ ingress }}' AND namespace = '{{ namespace }}' +``` + +Reference the template in the canary analysis: + +```yaml + analysis: + metrics: + - name: "error rate" + templateRef: + name: newrelic-error-rate + namespace: istio-system + thresholdRange: + max: 5 + interval: 1m +``` From 8b3296c065b6f7affbcb403bdf6cba9273065bcc Mon Sep 17 00:00:00 2001 From: Filip Petkovski Date: Thu, 10 Sep 2020 09:19:36 +0200 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: Stefan Prodan --- docs/gitbook/usage/metrics.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/gitbook/usage/metrics.md b/docs/gitbook/usage/metrics.md index 0dc7ac21c..84b68a359 100644 --- a/docs/gitbook/usage/metrics.md +++ b/docs/gitbook/usage/metrics.md @@ -339,7 +339,7 @@ apiVersion: flagger.app/v1beta1 kind: MetricTemplate metadata: name: newrelic-error-rate - namespace: istio-system + namespace: ingress-nginx spec: provider: type: newrelic @@ -362,7 +362,7 @@ Reference the template in the canary analysis: - name: "error rate" templateRef: name: newrelic-error-rate - namespace: istio-system + namespace: ingress-nginx thresholdRange: max: 5 interval: 1m