cockroachdb / cockroachdb/cockroach

pkg/util/metric: merge all histogram implementations into a single implementation

Open
#116,584 1 comment 0 reactions 0 assignees View on GitHub
A-observability-inf C-enhancement quality-friday T-observability
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.