Fix GetMetricForFingerprint() metric mutability.

Some users of GetMetricForFingerprint() end up modifying the returned metric
labelset. Since the memory storage's implementation of
GetMetricForFingerprint() returned a pointer to the metric (and maps are
reference types anyways), the external mutation propagated back into the memory
storage.

The fix is to make a copy of the metric before returning it.

rules/ast/persistence_adapter.go

@@ -116,7 +116,7 @@ func (v *viewAdapter) GetValueAtTime(fingerprints model.Fingerprints, timestamp
 		}
 		if samplePair != nil {
 			samples = append(samples, model.Sample{
-				Metric:    *m,
+				Metric:    m,
 				Value:     samplePair.Value,
 				Timestamp: timestamp,
 			})
@@ -140,7 +140,7 @@ func (v *viewAdapter) GetBoundaryValues(fingerprints model.Fingerprints, interva
 		}
 
 		sampleSet := model.SampleSet{
-			Metric: *m,
+			Metric: m,
 			Values: samplePairs,
 		}
 		sampleSets = append(sampleSets, sampleSet)
@@ -162,7 +162,7 @@ func (v *viewAdapter) GetRangeValues(fingerprints model.Fingerprints, interval *
 		}
 
 		sampleSet := model.SampleSet{
-			Metric: *m,
+			Metric: m,
 			Values: samplePairs,
 		}
 		sampleSets = append(sampleSets, sampleSet)

storage/metric/end_to_end_test.go

@@ -172,17 +172,15 @@ func GetMetricForFingerprintTests(p MetricPersistence, t test.Tester) {
 		t.Errorf("Expected one element.")
 	}
 
-	v, e := p.GetMetricForFingerprint(result[0])
+	metric, err := p.GetMetricForFingerprint(result[0])
-	if e != nil {
+	if err != nil {
-		t.Error(e)
+		t.Error(err)
 	}
 
-	if v == nil {
+	if metric == nil {
 		t.Fatal("Did not expect nil.")
 	}
 
-	metric := *v
-
 	if len(metric) != 1 {
 		t.Errorf("Expected one-dimensional metric.")
 	}
@@ -203,20 +201,43 @@ func GetMetricForFingerprintTests(p MetricPersistence, t test.Tester) {
 		t.Errorf("Expected one element.")
 	}
 
-	v, e = p.GetMetricForFingerprint(result[0])
+	metric, err = p.GetMetricForFingerprint(result[0])
 
-	if v == nil {
+	if metric == nil {
 		t.Fatal("Did not expect nil.")
 	}
 
-	metric = *v
+	if err != nil {
-
+		t.Error(err)
-	if e != nil {
-		t.Error(e)
 	}
 
 	if len(metric) != 2 {
-		t.Errorf("Expected one-dimensional metric.")
+		t.Errorf("Expected two-dimensional metric.")
+	}
+
+	if metric["request_type"] != "your_dad" {
+		t.Errorf("Expected metric to match.")
+	}
+
+	if metric["one-off"] != "value" {
+		t.Errorf("Expected metric to match.")
+	}
+
+	// Verify that mutating a returned metric does not result in the mutated
+	// metric to be returned at the next GetMetricForFingerprint() call.
+	metric["one-off"] = "new value"
+	metric, err = p.GetMetricForFingerprint(result[0])
+
+	if metric == nil {
+		t.Fatal("Did not expect nil.")
+	}
+
+	if err != nil {
+		t.Error(err)
+	}
+
+	if len(metric) != 2 {
+		t.Errorf("Expected two-dimensional metric.")
 	}
 
 	if metric["request_type"] != "your_dad" {

storage/metric/interface.go

@@ -45,7 +45,7 @@ type MetricPersistence interface {
 	GetFingerprintsForLabelName(model.LabelName) (model.Fingerprints, error)
 
 	// Get the metric associated with the provided fingerprint.
-	GetMetricForFingerprint(model.Fingerprint) (*model.Metric, error)
+	GetMetricForFingerprint(model.Fingerprint) (model.Metric, error)
 
 	GetValueAtTime(model.Fingerprint, time.Time) model.Values
 	GetBoundaryValues(model.Fingerprint, model.Interval) (first model.Values, second model.Values)

storage/metric/leveldb.go

@@ -772,7 +772,7 @@ func (l *LevelDBMetricPersistence) GetFingerprintsForLabelName(labelName model.L
 	return
 }
 
-func (l *LevelDBMetricPersistence) GetMetricForFingerprint(f model.Fingerprint) (m *model.Metric, err error) {
+func (l *LevelDBMetricPersistence) GetMetricForFingerprint(f model.Fingerprint) (m model.Metric, err error) {
 	defer func(begin time.Time) {
 		duration := time.Since(begin)
 
@@ -790,16 +790,12 @@ func (l *LevelDBMetricPersistence) GetMetricForFingerprint(f model.Fingerprint)
 		return
 	}
 
-	metric := model.Metric{}
+	m = model.Metric{}
 
 	for _, v := range unmarshaled.LabelPair {
-		metric[model.LabelName(*v.Name)] = model.LabelValue(*v.Value)
+		m[model.LabelName(*v.Name)] = model.LabelValue(*v.Value)
 	}
 
-	// Explicit address passing here shaves immense amounts of time off of the
-	// code flow due to less tight-loop dereferencing.
-	m = &metric
-
 	return
 }
 

storage/metric/memory.go

@@ -199,13 +199,16 @@ func (s memorySeriesStorage) GetFingerprintsForLabelName(l model.LabelName) (fin
 	return
 }
 
-func (s memorySeriesStorage) GetMetricForFingerprint(f model.Fingerprint) (metric *model.Metric, err error) {
+func (s memorySeriesStorage) GetMetricForFingerprint(f model.Fingerprint) (metric model.Metric, err error) {
 	series, ok := s.fingerprintToSeries[f]
 	if !ok {
 		return
 	}
 
-	metric = &series.metric
+	metric = model.Metric{}
+	for label, value := range series.metric {
+		metric[label] = value
+	}
 
 	return
 }

storage/metric/tiered.go

@@ -596,7 +596,7 @@ func (t *TieredStorage) GetFingerprintsForLabelSet(labelSet model.LabelSet) (fin
 }
 
 // Get the metric associated with the provided fingerprint.
-func (t *TieredStorage) GetMetricForFingerprint(f model.Fingerprint) (m *model.Metric, err error) {
+func (t *TieredStorage) GetMetricForFingerprint(f model.Fingerprint) (m model.Metric, err error) {
 	m, err = t.memoryArena.GetMetricForFingerprint(f)
 	if err != nil {
 		return