prometheus: fix the exposition so histograms, counters and types are valid

[sathvik09]

Summary

The prometheus handler accepts everything the stats API produces, and much of what it exposes is not valid Prometheus. Most seriously, no histogram it publishes can be evaluated by histogram_quantile() — it never emits a +Inf bucket, and by default it emits no _bucket series at all. Counters are also published as unknown under OpenMetrics because they lack the _total suffix the encoder keys on.

This fixes the exposition. Handler keeps its name, its fields and its place in stats.MultiHandler; consumers get the fix on a version bump with no application changes.

Other handlers (datadog, influxdb, otlp, veneur) are untouched.

⚠️ Breaking for anyone scraping this package

Nothing fails to compile, but the published series change:

• every counter is renamed with a _total suffix
• timestamps are no longer exposed, which changes staleness behaviour
• histograms that previously published no buckets now publish them

HISTORY.md carries the full entry.

The defects

#	Defect	Effect
1	makeMetricBuckets allocated exactly len(buckets) entries and never appended an overflow bucket	histogram_quantile() returns NaN unless the highest bucket is +Inf. Observations above the top boundary were counted in _sum/_count but landed in no bucket
2	stats.Buckets is empty by default and a miss returned a nil slice with no error	collect ranged over it zero times and wrote no _bucket series, while _sum/_count were emitted unconditionally — so nothing looked wrong
3	label.less compared values as raw strings	+Inf sorted first (+ is ASCII 43, digits start at 48) and 10 sorted ahead of 2
4	WriteStats deduplicated # TYPE on the bare field name, scope discarded	Same-named fields from different engine prefixes looked like repeats; every one after the first ingested as untyped. Deriving sub-engines with WithPrefix exists precisely so subsystems can reuse short names like hits, so this fired readily
5	appendMetric wrote an explicit timestamp	A series carrying one opts out of stale-marker handling: the scraper keeps serving its last value for 5 minutes after the series stops being exported
6	Counters had no _total suffix	The OpenMetrics encoder keys the type line on the suffix, so counters published as unknown
7	Observe and Buckets.Set name the same metric differently	Observe takes a name relative to the engine; Set needs the fully-qualified name. A mismatch is an ordinary map miss — a mistyped key and no key at all produce identical output, so the histogram silently loses its buckets

Two things worth reviewer attention

The +Inf fix is not one line. metricState.update rebuilds the bucket set when len(state.buckets) != len(buckets). Appending +Inf makes the stored slice permanently one longer than the registry slice, so without moving that check every observation reallocates and zeroes the counts — _count climbing while every _bucket stays at 0 or 1. That is worse than the defect being fixed, so both changes are in one commit with a regression test.

The # TYPE fix is two changes that must land together. Dedup on scope + root name, and sort by scope before name. Sorting alone leaves the unscoped dedup suppressing types across a scope boundary. Deduping alone is worse than the defect: with the old ordering a histogram's _bucket series group by boundary across every scope while _count and _sum sort away from them, so one family declares its type thirteen times instead of once. There is a test for each half failing on its own.

New API

Engine.SetBuckets(name string, buckets ...any) derives the registry key from the engine's own prefix, so callers pass the same string they pass to Observe and the two cannot drift. A WithPrefix sub-engine computes its own key, removing the one-registration-per-derived-prefix problem. Additive — HistogramBuckets.Set is unchanged.

engine := stats.NewEngine("app", prometheus.DefaultHandler)
engine.SetBuckets("request.latency", 0.005, 0.01, 0.025, 0.05, 0.1, 0.5, 1)
engine.Observe("request.latency", elapsed)

prometheus.DefaultBuckets is the fallback for histograms with nothing registered — the reference client's boundaries, suited to latencies in seconds. It is a floor that keeps percentiles computable, not a substitute for choosing boundaries.

Verification

• go test ./..., go vet ./..., gofmt -l — clean
• go test -race ./prometheus/... . — clean
• Every fix is pinned by a test that fails without it, checked by reverting each one individually rather than assumed
• Output parsed by the reference parser (prometheus/common/expfmt, in a throwaway module so go.mod is untouched): counters parse as COUNTER, both sub-engine families typed, histogram parses as HISTOGRAM with +Inf == _count, buckets strictly increasing, no timestamps. Quantiles over 100 deterministic observations come out exact — p50 0.505, p95 0.9595, p99 0.9999, no NaN

Note

FieldType's zero value is Counter, and reportVersionOnce builds bare Field{} literals rather than calling MakeField, so the internal version metrics are counters and are renamed to go_version_value_total / stats_version_value_total. Consistent with the rule, though they are semantically info metrics. Left alone — changing their type is a separate decision.

🤖 Generated with Claude Code