Skip to content

prometheus: validate exponential histogram scale range (#6779) #6822

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- The semantic conventions have been upgraded from `v1.26.0` to `v1.34.0` in `go.opentelemetry.io/otel/sdk/trace`. (#6835)
- The semantic conventions have been upgraded from `v1.26.0` to `v1.34.0` in `go.opentelemetry.io/otel/trace`. (#6836)

### Fixed

- Validate exponential histogram scale range for Prometheus compatibility in `go.opentelemetry.io/otel/exporters/prometheus`. (#6822)

<!-- Released section -->
<!-- Don't change this section unless doing release -->

Expand Down
103 changes: 98 additions & 5 deletions exporters/prometheus/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,79 @@
}
}

// downscaleExponentialBucket re-aggregates bucket counts when downscaling to a coarser resolution.
func downscaleExponentialBucket(bucket metricdata.ExponentialBucket, scaleDelta int32) metricdata.ExponentialBucket {
if len(bucket.Counts) == 0 || scaleDelta < 1 {
return metricdata.ExponentialBucket{
Offset: bucket.Offset >> scaleDelta,
Counts: append([]uint64(nil), bucket.Counts...), // copy slice
}
}

// The new offset is scaled down
newOffset := bucket.Offset >> scaleDelta

// Pre-calculate the new bucket count to avoid growing slice
// Each group of 2^scaleDelta buckets will merge into one bucket
//nolint:gosec // Length is bounded by slice allocation
lastBucketIdx := bucket.Offset + int32(len(bucket.Counts)) - 1
lastNewIdx := lastBucketIdx >> scaleDelta
newBucketCount := int(lastNewIdx - newOffset + 1)

if newBucketCount <= 0 {
return metricdata.ExponentialBucket{
Offset: newOffset,
Counts: []uint64{},
}
}

newCounts := make([]uint64, newBucketCount)

// Merge buckets according to the scale difference
for i, count := range bucket.Counts {
if count == 0 {
continue
}

// Calculate which new bucket this count belongs to
//nolint:gosec // Index is bounded by loop iteration
originalIdx := bucket.Offset + int32(i)
newIdx := originalIdx >> scaleDelta

// Calculate the position in the new counts array
position := newIdx - newOffset
//nolint:gosec // Length is bounded by allocation
if position >= 0 && position < int32(len(newCounts)) {
newCounts[position] += count
}
}

// Trim leading and trailing zeros to minimize memory usage
firstNonZero := 0
for firstNonZero < len(newCounts) && newCounts[firstNonZero] == 0 {
firstNonZero++
}

Check warning on line 298 in exporters/prometheus/exporter.go

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/exporter.go#L297-L298

Added lines #L297 - L298 were not covered by tests

if firstNonZero == len(newCounts) {
// All zeros
return metricdata.ExponentialBucket{
Offset: newOffset,
Counts: []uint64{},
}
}

Check warning on line 306 in exporters/prometheus/exporter.go

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/exporter.go#L301-L306

Added lines #L301 - L306 were not covered by tests

lastNonZero := len(newCounts) - 1
for lastNonZero > firstNonZero && newCounts[lastNonZero] == 0 {
lastNonZero--
}

Check warning on line 311 in exporters/prometheus/exporter.go

View check run for this annotation

Codecov / codecov/patch

exporters/prometheus/exporter.go#L310-L311

Added lines #L310 - L311 were not covered by tests

return metricdata.ExponentialBucket{
//nolint:gosec // firstNonZero is bounded by newCounts length
Offset: newOffset + int32(firstNonZero),
Counts: newCounts[firstNonZero : lastNonZero+1],
}
}

func addExponentialHistogramMetric[N int64 | float64](
ch chan<- prometheus.Metric,
histogram metricdata.ExponentialHistogram[N],
Expand All @@ -258,23 +331,43 @@

desc := prometheus.NewDesc(name, m.Description, keys, nil)

// Prometheus native histograms support scales in the range [-4, 8]
scale := dp.Scale
if scale < -4 {
// Reject scales below -4 as they cannot be represented in Prometheus
otel.Handle(fmt.Errorf(
"exponential histogram scale %d is below minimum supported scale -4, skipping data point",
scale))
continue
}

// If scale > 8, we need to downscale the buckets to match the clamped scale
positiveBucket := dp.PositiveBucket
negativeBucket := dp.NegativeBucket
if scale > 8 {
scaleDelta := scale - 8
positiveBucket = downscaleExponentialBucket(dp.PositiveBucket, scaleDelta)
negativeBucket = downscaleExponentialBucket(dp.NegativeBucket, scaleDelta)
scale = 8
}

// From spec: note that Prometheus Native Histograms buckets are indexed by upper boundary while Exponential Histograms are indexed by lower boundary, the result being that the Offset fields are different-by-one.
positiveBuckets := make(map[int]int64)
for i, c := range dp.PositiveBucket.Counts {
for i, c := range positiveBucket.Counts {
if c > math.MaxInt64 {
otel.Handle(fmt.Errorf("positive count %d is too large to be represented as int64", c))
continue
}
positiveBuckets[int(dp.PositiveBucket.Offset)+i+1] = int64(c) // nolint: gosec // Size check above.
positiveBuckets[int(positiveBucket.Offset)+i+1] = int64(c) // nolint: gosec // Size check above.
}

negativeBuckets := make(map[int]int64)
for i, c := range dp.NegativeBucket.Counts {
for i, c := range negativeBucket.Counts {
if c > math.MaxInt64 {
otel.Handle(fmt.Errorf("negative count %d is too large to be represented as int64", c))
continue
}
negativeBuckets[int(dp.NegativeBucket.Offset)+i+1] = int64(c) // nolint: gosec // Size check above.
negativeBuckets[int(negativeBucket.Offset)+i+1] = int64(c) // nolint: gosec // Size check above.
}

m, err := prometheus.NewConstNativeHistogram(
Expand All @@ -284,7 +377,7 @@
positiveBuckets,
negativeBuckets,
dp.ZeroCount,
dp.Scale,
scale,
dp.ZeroThreshold,
dp.StartTime,
values...)
Expand Down
Loading
Loading