Revert "Fix Graphite Scaler doesn't properly handle null responses #2944

 (#2945)"

This reverts commit 85e8382.

CHANGELOG.md

@@ -60,7 +60,6 @@ To learn more about our roadmap, we recommend reading [this document](ROADMAP.md
 - **Datadog Scaler:** Validate query to contain `{` to prevent panic on invalid query ([#2625](https://github.com/kedacore/keda/issues/2625))
 - **Datadog Scaler:** Several improvements, including a new optional parameter `metricUnavailableValue` to fill data when no Datadog metric was returned ([#2657](https://github.com/kedacore/keda/issues/2657))
 - **Datadog Scaler:** Rely on Datadog API to validate the query ([2761](https://github.com/kedacore/keda/issues/2761))
-- **Graphite Scaler**  Use the latest non-null datapoint returned by query. ([#2625](https://github.com/kedacore/keda/issues/2944))
 - **Kafka Scaler:** Make "disable" a valid value for tls auth parameter ([#2608](https://github.com/kedacore/keda/issues/2608))
 - **Kafka Scaler:** New `scaleToZeroOnInvalidOffset` to control behavior when partitions have an invalid offset ([#2033](https://github.com/kedacore/keda/issues/2033)[#2612](https://github.com/kedacore/keda/issues/2612))
 - **Metric API Scaler:** Improve error handling on not-ok response ([#2317](https://github.com/kedacore/keda/issues/2317))

pkg/scalers/graphite_scaler.go

@@ -50,7 +50,7 @@ type graphiteMetadata struct {
 type grapQueryResult []struct {
 	Target     string                 `json:"target"`
 	Tags       map[string]interface{} `json:"tags"`
-	Datapoints [][]*float64           `json:"datapoints,omitempty"`
+	Datapoints [][]float64            `json:"datapoints"`
 }
 
 var graphiteLog = logf.Log.WithName("graphite_scaler")
@@ -201,14 +201,10 @@ func (s *graphiteScaler) executeGrapQuery(ctx context.Context) (float64, error)
 		return 0, nil
 	}
 
-	// Return the most recent non-null datapoint
-	for i := len(result[0].Datapoints) - 1; i >= 0; i-- {
-		if datapoint := result[0].Datapoints[i][0]; datapoint != nil {
-			return *datapoint, nil
-		}
-	}
+	latestDatapoint := len(result[0].Datapoints) - 1
+	datapoint := result[0].Datapoints[latestDatapoint][0]
 
-	return -1, fmt.Errorf("no valid non-null response in query %s, try increasing your queryTime or check your query", s.metadata.query)
+	return datapoint, nil
 }
 
 func (s *graphiteScaler) GetMetrics(ctx context.Context, metricName string, metricSelector labels.Selector) ([]external_metrics.ExternalMetricValue, error) {

pkg/scalers/graphite_scaler_test.go

@@ -2,12 +2,8 @@ package scalers
 
 import (
 	"context"
-	"net/http"
-	"net/http/httptest"
 	"strings"
 	"testing"
-
-	"github.com/stretchr/testify/assert"
 )
 
 type parseGraphiteMetadataTestData struct {
@@ -57,59 +53,6 @@ var testGraphiteAuthMetadata = []graphiteAuthMetadataTestData{
 	{map[string]string{"serverAddress": "http://localhost:81", "metricName": "request-count", "threshold": "100", "query": "stats.counters.http.hello-world.request.count.count", "queryTime": "-30Seconds", "authMode": "tls"}, map[string]string{"username": "user"}, true},
 }
 
-type grapQueryResultTestData struct {
-	name           string
-	bodyStr        string
-	responseStatus int
-	expectedValue  float64
-	isError        bool
-}
-
-var testGrapQueryResults = []grapQueryResultTestData{
-	{
-		name:           "no results",
-		bodyStr:        `[{"target":"sumSeries(metric)","tags":{"name":"metric","aggregatedBy":"sum"},"datapoints":[]}]`,
-		responseStatus: http.StatusOK,
-		expectedValue:  0,
-		isError:        false,
-	},
-	{
-		name:           "valid response, latest datapoint is non-null",
-		bodyStr:        `[{"target":"sumSeries(metric)","tags":{"name":"metric","aggregatedBy":"sum"},"datapoints":[[1,10000000]]}]`,
-		responseStatus: http.StatusOK,
-		expectedValue:  1,
-		isError:        false,
-	},
-	{
-		name:           "valid response, latest datapoint is null",
-		bodyStr:        `[{"target":"sumSeries(metric)","tags":{"name":"metric","aggregatedBy":"sum"},"datapoints":[[1,10000000],[null,10000010]]}]`,
-		responseStatus: http.StatusOK,
-		expectedValue:  1,
-		isError:        false,
-	},
-	{
-		name:           "invalid response, all datapoints are null",
-		bodyStr:        `[{"target":"sumSeries(metric)","tags":{"name":"metric","aggregatedBy":"sum"},"datapoints":[[null,10000000],[null,10000010]]}]`,
-		responseStatus: http.StatusOK,
-		expectedValue:  -1,
-		isError:        true,
-	},
-	{
-		name:           "multiple results",
-		bodyStr:        `[{"target":"sumSeries(metric1)","tags":{"name":"metric1","aggregatedBy":"sum"},"datapoints":[[1,1000000]]}, {"target":"sumSeries(metric2)","tags":{"name":"metric2","aggregatedBy":"sum"},"datapoints":[[1,1000000]]}]`,
-		responseStatus: http.StatusOK,
-		expectedValue:  -1,
-		isError:        true,
-	},
-	{
-		name:           "error status response",
-		bodyStr:        `{}`,
-		responseStatus: http.StatusBadRequest,
-		expectedValue:  -1,
-		isError:        true,
-	},
-}
-
 func TestGraphiteParseMetadata(t *testing.T) {
 	for _, testData := range testGrapMetadata {
 		_, err := parseGraphiteMetadata(&ScalerConfig{TriggerMetadata: testData.metadata})
@@ -159,34 +102,3 @@ func TestGraphiteScalerAuthParams(t *testing.T) {
 		}
 	}
 }
-
-func TestGrapScalerExecuteGrapQuery(t *testing.T) {
-	for _, testData := range testGrapQueryResults {
-		t.Run(testData.name, func(t *testing.T) {
-			server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
-				writer.WriteHeader(testData.responseStatus)
-
-				if _, err := writer.Write([]byte(testData.bodyStr)); err != nil {
-					t.Fatal(err)
-				}
-			}))
-
-			scaler := graphiteScaler{
-				metadata: &graphiteMetadata{
-					serverAddress: server.URL,
-				},
-				httpClient: http.DefaultClient,
-			}
-
-			value, err := scaler.executeGrapQuery(context.TODO())
-
-			assert.Equal(t, testData.expectedValue, value)
-
-			if testData.isError {
-				assert.Error(t, err)
-			} else {
-				assert.NoError(t, err)
-			}
-		})
-	}
-}