From d049c340d9c4d061d6eae4af6a74ece8924f8c2d Mon Sep 17 00:00:00 2001 From: Kyle Eckhart Date: Wed, 12 Feb 2025 16:16:49 -0500 Subject: [PATCH 1/3] 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/3] 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 6ada5c22f007a80a43cef35989bc63205ed2a337 Mon Sep 17 00:00:00 2001 From: bwplotka Date: Thu, 20 Feb 2025 10:40:43 +0000 Subject: [PATCH 3/3] fastdto attempt. Signed-off-by: bwplotka --- prometheus/counter.go | 4 +- prometheus/desc.go | 30 ++++----- prometheus/desc_test.go | 14 +++++ prometheus/gauge.go | 4 +- prometheus/internal/fastdto/labels.go | 58 ++++++++++++++++++ prometheus/internal/fastdto/labels_test.go | 71 ++++++++++++++++++++++ prometheus/registry.go | 2 +- prometheus/value.go | 26 +++----- prometheus/value_test.go | 14 +++++ prometheus/wrap.go | 4 +- 10 files changed, 188 insertions(+), 39 deletions(-) create mode 100644 prometheus/internal/fastdto/labels.go create mode 100644 prometheus/internal/fastdto/labels_test.go diff --git a/prometheus/counter.go b/prometheus/counter.go index e5c242254..5c64f92d6 100644 --- a/prometheus/counter.go +++ b/prometheus/counter.go @@ -19,6 +19,8 @@ import ( "sync/atomic" "time" + "github.com/prometheus/client_golang/prometheus/internal/fastdto" + dto "github.com/prometheus/client_model/go" "google.golang.org/protobuf/types/known/timestamppb" ) @@ -94,7 +96,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: fastdto.ToDTOLabelPair(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 2f9e84879..009f9b296 100644 --- a/prometheus/desc.go +++ b/prometheus/desc.go @@ -18,12 +18,10 @@ import ( "sort" "strings" + "github.com/prometheus/client_golang/prometheus/internal/fastdto" + "github.com/cespare/xxhash/v2" - 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 @@ -56,7 +54,7 @@ type Desc struct { variableLabelOrder []int // labelPairs contains the sorted DTO label pairs based on the constant labels // and variable labels - labelPairs []*dto.LabelPair + labelPairs []fastdto.LabelPair // 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. @@ -164,24 +162,23 @@ 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)) + d.labelPairs = make([]fastdto.LabelPair, len(constLabels)+len(d.variableLabels.names)) + i := 0 for n, v := range constLabels { - d.labelPairs = append(d.labelPairs, &dto.LabelPair{ - Name: proto.String(n), - Value: proto.String(v), - }) + d.labelPairs[i].Name = n + d.labelPairs[i].Value = v + i++ } for _, labelName := range d.variableLabels.names { - d.labelPairs = append(d.labelPairs, &dto.LabelPair{ - Name: proto.String(labelName), - }) + d.labelPairs[i].Name = labelName + i++ } - sort.Sort(internal.LabelPairSorter(d.labelPairs)) + sort.Sort(fastdto.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 { + if pair.Value != "" { continue } for sourceIndex, variableLabel := range d.variableLabels.names { @@ -190,7 +187,6 @@ func (v2) NewDesc(fqName, help string, variableLabels ConstrainableLabels, const } } } - return d } @@ -207,7 +203,7 @@ 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 { + if lp.Value == "" { continue } lpStrings = append( diff --git a/prometheus/desc_test.go b/prometheus/desc_test.go index 0a797de39..531db2f1e 100644 --- a/prometheus/desc_test.go +++ b/prometheus/desc_test.go @@ -63,6 +63,18 @@ func TestNewInvalidDesc_String(t *testing.T) { } } +/* + export bench=newDesc && go test ./prometheus \ + -run '^$' -bench '^BenchmarkNewDesc/labels=10' \ + -benchtime 5s -benchmem -cpu 2 -timeout 999m \ + -memprofile=${bench}.mem.pprof \ + | tee ${bench}.txt + + export bench=newDesc-v2 && go test ./prometheus \ + -run '^$' -bench '^BenchmarkNewDesc' \ + -benchtime 5s -benchmem -count=6 -cpu 2 -timeout 999m \ + | tee ${bench}.txt +*/ func BenchmarkNewDesc(b *testing.B) { for _, bm := range []struct { labelCount int @@ -82,6 +94,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..7bc2c1503 100644 --- a/prometheus/gauge.go +++ b/prometheus/gauge.go @@ -18,6 +18,8 @@ import ( "sync/atomic" "time" + "github.com/prometheus/client_golang/prometheus/internal/fastdto" + dto "github.com/prometheus/client_model/go" ) @@ -82,7 +84,7 @@ func NewGauge(opts GaugeOpts) Gauge { nil, opts.ConstLabels, ) - result := &gauge{desc: desc, labelPairs: desc.labelPairs} + result := &gauge{desc: desc, labelPairs: fastdto.ToDTOLabelPair(desc.labelPairs)} result.init(result) // Init self-collection. return result } diff --git a/prometheus/internal/fastdto/labels.go b/prometheus/internal/fastdto/labels.go new file mode 100644 index 000000000..7c89c3f70 --- /dev/null +++ b/prometheus/internal/fastdto/labels.go @@ -0,0 +1,58 @@ +// Copyright 2025 The Prometheus Authors +// 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 fastdto + +import ( + dto "github.com/prometheus/client_model/go" +) + +type LabelPair struct { + Name string `protobuf:"bytes,1,opt,name=name" json:"name,omitempty"` + Value string `protobuf:"bytes,2,opt,name=value" json:"value,omitempty"` +} + +func (p LabelPair) GetName() string { + return p.Name +} + +func (p LabelPair) GetValue() string { + return p.Value +} + +// LabelPairSorter implements sort.Interface. It is used to sort a slice of +// LabelPairs +type LabelPairSorter []LabelPair + +func (s LabelPairSorter) Len() int { + return len(s) +} + +func (s LabelPairSorter) Swap(i, j int) { + s[i], s[j] = s[j], s[i] +} + +func (s LabelPairSorter) Less(i, j int) bool { + return s[i].Name < s[j].Name +} + +func ToDTOLabelPair(in []LabelPair) []*dto.LabelPair { + ret := make([]*dto.LabelPair, len(in)) + for i := range in { + ret[i] = &dto.LabelPair{ + Name: &(in[i].Name), + Value: &(in[i].Value), + } + } + return ret +} diff --git a/prometheus/internal/fastdto/labels_test.go b/prometheus/internal/fastdto/labels_test.go new file mode 100644 index 000000000..1a710b8d5 --- /dev/null +++ b/prometheus/internal/fastdto/labels_test.go @@ -0,0 +1,71 @@ +// Copyright 2025 The Prometheus Authors +// 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 fastdto + +import ( + "sort" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + dto "github.com/prometheus/client_model/go" + "google.golang.org/protobuf/proto" +) + +func BenchmarkToDTOLabelPairs(b *testing.B) { + test := []LabelPair{ + {"foo", "bar"}, + {"foo2", "bar2"}, + {"foo3", "bar3"}, + } + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + _ = ToDTOLabelPair(test) + } +} + +func TestLabelPairSorter(t *testing.T) { + test := []LabelPair{ + {"foo3", "bar3"}, + {"foo", "bar"}, + {"foo2", "bar2"}, + } + sort.Sort(LabelPairSorter(test)) + + expected := []LabelPair{ + {"foo", "bar"}, + {"foo2", "bar2"}, + {"foo3", "bar3"}, + } + if diff := cmp.Diff(test, expected); diff != "" { + t.Fatal(diff) + } +} + +func TestToDTOLabelPair(t *testing.T) { + test := []LabelPair{ + {"foo", "bar"}, + {"foo2", "bar2"}, + {"foo3", "bar3"}, + } + expected := []*dto.LabelPair{ + {Name: proto.String("foo"), Value: proto.String("bar")}, + {Name: proto.String("foo2"), Value: proto.String("bar2")}, + {Name: proto.String("foo3"), Value: proto.String("bar3")}, + } + if diff := cmp.Diff(ToDTOLabelPair(test), expected, cmpopts.IgnoreUnexported(dto.LabelPair{})); diff != "" { + t.Fatal(diff) + } +} diff --git a/prometheus/registry.go b/prometheus/registry.go index 27402266b..edc133f32 100644 --- a/prometheus/registry.go +++ b/prometheus/registry.go @@ -971,7 +971,7 @@ func checkDescConsistency( for i, lpFromDesc := range desc.labelPairs { lpFromMetric := dtoMetric.Label[i] if lpFromDesc.GetName() != lpFromMetric.GetName() || - lpFromDesc.Value != nil && lpFromDesc.GetValue() != lpFromMetric.GetValue() { + lpFromDesc.Value != "" && lpFromDesc.GetValue() != lpFromMetric.GetValue() { return fmt.Errorf( "labels in collected metric %s %s are inconsistent with descriptor %s", metricFamily.GetName(), dtoMetric, desc, diff --git a/prometheus/value.go b/prometheus/value.go index 4af993cf2..ce58468fd 100644 --- a/prometheus/value.go +++ b/prometheus/value.go @@ -19,6 +19,8 @@ import ( "time" "unicode/utf8" + "github.com/prometheus/client_golang/prometheus/internal/fastdto" + dto "github.com/prometheus/client_model/go" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/timestamppb" @@ -216,28 +218,18 @@ func MakeLabelPairs(desc *Desc, labelValues []string) []*dto.LabelPair { // Super fast path. return nil } + + // Conversion copies all but strings. + ret := fastdto.ToDTOLabelPair(desc.labelPairs) if len(desc.variableLabels.names) == 0 { // Moderately fast path. - return desc.labelPairs - } - 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, - } - } else { - labelToAdd = lp - } - labelPairs = append(labelPairs, labelToAdd) + return ret } + for i, outputIndex := range desc.variableLabelOrder { - labelPairs[outputIndex].Value = proto.String(labelValues[i]) + ret[outputIndex].Value = &labelValues[i] // Reusing string, assuming it's safe. } - - return labelPairs + return ret } // ExemplarMaxRunes is the max total number of runes allowed in exemplar labels. diff --git a/prometheus/value_test.go b/prometheus/value_test.go index ba4787ab5..135b8c29a 100644 --- a/prometheus/value_test.go +++ b/prometheus/value_test.go @@ -217,6 +217,18 @@ var new10LabelsDescFunc = func() *Desc { 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"}) } +/* + export bench=makeLabelPairs-v2 && go test ./prometheus \ + -run '^$' -bench '^BenchmarkMakeLabelPairs' \ + -benchtime 5s -benchmem -count=6 -cpu 2 -timeout 999m \ + | tee ${bench}.txt + + export bench=makeLabelPairs-v2pp && go test ./prometheus \ + -run '^$' -bench '^BenchmarkMakeLabelPairs' \ + -benchtime 5s -benchmem -cpu 2 -timeout 999m \ + -memprofile=${bench}.mem.pprof \ + | tee ${bench}.txt +*/ func BenchmarkMakeLabelPairs(b *testing.B) { for _, bm := range []struct { desc *Desc @@ -236,6 +248,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) } diff --git a/prometheus/wrap.go b/prometheus/wrap.go index 6afec9c26..e89f51b51 100644 --- a/prometheus/wrap.go +++ b/prometheus/wrap.go @@ -190,8 +190,8 @@ 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 + if lp.Value == "" { + constLabels[lp.Name] = lp.Value } } for ln, lv := range labels {