Merge pull request #226 from prometheus/beorn7/alloc

Bring back zero-alloc label-value access for metric vecs

Author: beorn7

CHANGELOG.md

@@ -1,3 +1,26 @@
+## 0.8.0 / 2016-08-17
+* [CHANGE] Registry is doing more consistency checks. This might break
+  existing setups that used to export inconsistent metrics.
+* [CHANGE] Pushing to Pushgateway moved to package `push` and changed to allow
+  arbitrary grouping.
+* [CHANGE] Removed `SelfCollector`.
+* [CHANGE] Removed `PanicOnCollectError` and `EnableCollectChecks` methods.
+* [CHANGE] Moved packages to the prometheus/common repo: `text`, `model`,
+  `extraction`.
+* [CHANGE] Deprecated a number of functions.
+* [FEATURE] Allow custom registries. Added `Registerer` and `Gatherer`
+  interfaces.
+* [FEATURE] Separated HTTP exposition, allowing custom HTTP handlers (package
+  `promhttp`) and enabling the creation of other exposition mechanisms.
+* [FEATURE] `MustRegister` is variadic now, allowing registration of many
+  collectors in one call.
+* [FEATURE] Added HTTP API v1 package.
+* [ENHANCEMENT] Numerous documentation improvements.
+* [ENHANCEMENT] Improved metric sorting.
+* [ENHANCEMENT] Inlined fnv64a hashing for improved performance.
+* [ENHANCEMENT] Several test improvements.
+* [BUGFIX] Handle collisions in MetricVec.
+
 ## 0.7.0 / 2015-07-27
 * [CHANGE] Rename ExporterLabelPrefix to ExportedLabelPrefix.
 * [BUGFIX] Closed gaps in metric consistency check.

VERSION

@@ -1 +1 @@
-0.7.0
+0.8.0

prometheus/counter.go

