Merge pull request #178 from github/meiji163/no-hosts-no-err

Return 200 if no hosts found

Author: meiji163

go.mod

@@ -17,8 +17,12 @@ require (
 	vitess.io/vitess v2.1.1+incompatible
 )
 
+require github.com/stretchr/testify v1.3.0
+
 require (
+	github.com/davecgh/go-spew v1.1.0 // indirect
 	github.com/golang/protobuf v1.2.0 // indirect
 	github.com/hashicorp/go-immutable-radix v1.0.0 // indirect
 	github.com/hashicorp/golang-lru v0.5.0 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
 )

go.sum

@@ -10,6 +10,7 @@ github.com/bradfitz/gomemcache v0.0.0-20190913173617-a41fca850d0b h1:L/QXpzIa3pO
 github.com/bradfitz/gomemcache v0.0.0-20190913173617-a41fca850d0b/go.mod h1:H0wQNHz2YrLsuXOZozoeDmnHXkNCRmMW0gwFWDfEZDA=
 github.com/circonus-labs/circonus-gometrics v2.3.1+incompatible/go.mod h1:nmEj6Dob7S7YxXgwXpfOuvO54S+tGdZdw9fuRZt25Ag=
 github.com/circonus-labs/circonusllhist v0.1.3/go.mod h1:kMXHVDlOchFAehlya5ePtbp5jckzBHf4XRpQvBOLI+I=
+github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/go-sql-driver/mysql v1.6.0 h1:BCTh4TKNUYmOmMUcQ3IipzF5prigylS7XXjEkfCHuOE=
 github.com/go-sql-driver/mysql v1.6.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LBy8hT2VhHyBg=
@@ -35,6 +36,7 @@ github.com/pascaldekloe/goe v0.1.0/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144T
 github.com/patrickmn/go-cache v2.1.0+incompatible h1:HRMgzkcYKYpi3C8ajMPV8OFXaaRUnok+kx1WdO15EQc=
 github.com/patrickmn/go-cache v2.1.0+incompatible/go.mod h1:3Qf8kWWT7OJRJbdiICTKqZju1ZixQ/KpMGzzAfe6+WQ=
 github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
+github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/prometheus/client_golang v0.9.2/go.mod h1:OsXs2jCmiKlQ1lTBmv21f2mNfw4xf/QclQDMrYNZzcM=
 github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910/go.mod h1:MbSGuTsp3dbXC40dX6PRTWyKYBIrTGTE9sqQNg2J8bo=
@@ -43,6 +45,7 @@ github.com/prometheus/procfs v0.0.0-20181204211112-1dc9a6cbc91a/go.mod h1:c3At6R
 github.com/rcrowley/go-metrics v0.0.0-20190826022208-cac0b30c2563 h1:dY6ETXrvDG7Sa4vE8ZQG4yqWg6UnOcbqTAahkV813vQ=
 github.com/rcrowley/go-metrics v0.0.0-20190826022208-cac0b30c2563/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/tv42/httpunix v0.0.0-20150427012821-b75d8614f926/go.mod h1:9ESjWnEqriFuLhtthL60Sar/7RFoluCcXsuvEwTV5KM=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=

pkg/base/throttle_metric.go

