Skip to content

Commit

Permalink
fix: initialize histogram buckets to 0 to avoid them being downsampled (
Browse files Browse the repository at this point in the history 
#4368)

* initialized histogram buckets to 0 to avoid them being downsampled
  • Loading branch information
@javiermolinar
javiermolinar authored Nov 22, 2024
1 parent 29ef5ab commit 069bd1b
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 4 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@
* [BUGFIX] Fix several issues with exemplar values for traceql metrics [#4366](https://github.com/grafana/tempo/pull/4366) (@mdisibio)
* [BUGFIX] Skip computing exemplars for instant queries. [#4204](https://github.com/grafana/tempo/pull/4204) (@javiermolinar)
* [BUGFIX] Gave context to orphaned spans related to various maintenance processes. [#4260](https://github.com/grafana/tempo/pull/4260) (@joe-elliott)
* [BUGFIX] Utilize S3Pass and S3User parameters in tempo-cli options, which were previously unused in the code. [#4259](https://github.com/grafana/tempo/pull/4259) (@faridtmammadov)
* [BUGFIX] Utilize S3Pass and S3User parameters in tempo-cli options, which were previously unused in the code. [#44236](https://github.com/grafana/tempo/pull/4259) (@faridtmammadov)
* [BUGFIX] Initialize histogram buckets to 0 to avoid downsampling. [#4366](https://github.com/grafana/tempo/pull/4366) (@javiermolinar)


# v2.6.1

Expand Down
12 changes: 11 additions & 1 deletion modules/generator/registry/histogram.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,6 @@ func (h *histogram) collectMetrics(appender storage.Appender, timeMs int64) (act
if err != nil {
return
}
s.registerSeenSeries()
}

// sum
Expand All @@ -226,6 +225,13 @@ func (h *histogram) collectMetrics(appender storage.Appender, timeMs int64) (act

// bucket
for i := range h.bucketLabels {
if s.isNew() {
endOfLastMinuteMs := getEndOfLastMinuteMs(timeMs)
_, err = appender.Append(0, s.bucketLabels[i], endOfLastMinuteMs, 0)
if err != nil {
return
}
}
ref, err := appender.Append(0, s.bucketLabels[i], timeMs, s.buckets[i].Load())
if err != nil {
return activeSeries, err
Expand All @@ -248,6 +254,10 @@ func (h *histogram) collectMetrics(appender storage.Appender, timeMs int64) (act
// clear the exemplar so we don't emit it again
s.exemplars[i].Store("")
}

if s.isNew() {
s.registerSeenSeries()
}
}

return
Expand Down
33 changes: 33 additions & 0 deletions modules/generator/registry/histogram_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,20 @@ func Test_histogram(t *testing.T) {
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-1"}, endOfLastMinuteMs, 0), // Zero entry for value-1 series
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-1"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_sum", "label": "value-1"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "1"}, endOfLastMinuteMs, 0), // Zero entry for bucket
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "1"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "2"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "2"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "+Inf"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "+Inf"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-2"}, endOfLastMinuteMs, 0), // Zero entry for value-2 series
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-2"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_sum", "label": "value-2"}, collectionTimeMs, 1.5),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "1"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "1"}, collectionTimeMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "2"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "2"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "+Inf"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "+Inf"}, collectionTimeMs, 1),
}
expectedExemplars := []exemplarSample{
Expand Down Expand Up @@ -78,8 +84,11 @@ func Test_histogram(t *testing.T) {
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-3"}, endOfLastMinuteMs, 0), // Zero entry for value-3 series
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-3"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_sum", "label": "value-3"}, collectionTimeMs, 3),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-3", "le": "1"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-3", "le": "1"}, collectionTimeMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-3", "le": "2"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-3", "le": "2"}, collectionTimeMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-3", "le": "+Inf"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-3", "le": "+Inf"}, collectionTimeMs, 1),
}
expectedExemplars = []exemplarSample{
Expand Down Expand Up @@ -161,14 +170,20 @@ func Test_histogram_cantAdd(t *testing.T) {
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-1"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-1"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_sum", "label": "value-1"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "1"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "1"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "2"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "2"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "+Inf"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "+Inf"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-2"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-2"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_sum", "label": "value-2"}, collectionTimeMs, 1.5),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "1"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "1"}, collectionTimeMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "2"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "2"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "+Inf"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "+Inf"}, collectionTimeMs, 1),
}
collectMetricAndAssert(t, h, collectionTimeMs, 10, expectedSamples, nil)
Expand Down Expand Up @@ -218,14 +233,20 @@ func Test_histogram_removeStaleSeries(t *testing.T) {
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-1"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-1"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_sum", "label": "value-1"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "1"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "1"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "2"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "2"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "+Inf"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "+Inf"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-2"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-2"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_sum", "label": "value-2"}, collectionTimeMs, 1.5),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "1"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "1"}, collectionTimeMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "2"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "2"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "+Inf"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "+Inf"}, collectionTimeMs, 1),
}
collectMetricAndAssert(t, h, collectionTimeMs, 10, expectedSamples, nil)
Expand Down Expand Up @@ -265,14 +286,20 @@ func Test_histogram_externalLabels(t *testing.T) {
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-1", "external_label": "external_value"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-1", "external_label": "external_value"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_sum", "label": "value-1", "external_label": "external_value"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "1", "external_label": "external_value"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "1", "external_label": "external_value"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "2", "external_label": "external_value"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "2", "external_label": "external_value"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "+Inf", "external_label": "external_value"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "+Inf", "external_label": "external_value"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-2", "external_label": "external_value"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-2", "external_label": "external_value"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_sum", "label": "value-2", "external_label": "external_value"}, collectionTimeMs, 1.5),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "1", "external_label": "external_value"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "1", "external_label": "external_value"}, collectionTimeMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "2", "external_label": "external_value"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "2", "external_label": "external_value"}, collectionTimeMs, 1),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "+Inf", "external_label": "external_value"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-2", "le": "+Inf", "external_label": "external_value"}, collectionTimeMs, 1),
}
collectMetricAndAssert(t, h, collectionTimeMs, 10, expectedSamples, nil)
Expand Down Expand Up @@ -359,8 +386,11 @@ func Test_histogram_concurrencyCorrectness(t *testing.T) {
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-1"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-1"}, collectionTimeMs, float64(totalCount.Load())),
newSample(map[string]string{"__name__": "my_histogram_sum", "label": "value-1"}, collectionTimeMs, 2*float64(totalCount.Load())),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "1"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "1"}, collectionTimeMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "2"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "2"}, collectionTimeMs, float64(totalCount.Load())),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "+Inf"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "+Inf"}, collectionTimeMs, float64(totalCount.Load())),
}
collectMetricAndAssert(t, h, collectionTimeMs, 5, expectedSamples, nil)
Expand All @@ -377,8 +407,11 @@ func Test_histogram_span_multiplier(t *testing.T) {
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-1"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_count", "label": "value-1"}, collectionTimeMs, 6.5),
newSample(map[string]string{"__name__": "my_histogram_sum", "label": "value-1"}, collectionTimeMs, 11.5),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "1"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "1"}, collectionTimeMs, 1.5),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "2"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "2"}, collectionTimeMs, 6.5),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "+Inf"}, endOfLastMinuteMs, 0),
newSample(map[string]string{"__name__": "my_histogram_bucket", "label": "value-1", "le": "+Inf"}, collectionTimeMs, 6.5),
}
collectMetricAndAssert(t, h, collectionTimeMs, 5, expectedSamples, nil)
Expand Down
20 changes: 18 additions & 2 deletions modules/generator/registry/native_histogram.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,6 @@ func (h *nativeHistogram) classicHistograms(appender storage.Appender, timeMs in
if err != nil {
return activeSeries, err
}
s.registerSeenSeries()
}

