Add Rust implementation alongside Go, with shared conformance and Prometheus integration - #15
Open
angles-n-daemons wants to merge 8 commits into
Open
angles-n-daemons wants to merge 8 commits into
angles-n-daemons wants to merge 8 commits into
Conversation
Relocate the Go sources into go/ so the repository can host the Rust implementation alongside it under rust/. The Go module path is unchanged (github.com/cockroachdb/goodhistogram), so the package import path becomes github.com/cockroachdb/goodhistogram/go; update the README accordingly. Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
Add a Rust implementation of goodhistogram under rust/, kept in the same repository as the Go source so the two stay in lockstep. The crate has no dependencies: recording is a couple of atomics and std bit ops. Includes the core histogram, quantile estimation, and OpenMetrics exposition helpers (conventional_buckets, to_openmetrics), unit tests, a CI workflow, and a README. Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
Add datatest/, one self-contained scenario per file: a hand-authored input section (params + values) and a golden section generated from the Go implementation (OpenMetrics exposition + quantiles). The Go suite owns the golden (go test ./go/ -run TestDatatest -rewrite); the Rust suite replays the same files and checks it reproduces them, so the two implementations can never silently drift. Includes simple scenarios plus edge cases and a README. Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
Add two optional, off-by-default features that expose goodhistogram to the Rust
metrics ecosystem as a near-drop-in duration histogram (observe seconds, store
nanoseconds, export seconds), both exporting the full bucket set:
- prometheus: tikv prometheus crate Collector (PromHistogram,
PromHistogramVec). Takes cumulative bucket counts and
synthesizes +Inf from the sample count.
- prometheus-client: OpenMetrics-native Collector (HistogramCollector).
Takes per-bucket counts and accumulates them itself.
The default build stays dependency-free. CI now also runs clippy and tests with
--all-features.
Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
The Go datatest check regenerated the golden text and string-compared it, but the golden contains floats from math.Pow/Sqrt whose last bit differs across platforms (the arm64 dev machines vs the amd64 CI runners), so CI failed on boundary/quantile text that was numerically identical. Parse and replay instead, mirroring the Rust suite: counts and sum exact, boundaries and quantiles within a small relative tolerance. -rewrite still regenerates. (The Rust CI already passed on amd64 with exact count comparison against the arm64-generated golden, confirming the counts are platform-stable.) Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
Bring the Rust crate's public surface in line with the Go implementation:
- Match Go's default: an unset error_bound now resolves to 0.10 (schema 3)
via DEFAULT_ERROR_BOUND, not STANDARD_ERROR_BOUND. (Go and Rust had
diverged on the zero-value default.)
- Add the 9 Params presets (COARSE/STANDARD/FINE + HIRES_LATENCY/IO_LATENCY/
RESPONSE_TIME/LONG_RUNNING/DATA_SIZE/MEMORY_USAGE), matching Go's values.
- Store the schema in Config; add Snapshot::schema() and Histogram::schema().
- Add Snapshot::values_at_quantiles (batch) and Histogram::values_at_quantiles_into
(buffer-reusing live reads), plus Snapshot::merge/sub for windowed views.
- Add a core HistogramVec (labeled histogram map) mirroring Go's vec.go,
minus the prometheus Collector (that lives in the prometheus feature).
Native prometheus-histogram export is intentionally not added: neither the tikv
prometheus 0.13 proto nor prometheus-client 0.22 supports native encoding.
Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
Verify the new parity surface cross-language:
- Add a schema line to the datatest golden (Go emits Snapshot.Schema(); the
Go and Rust suites both assert it).
- Add a presets conformance fixture (datatest/presets.golden) generated from
Go; rust/tests/presets.rs asserts the Rust preset consts reproduce each
preset's lo/hi/error_bound/schema/num_buckets. TestPresets also verifies it
from the Go side.
- Add Go quantile-parity tests pinning that ValuesAtQuantiles and
ValuesAtQuantilesInto match ValueAtQuantile per quantile, which is the
assumption the Rust map-based port relies on.
Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
Member
|
It's not great to have an import path I would keep the If not, at least make it |
Contributor
Author
|
@RaduBerinde that's a good point, I'll revert that - it makes a lot of sense (at least for the public api) |
Address Radu’s review by retaining the original root import path. Move the Go implementation to internal and expose its types, presets, and constructors through a root package. Update conformance commands and add consumer tests for the public API.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a Rust implementation alongside Go, with shared conformance tests and optional Prometheus integrations. Existing Go consumers keep importing
github.com/cockroachdb/goodhistogram.go.modand the public Go API at the repository root. The implementation lives ininternal/, exposed through type aliases, presets, and constructor wrappers. Rust stays inrust/.datatest/scenarios and preset fixtures. Go owns the golden output; Rust replays it to check bucket counts, sums, schemas, quantiles, and preset compatibility. Regenerate withgo test ./internal/ -run 'TestDatatest|TestPresets' -rewrite.prometheusandprometheus-clientintegrations for duration histograms that observe seconds, store nanoseconds, and export seconds with the full bucket set.Validation: Go build, vet, and tests; module tidy unchanged; Rust default and all-feature tests, including shared conformance; Rust formatting.
Native/exponential exposition conformance, a
metrics-facade recorder, and adoption in basalt remain follow-up work.