From d049c340d9c4d061d6eae4af6a74ece8924f8c2d Mon Sep 17 00:00:00 2001 From: Kyle Eckhart Date: Wed, 12 Feb 2025 16:16:49 -0500 Subject: [PATCH 1/4] Replace constLabels with a full set of sorted labelPairs Signed-off-by: Kyle Eckhart --- prometheus/counter.go | 2 +- prometheus/desc.go | 46 +++++++++++--- prometheus/gauge.go | 2 +- prometheus/histogram.go | 7 +-- prometheus/registry.go | 12 +--- prometheus/summary.go | 7 +-- prometheus/value.go | 30 ++++----- prometheus/value_test.go | 131 +++++++++++++++++++++++++++++++++++++++ prometheus/wrap.go | 17 ++--- 9 files changed, 201 insertions(+), 53 deletions(-) diff --git a/prometheus/counter.go b/prometheus/counter.go index 2996aef6a..e5c242254 100644 --- a/prometheus/counter.go +++ b/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 diff --git a/prometheus/desc.go b/prometheus/desc.go index ad347113c..294fe3fb1 100644 --- a/prometheus/desc.go +++ b/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()), diff --git a/prometheus/gauge.go b/prometheus/gauge.go index aa1846365..a87f8ccf1 100644 --- a/prometheus/gauge.go +++ b/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 } diff --git a/prometheus/histogram.go b/prometheus/histogram.go index 1a279035b..d2a8fd78e 100644 --- a/prometheus/histogram.go +++ b/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) } diff --git a/prometheus/registry.go b/prometheus/registry.go index c6fd2f58b..27402266b 100644 --- a/prometheus/registry.go +++ b/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() { diff --git a/prometheus/summary.go b/prometheus/summary.go index 76a9e12f4..3d3263d91 100644 --- a/prometheus/summary.go +++ b/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) } diff --git a/prometheus/value.go b/prometheus/value.go index cc23011fa..4bb296618 100644 --- a/prometheus/value.go +++ b/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 } diff --git a/prometheus/value_test.go b/prometheus/value_test.go index 23da6b217..ba3b9eb4d 100644 --- a/prometheus/value_test.go +++ b/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) + } + }) + } +} diff --git a/prometheus/wrap.go b/prometheus/wrap.go index 25da157f1..6afec9c26 100644 --- a/prometheus/wrap.go +++ b/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 From 6da4db6dc2f8348a455b3bca5f657b47a3c02b4c Mon Sep 17 00:00:00 2001 From: Kyle Eckhart Date: Wed, 19 Feb 2025 17:24:18 -0500 Subject: [PATCH 2/4] Replace lookup map with a slice, improve variable names + comments, and add a benchmark for NewDesc Signed-off-by: Kyle Eckhart --- prometheus/desc.go | 18 +++---- prometheus/desc_test.go | 27 ++++++++++ prometheus/value.go | 12 +++-- prometheus/value_test.go | 105 +++++++++++++++++++++++++++------------ 4 files changed, 113 insertions(+), 49 deletions(-) diff --git a/prometheus/desc.go b/prometheus/desc.go index 294fe3fb1..2f9e84879 100644 --- a/prometheus/desc.go +++ b/prometheus/desc.go @@ -50,14 +50,13 @@ type Desc struct { // variableLabels contains names of labels and normalization function for // which the metric maintains variable values. variableLabels *compiledLabels + // variableLabelOrder maps variableLabels indexes to the position in the + // pre-computed labelPairs slice. This allows fast MakeLabelPair function + // that have to place ordered variable label values into pre-sorted labelPairs. + variableLabelOrder []int // 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. @@ -179,18 +178,15 @@ func (v2) NewDesc(fqName, help string, variableLabels ConstrainableLabels, const } 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)) + d.variableLabelOrder = make([]int, len(d.variableLabels.names)) for outputIndex, pair := range d.labelPairs { - // Constant labels have values variable labels do not + // 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 + d.variableLabelOrder[sourceIndex] = outputIndex } } } diff --git a/prometheus/desc_test.go b/prometheus/desc_test.go index 5a8429009..0a797de39 100644 --- a/prometheus/desc_test.go +++ b/prometheus/desc_test.go @@ -14,6 +14,7 @@ package prometheus import ( + "fmt" "testing" ) @@ -61,3 +62,29 @@ func TestNewInvalidDesc_String(t *testing.T) { t.Errorf("String: unexpected output: %s", desc.String()) } } + +func BenchmarkNewDesc(b *testing.B) { + for _, bm := range []struct { + labelCount int + descFunc func() *Desc + }{ + { + labelCount: 1, + descFunc: new1LabelDescFunc, + }, + { + labelCount: 3, + descFunc: new3LabelsDescFunc, + }, + { + labelCount: 10, + descFunc: new10LabelsDescFunc, + }, + } { + b.Run(fmt.Sprintf("labels=%v", bm.labelCount), func(b *testing.B) { + for i := 0; i < b.N; i++ { + bm.descFunc() + } + }) + } +} diff --git a/prometheus/value.go b/prometheus/value.go index 4bb296618..4af993cf2 100644 --- a/prometheus/value.go +++ b/prometheus/value.go @@ -221,20 +221,22 @@ func MakeLabelPairs(desc *Desc, labelValues []string) []*dto.LabelPair { return desc.labelPairs } labelPairs := make([]*dto.LabelPair, 0, len(desc.labelPairs)) - for i, lp := range desc.labelPairs { + for _, 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 + // 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]), + Name: lp.Name, } } else { labelToAdd = lp } labelPairs = append(labelPairs, labelToAdd) } + for i, outputIndex := range desc.variableLabelOrder { + labelPairs[outputIndex].Value = proto.String(labelValues[i]) + } + return labelPairs } diff --git a/prometheus/value_test.go b/prometheus/value_test.go index ba3b9eb4d..ba4787ab5 100644 --- a/prometheus/value_test.go +++ b/prometheus/value_test.go @@ -14,6 +14,7 @@ package prometheus import ( + "fmt" "reflect" "testing" "time" @@ -192,49 +193,87 @@ func TestMakeLabelPairs(t *testing.T) { } } -func Benchmark_MakeLabelPairs(b *testing.B) { - benchFunc := func(desc *Desc, variableLabelValues []string) { - MakeLabelPairs(desc, variableLabelValues) - } +var new1LabelDescFunc = func() *Desc { + return NewDesc( + "metric", + "help", + []string{"var-label-1"}, + Labels{"const-label-1": "value"}) +} + +var new3LabelsDescFunc = func() *Desc { + return 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"}) +} + +var new10LabelsDescFunc = func() *Desc { + return 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", "var-label-6"}, + 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"}) +} - benchmarks := []struct { - name string - bench func(desc *Desc, variableLabelValues []string) +func BenchmarkMakeLabelPairs(b *testing.B) { + for _, bm := range []struct { desc *Desc - variableLabelValues []string + makeLabelPairValues []string }{ { - name: "1 label", - desc: NewDesc( - "metric", - "help", - []string{"var-label-1"}, - Labels{"const-label-1": "value"}), - variableLabelValues: []string{"value"}, + desc: new1LabelDescFunc(), + makeLabelPairValues: []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"}, + desc: new3LabelsDescFunc(), + makeLabelPairValues: []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"}, + desc: new10LabelsDescFunc(), + makeLabelPairValues: []string{"value", "value", "value", "value", "value", "value", "value", "value", "value", "value"}, }, - } - for _, bm := range benchmarks { - b.Run(bm.name, func(b *testing.B) { + } { + b.Run(fmt.Sprintf("labels=%v", len(bm.makeLabelPairValues)), func(b *testing.B) { for i := 0; i < b.N; i++ { - benchFunc(bm.desc, bm.variableLabelValues) + MakeLabelPairs(bm.desc, bm.makeLabelPairValues) + } + }) + } +} + +func BenchmarkConstMetricFlow(b *testing.B) { + for _, bm := range []struct { + descFunc func() *Desc + labelValues []string + }{ + { + descFunc: new1LabelDescFunc, + labelValues: []string{"value"}, + }, + { + descFunc: new3LabelsDescFunc, + labelValues: []string{"value", "value", "value"}, + }, + { + descFunc: new10LabelsDescFunc, + labelValues: []string{"value", "value", "value", "value", "value", "value", "value", "value", "value", "value"}, + }, + } { + b.Run(fmt.Sprintf("labels=%v", len(bm.labelValues)), func(b *testing.B) { + for _, metricsToCreate := range []int{1, 2, 3, 5} { + b.Run(fmt.Sprintf("metrics=%v", metricsToCreate), func(b *testing.B) { + for i := 0; i < b.N; i++ { + desc := bm.descFunc() + for j := 0; j < metricsToCreate; j++ { + _, err := NewConstMetric(desc, GaugeValue, 1.0, bm.labelValues...) + if err != nil { + b.Fatal(err) + } + } + } + }) } }) } From d0e9c61dd50e83f7544af11d6159614d0d524693 Mon Sep 17 00:00:00 2001 From: Kyle Eckhart Date: Thu, 6 Mar 2025 14:42:59 -0500 Subject: [PATCH 3/4] Refactor to use the already sorted labelNames with a secondary slice for ordering Signed-off-by: Kyle Eckhart --- prometheus/counter.go | 2 +- prometheus/desc.go | 76 +++++++++++++++++++++------------------- prometheus/desc_test.go | 2 ++ prometheus/gauge.go | 2 +- prometheus/histogram.go | 8 ++++- prometheus/registry.go | 27 +++++++++----- prometheus/summary.go | 8 ++++- prometheus/value.go | 26 ++++++-------- prometheus/value_test.go | 4 +++ prometheus/wrap.go | 18 +++++----- 10 files changed, 98 insertions(+), 75 deletions(-) diff --git a/prometheus/counter.go b/prometheus/counter.go index e5c242254..2996aef6a 100644 --- a/prometheus/counter.go +++ b/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.labelPairs, now: opts.now} + result := &counter{desc: desc, labelPairs: desc.constLabelPairs, now: opts.now} result.init(result) // Init self-collection. result.createdTs = timestamppb.New(opts.now()) return result diff --git a/prometheus/desc.go b/prometheus/desc.go index 2f9e84879..1696abd86 100644 --- a/prometheus/desc.go +++ b/prometheus/desc.go @@ -22,8 +22,6 @@ import ( dto "github.com/prometheus/client_model/go" "github.com/prometheus/common/model" "google.golang.org/protobuf/proto" - - "github.com/prometheus/client_golang/prometheus/internal" ) // Desc is the descriptor used by every Prometheus Metric. It is essentially @@ -50,13 +48,11 @@ type Desc struct { // variableLabels contains names of labels and normalization function for // which the metric maintains variable values. variableLabels *compiledLabels - // variableLabelOrder maps variableLabels indexes to the position in the - // pre-computed labelPairs slice. This allows fast MakeLabelPair function - // that have to place ordered variable label values into pre-sorted labelPairs. - variableLabelOrder []int - // labelPairs contains the sorted DTO label pairs based on the constant labels + // constLabelPairs contains the sorted DTO label pairs based on the constant labels // and variable labels - labelPairs []*dto.LabelPair + constLabelPairs []*dto.LabelPair + + orderedLabels []labelMapping // 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. @@ -70,6 +66,21 @@ type Desc struct { err error } +type labelMapping struct { + constLabelIndex int + + variableLabelIndex int + variableLabelName *string +} + +func newLabelMapping() labelMapping { + return labelMapping{ + constLabelIndex: -1, + variableLabelIndex: -1, + variableLabelName: nil, + } +} + // NewDesc allocates and initializes a new Desc. Errors are recorded in the Desc // and will be reported on registration time. variableLabels and constLabels can // be nil if no such labels should be set. fqName must not be empty. @@ -137,7 +148,7 @@ func (v2) NewDesc(fqName, help string, variableLabels ConstrainableLabels, const d.err = fmt.Errorf("%q is not a valid label name for metric %q", label, fqName) return d } - labelNames = append(labelNames, "$"+label) + labelNames = append(labelNames, label) labelNameSet[label] = struct{}{} } if len(labelNames) != len(labelNameSet) { @@ -164,31 +175,25 @@ func (v2) NewDesc(fqName, help string, variableLabels ConstrainableLabels, const } d.dimHash = xxh.Sum64() - d.labelPairs = make([]*dto.LabelPair, 0, len(constLabels)+len(d.variableLabels.names)) - for n, v := range constLabels { - d.labelPairs = append(d.labelPairs, &dto.LabelPair{ - Name: proto.String(n), - Value: proto.String(v), - }) - } - for _, labelName := range d.variableLabels.names { - d.labelPairs = append(d.labelPairs, &dto.LabelPair{ - Name: proto.String(labelName), - }) - } - sort.Sort(internal.LabelPairSorter(d.labelPairs)) - - d.variableLabelOrder = make([]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.variableLabelOrder[sourceIndex] = outputIndex + d.constLabelPairs = make([]*dto.LabelPair, 0, len(constLabels)) + d.orderedLabels = make([]labelMapping, len(labelNames)) + for i, n := range labelNames { + lm := newLabelMapping() + if l, ok := constLabels[n]; ok { + d.constLabelPairs = append(d.constLabelPairs, &dto.LabelPair{ + Name: proto.String(n), + Value: proto.String(l), + }) + lm.constLabelIndex = len(d.constLabelPairs) - 1 + } else { + for variableLabelIndex, variableLabel := range variableLabels.labelNames() { + if variableLabel == n { + lm.variableLabelIndex = variableLabelIndex + lm.variableLabelName = proto.String(variableLabel) + } } } + d.orderedLabels[i] = lm } return d @@ -205,11 +210,8 @@ func NewInvalidDesc(err error) *Desc { } func (d *Desc) String() string { - lpStrings := make([]string, 0, len(d.labelPairs)) - for _, lp := range d.labelPairs { - if lp.Value == nil { - continue - } + lpStrings := make([]string, 0, len(d.constLabelPairs)) + for _, lp := range d.constLabelPairs { lpStrings = append( lpStrings, fmt.Sprintf("%s=%q", lp.GetName(), lp.GetValue()), diff --git a/prometheus/desc_test.go b/prometheus/desc_test.go index 0a797de39..3e0d8cab7 100644 --- a/prometheus/desc_test.go +++ b/prometheus/desc_test.go @@ -82,6 +82,8 @@ func BenchmarkNewDesc(b *testing.B) { }, } { b.Run(fmt.Sprintf("labels=%v", bm.labelCount), func(b *testing.B) { + b.ReportAllocs() + b.ResetTimer() for i := 0; i < b.N; i++ { bm.descFunc() } diff --git a/prometheus/gauge.go b/prometheus/gauge.go index a87f8ccf1..aa1846365 100644 --- a/prometheus/gauge.go +++ b/prometheus/gauge.go @@ -82,7 +82,7 @@ func NewGauge(opts GaugeOpts) Gauge { nil, opts.ConstLabels, ) - result := &gauge{desc: desc, labelPairs: desc.labelPairs} + result := &gauge{desc: desc, labelPairs: desc.constLabelPairs} result.init(result) // Init self-collection. return result } diff --git a/prometheus/histogram.go b/prometheus/histogram.go index d2a8fd78e..380fce5d9 100644 --- a/prometheus/histogram.go +++ b/prometheus/histogram.go @@ -537,7 +537,13 @@ func newHistogram(desc *Desc, opts HistogramOpts, labelValues ...string) Histogr panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels.names, labelValues)) } - for _, lp := range desc.labelPairs { + for _, n := range desc.variableLabels.names { + if n == bucketLabel { + panic(errBucketLabelNotAllowed) + } + } + + for _, lp := range desc.constLabelPairs { if lp.GetName() == bucketLabel { panic(errBucketLabelNotAllowed) } diff --git a/prometheus/registry.go b/prometheus/registry.go index 27402266b..3bc546605 100644 --- a/prometheus/registry.go +++ b/prometheus/registry.go @@ -962,20 +962,31 @@ func checkDescConsistency( } // Is the desc consistent with the content of the metric? - if len(desc.labelPairs) != len(dtoMetric.Label) { + if len(desc.orderedLabels) != len(dtoMetric.Label) { return fmt.Errorf( "labels in collected metric %s %s are inconsistent with descriptor %s", metricFamily.GetName(), dtoMetric, desc, ) } - for i, lpFromDesc := range desc.labelPairs { + for i, lm := range desc.orderedLabels { lpFromMetric := dtoMetric.Label[i] - if lpFromDesc.GetName() != lpFromMetric.GetName() || - lpFromDesc.Value != nil && lpFromDesc.GetValue() != lpFromMetric.GetValue() { - return fmt.Errorf( - "labels in collected metric %s %s are inconsistent with descriptor %s", - metricFamily.GetName(), dtoMetric, desc, - ) + if lm.constLabelIndex > -1 { + lpFromDesc := desc.constLabelPairs[lm.constLabelIndex] + if lpFromDesc.GetName() != lpFromMetric.GetName() || + lpFromDesc.Value != nil && lpFromDesc.GetValue() != lpFromMetric.GetValue() { + return fmt.Errorf( + "labels in collected metric %s %s are inconsistent with descriptor %s", + metricFamily.GetName(), dtoMetric, desc, + ) + } + } else { + variableLabelName := *lm.variableLabelName + if variableLabelName != lpFromMetric.GetName() { + return fmt.Errorf( + "labels in collected metric %s %s are inconsistent with descriptor %s", + metricFamily.GetName(), dtoMetric, desc, + ) + } } } return nil diff --git a/prometheus/summary.go b/prometheus/summary.go index 3d3263d91..fb07ca6b3 100644 --- a/prometheus/summary.go +++ b/prometheus/summary.go @@ -196,7 +196,13 @@ func newSummary(desc *Desc, opts SummaryOpts, labelValues ...string) Summary { panic(makeInconsistentCardinalityError(desc.fqName, desc.variableLabels.names, labelValues)) } - for _, lp := range desc.labelPairs { + for _, n := range desc.variableLabels.names { + if n == quantileLabel { + panic(errQuantileLabelNotAllowed) + } + } + + for _, lp := range desc.constLabelPairs { if lp.GetName() == quantileLabel { panic(errQuantileLabelNotAllowed) } diff --git a/prometheus/value.go b/prometheus/value.go index 4af993cf2..e240ef8b4 100644 --- a/prometheus/value.go +++ b/prometheus/value.go @@ -212,31 +212,25 @@ func populateMetric( // This function is only needed for custom Metric implementations. See MetricVec // example. func MakeLabelPairs(desc *Desc, labelValues []string) []*dto.LabelPair { - if len(desc.labelPairs) == 0 { + if len(desc.orderedLabels) == 0 { // Super fast path. return nil } if len(desc.variableLabels.names) == 0 { // Moderately fast path. - return desc.labelPairs + return desc.constLabelPairs } - labelPairs := make([]*dto.LabelPair, 0, len(desc.labelPairs)) - for _, 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 { - labelToAdd = &dto.LabelPair{ - Name: lp.Name, - } + labelPairs := make([]*dto.LabelPair, len(desc.orderedLabels)) + for i, lm := range desc.orderedLabels { + if lm.constLabelIndex != -1 { + labelPairs[i] = desc.constLabelPairs[lm.constLabelIndex] } else { - labelToAdd = lp + labelPairs[i] = &dto.LabelPair{ + Name: lm.variableLabelName, + Value: proto.String(labelValues[lm.variableLabelIndex]), + } } - labelPairs = append(labelPairs, labelToAdd) } - for i, outputIndex := range desc.variableLabelOrder { - labelPairs[outputIndex].Value = proto.String(labelValues[i]) - } - return labelPairs } diff --git a/prometheus/value_test.go b/prometheus/value_test.go index ba4787ab5..e6ad18ce6 100644 --- a/prometheus/value_test.go +++ b/prometheus/value_test.go @@ -236,6 +236,8 @@ func BenchmarkMakeLabelPairs(b *testing.B) { }, } { b.Run(fmt.Sprintf("labels=%v", len(bm.makeLabelPairValues)), func(b *testing.B) { + b.ReportAllocs() + b.ResetTimer() for i := 0; i < b.N; i++ { MakeLabelPairs(bm.desc, bm.makeLabelPairValues) } @@ -264,6 +266,8 @@ func BenchmarkConstMetricFlow(b *testing.B) { b.Run(fmt.Sprintf("labels=%v", len(bm.labelValues)), func(b *testing.B) { for _, metricsToCreate := range []int{1, 2, 3, 5} { b.Run(fmt.Sprintf("metrics=%v", metricsToCreate), func(b *testing.B) { + b.ReportAllocs() + b.ResetTimer() for i := 0; i < b.N; i++ { desc := bm.descFunc() for j := 0; j < metricsToCreate; j++ { diff --git a/prometheus/wrap.go b/prometheus/wrap.go index 6afec9c26..43931cdaa 100644 --- a/prometheus/wrap.go +++ b/prometheus/wrap.go @@ -188,20 +188,18 @@ func (m *wrappingMetric) Write(out *dto.Metric) error { func wrapDesc(desc *Desc, prefix string, labels Labels) *Desc { constLabels := Labels{} - for _, lp := range desc.labelPairs { - // Variable labels have no values - if lp.Value != nil { - constLabels[*lp.Name] = *lp.Value - } + for _, lp := range desc.constLabelPairs { + 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, - labelPairs: desc.labelPairs, - err: fmt.Errorf("attempted wrapping with already existing label name %q", ln), + fqName: desc.fqName, + help: desc.help, + variableLabels: desc.variableLabels, + constLabelPairs: desc.constLabelPairs, + orderedLabels: desc.orderedLabels, + err: fmt.Errorf("attempted wrapping with already existing label name %q", ln), } } constLabels[ln] = lv From e87a0ad8faf6e2f81dc56d6b89453756075a1245 Mon Sep 17 00:00:00 2001 From: Kyle Eckhart Date: Thu, 6 Mar 2025 14:46:30 -0500 Subject: [PATCH 4/4] Cleanup uneeded diff and add a comment to orderedLabels Signed-off-by: Kyle Eckhart --- prometheus/desc.go | 3 ++- prometheus/histogram.go | 1 - prometheus/summary.go | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/prometheus/desc.go b/prometheus/desc.go index 1696abd86..5e50cb2c0 100644 --- a/prometheus/desc.go +++ b/prometheus/desc.go @@ -51,7 +51,8 @@ type Desc struct { // constLabelPairs contains the sorted DTO label pairs based on the constant labels // and variable labels constLabelPairs []*dto.LabelPair - + // orderedLabels contains the sorted labels with necessary fields to construct the + // final label pairs when needed. orderedLabels []labelMapping // id is a hash of the values of the ConstLabels and fqName. This // must be unique among all registered descriptors and can therefore be diff --git a/prometheus/histogram.go b/prometheus/histogram.go index 380fce5d9..1a279035b 100644 --- a/prometheus/histogram.go +++ b/prometheus/histogram.go @@ -542,7 +542,6 @@ func newHistogram(desc *Desc, opts HistogramOpts, labelValues ...string) Histogr panic(errBucketLabelNotAllowed) } } - for _, lp := range desc.constLabelPairs { if lp.GetName() == bucketLabel { panic(errBucketLabelNotAllowed) diff --git a/prometheus/summary.go b/prometheus/summary.go index fb07ca6b3..76a9e12f4 100644 --- a/prometheus/summary.go +++ b/prometheus/summary.go @@ -201,7 +201,6 @@ func newSummary(desc *Desc, opts SummaryOpts, labelValues ...string) Summary { panic(errQuantileLabelNotAllowed) } } - for _, lp := range desc.constLabelPairs { if lp.GetName() == quantileLabel { panic(errQuantileLabelNotAllowed)