@@ -17,7 +17,7 @@ type MetricResult interface {
 type MetricResultFunc func() (metricResult MetricResult, threshold float64)
 
 var ThresholdExceededError = errors.New("Threshold exceeded")
-var noHostsError = errors.New("No hosts found")
+var NoHostsError = errors.New("No hosts found")
 var noResultYetError = errors.New("Metric not collected yet")
 var NoSuchMetricError = errors.New("No such metric")
 
@@ -31,7 +31,7 @@ func IsDialTcpError(e error) bool {
 type noHostsMetricResult struct{}
 
 func (metricResult *noHostsMetricResult) Get() (float64, error) {
-	return 0, noHostsError
+	return 0, NoHostsError
 }
 
 var NoHostsMetricResult = &noHostsMetricResult{}

pkg/mysql/mysql_inventory.go

@@ -7,6 +7,7 @@ package mysql
 
 import (
 	"fmt"
+
 	"github.com/github/freno/pkg/base"
 )
 

pkg/throttle/check.go

@@ -7,6 +7,8 @@ import (
 
 	"fmt"
 
+	"errors"
+
 	"github.com/github/freno/pkg/base"
 	metrics "github.com/rcrowley/go-metrics"
 )
@@ -65,6 +67,9 @@ func (check *ThrottlerCheck) checkAppMetricResult(appName string, storeType stri
 	} else if err == base.NoSuchMetricError {
 		// not collected yet, or metric does not exist
 		statusCode = http.StatusNotFound // 404
+	} else if errors.Is(err, base.NoHostsError) {
+		// If no hosts we report 0 for the metric.
+		statusCode = http.StatusOK // 200
 	} else if err != nil {
 		// any error
 		statusCode = http.StatusInternalServerError // 500

pkg/throttle/check_result.go

@@ -1,8 +1,9 @@
 package throttle
 
 import (
-	"github.com/github/freno/pkg/base"
 	"net/http"
+
+	"github.com/github/freno/pkg/base"
 )
 
 // CheckResult is the result for an app inquiring on a metric. It also exports as JSON via the API

pkg/throttle/throttler_test.go

@@ -0,0 +1,55 @@
+package throttle
+
+import (
+	"net/http"
+	"testing"
+
+	"github.com/github/freno/pkg/base"
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_checkAppMetricResult(t *testing.T) {
+	throttler := NewThrottler()
+
+	t.Run("threshold exceeded", func(t *testing.T) {
+		check := NewThrottlerCheck(throttler)
+		metricResultFunc := func() (metricResult base.MetricResult, threshold float64) {
+			return base.NewSimpleMetricResult(10.0), 1.0
+		}
+		result := check.checkAppMetricResult("test-app", "mysql", "test-cluster", metricResultFunc, &CheckFlags{})
+		assert.Equal(t, 10.0, result.Value)
+		assert.EqualError(t, result.Error, base.ThresholdExceededError.Error())
+		assert.Equal(t, http.StatusTooManyRequests, result.StatusCode)
+	})
+	t.Run("not throttled", func(t *testing.T) {
+		check := NewThrottlerCheck(throttler)
+		metricResultFunc := func() (metricResult base.MetricResult, threshold float64) {
+			return base.NewSimpleMetricResult(1.0), 10.0
+		}
+		result := check.checkAppMetricResult("test-app", "mysql", "test-cluster", metricResultFunc, &CheckFlags{})
+		assert.Equal(t, 1.0, result.Value)
+		assert.Nil(t, result.Error)
+		assert.Equal(t, http.StatusOK, result.StatusCode)
+	})
+	t.Run("no hosts", func(t *testing.T) {
+		check := NewThrottlerCheck(throttler)
+		metricResultFunc := func() (metricResult base.MetricResult, threshold float64) {
+			return base.NoHostsMetricResult, 10.0
+		}
+		result := check.checkAppMetricResult("test-app", "mysql", "test-cluster", metricResultFunc, &CheckFlags{})
+		assert.Equal(t, 0.0, result.Value)
+		assert.EqualError(t, result.Error, base.NoHostsError.Error())
+		assert.Equal(t, http.StatusOK, result.StatusCode)
+	})
+	t.Run("low priority denied", func(t *testing.T) {
+		check := NewThrottlerCheck(throttler)
+		throttler.nonLowPriorityAppRequestsThrottled.SetDefault("mysql/test-cluster", true)
+		metricResultFunc := func() (metricResult base.MetricResult, threshold float64) {
+			return base.NewSimpleMetricResult(1.0), 10.0
+		}
+		result := check.checkAppMetricResult("test-app", "mysql", "test-cluster", metricResultFunc, &CheckFlags{LowPriority: true})
+		assert.EqualError(t, result.Error, base.AppDeniedError.Error())
+		assert.Equal(t, http.StatusExpectationFailed, result.StatusCode)
+
+	})
+}