Skip to content

Commit

Permalink
Prometheus scaler add option to ignore null value (#3073)
Browse files Browse the repository at this point in the history
  • Loading branch information
@bamboo12366
bamboo12366 authored Jun 1, 2022
1 parent 9c2803c commit 97fb357
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 46 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
32 changes: 29 additions & 3 deletions pkg/scalers/prometheus_scaler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -29,6 +29,11 @@ const (
promNamespace = "namespace"
promCortexScopeOrgID = "cortexOrgID"
promCortexHeaderKey = "X-Scope-OrgID"
ignoreNullValues = "ignoreNullValues"
)

var (
defaultIgnoreNullValues = true
)

type prometheusScaler struct {
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down
124 changes: 81 additions & 43 deletions pkg/scalers/prometheus_scaler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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{
Expand Down Expand Up @@ -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,
},
}

Expand All @@ -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,
}
Expand All @@ -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) {
Expand All @@ -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,
}
Expand Down

0 comments on commit 97fb357

Please sign in to comment.