Implement review suggestions

metrics/api.go

@@ -69,7 +69,7 @@ func WriteMetrics(w io.Writer, permission api.Permission, expertiseLevel config.
 	registryLock.RLock()
 	defer registryLock.RUnlock()
 
-	// Check if metric ID is already registered.
+	// Write all matching metrics.
 	for _, metric := range registry {
 		if permission >= metric.Opts().Permission &&
 			expertiseLevel >= metric.Opts().ExpertiseLevel {
@@ -96,28 +96,25 @@ func writeMetricsTo(ctx context.Context, url string) error {
 	}
 
 	// Send.
-	client := &http.Client{}
-	resp, err := client.Do(req)
+	resp, err := http.DefaultClient.Do(req)
 	if err != nil {
 		return err
 	}
 	defer resp.Body.Close()
 
 	// Check return status.
-	switch resp.StatusCode {
-	case http.StatusOK,
-		http.StatusAccepted,
-		http.StatusNoContent:
+	if resp.StatusCode >= 200 && resp.StatusCode <= 299 {
 		return nil
-	default:
-		body, _ := ioutil.ReadAll(resp.Body)
-		return fmt.Errorf(
-			"got %s while writing metrics to %s: %s",
-			resp.Status,
-			url,
-			body,
-		)
 	}
+
+	// Get and return error.
+	body, _ := ioutil.ReadAll(resp.Body)
+	return fmt.Errorf(
+		"got %s while writing metrics to %s: %s",
+		resp.Status,
+		url,
+		body,
+	)
 }
 
 func metricsWriter(ctx context.Context) error {

metrics/metric.go

@@ -126,10 +126,11 @@ func (m *metricBase) buildLabeledID() string {
 		return metricID
 	}
 
-	// Add global labels to the custom ones.
-	// This overrides conflicts.
+	// Add global labels to the custom ones, if they don't exist yet.
 	for labelName, labelValue := range globalLabels {
-		m.Labels[labelName] = labelValue
+		if _, ok := m.Labels[labelName]; !ok {
+			m.Labels[labelName] = labelValue
+		}
 	}
 
 	// Render labels into a slice and sort them in order to make the labeled ID

metrics/metric_runtime.go

@@ -9,6 +9,9 @@ import (
 )
 
 func init() {
+	registryLock.Lock()
+	defer registryLock.Unlock()
+
 	registry = append(registry, &runtimeMetrics{})
 }
 

metrics/module.go

@@ -20,7 +20,8 @@ var (
 	metricNamespace       string
 	globalLabels          = make(map[string]string)
 
-	pushURL string
+	pushURL        string
+	metricInstance string
 
 	// ErrAlreadyStarted is returned when an operation is only valid before the
 	// first metric is registered, and is called after.
@@ -36,11 +37,19 @@ var (
 
 func init() {
 	flag.StringVar(&pushURL, "push-metrics", "", "URL to push prometheus metrics to")
+	flag.StringVar(&metricInstance, "metrics-instance", "", "Set the global instance label")
 
 	module = modules.Register("metrics", prep, start, stop, "database", "api")
 }
 
 func prep() error {
+	// Add metric instance name as global variable if set.
+	if metricInstance != "" {
+		if err := AddGlobalLabel("instance", metricInstance); err != nil {
+			return err
+		}
+	}
+
 	return registerInfoMetric()
 }
 
@@ -75,7 +84,7 @@ func register(m Metric) error {
 
 	// Add new metric to registry and sort it.
 	registry = append(registry, m)
-	sort.Sort(metricRegistry(registry))
+	sort.Sort(byLabeledID(registry))
 
 	// Set flag that first metric is now registered.
 	firstMetricRegistered = true
@@ -124,8 +133,8 @@ func AddGlobalLabel(name, value string) error {
 	return nil
 }
 
-type metricRegistry []Metric
+type byLabeledID []Metric
 
-func (r metricRegistry) Len() int           { return len(r) }
-func (r metricRegistry) Less(i, j int) bool { return r[i].LabeledID() < r[j].LabeledID() }
-func (r metricRegistry) Swap(i, j int)      { r[i], r[j] = r[j], r[i] }
+func (r byLabeledID) Len() int           { return len(r) }
+func (r byLabeledID) Less(i, j int) bool { return r[i].LabeledID() < r[j].LabeledID() }
+func (r byLabeledID) Swap(i, j int)      { r[i], r[j] = r[j], r[i] }

metrics/persistence.go

@@ -14,13 +14,18 @@ import (
 
 var (
 	storage       *metricsStorage
-	storageLoaded = abool.New()
 	storageKey    string
+	storageInit   = abool.New()
+	storageLoaded = abool.New()
 
 	db = database.NewInterface(&database.Options{
 		Local:    true,
 		Internal: true,
 	})
+
+	// ErrAlreadyInitialized is returned when trying to initialize an option
+	// more than once.
+	ErrAlreadyInitialized = errors.New("already initialized")
 )
 
 type metricsStorage struct {
@@ -39,9 +44,9 @@ type metricsStorage struct {
 // persistence.
 // May only be called once.
 func EnableMetricPersistence(key string) error {
-	// Check if already loaded.
-	if storageLoaded.IsSet() {
-		return nil
+	// Check if already initialized.
+	if !storageInit.SetToIf(false, true) {
+		return ErrAlreadyInitialized
 	}
 
 	// Set storage key.
@@ -85,7 +90,7 @@ func (c *Counter) loadState() {
 
 func storePersistentMetrics() {
 	// Check if persistence is enabled.
-	if storageKey == "" {
+	if !storageInit.IsSet() || storageKey == "" {
 		return
 	}