fix/datadog-scaler-null-last-point (#3954)

Signed-off-by: Tony Lee <[email protected]> Signed-off-by: Tony Lee <[email protected]> Signed-off-by: Zbynek Roubalik <[email protected]> Co-authored-by: Tony Lee <[email protected]> Co-authored-by: Zbynek Roubalik <[email protected]>
kedacore · Dec 8, 2022 · 455eab3 · 455eab3
1 parent b07e7e9
commit 455eab3
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 16 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -92,6 +92,11 @@ issues:
     - path: stan_scaler.go
       linters:
         - dupl
+  # Exclude for datadog_scaler, reason:
+  # Introduce new parameters to fix DataDog API response issue #3906 (PR #3954)
+    - path: datadog_scaler.go
+      linters:
+        - gocyclo
   # Exclude for mongo_scaler and couchdb_scaler, reason:
   # pkg/scalers/couchdb_scaler.go:144: 144-174 lines are duplicate of `pkg/scalers/mongo_scaler.go:155-185` (dupl)
     - path: couchdb_scaler.go
@@ -112,5 +117,3 @@ linters-settings:
       - standard
       - default
       - prefix(github.com/kedacore/keda)
-
-
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -88,6 +88,7 @@ To learn more about active deprecations, we recommend checking [GitHub Discussio
 - **General:** Respect optional parameter inside envs for ScaledJobs ([#3568](https://github.com/kedacore/keda/issues/3568))
 - **General:** Close is called twice on PushScaler's deletion ([#3881](https://github.com/kedacore/keda/issues/3881))
 - **Azure Blob Scaler** Store forgotten logger ([#3811](https://github.com/kedacore/keda/issues/3811))
+- **Datadog Scaler** The last data point of some specific query is always null ([#3906](https://github.com/kedacore/keda/issues/3906))
 - **GCP Stackdriver Scalar:** Update Stackdriver client to handle detecting double and int64 value types ([#3777](https://github.com/kedacore/keda/issues/3777))
 - **New Relic Scaler** Store forgotten logger ([#3945](https://github.com/kedacore/keda/issues/3945))
 - **Prometheus Scaler:** Treat Inf the same as Null result ([#3644](https://github.com/kedacore/keda/issues/3644))

diff --git a/pkg/scalers/datadog_scaler.go b/pkg/scalers/datadog_scaler.go
@@ -23,18 +23,20 @@ type datadogScaler struct {
 }
 
 type datadogMetadata struct {
-	apiKey               string
-	appKey               string
-	datadogSite          string
-	query                string
-	queryValue           float64
-	queryAggegrator      string
-	activationQueryValue float64
-	vType                v2.MetricTargetType
-	metricName           string
-	age                  int
-	useFiller            bool
-	fillValue            float64
+	apiKey                   string
+	appKey                   string
+	datadogSite              string
+	query                    string
+	queryValue               float64
+	queryAggegrator          string
+	activationQueryValue     float64
+	vType                    v2.MetricTargetType
+	metricName               string
+	age                      int
+	timeWindowOffset         int
+	lastAvailablePointOffset int
+	useFiller                bool
+	fillValue                float64
 }
 
 const maxString = "max"
@@ -86,13 +88,43 @@ func parseDatadogMetadata(config *ScalerConfig, logger logr.Logger) (*datadogMet
 		}
 		meta.age = age
 
+		if age < 0 {
+			return nil, fmt.Errorf("age should not be smaller than 0 seconds")
+		}
 		if age < 60 {
 			logger.Info("selecting a window smaller than 60 seconds can cause Datadog not finding a metric value for the query")
 		}
 	} else {
 		meta.age = 90 // Default window 90 seconds
 	}
 
+	if val, ok := config.TriggerMetadata["timeWindowOffset"]; ok {
+		timeWindowOffset, err := strconv.Atoi(val)
+		if err != nil {
+			return nil, fmt.Errorf("timeWindowOffset parsing error %s", err.Error())
+		}
+		if timeWindowOffset < 0 {
+			return nil, fmt.Errorf("timeWindowOffset should not be smaller than 0 seconds")
+		}
+		meta.timeWindowOffset = timeWindowOffset
+	} else {
+		meta.timeWindowOffset = 0 // Default delay 0 seconds
+	}
+
+	if val, ok := config.TriggerMetadata["lastAvailablePointOffset"]; ok {
+		lastAvailablePointOffset, err := strconv.Atoi(val)
+		if err != nil {
+			return nil, fmt.Errorf("lastAvailablePointOffset parsing error %s", err.Error())
+		}
+
+		if lastAvailablePointOffset < 0 {
+			return nil, fmt.Errorf("lastAvailablePointOffset should not be smaller than 0")
+		}
+		meta.lastAvailablePointOffset = lastAvailablePointOffset
+	} else {
+		meta.lastAvailablePointOffset = 0 // Default use the last point
+	}
+
 	if val, ok := config.TriggerMetadata["query"]; ok {
 		_, err := parseDatadogQuery(val)
 
@@ -262,7 +294,9 @@ func (s *datadogScaler) getQueryResult(ctx context.Context) (float64, error) {
 			"site": s.metadata.datadogSite,
 		})
 
-	resp, r, err := s.apiClient.MetricsApi.QueryMetrics(ctx, time.Now().Unix()-int64(s.metadata.age), time.Now().Unix(), s.metadata.query) //nolint:bodyclose
+	timeWindowTo := time.Now().Unix() - int64(s.metadata.timeWindowOffset)
+	timeWindowFrom := timeWindowTo - int64(s.metadata.age)
+	resp, r, err := s.apiClient.MetricsApi.QueryMetrics(ctx, timeWindowFrom, timeWindowTo, s.metadata.query) //nolint:bodyclose
 	if err != nil {
 		return -1, fmt.Errorf("error when retrieving Datadog metrics: %s", err)
 	}
@@ -304,6 +338,18 @@ func (s *datadogScaler) getQueryResult(ctx context.Context) (float64, error) {
 	for i := 0; i < len(series); i++ {
 		points := series[i].GetPointlist()
 		index := len(points) - 1
+		// Find out the last point != nil
+		for j := index; j >= 0; j-- {
+			if len(points[j]) >= 2 && points[j][1] != nil {
+				index = j
+				break
+			}
+		}
+		if index < s.metadata.lastAvailablePointOffset {
+			return 0, fmt.Errorf("index is smaller than the lastAvailablePointOffset")
+		}
+		index -= s.metadata.lastAvailablePointOffset
+
 		if len(points) == 0 || len(points[index]) < 2 || points[index][1] == nil {
 			if !s.metadata.useFiller {
 				return 0, fmt.Errorf("no Datadog metrics returned for the given time window")

diff --git a/pkg/scalers/datadog_scaler_test.go b/pkg/scalers/datadog_scaler_test.go
@@ -66,6 +66,9 @@ var testParseQueries = []datadogQueries{
 
 	// Missing filter
 	{"min:system.cpu.user", false, true},
+
+	// Find out last point with value
+	{"sum:trace.express.request.hits{*}.as_rate()/avg:kubernetes.cpu.requests{*}.rollup(10)", true, false},
 }
 
 func TestDatadogScalerParseQueries(t *testing.T) {
@@ -89,11 +92,15 @@ var testDatadogMetadata = []datadogAuthMetadataTestData{
 	{"", map[string]string{}, map[string]string{}, true},
 
 	// all properly formed
-	{"", map[string]string{"query": "sum:trace.redis.command.hits{env:none,service:redis}.as_count()", "queryValue": "7", "metricUnavailableValue": "1.5", "type": "average", "age": "60"}, map[string]string{"apiKey": "apiKey", "appKey": "appKey", "datadogSite": "datadogSite"}, false},
+	{"", map[string]string{"query": "sum:trace.redis.command.hits{env:none,service:redis}.as_count()", "queryValue": "7", "metricUnavailableValue": "1.5", "type": "average", "age": "60", "timeWindowOffset": "30", "lastAvailablePointOffset": "1"}, map[string]string{"apiKey": "apiKey", "appKey": "appKey", "datadogSite": "datadogSite"}, false},
 	// Multi-query all properly formed
 	{"", map[string]string{"query": "sum:trace.redis.command.hits{env:none,service:redis}.as_count(),sum:trace.redis.command.hits{env:none,service:redis}.as_count()/2", "queryValue": "7", "queryAggregator": "average", "metricUnavailableValue": "1.5", "type": "average", "age": "60"}, map[string]string{"apiKey": "apiKey", "appKey": "appKey", "datadogSite": "datadogSite"}, false},
 	// default age
 	{"", map[string]string{"query": "sum:trace.redis.command.hits{env:none,service:redis}.as_count()", "queryValue": "7", "type": "average"}, map[string]string{"apiKey": "apiKey", "appKey": "appKey", "datadogSite": "datadogSite"}, false},
+	// default timeWindowOffset
+	{"", map[string]string{"query": "sum:trace.redis.command.hits{env:none,service:redis}.as_count()", "queryValue": "7", "metricUnavailableValue": "1.5", "type": "average", "age": "60", "lastAvailablePointOffset": "1"}, map[string]string{"apiKey": "apiKey", "appKey": "appKey", "datadogSite": "datadogSite"}, false},
+	// default lastAvailablePointOffset
+	{"", map[string]string{"query": "sum:trace.redis.command.hits{env:none,service:redis}.as_count()", "queryValue": "7", "metricUnavailableValue": "1.5", "type": "average", "age": "60", "timeWindowOffset": "30"}, map[string]string{"apiKey": "apiKey", "appKey": "appKey", "datadogSite": "datadogSite"}, false},
 	// default type
 	{"", map[string]string{"query": "sum:trace.redis.command.hits{env:none,service:redis}.as_count()", "queryValue": "7", "age": "60"}, map[string]string{"apiKey": "apiKey", "appKey": "appKey", "datadogSite": "datadogSite"}, false},
 	// wrong type