From af21d456db19b0fc9fb7cf0d05933b84ab7f4181 Mon Sep 17 00:00:00 2001 From: beorn7 Date: Tue, 3 Mar 2015 13:23:11 +0100 Subject: [PATCH] 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 | 34 +------- model/metric_test.go | 14 ++-- model/signature.go | 74 +++++++++++++++-- model/signature_test.go | 180 +++++++++++++++++++++++++++++++--------- model/timestamp.go | 1 + 5 files changed, 211 insertions(+), 92 deletions(-) diff --git a/model/metric.go b/model/metric.go index 74f394b..48210ab 100644 --- a/model/metric.go +++ b/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)) - 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))) + return metricToFingerprint(m) } // Clone returns a copy of the Metric. diff --git a/model/metric_test.go b/model/metric_test.go index 7c31bdf..f4cd6ee 100644 --- a/model/metric_test.go +++ b/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, }, } diff --git a/model/signature.go b/model/signature.go index 4b392fb..ab77ae8 100644 --- a/model/signature.go +++ b/model/signature.go @@ -63,10 +63,10 @@ func LabelsToSignature(labels map[string]string) uint64 { hb := getHashAndBuf() defer putHashAndBuf(hb) - for k, v := range labels { - hb.b.WriteString(k) + for labelName, labelValue := range labels { + 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 -// values of a given label set. -func LabelValuesToSignature(labels map[string]string) uint64 { - if len(labels) == 0 { +// metricToFingerprint works exactly as LabelsToSignature but takes a Metric as +// parameter (rather than a label map) and returns a Fingerprint. +func metricToFingerprint(m Metric) Fingerprint { + 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 { - hb.b.WriteString(v) + for _, label := range labels { + 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 +} diff --git a/model/signature_test.go b/model/signature_test.go index ad20fa3..be31998 100644 --- a/model/signature_test.go +++ b/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) { - testLabelsToSignature(t) -} - -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) +func TestMetricToFingerprint(t *testing.T) { + var scenarios = []struct { + in Metric + out Fingerprint + }{ + { + in: Metric{}, + out: 14695981039346656037, + }, + { + in: Metric{"name": "garland, briggs", "fear": "love is not enough"}, + out: 12952432476264840823, + }, } - runtime.ReadMemStats(&ms) + for i, scenario := range scenarios { + actual := metricToFingerprint(scenario.in) - if got := ms.Alloc; alloc != got { - t.Fatal("expected LabelsToSignature with empty labels not to perform allocations") - } -} - -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) + if actual != scenario.out { + t.Errorf("%d. expected %d, got %d", i, scenario.out, actual) } } } -func BenchmarkLabelValuesToSignatureScalar(b *testing.B) { - benchmarkLabelValuesToSignature(b, nil, 14695981039346656037) +func TestSignatureForLabels(t *testing.T) { + 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) { - benchmarkLabelValuesToSignature(b, map[string]string{"first-label": "first-label-value"}, 2653746141194979650) -} +func TestSignatureWithoutLabels(t *testing.T) { + 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) { - benchmarkLabelValuesToSignature(b, map[string]string{"first-label": "first-label-value", "second-label": "second-label-value"}, 8893559499616767364) -} + for i, scenario := range scenarios { + actual := SignatureWithoutLabels(scenario.in, scenario.labels) -func BenchmarkLabelValuesToSignatureTriple(b *testing.B) { - benchmarkLabelValuesToSignature(b, map[string]string{"first-label": "first-label-value", "second-label": "second-label-value", "third-label": "third-label-value"}, 1685970066862087833) + if actual != scenario.out { + 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") + } +} diff --git a/model/timestamp.go b/model/timestamp.go index 09bd877..afffdcf 100644 --- a/model/timestamp.go +++ b/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 }