Merge pull request #620 from mihaitodor/prometheus-push-gateway-no-pu…

…sh-interval

Allow Prometheus PushGateway to work if PushInterval not set

lib/metrics/prometheus.go

@@ -148,29 +148,31 @@ func NewPrometheus(config Config, opts ...func(Type)) (Type, error) {
 		return nil, fmt.Errorf("failed to init path mapping: %v", err)
 	}
 
-	if len(p.config.PushURL) > 0 && len(p.config.PushInterval) > 0 {
+	if len(p.config.PushURL) > 0 {
 		p.pusher = push.New(p.config.PushURL, p.config.PushJobName).Gatherer(prometheus.DefaultGatherer)
 
 		if len(p.config.PushBasicAuth.Username) > 0 && len(p.config.PushBasicAuth.Password) > 0 {
 			p.pusher = p.pusher.BasicAuth(p.config.PushBasicAuth.Username, p.config.PushBasicAuth.Password)
 		}
 
-		interval, err := time.ParseDuration(p.config.PushInterval)
-		if err != nil {
-			return nil, fmt.Errorf("failed to parse push interval: %v", err)
-		}
-		go func() {
-			for {
-				select {
-				case <-p.closedChan:
-					return
-				case <-time.After(interval):
-					if err = p.pusher.Push(); err != nil {
-						p.log.Errorf("Failed to push metrics: %v\n", err)
+		if len(p.config.PushInterval) > 0 {
+			interval, err := time.ParseDuration(p.config.PushInterval)
+			if err != nil {
+				return nil, fmt.Errorf("failed to parse push interval: %v", err)
+			}
+			go func() {
+				for {
+					select {
+					case <-p.closedChan:
+						return
+					case <-time.After(interval):
+						if err = p.pusher.Push(); err != nil {
+							p.log.Errorf("Failed to push metrics: %v\n", err)
+						}
 					}
 				}
-			}
-		}()
+			}()
+		}
 	}
 
 	return p, nil

lib/metrics/prometheus_test.go

@@ -0,0 +1,84 @@
+package metrics
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestPrometheusNoPushGateway(t *testing.T) {
+	config := NewConfig()
+
+	p, err := NewPrometheus(config)
+	assert.NoError(t, err)
+	assert.NotNil(t, p)
+	assert.Nil(t, p.(*Prometheus).pusher)
+}
+
+func TestPrometheusWithPushGateway(t *testing.T) {
+	pusherChan := make(chan struct{})
+	server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+		pusherChan <- struct{}{}
+	}))
+	defer server.Close()
+
+	config := NewConfig()
+	config.Prometheus.PushURL = server.URL
+
+	p, err := NewPrometheus(config)
+	assert.NoError(t, err)
+	assert.NotNil(t, p)
+	assert.NotNil(t, p.(*Prometheus).pusher)
+
+	go func() {
+		err = p.Close()
+		assert.NoError(t, err)
+	}()
+
+	// Wait for message for the PushGateway after close
+	select {
+	case <-pusherChan:
+	case <-time.After(100 * time.Millisecond):
+		assert.Fail(t, "PushGateway did not receive expected messages")
+	}
+}
+
+func TestPrometheusWithPushGatewayAndPushInterval(t *testing.T) {
+	pusherChan := make(chan struct{})
+	server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+		pusherChan <- struct{}{}
+	}))
+	defer server.Close()
+
+	pushInterval := 1 * time.Millisecond
+	config := NewConfig()
+	config.Prometheus.PushURL = server.URL
+	config.Prometheus.PushInterval = pushInterval.String()
+
+	p, err := NewPrometheus(config)
+	assert.NoError(t, err)
+	assert.NotNil(t, p)
+	assert.NotNil(t, p.(*Prometheus).pusher)
+
+	// Wait for first message for the PushGateway
+	select {
+	case <-pusherChan:
+	case <-time.After(100 * time.Millisecond):
+		assert.Fail(t, "PushGateway did not receive expected messages")
+	}
+
+	go func() {
+		err = p.Close()
+		assert.NoError(t, err)
+	}()
+
+	// Wait for another message for the PushGateway (might not be the one sent on close)
+	select {
+	case <-pusherChan:
+	case <-time.After(100 * time.Millisecond):
+		assert.Fail(t, "PushGateway did not receive expected messages after close")
+	}
+}