Merge pull request #1157 from prometheus/cut-1.13.1

Bugfixes + Cut 1.13.1 + documenting release process.

Author: kakkoyun

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).

README.md

@@ -69,3 +69,24 @@ 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 a minor version:
+
+1. Create a new branch `release-<major>.<minor>` 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. `<yourname>/cut-<major>.<minor>.<patch>`,
+3. Change the `VERSION` file.
+4. Update `CHANGELOG` (only user-impacting changes to mention).
+5. Create PR, and get it reviewed.
+6. Once merged, create a release with the `release-<major>.<minor>` tag on GitHub with the `<version> / <date>` 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 (!).
+
+> NOTE: In case of merge conflicts, you can checkout the release branch in a new branch, e.g. <yourname>/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.
+
+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.

VERSION

@@ -1 +1 @@
-1.13.0
+1.13.1

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)
 }

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.

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,
 				}

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)
 		}
 	})
 }

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
 	}

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) {