Merge pull request #223 from spiral/ValeryPiashchynski-Fix_warning_and_issues

Fixed the issues and warnings from linters by @ValeryPiashchynski

4 files changed, 71 insertions, 24 deletions

diff --git a/service/metrics/rpc.go b/service/metrics/rpc.go
index ee8ef984..9e7de640 100644
--- a/service/metrics/rpc.go
+++ b/service/metrics/rpc.go
@@ -32,26 +32,26 @@ func (rpc *rpcServer) Add(m *Metric, ok *bool) (err error) {
 		return fmt.Errorf("undefined collector `%s`", m.Name)
 	}
 
-	switch c.(type) {
+	switch c := c.(type) {
 	case prometheus.Gauge:
-		c.(prometheus.Gauge).Add(m.Value)
+		c.Add(m.Value)
 
 	case *prometheus.GaugeVec:
 		if len(m.Labels) == 0 {
 			return fmt.Errorf("required labels for collector `%s`", m.Name)
 		}
 
-		c.(*prometheus.GaugeVec).WithLabelValues(m.Labels...).Add(m.Value)
+		c.WithLabelValues(m.Labels...).Add(m.Value)
 
 	case prometheus.Counter:
-		c.(prometheus.Counter).Add(m.Value)
+		c.Add(m.Value)
 
 	case *prometheus.CounterVec:
 		if len(m.Labels) == 0 {
 			return fmt.Errorf("required labels for collector `%s`", m.Name)
 		}
 
-		c.(*prometheus.CounterVec).WithLabelValues(m.Labels...).Add(m.Value)
+		c.WithLabelValues(m.Labels...).Add(m.Value)
 
 	default:
 		return fmt.Errorf("collector `%s` does not support method `Add`", m.Name)
@@ -74,16 +74,16 @@ func (rpc *rpcServer) Sub(m *Metric, ok *bool) (err error) {
 		return fmt.Errorf("undefined collector `%s`", m.Name)
 	}
 
-	switch c.(type) {
+	switch c := c.(type) {
 	case prometheus.Gauge:
-		c.(prometheus.Gauge).Sub(m.Value)
+		c.Sub(m.Value)
 
 	case *prometheus.GaugeVec:
 		if len(m.Labels) == 0 {
 			return fmt.Errorf("required labels for collector `%s`", m.Name)
 		}
 
-		c.(*prometheus.GaugeVec).WithLabelValues(m.Labels...).Sub(m.Value)
+		c.WithLabelValues(m.Labels...).Sub(m.Value)
 	default:
 		return fmt.Errorf("collector `%s` does not support method `Sub`", m.Name)
 	}
@@ -105,23 +105,23 @@ func (rpc *rpcServer) Observe(m *Metric, ok *bool) (err error) {
 		return fmt.Errorf("undefined collector `%s`", m.Name)
 	}
 
-	switch c.(type) {
+	switch c := c.(type) {
 	case *prometheus.SummaryVec:
 		if len(m.Labels) == 0 {
 			return fmt.Errorf("required labels for collector `%s`", m.Name)
 		}
 
-		c.(*prometheus.SummaryVec).WithLabelValues(m.Labels...).Observe(m.Value)
+		c.WithLabelValues(m.Labels...).Observe(m.Value)
 
 	case prometheus.Histogram:
-		c.(prometheus.Histogram).Observe(m.Value)
+		c.Observe(m.Value)
 
 	case *prometheus.HistogramVec:
 		if len(m.Labels) == 0 {
 			return fmt.Errorf("required labels for collector `%s`", m.Name)
 		}
 
-		c.(*prometheus.HistogramVec).WithLabelValues(m.Labels...).Observe(m.Value)
+		c.WithLabelValues(m.Labels...).Observe(m.Value)
 	default:
 		return fmt.Errorf("collector `%s` does not support method `Observe`", m.Name)
 	}
@@ -143,16 +143,16 @@ func (rpc *rpcServer) Set(m *Metric, ok *bool) (err error) {
 		return fmt.Errorf("undefined collector `%s`", m.Name)
 	}
 
-	switch c.(type) {
+	switch c := c.(type) {
 	case prometheus.Gauge:
-		c.(prometheus.Gauge).Set(m.Value)
+		c.Set(m.Value)
 
 	case *prometheus.GaugeVec:
 		if len(m.Labels) == 0 {
 			return fmt.Errorf("required labels for collector `%s`", m.Name)
 		}
 
-		c.(*prometheus.GaugeVec).WithLabelValues(m.Labels...).Set(m.Value)
+		c.WithLabelValues(m.Labels...).Set(m.Value)
 
 	default:
 		return fmt.Errorf("collector `%s` does not support method `Set`", m.Name)
diff --git a/service/metrics/rpc_test.go b/service/metrics/rpc_test.go
index 136f031c..2468c083 100644
--- a/service/metrics/rpc_test.go
+++ b/service/metrics/rpc_test.go
@@ -42,7 +42,12 @@ func setup(t *testing.T, metric string, portNum string) (*rpc2.Client, service.C
 
 	assert.True(t, s.(*Service).Enabled())
 
-	go func() { c.Serve() }()
+	go func() {
+		err := c.Serve()
+		if err != nil {
+			t.Errorf("error during the Serve: error %v", err)
+		}
+	}()
 	time.Sleep(time.Millisecond * 200)
 
 	client, err := rs.Client()
diff --git a/service/metrics/service.go b/service/metrics/service.go
index b581f041..6fa4da50 100644
--- a/service/metrics/service.go
+++ b/service/metrics/service.go
@@ -4,8 +4,10 @@ package metrics
 
 import (
 	"context"
+	"fmt"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
+	"github.com/sirupsen/logrus"
 	"github.com/spiral/roadrunner/service/rpc"
 	"net/http"
 	"sync"
@@ -17,6 +19,7 @@ const ID = "metrics"
 // Service to manage application metrics using Prometheus.
 type Service struct {
 	cfg        *Config
+	log        *logrus.Logger
 	mu         sync.Mutex
 	http       *http.Server
 	collectors sync.Map
@@ -24,8 +27,9 @@ type Service struct {
 }
 
 // Init service.
-func (s *Service) Init(cfg *Config, r *rpc.Service) (bool, error) {
+func (s *Service) Init(cfg *Config, r *rpc.Service, log *logrus.Logger) (bool, error) {
 	s.cfg = cfg
+	s.log = log
 	s.registry = prometheus.NewRegistry()
 
 	s.registry.MustRegister(prometheus.NewProcessCollector(prometheus.ProcessCollectorOpts{}))
@@ -78,7 +82,12 @@ func (s *Service) Serve() error {
 	)}
 	s.mu.Unlock()
 
-	return s.http.ListenAndServe()
+	err = s.http.ListenAndServe()
+	if err == nil || err == http.ErrServerClosed {
+		return nil
+	}
+
+	return err
 }
 
 // Stop prometheus metrics service.
@@ -88,7 +97,13 @@ func (s *Service) Stop() {
 
 	if s.http != nil {
 		// gracefully stop server
-		go s.http.Shutdown(context.Background())
+		go func() {
+			err := s.http.Shutdown(context.Background())
+			if err != nil {
+				// Function should be Stop() error
+				s.log.Error(fmt.Errorf("error shutting down the metrics server: error %v", err))
+			}
+		}()
 	}
 }
 
diff --git a/service/metrics/service_test.go b/service/metrics/service_test.go
index 0cf6fd95..62e6f6d7 100644
--- a/service/metrics/service_test.go
+++ b/service/metrics/service_test.go
@@ -43,9 +43,16 @@ func get(url string) (string, *http.Response, error) {
 	if err != nil {
 		return "", nil, err
 	}
-	defer r.Body.Close()
 
 	b, err := ioutil.ReadAll(r.Body)
+	if err != nil {
+		return "", nil, err
+	}
+
+	err = r.Body.Close()
+	if err != nil {
+		return "", nil, err
+	}
 	return string(b), r, err
 }
 
@@ -63,7 +70,12 @@ func TestService_Serve(t *testing.T) {
 	s, _ := c.Get(ID)
 	assert.NotNil(t, s)
 
-	go func() { c.Serve() }()
+	go func() {
+		err := c.Serve()
+		if err != nil {
+			t.Errorf("error during the Serve: error %v", err)
+		}
+	}()
 	time.Sleep(time.Millisecond * 100)
 	defer c.Stop()
 
@@ -94,7 +106,12 @@ func Test_ServiceCustomMetric(t *testing.T) {
 
 	assert.NoError(t, s.(*Service).Register(collector))
 
-	go func() { c.Serve() }()
+	go func() {
+		err := c.Serve()
+		if err != nil {
+			t.Errorf("error during the Serve: error %v", err)
+		}
+	}()
 	time.Sleep(time.Millisecond * 100)
 	defer c.Stop()
 
@@ -127,7 +144,12 @@ func Test_ServiceCustomMetricMust(t *testing.T) {
 
 	s.(*Service).MustRegister(collector)
 
-	go func() { c.Serve() }()
+	go func() {
+		err := c.Serve()
+		if err != nil {
+			t.Errorf("error during the Serve: error %v", err)
+		}
+	}()
 	time.Sleep(time.Millisecond * 100)
 	defer c.Stop()
 
@@ -160,7 +182,12 @@ func Test_ConfiguredMetric(t *testing.T) {
 
 	assert.True(t, s.(*Service).Enabled())
 
-	go func() { c.Serve() }()
+	go func() {
+		err := c.Serve()
+		if err != nil {
+			t.Errorf("error during the Serve: error %v", err)
+		}
+	}()
 	time.Sleep(time.Millisecond * 100)
 	defer c.Stop()