Replace constLabels with a full set of sorted labelPairs

Signed-off-by: Kyle Eckhart <[email protected]>

prometheus/counter.go

@@ -94,7 +94,7 @@ func NewCounter(opts CounterOpts) Counter {
 	if opts.now == nil {
 		opts.now = time.Now
 	}
-	result := &counter{desc: desc, labelPairs: desc.constLabelPairs, now: opts.now}
+	result := &counter{desc: desc, labelPairs: desc.labelPairs, now: opts.now}
 	result.init(result) // Init self-collection.
 	result.createdTs = timestamppb.New(opts.now())
 	return result

prometheus/desc.go

@@ -47,12 +47,17 @@ type Desc struct {
 	fqName string
 	// help provides some helpful information about this metric.
 	help string
-	// constLabelPairs contains precalculated DTO label pairs based on
-	// the constant labels.
-	constLabelPairs []*dto.LabelPair
 	// variableLabels contains names of labels and normalization function for
 	// which the metric maintains variable values.
 	variableLabels *compiledLabels
+	// labelPairs contains the sorted DTO label pairs based on the constant labels
+	// and variable labels
+	labelPairs []*dto.LabelPair
+	// variableLabelIndexesInLabelPairs holds all indexes variable labels in the
+	// labelPairs with the expected index of the variableLabel. This makes it easy
+	// to identify all variable labels in the labelPairs and where to get their value
+	// from when given the variable label values
+	variableLabelIndexesInLabelPairs map[int]int
 	// id is a hash of the values of the ConstLabels and fqName. This
 	// must be unique among all registered descriptors and can therefore be
 	// used as an identifier of the descriptor.
@@ -160,14 +165,36 @@ func (v2) NewDesc(fqName, help string, variableLabels ConstrainableLabels, const
 	}
 	d.dimHash = xxh.Sum64()
 
-	d.constLabelPairs = make([]*dto.LabelPair, 0, len(constLabels))
+	d.labelPairs = make([]*dto.LabelPair, 0, len(constLabels)+len(d.variableLabels.names))
 	for n, v := range constLabels {
-		d.constLabelPairs = append(d.constLabelPairs, &dto.LabelPair{
+		d.labelPairs = append(d.labelPairs, &dto.LabelPair{
 			Name:  proto.String(n),
 			Value: proto.String(v),
 		})
 	}
-	sort.Sort(internal.LabelPairSorter(d.constLabelPairs))
+	for _, labelName := range d.variableLabels.names {
+		d.labelPairs = append(d.labelPairs, &dto.LabelPair{
+			Name: proto.String(labelName),
+		})
+	}
+	sort.Sort(internal.LabelPairSorter(d.labelPairs))
+
+	// In order to facilitate mapping from the unsorted variable labels to
+	// the sorted variable labels we generate a mapping from output labelPair
+	// index -> variableLabel index for constructing the final label pairs later
+	d.variableLabelIndexesInLabelPairs = make(map[int]int, len(d.variableLabels.names))
+	for outputIndex, pair := range d.labelPairs {
+		// Constant labels have values variable labels do not
+		if pair.Value != nil {
+			continue
+		}
+		for sourceIndex, variableLabel := range d.variableLabels.names {
+			if variableLabel == pair.GetName() {
+				d.variableLabelIndexesInLabelPairs[outputIndex] = sourceIndex
+			}
+		}
+	}
+
 	return d
 }
 
@@ -182,8 +209,11 @@ func NewInvalidDesc(err error) *Desc {
 }
 
 func (d *Desc) String() string {
-	lpStrings := make([]string, 0, len(d.constLabelPairs))
-	for _, lp := range d.constLabelPairs {
+	lpStrings := make([]string, 0, len(d.labelPairs))
+	for _, lp := range d.labelPairs {
+		if lp.Value == nil {
+			continue
+		}
 		lpStrings = append(
 			lpStrings,
 			fmt.Sprintf("%s=%q", lp.GetName(), lp.GetValue()),

prometheus/gauge.go

@@ -82,7 +82,7 @@ func NewGauge(opts GaugeOpts) Gauge {
 		nil,
 		opts.ConstLabels,
 	)
-	result := &gauge{desc: desc, labelPairs: desc.constLabelPairs}
+	result := &gauge{desc: desc, labelPairs: desc.labelPairs}
 	result.init(result) // Init self-collection.
 	return result
 }

prometheus/histogram.go

@@ -537,12 +537,7 @@ func newHistogram(desc *Desc, opts HistogramOpts, labelValues ...string) Histogr
 		panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels.names, labelValues))
 	}
 
-	for _, n := range desc.variableLabels.names {
-		if n == bucketLabel {
-			panic(errBucketLabelNotAllowed)
-		}
-	}
-	for _, lp := range desc.constLabelPairs {
+	for _, lp := range desc.labelPairs {
 		if lp.GetName() == bucketLabel {
 			panic(errBucketLabelNotAllowed)
 		}

prometheus/registry.go

@@ -962,21 +962,13 @@ func checkDescConsistency(
 	}
 
 	// Is the desc consistent with the content of the metric?
-	lpsFromDesc := make([]*dto.LabelPair, len(desc.constLabelPairs), len(dtoMetric.Label))
-	copy(lpsFromDesc, desc.constLabelPairs)
-	for _, l := range desc.variableLabels.names {
-		lpsFromDesc = append(lpsFromDesc, &dto.LabelPair{
-			Name: proto.String(l),
-		})
-	}
-	if len(lpsFromDesc) != len(dtoMetric.Label) {
+	if len(desc.labelPairs) != len(dtoMetric.Label) {
 		return fmt.Errorf(
 			"labels in collected metric %s %s are inconsistent with descriptor %s",
 			metricFamily.GetName(), dtoMetric, desc,
 		)
 	}
-	sort.Sort(internal.LabelPairSorter(lpsFromDesc))
-	for i, lpFromDesc := range lpsFromDesc {
+	for i, lpFromDesc := range desc.labelPairs {
 		lpFromMetric := dtoMetric.Label[i]
 		if lpFromDesc.GetName() != lpFromMetric.GetName() ||
 			lpFromDesc.Value != nil && lpFromDesc.GetValue() != lpFromMetric.GetValue() {

prometheus/summary.go

@@ -196,12 +196,7 @@ func newSummary(desc *Desc, opts SummaryOpts, labelValues ...string) Summary {
 		panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels.names, labelValues))
 	}
 
-	for _, n := range desc.variableLabels.names {
-		if n == quantileLabel {
-			panic(errQuantileLabelNotAllowed)
-		}
-	}
-	for _, lp := range desc.constLabelPairs {
+	for _, lp := range desc.labelPairs {
 		if lp.GetName() == quantileLabel {
 			panic(errQuantileLabelNotAllowed)
 		}

prometheus/value.go

@@ -16,12 +16,9 @@ package prometheus
 import (
 	"errors"
 	"fmt"
-	"sort"
 	"time"
 	"unicode/utf8"
 
-	"github.com/prometheus/client_golang/prometheus/internal"
-
 	dto "github.com/prometheus/client_model/go"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/types/known/timestamppb"
@@ -215,24 +212,29 @@ func populateMetric(
 // This function is only needed for custom Metric implementations. See MetricVec
 // example.
 func MakeLabelPairs(desc *Desc, labelValues []string) []*dto.LabelPair {
-	totalLen := len(desc.variableLabels.names) + len(desc.constLabelPairs)
-	if totalLen == 0 {
+	if len(desc.labelPairs) == 0 {
 		// Super fast path.
 		return nil
 	}
 	if len(desc.variableLabels.names) == 0 {
 		// Moderately fast path.
-		return desc.constLabelPairs
+		return desc.labelPairs
 	}
-	labelPairs := make([]*dto.LabelPair, 0, totalLen)
-	for i, l := range desc.variableLabels.names {
-		labelPairs = append(labelPairs, &dto.LabelPair{
-			Name:  proto.String(l),
-			Value: proto.String(labelValues[i]),
-		})
+	labelPairs := make([]*dto.LabelPair, 0, len(desc.labelPairs))
+	for i, lp := range desc.labelPairs {
+		var labelToAdd *dto.LabelPair
+		// Variable labels have no value and need to be inserted with a new dto.LabelPair containing the labelValue
+		if lp.Value == nil {
+			variableLabelIndex := desc.variableLabelIndexesInLabelPairs[i]
+			labelToAdd = &dto.LabelPair{
+				Name:  lp.Name,
+				Value: proto.String(labelValues[variableLabelIndex]),
+			}
+		} else {
+			labelToAdd = lp
+		}
+		labelPairs = append(labelPairs, labelToAdd)
 	}
-	labelPairs = append(labelPairs, desc.constLabelPairs...)
-	sort.Sort(internal.LabelPairSorter(labelPairs))
 	return labelPairs
 }
 

prometheus/value_test.go

@@ -14,10 +14,12 @@
 package prometheus
 
 import (
+	"reflect"
 	"testing"
 	"time"
 
 	dto "github.com/prometheus/client_model/go"
+	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/types/known/timestamppb"
 )
 
@@ -108,3 +110,132 @@ func TestNewConstMetricWithCreatedTimestamp(t *testing.T) {
 		})
 	}
 }
+
+func TestMakeLabelPairs(t *testing.T) {
+	tests := []struct {
+		name        string
+		desc        *Desc
+		labelValues []string
+		want        []*dto.LabelPair
+	}{
+		{
+			name:        "no labels",
+			desc:        NewDesc("metric-1", "", nil, nil),
+			labelValues: nil,
+			want:        nil,
+		},
+		{
+			name: "only constant labels",
+			desc: NewDesc("metric-1", "", nil, map[string]string{
+				"label-1": "1",
+				"label-2": "2",
+				"label-3": "3",
+			}),
+			labelValues: nil,
+			want: []*dto.LabelPair{
+				{Name: proto.String("label-1"), Value: proto.String("1")},
+				{Name: proto.String("label-2"), Value: proto.String("2")},
+				{Name: proto.String("label-3"), Value: proto.String("3")},
+			},
+		},
+		{
+			name:        "only variable labels",
+			desc:        NewDesc("metric-1", "", []string{"var-label-1", "var-label-2", "var-label-3"}, nil),
+			labelValues: []string{"1", "2", "3"},
+			want: []*dto.LabelPair{
+				{Name: proto.String("var-label-1"), Value: proto.String("1")},
+				{Name: proto.String("var-label-2"), Value: proto.String("2")},
+				{Name: proto.String("var-label-3"), Value: proto.String("3")},
+			},
+		},
+		{
+			name: "variable and const labels",
+			desc: NewDesc("metric-1", "", []string{"var-label-1", "var-label-2", "var-label-3"}, map[string]string{
+				"label-1": "1",
+				"label-2": "2",
+				"label-3": "3",
+			}),
+			labelValues: []string{"1", "2", "3"},
+			want: []*dto.LabelPair{
+				{Name: proto.String("label-1"), Value: proto.String("1")},
+				{Name: proto.String("label-2"), Value: proto.String("2")},
+				{Name: proto.String("label-3"), Value: proto.String("3")},
+				{Name: proto.String("var-label-1"), Value: proto.String("1")},
+				{Name: proto.String("var-label-2"), Value: proto.String("2")},
+				{Name: proto.String("var-label-3"), Value: proto.String("3")},
+			},
+		},
+		{
+			name: "unsorted variable and const labels are sorted",
+			desc: NewDesc("metric-1", "", []string{"var-label-3", "var-label-2", "var-label-1"}, map[string]string{
+				"label-3": "3",
+				"label-2": "2",
+				"label-1": "1",
+			}),
+			labelValues: []string{"3", "2", "1"},
+			want: []*dto.LabelPair{
+				{Name: proto.String("label-1"), Value: proto.String("1")},
+				{Name: proto.String("label-2"), Value: proto.String("2")},
+				{Name: proto.String("label-3"), Value: proto.String("3")},
+				{Name: proto.String("var-label-1"), Value: proto.String("1")},
+				{Name: proto.String("var-label-2"), Value: proto.String("2")},
+				{Name: proto.String("var-label-3"), Value: proto.String("3")},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := MakeLabelPairs(tt.desc, tt.labelValues); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("%v != %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func Benchmark_MakeLabelPairs(b *testing.B) {
+	benchFunc := func(desc *Desc, variableLabelValues []string) {
+		MakeLabelPairs(desc, variableLabelValues)
+	}
+
+	benchmarks := []struct {
+		name                string
+		bench               func(desc *Desc, variableLabelValues []string)
+		desc                *Desc
+		variableLabelValues []string
+	}{
+		{
+			name: "1 label",
+			desc: NewDesc(
+				"metric",
+				"help",
+				[]string{"var-label-1"},
+				Labels{"const-label-1": "value"}),
+			variableLabelValues: []string{"value"},
+		},
+		{
+			name: "3 labels",
+			desc: NewDesc(
+				"metric",
+				"help",
+				[]string{"var-label-1", "var-label-3", "var-label-2"},
+				Labels{"const-label-1": "value", "const-label-3": "value", "const-label-2": "value"}),
+			variableLabelValues: []string{"value", "value", "value"},
+		},
+		{
+			name: "10 labels",
+			desc: NewDesc(
+				"metric",
+				"help",
+				[]string{"var-label-5", "var-label-1", "var-label-3", "var-label-2", "var-label-10", "var-label-4", "var-label-7", "var-label-8", "var-label-9"},
+				Labels{"const-label-4": "value", "const-label-1": "value", "const-label-7": "value", "const-label-2": "value", "const-label-9": "value", "const-label-8": "value", "const-label-10": "value", "const-label-3": "value", "const-label-6": "value", "const-label-5": "value"}),
+			variableLabelValues: []string{"value", "value", "value", "value", "value", "value", "value", "value", "value", "value"},
+		},
+	}
+	for _, bm := range benchmarks {
+		b.Run(bm.name, func(b *testing.B) {
+			for i := 0; i < b.N; i++ {
+				benchFunc(bm.desc, bm.variableLabelValues)
+			}
+		})
+	}
+}

prometheus/wrap.go

@@ -188,17 +188,20 @@ func (m *wrappingMetric) Write(out *dto.Metric) error {
 
 func wrapDesc(desc *Desc, prefix string, labels Labels) *Desc {
 	constLabels := Labels{}
-	for _, lp := range desc.constLabelPairs {
-		constLabels[*lp.Name] = *lp.Value
+	for _, lp := range desc.labelPairs {
+		// Variable labels have no values
+		if lp.Value != nil {
+			constLabels[*lp.Name] = *lp.Value
+		}
 	}
 	for ln, lv := range labels {
 		if _, alreadyUsed := constLabels[ln]; alreadyUsed {
 			return &Desc{
-				fqName:          desc.fqName,
-				help:            desc.help,
-				variableLabels:  desc.variableLabels,
-				constLabelPairs: desc.constLabelPairs,
-				err:             fmt.Errorf("attempted wrapping with already existing label name %q", ln),
+				fqName:         desc.fqName,
+				help:           desc.help,
+				variableLabels: desc.variableLabels,
+				labelPairs:     desc.labelPairs,
+				err:            fmt.Errorf("attempted wrapping with already existing label name %q", ln),
 			}
 		}
 		constLabels[ln] = lv