// sum
Expand Down Expand Up @@ -346,6 +345,13 @@ func (h *nativeHistogram) classicHistograms(appender storage.Appender, timeMs in
if bucket.GetUpperBound() == math.Inf(1) {
infBucketWasAdded = true
}
if s.isNew() {
endOfLastMinuteMs := getEndOfLastMinuteMs(timeMs)
_, appendErr := appender.Append(0, s.lb.Labels(), endOfLastMinuteMs, 0)
if appendErr != nil {
return activeSeries, appendErr
}
}

ref, appendErr := appender.Append(0, s.lb.Labels(), timeMs, getIfGreaterThenZeroOr(bucket.GetCumulativeCountFloat(), bucket.GetCumulativeCount()))
if appendErr != nil {
Expand All @@ -369,7 +375,13 @@ func (h *nativeHistogram) classicHistograms(appender storage.Appender, timeMs in
if !infBucketWasAdded {
// Add +Inf bucket
s.lb.Set(labels.BucketLabel, "+Inf")

if s.isNew() {
endOfLastMinuteMs := getEndOfLastMinuteMs(timeMs)
_, err = appender.Append(0, s.lb.Labels(), endOfLastMinuteMs, 0)
if err != nil {
return activeSeries, err
}
}
_, err = appender.Append(0, s.lb.Labels(), timeMs, getIfGreaterThenZeroOr(s.histogram.GetSampleCountFloat(), s.histogram.GetSampleCount()))
if err != nil {
return activeSeries, err
Expand All @@ -380,6 +392,10 @@ func (h *nativeHistogram) classicHistograms(appender storage.Appender, timeMs in
// drop "le" label again
s.lb.Del(labels.BucketLabel)

if s.isNew() {
s.registerSeenSeries()
}

return
}

Expand Down
Loading

0 comments on commit 069bd1b

Please sign in to comment.