Unify fingerprinting aka signature calculation.

This fixes https://github.com/prometheus/client_golang/issues/74 .

IT CHANGES THE FINGERPRINTING AND THEREFORE INVALIDATES EACH AND EVERY
PERSISTED FINGERPRINT (I.E. YOU HAVE TO WIPE THE STORAGE TO USE THIS).

This commit removes the LabelValuesToSignature function as it is used
nowhere (and broken in the way it is implemented right now).

Also, remove one more golint warning.

model/metric.go

@@ -14,10 +14,8 @@
 package model
 
 import (
-	"encoding/binary"
 	"encoding/json"
 	"fmt"
-	"hash/fnv"
 	"sort"
 	"strings"
 )
@@ -66,37 +64,7 @@ func (m Metric) String() string {
 
 // Fingerprint returns a Metric's Fingerprint.
 func (m Metric) Fingerprint() Fingerprint {
-	labelNames := make([]string, 0, len(m))
+	return metricToFingerprint(m)
-	maxLength := 0
-
-	for labelName, labelValue := range m {
-		labelNames = append(labelNames, string(labelName))
-		if len(labelName) > maxLength {
-			maxLength = len(labelName)
-		}
-		if len(labelValue) > maxLength {
-			maxLength = len(labelValue)
-		}
-	}
-
-	sort.Strings(labelNames)
-
-	summer := fnv.New64a()
-	buf := make([]byte, maxLength)
-
-	for _, labelName := range labelNames {
-		labelValue := m[LabelName(labelName)]
-
-		copy(buf, labelName)
-		summer.Write(buf[:len(labelName)])
-
-		summer.Write(separator)
-
-		copy(buf, labelValue)
-		summer.Write(buf[:len(labelValue)])
-	}
-
-	return Fingerprint(binary.LittleEndian.Uint64(summer.Sum(nil)))
 }
 
 // Clone returns a copy of the Metric.

model/metric_test.go

@@ -22,7 +22,7 @@ func testMetric(t testing.TB) {
 	}{
 		{
 			input:       Metric{},
-			fingerprint: 2676020557754725067,
+			fingerprint: 14695981039346656037,
 		},
 		{
 			input: Metric{
@@ -30,31 +30,27 @@ func testMetric(t testing.TB) {
 				"occupation":   "robot",
 				"manufacturer": "westinghouse",
 			},
-			fingerprint: 13260944541294022935,
+			fingerprint: 11310079640881077873,
 		},
 		{
 			input: Metric{
 				"x": "y",
 			},
-			fingerprint: 1470933794305433534,
+			fingerprint: 13948396922932177635,
 		},
-		// The following two demonstrate a bug in fingerprinting. They
-		// should not have the same fingerprint with a sane
-		// fingerprinting function. See
-		// https://github.com/prometheus/client_golang/issues/74 .
 		{
 			input: Metric{
 				"a": "bb",
 				"b": "c",
 			},
-			fingerprint: 3734646176939799877,
+			fingerprint: 3198632812309449502,
 		},
 		{
 			input: Metric{
 				"a":  "b",
 				"bb": "c",
 			},
-			fingerprint: 3734646176939799877,
+			fingerprint: 5774953389407657638,
 		},
 	}
 

model/signature.go

@@ -63,10 +63,10 @@ func LabelsToSignature(labels map[string]string) uint64 {
 	hb := getHashAndBuf()
 	defer putHashAndBuf(hb)
 
-	for k, v := range labels {
+	for labelName, labelValue := range labels {
-		hb.b.WriteString(k)
+		hb.b.WriteString(labelName)
 		hb.b.WriteByte(SeparatorByte)
-		hb.b.WriteString(v)
+		hb.b.WriteString(labelValue)
 		hb.h.Write(hb.b.Bytes())
 		result ^= hb.h.Sum64()
 		hb.h.Reset()
@@ -75,10 +75,34 @@ func LabelsToSignature(labels map[string]string) uint64 {
 	return result
 }
 
-// LabelValuesToSignature returns a unique signature (i.e., fingerprint) for the
+// metricToFingerprint works exactly as LabelsToSignature but takes a Metric as
-// values of a given label set.
+// parameter (rather than a label map) and returns a Fingerprint.
-func LabelValuesToSignature(labels map[string]string) uint64 {
+func metricToFingerprint(m Metric) Fingerprint {
-	if len(labels) == 0 {
+	if len(m) == 0 {
+		return Fingerprint(emptyLabelSignature)
+	}
+
+	var result uint64
+	hb := getHashAndBuf()
+	defer putHashAndBuf(hb)
+
+	for labelName, labelValue := range m {
+		hb.b.WriteString(string(labelName))
+		hb.b.WriteByte(SeparatorByte)
+		hb.b.WriteString(string(labelValue))
+		hb.h.Write(hb.b.Bytes())
+		result ^= hb.h.Sum64()
+		hb.h.Reset()
+		hb.b.Reset()
+	}
+	return Fingerprint(result)
+}
+
+// SignatureForLabels works like LabelsToSignature but takes a Metric as
+// parameter (rather than a label map) and only includes the labels with the
+// specified LabelNames into the signature calculation.
+func SignatureForLabels(m Metric, labels LabelNames) uint64 {
+	if len(m) == 0 || len(labels) == 0 {
 		return emptyLabelSignature
 	}
 
@@ -86,8 +110,10 @@ func LabelValuesToSignature(labels map[string]string) uint64 {
 	hb := getHashAndBuf()
 	defer putHashAndBuf(hb)
 
-	for _, v := range labels {
+	for _, label := range labels {
-		hb.b.WriteString(v)
+		hb.b.WriteString(string(label))
+		hb.b.WriteByte(SeparatorByte)
+		hb.b.WriteString(string(m[label]))
 		hb.h.Write(hb.b.Bytes())
 		result ^= hb.h.Sum64()
 		hb.h.Reset()
@@ -95,3 +121,33 @@ func LabelValuesToSignature(labels map[string]string) uint64 {
 	}
 	return result
 }
+
+// SignatureWithoutLabels works like LabelsToSignature but takes a Metric as
+// parameter (rather than a label map) and excludes the labels with any of the
+// specified LabelNames from the signature calculation.
+func SignatureWithoutLabels(m Metric, labels map[LabelName]struct{}) uint64 {
+	if len(m) == 0 {
+		return emptyLabelSignature
+	}
+
+	var result uint64
+	hb := getHashAndBuf()
+	defer putHashAndBuf(hb)
+
+	for labelName, labelValue := range m {
+		if _, exclude := labels[labelName]; exclude {
+			continue
+		}
+		hb.b.WriteString(string(labelName))
+		hb.b.WriteByte(SeparatorByte)
+		hb.b.WriteString(string(labelValue))
+		hb.h.Write(hb.b.Bytes())
+		result ^= hb.h.Sum64()
+		hb.h.Reset()
+		hb.b.Reset()
+	}
+	if result == 0 {
+		return emptyLabelSignature
+	}
+	return result
+}

model/signature_test.go

@@ -18,7 +18,7 @@ import (
 	"testing"
 )
 
-func testLabelsToSignature(t testing.TB) {
+func TestLabelsToSignature(t *testing.T) {
 	var scenarios = []struct {
 		in  map[string]string
 		out uint64
@@ -42,57 +42,112 @@ func testLabelsToSignature(t testing.TB) {
 	}
 }
 
-func TestLabelToSignature(t *testing.T) {
+func TestMetricToFingerprint(t *testing.T) {
-	testLabelsToSignature(t)
+	var scenarios = []struct {
-}
+		in  Metric
-
+		out Fingerprint
-func TestEmptyLabelSignature(t *testing.T) {
+	}{
-	input := []map[string]string{nil, {}}
+		{
-
+			in:  Metric{},
-	var ms runtime.MemStats
+			out: 14695981039346656037,
-	runtime.ReadMemStats(&ms)
+		},
-
+		{
-	alloc := ms.Alloc
+			in:  Metric{"name": "garland, briggs", "fear": "love is not enough"},
-
+			out: 12952432476264840823,
-	for _, labels := range input {
+		},
-		LabelsToSignature(labels)
 	}
 
-	runtime.ReadMemStats(&ms)
+	for i, scenario := range scenarios {
+		actual := metricToFingerprint(scenario.in)
 
-	if got := ms.Alloc; alloc != got {
+		if actual != scenario.out {
-		t.Fatal("expected LabelsToSignature with empty labels not to perform allocations")
+			t.Errorf("%d. expected %d, got %d", i, scenario.out, actual)
-	}
-}
-
-func BenchmarkLabelToSignature(b *testing.B) {
-	for i := 0; i < b.N; i++ {
-		testLabelsToSignature(b)
-	}
-}
-
-func benchmarkLabelValuesToSignature(b *testing.B, l map[string]string, e uint64) {
-	for i := 0; i < b.N; i++ {
-		if a := LabelValuesToSignature(l); a != e {
-			b.Fatalf("expected signature of %d for %s, got %d", e, l, a)
 		}
 	}
 }
 
-func BenchmarkLabelValuesToSignatureScalar(b *testing.B) {
+func TestSignatureForLabels(t *testing.T) {
-	benchmarkLabelValuesToSignature(b, nil, 14695981039346656037)
+	var scenarios = []struct {
+		in     Metric
+		labels LabelNames
+		out    uint64
+	}{
+		{
+			in:     Metric{},
+			labels: nil,
+			out:    14695981039346656037,
+		},
+		{
+			in:     Metric{"name": "garland, briggs", "fear": "love is not enough"},
+			labels: LabelNames{"fear", "name"},
+			out:    12952432476264840823,
+		},
+		{
+			in:     Metric{"name": "garland, briggs", "fear": "love is not enough", "foo": "bar"},
+			labels: LabelNames{"fear", "name"},
+			out:    12952432476264840823,
+		},
+		{
+			in:     Metric{"name": "garland, briggs", "fear": "love is not enough"},
+			labels: LabelNames{},
+			out:    14695981039346656037,
+		},
+		{
+			in:     Metric{"name": "garland, briggs", "fear": "love is not enough"},
+			labels: nil,
+			out:    14695981039346656037,
+		},
+	}
+
+	for i, scenario := range scenarios {
+		actual := SignatureForLabels(scenario.in, scenario.labels)
+
+		if actual != scenario.out {
+			t.Errorf("%d. expected %d, got %d", i, scenario.out, actual)
+		}
+	}
 }
 
-func BenchmarkLabelValuesToSignatureSingle(b *testing.B) {
+func TestSignatureWithoutLabels(t *testing.T) {
-	benchmarkLabelValuesToSignature(b, map[string]string{"first-label": "first-label-value"}, 2653746141194979650)
+	var scenarios = []struct {
-}
+		in     Metric
+		labels map[LabelName]struct{}
+		out    uint64
+	}{
+		{
+			in:     Metric{},
+			labels: nil,
+			out:    14695981039346656037,
+		},
+		{
+			in:     Metric{"name": "garland, briggs", "fear": "love is not enough"},
+			labels: map[LabelName]struct{}{"fear": struct{}{}, "name": struct{}{}},
+			out:    14695981039346656037,
+		},
+		{
+			in:     Metric{"name": "garland, briggs", "fear": "love is not enough", "foo": "bar"},
+			labels: map[LabelName]struct{}{"foo": struct{}{}},
+			out:    12952432476264840823,
+		},
+		{
+			in:     Metric{"name": "garland, briggs", "fear": "love is not enough"},
+			labels: map[LabelName]struct{}{},
+			out:    12952432476264840823,
+		},
+		{
+			in:     Metric{"name": "garland, briggs", "fear": "love is not enough"},
+			labels: nil,
+			out:    12952432476264840823,
+		},
+	}
 
-func BenchmarkLabelValuesToSignatureDouble(b *testing.B) {
+	for i, scenario := range scenarios {
-	benchmarkLabelValuesToSignature(b, map[string]string{"first-label": "first-label-value", "second-label": "second-label-value"}, 8893559499616767364)
+		actual := SignatureWithoutLabels(scenario.in, scenario.labels)
-}
 
-func BenchmarkLabelValuesToSignatureTriple(b *testing.B) {
+		if actual != scenario.out {
-	benchmarkLabelValuesToSignature(b, map[string]string{"first-label": "first-label-value", "second-label": "second-label-value", "third-label": "third-label-value"}, 1685970066862087833)
+			t.Errorf("%d. expected %d, got %d", i, scenario.out, actual)
+		}
+	}
 }
 
 func benchmarkLabelToSignature(b *testing.B, l map[string]string, e uint64) {
@@ -118,3 +173,46 @@ func BenchmarkLabelToSignatureDouble(b *testing.B) {
 func BenchmarkLabelToSignatureTriple(b *testing.B) {
 	benchmarkLabelToSignature(b, map[string]string{"first-label": "first-label-value", "second-label": "second-label-value", "third-label": "third-label-value"}, 15738406913934009676)
 }
+
+func benchmarkMetricToFingerprint(b *testing.B, m Metric, e Fingerprint) {
+	for i := 0; i < b.N; i++ {
+		if a := metricToFingerprint(m); a != e {
+			b.Fatalf("expected signature of %d for %s, got %d", e, m, a)
+		}
+	}
+}
+
+func BenchmarkMetricToFingerprintScalar(b *testing.B) {
+	benchmarkMetricToFingerprint(b, nil, 14695981039346656037)
+}
+
+func BenchmarkMetricToFingerprintSingle(b *testing.B) {
+	benchmarkMetricToFingerprint(b, Metric{"first-label": "first-label-value"}, 5147259542624943964)
+}
+
+func BenchmarkMetricToFingerprintDouble(b *testing.B) {
+	benchmarkMetricToFingerprint(b, Metric{"first-label": "first-label-value", "second-label": "second-label-value"}, 18269973311206963528)
+}
+
+func BenchmarkMetricToFingerprintTriple(b *testing.B) {
+	benchmarkMetricToFingerprint(b, Metric{"first-label": "first-label-value", "second-label": "second-label-value", "third-label": "third-label-value"}, 15738406913934009676)
+}
+
+func TestEmptyLabelSignature(t *testing.T) {
+	input := []map[string]string{nil, {}}
+
+	var ms runtime.MemStats
+	runtime.ReadMemStats(&ms)
+
+	alloc := ms.Alloc
+
+	for _, labels := range input {
+		LabelsToSignature(labels)
+	}
+
+	runtime.ReadMemStats(&ms)
+
+	if got := ms.Alloc; alloc != got {
+		t.Fatal("expected LabelsToSignature with empty labels not to perform allocations")
+	}
+}

model/timestamp.go

@@ -88,6 +88,7 @@ func (t Timestamp) String() string {
 	return strconv.FormatFloat(float64(t)/float64(second), 'f', -1, 64)
 }
 
+// MarshalJSON implements the json.Marshaler interface.
 func (t Timestamp) MarshalJSON() ([]byte, error) {
 	return []byte(t.String()), nil
 }