@@ -82,7 +82,7 @@ func (c *counter) Add(v float64) {
 // CounterVec embeds MetricVec. See there for a full list of methods with
 // detailed documentation.
 type CounterVec struct {
-	MetricVec
+	*MetricVec
 }
 
 // NewCounterVec creates a new CounterVec based on the provided CounterOpts and

prometheus/gauge.go

@@ -58,7 +58,7 @@ func NewGauge(opts GaugeOpts) Gauge {
 // (e.g. number of operations queued, partitioned by user and operation
 // type). Create instances with NewGaugeVec.
 type GaugeVec struct {
-	MetricVec
+	*MetricVec
 }
 
 // NewGaugeVec creates a new GaugeVec based on the provided GaugeOpts and

prometheus/histogram.go

@@ -287,7 +287,7 @@ func (h *histogram) Write(out *dto.Metric) error {
 // (e.g. HTTP request latencies, partitioned by status code and method). Create
 // instances with NewHistogramVec.
 type HistogramVec struct {
-	MetricVec
+	*MetricVec
 }
 
 // NewHistogramVec creates a new HistogramVec based on the provided HistogramOpts and

prometheus/summary.go

@@ -390,7 +390,7 @@ func (s quantSort) Less(i, j int) bool {
 // (e.g. HTTP request latencies, partitioned by status code and method). Create
 // instances with NewSummaryVec.
 type SummaryVec struct {
-	MetricVec
+	*MetricVec
 }
 
 // NewSummaryVec creates a new SummaryVec based on the provided SummaryOpts and

prometheus/untyped.go

@@ -56,7 +56,7 @@ func NewUntyped(opts UntypedOpts) Untyped {
 // labels. This is used if you want to count the same thing partitioned by
 // various dimensions. Create instances with NewUntypedVec.
 type UntypedVec struct {
-	MetricVec
+	*MetricVec
 }
 
 // NewUntypedVec creates a new UntypedVec based on the provided UntypedOpts and

prometheus/vec.go

@@ -37,8 +37,8 @@ type MetricVec struct {
 
 // newMetricVec returns an initialized MetricVec. The concrete value is
 // returned for embedding into another struct.
-func newMetricVec(desc *Desc, newMetric func(lvs ...string) Metric) MetricVec {
-	return MetricVec{
+func newMetricVec(desc *Desc, newMetric func(lvs ...string) Metric) *MetricVec {
+	return &MetricVec{
 		children:    map[uint64][]metricWithLabelValues{},
 		desc:        desc,
 		newMetric:   newMetric,
@@ -102,7 +102,7 @@ func (m *MetricVec) GetMetricWithLabelValues(lvs ...string) (Metric, error) {
 		return nil, err
 	}
 
-	return m.getOrCreateMetric(h, lvs), nil
+	return m.getOrCreateMetricWithLabelValues(h, lvs), nil
 }
 
 // GetMetricWith returns the Metric for the given Labels map (the label names
@@ -123,7 +123,7 @@ func (m *MetricVec) GetMetricWith(labels Labels) (Metric, error) {
 		return nil, err
 	}
 
-	return m.getOrCreateMetric(h, labels), nil
+	return m.getOrCreateMetricWithLabels(h, labels), nil
 }
 
 // WithLabelValues works as GetMetricWithLabelValues, but panics if an error
@@ -171,7 +171,7 @@ func (m *MetricVec) DeleteLabelValues(lvs ...string) bool {
 	if err != nil {
 		return false
 	}
-	return m.deleteByHash(h, lvs)
+	return m.deleteByHashWithLabelValues(h, lvs)
 }
 
 // Delete deletes the metric where the variable labels are the same as those
@@ -193,21 +193,40 @@ func (m *MetricVec) Delete(labels Labels) bool {
 		return false
 	}
 
-	return m.deleteByHash(h, labels)
+	return m.deleteByHashWithLabels(h, labels)
 }
 
-// deleteByHash removes the metric from the hash bucket h. If there are
-// multiple matches in the bucket, use lvs to select a metric and remove only
-// that metric.
-//
-// lvs MUST be of type Labels or []string or this method will panic.
-func (m *MetricVec) deleteByHash(h uint64, lvs interface{}) bool {
+// deleteByHashWithLabelValues removes the metric from the hash bucket h. If
+// there are multiple matches in the bucket, use lvs to select a metric and
+// remove only that metric.
+func (m *MetricVec) deleteByHashWithLabelValues(h uint64, lvs []string) bool {
 	metrics, ok := m.children[h]
 	if !ok {
 		return false
 	}
 
-	i := m.findMetric(metrics, lvs)
+	i := m.findMetricWithLabelValues(metrics, lvs)
+	if i >= len(metrics) {
+		return false
+	}
+
+	if len(metrics) > 1 {
+		m.children[h] = append(metrics[:i], metrics[i+1:]...)
+	} else {
+		delete(m.children, h)
+	}
+	return true
+}
+
+// deleteByHashWithLabels removes the metric from the hash bucket h. If there
+// are multiple matches in the bucket, use lvs to select a metric and remove
+// only that metric.
+func (m *MetricVec) deleteByHashWithLabels(h uint64, labels Labels) bool {
+	metrics, ok := m.children[h]
+	if !ok {
+		return false
+	}
+	i := m.findMetricWithLabels(metrics, labels)
 	if i >= len(metrics) {
 		return false
 	}
@@ -258,119 +277,128 @@ func (m *MetricVec) hashLabels(labels Labels) (uint64, error) {
 	return h, nil
 }
 
-// getOrCreateMetric retrieves the metric by hash and label value or creates it
-// and returns the new one.
+// getOrCreateMetricWithLabelValues retrieves the metric by hash and label value
+// or creates it and returns the new one.
 //
-// lvs MUST be of type Labels or []string or this method will panic.
+// This function holds the mutex.
+func (m *MetricVec) getOrCreateMetricWithLabelValues(hash uint64, lvs []string) Metric {
+	m.mtx.RLock()
+	metric, ok := m.getMetricWithLabelValues(hash, lvs)
+	m.mtx.RUnlock()
+	if ok {
+		return metric
+	}
+
+	m.mtx.Lock()
+	defer m.mtx.Unlock()
+	metric, ok = m.getMetricWithLabelValues(hash, lvs)
+	if !ok {
+		// Copy to avoid allocation in case wo don't go down this code path.
+		copiedLVs := make([]string, len(lvs))
+		copy(copiedLVs, lvs)
+		metric = m.newMetric(copiedLVs...)
+		m.children[hash] = append(m.children[hash], metricWithLabelValues{values: copiedLVs, metric: metric})
+	}
+	return metric
+}
+
+// getOrCreateMetricWithLabelValues retrieves the metric by hash and label value
+// or creates it and returns the new one.
 //
 // This function holds the mutex.
-func (m *MetricVec) getOrCreateMetric(hash uint64, lvs interface{}) Metric {
+func (m *MetricVec) getOrCreateMetricWithLabels(hash uint64, labels Labels) Metric {
 	m.mtx.RLock()
-	metric, ok := m.getMetric(hash, lvs)
+	metric, ok := m.getMetricWithLabels(hash, labels)
 	m.mtx.RUnlock()
 	if ok {
 		return metric
 	}
 
 	m.mtx.Lock()
 	defer m.mtx.Unlock()
-	metric, ok = m.getMetric(hash, lvs)
+	metric, ok = m.getMetricWithLabels(hash, labels)
 	if !ok {
-		lvs := m.copyLabelValues(lvs)
+		lvs := m.extractLabelValues(labels)
 		metric = m.newMetric(lvs...)
 		m.children[hash] = append(m.children[hash], metricWithLabelValues{values: lvs, metric: metric})
 	}
 	return metric
 }
 
-// getMetric while handling possible collisions in the hash space. Must be
-// called while holding read mutex.
-//
-// lvs must be of type Labels or []string.
-func (m *MetricVec) getMetric(h uint64, lvs interface{}) (Metric, bool) {
+// getMetricWithLabelValues gets a metric while handling possible collisions in
+// the hash space. Must be called while holding read mutex.
+func (m *MetricVec) getMetricWithLabelValues(h uint64, lvs []string) (Metric, bool) {
 	metrics, ok := m.children[h]
 	if ok {
-		return m.selectMetric(metrics, lvs)
+		if i := m.findMetricWithLabelValues(metrics, lvs); i < len(metrics) {
+			return metrics[i].metric, true
+		}
 	}
-
 	return nil, false
 }
 
-func (m *MetricVec) selectMetric(metrics []metricWithLabelValues, lvs interface{}) (Metric, bool) {
-	i := m.findMetric(metrics, lvs)
-
-	if i < len(metrics) {
-		return metrics[i].metric, true
+// getMetricWithLabels gets a metric while handling possible collisions in
+// the hash space. Must be called while holding read mutex.
+func (m *MetricVec) getMetricWithLabels(h uint64, labels Labels) (Metric, bool) {
+	metrics, ok := m.children[h]
+	if ok {
+		if i := m.findMetricWithLabels(metrics, labels); i < len(metrics) {
+			return metrics[i].metric, true
+		}
 	}
-
 	return nil, false
 }
 
-// findMetric returns the index of the matching metric or len(metrics) if not
-// found.
-func (m *MetricVec) findMetric(metrics []metricWithLabelValues, lvs interface{}) int {
+// findMetricWithLabelValues returns the index of the matching metric or
+// len(metrics) if not found.
+func (m *MetricVec) findMetricWithLabelValues(metrics []metricWithLabelValues, lvs []string) int {
 	for i, metric := range metrics {
-		if m.matchLabels(metric.values, lvs) {
+		if m.matchLabelValues(metric.values, lvs) {
 			return i
 		}
 	}
-
 	return len(metrics)
 }
 
-func (m *MetricVec) matchLabels(values []string, lvs interface{}) bool {
-	switch lvs := lvs.(type) {
-	case []string:
-		if len(values) != len(lvs) {
-			return false
-		}
-
-		for i, v := range values {
-			if v != lvs[i] {
-				return false
-			}
+// findMetricWithLabels returns the index of the matching metric or len(metrics)
+// if not found.
+func (m *MetricVec) findMetricWithLabels(metrics []metricWithLabelValues, labels Labels) int {
+	for i, metric := range metrics {
+		if m.matchLabels(metric.values, labels) {
+			return i
 		}
+	}
+	return len(metrics)
+}
 
-		return true
-	case Labels:
-		if len(lvs) != len(values) {
+func (m *MetricVec) matchLabelValues(values []string, lvs []string) bool {
+	if len(values) != len(lvs) {
+		return false
+	}
+	for i, v := range values {
+		if v != lvs[i] {
 			return false
 		}
-
-		for i, k := range m.desc.variableLabels {
-			if values[i] != lvs[k] {
-				return false
-			}
-		}
-
-		return true
-	default:
-		// If we reach this condition, there is an unexpected type being used
-		// as a labels value. Either add branch here for the new type or fix
-		// the bug causing the type to be passed in.
-		panic("unsupported type")
 	}
+	return true
 }
 
-// copyLabelValues copies the labels values into common string slice format to
-// use when allocating the metric and to keep track of hash collision
-// ambiguity.
-//
-// lvs must be of type Labels or []string or this method will panic.
-func (m *MetricVec) copyLabelValues(lvs interface{}) []string {
-	var labelValues []string
-	switch lvs := lvs.(type) {
-	case []string:
-		labelValues = make([]string, len(lvs))
-		copy(labelValues, lvs)
-	case Labels:
-		labelValues = make([]string, len(lvs))
-		for i, k := range m.desc.variableLabels {
-			labelValues[i] = lvs[k]
+func (m *MetricVec) matchLabels(values []string, labels Labels) bool {
+	if len(labels) != len(values) {
+		return false
+	}
+	for i, k := range m.desc.variableLabels {
+		if values[i] != labels[k] {
+			return false
 		}
-	default:
-		panic(fmt.Sprintf("unsupported type for lvs: %#v", lvs))
 	}
+	return true
+}
 
+func (m *MetricVec) extractLabelValues(labels Labels) []string {
+	labelValues := make([]string, len(labels))
+	for i, k := range m.desc.variableLabels {
+		labelValues[i] = labels[k]
+	}
 	return labelValues
 }

prometheus/vec_test.go

@@ -225,7 +225,7 @@ func TestCounterVecEndToEndWithCollision(t *testing.T) {
 		t.Errorf("got label value %q, want %q", got, want)
 	}
 	if got, want := m.GetCounter().GetValue(), 1.; got != want {
-		t.Errorf("got value %d, want %d", got, want)
+		t.Errorf("got value %f, want %f", got, want)
 	}
 	m.Reset()
 	if err := vec.WithLabelValues("!0IC=VloaY").Write(m); err != nil {
@@ -235,7 +235,7 @@ func TestCounterVecEndToEndWithCollision(t *testing.T) {
 		t.Errorf("got label value %q, want %q", got, want)
 	}
 	if got, want := m.GetCounter().GetValue(), 2.; got != want {
-		t.Errorf("got value %d, want %d", got, want)
+		t.Errorf("got value %f, want %f", got, want)
 	}
 }