From 2ce58a71db3c8e2a278828c6f447037ca84f2a56 Mon Sep 17 00:00:00 2001 From: Kemal Akkoyun Date: Tue, 18 Jan 2022 14:50:02 +0100 Subject: [PATCH 01/14] Cut v1.12.0 Signed-off-by: Kemal Akkoyun --- CHANGELOG.md | 8 ++++++++ VERSION | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b5d774dce..116bd35a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +## 1.12.0 / 2022-01-18 + +* [CHANGE] example/random: Move flags and metrics into main() #935 +* [FEATURE] API client: Support wal replay status api #944 +* [FEATURE] Use the runtime/metrics package for the Go collector for 1.17+ #955 +* [ENHANCEMENT] API client: Update /api/v1/status/tsdb to include headStats #925 +* [ENHANCEMENT] promhttp: Check validity of method and code label values #962 + ## 1.11.0 / 2021-06-07 * [CHANGE] Add new collectors package. #862 diff --git a/VERSION b/VERSION index 1cac385c6..0eed1a29e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.11.0 +1.12.0 From 08a53e57a21a33e5cd952ce3d624d74b4cba9d10 Mon Sep 17 00:00:00 2001 From: Kemal Akkoyun Date: Wed, 19 Jan 2022 08:46:00 +0100 Subject: [PATCH 02/14] Bump the day Signed-off-by: Kemal Akkoyun --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 116bd35a4..274008214 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## 1.12.0 / 2022-01-18 +## 1.12.0 / 2022-01-19 * [CHANGE] example/random: Move flags and metrics into main() #935 * [FEATURE] API client: Support wal replay status api #944 From 772b89389c848c8e9559665e78690c965ec2437d Mon Sep 17 00:00:00 2001 From: Michael Knyszek Date: Fri, 21 Jan 2022 02:34:45 -0500 Subject: [PATCH 03/14] Make the Go 1.17 collector thread-safe (#969) --- prometheus/collector.go | 8 +++++++ prometheus/go_collector_go117.go | 32 +++++++++++++++++++++------ prometheus/go_collector_go117_test.go | 22 ++++++++++++++++++ 3 files changed, 55 insertions(+), 7 deletions(-) diff --git a/prometheus/collector.go b/prometheus/collector.go index 1e839650d..ac1ca3cf5 100644 --- a/prometheus/collector.go +++ b/prometheus/collector.go @@ -118,3 +118,11 @@ func (c *selfCollector) Describe(ch chan<- *Desc) { func (c *selfCollector) Collect(ch chan<- Metric) { ch <- c.self } + +// collectorMetric is a metric that is also a collector. +// Because of selfCollector, most (if not all) Metrics in +// this package are also collectors. +type collectorMetric interface { + Metric + Collector +} diff --git a/prometheus/go_collector_go117.go b/prometheus/go_collector_go117.go index d53474243..504684039 100644 --- a/prometheus/go_collector_go117.go +++ b/prometheus/go_collector_go117.go @@ -32,9 +32,10 @@ type goCollector struct { base baseGoCollector // rm... fields all pertain to the runtime/metrics package. + rmSampleMu sync.Mutex rmSampleBuf []metrics.Sample rmSampleMap map[string]*metrics.Sample - rmMetrics []Metric + rmMetrics []collectorMetric // With Go 1.17, the runtime/metrics package was introduced. // From that point on, metric names produced by the runtime/metrics @@ -58,7 +59,7 @@ func NewGoCollector() Collector { } // Generate a Desc and ValueType for each runtime/metrics metric. - metricSet := make([]Metric, 0, len(descriptions)) + metricSet := make([]collectorMetric, 0, len(descriptions)) sampleBuf := make([]metrics.Sample, 0, len(descriptions)) sampleMap := make(map[string]*metrics.Sample, len(descriptions)) for i := range descriptions { @@ -76,7 +77,7 @@ func NewGoCollector() Collector { sampleBuf = append(sampleBuf, metrics.Sample{Name: d.Name}) sampleMap[d.Name] = &sampleBuf[len(sampleBuf)-1] - var m Metric + var m collectorMetric if d.Kind == metrics.KindFloat64Histogram { _, hasSum := rmExactSumMap[d.Name] m = newBatchHistogram( @@ -130,9 +131,19 @@ func (c *goCollector) Collect(ch chan<- Metric) { // Collect base non-memory metrics. c.base.Collect(ch) + // Collect must be thread-safe, so prevent concurrent use of + // rmSampleBuf. Just read into rmSampleBuf but write all the data + // we get into our Metrics or MemStats. + // + // Note that we cannot simply read and then clone rmSampleBuf + // because we'd need to perform a deep clone of it, which is likely + // not worth it. + c.rmSampleMu.Lock() + // Populate runtime/metrics sample buffer. metrics.Read(c.rmSampleBuf) + // Update all our metrics from rmSampleBuf. for i, sample := range c.rmSampleBuf { // N.B. switch on concrete type because it's significantly more efficient // than checking for the Counter and Gauge interface implementations. In @@ -146,22 +157,29 @@ func (c *goCollector) Collect(ch chan<- Metric) { if v1 > v0 { m.Add(unwrapScalarRMValue(sample.Value) - m.get()) } - m.Collect(ch) case *gauge: m.Set(unwrapScalarRMValue(sample.Value)) - m.Collect(ch) case *batchHistogram: m.update(sample.Value.Float64Histogram(), c.exactSumFor(sample.Name)) - m.Collect(ch) default: panic("unexpected metric type") } } - // ms is a dummy MemStats that we populate ourselves so that we can // populate the old metrics from it. var ms runtime.MemStats memStatsFromRM(&ms, c.rmSampleMap) + + c.rmSampleMu.Unlock() + + // Export all the metrics to ch. + // At this point we must not access rmSampleBuf or rmSampleMap, because + // a concurrent caller could use it. It's safe to Collect all our Metrics, + // however, because they're updated in a thread-safe way while MemStats + // is local to this call of Collect. + for _, m := range c.rmMetrics { + m.Collect(ch) + } for _, i := range c.msMetrics { ch <- MustNewConstMetric(i.desc, i.valType, i.eval(&ms)) } diff --git a/prometheus/go_collector_go117_test.go b/prometheus/go_collector_go117_test.go index 653e332b9..e780bce4e 100644 --- a/prometheus/go_collector_go117_test.go +++ b/prometheus/go_collector_go117_test.go @@ -280,3 +280,25 @@ func TestExpectedRuntimeMetrics(t *testing.T) { t.Log("where X is the Go version you are currently using") } } + +func TestGoCollectorConcurrency(t *testing.T) { + c := NewGoCollector().(*goCollector) + + // Set up multiple goroutines to Collect from the + // same GoCollector. In race mode with GOMAXPROCS > 1, + // this test should fail often if Collect is not + // concurrent-safe. + for i := 0; i < 4; i++ { + go func() { + ch := make(chan Metric) + go func() { + // Drain all metrics recieved until the + // channel is closed. + for range ch { + } + }() + c.Collect(ch) + close(ch) + }() + } +} From d32edd60839f63fa0465ef1df751ea72b5326e13 Mon Sep 17 00:00:00 2001 From: Michael Knyszek Date: Tue, 25 Jan 2022 02:43:45 -0500 Subject: [PATCH 04/14] Use simpler locking in the Go 1.17 collector (#975) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A previous PR made it so that the Go 1.17 collector locked only around uses of rmSampleBuf, but really that means that Metric values may be sent over the channel containing some values from future metrics.Read calls. While generally-speaking this isn't a problem, we lose any consistency guarantees provided by the runtime/metrics package. Also, that optimization to not just lock around all of Collect was premature. Truthfully, Collect is called relatively infrequently, and its critical path is fairly fast (10s of µs). To prove it, this change also adds a benchmark. name old time/op new time/op delta GoCollector-16 43.7µs ± 2% 43.2µs ± 2% ~ (p=0.190 n=9+9) Note that because the benchmark is single-threaded it actually looks like it might be getting *slightly* faster, because all those Collect calls for the Metrics are direct calls instead of interface calls. Signed-off-by: Michael Anthony Knyszek --- prometheus/go_collector_go117.go | 33 ++++++++++++++------------- prometheus/go_collector_go117_test.go | 2 +- prometheus/go_collector_test.go | 17 ++++++++++++++ 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/prometheus/go_collector_go117.go b/prometheus/go_collector_go117.go index 504684039..a9b1fbaea 100644 --- a/prometheus/go_collector_go117.go +++ b/prometheus/go_collector_go117.go @@ -31,8 +31,11 @@ import ( type goCollector struct { base baseGoCollector + // mu protects updates to all fields ensuring a consistent + // snapshot is always produced by Collect. + mu sync.Mutex + // rm... fields all pertain to the runtime/metrics package. - rmSampleMu sync.Mutex rmSampleBuf []metrics.Sample rmSampleMap map[string]*metrics.Sample rmMetrics []collectorMetric @@ -135,10 +138,16 @@ func (c *goCollector) Collect(ch chan<- Metric) { // rmSampleBuf. Just read into rmSampleBuf but write all the data // we get into our Metrics or MemStats. // - // Note that we cannot simply read and then clone rmSampleBuf - // because we'd need to perform a deep clone of it, which is likely - // not worth it. - c.rmSampleMu.Lock() + // This lock also ensures that the Metrics we send out are all from + // the same updates, ensuring their mutual consistency insofar as + // is guaranteed by the runtime/metrics package. + // + // N.B. This locking is heavy-handed, but Collect is expected to be called + // relatively infrequently. Also the core operation here, metrics.Read, + // is fast (O(tens of microseconds)) so contention should certainly be + // low, though channel operations and any allocations may add to that. + c.mu.Lock() + defer c.mu.Unlock() // Populate runtime/metrics sample buffer. metrics.Read(c.rmSampleBuf) @@ -157,10 +166,13 @@ func (c *goCollector) Collect(ch chan<- Metric) { if v1 > v0 { m.Add(unwrapScalarRMValue(sample.Value) - m.get()) } + m.Collect(ch) case *gauge: m.Set(unwrapScalarRMValue(sample.Value)) + m.Collect(ch) case *batchHistogram: m.update(sample.Value.Float64Histogram(), c.exactSumFor(sample.Name)) + m.Collect(ch) default: panic("unexpected metric type") } @@ -169,17 +181,6 @@ func (c *goCollector) Collect(ch chan<- Metric) { // populate the old metrics from it. var ms runtime.MemStats memStatsFromRM(&ms, c.rmSampleMap) - - c.rmSampleMu.Unlock() - - // Export all the metrics to ch. - // At this point we must not access rmSampleBuf or rmSampleMap, because - // a concurrent caller could use it. It's safe to Collect all our Metrics, - // however, because they're updated in a thread-safe way while MemStats - // is local to this call of Collect. - for _, m := range c.rmMetrics { - m.Collect(ch) - } for _, i := range c.msMetrics { ch <- MustNewConstMetric(i.desc, i.valType, i.eval(&ms)) } diff --git a/prometheus/go_collector_go117_test.go b/prometheus/go_collector_go117_test.go index e780bce4e..fe715fc88 100644 --- a/prometheus/go_collector_go117_test.go +++ b/prometheus/go_collector_go117_test.go @@ -292,7 +292,7 @@ func TestGoCollectorConcurrency(t *testing.T) { go func() { ch := make(chan Metric) go func() { - // Drain all metrics recieved until the + // Drain all metrics received until the // channel is closed. for range ch { } diff --git a/prometheus/go_collector_test.go b/prometheus/go_collector_test.go index 9cc1b2e7f..47b944db5 100644 --- a/prometheus/go_collector_test.go +++ b/prometheus/go_collector_test.go @@ -154,3 +154,20 @@ func TestGoCollectorGC(t *testing.T) { break } } + +func BenchmarkGoCollector(b *testing.B) { + c := NewGoCollector().(*goCollector) + + b.ResetTimer() + for i := 0; i < b.N; i++ { + ch := make(chan Metric, 8) + go func() { + // Drain all metrics received until the + // channel is closed. + for range ch { + } + }() + c.Collect(ch) + close(ch) + } +} From 5a529ae06b6b2ccecd4c97092e2deda68877835e Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 25 Jan 2022 10:16:10 +0000 Subject: [PATCH 05/14] API client: make http reads more efficient (#976) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace `io.ReadAll` with `bytes.Buffer.ReadFrom`. Both need to resize a buffer until they have finished reading; the former increases by 1.25x each time while the latter uses 2x. Also added a benchmark to demonstrate the benefit: name old time/op new time/op delta Client/4KB-8 35.9µs ± 4% 35.3µs ± 3% ~ (p=0.310 n=5+5) Client/50KB-8 83.1µs ± 8% 69.5µs ± 1% -16.37% (p=0.008 n=5+5) Client/1000KB-8 891µs ± 6% 750µs ± 0% -15.83% (p=0.016 n=5+4) Client/2000KB-8 1.74ms ± 2% 1.35ms ± 1% -22.72% (p=0.008 n=5+5) name old alloc/op new alloc/op delta Client/4KB-8 20.2kB ± 0% 20.4kB ± 0% +1.26% (p=0.008 n=5+5) Client/50KB-8 218kB ± 0% 136kB ± 0% -37.65% (p=0.008 n=5+5) Client/1000KB-8 5.88MB ± 0% 2.11MB ± 0% -64.10% (p=0.008 n=5+5) Client/2000KB-8 11.7MB ± 0% 4.2MB ± 0% -63.93% (p=0.008 n=5+5) name old allocs/op new allocs/op delta Client/4KB-8 75.0 ± 0% 72.0 ± 0% -4.00% (p=0.008 n=5+5) Client/50KB-8 109 ± 0% 98 ± 0% -10.09% (p=0.079 n=4+5) Client/1000KB-8 617 ± 0% 593 ± 0% -3.89% (p=0.008 n=5+5) Client/2000KB-8 1.13k ± 0% 1.09k ± 0% -3.27% (p=0.008 n=5+5) Signed-off-by: Bryan Boreham --- api/client.go | 6 ++++-- api/client_test.go | 51 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/api/client.go b/api/client.go index f7ca60b67..1413f65fe 100644 --- a/api/client.go +++ b/api/client.go @@ -15,8 +15,8 @@ package api import ( + "bytes" "context" - "io/ioutil" "net" "net/http" "net/url" @@ -111,7 +111,9 @@ func (c *httpClient) Do(ctx context.Context, req *http.Request) (*http.Response, var body []byte done := make(chan struct{}) go func() { - body, err = ioutil.ReadAll(resp.Body) + var buf bytes.Buffer + _, err = buf.ReadFrom(resp.Body) + body = buf.Bytes() close(done) }() diff --git a/api/client_test.go b/api/client_test.go index 47094fccd..4215c73db 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -14,7 +14,11 @@ package api import ( + "bytes" + "context" + "fmt" "net/http" + "net/http/httptest" "net/url" "testing" ) @@ -111,3 +115,50 @@ func TestClientURL(t *testing.T) { } } } + +// Serve any http request with a response of N KB of spaces. +type serveSpaces struct { + sizeKB int +} + +func (t serveSpaces) ServeHTTP(w http.ResponseWriter, req *http.Request) { + kb := bytes.Repeat([]byte{' '}, 1024) + for i := 0; i < t.sizeKB; i++ { + w.Write(kb) + } +} + +func BenchmarkClient(b *testing.B) { + b.ReportAllocs() + ctx := context.Background() + + for _, sizeKB := range []int{4, 50, 1000, 2000} { + b.Run(fmt.Sprintf("%dKB", sizeKB), func(b *testing.B) { + + testServer := httptest.NewServer(serveSpaces{sizeKB}) + defer testServer.Close() + + client, err := NewClient(Config{ + Address: testServer.URL, + }) + if err != nil { + b.Fatalf("Failed to initialize client: %v", err) + } + url, err := url.Parse(testServer.URL + "/prometheus/api/v1/query?query=up") + if err != nil { + b.Fatalf("Failed to parse url: %v", err) + } + req := &http.Request{ + URL: url, + } + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, _, err := client.Do(ctx, req) + if err != nil { + b.Fatalf("Query failed: %v", err) + } + } + b.StopTimer() + }) + } +} From 9b785b0349a44934ac080294365eab060e7c1122 Mon Sep 17 00:00:00 2001 From: Michael Knyszek Date: Thu, 27 Jan 2022 23:46:45 -0500 Subject: [PATCH 06/14] Reduce granularity of histogram buckets for Go 1.17 collector (#974) The Go runtime/metrics package currently exports extremely granular histograms. Exponentially bucket any histogram with unit "seconds" or "bytes" instead to dramatically reduce the number of buckets, and thus the number of metrics. This change also adds a test to check for expected cardinality to prevent cardinality surprises in the future. Signed-off-by: Michael Anthony Knyszek --- prometheus/gen_go_collector_metrics_set.go | 50 ++++++++++++++ prometheus/go_collector_go117.go | 67 +++++++++++++------ prometheus/go_collector_go117_test.go | 48 +++++++++++-- prometheus/go_collector_metrics_go117_test.go | 2 + prometheus/internal/go_runtime_metrics.go | 65 ++++++++++++++++++ 5 files changed, 205 insertions(+), 27 deletions(-) diff --git a/prometheus/gen_go_collector_metrics_set.go b/prometheus/gen_go_collector_metrics_set.go index ac637771f..e33b39749 100644 --- a/prometheus/gen_go_collector_metrics_set.go +++ b/prometheus/gen_go_collector_metrics_set.go @@ -21,7 +21,9 @@ import ( "fmt" "go/format" "log" + "math" "os" + "runtime" "runtime/metrics" "strconv" "strings" @@ -35,6 +37,10 @@ func main() { if len(os.Args) != 2 { log.Fatal("requires Go version (e.g. go1.17) as an argument") } + toolVersion := runtime.Version() + if majorVersion := toolVersion[:strings.LastIndexByte(toolVersion, '.')]; majorVersion != os.Args[1] { + log.Fatalf("using Go version %q but expected Go version %q", majorVersion, os.Args[1]) + } version, err := parseVersion(os.Args[1]) if err != nil { log.Fatalf("parsing Go version: %v", err) @@ -45,9 +51,11 @@ func main() { err = testFile.Execute(&buf, struct { Descriptions []metrics.Description GoVersion goVersion + Cardinality int }{ Descriptions: metrics.All(), GoVersion: version, + Cardinality: rmCardinality(), }) if err != nil { log.Fatalf("executing template: %v", err) @@ -85,6 +93,46 @@ func parseVersion(s string) (goVersion, error) { return goVersion(i), err } +func rmCardinality() int { + cardinality := 0 + + // Collect all histogram samples so that we can get their buckets. + // The API guarantees that the buckets are always fixed for the lifetime + // of the process. + var histograms []metrics.Sample + for _, d := range metrics.All() { + if d.Kind == metrics.KindFloat64Histogram { + histograms = append(histograms, metrics.Sample{Name: d.Name}) + } else { + cardinality++ + } + } + + // Handle histograms. + metrics.Read(histograms) + for i := range histograms { + name := histograms[i].Name + buckets := internal.RuntimeMetricsBucketsForUnit( + histograms[i].Value.Float64Histogram().Buckets, + name[strings.IndexRune(name, ':')+1:], + ) + cardinality += len(buckets) + 3 // Plus total count, sum, and the implicit infinity bucket. + // runtime/metrics bucket boundaries are lower-bound-inclusive, but + // always represents each actual *boundary* so Buckets is always + // 1 longer than Counts, while in Prometheus the mapping is one-to-one, + // as the bottom bucket extends to -Inf, and the top infinity bucket is + // implicit. Therefore, we should have one fewer bucket than is listed + // above. + cardinality-- + if buckets[len(buckets)-1] == math.Inf(1) { + // We already counted the infinity bucket separately. + cardinality-- + } + } + + return cardinality +} + var testFile = template.Must(template.New("testFile").Funcs(map[string]interface{}{ "rm2prom": func(d metrics.Description) string { ns, ss, n, ok := internal.RuntimeMetricsToProm(&d) @@ -112,4 +160,6 @@ var expectedRuntimeMetrics = map[string]string{ {{- end -}} {{end}} } + +const expectedRuntimeMetricsCardinality = {{.Cardinality}} `)) diff --git a/prometheus/go_collector_go117.go b/prometheus/go_collector_go117.go index a9b1fbaea..d43bdcdda 100644 --- a/prometheus/go_collector_go117.go +++ b/prometheus/go_collector_go117.go @@ -20,6 +20,7 @@ import ( "math" "runtime" "runtime/metrics" + "strings" "sync" //nolint:staticcheck // Ignore SA1019. Need to keep deprecated package for compatibility. @@ -56,9 +57,20 @@ type goCollector struct { // Deprecated: Use collectors.NewGoCollector instead. func NewGoCollector() Collector { descriptions := metrics.All() - descMap := make(map[string]*metrics.Description) - for i := range descriptions { - descMap[descriptions[i].Name] = &descriptions[i] + + // Collect all histogram samples so that we can get their buckets. + // The API guarantees that the buckets are always fixed for the lifetime + // of the process. + var histograms []metrics.Sample + for _, d := range descriptions { + if d.Kind == metrics.KindFloat64Histogram { + histograms = append(histograms, metrics.Sample{Name: d.Name}) + } + } + metrics.Read(histograms) + bucketsMap := make(map[string][]float64) + for i := range histograms { + bucketsMap[histograms[i].Name] = histograms[i].Value.Float64Histogram().Buckets } // Generate a Desc and ValueType for each runtime/metrics metric. @@ -83,6 +95,7 @@ func NewGoCollector() Collector { var m collectorMetric if d.Kind == metrics.KindFloat64Histogram { _, hasSum := rmExactSumMap[d.Name] + unit := d.Name[strings.IndexRune(d.Name, ':')+1:] m = newBatchHistogram( NewDesc( BuildFQName(namespace, subsystem, name), @@ -90,6 +103,7 @@ func NewGoCollector() Collector { nil, nil, ), + internal.RuntimeMetricsBucketsForUnit(bucketsMap[d.Name], unit), hasSum, ) } else if d.Cumulative { @@ -299,13 +313,27 @@ type batchHistogram struct { // but Write calls may operate concurrently with updates. // Contention between these two sources should be rare. mu sync.Mutex - buckets []float64 // Inclusive lower bounds. + buckets []float64 // Inclusive lower bounds, like runtime/metrics. counts []uint64 sum float64 // Used if hasSum is true. } -func newBatchHistogram(desc *Desc, hasSum bool) *batchHistogram { - h := &batchHistogram{desc: desc, hasSum: hasSum} +// newBatchHistogram creates a new batch histogram value with the given +// Desc, buckets, and whether or not it has an exact sum available. +// +// buckets must always be from the runtime/metrics package, following +// the same conventions. +func newBatchHistogram(desc *Desc, buckets []float64, hasSum bool) *batchHistogram { + h := &batchHistogram{ + desc: desc, + buckets: buckets, + // Because buckets follows runtime/metrics conventions, there's + // 1 more value in the buckets list than there are buckets represented, + // because in runtime/metrics, the bucket values represent *boundaries*, + // and non-Inf boundaries are inclusive lower bounds for that bucket. + counts: make([]uint64, len(buckets)-1), + hasSum: hasSum, + } h.init(h) return h } @@ -313,28 +341,25 @@ func newBatchHistogram(desc *Desc, hasSum bool) *batchHistogram { // update updates the batchHistogram from a runtime/metrics histogram. // // sum must be provided if the batchHistogram was created to have an exact sum. +// h.buckets must be a strict subset of his.Buckets. func (h *batchHistogram) update(his *metrics.Float64Histogram, sum float64) { counts, buckets := his.Counts, his.Buckets - // Skip a -Inf bucket altogether. It's not clear how to represent that. - if math.IsInf(buckets[0], -1) { - buckets = buckets[1:] - counts = counts[1:] - } h.mu.Lock() defer h.mu.Unlock() - // Check if we're initialized. - if h.buckets == nil { - // Make copies of counts and buckets. It's really important - // that we don't retain his.Counts or his.Buckets anywhere since - // it's going to get reused. - h.buckets = make([]float64, len(buckets)) - copy(h.buckets, buckets) - - h.counts = make([]uint64, len(counts)) + // Clear buckets. + for i := range h.counts { + h.counts[i] = 0 + } + // Copy and reduce buckets. + var j int + for i, count := range counts { + h.counts[j] += count + if buckets[i+1] == h.buckets[j+1] { + j++ + } } - copy(h.counts, counts) if h.hasSum { h.sum = sum } diff --git a/prometheus/go_collector_go117_test.go b/prometheus/go_collector_go117_test.go index fe715fc88..9c5218fe0 100644 --- a/prometheus/go_collector_go117_test.go +++ b/prometheus/go_collector_go117_test.go @@ -140,9 +140,13 @@ func TestBatchHistogram(t *testing.T) { } metrics.Read(s) rmHist := s[0].Value.Float64Histogram() - // runtime/metrics histograms always have -Inf and +Inf buckets. - // We never handle -Inf and +Inf is implicit. - wantBuckets := len(rmHist.Buckets) - 2 + wantBuckets := internal.RuntimeMetricsBucketsForUnit(rmHist.Buckets, "bytes") + // runtime/metrics histograms always have a +Inf bucket and are lower + // bound inclusive. In contrast, we have an implicit +Inf bucket and + // are upper bound inclusive, so we can chop off the first bucket + // (since the conversion to upper bound inclusive will shift all buckets + // down one index) and the +Inf for the last bucket. + wantBuckets = wantBuckets[1 : len(wantBuckets)-1] // Check to make sure the output proto makes sense. pb := &dto.Metric{} @@ -151,14 +155,14 @@ func TestBatchHistogram(t *testing.T) { if math.IsInf(pb.Histogram.Bucket[len(pb.Histogram.Bucket)-1].GetUpperBound(), +1) { t.Errorf("found +Inf bucket") } - if got := len(pb.Histogram.Bucket); got != wantBuckets { - t.Errorf("got %d buckets in protobuf, want %d", got, wantBuckets) + if got := len(pb.Histogram.Bucket); got != len(wantBuckets) { + t.Errorf("got %d buckets in protobuf, want %d", got, len(wantBuckets)) } for i, bucket := range pb.Histogram.Bucket { // runtime/metrics histograms are lower-bound inclusive, but we're // upper-bound inclusive. So just make sure the new inclusive upper // bound is somewhere close by (in some cases it's equal). - wantBound := rmHist.Buckets[i+1] + wantBound := wantBuckets[i] if gotBound := *bucket.UpperBound; (wantBound-gotBound)/wantBound > 0.001 { t.Errorf("got bound %f, want within 0.1%% of %f", gotBound, wantBound) } @@ -244,6 +248,7 @@ func TestExpectedRuntimeMetrics(t *testing.T) { descs := metrics.All() rmSet := make(map[string]struct{}) + // Iterate over runtime-reported descriptions to find new metrics. for i := range descs { rmName := descs[i].Name rmSet[rmName] = struct{}{} @@ -263,6 +268,8 @@ func TestExpectedRuntimeMetrics(t *testing.T) { continue } } + // Now iterate over the expected metrics and look for removals. + cardinality := 0 for rmName, fqName := range expectedRuntimeMetrics { if _, ok := rmSet[rmName]; !ok { t.Errorf("runtime/metrics metric %s removed", rmName) @@ -272,6 +279,30 @@ func TestExpectedRuntimeMetrics(t *testing.T) { t.Errorf("runtime/metrics metric %s not appearing under expected name %s", rmName, fqName) continue } + + // While we're at it, check to make sure expected cardinality lines + // up, but at the point of the protobuf write to get as close to the + // real deal as possible. + // + // Note that we filter out non-runtime/metrics metrics here, because + // those are manually managed. + var m dto.Metric + if err := goMetricSet[fqName].Write(&m); err != nil { + t.Errorf("writing metric %s: %v", fqName, err) + continue + } + // N.B. These are the only fields populated by runtime/metrics metrics specifically. + // Other fields are populated by e.g. GCStats metrics. + switch { + case m.Counter != nil: + fallthrough + case m.Gauge != nil: + cardinality++ + case m.Histogram != nil: + cardinality += len(m.Histogram.Bucket) + 3 // + sum, count, and +inf + default: + t.Errorf("unexpected protobuf structure for metric %s", fqName) + } } if t.Failed() { @@ -279,6 +310,11 @@ func TestExpectedRuntimeMetrics(t *testing.T) { t.Log("\tgo run gen_go_collector_metrics_set.go go1.X") t.Log("where X is the Go version you are currently using") } + + expectCardinality := expectedRuntimeMetricsCardinality + if cardinality != expectCardinality { + t.Errorf("unexpected cardinality for runtime/metrics metrics: got %d, want %d", cardinality, expectCardinality) + } } func TestGoCollectorConcurrency(t *testing.T) { diff --git a/prometheus/go_collector_metrics_go117_test.go b/prometheus/go_collector_metrics_go117_test.go index 6c0a693ff..20e98ef56 100644 --- a/prometheus/go_collector_metrics_go117_test.go +++ b/prometheus/go_collector_metrics_go117_test.go @@ -37,3 +37,5 @@ var expectedRuntimeMetrics = map[string]string{ "/sched/goroutines:goroutines": "go_sched_goroutines_goroutines", "/sched/latencies:seconds": "go_sched_latencies_seconds", } + +const expectedRuntimeMetricsCardinality = 79 diff --git a/prometheus/internal/go_runtime_metrics.go b/prometheus/internal/go_runtime_metrics.go index afc8dff49..fe0a52180 100644 --- a/prometheus/internal/go_runtime_metrics.go +++ b/prometheus/internal/go_runtime_metrics.go @@ -17,6 +17,7 @@ package internal import ( + "math" "path" "runtime/metrics" "strings" @@ -75,3 +76,67 @@ func RuntimeMetricsToProm(d *metrics.Description) (string, string, string, bool) } return namespace, subsystem, name, valid } + +// RuntimeMetricsBucketsForUnit takes a set of buckets obtained for a runtime/metrics histogram +// type (so, lower-bound inclusive) and a unit from a runtime/metrics name, and produces +// a reduced set of buckets. This function always removes any -Inf bucket as it's represented +// as the bottom-most upper-bound inclusive bucket in Prometheus. +func RuntimeMetricsBucketsForUnit(buckets []float64, unit string) []float64 { + switch unit { + case "bytes": + // Rebucket as powers of 2. + return rebucketExp(buckets, 2) + case "seconds": + // Rebucket as powers of 10 and then merge all buckets greater + // than 1 second into the +Inf bucket. + b := rebucketExp(buckets, 10) + for i := range b { + if b[i] <= 1 { + continue + } + b[i] = math.Inf(1) + b = b[:i+1] + break + } + return b + } + return buckets +} + +// rebucketExp takes a list of bucket boundaries (lower bound inclusive) and +// downsamples the buckets to those a multiple of base apart. The end result +// is a roughly exponential (in many cases, perfectly exponential) bucketing +// scheme. +func rebucketExp(buckets []float64, base float64) []float64 { + bucket := buckets[0] + var newBuckets []float64 + // We may see a -Inf here, in which case, add it and skip it + // since we risk producing NaNs otherwise. + // + // We need to preserve -Inf values to maintain runtime/metrics + // conventions. We'll strip it out later. + if bucket == math.Inf(-1) { + newBuckets = append(newBuckets, bucket) + buckets = buckets[1:] + bucket = buckets[0] + } + // From now on, bucket should always have a non-Inf value because + // Infs are only ever at the ends of the bucket lists, so + // arithmetic operations on it are non-NaN. + for i := 1; i < len(buckets); i++ { + if bucket >= 0 && buckets[i] < bucket*base { + // The next bucket we want to include is at least bucket*base. + continue + } else if bucket < 0 && buckets[i] < bucket/base { + // In this case the bucket we're targeting is negative, and since + // we're ascending through buckets here, we need to divide to get + // closer to zero exponentially. + continue + } + // The +Inf bucket will always be the last one, and we'll always + // end up including it here because bucket + newBuckets = append(newBuckets, bucket) + bucket = buckets[i] + } + return append(newBuckets, bucket) +} From 39cf574e9943feaf0a61a9aa259139a2a6c3e02c Mon Sep 17 00:00:00 2001 From: Kemal Akkoyun Date: Sat, 29 Jan 2022 15:48:34 +0100 Subject: [PATCH 07/14] Cut v1.12.1 (#978) * Cut v1.12.1 Signed-off-by: Kemal Akkoyun * Apply review suggestions Signed-off-by: Kemal Akkoyun --- CHANGELOG.md | 7 +++++++ VERSION | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 274008214..cf231ffd6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## 1.12.1 / 2022-01-29 + +* [BUGFIX] Make the Go 1.17 collector concurrency-safe #969 + * Use simpler locking in the Go 1.17 collector #975 +* [BUGFIX] Reduce granularity of histogram buckets for Go 1.17 collector #974 +* [ENHANCEMENT] API client: make HTTP reads more efficient #976 + ## 1.12.0 / 2022-01-19 * [CHANGE] example/random: Move flags and metrics into main() #935 diff --git a/VERSION b/VERSION index 0eed1a29e..f8f4f03b3 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.12.0 +1.12.1 From 585540a010b33e60cb4755c0ab50649fd2b91c3c Mon Sep 17 00:00:00 2001 From: alissa-tung Date: Wed, 16 Mar 2022 17:46:48 +0800 Subject: [PATCH 08/14] Fix deprecated `NewBuildInfoCollector` API Update `examples/random/main.go`: `prometheus.NewBuildInfoCollector` is deprecated. Use `collectors.NewBuildInfoCollector` instead. Signed-off-by: alissa-tung --- examples/random/main.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/examples/random/main.go b/examples/random/main.go index cf44863de..13214238a 100644 --- a/examples/random/main.go +++ b/examples/random/main.go @@ -26,6 +26,7 @@ import ( "time" "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/collectors" "github.com/prometheus/client_golang/prometheus/promhttp" ) @@ -67,7 +68,7 @@ func main() { prometheus.MustRegister(rpcDurations) prometheus.MustRegister(rpcDurationsHistogram) // Add Go module build info. - prometheus.MustRegister(prometheus.NewBuildInfoCollector()) + prometheus.MustRegister(collectors.NewBuildInfoCollector()) start := time.Now() From d498b3cdd90d3ef23c85e96ced33035cc6013739 Mon Sep 17 00:00:00 2001 From: Bartlomiej Plotka Date: Wed, 13 Apr 2022 10:55:22 +0200 Subject: [PATCH 09/14] gocollector: Added options to Go Collector for changing the (#1031) * Renamed files. Signed-off-by: Bartlomiej Plotka * gocollector: Added options to Go Collector for diffetent collections. Fixes https://github.com/prometheus/client_golang/issues/983 Also: * fixed TestMemStatsEquivalence, it was noop before (: * Removed gc_cpu_fraction metric completely, since it's not working completely for Go1.17+ Signed-off-by: Bartlomiej Plotka --- prometheus/collectors/collectors.go | 24 ++ ...{go_collector.go => go_collector_go116.go} | 26 +- prometheus/collectors/go_collector_latest.go | 91 +++++++ prometheus/go_collector.go | 10 +- prometheus/go_collector_go116.go | 17 +- ...lector_go117.go => go_collector_latest.go} | 225 +++++++++++++----- ...17_test.go => go_collector_latest_test.go} | 162 ++++++++----- 7 files changed, 396 insertions(+), 159 deletions(-) rename prometheus/collectors/{go_collector.go => go_collector_go116.go} (64%) create mode 100644 prometheus/collectors/go_collector_latest.go rename prometheus/{go_collector_go117.go => go_collector_latest.go} (61%) rename prometheus/{go_collector_go117_test.go => go_collector_latest_test.go} (72%) diff --git a/prometheus/collectors/collectors.go b/prometheus/collectors/collectors.go index c4d0f5c35..f4c92913a 100644 --- a/prometheus/collectors/collectors.go +++ b/prometheus/collectors/collectors.go @@ -14,3 +14,27 @@ // Package collectors provides implementations of prometheus.Collector to // conveniently collect process and Go-related metrics. package collectors + +import "github.com/prometheus/client_golang/prometheus" + +// NewBuildInfoCollector returns a collector collecting a single metric +// "go_build_info" with the constant value 1 and three labels "path", "version", +// and "checksum". Their label values contain the main module path, version, and +// checksum, respectively. The labels will only have meaningful values if the +// binary is built with Go module support and from source code retrieved from +// the source repository (rather than the local file system). This is usually +// accomplished by building from outside of GOPATH, specifying the full address +// of the main package, e.g. "GO111MODULE=on go run +// github.com/prometheus/client_golang/examples/random". If built without Go +// module support, all label values will be "unknown". If built with Go module +// support but using the source code from the local file system, the "path" will +// be set appropriately, but "checksum" will be empty and "version" will be +// "(devel)". +// +// This collector uses only the build information for the main module. See +// https://github.com/povilasv/prommod for an example of a collector for the +// module dependencies. +func NewBuildInfoCollector() prometheus.Collector { + //nolint:staticcheck // Ignore SA1019 until v2. + return prometheus.NewBuildInfoCollector() +} diff --git a/prometheus/collectors/go_collector.go b/prometheus/collectors/go_collector_go116.go similarity index 64% rename from prometheus/collectors/go_collector.go rename to prometheus/collectors/go_collector_go116.go index edaa4e50b..effc57840 100644 --- a/prometheus/collectors/go_collector.go +++ b/prometheus/collectors/go_collector_go116.go @@ -11,6 +11,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +//go:build !go1.17 +// +build !go1.17 + package collectors import "github.com/prometheus/client_golang/prometheus" @@ -42,28 +45,5 @@ import "github.com/prometheus/client_golang/prometheus" // NOTE: The problem is solved in Go 1.15, see // https://github.com/golang/go/issues/19812 for the related Go issue. func NewGoCollector() prometheus.Collector { - //nolint:staticcheck // Ignore SA1019 until v2. return prometheus.NewGoCollector() } - -// NewBuildInfoCollector returns a collector collecting a single metric -// "go_build_info" with the constant value 1 and three labels "path", "version", -// and "checksum". Their label values contain the main module path, version, and -// checksum, respectively. The labels will only have meaningful values if the -// binary is built with Go module support and from source code retrieved from -// the source repository (rather than the local file system). This is usually -// accomplished by building from outside of GOPATH, specifying the full address -// of the main package, e.g. "GO111MODULE=on go run -// github.com/prometheus/client_golang/examples/random". If built without Go -// module support, all label values will be "unknown". If built with Go module -// support but using the source code from the local file system, the "path" will -// be set appropriately, but "checksum" will be empty and "version" will be -// "(devel)". -// -// This collector uses only the build information for the main module. See -// https://github.com/povilasv/prommod for an example of a collector for the -// module dependencies. -func NewBuildInfoCollector() prometheus.Collector { - //nolint:staticcheck // Ignore SA1019 until v2. - return prometheus.NewBuildInfoCollector() -} diff --git a/prometheus/collectors/go_collector_latest.go b/prometheus/collectors/go_collector_latest.go new file mode 100644 index 000000000..a4657a4f7 --- /dev/null +++ b/prometheus/collectors/go_collector_latest.go @@ -0,0 +1,91 @@ +// Copyright 2021 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build go1.17 +// +build go1.17 + +package collectors + +import "github.com/prometheus/client_golang/prometheus" + +//nolint:staticcheck // Ignore SA1019 until v2. +type goOptions = prometheus.GoCollectorOptions +type goOption func(o *goOptions) + +type GoCollectionOption uint32 + +const ( + // GoRuntimeMemStatsCollection represents the metrics represented by runtime.MemStats structure such as + // go_memstats_alloc_bytes + // go_memstats_alloc_bytes_total + // go_memstats_sys_bytes + // go_memstats_lookups_total + // go_memstats_mallocs_total + // go_memstats_frees_total + // go_memstats_heap_alloc_bytes + // go_memstats_heap_sys_bytes + // go_memstats_heap_idle_bytes + // go_memstats_heap_inuse_bytes + // go_memstats_heap_released_bytes + // go_memstats_heap_objects + // go_memstats_stack_inuse_bytes + // go_memstats_stack_sys_bytes + // go_memstats_mspan_inuse_bytes + // go_memstats_mspan_sys_bytes + // go_memstats_mcache_inuse_bytes + // go_memstats_mcache_sys_bytes + // go_memstats_buck_hash_sys_bytes + // go_memstats_gc_sys_bytes + // go_memstats_other_sys_bytes + // go_memstats_next_gc_bytes + // so the metrics known from pre client_golang v1.12.0, except skipped go_memstats_gc_cpu_fraction (see + // https://github.com/prometheus/client_golang/issues/842#issuecomment-861812034 for explanation. + // + // NOTE that this mode represents runtime.MemStats statistics, but they are + // actually implemented using new runtime/metrics package. + // Deprecated: Use GoRuntimeMetricsCollection instead going forward. + GoRuntimeMemStatsCollection GoCollectionOption = 1 << iota + // GoRuntimeMetricsCollection is the new set of metrics represented by runtime/metrics package and follows + // consistent naming. The exposed metric set depends on Go version, but it is controlled against + // unexpected cardinality. This set has overlapping information with GoRuntimeMemStatsCollection, just with + // new names. GoRuntimeMetricsCollection is what is recommended for using going forward. + GoRuntimeMetricsCollection +) + +// WithGoCollections allows enabling different collections for Go collector on top of base metrics +// like go_goroutines, go_threads, go_gc_duration_seconds, go_memstats_last_gc_time_seconds, go_info. +// +// Check GoRuntimeMemStatsCollection and GoRuntimeMetricsCollection for more details. You can use none, +// one or more collections at once. For example: +// WithGoCollections(GoRuntimeMemStatsCollection | GoRuntimeMetricsCollection) means both GoRuntimeMemStatsCollection +// metrics and GoRuntimeMetricsCollection will be exposed. +// +// Use WithGoCollections(GoRuntimeMemStatsCollection) to have Go collector working in +// the compatibility mode with client_golang pre v1.12 (move to runtime/metrics). +func WithGoCollections(flags uint32) goOption { + return func(o *goOptions) { + o.EnabledCollections = flags + } +} + +// NewGoCollector returns a collector that exports metrics about the current Go +// process using debug.GCStats using runtime/metrics. +func NewGoCollector(opts ...goOption) prometheus.Collector { + //nolint:staticcheck // Ignore SA1019 until v2. + promPkgOpts := make([]func(o *prometheus.GoCollectorOptions), len(opts)) + for i, opt := range opts { + promPkgOpts[i] = opt + } + //nolint:staticcheck // Ignore SA1019 until v2. + return prometheus.NewGoCollector(promPkgOpts...) +} diff --git a/prometheus/go_collector.go b/prometheus/go_collector.go index 08195b410..4d792aa29 100644 --- a/prometheus/go_collector.go +++ b/prometheus/go_collector.go @@ -197,14 +197,6 @@ func goRuntimeMemStats() memStatsMetrics { ), eval: func(ms *runtime.MemStats) float64 { return float64(ms.NextGC) }, valType: GaugeValue, - }, { - desc: NewDesc( - memstatNamespace("gc_cpu_fraction"), - "The fraction of this program's available CPU time used by the GC since the program started.", - nil, nil, - ), - eval: func(ms *runtime.MemStats) float64 { return ms.GCCPUFraction }, - valType: GaugeValue, }, } } @@ -268,7 +260,6 @@ func (c *baseGoCollector) Collect(ch chan<- Metric) { quantiles[0.0] = stats.PauseQuantiles[0].Seconds() ch <- MustNewConstSummary(c.gcDesc, uint64(stats.NumGC), stats.PauseTotal.Seconds(), quantiles) ch <- MustNewConstMetric(c.gcLastTimeDesc, GaugeValue, float64(stats.LastGC.UnixNano())/1e9) - ch <- MustNewConstMetric(c.goInfoDesc, GaugeValue, 1) } @@ -278,6 +269,7 @@ func memstatNamespace(s string) string { // memStatsMetrics provide description, evaluator, runtime/metrics name, and // value type for memstat metrics. +// TODO(bwplotka): Remove with end Go 1.16 EOL and replace with runtime/metrics.Description type memStatsMetrics []struct { desc *Desc eval func(*runtime.MemStats) float64 diff --git a/prometheus/go_collector_go116.go b/prometheus/go_collector_go116.go index 24526131e..897a6e906 100644 --- a/prometheus/go_collector_go116.go +++ b/prometheus/go_collector_go116.go @@ -40,13 +40,28 @@ type goCollector struct { // // Deprecated: Use collectors.NewGoCollector instead. func NewGoCollector() Collector { + msMetrics := goRuntimeMemStats() + msMetrics = append(msMetrics, struct { + desc *Desc + eval func(*runtime.MemStats) float64 + valType ValueType + }{ + // This metric is omitted in Go1.17+, see https://github.com/prometheus/client_golang/issues/842#issuecomment-861812034 + desc: NewDesc( + memstatNamespace("gc_cpu_fraction"), + "The fraction of this program's available CPU time used by the GC since the program started.", + nil, nil, + ), + eval: func(ms *runtime.MemStats) float64 { return ms.GCCPUFraction }, + valType: GaugeValue, + }) return &goCollector{ base: newBaseGoCollector(), msLast: &runtime.MemStats{}, msRead: runtime.ReadMemStats, msMaxWait: time.Second, msMaxAge: 5 * time.Minute, - msMetrics: goRuntimeMemStats(), + msMetrics: msMetrics, } } diff --git a/prometheus/go_collector_go117.go b/prometheus/go_collector_latest.go similarity index 61% rename from prometheus/go_collector_go117.go rename to prometheus/go_collector_latest.go index d43bdcdda..944794f1f 100644 --- a/prometheus/go_collector_go117.go +++ b/prometheus/go_collector_latest.go @@ -29,7 +29,66 @@ import ( dto "github.com/prometheus/client_model/go" ) +const ( + goGCHeapTinyAllocsObjects = "/gc/heap/tiny/allocs:objects" + goGCHeapAllocsObjects = "/gc/heap/allocs:objects" + goGCHeapFreesObjects = "/gc/heap/frees:objects" + goGCHeapAllocsBytes = "/gc/heap/allocs:bytes" + goGCHeapObjects = "/gc/heap/objects:objects" + goGCHeapGoalBytes = "/gc/heap/goal:bytes" + goMemoryClassesTotalBytes = "/memory/classes/total:bytes" + goMemoryClassesHeapObjectsBytes = "/memory/classes/heap/objects:bytes" + goMemoryClassesHeapUnusedBytes = "/memory/classes/heap/unused:bytes" + goMemoryClassesHeapReleasedBytes = "/memory/classes/heap/released:bytes" + goMemoryClassesHeapFreeBytes = "/memory/classes/heap/free:bytes" + goMemoryClassesHeapStacksBytes = "/memory/classes/heap/stacks:bytes" + goMemoryClassesOSStacksBytes = "/memory/classes/os-stacks:bytes" + goMemoryClassesMetadataMSpanInuseBytes = "/memory/classes/metadata/mspan/inuse:bytes" + goMemoryClassesMetadataMSPanFreeBytes = "/memory/classes/metadata/mspan/free:bytes" + goMemoryClassesMetadataMCacheInuseBytes = "/memory/classes/metadata/mcache/inuse:bytes" + goMemoryClassesMetadataMCacheFreeBytes = "/memory/classes/metadata/mcache/free:bytes" + goMemoryClassesProfilingBucketsBytes = "/memory/classes/profiling/buckets:bytes" + goMemoryClassesMetadataOtherBytes = "/memory/classes/metadata/other:bytes" + goMemoryClassesOtherBytes = "/memory/classes/other:bytes" +) + +// runtime/metrics names required for runtimeMemStats like logic. +var rmForMemStats = []string{goGCHeapTinyAllocsObjects, + goGCHeapAllocsObjects, + goGCHeapFreesObjects, + goGCHeapAllocsBytes, + goGCHeapObjects, + goGCHeapGoalBytes, + goMemoryClassesTotalBytes, + goMemoryClassesHeapObjectsBytes, + goMemoryClassesHeapUnusedBytes, + goMemoryClassesHeapReleasedBytes, + goMemoryClassesHeapFreeBytes, + goMemoryClassesHeapStacksBytes, + goMemoryClassesOSStacksBytes, + goMemoryClassesMetadataMSpanInuseBytes, + goMemoryClassesMetadataMSPanFreeBytes, + goMemoryClassesMetadataMCacheInuseBytes, + goMemoryClassesMetadataMCacheFreeBytes, + goMemoryClassesProfilingBucketsBytes, + goMemoryClassesMetadataOtherBytes, + goMemoryClassesOtherBytes, +} + +func bestEffortLookupRM(lookup []string) []metrics.Description { + ret := make([]metrics.Description, 0, len(lookup)) + for _, rm := range metrics.All() { + for _, m := range lookup { + if m == rm.Name { + ret = append(ret, rm) + } + } + } + return ret +} + type goCollector struct { + opt GoCollectorOptions base baseGoCollector // mu protects updates to all fields ensuring a consistent @@ -51,12 +110,46 @@ type goCollector struct { msMetrics memStatsMetrics } +const ( + // Those are not exposed due to need to move Go collector to another package in v2. + // See issue https://github.com/prometheus/client_golang/issues/1030. + goRuntimeMemStatsCollection uint32 = 1 << iota + goRuntimeMetricsCollection +) + +// GoCollectorOptions should not be used be directly by anything, except `collectors` package. +// Use it via collectors package instead. See issue +// https://github.com/prometheus/client_golang/issues/1030. +// +// Deprecated: Use collectors.WithGoCollections +type GoCollectorOptions struct { + // EnabledCollection sets what type of collections collector should expose on top of base collection. + // By default it's goMemStatsCollection | goRuntimeMetricsCollection. + EnabledCollections uint32 +} + +func (c GoCollectorOptions) isEnabled(flag uint32) bool { + return c.EnabledCollections&flag != 0 +} + +const defaultGoCollections = goRuntimeMemStatsCollection | goRuntimeMetricsCollection + // NewGoCollector is the obsolete version of collectors.NewGoCollector. // See there for documentation. // // Deprecated: Use collectors.NewGoCollector instead. -func NewGoCollector() Collector { - descriptions := metrics.All() +func NewGoCollector(opts ...func(o *GoCollectorOptions)) Collector { + opt := GoCollectorOptions{EnabledCollections: defaultGoCollections} + for _, o := range opts { + o(&opt) + } + + var descriptions []metrics.Description + if opt.isEnabled(goRuntimeMetricsCollection) { + descriptions = metrics.All() + } else if opt.isEnabled(goRuntimeMemStatsCollection) { + descriptions = bestEffortLookupRM(rmForMemStats) + } // Collect all histogram samples so that we can get their buckets. // The API guarantees that the buckets are always fixed for the lifetime @@ -67,7 +160,11 @@ func NewGoCollector() Collector { histograms = append(histograms, metrics.Sample{Name: d.Name}) } } - metrics.Read(histograms) + + if len(histograms) > 0 { + metrics.Read(histograms) + } + bucketsMap := make(map[string][]float64) for i := range histograms { bucketsMap[histograms[i].Name] = histograms[i].Value.Float64Histogram().Buckets @@ -83,7 +180,7 @@ func NewGoCollector() Collector { if !ok { // Just ignore this metric; we can't do anything with it here. // If a user decides to use the latest version of Go, we don't want - // to fail here. This condition is tested elsewhere. + // to fail here. This condition is tested in TestExpectedRuntimeMetrics. continue } @@ -123,12 +220,18 @@ func NewGoCollector() Collector { } metricSet = append(metricSet, m) } + + var msMetrics memStatsMetrics + if opt.isEnabled(goRuntimeMemStatsCollection) { + msMetrics = goRuntimeMemStats() + } return &goCollector{ + opt: opt, base: newBaseGoCollector(), rmSampleBuf: sampleBuf, rmSampleMap: sampleMap, rmMetrics: metricSet, - msMetrics: goRuntimeMemStats(), + msMetrics: msMetrics, } } @@ -163,40 +266,47 @@ func (c *goCollector) Collect(ch chan<- Metric) { c.mu.Lock() defer c.mu.Unlock() - // Populate runtime/metrics sample buffer. - metrics.Read(c.rmSampleBuf) - - // Update all our metrics from rmSampleBuf. - for i, sample := range c.rmSampleBuf { - // N.B. switch on concrete type because it's significantly more efficient - // than checking for the Counter and Gauge interface implementations. In - // this case, we control all the types here. - switch m := c.rmMetrics[i].(type) { - case *counter: - // Guard against decreases. This should never happen, but a failure - // to do so will result in a panic, which is a harsh consequence for - // a metrics collection bug. - v0, v1 := m.get(), unwrapScalarRMValue(sample.Value) - if v1 > v0 { - m.Add(unwrapScalarRMValue(sample.Value) - m.get()) + if len(c.rmSampleBuf) > 0 { + // Populate runtime/metrics sample buffer. + metrics.Read(c.rmSampleBuf) + } + + if c.opt.isEnabled(goRuntimeMetricsCollection) { + // Collect all our metrics from rmSampleBuf. + for i, sample := range c.rmSampleBuf { + // N.B. switch on concrete type because it's significantly more efficient + // than checking for the Counter and Gauge interface implementations. In + // this case, we control all the types here. + switch m := c.rmMetrics[i].(type) { + case *counter: + // Guard against decreases. This should never happen, but a failure + // to do so will result in a panic, which is a harsh consequence for + // a metrics collection bug. + v0, v1 := m.get(), unwrapScalarRMValue(sample.Value) + if v1 > v0 { + m.Add(unwrapScalarRMValue(sample.Value) - m.get()) + } + m.Collect(ch) + case *gauge: + m.Set(unwrapScalarRMValue(sample.Value)) + m.Collect(ch) + case *batchHistogram: + m.update(sample.Value.Float64Histogram(), c.exactSumFor(sample.Name)) + m.Collect(ch) + default: + panic("unexpected metric type") } - m.Collect(ch) - case *gauge: - m.Set(unwrapScalarRMValue(sample.Value)) - m.Collect(ch) - case *batchHistogram: - m.update(sample.Value.Float64Histogram(), c.exactSumFor(sample.Name)) - m.Collect(ch) - default: - panic("unexpected metric type") } } + // ms is a dummy MemStats that we populate ourselves so that we can - // populate the old metrics from it. - var ms runtime.MemStats - memStatsFromRM(&ms, c.rmSampleMap) - for _, i := range c.msMetrics { - ch <- MustNewConstMetric(i.desc, i.valType, i.eval(&ms)) + // populate the old metrics from it if goMemStatsCollection is enabled. + if c.opt.isEnabled(goRuntimeMemStatsCollection) { + var ms runtime.MemStats + memStatsFromRM(&ms, c.rmSampleMap) + for _, i := range c.msMetrics { + ch <- MustNewConstMetric(i.desc, i.valType, i.eval(&ms)) + } } } @@ -261,35 +371,30 @@ func memStatsFromRM(ms *runtime.MemStats, rm map[string]*metrics.Sample) { // while having Mallocs - Frees still represent a live object count. // Unfortunately, MemStats doesn't actually export a large allocation count, // so it's impossible to pull this number out directly. - tinyAllocs := lookupOrZero("/gc/heap/tiny/allocs:objects") - ms.Mallocs = lookupOrZero("/gc/heap/allocs:objects") + tinyAllocs - ms.Frees = lookupOrZero("/gc/heap/frees:objects") + tinyAllocs + tinyAllocs := lookupOrZero(goGCHeapTinyAllocsObjects) + ms.Mallocs = lookupOrZero(goGCHeapAllocsObjects) + tinyAllocs + ms.Frees = lookupOrZero(goGCHeapFreesObjects) + tinyAllocs - ms.TotalAlloc = lookupOrZero("/gc/heap/allocs:bytes") - ms.Sys = lookupOrZero("/memory/classes/total:bytes") + ms.TotalAlloc = lookupOrZero(goGCHeapAllocsBytes) + ms.Sys = lookupOrZero(goMemoryClassesTotalBytes) ms.Lookups = 0 // Already always zero. - ms.HeapAlloc = lookupOrZero("/memory/classes/heap/objects:bytes") + ms.HeapAlloc = lookupOrZero(goMemoryClassesHeapObjectsBytes) ms.Alloc = ms.HeapAlloc - ms.HeapInuse = ms.HeapAlloc + lookupOrZero("/memory/classes/heap/unused:bytes") - ms.HeapReleased = lookupOrZero("/memory/classes/heap/released:bytes") - ms.HeapIdle = ms.HeapReleased + lookupOrZero("/memory/classes/heap/free:bytes") + ms.HeapInuse = ms.HeapAlloc + lookupOrZero(goMemoryClassesHeapUnusedBytes) + ms.HeapReleased = lookupOrZero(goMemoryClassesHeapReleasedBytes) + ms.HeapIdle = ms.HeapReleased + lookupOrZero(goMemoryClassesHeapFreeBytes) ms.HeapSys = ms.HeapInuse + ms.HeapIdle - ms.HeapObjects = lookupOrZero("/gc/heap/objects:objects") - ms.StackInuse = lookupOrZero("/memory/classes/heap/stacks:bytes") - ms.StackSys = ms.StackInuse + lookupOrZero("/memory/classes/os-stacks:bytes") - ms.MSpanInuse = lookupOrZero("/memory/classes/metadata/mspan/inuse:bytes") - ms.MSpanSys = ms.MSpanInuse + lookupOrZero("/memory/classes/metadata/mspan/free:bytes") - ms.MCacheInuse = lookupOrZero("/memory/classes/metadata/mcache/inuse:bytes") - ms.MCacheSys = ms.MCacheInuse + lookupOrZero("/memory/classes/metadata/mcache/free:bytes") - ms.BuckHashSys = lookupOrZero("/memory/classes/profiling/buckets:bytes") - ms.GCSys = lookupOrZero("/memory/classes/metadata/other:bytes") - ms.OtherSys = lookupOrZero("/memory/classes/other:bytes") - ms.NextGC = lookupOrZero("/gc/heap/goal:bytes") - - // N.B. LastGC is omitted because runtime.GCStats already has this. - // See https://github.com/prometheus/client_golang/issues/842#issuecomment-861812034 - // for more details. - ms.LastGC = 0 + ms.HeapObjects = lookupOrZero(goGCHeapObjects) + ms.StackInuse = lookupOrZero(goMemoryClassesHeapStacksBytes) + ms.StackSys = ms.StackInuse + lookupOrZero(goMemoryClassesOSStacksBytes) + ms.MSpanInuse = lookupOrZero(goMemoryClassesMetadataMSpanInuseBytes) + ms.MSpanSys = ms.MSpanInuse + lookupOrZero(goMemoryClassesMetadataMSPanFreeBytes) + ms.MCacheInuse = lookupOrZero(goMemoryClassesMetadataMCacheInuseBytes) + ms.MCacheSys = ms.MCacheInuse + lookupOrZero(goMemoryClassesMetadataMCacheFreeBytes) + ms.BuckHashSys = lookupOrZero(goMemoryClassesProfilingBucketsBytes) + ms.GCSys = lookupOrZero(goMemoryClassesMetadataOtherBytes) + ms.OtherSys = lookupOrZero(goMemoryClassesOtherBytes) + ms.NextGC = lookupOrZero(goGCHeapGoalBytes) // N.B. GCCPUFraction is intentionally omitted. This metric is not useful, // and often misleading due to the fact that it's an average over the lifetime diff --git a/prometheus/go_collector_go117_test.go b/prometheus/go_collector_latest_test.go similarity index 72% rename from prometheus/go_collector_go117_test.go rename to prometheus/go_collector_latest_test.go index 9c5218fe0..a7fcaddbd 100644 --- a/prometheus/go_collector_go117_test.go +++ b/prometheus/go_collector_latest_test.go @@ -28,78 +28,96 @@ import ( dto "github.com/prometheus/client_model/go" ) -func TestGoCollectorRuntimeMetrics(t *testing.T) { - metrics := collectGoMetrics(t) - - msChecklist := make(map[string]bool) - for _, m := range goRuntimeMemStats() { - msChecklist[m.desc.fqName] = false +func TestRmForMemStats(t *testing.T) { + if got, want := len(bestEffortLookupRM(rmForMemStats)), len(rmForMemStats); got != want { + t.Errorf("got %d, want %d metrics", got, want) } +} - if len(metrics) == 0 { - t.Fatal("no metrics created by Collect") +func expectedBaseMetrics() map[string]struct{} { + metrics := map[string]struct{}{} + b := newBaseGoCollector() + for _, m := range []string{ + b.gcDesc.fqName, + b.goInfoDesc.fqName, + b.goroutinesDesc.fqName, + b.gcLastTimeDesc.fqName, + b.threadsDesc.fqName, + } { + metrics[m] = struct{}{} } + return metrics +} - // Check a few specific metrics. - // - // Checking them all is somewhat pointless because the runtime/metrics - // metrics are going to shift underneath us. Also if we try to check - // against the runtime/metrics package in an automated fashion we're kind - // of missing the point, because we have to do all the same work the code - // has to do to perform the translation. Same for supporting old metric - // names (the best we can do here is make sure they're all accounted for). - var sysBytes, allocs float64 - for _, m := range metrics { - name := m.Desc().fqName - switch name { - case "go_memory_classes_total_bytes": - checkMemoryMetric(t, m, &sysBytes) - case "go_sys_bytes": - checkMemoryMetric(t, m, &sysBytes) - case "go_gc_heap_allocs_bytes_total": - checkMemoryMetric(t, m, &allocs) - case "go_alloc_bytes_total": - checkMemoryMetric(t, m, &allocs) - } - if present, ok := msChecklist[name]; ok { - if present { - t.Errorf("memstats metric %s found more than once", name) - } - msChecklist[name] = true - } +func addExpectedRuntimeMemStats(metrics map[string]struct{}) map[string]struct{} { + for _, m := range goRuntimeMemStats() { + metrics[m.desc.fqName] = struct{}{} } - for name := range msChecklist { - if present := msChecklist[name]; !present { - t.Errorf("memstats metric %s not collected", name) - } + return metrics +} + +func addExpectedRuntimeMetrics(metrics map[string]struct{}) map[string]struct{} { + for _, m := range expectedRuntimeMetrics { + metrics[m] = struct{}{} } + return metrics } -func checkMemoryMetric(t *testing.T, m Metric, expValue *float64) { - t.Helper() +func TestGoCollector(t *testing.T) { + for _, tcase := range []struct { + collections uint32 + expectedFQNameSet map[string]struct{} + }{ + { + collections: 0, + expectedFQNameSet: expectedBaseMetrics(), + }, + { + collections: goRuntimeMemStatsCollection, + expectedFQNameSet: addExpectedRuntimeMemStats(expectedBaseMetrics()), + }, + { + collections: goRuntimeMetricsCollection, + expectedFQNameSet: addExpectedRuntimeMetrics(expectedBaseMetrics()), + }, + { + collections: goRuntimeMemStatsCollection | goRuntimeMetricsCollection, + expectedFQNameSet: addExpectedRuntimeMemStats(addExpectedRuntimeMetrics(expectedBaseMetrics())), + }, + } { + if ok := t.Run("", func(t *testing.T) { + goMetrics := collectGoMetrics(t, tcase.collections) + goMetricSet := make(map[string]Metric) + for _, m := range goMetrics { + goMetricSet[m.Desc().fqName] = m + } - pb := &dto.Metric{} - m.Write(pb) - var value float64 - if g := pb.GetGauge(); g != nil { - value = g.GetValue() - } else { - value = pb.GetCounter().GetValue() - } - if value <= 0 { - t.Error("bad value for total memory") - } - if *expValue == 0 { - *expValue = value - } else if value != *expValue { - t.Errorf("legacy metric and runtime/metrics metric do not match: want %d, got %d", int64(*expValue), int64(value)) + for i := range goMetrics { + name := goMetrics[i].Desc().fqName + + if _, ok := tcase.expectedFQNameSet[name]; !ok { + t.Errorf("found unpexpected metric %s", name) + continue + } + } + + // Now iterate over the expected metrics and look for removals. + for expectedName := range tcase.expectedFQNameSet { + if _, ok := goMetricSet[expectedName]; !ok { + t.Errorf("missing expected metric %s in collection", expectedName) + continue + } + } + }); !ok { + return + } } } var sink interface{} func TestBatchHistogram(t *testing.T) { - goMetrics := collectGoMetrics(t) + goMetrics := collectGoMetrics(t, defaultGoCollections) var mhist Metric for _, m := range goMetrics { @@ -126,7 +144,7 @@ func TestBatchHistogram(t *testing.T) { for i := 0; i < 100; i++ { sink = make([]byte, 128) } - collectGoMetrics(t) + collectGoMetrics(t, defaultGoCollections) for i, v := range hist.counts { if v != countsCopy[i] { t.Error("counts changed during new collection") @@ -175,10 +193,12 @@ func TestBatchHistogram(t *testing.T) { } } -func collectGoMetrics(t *testing.T) []Metric { +func collectGoMetrics(t *testing.T, enabledCollections uint32) []Metric { t.Helper() - c := NewGoCollector().(*goCollector) + c := NewGoCollector(func(o *GoCollectorOptions) { + o.EnabledCollections = enabledCollections + }).(*goCollector) // Collect all metrics. ch := make(chan Metric) @@ -201,7 +221,8 @@ func collectGoMetrics(t *testing.T) []Metric { func TestMemStatsEquivalence(t *testing.T) { var msReal, msFake runtime.MemStats - descs := metrics.All() + descs := bestEffortLookupRM(rmForMemStats) + samples := make([]metrics.Sample, len(descs)) samplesMap := make(map[string]*metrics.Sample) for i := range descs { @@ -214,9 +235,9 @@ func TestMemStatsEquivalence(t *testing.T) { // Populate msReal. runtime.ReadMemStats(&msReal) - - // Populate msFake. + // Populate msFake and hope that no GC happened in between (: metrics.Read(samples) + memStatsFromRM(&msFake, samplesMap) // Iterate over them and make sure they're somewhat close. @@ -227,9 +248,16 @@ func TestMemStatsEquivalence(t *testing.T) { for i := 0; i < msRealValue.NumField(); i++ { fr := msRealValue.Field(i) ff := msFakeValue.Field(i) - switch typ.Kind() { + + if typ.Field(i).Name == "PauseTotalNs" || typ.Field(i).Name == "LastGC" { + // We don't use those fields for metrics, + // thus we are not interested in having this filled. + continue + } + switch fr.Kind() { + // Fields which we are interested in are all uint64s. + // The only float64 field GCCPUFraction is by design omitted. case reflect.Uint64: - // N.B. Almost all fields of MemStats are uint64s. vr := fr.Interface().(uint64) vf := ff.Interface().(uint64) if float64(vr-vf)/float64(vf) > 0.05 { @@ -240,7 +268,7 @@ func TestMemStatsEquivalence(t *testing.T) { } func TestExpectedRuntimeMetrics(t *testing.T) { - goMetrics := collectGoMetrics(t) + goMetrics := collectGoMetrics(t, goRuntimeMetricsCollection) goMetricSet := make(map[string]Metric) for _, m := range goMetrics { goMetricSet[m.Desc().fqName] = m @@ -253,6 +281,7 @@ func TestExpectedRuntimeMetrics(t *testing.T) { rmName := descs[i].Name rmSet[rmName] = struct{}{} + // expectedRuntimeMetrics depends on Go version. expFQName, ok := expectedRuntimeMetrics[rmName] if !ok { t.Errorf("found new runtime/metrics metric %s", rmName) @@ -268,6 +297,7 @@ func TestExpectedRuntimeMetrics(t *testing.T) { continue } } + // Now iterate over the expected metrics and look for removals. cardinality := 0 for rmName, fqName := range expectedRuntimeMetrics { From 7eb9d111f99f25fecf9ae3825563bcedefbe93b9 Mon Sep 17 00:00:00 2001 From: Bartlomiej Plotka Date: Wed, 13 Apr 2022 20:43:29 +0200 Subject: [PATCH 10/14] gocollector: Reverted client_golang v1.12 addition of runtime/metrics metrics by default. (#1033) Fixes https://github.com/prometheus/client_golang/issues/967 Signed-off-by: Bartlomiej Plotka --- CHANGELOG.md | 6 ++++++ prometheus/collectors/go_collector_latest.go | 4 ++-- prometheus/go_collector_latest.go | 2 +- prometheus/go_collector_latest_test.go | 2 +- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf231ffd6..d515e692f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## Unreleased + +* [CHANGE] Minimum required Go version is now 1.16. +* [CHANGE] Added `collectors.WithGoCollections` that allows to choose what collection of Go runtime metrics user wants: Equivalent of [`MemStats` structure](https://pkg.go.dev/runtime#MemStats) configured using `GoRuntimeMemStatsCollection`, new based on dedicated [runtime/metrics](https://pkg.go.dev/runtime/metrics) metrics represented by `GoRuntimeMetricsCollection` option, or both by specifying `GoRuntimeMemStatsCollection | GoRuntimeMetricsCollection` flag. +* [CHANGE] :warning: Change in `collectors.NewGoCollector` metrics: Reverting addition of new ~80 runtime metrics by default. You can enable this back with `GoRuntimeMetricsCollection` option or `GoRuntimeMemStatsCollection | GoRuntimeMetricsCollection` for smooth transition. + ## 1.12.1 / 2022-01-29 * [BUGFIX] Make the Go 1.17 collector concurrency-safe #969 diff --git a/prometheus/collectors/go_collector_latest.go b/prometheus/collectors/go_collector_latest.go index a4657a4f7..58b0a5b6e 100644 --- a/prometheus/collectors/go_collector_latest.go +++ b/prometheus/collectors/go_collector_latest.go @@ -70,8 +70,8 @@ const ( // WithGoCollections(GoRuntimeMemStatsCollection | GoRuntimeMetricsCollection) means both GoRuntimeMemStatsCollection // metrics and GoRuntimeMetricsCollection will be exposed. // -// Use WithGoCollections(GoRuntimeMemStatsCollection) to have Go collector working in -// the compatibility mode with client_golang pre v1.12 (move to runtime/metrics). +// The current default is GoRuntimeMemStatsCollection, so the compatibility mode with +// client_golang pre v1.12 (move to runtime/metrics). func WithGoCollections(flags uint32) goOption { return func(o *goOptions) { o.EnabledCollections = flags diff --git a/prometheus/go_collector_latest.go b/prometheus/go_collector_latest.go index 944794f1f..8528ea705 100644 --- a/prometheus/go_collector_latest.go +++ b/prometheus/go_collector_latest.go @@ -132,7 +132,7 @@ func (c GoCollectorOptions) isEnabled(flag uint32) bool { return c.EnabledCollections&flag != 0 } -const defaultGoCollections = goRuntimeMemStatsCollection | goRuntimeMetricsCollection +const defaultGoCollections = goRuntimeMemStatsCollection // NewGoCollector is the obsolete version of collectors.NewGoCollector. // See there for documentation. diff --git a/prometheus/go_collector_latest_test.go b/prometheus/go_collector_latest_test.go index a7fcaddbd..88158df5b 100644 --- a/prometheus/go_collector_latest_test.go +++ b/prometheus/go_collector_latest_test.go @@ -117,7 +117,7 @@ func TestGoCollector(t *testing.T) { var sink interface{} func TestBatchHistogram(t *testing.T) { - goMetrics := collectGoMetrics(t, defaultGoCollections) + goMetrics := collectGoMetrics(t, goRuntimeMetricsCollection) var mhist Metric for _, m := range goMetrics { From 049d0fe55b7ae7a00e3d15ab9fdd5053a2cbf04a Mon Sep 17 00:00:00 2001 From: Kemal Akkoyun Date: Mon, 9 May 2022 10:33:45 +0200 Subject: [PATCH 11/14] prometheus: Fix convention violating names for generated collector metrics (#1048) * Fix convention violating names for generated collector metrics Signed-off-by: Kemal Akkoyun * Add new Go collector example Signed-off-by: Kemal Akkoyun --- .gitignore | 1 + CHANGELOG.md | 1 + Dockerfile | 7 ++- examples/gocollector/main.go | 55 +++++++++++++++++++ prometheus/collectors/go_collector_latest.go | 4 +- prometheus/gen_go_collector_metrics_set.go | 9 ++- prometheus/go_collector_latest_test.go | 5 +- prometheus/go_collector_metrics_go117_test.go | 6 +- prometheus/go_collector_metrics_go118_test.go | 41 ++++++++++++++ prometheus/internal/go_runtime_metrics.go | 14 ++--- 10 files changed, 126 insertions(+), 17 deletions(-) create mode 100644 examples/gocollector/main.go create mode 100644 prometheus/go_collector_metrics_go118_test.go diff --git a/.gitignore b/.gitignore index a6114ab82..539dcdbdb 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ # Examples examples/simple/simple examples/random/random +examples/gocollector/gocollector # Typical backup/temporary files of editors *~ diff --git a/CHANGELOG.md b/CHANGELOG.md index d515e692f..7740be244 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ * [CHANGE] Minimum required Go version is now 1.16. * [CHANGE] Added `collectors.WithGoCollections` that allows to choose what collection of Go runtime metrics user wants: Equivalent of [`MemStats` structure](https://pkg.go.dev/runtime#MemStats) configured using `GoRuntimeMemStatsCollection`, new based on dedicated [runtime/metrics](https://pkg.go.dev/runtime/metrics) metrics represented by `GoRuntimeMetricsCollection` option, or both by specifying `GoRuntimeMemStatsCollection | GoRuntimeMetricsCollection` flag. * [CHANGE] :warning: Change in `collectors.NewGoCollector` metrics: Reverting addition of new ~80 runtime metrics by default. You can enable this back with `GoRuntimeMetricsCollection` option or `GoRuntimeMemStatsCollection | GoRuntimeMetricsCollection` for smooth transition. +* [BUGFIX] Fix the bug that causes generated histogram metric names to end with `_total`. `go_gc_heap_allocs_by_size_bytes_total` -> `go_gc_heap_allocs_by_size_bytes`, `go_gc_heap_frees_by_size_bytes_total` -> `go_gc_heap_allocs_by_size_bytes` and`go_gc_pauses_seconds_total` -> `go_gc_pauses_seconds`. ## 1.12.1 / 2022-01-29 diff --git a/Dockerfile b/Dockerfile index 4da5f16d5..567945cc7 100644 --- a/Dockerfile +++ b/Dockerfile @@ -13,11 +13,14 @@ WORKDIR /go/src/github.com/prometheus/client_golang/examples/random RUN CGO_ENABLED=0 GOOS=linux go build -a -tags netgo -ldflags '-w' WORKDIR /go/src/github.com/prometheus/client_golang/examples/simple RUN CGO_ENABLED=0 GOOS=linux go build -a -tags netgo -ldflags '-w' +WORKDIR /go/src/github.com/prometheus/client_golang/examples/gocollector +RUN CGO_ENABLED=0 GOOS=linux go build -a -tags netgo -ldflags '-w' # Final image. FROM quay.io/prometheus/busybox:latest LABEL maintainer="The Prometheus Authors " COPY --from=builder /go/src/github.com/prometheus/client_golang/examples/random \ - /go/src/github.com/prometheus/client_golang/examples/simple ./ + /go/src/github.com/prometheus/client_golang/examples/simple \ + /go/src/github.com/prometheus/client_golang/examples/gocollector ./ EXPOSE 8080 -CMD ["echo", "Please run an example. Either /random or /simple"] +CMD ["echo", "Please run an example. Either /random, /simple or /gocollector"] diff --git a/examples/gocollector/main.go b/examples/gocollector/main.go new file mode 100644 index 000000000..93f8c227b --- /dev/null +++ b/examples/gocollector/main.go @@ -0,0 +1,55 @@ +// Copyright 2022 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build go1.17 +// +build go1.17 + +// A minimal example of how to include Prometheus instrumentation. +package main + +import ( + "flag" + "fmt" + "log" + "net/http" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/collectors" + "github.com/prometheus/client_golang/prometheus/promhttp" +) + +var addr = flag.String("listen-address", ":8080", "The address to listen on for HTTP requests.") + +func main() { + flag.Parse() + + // Create a new registry. + reg := prometheus.NewRegistry() + + // Add Go module build info. + reg.MustRegister(collectors.NewBuildInfoCollector()) + reg.MustRegister(collectors.NewGoCollector( + collectors.WithGoCollections(collectors.GoRuntimeMemStatsCollection | collectors.GoRuntimeMetricsCollection), + )) + + // Expose the registered metrics via HTTP. + http.Handle("/metrics", promhttp.HandlerFor( + reg, + promhttp.HandlerOpts{ + // Opt into OpenMetrics to support exemplars. + EnableOpenMetrics: true, + }, + )) + fmt.Println("Hello world from new Go Collector!") + log.Fatal(http.ListenAndServe(*addr, nil)) +} diff --git a/prometheus/collectors/go_collector_latest.go b/prometheus/collectors/go_collector_latest.go index 58b0a5b6e..01790e885 100644 --- a/prometheus/collectors/go_collector_latest.go +++ b/prometheus/collectors/go_collector_latest.go @@ -72,9 +72,9 @@ const ( // // The current default is GoRuntimeMemStatsCollection, so the compatibility mode with // client_golang pre v1.12 (move to runtime/metrics). -func WithGoCollections(flags uint32) goOption { +func WithGoCollections(flags GoCollectionOption) goOption { return func(o *goOptions) { - o.EnabledCollections = flags + o.EnabledCollections = uint32(flags) } } diff --git a/prometheus/gen_go_collector_metrics_set.go b/prometheus/gen_go_collector_metrics_set.go index e33b39749..c6c770405 100644 --- a/prometheus/gen_go_collector_metrics_set.go +++ b/prometheus/gen_go_collector_metrics_set.go @@ -38,10 +38,17 @@ func main() { log.Fatal("requires Go version (e.g. go1.17) as an argument") } toolVersion := runtime.Version() +<<<<<<< HEAD if majorVersion := toolVersion[:strings.LastIndexByte(toolVersion, '.')]; majorVersion != os.Args[1] { log.Fatalf("using Go version %q but expected Go version %q", majorVersion, os.Args[1]) +======= + mtv := majorVersion(toolVersion) + mv := majorVersion(os.Args[1]) + if mtv != mv { + log.Fatalf("using Go version %q but expected Go version %q", mtv, mv) +>>>>>>> f251146 (prometheus: Fix convention violating names for generated collector metrics (#1048)) } - version, err := parseVersion(os.Args[1]) + version, err := parseVersion(mv) if err != nil { log.Fatalf("parsing Go version: %v", err) } diff --git a/prometheus/go_collector_latest_test.go b/prometheus/go_collector_latest_test.go index 88158df5b..11094c828 100644 --- a/prometheus/go_collector_latest_test.go +++ b/prometheus/go_collector_latest_test.go @@ -24,8 +24,9 @@ import ( "sync" "testing" - "github.com/prometheus/client_golang/prometheus/internal" dto "github.com/prometheus/client_model/go" + + "github.com/prometheus/client_golang/prometheus/internal" ) func TestRmForMemStats(t *testing.T) { @@ -121,7 +122,7 @@ func TestBatchHistogram(t *testing.T) { var mhist Metric for _, m := range goMetrics { - if m.Desc().fqName == "go_gc_heap_allocs_by_size_bytes_total" { + if m.Desc().fqName == "go_gc_heap_allocs_by_size_bytes" { mhist = m break } diff --git a/prometheus/go_collector_metrics_go117_test.go b/prometheus/go_collector_metrics_go117_test.go index 20e98ef56..70c27333d 100644 --- a/prometheus/go_collector_metrics_go117_test.go +++ b/prometheus/go_collector_metrics_go117_test.go @@ -10,16 +10,16 @@ var expectedRuntimeMetrics = map[string]string{ "/gc/cycles/automatic:gc-cycles": "go_gc_cycles_automatic_gc_cycles_total", "/gc/cycles/forced:gc-cycles": "go_gc_cycles_forced_gc_cycles_total", "/gc/cycles/total:gc-cycles": "go_gc_cycles_total_gc_cycles_total", - "/gc/heap/allocs-by-size:bytes": "go_gc_heap_allocs_by_size_bytes_total", + "/gc/heap/allocs-by-size:bytes": "go_gc_heap_allocs_by_size_bytes", "/gc/heap/allocs:bytes": "go_gc_heap_allocs_bytes_total", "/gc/heap/allocs:objects": "go_gc_heap_allocs_objects_total", - "/gc/heap/frees-by-size:bytes": "go_gc_heap_frees_by_size_bytes_total", + "/gc/heap/frees-by-size:bytes": "go_gc_heap_frees_by_size_bytes", "/gc/heap/frees:bytes": "go_gc_heap_frees_bytes_total", "/gc/heap/frees:objects": "go_gc_heap_frees_objects_total", "/gc/heap/goal:bytes": "go_gc_heap_goal_bytes", "/gc/heap/objects:objects": "go_gc_heap_objects_objects", "/gc/heap/tiny/allocs:objects": "go_gc_heap_tiny_allocs_objects_total", - "/gc/pauses:seconds": "go_gc_pauses_seconds_total", + "/gc/pauses:seconds": "go_gc_pauses_seconds", "/memory/classes/heap/free:bytes": "go_memory_classes_heap_free_bytes", "/memory/classes/heap/objects:bytes": "go_memory_classes_heap_objects_bytes", "/memory/classes/heap/released:bytes": "go_memory_classes_heap_released_bytes", diff --git a/prometheus/go_collector_metrics_go118_test.go b/prometheus/go_collector_metrics_go118_test.go new file mode 100644 index 000000000..cdef74dd4 --- /dev/null +++ b/prometheus/go_collector_metrics_go118_test.go @@ -0,0 +1,41 @@ +// Code generated by gen_go_collector_metrics_set.go; DO NOT EDIT. +//go:generate go run gen_go_collector_metrics_set.go go1.18 + +//go:build go1.18 && !go1.19 +// +build go1.18,!go1.19 + +package prometheus + +var expectedRuntimeMetrics = map[string]string{ + "/gc/cycles/automatic:gc-cycles": "go_gc_cycles_automatic_gc_cycles_total", + "/gc/cycles/forced:gc-cycles": "go_gc_cycles_forced_gc_cycles_total", + "/gc/cycles/total:gc-cycles": "go_gc_cycles_total_gc_cycles_total", + "/gc/heap/allocs-by-size:bytes": "go_gc_heap_allocs_by_size_bytes", + "/gc/heap/allocs:bytes": "go_gc_heap_allocs_bytes_total", + "/gc/heap/allocs:objects": "go_gc_heap_allocs_objects_total", + "/gc/heap/frees-by-size:bytes": "go_gc_heap_frees_by_size_bytes", + "/gc/heap/frees:bytes": "go_gc_heap_frees_bytes_total", + "/gc/heap/frees:objects": "go_gc_heap_frees_objects_total", + "/gc/heap/goal:bytes": "go_gc_heap_goal_bytes", + "/gc/heap/objects:objects": "go_gc_heap_objects_objects", + "/gc/heap/tiny/allocs:objects": "go_gc_heap_tiny_allocs_objects_total", + "/gc/pauses:seconds": "go_gc_pauses_seconds", + "/memory/classes/heap/free:bytes": "go_memory_classes_heap_free_bytes", + "/memory/classes/heap/objects:bytes": "go_memory_classes_heap_objects_bytes", + "/memory/classes/heap/released:bytes": "go_memory_classes_heap_released_bytes", + "/memory/classes/heap/stacks:bytes": "go_memory_classes_heap_stacks_bytes", + "/memory/classes/heap/unused:bytes": "go_memory_classes_heap_unused_bytes", + "/memory/classes/metadata/mcache/free:bytes": "go_memory_classes_metadata_mcache_free_bytes", + "/memory/classes/metadata/mcache/inuse:bytes": "go_memory_classes_metadata_mcache_inuse_bytes", + "/memory/classes/metadata/mspan/free:bytes": "go_memory_classes_metadata_mspan_free_bytes", + "/memory/classes/metadata/mspan/inuse:bytes": "go_memory_classes_metadata_mspan_inuse_bytes", + "/memory/classes/metadata/other:bytes": "go_memory_classes_metadata_other_bytes", + "/memory/classes/os-stacks:bytes": "go_memory_classes_os_stacks_bytes", + "/memory/classes/other:bytes": "go_memory_classes_other_bytes", + "/memory/classes/profiling/buckets:bytes": "go_memory_classes_profiling_buckets_bytes", + "/memory/classes/total:bytes": "go_memory_classes_total_bytes", + "/sched/goroutines:goroutines": "go_sched_goroutines_goroutines", + "/sched/latencies:seconds": "go_sched_latencies_seconds", +} + +const expectedRuntimeMetricsCardinality = 79 diff --git a/prometheus/internal/go_runtime_metrics.go b/prometheus/internal/go_runtime_metrics.go index fe0a52180..6cbe063a2 100644 --- a/prometheus/internal/go_runtime_metrics.go +++ b/prometheus/internal/go_runtime_metrics.go @@ -62,7 +62,7 @@ func RuntimeMetricsToProm(d *metrics.Description) (string, string, string, bool) // other data. name = strings.ReplaceAll(name, "-", "_") name = name + "_" + unit - if d.Cumulative { + if d.Cumulative && d.Kind != metrics.KindFloat64Histogram { name = name + "_total" } @@ -84,12 +84,12 @@ func RuntimeMetricsToProm(d *metrics.Description) (string, string, string, bool) func RuntimeMetricsBucketsForUnit(buckets []float64, unit string) []float64 { switch unit { case "bytes": - // Rebucket as powers of 2. - return rebucketExp(buckets, 2) + // Re-bucket as powers of 2. + return reBucketExp(buckets, 2) case "seconds": - // Rebucket as powers of 10 and then merge all buckets greater + // Re-bucket as powers of 10 and then merge all buckets greater // than 1 second into the +Inf bucket. - b := rebucketExp(buckets, 10) + b := reBucketExp(buckets, 10) for i := range b { if b[i] <= 1 { continue @@ -103,11 +103,11 @@ func RuntimeMetricsBucketsForUnit(buckets []float64, unit string) []float64 { return buckets } -// rebucketExp takes a list of bucket boundaries (lower bound inclusive) and +// reBucketExp takes a list of bucket boundaries (lower bound inclusive) and // downsamples the buckets to those a multiple of base apart. The end result // is a roughly exponential (in many cases, perfectly exponential) bucketing // scheme. -func rebucketExp(buckets []float64, base float64) []float64 { +func reBucketExp(buckets []float64, base float64) []float64 { bucket := buckets[0] var newBuckets []float64 // We may see a -Inf here, in which case, add it and skip it From 5fe1d33cea76068edd4ece5f58e52f81d225b13c Mon Sep 17 00:00:00 2001 From: Kemal Akkoyun Date: Fri, 13 May 2022 10:04:45 +0200 Subject: [PATCH 12/14] Remove -Inf buckets from go collector histograms (#1049) * Remove -Inf buckets from go collector histograms Signed-off-by: Kemal Akkoyun * Update prometheus/collectors/go_collector_latest_test.go Co-authored-by: Bartlomiej Plotka Signed-off-by: Kemal Akkoyun * Simplify Signed-off-by: Kemal Akkoyun Co-authored-by: Bartlomiej Plotka --- .../collectors/go_collector_latest_test.go | 39 +++++++++++++++++++ prometheus/go_collector_latest.go | 14 +++++-- prometheus/go_collector_metrics_go117_test.go | 2 +- prometheus/go_collector_metrics_go118_test.go | 2 +- 4 files changed, 52 insertions(+), 5 deletions(-) create mode 100644 prometheus/collectors/go_collector_latest_test.go diff --git a/prometheus/collectors/go_collector_latest_test.go b/prometheus/collectors/go_collector_latest_test.go new file mode 100644 index 000000000..126864c32 --- /dev/null +++ b/prometheus/collectors/go_collector_latest_test.go @@ -0,0 +1,39 @@ +// Copyright 2021 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build go1.17 +// +build go1.17 + +package collectors + +import ( + "encoding/json" + "testing" + + "github.com/prometheus/client_golang/prometheus" +) + +func TestGoCollectorMarshalling(t *testing.T) { + reg := prometheus.NewRegistry() + reg.MustRegister(NewGoCollector( + WithGoCollections(GoRuntimeMemStatsCollection | GoRuntimeMetricsCollection), + )) + result, err := reg.Gather() + if err != nil { + t.Fatal(err) + } + + if _, err := json.Marshal(result); err != nil { + t.Errorf("json marshalling shoud not fail, %v", err) + } +} diff --git a/prometheus/go_collector_latest.go b/prometheus/go_collector_latest.go index 8528ea705..a0fe95eb1 100644 --- a/prometheus/go_collector_latest.go +++ b/prometheus/go_collector_latest.go @@ -25,8 +25,9 @@ import ( //nolint:staticcheck // Ignore SA1019. Need to keep deprecated package for compatibility. "github.com/golang/protobuf/proto" - "github.com/prometheus/client_golang/prometheus/internal" dto "github.com/prometheus/client_model/go" + + "github.com/prometheus/client_golang/prometheus/internal" ) const ( @@ -429,6 +430,11 @@ type batchHistogram struct { // buckets must always be from the runtime/metrics package, following // the same conventions. func newBatchHistogram(desc *Desc, buckets []float64, hasSum bool) *batchHistogram { + // We need to remove -Inf values. runtime/metrics keeps them around. + // But -Inf bucket should not be allowed for prometheus histograms. + if buckets[0] == math.Inf(-1) { + buckets = buckets[1:] + } h := &batchHistogram{ desc: desc, buckets: buckets, @@ -487,8 +493,10 @@ func (h *batchHistogram) Write(out *dto.Metric) error { for i, count := range h.counts { totalCount += count if !h.hasSum { - // N.B. This computed sum is an underestimate. - sum += h.buckets[i] * float64(count) + if count != 0 { + // N.B. This computed sum is an underestimate. + sum += h.buckets[i] * float64(count) + } } // Skip the +Inf bucket, but only for the bucket list. diff --git a/prometheus/go_collector_metrics_go117_test.go b/prometheus/go_collector_metrics_go117_test.go index 70c27333d..1b8a8698c 100644 --- a/prometheus/go_collector_metrics_go117_test.go +++ b/prometheus/go_collector_metrics_go117_test.go @@ -38,4 +38,4 @@ var expectedRuntimeMetrics = map[string]string{ "/sched/latencies:seconds": "go_sched_latencies_seconds", } -const expectedRuntimeMetricsCardinality = 79 +const expectedRuntimeMetricsCardinality = 77 diff --git a/prometheus/go_collector_metrics_go118_test.go b/prometheus/go_collector_metrics_go118_test.go index cdef74dd4..44bfb86f7 100644 --- a/prometheus/go_collector_metrics_go118_test.go +++ b/prometheus/go_collector_metrics_go118_test.go @@ -38,4 +38,4 @@ var expectedRuntimeMetrics = map[string]string{ "/sched/latencies:seconds": "go_sched_latencies_seconds", } -const expectedRuntimeMetricsCardinality = 79 +const expectedRuntimeMetricsCardinality = 77 From 0e136d10da543305a896bfed9f0a68f12697af5d Mon Sep 17 00:00:00 2001 From: Kemal Akkoyun Date: Fri, 13 May 2022 12:00:21 +0200 Subject: [PATCH 13/14] Cut v1.12.2 (#1052) * Cut v1.12.2 Signed-off-by: Kemal Akkoyun * Apply suggestions Signed-off-by: Kemal Akkoyun * Update CHANGELOG.md Co-authored-by: Bartlomiej Plotka Signed-off-by: Kemal Akkoyun Co-authored-by: Bartlomiej Plotka --- CHANGELOG.md | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7740be244..b526da958 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,9 +1,16 @@ ## Unreleased * [CHANGE] Minimum required Go version is now 1.16. + +## 1.12.2 / 2022-01-29 + * [CHANGE] Added `collectors.WithGoCollections` that allows to choose what collection of Go runtime metrics user wants: Equivalent of [`MemStats` structure](https://pkg.go.dev/runtime#MemStats) configured using `GoRuntimeMemStatsCollection`, new based on dedicated [runtime/metrics](https://pkg.go.dev/runtime/metrics) metrics represented by `GoRuntimeMetricsCollection` option, or both by specifying `GoRuntimeMemStatsCollection | GoRuntimeMetricsCollection` flag. -* [CHANGE] :warning: Change in `collectors.NewGoCollector` metrics: Reverting addition of new ~80 runtime metrics by default. You can enable this back with `GoRuntimeMetricsCollection` option or `GoRuntimeMemStatsCollection | GoRuntimeMetricsCollection` for smooth transition. -* [BUGFIX] Fix the bug that causes generated histogram metric names to end with `_total`. `go_gc_heap_allocs_by_size_bytes_total` -> `go_gc_heap_allocs_by_size_bytes`, `go_gc_heap_frees_by_size_bytes_total` -> `go_gc_heap_allocs_by_size_bytes` and`go_gc_pauses_seconds_total` -> `go_gc_pauses_seconds`. +* [CHANGE] :warning: Change in `collectors.NewGoCollector` metrics: Reverting addition of new ~80 runtime metrics by default. You can enable this back with `GoRuntimeMetricsCollection` option or `GoRuntimeMemStatsCollection | GoRuntimeMetricsCollection` for smooth transition. +* [BUGFIX] Fixed the bug that causes generated histogram metric names to end with `_total`. ⚠️ This changes 3 metric names in the new Go collector that was reverted from default in this release. + * `go_gc_heap_allocs_by_size_bytes_total` -> `go_gc_heap_allocs_by_size_bytes`, + * `go_gc_heap_frees_by_size_bytes_total` -> `go_gc_heap_allocs_by_size_bytes` + * `go_gc_pauses_seconds_total` -> `go_gc_pauses_seconds`. +* [CHANCE] Removed `-Inf` buckets from new Go Collector histograms. ## 1.12.1 / 2022-01-29 From 5da7b61481dcba5ce7fed88719edc94a8716feec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Rabenstein?= Date: Wed, 6 Jul 2022 16:42:00 +0200 Subject: [PATCH 14/14] Fix version number in VERSION (#1080) Signed-off-by: beorn7 --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index f8f4f03b3..6b89d58f8 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.12.1 +1.12.2