From 30948120dc46caf6d953ddc14c042bcca42499d3 Mon Sep 17 00:00:00 2001 From: beorn7 Date: Thu, 25 May 2023 13:58:22 +0200 Subject: [PATCH 1/4] histogram: expose bug in bucket key calculation The current code doesn't work fork negative schemas if the observed value should go into a bucket with a non-positive key. Signed-off-by: beorn7 --- prometheus/histogram_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/prometheus/histogram_test.go b/prometheus/histogram_test.go index 079866b1d..ddbe575b7 100644 --- a/prometheus/histogram_test.go +++ b/prometheus/histogram_test.go @@ -500,23 +500,26 @@ func TestNativeHistogram(t *testing.T) { { name: "factor 4 results in schema -1", observations: []float64{ + 0.0156251, 0.0625, // Bucket -2: (0.015625, 0.0625) + 0.1, 0.25, // Bucket -1: (0.0625, 0.25] 0.5, 1, // Bucket 0: (0.25, 1] 1.5, 2, 3, 3.5, // Bucket 1: (1, 4] 5, 6, 7, // Bucket 2: (4, 16] 33.33, // Bucket 3: (16, 64] }, factor: 4, - want: `sample_count:10 sample_sum:62.83 schema:-1 zero_threshold:2.938735877055719e-39 zero_count:0 positive_span: positive_delta:2 positive_delta:2 positive_delta:-1 positive_delta:-2 `, + want: `sample_count:14 sample_sum:63.2581251 schema:-1 zero_threshold:2.938735877055719e-39 zero_count:0 positive_span: positive_delta:2 positive_delta:0 positive_delta:0 positive_delta:2 positive_delta:-1 positive_delta:-2 `, }, { name: "factor 17 results in schema -2", observations: []float64{ + 0.0156251, 0.0625, 0.1, 0.25, // Bucket -1: (0.015625, 0.25] 0.5, 1, // Bucket 0: (0.0625, 1] 1.5, 2, 3, 3.5, 5, 6, 7, // Bucket 1: (1, 16] 33.33, // Bucket 2: (16, 256] }, factor: 17, - want: `sample_count:10 sample_sum:62.83 schema:-2 zero_threshold:2.938735877055719e-39 zero_count:0 positive_span: positive_delta:2 positive_delta:5 positive_delta:-6 `, + want: `sample_count:14 sample_sum:63.2581251 schema:-2 zero_threshold:2.938735877055719e-39 zero_count:0 positive_span: positive_delta:4 positive_delta:-2 positive_delta:5 positive_delta:-6 `, }, { name: "negative buckets", From 77e97da564bd65455499c35b20e23dfa144d6925 Mon Sep 17 00:00:00 2001 From: beorn7 Date: Thu, 25 May 2023 19:03:43 +0200 Subject: [PATCH 2/4] histogram: Fix bug in bucket key calculation The current code doesn't work fork negative schemas if the observed value should go into a bucket with a non-positive key. Signed-off-by: beorn7 --- prometheus/histogram.go | 4 ++-- prometheus/histogram_test.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/prometheus/histogram.go b/prometheus/histogram.go index 5b69965b2..8a412260d 100644 --- a/prometheus/histogram.go +++ b/prometheus/histogram.go @@ -639,8 +639,8 @@ func (hc *histogramCounts) observe(v float64, bucket int, doSparse bool) { if frac == 0.5 { key-- } - div := 1 << -schema - key = (key + div - 1) / div + offset := (1 << -schema) - 1 + key = (key + offset) >> -schema } if isInf { key++ diff --git a/prometheus/histogram_test.go b/prometheus/histogram_test.go index ddbe575b7..6357c231d 100644 --- a/prometheus/histogram_test.go +++ b/prometheus/histogram_test.go @@ -513,13 +513,13 @@ func TestNativeHistogram(t *testing.T) { { name: "factor 17 results in schema -2", observations: []float64{ - 0.0156251, 0.0625, 0.1, 0.25, // Bucket -1: (0.015625, 0.25] - 0.5, 1, // Bucket 0: (0.0625, 1] + 0.0156251, 0.0625, // Bucket -1: (0.015625, 0.0625] + 0.1, 0.25, 0.5, 1, // Bucket 0: (0.0625, 1] 1.5, 2, 3, 3.5, 5, 6, 7, // Bucket 1: (1, 16] 33.33, // Bucket 2: (16, 256] }, factor: 17, - want: `sample_count:14 sample_sum:63.2581251 schema:-2 zero_threshold:2.938735877055719e-39 zero_count:0 positive_span: positive_delta:4 positive_delta:-2 positive_delta:5 positive_delta:-6 `, + want: `sample_count:14 sample_sum:63.2581251 schema:-2 zero_threshold:2.938735877055719e-39 zero_count:0 positive_span: positive_delta:2 positive_delta:2 positive_delta:3 positive_delta:-6 `, }, { name: "negative buckets", From 8840afcfc2c3ff3d40357552dbc1d9d43c4bae67 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 7 Jun 2023 08:12:43 +0100 Subject: [PATCH 3/4] Bump github.com/prometheus/procfs from 0.9.0 to 0.10.1 (#1283) Bumps [github.com/prometheus/procfs](https://github.com/prometheus/procfs) from 0.9.0 to 0.10.1. - [Release notes](https://github.com/prometheus/procfs/releases) - [Commits](https://github.com/prometheus/procfs/compare/v0.9.0...v0.10.1) --- updated-dependencies: - dependency-name: github.com/prometheus/procfs dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- go.mod | 4 ++-- go.sum | 11 +++++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/go.mod b/go.mod index 6721705a4..8873a995c 100644 --- a/go.mod +++ b/go.mod @@ -9,8 +9,8 @@ require ( github.com/json-iterator/go v1.1.12 github.com/prometheus/client_model v0.3.0 github.com/prometheus/common v0.42.0 - github.com/prometheus/procfs v0.9.0 - golang.org/x/sys v0.7.0 + github.com/prometheus/procfs v0.10.1 + golang.org/x/sys v0.8.0 google.golang.org/protobuf v1.30.0 ) diff --git a/go.sum b/go.sum index 327eaccd7..526afc2bd 100644 --- a/go.sum +++ b/go.sum @@ -157,8 +157,8 @@ github.com/prometheus/common v0.37.0/go.mod h1:phzohg0JFMnBEFGxTDbfu3QyL5GI8gTQJ github.com/prometheus/common v0.42.0 h1:EKsfXEYo4JpWMHH5cg+KOUWeuJSov1Id8zGR8eeI1YM= github.com/prometheus/common v0.42.0/go.mod h1:xBwqVerjNdUDjgODMpudtOMwlOwf2SaTr1yjz4b7Zbc= github.com/prometheus/procfs v0.8.0/go.mod h1:z7EfXMXOkbkqb9IINtpCn86r/to3BnA0uaxHdg830/4= -github.com/prometheus/procfs v0.9.0 h1:wzCHvIvM5SxWqYvwgVL7yJY8Lz3PKn49KQtpgMYJfhI= -github.com/prometheus/procfs v0.9.0/go.mod h1:+pB4zwohETzFnmlpe6yd2lSc+0/46IYZRB/chUwxUZY= +github.com/prometheus/procfs v0.10.1 h1:kYK1Va/YMlutzCGazswoHKo//tZVlFpKYh+PymziUAg= +github.com/prometheus/procfs v0.10.1/go.mod h1:nwNm2aOCAYw8uTR/9bWRREkZFxAUcWzPHWJq+XBB/FM= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8= github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= @@ -268,7 +268,7 @@ golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220601150217-0de741cfad7f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.2.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -299,10 +299,9 @@ golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.7.0 h1:3jlCCIQZPdOYu1h8BkNvLz8Kgwtae2cagcG/VamtZRU= -golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.8.0 h1:EBmGv8NaZBZTWvrbjNoL6HVt+IVy3QDQpJs7VRIw3tU= +golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= From a09a1d34cbc74daa8ed70234b99467a30b020a40 Mon Sep 17 00:00:00 2001 From: Bulat Khasanov Date: Wed, 7 Jun 2023 10:39:02 +0300 Subject: [PATCH 4/4] Reduce constrainLabels allocations (#1272) * Add bench Signed-off-by: Bulat Khasanov * Reduce constrainLabels allocations Signed-off-by: Bulat Khasanov --------- Signed-off-by: Bulat Khasanov --- prometheus/vec.go | 35 ++++++++++++++++++++++++++++++----- prometheus/vec_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/prometheus/vec.go b/prometheus/vec.go index 386fb2d23..f0d0015a0 100644 --- a/prometheus/vec.go +++ b/prometheus/vec.go @@ -20,6 +20,24 @@ import ( "github.com/prometheus/common/model" ) +var labelsPool = &sync.Pool{ + New: func() interface{} { + return make(Labels) + }, +} + +func getLabelsFromPool() Labels { + return labelsPool.Get().(Labels) +} + +func putLabelsToPool(labels Labels) { + for k := range labels { + delete(labels, k) + } + + labelsPool.Put(labels) +} + // MetricVec is a Collector to bundle metrics of the same name that differ in // their label values. MetricVec is not used directly but as a building block // for implementations of vectors of a given metric type, like GaugeVec, @@ -93,6 +111,8 @@ func (m *MetricVec) DeleteLabelValues(lvs ...string) bool { // there for pros and cons of the two methods. func (m *MetricVec) Delete(labels Labels) bool { labels = constrainLabels(m.desc, labels) + defer putLabelsToPool(labels) + h, err := m.hashLabels(labels) if err != nil { return false @@ -109,6 +129,8 @@ func (m *MetricVec) Delete(labels Labels) bool { // To match curried labels with DeletePartialMatch, it must be called on the base vector. func (m *MetricVec) DeletePartialMatch(labels Labels) int { labels = constrainLabels(m.desc, labels) + defer putLabelsToPool(labels) + return m.metricMap.deleteByLabels(labels, m.curry) } @@ -229,6 +251,8 @@ func (m *MetricVec) GetMetricWithLabelValues(lvs ...string) (Metric, error) { // for example GaugeVec. func (m *MetricVec) GetMetricWith(labels Labels) (Metric, error) { labels = constrainLabels(m.desc, labels) + defer putLabelsToPool(labels) + h, err := m.hashLabels(labels) if err != nil { return nil, err @@ -647,15 +671,16 @@ func inlineLabelValues(lvs []string, curry []curriedLabelValue) []string { } func constrainLabels(desc *Desc, labels Labels) Labels { - constrainedValues := make(Labels, len(labels)) + constrainedLabels := getLabelsFromPool() for l, v := range labels { if i, ok := indexOf(l, desc.variableLabels.labelNames()); ok { - constrainedValues[l] = desc.variableLabels[i].Constrain(v) - continue + v = desc.variableLabels[i].Constrain(v) } - constrainedValues[l] = v + + constrainedLabels[l] = v } - return constrainedValues + + return constrainedLabels } func constrainLabelValues(desc *Desc, lvs []string, curry []curriedLabelValue) []string { diff --git a/prometheus/vec_test.go b/prometheus/vec_test.go index 63f52af23..89bd85429 100644 --- a/prometheus/vec_test.go +++ b/prometheus/vec_test.go @@ -904,6 +904,13 @@ func testConstrainedCurryVec(t *testing.T, vec *CounterVec, constraint func(stri }) } +func BenchmarkMetricVecWithBasic(b *testing.B) { + benchmarkMetricVecWith(b, Labels{ + "l1": "onevalue", + "l2": "twovalue", + }) +} + func BenchmarkMetricVecWithLabelValuesBasic(b *testing.B) { benchmarkMetricVecWithLabelValues(b, map[string][]string{ "l1": {"onevalue"}, @@ -948,6 +955,27 @@ func benchmarkMetricVecWithLabelValuesCardinality(b *testing.B, nkeys, nvalues i benchmarkMetricVecWithLabelValues(b, labels) } +func benchmarkMetricVecWith(b *testing.B, labels map[string]string) { + var keys []string + for k := range labels { + keys = append(keys, k) + } + + vec := NewGaugeVec( + GaugeOpts{ + Name: "test", + Help: "helpless", + }, + keys, + ) + + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + vec.With(labels) + } +} + func benchmarkMetricVecWithLabelValues(b *testing.B, labels map[string][]string) { var keys []string for k := range labels { // Map order dependent, who cares though.