From 7c46c150bd3f2a4b7a1a484e86abb1c20ae1d3bc Mon Sep 17 00:00:00 2001 From: Rafael Franco <6237457+donotnoot@users.noreply.github.com> Date: Mon, 12 Sep 2022 10:17:56 +0100 Subject: [PATCH 1/4] Clarify documentation around what constructors do (#1125) The wording of the documentation is slightly misleading. Before this commit, it says that the returned collectors are "already registered". This could be interpreted in two ways, one could think that promauto keeps some sort of cache of already registered Collectors that are returned by this package, and the other way is that the Collectors constructed are registered before being returned. What is actually happening is the latter, and the wording after this PR leaves no room to think that the former could be the case. Signed-off-by: Rafael Franco Signed-off-by: Rafael Franco --- prometheus/promauto/auto.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/prometheus/promauto/auto.go b/prometheus/promauto/auto.go index f8d50d1..a9c3108 100644 --- a/prometheus/promauto/auto.go +++ b/prometheus/promauto/auto.go @@ -14,13 +14,13 @@ // Package promauto provides alternative constructors for the fundamental // Prometheus metric types and their …Vec and …Func variants. The difference to // their counterparts in the prometheus package is that the promauto -// constructors return Collectors that are already registered with a -// registry. There are two sets of constructors. The constructors in the first -// set are top-level functions, while the constructors in the other set are -// methods of the Factory type. The top-level function return Collectors -// registered with the global registry (prometheus.DefaultRegisterer), while the -// methods return Collectors registered with the registry the Factory was -// constructed with. All constructors panic if the registration fails. +// constructors register the Collectors with a registry before returning them. +// There are two sets of constructors. The constructors in the first set are +// top-level functions, while the constructors in the other set are methods of +// the Factory type. The top-level function return Collectors registered with +// the global registry (prometheus.DefaultRegisterer), while the methods return +// Collectors registered with the registry the Factory was constructed with. All +// constructors panic if the registration fails. // // The following example is a complete program to create a histogram of normally // distributed random numbers from the math/rand package: From 9801a4e3ceb49d24dfaf4fa6fc8c2d58aa3b2dc9 Mon Sep 17 00:00:00 2001 From: rogerogers <40619032+rogerogers@users.noreply.github.com> Date: Mon, 12 Sep 2022 17:24:20 +0800 Subject: [PATCH 2/4] Examples: Replace deprecated WithGoCollections with WithGoCollectorRuntimeMetrics (#1130) Signed-off-by: rogerogers Signed-off-by: rogerogers --- examples/gocollector/main.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/examples/gocollector/main.go b/examples/gocollector/main.go index 93f8c22..9116314 100644 --- a/examples/gocollector/main.go +++ b/examples/gocollector/main.go @@ -22,6 +22,7 @@ import ( "fmt" "log" "net/http" + "regexp" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/collectors" @@ -39,7 +40,7 @@ func main() { // Add Go module build info. reg.MustRegister(collectors.NewBuildInfoCollector()) reg.MustRegister(collectors.NewGoCollector( - collectors.WithGoCollections(collectors.GoRuntimeMemStatsCollection | collectors.GoRuntimeMetricsCollection), + collectors.WithGoCollectorRuntimeMetrics(collectors.GoRuntimeMetricsRule{Matcher: regexp.MustCompile("/.*")}), )) // Expose the registered metrics via HTTP. From dcea97eee2b3257f34fd3203cb922eedeabb42a6 Mon Sep 17 00:00:00 2001 From: Balint Zsilavecz Date: Thu, 13 Oct 2022 12:52:19 +0100 Subject: [PATCH 3/4] Fix `CumulativeCount` value of `+Inf` bucket created from exemplar (#1148) * Fix `CumulativeCount` value of `+Inf` bucket created from exemplar Signed-off-by: Balint Zsilavecz * Update prometheus/metric_test.go Co-authored-by: Bartlomiej Plotka Signed-off-by: Balint Zsilavecz * Clarify description of implicit `+Inf` bucket count Signed-off-by: Balint Zsilavecz * Fix test variables Signed-off-by: Balint Zsilavecz Signed-off-by: Balint Zsilavecz Co-authored-by: Bartlomiej Plotka --- prometheus/histogram.go | 2 +- prometheus/metric.go | 2 +- prometheus/metric_test.go | 10 +++++++--- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/prometheus/histogram.go b/prometheus/histogram.go index 0d47fec..73e814a 100644 --- a/prometheus/histogram.go +++ b/prometheus/histogram.go @@ -613,7 +613,7 @@ func (h *constHistogram) Write(out *dto.Metric) error { // to send it to Prometheus in the Collect method. // // buckets is a map of upper bounds to cumulative counts, excluding the +Inf -// bucket. +// bucket. The +Inf bucket is implicit, and its value is equal to the provided count. // // NewConstHistogram returns an error if the length of labelValues is not // consistent with the variable labels in Desc or if Desc is invalid. diff --git a/prometheus/metric.go b/prometheus/metric.go index f0941f6..b5119c5 100644 --- a/prometheus/metric.go +++ b/prometheus/metric.go @@ -187,7 +187,7 @@ func (m *withExemplarsMetric) Write(pb *dto.Metric) error { } else { // The +Inf bucket should be explicitly added if there is an exemplar for it, similar to non-const histogram logic in https://github.com/prometheus/client_golang/blob/main/prometheus/histogram.go#L357-L365. b := &dto.Bucket{ - CumulativeCount: proto.Uint64(pb.Histogram.Bucket[len(pb.Histogram.GetBucket())-1].GetCumulativeCount()), + CumulativeCount: proto.Uint64(pb.Histogram.GetSampleCount()), UpperBound: proto.Float64(math.Inf(1)), Exemplar: e, } diff --git a/prometheus/metric_test.go b/prometheus/metric_test.go index 2d69b08..dd7d843 100644 --- a/prometheus/metric_test.go +++ b/prometheus/metric_test.go @@ -79,10 +79,14 @@ func TestWithExemplarsMetric(t *testing.T) { } } - infBucket := metric.GetHistogram().Bucket[len(metric.GetHistogram().Bucket)-1].GetUpperBound() + infBucket := metric.GetHistogram().Bucket[len(metric.GetHistogram().Bucket)-1] - if infBucket != math.Inf(1) { - t.Errorf("want %v, got %v", math.Inf(1), infBucket) + if want, got := math.Inf(1), infBucket.GetUpperBound(); want != got { + t.Errorf("want %v, got %v", want, got) + } + + if want, got := uint64(4711), infBucket.GetCumulativeCount(); want != got { + t.Errorf("want %v, got %v", want, got) } }) } From 10b0550932c38325813a7a52716064fdee176387 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20St=C3=A4ber?= Date: Mon, 17 Oct 2022 20:50:50 +0200 Subject: [PATCH 4/4] Fix race condition with Exemplar in Counter (#1146) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix race condition with Exemplar in Counter Potential fix for #1145. Signed-off-by: Fabian Stäber * Fix race condition with Exemplar in Counter Signed-off-by: Fabian Stäber Signed-off-by: Fabian Stäber --- prometheus/counter.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/prometheus/counter.go b/prometheus/counter.go index de30de6..3668a16 100644 --- a/prometheus/counter.go +++ b/prometheus/counter.go @@ -140,12 +140,13 @@ func (c *counter) get() float64 { } func (c *counter) Write(out *dto.Metric) error { - val := c.get() - + // Read the Exemplar first and the value second. This is to avoid a race condition + // where users see an exemplar for a not-yet-existing observation. var exemplar *dto.Exemplar if e := c.exemplar.Load(); e != nil { exemplar = e.(*dto.Exemplar) } + val := c.get() return populateMetric(CounterValue, val, c.labelPairs, exemplar, out) }