Skip to content

Commit

Permalink
Refactor New Relic scaler (kedacore#6326)
Browse files Browse the repository at this point in the history 
Signed-off-by: rickbrouwer <[email protected]>
  • Loading branch information
@rickbrouwer @robpickerill
rickbrouwer authored and robpickerill committed Nov 30, 2024
1 parent c988e5e commit c7693f4
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 124 deletions.
147 changes: 38 additions & 109 deletions pkg/scalers/newrelic_scaler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ package scalers
import (
"context"
"fmt"
"log"
"strconv"

"github.com/go-logr/logr"
"github.com/newrelic/newrelic-client-go/newrelic"
Expand All @@ -17,31 +15,25 @@ import (
)

const (
account = "account"
queryKeyParamater = "queryKey"
regionParameter = "region"
nrql = "nrql"
threshold = "threshold"
noDataError = "noDataError"
scalerName = "new-relic"
scalerName = "new-relic"
)

type newrelicScaler struct {
metricType v2.MetricTargetType
metadata *newrelicMetadata
metadata newrelicMetadata
nrClient *newrelic.NewRelic
logger logr.Logger
}

type newrelicMetadata struct {
account int
region string
queryKey string
noDataError bool
nrql string
threshold float64
activationThreshold float64
triggerIndex int
Account int `keda:"name=account, order=authParams;triggerMetadata"`
Region string `keda:"name=region, order=authParams;triggerMetadata, default=US"`
QueryKey string `keda:"name=queryKey, order=authParams;triggerMetadata"`
NoDataError bool `keda:"name=noDataError, order=triggerMetadata, default=false"`
NRQL string `keda:"name=nrql, order=triggerMetadata"`
Threshold float64 `keda:"name=threshold, order=triggerMetadata"`
ActivationThreshold float64 `keda:"name=activationThreshold, order=triggerMetadata, default=0"`
TriggerIndex int
}

func NewNewRelicScaler(config *scalersconfig.ScalerConfig) (Scaler, error) {
Expand All @@ -52,128 +44,69 @@ func NewNewRelicScaler(config *scalersconfig.ScalerConfig) (Scaler, error) {

logger := InitializeLogger(config, fmt.Sprintf("%s_scaler", scalerName))

meta, err := parseNewRelicMetadata(config, logger)
meta, err := parseNewRelicMetadata(config)
if err != nil {
return nil, fmt.Errorf("error parsing %s metadata: %w", scalerName, err)
}

nrClient, err := newrelic.New(
newrelic.ConfigPersonalAPIKey(meta.queryKey),
newrelic.ConfigRegion(meta.region))

newrelic.ConfigPersonalAPIKey(meta.QueryKey),
newrelic.ConfigRegion(meta.Region))
if err != nil {
log.Fatal("error initializing client:", err)
return nil, fmt.Errorf("error initializing client: %w", err)
}

logMsg := fmt.Sprintf("Initializing New Relic Scaler (account %d in region %s)", meta.account, meta.region)

logger.Info(logMsg)
logger.Info(fmt.Sprintf("Initializing New Relic Scaler (account %d in region %s)", meta.Account, meta.Region))

return &newrelicScaler{
metricType: metricType,
metadata: meta,
nrClient: nrClient,
logger: logger}, nil
logger: logger,
}, nil
}

func parseNewRelicMetadata(config *scalersconfig.ScalerConfig, logger logr.Logger) (*newrelicMetadata, error) {
func parseNewRelicMetadata(config *scalersconfig.ScalerConfig) (newrelicMetadata, error) {
meta := newrelicMetadata{}
var err error

val, err := GetFromAuthOrMeta(config, account)
if err != nil {
return nil, err
}

t, err := strconv.Atoi(val)
if err != nil {
return nil, fmt.Errorf("error parsing %s: %w", account, err)
}
meta.account = t

if val, ok := config.TriggerMetadata[nrql]; ok && val != "" {
meta.nrql = val
} else {
return nil, fmt.Errorf("no %s given", nrql)
}

queryKey, err := GetFromAuthOrMeta(config, queryKeyParamater)
if err != nil {
return nil, err
}
meta.queryKey = queryKey

region, err := GetFromAuthOrMeta(config, regionParameter)
err := config.TypedConfig(&meta)
if err != nil {
region = "US"
logger.Info("Using default 'US' region")
return meta, fmt.Errorf("error parsing newrelic metadata: %w", err)
}
meta.region = region

if val, ok := config.TriggerMetadata[threshold]; ok && val != "" {
t, err := strconv.ParseFloat(val, 64)
if err != nil {
return nil, fmt.Errorf("error parsing %s", threshold)
}
meta.threshold = t
} else {
if config.AsMetricSource {
meta.threshold = 0
} else {
return nil, fmt.Errorf("missing %s value", threshold)
}
if config.AsMetricSource {
meta.Threshold = 0
}

meta.activationThreshold = 0
if val, ok := config.TriggerMetadata["activationThreshold"]; ok {
activationThreshold, err := strconv.ParseFloat(val, 64)
if err != nil {
return nil, fmt.Errorf("queryValue parsing error %w", err)
}
meta.activationThreshold = activationThreshold
}

// If Query Return an Empty Data , shall we treat it as an error or not
// default is NO error is returned when query result is empty/no data
if val, ok := config.TriggerMetadata[noDataError]; ok {
noDataError, err := strconv.ParseBool(val)
if err != nil {
return nil, fmt.Errorf("noDataError has invalid value")
}
meta.noDataError = noDataError
} else {
meta.noDataError = false
}
meta.triggerIndex = config.TriggerIndex
return &meta, nil
meta.TriggerIndex = config.TriggerIndex
return meta, nil
}

func (s *newrelicScaler) Close(context.Context) error {
return nil
}

func (s *newrelicScaler) executeNewRelicQuery(ctx context.Context) (float64, error) {
nrdbQuery := nrdb.NRQL(s.metadata.nrql)
resp, err := s.nrClient.Nrdb.QueryWithContext(ctx, s.metadata.account, nrdbQuery)
nrdbQuery := nrdb.NRQL(s.metadata.NRQL)
resp, err := s.nrClient.Nrdb.QueryWithContext(ctx, s.metadata.Account, nrdbQuery)
if err != nil {
return 0, fmt.Errorf("error running NRQL %s (%s)", s.metadata.nrql, err.Error())
return 0, fmt.Errorf("error running NRQL %s: %w", s.metadata.NRQL, err)
}
// Check for empty results set, as New Relic lib does not report these as errors

if len(resp.Results) == 0 {
if s.metadata.noDataError {
return 0, fmt.Errorf("query return no results %s", s.metadata.nrql)
if s.metadata.NoDataError {
return 0, fmt.Errorf("query returned no results: %s", s.metadata.NRQL)
}
return 0, nil
}
// Only use the first result from the query, as the query should not be multi row
for _, v := range resp.Results[0] {
val, ok := v.(float64)
if ok {
if val, ok := v.(float64); ok {
return val, nil
}
}
if s.metadata.noDataError {
return 0, fmt.Errorf("query return no results %s", s.metadata.nrql)

if s.metadata.NoDataError {
return 0, fmt.Errorf("query returned no numeric results: %s", s.metadata.NRQL)
}
return 0, nil
}
Expand All @@ -186,21 +119,17 @@ func (s *newrelicScaler) GetMetricsAndActivity(ctx context.Context, metricName s
}

metric := GenerateMetricInMili(metricName, val)

return []external_metrics.ExternalMetricValue{metric}, val > s.metadata.activationThreshold, nil
return []external_metrics.ExternalMetricValue{metric}, val > s.metadata.ActivationThreshold, nil
}

func (s *newrelicScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec {
metricName := kedautil.NormalizeString(scalerName)

externalMetric := &v2.ExternalMetricSource{
Metric: v2.MetricIdentifier{
Name: GenerateMetricNameWithIndex(s.metadata.triggerIndex, metricName),
Name: GenerateMetricNameWithIndex(s.metadata.TriggerIndex, metricName),
},
Target: GetMetricTargetMili(s.metricType, s.metadata.threshold),
}
metricSpec := v2.MetricSpec{
External: externalMetric, Type: externalMetricType,
Target: GetMetricTargetMili(s.metricType, s.metadata.Threshold),
}
metricSpec := v2.MetricSpec{External: externalMetric, Type: externalMetricType}
return []v2.MetricSpec{metricSpec}
}
43 changes: 28 additions & 15 deletions pkg/scalers/newrelic_scaler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"testing"

"github.com/go-logr/logr"
v2 "k8s.io/api/autoscaling/v2"

"github.com/kedacore/keda/v2/pkg/scalers/scalersconfig"
)
Expand All @@ -26,7 +27,7 @@ var testNewRelicMetadata = []parseNewRelicMetadataTestData{
{map[string]string{}, map[string]string{}, true},
// all properly formed
{map[string]string{"account": "0", "threshold": "100", "queryKey": "somekey", "nrql": "SELECT average(cpuUsedCores) as result FROM K8sContainerSample WHERE containerName='coredns'"}, map[string]string{}, false},
// all properly formed
// all properly formed with region and activationThreshold
{map[string]string{"account": "0", "region": "EU", "threshold": "100", "activationThreshold": "20", "queryKey": "somekey", "nrql": "SELECT average(cpuUsedCores) as result FROM K8sContainerSample WHERE containerName='coredns'"}, map[string]string{}, false},
// account passed via auth params
{map[string]string{"region": "EU", "threshold": "100", "queryKey": "somekey", "nrql": "SELECT average(cpuUsedCores) as result FROM K8sContainerSample WHERE containerName='coredns'"}, map[string]string{"account": "0"}, false},
Expand All @@ -48,7 +49,7 @@ var testNewRelicMetadata = []parseNewRelicMetadataTestData{
{map[string]string{"account": "0", "threshold": "100", "queryKey": "somekey"}, map[string]string{}, true},
// noDataError invalid value
{map[string]string{"account": "0", "threshold": "100", "queryKey": "somekey", "noDataError": "invalid", "nrql": "SELECT average(cpuUsedCores) as result FROM K8sContainerSample WHERE containerName='coredns'"}, map[string]string{}, true},
// noDataError valid value
// noDataError valid values
{map[string]string{"account": "0", "threshold": "100", "queryKey": "somekey", "noDataError": "true", "nrql": "SELECT average(cpuUsedCores) as result FROM K8sContainerSample WHERE containerName='coredns'"}, map[string]string{}, false},
{map[string]string{"account": "0", "threshold": "100", "queryKey": "somekey", "noDataError": "false", "nrql": "SELECT average(cpuUsedCores) as result FROM K8sContainerSample WHERE containerName='coredns'"}, map[string]string{}, false},
{map[string]string{"account": "0", "threshold": "100", "queryKey": "somekey", "noDataError": "0", "nrql": "SELECT average(cpuUsedCores) as result FROM K8sContainerSample WHERE containerName='coredns'"}, map[string]string{}, false},
Expand All @@ -61,27 +62,39 @@ var newrelicMetricIdentifiers = []newrelicMetricIdentifier{
}

func TestNewRelicParseMetadata(t *testing.T) {
for _, testData := range testNewRelicMetadata {
_, err := parseNewRelicMetadata(&scalersconfig.ScalerConfig{TriggerMetadata: testData.metadata, AuthParams: testData.authParams}, logr.Discard())
if err != nil && !testData.isError {
fmt.Printf("X: %s", testData.metadata)
t.Error("Expected success but got error", err)
}
if testData.isError && err == nil {
fmt.Printf("X: %s", testData.metadata)
t.Error("Expected error but got success")
}
for i, testData := range testNewRelicMetadata {
t.Run(fmt.Sprintf("test case %d", i), func(t *testing.T) {
_, err := parseNewRelicMetadata(&scalersconfig.ScalerConfig{
TriggerMetadata: testData.metadata,
AuthParams: testData.authParams,
})
if err != nil && !testData.isError {
t.Errorf("Test case %d: Expected success but got error: %v\nMetadata: %v\nAuthParams: %v",
i, err, testData.metadata, testData.authParams)
}
if testData.isError && err == nil {
t.Errorf("Test case %d: Expected error but got success\nMetadata: %v\nAuthParams: %v",
i, testData.metadata, testData.authParams)
}
})
}
}

func TestNewRelicGetMetricSpecForScaling(t *testing.T) {
for _, testData := range newrelicMetricIdentifiers {
meta, err := parseNewRelicMetadata(&scalersconfig.ScalerConfig{TriggerMetadata: testData.metadataTestData.metadata, AuthParams: testData.metadataTestData.authParams, TriggerIndex: testData.triggerIndex}, logr.Discard())
meta, err := parseNewRelicMetadata(&scalersconfig.ScalerConfig{
TriggerMetadata: testData.metadataTestData.metadata,
AuthParams: testData.metadataTestData.authParams,
TriggerIndex: testData.triggerIndex,
})
if err != nil {
t.Fatal("Could not parse metadata:", err)
}
mockNewRelicScaler := newrelicScaler{
metadata: meta,
nrClient: nil,
metadata: meta,
nrClient: nil,
logger: logr.Discard(),
metricType: v2.AverageValueMetricType,
}

metricSpec := mockNewRelicScaler.GetMetricSpecForScaling(context.Background())
Expand Down
8 changes: 8 additions & 0 deletions pkg/scalers/scalersconfig/typed_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,14 @@ func setConfigValueHelper(params Params, valFromConfig string, field reflect.Val
if field.Kind() == reflect.Slice {
return setConfigValueSlice(params, valFromConfig, field)
}
if field.Kind() == reflect.Bool {
boolVal, err := strconv.ParseBool(valFromConfig)
if err != nil {
return fmt.Errorf("unable to parse boolean value %q: %w", valFromConfig, err)
}
field.SetBool(boolVal)
return nil
}
if field.CanInterface() {
ifc := reflect.New(field.Type()).Interface()
if err := json.Unmarshal([]byte(valFromConfig), &ifc); err != nil {
Expand Down

0 comments on commit c7693f4

Please sign in to comment.