Remove -Inf buckets from go collector histograms (#1049) · prometheus/client_golang@5fe1d33 (original) (raw)
4 files changed
lines changed
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
1 | +// Copyright 2021 The Prometheus Authors | |
2 | +// Licensed under the Apache License, Version 2.0 (the "License"); | |
3 | +// you may not use this file except in compliance with the License. | |
4 | +// You may obtain a copy of the License at | |
5 | +// | |
6 | +// http://www.apache.org/licenses/LICENSE-2.0 | |
7 | +// | |
8 | +// Unless required by applicable law or agreed to in writing, software | |
9 | +// distributed under the License is distributed on an "AS IS" BASIS, | |
10 | +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | |
11 | +// See the License for the specific language governing permissions and | |
12 | +// limitations under the License. | |
13 | + | |
14 | +//go:build go1.17 | |
15 | +// +build go1.17 | |
16 | + | |
17 | +package collectors | |
18 | + | |
19 | +import ( | |
20 | +"encoding/json" | |
21 | +"testing" | |
22 | + | |
23 | +"github.com/prometheus/client_golang/prometheus" | |
24 | +) | |
25 | + | |
26 | +func TestGoCollectorMarshalling(t *testing.T) { | |
27 | +reg := prometheus.NewRegistry() | |
28 | +reg.MustRegister(NewGoCollector( | |
29 | +WithGoCollections(GoRuntimeMemStatsCollection | GoRuntimeMetricsCollection), | |
30 | + )) | |
31 | +result, err := reg.Gather() | |
32 | +if err != nil { | |
33 | +t.Fatal(err) | |
34 | + } | |
35 | + | |
36 | +if _, err := json.Marshal(result); err != nil { | |
37 | +t.Errorf("json marshalling shoud not fail, %v", err) | |
38 | + } | |
39 | +} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -25,8 +25,9 @@ import ( | ||
25 | 25 | |
26 | 26 | //nolint:staticcheck // Ignore SA1019. Need to keep deprecated package for compatibility. |
27 | 27 | "github.com/golang/protobuf/proto" |
28 | -"github.com/prometheus/client_golang/prometheus/internal" | |
29 | 28 | dto "github.com/prometheus/client_model/go" |
29 | + | |
30 | +"github.com/prometheus/client_golang/prometheus/internal" | |
30 | 31 | ) |
31 | 32 | |
32 | 33 | const ( |
@@ -429,6 +430,11 @@ type batchHistogram struct { | ||
429 | 430 | // buckets must always be from the runtime/metrics package, following |
430 | 431 | // the same conventions. |
431 | 432 | func newBatchHistogram(desc *Desc, buckets []float64, hasSum bool) *batchHistogram { |
433 | +// We need to remove -Inf values. runtime/metrics keeps them around. | |
434 | +// But -Inf bucket should not be allowed for prometheus histograms. | |
435 | +if buckets[0] == math.Inf(-1) { | |
436 | +buckets = buckets[1:] | |
437 | + } | |
432 | 438 | h := &batchHistogram{ |
433 | 439 | desc: desc, |
434 | 440 | buckets: buckets, |
@@ -487,8 +493,10 @@ func (h *batchHistogram) Write(out *dto.Metric) error { | ||
487 | 493 | for i, count := range h.counts { |
488 | 494 | totalCount += count |
489 | 495 | if !h.hasSum { |
490 | -// N.B. This computed sum is an underestimate. | |
491 | -sum += h.buckets[i] * float64(count) | |
496 | +if count != 0 { | |
497 | +// N.B. This computed sum is an underestimate. | |
498 | +sum += h.buckets[i] * float64(count) | |
499 | + } | |
492 | 500 | } |
493 | 501 | |
494 | 502 | // Skip the +Inf bucket, but only for the bucket list. |