Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed panic while collecting Prometheus metrics #4483

Merged
merged 1 commit into from
Sep 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
* [BUGFIX] Memberlist: forward only changes, not entire original message. #4419
* [BUGFIX] Memberlist: don't accept old tombstones as incoming change, and don't forward such messages to other gossip members. #4420
* [BUGFIX] Querier: fixed panic when querying exemplars and using `-distributor.shard-by-all-labels=false`. #4473
* [BUGFIX] Compactor: fixed panic while collecting Prometheus metrics. #4483

## 1.10.0 / 2021-08-03

Expand Down
30 changes: 24 additions & 6 deletions pkg/util/metrics_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -454,14 +454,17 @@ func (s *SummaryData) Metric(desc *prometheus.Desc, labelValues ...string) prome
return prometheus.MustNewConstSummary(desc, s.sampleCount, s.sampleSum, s.quantiles, labelValues...)
}

// HistogramData keeps data required to build histogram Metric
// HistogramData keeps data required to build histogram Metric.
type HistogramData struct {
sampleCount uint64
sampleSum float64
buckets map[float64]uint64
}

// Adds histogram from gathered metrics to this histogram data.
// AddHistogram adds histogram from gathered metrics to this histogram data.
// Do not call this function after Metric() has been invoked, because histogram created by Metric
// is using the buckets map (doesn't make a copy), and it's not allowed to change the buckets
// after they've been passed to a prometheus.Metric.
func (d *HistogramData) AddHistogram(histo *dto.Histogram) {
d.sampleCount += histo.GetSampleCount()
d.sampleSum += histo.GetSampleSum()
Expand All @@ -477,7 +480,10 @@ func (d *HistogramData) AddHistogram(histo *dto.Histogram) {
}
}

// Merged another histogram data into this one.
// AddHistogramData merges another histogram data into this one.
// Do not call this function after Metric() has been invoked, because histogram created by Metric
// is using the buckets map (doesn't make a copy), and it's not allowed to change the buckets
// after they've been passed to a prometheus.Metric.
func (d *HistogramData) AddHistogramData(histo HistogramData) {
d.sampleCount += histo.sampleCount
d.sampleSum += histo.sampleSum
Expand All @@ -492,12 +498,22 @@ func (d *HistogramData) AddHistogramData(histo HistogramData) {
}
}

// Return prometheus metric from this histogram data.
// Metric returns prometheus metric from this histogram data.
//
// Note that returned metric shares bucket with this HistogramData, so avoid
// doing more modifications to this HistogramData after calling Metric.
func (d *HistogramData) Metric(desc *prometheus.Desc, labelValues ...string) prometheus.Metric {
return prometheus.MustNewConstHistogram(desc, d.sampleCount, d.sampleSum, d.buckets, labelValues...)
}

// Creates new histogram data collector.
// Copy returns a copy of this histogram data.
func (d *HistogramData) Copy() *HistogramData {
cp := &HistogramData{}
cp.AddHistogramData(*d)
return cp
}

// NewHistogramDataCollector creates new histogram data collector.
func NewHistogramDataCollector(desc *prometheus.Desc) *HistogramDataCollector {
return &HistogramDataCollector{
desc: desc,
Expand All @@ -522,7 +538,9 @@ func (h *HistogramDataCollector) Collect(out chan<- prometheus.Metric) {
h.dataMu.RLock()
defer h.dataMu.RUnlock()

out <- h.data.Metric(h.desc)
// We must create a copy of the HistogramData data structure before calling Metric()
// to honor its contract.
out <- h.data.Copy().Metric(h.desc)
}

func (h *HistogramDataCollector) Add(hd HistogramData) {
Expand Down