From 8cc2b6c472a5d4cfc3d9fe37c5a23f7239faf707 Mon Sep 17 00:00:00 2001 From: Dave Henderson Date: Mon, 22 Aug 2022 01:28:46 -0400 Subject: [PATCH 1/5] Fix double-counting bug in promhttp.InstrumentRoundTripperCounter (#1118) Signed-off-by: Dave Henderson Signed-off-by: Dave Henderson --- prometheus/promhttp/instrument_client.go | 1 - prometheus/promhttp/instrument_client_test.go | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/prometheus/promhttp/instrument_client.go b/prometheus/promhttp/instrument_client.go index 097aff2df..57bb5f945 100644 --- a/prometheus/promhttp/instrument_client.go +++ b/prometheus/promhttp/instrument_client.go @@ -78,7 +78,6 @@ func InstrumentRoundTripperCounter(counter *prometheus.CounterVec, next http.Rou 1, rtOpts.getExemplarFn(r.Context()), ) - counter.With(labels(code, method, r.Method, resp.StatusCode, rtOpts.extraMethods...)).Inc() } return resp, err } diff --git a/prometheus/promhttp/instrument_client_test.go b/prometheus/promhttp/instrument_client_test.go index 98667e8f1..ce7c4da54 100644 --- a/prometheus/promhttp/instrument_client_test.go +++ b/prometheus/promhttp/instrument_client_test.go @@ -25,6 +25,7 @@ import ( "time" "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/testutil" dto "github.com/prometheus/client_model/go" "google.golang.org/protobuf/proto" @@ -250,6 +251,19 @@ func TestClientMiddlewareAPI_WithRequestContext(t *testing.T) { t.Errorf("metric family %s must not be empty", mf.GetName()) } } + + // make sure counters aren't double-incremented (see #1117) + expected := ` + # HELP client_api_requests_total A counter for requests from the wrapped client. + # TYPE client_api_requests_total counter + client_api_requests_total{code="200",method="get"} 1 + ` + + if err := testutil.GatherAndCompare(reg, strings.NewReader(expected), + "client_api_requests_total", + ); err != nil { + t.Fatal(err) + } } func TestClientMiddlewareAPIWithRequestContextTimeout(t *testing.T) { From 1f93f64580770181b19e685e1a243923fb61d476 Mon Sep 17 00:00:00 2001 From: Balint Zsilavecz Date: Thu, 13 Oct 2022 12:52:19 +0100 Subject: [PATCH 2/5] 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 0d47fecdc..73e814a4d 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 f0941f6f0..b5119c504 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 2d69b08f9..dd7d84301 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 ddd7f0edcd31dd27b31ee9c54b5c22d44258d5d1 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 3/5] 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 de30de6da..3668a16b3 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) } From 078f11f85b2cb5d535f5856903e73b758a8f0568 Mon Sep 17 00:00:00 2001 From: bwplotka Date: Tue, 1 Nov 2022 16:37:13 +0000 Subject: [PATCH 4/5] Cut 1.13.1 release (+ documenting release process). Signed-off-by: bwplotka --- CHANGELOG.md | 6 ++++++ README.md | 19 +++++++++++++++++++ VERSION | 2 +- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index faccd3d28..14ae670f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ ## Unreleased +## 1.13.1 / 2022-11-01 + +* [BUGFIX] Fix race condition with Exemplar in Counter. #1146 +* [BUGFIX] Fix `CumulativeCount` value of `+Inf` bucket created from exemplar. #1148 +* [BUGFIX] Fix double-counting bug in `promhttp.InstrumentRoundTripperCounter`. #1118 + ## 1.13.0 / 2022-08-05 * [CHANGE] Minimum required Go version is now 1.17 (we also test client_golang against new 1.19 version). diff --git a/README.md b/README.md index 40c61468e..af6889e30 100644 --- a/README.md +++ b/README.md @@ -69,3 +69,22 @@ See the [contributing guidelines](CONTRIBUTING.md) and the [Community section](http://prometheus.io/community/) of the homepage. `clint_golang` community is also present on the CNCF Slack `#prometheus-client_golang`. + +### For Maintainers: Release Process + +To cut minor version: + +1. Create new branch `release-.` on top of main commit you want to cut version from and push it. +2. Create new branch on top of release branch. +3. Change `VERSION` file. +4. Update `CHANGELOG` (only user-impacting changes to mention). +5. Create PR, get it reviewed. +6. Once merged, create release for `release-.` tag on GitHub with ` / ` title. +7. Announce on prometheus-announce mailing list, slack and Twitter. +8. Merge release branch back to main using "merge without squashing" approach (!). + +To cut patch version: + +1. Create branch on top of release branch you want to use. +2. Cherry-pick fixes from main or add commits to fix critical bugs only for that patch release. +3. Follow steps 3-8 as above. diff --git a/VERSION b/VERSION index feaae22ba..b50dd27dd 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.13.0 +1.13.1 From 79ca0eb2ba90a9c1754d29177d0bfe3afb425449 Mon Sep 17 00:00:00 2001 From: bwplotka Date: Tue, 1 Nov 2022 16:54:19 +0000 Subject: [PATCH 5/5] =?UTF-8?q?Added=20tip=20from=20Bj=C3=B6rn=20+=20Gramm?= =?UTF-8?q?arly.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: bwplotka --- README.md | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index af6889e30..ab2653dd1 100644 --- a/README.md +++ b/README.md @@ -72,19 +72,21 @@ See the [contributing guidelines](CONTRIBUTING.md) and the ### For Maintainers: Release Process -To cut minor version: +To cut a minor version: -1. Create new branch `release-.` on top of main commit you want to cut version from and push it. -2. Create new branch on top of release branch. -3. Change `VERSION` file. +1. Create a new branch `release-.` on top of the `main` commit you want to cut the version from and push it. +2. Create a new branch on top of the release branch, e.g. `/cut-..`, +3. Change the `VERSION` file. 4. Update `CHANGELOG` (only user-impacting changes to mention). -5. Create PR, get it reviewed. -6. Once merged, create release for `release-.` tag on GitHub with ` / ` title. -7. Announce on prometheus-announce mailing list, slack and Twitter. -8. Merge release branch back to main using "merge without squashing" approach (!). +5. Create PR, and get it reviewed. +6. Once merged, create a release with the `release-.` tag on GitHub with the ` / ` title. +7. Announce on the prometheus-announce mailing list, slack and Twitter. +8. Merge the release branch back to the `main` using the "merge without squashing" approach (!). -To cut patch version: +> NOTE: In case of merge conflicts, you can checkout the release branch in a new branch, e.g. /resolve-conflicts`, fix the merge problems there, and then do a PR into main from the new branch. In that way, you still get all the commits in the release branch back into `main`, but leave the release branch alone. -1. Create branch on top of release branch you want to use. -2. Cherry-pick fixes from main or add commits to fix critical bugs only for that patch release. +To cut the patch version: + +1. Create a branch on top of the release branch you want to use. +2. Cherry-pick the fixes from the `main` branch (or add new commits) to fix critical bugs for that patch release. 3. Follow steps 3-8 as above.