Remove -Inf buckets from go collector histograms (#1049)

* Remove -Inf buckets from go collector histograms

Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>

* Update prometheus/collectors/go_collector_latest_test.go

Co-authored-by: Bartlomiej Plotka <bwplotka@gmail.com>
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>

* Simplify

Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>

Co-authored-by: Bartlomiej Plotka <bwplotka@gmail.com>
This commit is contained in:
Kemal Akkoyun 2022-05-13 10:04:45 +02:00 committed by GitHub
parent f25114699a
commit 35c82f2c7e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 52 additions and 5 deletions

View File

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

View File

@ -25,8 +25,9 @@ import (
//nolint:staticcheck // Ignore SA1019. Need to keep deprecated package for compatibility. //nolint:staticcheck // Ignore SA1019. Need to keep deprecated package for compatibility.
"github.com/golang/protobuf/proto" "github.com/golang/protobuf/proto"
"github.com/prometheus/client_golang/prometheus/internal"
dto "github.com/prometheus/client_model/go" dto "github.com/prometheus/client_model/go"
"github.com/prometheus/client_golang/prometheus/internal"
) )
const ( const (
@ -429,6 +430,11 @@ type batchHistogram struct {
// buckets must always be from the runtime/metrics package, following // buckets must always be from the runtime/metrics package, following
// the same conventions. // the same conventions.
func newBatchHistogram(desc *Desc, buckets []float64, hasSum bool) *batchHistogram { 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{ h := &batchHistogram{
desc: desc, desc: desc,
buckets: buckets, buckets: buckets,
@ -487,8 +493,10 @@ func (h *batchHistogram) Write(out *dto.Metric) error {
for i, count := range h.counts { for i, count := range h.counts {
totalCount += count totalCount += count
if !h.hasSum { if !h.hasSum {
// N.B. This computed sum is an underestimate. if count != 0 {
sum += h.buckets[i] * float64(count) // N.B. This computed sum is an underestimate.
sum += h.buckets[i] * float64(count)
}
} }
// Skip the +Inf bucket, but only for the bucket list. // Skip the +Inf bucket, but only for the bucket list.

View File

@ -38,4 +38,4 @@ var expectedRuntimeMetrics = map[string]string{
"/sched/latencies:seconds": "go_sched_latencies_seconds", "/sched/latencies:seconds": "go_sched_latencies_seconds",
} }
const expectedRuntimeMetricsCardinality = 79 const expectedRuntimeMetricsCardinality = 77

View File

@ -38,4 +38,4 @@ var expectedRuntimeMetrics = map[string]string{
"/sched/latencies:seconds": "go_sched_latencies_seconds", "/sched/latencies:seconds": "go_sched_latencies_seconds",
} }
const expectedRuntimeMetricsCardinality = 79 const expectedRuntimeMetricsCardinality = 77