From 35c82f2c7ee8a3f535cf3494cd4b2d9b4cba60b1 Mon Sep 17 00:00:00 2001 From: Kemal Akkoyun Date: Fri, 13 May 2022 10:04:45 +0200 Subject: [PATCH] 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 0000000..126864c --- /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 8528ea7..a0fe95e 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 70c2733..1b8a869 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 cdef74d..44bfb86 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