cockroachdb / cockroachdb/cockroach
pkg/util/metric: merge all histogram implementations into a single implementation
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Is your feature request related to a problem? Please describe.**
Today, we have 5 different histogram implementations:
- `Histogram`
- `HdrHistogram`
- `ManualWindowHistogram`
- `AggHistogram`
- `runtimeHistogram`
With so many implementations, it's no wonder that we find ourselves dealing with histogram bugs lately. Updating the code requires one to be very thorough, updating 5 different implementations where the "right" thing to do isn't always obvious. It's easy to miss something and introduce bugs. For example, just recently I can recall a few instances where bugs were introduced to the histogram library:
- https://github.com/cockroachdb/cockroach/issues/115825
- https://github.com/cockroachdb/cockroach/issues/114175
- https://github.com/cockroachdb/cockroach/issues/112947
Histograms are critical to customer trust. This level of toil is unacceptable, and action needs to be taken to improve our situation here.
**Describe the solution you'd like**
Many of these histograms mostly wrap `Histogram`, with small differences. Does an entirely separate implementation need to exist? Instead, I propose we:
- Unite all histogram implementations around a **single** one, where options (like the `metric.HistogramOptions` that already exists today) can be used to configure desired behavior.
- Fully deprecate the `HdrHistogram` once the unified histogram is tested & proven.
Jira issue: CRDB-34675
Contributor guide
Assessment
This issue has not been assessed yet.