service_exporter: control access to metrics map with a mutex (#28)

* service_exporter: control access to metrics map with a mutex

* service_exporter: fix tests by adding metricsLock field

* service_exporter: fix tests, impoert sync package

* service_exporter: add missing braces

pkg/service_exporter/service_exporter.go

@@ -2,6 +2,7 @@ package serviceexporter
 
 import (
 	"fmt"
+	"sync"
 	"time"
 
 	"github.com/pkg/errors"
@@ -31,6 +32,7 @@ type ServiceQuotasExporter struct {
 	metricsRegion   string
 	quotasClient    servicequotas.QuotasInterface
 	metrics         map[string]Metric
+	metricsLock     *sync.Mutex
 	refreshPeriod   int
 	waitForMetrics  chan struct{}
 	includedAWSTags []string
@@ -48,6 +50,7 @@ func NewServiceQuotasExporter(region, profile string, refreshPeriod int, include
 		metricsRegion:   region,
 		quotasClient:    quotasClient,
 		metrics:         map[string]Metric{},
+		metricsLock:     &sync.Mutex{},
 		refreshPeriod:   refreshPeriod,
 		waitForMetrics:  ch,
 		includedAWSTags: includedAWSTags,
@@ -73,6 +76,9 @@ func (e *ServiceQuotasExporter) createOrUpdateQuotasAndDescriptions(update bool)
 		log.Fatalf("Could not retrieve quotas and limits: %s", err)
 	}
 
+	e.metricsLock.Lock()
+	defer e.metricsLock.Unlock()
+
 	for _, quota := range quotas {
 		key := metricKey(quota)
 		resourceID := quota.Identifier()
@@ -129,6 +135,9 @@ func (e *ServiceQuotasExporter) Describe(ch chan<- *prometheus.Desc) {
 
 // Collect implements the collect function for prometheus collectors
 func (e *ServiceQuotasExporter) Collect(ch chan<- prometheus.Metric) {
+	e.metricsLock.Lock()
+	defer e.metricsLock.Unlock()
+
 	for _, metric := range e.metrics {
 		ch <- prometheus.MustNewConstMetric(metric.limitDesc, prometheus.GaugeValue, metric.limit, metric.labelValues...)
 		ch <- prometheus.MustNewConstMetric(metric.usageDesc, prometheus.GaugeValue, metric.usage, metric.labelValues...)

pkg/service_exporter/service_exporter_test.go

@@ -1,6 +1,7 @@
 package serviceexporter
 
 import (
+	"sync"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -37,6 +38,7 @@ func TestUpdateMetrics(t *testing.T) {
 			"i-asdasd1": Metric{usage: 3, limit: 5, labelValues: []string{"before-dummy-value"}},
 			"i-asdasd2": Metric{usage: 2, limit: 2},
 		},
+		metricsLock:     &sync.Mutex{},
 		includedAWSTags: []string{"dummy-tag"},
 		refreshPeriod:   360,
 	}
@@ -47,6 +49,8 @@ func TestUpdateMetrics(t *testing.T) {
 		"i-asdasd1": Metric{usage: 5, limit: 10, labelValues: []string{"i-asdasd1", "dummy-value"}},
 		"i-asdasd2": Metric{usage: 2, limit: 3, labelValues: []string{"i-asdasd2", ""}},
 	}
+	exporter.metricsLock.Lock()
+	defer exporter.metricsLock.Unlock()
 	assert.Equal(t, expectedMetrics, exporter.metrics)
 }
 
@@ -77,6 +81,7 @@ func TestCreateQuotasAndDescriptions(t *testing.T) {
 		metricsRegion:   region,
 		quotasClient:    quotasClient,
 		metrics:         map[string]Metric{},
+		metricsLock:     &sync.Mutex{},
 		refreshPeriod:   360,
 		waitForMetrics:  ch,
 		includedAWSTags: []string{"dummy-tag", "dummy-tag2"},
@@ -105,6 +110,8 @@ func TestCreateQuotasAndDescriptions(t *testing.T) {
 		},
 	}
 
+	exporter.metricsLock.Lock()
+	defer exporter.metricsLock.Unlock()
 	assert.Equal(t, expectedMetrics, exporter.metrics)
 }
 
@@ -130,6 +137,7 @@ func TestCreateQuotasAndDescriptionsRefresh(t *testing.T) {
 		metrics: map[string]Metric{
 			"i-asdasd1": Metric{usage: 3, limit: 5, labelValues: []string{"before-dummy-value"}, usageDesc: desc},
 		},
+		metricsLock:     &sync.Mutex{},
 		waitForMetrics:  ch,
 		includedAWSTags: []string{"dummy-tag"},
 		refreshPeriod:   360,
@@ -141,6 +149,8 @@ func TestCreateQuotasAndDescriptionsRefresh(t *testing.T) {
 		"i-asdasd1": Metric{usage: 5, limit: 10, labelValues: []string{"i-asdasd1", "dummy-value"}, usageDesc: desc},
 	}
 
+	exporter.metricsLock.Lock()
+	defer exporter.metricsLock.Unlock()
 	assert.Equal(t, expectedMetrics, exporter.metrics)
 
 	close(ch) // should panic if it was already closed