From 97fb357dbd9915e690eb32b036ec74cd525b8b7e Mon Sep 17 00:00:00 2001 From: yoyoyoyi <38064424+bamboo12366@users.noreply.github.com> Date: Wed, 1 Jun 2022 16:03:54 +0800 Subject: [PATCH] Prometheus scaler add option to ignore null value (#3073) --- CHANGELOG.md | 1 + pkg/scalers/prometheus_scaler.go | 32 ++++++- pkg/scalers/prometheus_scaler_test.go | 124 +++++++++++++++++--------- 3 files changed, 111 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 414eafd464b..23ee0b0b953 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ To learn more about our roadmap, we recommend reading [this document](ROADMAP.md ### Improvements - **General:** Use more readable timestamps in KEDA Operator logs ([#3066](https://github.com/kedacore/keda/issue/3066)) +- **Prometheus Scaler:** Add ignoreNullValues to return error when prometheus return null in values ([#3065](https://github.com/kedacore/keda/issues/3065)) - **Selenium Grid Scaler:** Edge active sessions not being properly counted ([#2709](https://github.com/kedacore/keda/issues/2709)) - **Selenium Grid Scaler:** Max Sessions implementation issue ([#3061](https://github.com/kedacore/keda/issues/3061)) diff --git a/pkg/scalers/prometheus_scaler.go b/pkg/scalers/prometheus_scaler.go index 8163a6d8833..5236901d374 100644 --- a/pkg/scalers/prometheus_scaler.go +++ b/pkg/scalers/prometheus_scaler.go @@ -10,7 +10,7 @@ import ( "strconv" "time" - v2beta2 "k8s.io/api/autoscaling/v2beta2" + "k8s.io/api/autoscaling/v2beta2" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -29,6 +29,11 @@ const ( promNamespace = "namespace" promCortexScopeOrgID = "cortexOrgID" promCortexHeaderKey = "X-Scope-OrgID" + ignoreNullValues = "ignoreNullValues" +) + +var ( + defaultIgnoreNullValues = true ) type prometheusScaler struct { @@ -46,6 +51,11 @@ type prometheusMetadata struct { namespace string scalerIndex int cortexOrgID string + // sometimes should consider there is an error we can accept + // default value is true/t, to ignore the null value return from prometheus + // change to false/f if can not accept prometheus return null values + // https://github.com/kedacore/keda/issues/3065 + ignoreNullValues bool } type promQueryResult struct { @@ -134,6 +144,16 @@ func parsePrometheusMetadata(config *ScalerConfig) (meta *prometheusMetadata, er meta.cortexOrgID = val } + meta.ignoreNullValues = defaultIgnoreNullValues + if val, ok := config.TriggerMetadata[ignoreNullValues]; ok && val != "" { + ignoreNullValues, err := strconv.ParseBool(val) + if err != nil { + return nil, fmt.Errorf("err incorrect value for ignoreNullValues given: %s, "+ + "please use true or false", val) + } + meta.ignoreNullValues = ignoreNullValues + } + meta.scalerIndex = config.ScalerIndex // parse auth configs from ScalerConfig @@ -223,14 +243,20 @@ func (s *prometheusScaler) ExecutePromQuery(ctx context.Context) (float64, error // allow for zero element or single element result sets if len(result.Data.Result) == 0 { - return 0, nil + if s.metadata.ignoreNullValues { + return 0, nil + } + return -1, fmt.Errorf("prometheus metrics %s target may be lost, the result is empty", s.metadata.metricName) } else if len(result.Data.Result) > 1 { return -1, fmt.Errorf("prometheus query %s returned multiple elements", s.metadata.query) } valueLen := len(result.Data.Result[0].Value) if valueLen == 0 { - return 0, nil + if s.metadata.ignoreNullValues { + return 0, nil + } + return -1, fmt.Errorf("prometheus metrics %s target may be lost, the value list is empty", s.metadata.metricName) } else if valueLen < 2 { return -1, fmt.Errorf("prometheus query %s didn't return enough values", s.metadata.query) } diff --git a/pkg/scalers/prometheus_scaler_test.go b/pkg/scalers/prometheus_scaler_test.go index 65dabd6c60b..ee5dbc9f075 100644 --- a/pkg/scalers/prometheus_scaler_test.go +++ b/pkg/scalers/prometheus_scaler_test.go @@ -27,6 +27,8 @@ var testPromMetadata = []parsePrometheusMetadataTestData{ {map[string]string{"serverAddress": "http://localhost:9090", "metricName": "http_requests_total", "threshold": "100", "query": "up"}, false}, // all properly formed, with namespace {map[string]string{"serverAddress": "http://localhost:9090", "metricName": "http_requests_total", "threshold": "100", "query": "up", "namespace": "foo"}, false}, + // all properly formed, with ignoreNullValues + {map[string]string{"serverAddress": "http://localhost:9090", "metricName": "http_requests_total", "threshold": "100", "query": "up", "ignoreNullValues": "false"}, false}, // missing serverAddress {map[string]string{"serverAddress": "", "metricName": "http_requests_total", "threshold": "100", "query": "up"}, true}, // missing metricName @@ -37,6 +39,8 @@ var testPromMetadata = []parsePrometheusMetadataTestData{ {map[string]string{"serverAddress": "http://localhost:9090", "metricName": "http_requests_total", "threshold": "one", "query": "up"}, true}, // missing query {map[string]string{"serverAddress": "http://localhost:9090", "metricName": "http_requests_total", "threshold": "100", "query": ""}, true}, + // ignoreNullValues with wrong value + {map[string]string{"serverAddress": "http://localhost:9090", "metricName": "http_requests_total", "threshold": "100", "query": "up", "ignoreNullValues": "xxxx"}, true}, } var prometheusMetricIdentifiers = []prometheusMetricIdentifier{ @@ -126,55 +130,86 @@ func TestPrometheusScalerAuthParams(t *testing.T) { } type prometheusQromQueryResultTestData struct { - name string - bodyStr string - responseStatus int - expectedValue float64 - isError bool + name string + bodyStr string + responseStatus int + expectedValue float64 + isError bool + ignoreNullValues bool } var testPromQueryResult = []prometheusQromQueryResultTestData{ { - name: "no results", - bodyStr: `{}`, - responseStatus: http.StatusOK, - expectedValue: 0, - isError: false, + name: "no results", + bodyStr: `{}`, + responseStatus: http.StatusOK, + expectedValue: 0, + isError: false, + ignoreNullValues: true, + }, + { + name: "no values", + bodyStr: `{"data":{"result":[]}}`, + responseStatus: http.StatusOK, + expectedValue: 0, + isError: false, + ignoreNullValues: true, + }, + { + name: "no values but shouldn't ignore", + bodyStr: `{"data":{"result":[]}}`, + responseStatus: http.StatusOK, + expectedValue: -1, + isError: true, + ignoreNullValues: false, + }, + { + name: "value is empty list", + bodyStr: `{"data":{"result":[{"value": []}]}}`, + responseStatus: http.StatusOK, + expectedValue: 0, + isError: false, + ignoreNullValues: true, }, { - name: "no values", - bodyStr: `{"data":{"result":[]}}`, - responseStatus: http.StatusOK, - expectedValue: 0, - isError: false, + name: "value is empty list but shouldn't ignore", + bodyStr: `{"data":{"result":[{"value": []}]}}`, + responseStatus: http.StatusOK, + expectedValue: -1, + isError: true, + ignoreNullValues: false, }, { - name: "valid value", - bodyStr: `{"data":{"result":[{"value": ["1", "2"]}]}}`, - responseStatus: http.StatusOK, - expectedValue: 2, - isError: false, + name: "valid value", + bodyStr: `{"data":{"result":[{"value": ["1", "2"]}]}}`, + responseStatus: http.StatusOK, + expectedValue: 2, + isError: false, + ignoreNullValues: true, }, { - name: "not enough values", - bodyStr: `{"data":{"result":[{"value": ["1"]}]}}`, - responseStatus: http.StatusOK, - expectedValue: -1, - isError: true, + name: "not enough values", + bodyStr: `{"data":{"result":[{"value": ["1"]}]}}`, + responseStatus: http.StatusOK, + expectedValue: -1, + isError: true, + ignoreNullValues: true, }, { - name: "multiple results", - bodyStr: `{"data":{"result":[{},{}]}}`, - responseStatus: http.StatusOK, - expectedValue: -1, - isError: true, + name: "multiple results", + bodyStr: `{"data":{"result":[{},{}]}}`, + responseStatus: http.StatusOK, + expectedValue: -1, + isError: true, + ignoreNullValues: true, }, { - name: "error status response", - bodyStr: `{}`, - responseStatus: http.StatusBadRequest, - expectedValue: -1, - isError: true, + name: "error status response", + bodyStr: `{}`, + responseStatus: http.StatusBadRequest, + expectedValue: -1, + isError: true, + ignoreNullValues: true, }, } @@ -191,7 +226,8 @@ func TestPrometheusScalerExecutePromQuery(t *testing.T) { scaler := prometheusScaler{ metadata: &prometheusMetadata{ - serverAddress: server.URL, + serverAddress: server.URL, + ignoreNullValues: testData.ignoreNullValues, }, httpClient: http.DefaultClient, } @@ -211,11 +247,12 @@ func TestPrometheusScalerExecutePromQuery(t *testing.T) { func TestPrometheusScalerCortexHeader(t *testing.T) { testData := prometheusQromQueryResultTestData{ - name: "no values", - bodyStr: `{"data":{"result":[]}}`, - responseStatus: http.StatusOK, - expectedValue: 0, - isError: false, + name: "no values", + bodyStr: `{"data":{"result":[]}}`, + responseStatus: http.StatusOK, + expectedValue: 0, + isError: false, + ignoreNullValues: true, } cortexOrgValue := "my-org" server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { @@ -229,8 +266,9 @@ func TestPrometheusScalerCortexHeader(t *testing.T) { scaler := prometheusScaler{ metadata: &prometheusMetadata{ - serverAddress: server.URL, - cortexOrgID: cortexOrgValue, + serverAddress: server.URL, + cortexOrgID: cortexOrgValue, + ignoreNullValues: testData.ignoreNullValues, }, httpClient: http.DefaultClient, }