From 951d624dc0c1dda3ee47661150105f355ad62983 Mon Sep 17 00:00:00 2001 From: Friedrich Albert Kyuri Date: Wed, 14 Feb 2024 13:09:34 +0100 Subject: [PATCH 01/10] Metrics API Scaler support different format Signed-off-by: Friedrich Albert Kyuri --- pkg/scalers/metrics_api_scaler.go | 153 +++++++++++++++++++++++-- pkg/scalers/metrics_api_scaler_test.go | 65 +++++------ pkg/util/value_by_path.go | 35 ++++++ pkg/util/value_by_path_test.go | 71 ++++++++++++ 4 files changed, 283 insertions(+), 41 deletions(-) create mode 100644 pkg/util/value_by_path.go create mode 100644 pkg/util/value_by_path_test.go diff --git a/pkg/scalers/metrics_api_scaler.go b/pkg/scalers/metrics_api_scaler.go index d5e0820e3bc..0d69cec1e2e 100644 --- a/pkg/scalers/metrics_api_scaler.go +++ b/pkg/scalers/metrics_api_scaler.go @@ -1,7 +1,10 @@ package scalers import ( + "bufio" + "bytes" "context" + "encoding/xml" "errors" "fmt" "io" @@ -10,6 +13,8 @@ import ( "strconv" "strings" + "gopkg.in/yaml.v3" + "github.com/go-logr/logr" "github.com/tidwall/gjson" v2 "k8s.io/api/autoscaling/v2" @@ -32,6 +37,7 @@ type metricsAPIScalerMetadata struct { targetValue float64 activationTargetValue float64 url string + format APIFormat valueLocation string unsafeSsl bool @@ -62,7 +68,27 @@ type metricsAPIScalerMetadata struct { } const ( - methodValueQuery = "query" + methodValueQuery = "query" + valueLocationWrongErrorMsg = "valueLocation must point to value of type number or a string representing a Quantity got: '%s'" +) + +type APIFormat string + +// Enum for APIFormat: +const ( + PrometheusFormat APIFormat = "prometheus" + JSONFormat APIFormat = "json" + XMLFormat APIFormat = "xml" + YAMLFormat APIFormat = "yaml" +) + +var ( + supportedFormats = []APIFormat{ + PrometheusFormat, + JSONFormat, + XMLFormat, + YAMLFormat, + } ) // NewMetricsAPIScaler creates a new HTTP scaler @@ -137,6 +163,16 @@ func parseMetricsAPIMetadata(config *scalersconfig.ScalerConfig) (*metricsAPISca return nil, fmt.Errorf("no url given in metadata") } + if val, ok := config.TriggerMetadata["format"]; ok { + meta.format = APIFormat(strings.TrimSpace(val)) + if !kedautil.Contains(supportedFormats, meta.format) { + return nil, fmt.Errorf("format %s not supported", meta.format) + } + } else { + // default format is JSON for backward compatibility + meta.format = JSONFormat + } + if val, ok := config.TriggerMetadata["valueLocation"]; ok { meta.valueLocation = val } else { @@ -211,23 +247,126 @@ func parseMetricsAPIMetadata(config *scalersconfig.ScalerConfig) (*metricsAPISca return &meta, nil } -// GetValueFromResponse uses provided valueLocation to access the numeric value in provided body -func GetValueFromResponse(body []byte, valueLocation string) (float64, error) { +// GetValueFromResponse uses provided valueLocation to access the numeric value in provided body using the format specified. +func GetValueFromResponse(body []byte, valueLocation string, format APIFormat) (float64, error) { + switch format { + case PrometheusFormat: + return getValueFromPrometheusResponse(body, valueLocation) + case JSONFormat: + return getValueFromJSONResponse(body, valueLocation) + case XMLFormat: + return getValueFromXMLResponse(body, valueLocation) + case YAMLFormat: + return getValueFromYAMLResponse(body, valueLocation) + } + + return 0, fmt.Errorf("format %s not supported", format) +} + +// getValueFromPrometheusResponse uses provided valueLocation to access the numeric value in provided body +func getValueFromPrometheusResponse(body []byte, valueLocation string) (float64, error) { + scanner := bufio.NewScanner(bytes.NewReader(body)) + for scanner.Scan() { + line := scanner.Text() + fields := strings.Fields(line) + if len(fields) == 0 || strings.HasPrefix(fields[0], "#") { + continue + } + if len(fields) == 2 && strings.HasPrefix(fields[0], valueLocation) { + value, err := strconv.ParseFloat(fields[1], 64) + if err != nil { + return 0, err + } + return value, nil + } + } + + if err := scanner.Err(); err != nil { + return 0, err + } + + return 0, fmt.Errorf("Value %s not found", valueLocation) +} + +// getValueFromJSONResponse uses provided valueLocation to access the numeric value in provided body using GSON +func getValueFromJSONResponse(body []byte, valueLocation string) (float64, error) { r := gjson.GetBytes(body, valueLocation) - errorMsg := "valueLocation must point to value of type number or a string representing a Quantity got: '%s'" if r.Type == gjson.String { v, err := resource.ParseQuantity(r.String()) if err != nil { - return 0, fmt.Errorf(errorMsg, r.String()) + return 0, fmt.Errorf(valueLocationWrongErrorMsg, r.String()) } return v.AsApproximateFloat64(), nil } if r.Type != gjson.Number { - return 0, fmt.Errorf(errorMsg, r.Type.String()) + return 0, fmt.Errorf(valueLocationWrongErrorMsg, r.Type.String()) } return r.Num, nil } +// getValueFromXMLResponse uses provided valueLocation to access the numeric value in provided body +func getValueFromXMLResponse(body []byte, valueLocation string) (float64, error) { + var xmlMap map[string]interface{} + err := xml.Unmarshal(body, &xmlMap) + if err != nil { + return 0, err + } + + path, err := kedautil.GetValueByPath(xmlMap, valueLocation) + if err != nil { + return 0, err + } + + switch v := path.(type) { + case int: + return float64(v), nil + case int64: + return float64(v), nil + case float64: + return v, nil + case string: + r, err := resource.ParseQuantity(v) + if err != nil { + return 0, fmt.Errorf(valueLocationWrongErrorMsg, v) + } + return r.AsApproximateFloat64(), nil + default: + return 0, fmt.Errorf("valueLocation must point to value of type number or a string representing a Quantity got: '%s'", v) + } +} + +// getValueFromYAMLResponse uses provided valueLocation to access the numeric value in provided body +// using generic ketautil.GetValueByPath +func getValueFromYAMLResponse(body []byte, valueLocation string) (float64, error) { + var yamlMap map[string]interface{} + err := yaml.Unmarshal(body, &yamlMap) + if err != nil { + return 0, err + } + + path, err := kedautil.GetValueByPath(yamlMap, valueLocation) + if err != nil { + return 0, err + } + + switch v := path.(type) { + case int: + return float64(v), nil + case int64: + return float64(v), nil + case float64: + return v, nil + case string: + r, err := resource.ParseQuantity(v) + if err != nil { + return 0, fmt.Errorf(valueLocationWrongErrorMsg, v) + } + return r.AsApproximateFloat64(), nil + default: + return 0, fmt.Errorf("valueLocation must point to value of type number or a string representing a Quantity got: '%s'", v) + } +} + func (s *metricsAPIScaler) getMetricValue(ctx context.Context) (float64, error) { request, err := getMetricAPIServerRequest(ctx, s.metadata) if err != nil { @@ -249,7 +388,7 @@ func (s *metricsAPIScaler) getMetricValue(ctx context.Context) (float64, error) if err != nil { return 0, err } - v, err := GetValueFromResponse(b, s.metadata.valueLocation) + v, err := GetValueFromResponse(b, s.metadata.valueLocation, s.metadata.format) if err != nil { return 0, err } diff --git a/pkg/scalers/metrics_api_scaler_test.go b/pkg/scalers/metrics_api_scaler_test.go index 7728d7d2fe0..aa827241134 100644 --- a/pkg/scalers/metrics_api_scaler_test.go +++ b/pkg/scalers/metrics_api_scaler_test.go @@ -123,42 +123,39 @@ func TestMetricsAPIGetMetricSpecForScaling(t *testing.T) { } func TestGetValueFromResponse(t *testing.T) { - d := []byte(`{"components":[{"id": "82328e93e", "tasks": 32, "str": "64", "k":"1k","wrong":"NaN"}],"count":2.43}`) - v, err := GetValueFromResponse(d, "components.0.tasks") - if err != nil { - t.Error("Expected success but got error", err) - } - if v != 32 { - t.Errorf("Expected %d got %f", 32, v) - } - - v, err = GetValueFromResponse(d, "count") - if err != nil { - t.Error("Expected success but got error", err) - } - if v != 2.43 { - t.Errorf("Expected %d got %f", 2, v) - } - - v, err = GetValueFromResponse(d, "components.0.str") - if err != nil { - t.Error("Expected success but got error", err) - } - if v != 64 { - t.Errorf("Expected %d got %f", 64, v) - } - v, err = GetValueFromResponse(d, "components.0.k") - if err != nil { - t.Error("Expected success but got error", err) - } - if v != 1000 { - t.Errorf("Expected %d got %f", 1000, v) - } + testCases := []struct { + name string + input []byte + key string + format APIFormat + expectVal interface{} + expectErr bool + }{ + {name: "integer", input: []byte(`{"components":[{"id": "82328e93e", "tasks": 32, "str": "64", "k":"1k","wrong":"NaN"}],"count":2.43}`), key: "count", format: JSONFormat, expectVal: 2.43}, + {name: "string", input: []byte(`{"components":[{"id": "82328e93e", "tasks": 32, "str": "64", "k":"1k","wrong":"NaN"}],"count":2.43}`), key: "components.0.str", format: JSONFormat, expectVal: 64}, + {name: "{}.[].{}", input: []byte(`{"components":[{"id": "82328e93e", "tasks": 32, "str": "64", "k":"1k","wrong":"NaN"}],"count":2.43}`), key: "components.0.tasks", format: JSONFormat, expectVal: 32}, + {name: "invalid data", input: []byte(`{"components":[{"id": "82328e93e", "tasks": 32, "str": "64", "k":"1k","wrong":"NaN"}],"count":2.43}`), key: "components.0.wrong", format: JSONFormat, expectErr: true}, + {name: "integer", input: []byte(`components: [{id: "82328e93e", tasks: 32, str: "64", k: "1k", wrong: "NaN"}] count: 2.43`), key: "count", format: YAMLFormat, expectVal: 2.43}, + {name: "string", input: []byte(`components: [{id: "82328e93e", tasks: 32, str: "64", k: "1k", wrong: "NaN"}] count: 2.43`), key: "components.0.str", format: YAMLFormat, expectVal: 64}, + {name: "{}.[].{}", input: []byte(`components: [{id: "82328e93e", tasks: 32, str: "64", k: "1k", wrong: "NaN"}] count: 2.43`), key: "components.0.tasks", format: YAMLFormat, expectVal: 32}, + {name: "invalid data", input: []byte(`components: [{id: "82328e93e", tasks: 32, str: "64", k: "1k", wrong: "NaN"}] count: 2.43`), key: "components.0.wrong", format: YAMLFormat, expectErr: true}, + } + + for _, tc := range testCases { + t.Run(string(tc.format)+": "+tc.name, func(t *testing.T) { + v, err := GetValueFromResponse(tc.input, tc.key, tc.format) + + if tc.expectErr && err == nil { + t.Error("Expected error but got success") + } else if !tc.expectErr && err != nil { + t.Error("Expected success but got error:", err) + } - _, err = GetValueFromResponse(d, "components.0.wrong") - if err == nil { - t.Error("Expected error but got success", err) + if !tc.expectErr && v != tc.expectVal { + t.Errorf("Expected %v, got %v", tc.expectVal, v) + } + }) } } diff --git a/pkg/util/value_by_path.go b/pkg/util/value_by_path.go new file mode 100644 index 00000000000..aafb7bed577 --- /dev/null +++ b/pkg/util/value_by_path.go @@ -0,0 +1,35 @@ +package util + +import ( + "fmt" + "strings" +) + +// GetValueByPath retrieves a value from a nested map using a dot-separated path +func GetValueByPath(data map[string]interface{}, path string) (interface{}, error) { + keys := strings.Split(path, ".") + current := data + + for _, key := range keys { + val, ok := current[key] + if !ok { + return nil, fmt.Errorf("key '%s' not found in path '%s'", key, path) + } + + switch v := val.(type) { + case map[interface{}]interface{}: + // Convert map[interface{}]interface{} to map[string]interface{} + current = make(map[string]interface{}) + for k, v := range v { + current[fmt.Sprintf("%v", k)] = v + } + case map[string]interface{}: + current = v + default: + // Reached the final value + return val, nil + } + } + + return nil, fmt.Errorf("path '%s' does not lead to a value", path) +} diff --git a/pkg/util/value_by_path_test.go b/pkg/util/value_by_path_test.go new file mode 100644 index 00000000000..b7b284fe8ae --- /dev/null +++ b/pkg/util/value_by_path_test.go @@ -0,0 +1,71 @@ +package util + +import ( + "reflect" + "testing" +) + +func TestGetValueByPath(t *testing.T) { + tests := []struct { + name string + input map[string]interface{} + path string + expected interface{} + wantErr bool + }{ + { + name: "Valid path - String value", + input: map[string]interface{}{ + "some": map[string]interface{}{ + "nested": map[string]interface{}{ + "key": "value", + }, + }, + }, + path: "some.nested.key", + expected: "value", + wantErr: false, + }, + { + name: "Valid path - Integer value", + input: map[string]interface{}{ + "another": map[string]interface{}{ + "nested": map[string]interface{}{ + "key": 42, + }, + }, + }, + path: "another.nested.key", + expected: 42, + wantErr: false, + }, + { + name: "Invalid path - Key not found", + input: map[string]interface{}{ + "some": map[string]interface{}{ + "nested": map[string]interface{}{ + "key": "value", + }, + }, + }, + path: "nonexistent.path", + expected: nil, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actual, err := GetValueByPath(tt.input, tt.path) + + if (err != nil) != tt.wantErr { + t.Errorf("Unexpected error status. got %v, wantErr %v", err, tt.wantErr) + return + } + + if !reflect.DeepEqual(actual, tt.expected) { + t.Errorf("Mismatched result. got %v, want %v", actual, tt.expected) + } + }) + } +} From 2cba99cfbb46f05ed778bbb67d252f8d9df784c6 Mon Sep 17 00:00:00 2001 From: Friedrich Albert Kyuri Date: Wed, 14 Feb 2024 13:44:46 +0100 Subject: [PATCH 02/10] fix golangci-lint issues Signed-off-by: Friedrich Albert Kyuri --- pkg/scalers/metrics_api_scaler.go | 2 +- pkg/scalers/metrics_api_scaler_test.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/scalers/metrics_api_scaler.go b/pkg/scalers/metrics_api_scaler.go index 0d69cec1e2e..cb5f9b5bc0e 100644 --- a/pkg/scalers/metrics_api_scaler.go +++ b/pkg/scalers/metrics_api_scaler.go @@ -285,7 +285,7 @@ func getValueFromPrometheusResponse(body []byte, valueLocation string) (float64, return 0, err } - return 0, fmt.Errorf("Value %s not found", valueLocation) + return 0, fmt.Errorf("value %s not found", valueLocation) } // getValueFromJSONResponse uses provided valueLocation to access the numeric value in provided body using GSON diff --git a/pkg/scalers/metrics_api_scaler_test.go b/pkg/scalers/metrics_api_scaler_test.go index aa827241134..96ab215ffc8 100644 --- a/pkg/scalers/metrics_api_scaler_test.go +++ b/pkg/scalers/metrics_api_scaler_test.go @@ -123,7 +123,6 @@ func TestMetricsAPIGetMetricSpecForScaling(t *testing.T) { } func TestGetValueFromResponse(t *testing.T) { - testCases := []struct { name string input []byte From fe853ae197ae97120bf72721a4adf85e13fc6466 Mon Sep 17 00:00:00 2001 From: Friedrich Albert Kyuri Date: Wed, 14 Feb 2024 13:51:00 +0100 Subject: [PATCH 03/10] trigger ci and go mod tidy Signed-off-by: Friedrich Albert Kyuri --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 67241b8fe68..b641b9aa34a 100644 --- a/go.mod +++ b/go.mod @@ -348,7 +348,7 @@ require ( gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect + gopkg.in/yaml.v3 v3.0.1 k8s.io/apiextensions-apiserver v0.29.0 // indirect k8s.io/gengo v0.0.0-20230829151522-9cce18d56c01 // indirect k8s.io/kms v0.29.0 // indirect From 6bb1d05d71fb2189213413e3090db44604ea723d Mon Sep 17 00:00:00 2001 From: Friedrich Albert Kyuri Date: Wed, 14 Feb 2024 14:03:24 +0100 Subject: [PATCH 04/10] gci Signed-off-by: Friedrich Albert Kyuri --- pkg/scalers/metrics_api_scaler.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/scalers/metrics_api_scaler.go b/pkg/scalers/metrics_api_scaler.go index cb5f9b5bc0e..2fd5bdda364 100644 --- a/pkg/scalers/metrics_api_scaler.go +++ b/pkg/scalers/metrics_api_scaler.go @@ -13,10 +13,9 @@ import ( "strconv" "strings" - "gopkg.in/yaml.v3" - "github.com/go-logr/logr" "github.com/tidwall/gjson" + "gopkg.in/yaml.v3" v2 "k8s.io/api/autoscaling/v2" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/metrics/pkg/apis/external_metrics" From c59af14d47c99127a76e1cd693976f3cb980ef2c Mon Sep 17 00:00:00 2001 From: Friedrich Albert Kyuri Date: Thu, 15 Feb 2024 11:21:09 +0100 Subject: [PATCH 05/10] fix tests for metrics_api_scaler; add new tests for value_by_path; fix []interface edge case for value_by_path Signed-off-by: Friedrich Albert Kyuri --- pkg/scalers/metrics_api_scaler_test.go | 20 ++++++++------------ pkg/util/value_by_path.go | 12 +++++++++--- pkg/util/value_by_path_test.go | 11 +++++++++++ 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/pkg/scalers/metrics_api_scaler_test.go b/pkg/scalers/metrics_api_scaler_test.go index 96ab215ffc8..6ed47f5d133 100644 --- a/pkg/scalers/metrics_api_scaler_test.go +++ b/pkg/scalers/metrics_api_scaler_test.go @@ -128,32 +128,28 @@ func TestGetValueFromResponse(t *testing.T) { input []byte key string format APIFormat - expectVal interface{} + expectVal float64 expectErr bool }{ {name: "integer", input: []byte(`{"components":[{"id": "82328e93e", "tasks": 32, "str": "64", "k":"1k","wrong":"NaN"}],"count":2.43}`), key: "count", format: JSONFormat, expectVal: 2.43}, {name: "string", input: []byte(`{"components":[{"id": "82328e93e", "tasks": 32, "str": "64", "k":"1k","wrong":"NaN"}],"count":2.43}`), key: "components.0.str", format: JSONFormat, expectVal: 64}, {name: "{}.[].{}", input: []byte(`{"components":[{"id": "82328e93e", "tasks": 32, "str": "64", "k":"1k","wrong":"NaN"}],"count":2.43}`), key: "components.0.tasks", format: JSONFormat, expectVal: 32}, {name: "invalid data", input: []byte(`{"components":[{"id": "82328e93e", "tasks": 32, "str": "64", "k":"1k","wrong":"NaN"}],"count":2.43}`), key: "components.0.wrong", format: JSONFormat, expectErr: true}, - {name: "integer", input: []byte(`components: [{id: "82328e93e", tasks: 32, str: "64", k: "1k", wrong: "NaN"}] count: 2.43`), key: "count", format: YAMLFormat, expectVal: 2.43}, - {name: "string", input: []byte(`components: [{id: "82328e93e", tasks: 32, str: "64", k: "1k", wrong: "NaN"}] count: 2.43`), key: "components.0.str", format: YAMLFormat, expectVal: 64}, - {name: "{}.[].{}", input: []byte(`components: [{id: "82328e93e", tasks: 32, str: "64", k: "1k", wrong: "NaN"}] count: 2.43`), key: "components.0.tasks", format: YAMLFormat, expectVal: 32}, - {name: "invalid data", input: []byte(`components: [{id: "82328e93e", tasks: 32, str: "64", k: "1k", wrong: "NaN"}] count: 2.43`), key: "components.0.wrong", format: YAMLFormat, expectErr: true}, + {name: "integer", input: []byte(`{components: [{id: 82328e93e, tasks: 32, str: '64', k: 1k, wrong: NaN}], count: 2.43}`), key: "count", format: YAMLFormat, expectVal: 2.43}, + {name: "string", input: []byte(`{components: [{id: 82328e93e, tasks: 32, str: '64', k: 1k, wrong: NaN}], count: 2.43}`), key: "components.0.str", format: YAMLFormat, expectVal: 64}, + {name: "{}.[].{}", input: []byte(`{components: [{id: 82328e93e, tasks: 32, str: '64', k: 1k, wrong: NaN}], count: 2.43}`), key: "components.0.tasks", format: YAMLFormat, expectVal: 32}, + {name: "invalid data", input: []byte(`{components: [{id: 82328e93e, tasks: 32, str: '64', k: 1k, wrong: NaN}], count: 2.43}`), key: "components.0.wrong", format: YAMLFormat, expectErr: true}, } for _, tc := range testCases { t.Run(string(tc.format)+": "+tc.name, func(t *testing.T) { v, err := GetValueFromResponse(tc.input, tc.key, tc.format) - if tc.expectErr && err == nil { - t.Error("Expected error but got success") - } else if !tc.expectErr && err != nil { - t.Error("Expected success but got error:", err) + if tc.expectErr { + assert.Error(t, err) } - if !tc.expectErr && v != tc.expectVal { - t.Errorf("Expected %v, got %v", tc.expectVal, v) - } + assert.EqualValues(t, tc.expectVal, v) }) } } diff --git a/pkg/util/value_by_path.go b/pkg/util/value_by_path.go index aafb7bed577..8e21428c2e0 100644 --- a/pkg/util/value_by_path.go +++ b/pkg/util/value_by_path.go @@ -16,15 +16,21 @@ func GetValueByPath(data map[string]interface{}, path string) (interface{}, erro return nil, fmt.Errorf("key '%s' not found in path '%s'", key, path) } - switch v := val.(type) { + switch typedValue := val.(type) { case map[interface{}]interface{}: // Convert map[interface{}]interface{} to map[string]interface{} current = make(map[string]interface{}) - for k, v := range v { + for k, v := range typedValue { + current[fmt.Sprintf("%v", k)] = v + } + case []interface{}: + // Convert map[interface{}]interface{} to map[string]interface{} + current = make(map[string]interface{}) + for k, v := range typedValue { current[fmt.Sprintf("%v", k)] = v } case map[string]interface{}: - current = v + current = typedValue default: // Reached the final value return val, nil diff --git a/pkg/util/value_by_path_test.go b/pkg/util/value_by_path_test.go index b7b284fe8ae..319eb53f6af 100644 --- a/pkg/util/value_by_path_test.go +++ b/pkg/util/value_by_path_test.go @@ -52,6 +52,17 @@ func TestGetValueByPath(t *testing.T) { expected: nil, wantErr: true, }, + { + name: "Interface slice", + input: map[string]interface{}{ + "some": []interface{}{ + 1, 2, 3, + }, + }, + path: "some.0", + expected: 1, + wantErr: false, + }, } for _, tt := range tests { From b8db664a1963f1302a917dd666a2a3ceb51dbf1c Mon Sep 17 00:00:00 2001 From: Friedrich Albert Kyuri Date: Thu, 29 Feb 2024 17:51:20 +0100 Subject: [PATCH 06/10] fix: add format field to e2e tests; edit comment about apiformat Signed-off-by: Friedrich Albert Kyuri --- pkg/scalers/metrics_api_scaler.go | 2 +- tests/scalers/metrics_api/metrics_api_test.go | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/scalers/metrics_api_scaler.go b/pkg/scalers/metrics_api_scaler.go index 2fd5bdda364..7bc34e7a187 100644 --- a/pkg/scalers/metrics_api_scaler.go +++ b/pkg/scalers/metrics_api_scaler.go @@ -73,7 +73,7 @@ const ( type APIFormat string -// Enum for APIFormat: +// Options for APIFormat: const ( PrometheusFormat APIFormat = "prometheus" JSONFormat APIFormat = "json" diff --git a/tests/scalers/metrics_api/metrics_api_test.go b/tests/scalers/metrics_api/metrics_api_test.go index 8781fd4c46c..6c8f973f728 100644 --- a/tests/scalers/metrics_api/metrics_api_test.go +++ b/tests/scalers/metrics_api/metrics_api_test.go @@ -162,9 +162,10 @@ spec: targetValue: "5" activationTargetValue: "20" url: "{{.MetricsServerEndpoint}}" - valueLocation: 'value' - authMode: "basic" - method: "query" + valueLocation: value + format: json + authMode: basic + method: query authenticationRef: name: {{.TriggerAuthName}} ` From 91779901573e7aef4eac2ac4dfa3eb21b95ae362 Mon Sep 17 00:00:00 2001 From: Friedrich Albert Kyuri Date: Thu, 29 Feb 2024 18:00:05 +0100 Subject: [PATCH 07/10] update changelog Signed-off-by: Friedrich Albert Kyuri --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b049fce527..5c7300b9fb0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ Here is an overview of all new **experimental** features: - **General**: Add command-line flag in Adapter to allow override of gRPC Authority Header ([#5449](https://github.com/kedacore/keda/issues/5449)) - **General**: Add OPENTELEMETRY flag in e2e test YAML ([#5375](https://github.com/kedacore/keda/issues/5375)) +- **Metrics API Scaler**: Add support for various formats: json, xml, yaml, prometheus ([#2633](https://github.com/kedacore/keda/issues/2633)) ### Fixes From cb37f8f7da690dcbdf8549533af8c6c379860c35 Mon Sep 17 00:00:00 2001 From: Murr Kyuri <39532283+Friedrich42@users.noreply.github.com> Date: Thu, 21 Mar 2024 14:43:08 +0100 Subject: [PATCH 08/10] Apply suggestions from code review Co-authored-by: Jan Wozniak Signed-off-by: Murr Kyuri <39532283+Friedrich42@users.noreply.github.com> --- pkg/scalers/metrics_api_scaler.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/scalers/metrics_api_scaler.go b/pkg/scalers/metrics_api_scaler.go index 7bc34e7a187..b3a678b3cc5 100644 --- a/pkg/scalers/metrics_api_scaler.go +++ b/pkg/scalers/metrics_api_scaler.go @@ -287,7 +287,7 @@ func getValueFromPrometheusResponse(body []byte, valueLocation string) (float64, return 0, fmt.Errorf("value %s not found", valueLocation) } -// getValueFromJSONResponse uses provided valueLocation to access the numeric value in provided body using GSON +// getValueFromJSONResponse uses provided valueLocation to access the numeric value in provided body using GJSON func getValueFromJSONResponse(body []byte, valueLocation string) (float64, error) { r := gjson.GetBytes(body, valueLocation) if r.Type == gjson.String { @@ -330,7 +330,7 @@ func getValueFromXMLResponse(body []byte, valueLocation string) (float64, error) } return r.AsApproximateFloat64(), nil default: - return 0, fmt.Errorf("valueLocation must point to value of type number or a string representing a Quantity got: '%s'", v) + return 0, fmt.Errorf(valueLocationWrongErrorMsg, v) } } @@ -362,7 +362,7 @@ func getValueFromYAMLResponse(body []byte, valueLocation string) (float64, error } return r.AsApproximateFloat64(), nil default: - return 0, fmt.Errorf("valueLocation must point to value of type number or a string representing a Quantity got: '%s'", v) + return 0, fmt.Errorf(valueLocationWrongErrorMsg, v) } } From b5c15feb0c11e03f6f0a0fc9f60006d0f9129c7e Mon Sep 17 00:00:00 2001 From: Friedrich Albert Kyuri Date: Thu, 21 Mar 2024 20:09:57 +0100 Subject: [PATCH 09/10] decrease duplication by defining a variable for inputs, revert test format field to ensure backwards compatibility Signed-off-by: Friedrich Albert Kyuri --- pkg/scalers/metrics_api_scaler_test.go | 20 +++++++++++-------- tests/scalers/metrics_api/metrics_api_test.go | 7 +++---- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/pkg/scalers/metrics_api_scaler_test.go b/pkg/scalers/metrics_api_scaler_test.go index 6ed47f5d133..cef44a7bfef 100644 --- a/pkg/scalers/metrics_api_scaler_test.go +++ b/pkg/scalers/metrics_api_scaler_test.go @@ -123,6 +123,9 @@ func TestMetricsAPIGetMetricSpecForScaling(t *testing.T) { } func TestGetValueFromResponse(t *testing.T) { + inputJSON := []byte(`{"components":[{"id": "82328e93e", "tasks": 32, "str": "64", "k":"1k","wrong":"NaN"}],"count":2.43}`) + inputYAML := []byte(`{components: [{id: 82328e93e, tasks: 32, str: '64', k: 1k, wrong: NaN}], count: 2.43}`) + testCases := []struct { name string input []byte @@ -131,14 +134,15 @@ func TestGetValueFromResponse(t *testing.T) { expectVal float64 expectErr bool }{ - {name: "integer", input: []byte(`{"components":[{"id": "82328e93e", "tasks": 32, "str": "64", "k":"1k","wrong":"NaN"}],"count":2.43}`), key: "count", format: JSONFormat, expectVal: 2.43}, - {name: "string", input: []byte(`{"components":[{"id": "82328e93e", "tasks": 32, "str": "64", "k":"1k","wrong":"NaN"}],"count":2.43}`), key: "components.0.str", format: JSONFormat, expectVal: 64}, - {name: "{}.[].{}", input: []byte(`{"components":[{"id": "82328e93e", "tasks": 32, "str": "64", "k":"1k","wrong":"NaN"}],"count":2.43}`), key: "components.0.tasks", format: JSONFormat, expectVal: 32}, - {name: "invalid data", input: []byte(`{"components":[{"id": "82328e93e", "tasks": 32, "str": "64", "k":"1k","wrong":"NaN"}],"count":2.43}`), key: "components.0.wrong", format: JSONFormat, expectErr: true}, - {name: "integer", input: []byte(`{components: [{id: 82328e93e, tasks: 32, str: '64', k: 1k, wrong: NaN}], count: 2.43}`), key: "count", format: YAMLFormat, expectVal: 2.43}, - {name: "string", input: []byte(`{components: [{id: 82328e93e, tasks: 32, str: '64', k: 1k, wrong: NaN}], count: 2.43}`), key: "components.0.str", format: YAMLFormat, expectVal: 64}, - {name: "{}.[].{}", input: []byte(`{components: [{id: 82328e93e, tasks: 32, str: '64', k: 1k, wrong: NaN}], count: 2.43}`), key: "components.0.tasks", format: YAMLFormat, expectVal: 32}, - {name: "invalid data", input: []byte(`{components: [{id: 82328e93e, tasks: 32, str: '64', k: 1k, wrong: NaN}], count: 2.43}`), key: "components.0.wrong", format: YAMLFormat, expectErr: true}, + {name: "integer", input: inputJSON, key: "count", format: JSONFormat, expectVal: 2.43}, + {name: "string", input: inputJSON, key: "components.0.str", format: JSONFormat, expectVal: 64}, + {name: "{}.[].{}", input: inputJSON, key: "components.0.tasks", format: JSONFormat, expectVal: 32}, + {name: "invalid data", input: inputJSON, key: "components.0.wrong", format: JSONFormat, expectErr: true}, + + {name: "integer", input: inputYAML, key: "count", format: YAMLFormat, expectVal: 2.43}, + {name: "string", input: inputYAML, key: "components.0.str", format: YAMLFormat, expectVal: 64}, + {name: "{}.[].{}", input: inputYAML, key: "components.0.tasks", format: YAMLFormat, expectVal: 32}, + {name: "invalid data", input: inputYAML, key: "components.0.wrong", format: YAMLFormat, expectErr: true}, } for _, tc := range testCases { diff --git a/tests/scalers/metrics_api/metrics_api_test.go b/tests/scalers/metrics_api/metrics_api_test.go index 6c8f973f728..8781fd4c46c 100644 --- a/tests/scalers/metrics_api/metrics_api_test.go +++ b/tests/scalers/metrics_api/metrics_api_test.go @@ -162,10 +162,9 @@ spec: targetValue: "5" activationTargetValue: "20" url: "{{.MetricsServerEndpoint}}" - valueLocation: value - format: json - authMode: basic - method: query + valueLocation: 'value' + authMode: "basic" + method: "query" authenticationRef: name: {{.TriggerAuthName}} ` From 177fd271ff101da8e5f9b326cc6c383048f1cc6e Mon Sep 17 00:00:00 2001 From: Friedrich Albert Kyuri Date: Fri, 22 Mar 2024 09:53:42 +0100 Subject: [PATCH 10/10] add docstring with examples Signed-off-by: Friedrich Albert Kyuri --- pkg/util/value_by_path.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/pkg/util/value_by_path.go b/pkg/util/value_by_path.go index 8e21428c2e0..83438dc77c8 100644 --- a/pkg/util/value_by_path.go +++ b/pkg/util/value_by_path.go @@ -6,6 +6,24 @@ import ( ) // GetValueByPath retrieves a value from a nested map using a dot-separated path +// It also supports .number syntax to access array elements. +// +// This is a helper function for niche use cases. +// Consider using https://pkg.go.dev/k8s.io/apimachinery@v0.29.3/pkg/apis/meta/v1/unstructured#NestedFieldNoCopy instead +// +// Examples: +// +// data := map[string]interface{}{ +// "a": map[string]interface{}{ +// "b": []interface{}{ +// map[string]interface{}{"c": 1}, +// map[string]interface{}{"c": 2}, +// }, +// }, +// } +// +// GetValueByPath(data, "a.b.0.c") // 1 +// GetValueByPath(data, "not.found") // error func GetValueByPath(data map[string]interface{}, path string) (interface{}, error) { keys := strings.Split(path, ".") current := data