From 0230dbf30504411a59f76908180f4a28a19b6487 Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Tue, 9 Apr 2013 02:33:17 +0200 Subject: [PATCH 1/2] Fix off-by-one bug in NewFingerprintFromMetric(). --- model/fingerprinting.go | 2 +- model/metric_test.go | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/model/fingerprinting.go b/model/fingerprinting.go index efdd64fa8..ae2593dc1 100644 --- a/model/fingerprinting.go +++ b/model/fingerprinting.go @@ -94,7 +94,7 @@ func NewFingerprintFromMetric(metric Metric) (f Fingerprint) { firstCharacterOfFirstLabelName = labelName[0:1] } if i == labelLength-1 { - lastCharacterOfLastLabelValue = string(labelValue[labelValueLength-2 : labelValueLength-1]) + lastCharacterOfLastLabelValue = string(labelValue[labelValueLength-1 : labelValueLength]) } summer.Write([]byte(labelName)) diff --git a/model/metric_test.go b/model/metric_test.go index ac72b2a51..44d2f0f47 100644 --- a/model/metric_test.go +++ b/model/metric_test.go @@ -35,9 +35,16 @@ func testMetric(t test.Tester) { "occupation": "robot", "manufacturer": "westinghouse", }, - rowkey: "04776841610193542734-f-56-o", + rowkey: "04776841610193542734-f-56-t", hash: 4776841610193542734, }, + { + input: map[string]string{ + "x": "y", + }, + rowkey: "01306929544689993150-x-2-y", + hash: 1306929544689993150, + }, } for i, scenario := range scenarios { From ebe05d1b83a56ee5854fa68502fe91ab45f76eab Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Tue, 9 Apr 2013 02:35:55 +0200 Subject: [PATCH 2/2] Fix logic bug in fingerprint Less() comparison. Seems like just using String() is the easiest way of doing this. --- model/fingerprinting.go | 16 ++------- model/fingerprinting_test.go | 68 ++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 14 deletions(-) create mode 100644 model/fingerprinting_test.go diff --git a/model/fingerprinting.go b/model/fingerprinting.go index ae2593dc1..ac3b1deb4 100644 --- a/model/fingerprinting.go +++ b/model/fingerprinting.go @@ -41,6 +41,7 @@ type Fingerprint interface { ToDTO() *dto.Fingerprint Less(Fingerprint) bool Equal(Fingerprint) bool + String() string } // Builds a Fingerprint from a row key. @@ -151,20 +152,7 @@ func (f fingerprint) LastCharacterOfLastLabelValue() string { } func (f fingerprint) Less(o Fingerprint) bool { - if f.Hash() < o.Hash() { - return true - } - if f.FirstCharacterOfFirstLabelName() < o.FirstCharacterOfFirstLabelName() { - return true - } - if f.LabelMatterLength() < o.LabelMatterLength() { - return true - } - if f.LastCharacterOfLastLabelValue() < o.LastCharacterOfLastLabelValue() { - return true - } - - return false + return f.String() < o.String() } func (f fingerprint) Equal(o Fingerprint) (equal bool) { diff --git a/model/fingerprinting_test.go b/model/fingerprinting_test.go new file mode 100644 index 000000000..e2b2dab6c --- /dev/null +++ b/model/fingerprinting_test.go @@ -0,0 +1,68 @@ +// Copyright 2013 Prometheus Team +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package model + +import ( + "testing" +) + +func TestFingerprintComparison(t *testing.T) { + fingerprints := []fingerprint{ + { + hash: 0, + firstCharacterOfFirstLabelName: "b", + labelMatterLength: 1, + lastCharacterOfLastLabelValue: "b", + }, + { + hash: 1, + firstCharacterOfFirstLabelName: "a", + labelMatterLength: 0, + lastCharacterOfLastLabelValue: "a", + }, + { + hash: 1, + firstCharacterOfFirstLabelName: "a", + labelMatterLength: 1000, + lastCharacterOfLastLabelValue: "b", + }, + { + hash: 1, + firstCharacterOfFirstLabelName: "b", + labelMatterLength: 0, + lastCharacterOfLastLabelValue: "a", + }, + { + hash: 1, + firstCharacterOfFirstLabelName: "b", + labelMatterLength: 1, + lastCharacterOfLastLabelValue: "a", + }, + { + hash: 1, + firstCharacterOfFirstLabelName: "b", + labelMatterLength: 1, + lastCharacterOfLastLabelValue: "b", + }, + } + for i := range fingerprints { + if i == 0 { + continue + } + + if !fingerprints[i-1].Less(fingerprints[i]) { + t.Errorf("%d expected %s < %s", i, fingerprints[i-1], fingerprints[i]) + } + } +}