Merge pull request #513 from prometheus/beorn7/registry

Fix the race in the registry

prometheus/registry.go

@@ -872,7 +872,13 @@ func checkMetricConsistency(
 	h = hashAddByte(h, separatorByte)
 	// Make sure label pairs are sorted. We depend on it for the consistency
 	// check.
-	sort.Sort(labelPairSorter(dtoMetric.Label))
+	if !sort.IsSorted(labelPairSorter(dtoMetric.Label)) {
+		// We cannot sort dtoMetric.Label in place as it is immutable by contract.
+		copiedLabels := make([]*dto.LabelPair, len(dtoMetric.Label))
+		copy(copiedLabels, dtoMetric.Label)
+		sort.Sort(labelPairSorter(copiedLabels))
+		dtoMetric.Label = copiedLabels
+	}
 	for _, lp := range dtoMetric.Label {
 		h = hashAdd(h, lp.GetName())
 		h = hashAddByte(h, separatorByte)
@@ -903,8 +909,8 @@ func checkDescConsistency(
 	}
 
 	// Is the desc consistent with the content of the metric?
-	lpsFromDesc := make([]*dto.LabelPair, 0, len(dtoMetric.Label))
-	lpsFromDesc = append(lpsFromDesc, desc.constLabelPairs...)
+	lpsFromDesc := make([]*dto.LabelPair, len(desc.constLabelPairs), len(dtoMetric.Label))
+	copy(lpsFromDesc, desc.constLabelPairs)
 	for _, l := range desc.variableLabels {
 		lpsFromDesc = append(lpsFromDesc, &dto.LabelPair{
 			Name: proto.String(l),

prometheus/registry_test.go

@@ -21,6 +21,7 @@ package prometheus_test
 
 import (
 	"bytes"
+	"fmt"
 	"io/ioutil"
 	"math/rand"
 	"net/http"
@@ -783,6 +784,11 @@ func TestAlreadyRegistered(t *testing.T) {
 // same HistogramVec is registered concurrently and the Gather method of the
 // registry is called concurrently.
 func TestHistogramVecRegisterGatherConcurrency(t *testing.T) {
+	labelNames := make([]string, 16) // Need at least 13 to expose #512.
+	for i := range labelNames {
+		labelNames[i] = fmt.Sprint("label_", i)
+	}
+
 	var (
 		reg = prometheus.NewPedanticRegistry()
 		hv  = prometheus.NewHistogramVec(
@@ -791,7 +797,7 @@ func TestHistogramVecRegisterGatherConcurrency(t *testing.T) {
 				Help:        "This helps testing.",
 				ConstLabels: prometheus.Labels{"foo": "bar"},
 			},
-			[]string{"one", "two", "three"},
+			labelNames,
 		)
 		labelValues = []string{"a", "b", "c", "alpha", "beta", "gamma", "aleph", "beth", "gimel"}
 		quit        = make(chan struct{})
@@ -806,11 +812,11 @@ func TestHistogramVecRegisterGatherConcurrency(t *testing.T) {
 				return
 			default:
 				obs := rand.NormFloat64()*.1 + .2
-				hv.WithLabelValues(
-					labelValues[rand.Intn(len(labelValues))],
-					labelValues[rand.Intn(len(labelValues))],
-					labelValues[rand.Intn(len(labelValues))],
-				).Observe(obs)
+				values := make([]string, 0, len(labelNames))
+				for range labelNames {
+					values = append(values, labelValues[rand.Intn(len(labelValues))])
+				}
+				hv.WithLabelValues(values...).Observe(obs)
 			}
 		}
 	}
@@ -848,7 +854,7 @@ func TestHistogramVecRegisterGatherConcurrency(t *testing.T) {
 					if len(g) != 1 {
 						t.Error("Gathered unexpected number of metric families:", len(g))
 					}
-					if len(g[0].Metric[0].Label) != 4 {
+					if len(g[0].Metric[0].Label) != len(labelNames)+1 {
 						t.Error("Gathered unexpected number of label pairs:", len(g[0].Metric[0].Label))
 					}
 				}