Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Performance of MakeLabelPairs #1734

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion prometheus/counter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
46 changes: 38 additions & 8 deletions prometheus/desc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}

Expand All @@ -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()),
Expand Down
2 changes: 1 addition & 1 deletion prometheus/gauge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
7 changes: 1 addition & 6 deletions prometheus/histogram.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
12 changes: 2 additions & 10 deletions prometheus/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
7 changes: 1 addition & 6 deletions prometheus/summary.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
30 changes: 16 additions & 14 deletions prometheus/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down
131 changes: 131 additions & 0 deletions prometheus/value_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
}
})
}
}
17 changes: 10 additions & 7 deletions prometheus/wrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading