From 83008591a3fc73bce0153fdab85950730625457e Mon Sep 17 00:00:00 2001 From: Sathvik Date: Sat, 19 Sep 2026 03:54:17 +0530 Subject: [PATCH 1/8] prometheus: always emit a +Inf histogram bucket makeMetricBuckets allocated exactly len(buckets) entries and never appended an overflow bucket, so observations above the highest registered boundary were counted in _sum and _count but landed in no bucket at all. histogram_quantile() returns NaN unless the highest bucket has an upper bound of +Inf, so no histogram on this path could be evaluated. The rebuild check in metricState.update has to move with it. The stored bucket set is now one entry longer than the registry slice, so comparing against len(buckets) never matches: every observation would reallocate the bucket set and discard the counts, leaving _count climbing while every _bucket stayed at 0 or 1. That is worse than the defect being fixed, which is why both changes are in one commit. +Inf currently sorts ahead of every numeric boundary because label values compare as raw strings and '+' is ASCII 43 while digits start at 48. The golden tests record that ordering; a follow-up commit fixes it. Co-Authored-By: Claude Opus 5 (1M context) --- prometheus/handler_test.go | 1 + prometheus/metric.go | 21 +++++++++-- prometheus/metric_test.go | 74 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 2 deletions(-) diff --git a/prometheus/handler_test.go b/prometheus/handler_test.go index 7e5d5ae..0a6211b 100644 --- a/prometheus/handler_test.go +++ b/prometheus/handler_test.go @@ -103,6 +103,7 @@ B{a="1"} 42 1496614320000 B{a="1",b="2"} 21 1496614320000 # TYPE C histogram +C_bucket{le="+Inf"} 4 1496614320000 C_bucket{le="0.25"} 2 1496614320000 C_bucket{le="0.5"} 3 1496614320000 C_bucket{le="0.75"} 3 1496614320000 diff --git a/prometheus/metric.go b/prometheus/metric.go index 77970e5..df76b44 100644 --- a/prometheus/metric.go +++ b/prometheus/metric.go @@ -1,6 +1,7 @@ package prometheus import ( + "math" "strconv" "strings" "sync" @@ -265,7 +266,10 @@ func (state *metricState) update(mtype metricType, value float64, time time.Time state.value = value case histogram: - if len(state.buckets) != len(buckets) { + // makeMetricBuckets appends a +Inf bucket, so the state holds one more + // entry than the registry slice. Comparing against len(buckets) here + // would rebuild — and zero the counts — on every observation. + if len(state.buckets) != len(buckets)+1 { state.buckets = makeMetricBuckets(buckets, state.labels) } state.buckets.update(value) @@ -361,8 +365,18 @@ type metricBucket struct { type metricBuckets []metricBucket +// makeMetricBuckets builds the bucket set for a histogram state, with one +// entry per registered boundary plus a final +Inf bucket. +// +// The +Inf bucket is not optional: histogram_quantile returns NaN unless the +// highest bucket has an upper bound of +Inf, and without it observations above +// the last registered boundary are counted in _sum and _count but land in no +// bucket at all. +// +// Callers that compare an existing bucket set against the registry slice to +// decide whether to rebuild must account for the extra entry. func makeMetricBuckets(buckets []stats.Value, labels labels) metricBuckets { - b := make(metricBuckets, len(buckets)) + b := make(metricBuckets, len(buckets)+1) s := le(buckets) for i := range buckets { @@ -372,6 +386,9 @@ func makeMetricBuckets(buckets []stats.Value, labels labels) metricBuckets { b[i].labels = labels.copyAppend(label{"le", le}) } + b[len(buckets)].limit = math.Inf(1) + b[len(buckets)].labels = labels.copyAppend(label{"le", "+Inf"}) + return b } diff --git a/prometheus/metric_test.go b/prometheus/metric_test.go index 362eb29..c8f5fe4 100644 --- a/prometheus/metric_test.go +++ b/prometheus/metric_test.go @@ -127,6 +127,12 @@ func TestMetricStore(t *testing.T) { {mtype: counter, scope: "test", name: "A", value: 4, labels: labels{{"id", "123"}}}, {mtype: gauge, scope: "test", name: "B", value: 42, labels: labels{{"a", "1"}}}, {mtype: gauge, scope: "test", name: "B", value: 21, labels: labels{{"a", "1"}, {"b", "2"}}}, + // The 10 observation exceeds the highest registered boundary and is + // counted here, so C_bucket{le="+Inf"} matches C_count. + // + // +Inf sorts first because label values compare as raw strings and '+' + // precedes every digit in ASCII. Fixed in a follow-up commit. + {mtype: histogram, scope: "test", name: "C_bucket", value: 4, labels: labels{{"le", "+Inf"}}}, {mtype: histogram, scope: "test", name: "C_bucket", value: 2, labels: labels{{"le", "0.25"}}}, {mtype: histogram, scope: "test", name: "C_bucket", value: 3, labels: labels{{"le", "0.5"}}}, {mtype: histogram, scope: "test", name: "C_bucket", value: 3, labels: labels{{"le", "0.75"}}}, @@ -269,3 +275,71 @@ func BenchmarkLE(b *testing.B) { le(buckets) } } + +// TestMetricStateBucketsNotRebuilt guards the interaction between the +Inf +// bucket and the rebuild check in metricState.update. +// +// makeMetricBuckets returns len(buckets)+1 entries. If update compares the +// stored slice against len(buckets) it never matches, so every observation +// reallocates the bucket set and discards the counts accumulated so far — +// leaving _count climbing while every _bucket stays at 0 or 1. +func TestMetricStateBucketsNotRebuilt(t *testing.T) { + buckets := []stats.Value{ + stats.ValueOf(0.25), + stats.ValueOf(0.5), + } + + state := newMetricState(labels{}) + + const observations = 10 + for range observations { + state.update(histogram, 0.1, time.Now(), buckets) + } + + if n := len(state.buckets); n != len(buckets)+1 { + t.Fatalf("expected %d buckets (registered + Inf), found %d", len(buckets)+1, n) + } + + // Every observation is 0.1, so all of them belong in the lowest bucket. + if c := state.buckets[0].count; c != observations { + t.Errorf("buckets were rebuilt: le=0.25 has count %d, expected %d", c, observations) + } + + if state.count != observations { + t.Errorf("count = %d, expected %d", state.count, observations) + } +} + +// TestMakeMetricBucketsAppendsInf covers the empty-registry case, where the +// +Inf bucket is the only one, and confirms it catches overflow. +func TestMakeMetricBucketsAppendsInf(t *testing.T) { + for _, test := range []struct { + name string + buckets []stats.Value + }{ + {name: "empty", buckets: nil}, + {name: "one boundary", buckets: []stats.Value{stats.ValueOf(1.0)}}, + } { + t.Run(test.name, func(t *testing.T) { + b := makeMetricBuckets(test.buckets, labels{}) + + if len(b) != len(test.buckets)+1 { + t.Fatalf("expected %d buckets, found %d", len(test.buckets)+1, len(b)) + } + + last := b[len(b)-1] + if !math.IsInf(last.limit, 1) { + t.Errorf("last bucket limit = %v, expected +Inf", last.limit) + } + if got := last.labels[len(last.labels)-1]; got != (label{"le", "+Inf"}) { + t.Errorf("last bucket label = %v, expected le=+Inf", got) + } + + // A value above every registered boundary must still be counted. + b.update(1e9) + if c := b[len(b)-1].count; c != 1 { + t.Errorf("overflow observation not counted in +Inf bucket (count = %d)", c) + } + }) + } +} From 3721eb85cb56e5ea4c18424ee684eaf1021ee897 Mon Sep 17 00:00:00 2001 From: Sathvik Date: Sat, 19 Sep 2026 03:55:14 +0530 Subject: [PATCH 2/8] prometheus: sort histogram le labels numerically label.less compared label values as raw strings, so "+Inf" sorted ahead of every boundary ('+' is ASCII 43, digits start at 48) and "10" sorted ahead of "2". byNameAndLabels.Less only delegates here, so histogram buckets came out in the wrong order. OpenMetrics requires buckets in increasing order; the text format is indifferent, but the ordering is also what makes the exposition readable and matches every other Prometheus client. The comparison is shared by every label, so the numeric path is scoped to "le" rather than applied wholesale, and falls back to string comparison when a value does not parse. Co-Authored-By: Claude Opus 5 (1M context) --- prometheus/handler_test.go | 2 +- prometheus/label.go | 21 +++++++++++++++++++- prometheus/label_test.go | 39 ++++++++++++++++++++++++++++++++++++++ prometheus/metric_test.go | 10 ++++------ 4 files changed, 64 insertions(+), 8 deletions(-) diff --git a/prometheus/handler_test.go b/prometheus/handler_test.go index 0a6211b..a026d98 100644 --- a/prometheus/handler_test.go +++ b/prometheus/handler_test.go @@ -103,11 +103,11 @@ B{a="1"} 42 1496614320000 B{a="1",b="2"} 21 1496614320000 # TYPE C histogram -C_bucket{le="+Inf"} 4 1496614320000 C_bucket{le="0.25"} 2 1496614320000 C_bucket{le="0.5"} 3 1496614320000 C_bucket{le="0.75"} 3 1496614320000 C_bucket{le="1"} 3 1496614320000 +C_bucket{le="+Inf"} 4 1496614320000 C_count 4 1496614320000 C_sum 10.7 1496614320000 ` diff --git a/prometheus/label.go b/prometheus/label.go index 7a8068d..5a053d8 100644 --- a/prometheus/label.go +++ b/prometheus/label.go @@ -1,6 +1,8 @@ package prometheus import ( + "strconv" + "github.com/segmentio/fasthash/jody" "github.com/segmentio/stats/v5" @@ -16,7 +18,24 @@ func (l label) equal(other label) bool { } func (l label) less(other label) bool { - return l.name < other.name || (l.name == other.name && l.value < other.value) + if l.name != other.name { + return l.name < other.name + } + + // Histogram bucket boundaries have to come out in increasing numeric + // order. Comparing them as strings puts "+Inf" first ('+' is ASCII 43, + // digits start at 48) and orders "10" ahead of "2". + // + // Scoped to "le" so that every other label keeps comparing as a string. + if l.name == "le" { + v1, err1 := strconv.ParseFloat(l.value, 64) + v2, err2 := strconv.ParseFloat(other.value, 64) + if err1 == nil && err2 == nil { + return v1 < v2 + } + } + + return l.value < other.value } type labels []label diff --git a/prometheus/label_test.go b/prometheus/label_test.go index 8fb10ad..00f0ce9 100644 --- a/prometheus/label_test.go +++ b/prometheus/label_test.go @@ -59,3 +59,42 @@ func TestLabelsLess(t *testing.T) { }) } } + +func TestLabelLessLE(t *testing.T) { + tests := []struct { + name string + l1 label + l2 label + less bool + }{ + // +Inf must sort after every numeric boundary. As raw strings it + // sorts first, because '+' is ASCII 43 and digits start at 48. + {name: "+Inf after a boundary", l1: label{"le", "0.25"}, l2: label{"le", "+Inf"}, less: true}, + {name: "+Inf not before a boundary", l1: label{"le", "+Inf"}, l2: label{"le", "0.25"}, less: false}, + {name: "+Inf equals itself", l1: label{"le", "+Inf"}, l2: label{"le", "+Inf"}, less: false}, + + // Numeric order, not lexical: "10" < "2" as a string. + {name: "10 after 2", l1: label{"le", "10"}, l2: label{"le", "2"}, less: false}, + {name: "2 before 10", l1: label{"le", "2"}, l2: label{"le", "10"}, less: true}, + {name: "fractional", l1: label{"le", "0.005"}, l2: label{"le", "0.01"}, less: true}, + + // Other labels keep comparing as strings, so a numeric-looking value + // on a non-le label is unaffected. + {name: "non-le stays lexical", l1: label{"code", "10"}, l2: label{"code", "2"}, less: true}, + + // A non-numeric le value falls back to string comparison rather than + // treating the parse failure as equality. + {name: "unparseable le", l1: label{"le", "abc"}, l2: label{"le", "abd"}, less: true}, + + // Name comparison still wins over value comparison. + {name: "different names", l1: label{"a", "9"}, l2: label{"le", "1"}, less: true}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if less := test.l1.less(test.l2); less != test.less { + t.Errorf("(%v < %v) = %t, expected %t", test.l1, test.l2, less, test.less) + } + }) + } +} diff --git a/prometheus/metric_test.go b/prometheus/metric_test.go index c8f5fe4..f3b6c33 100644 --- a/prometheus/metric_test.go +++ b/prometheus/metric_test.go @@ -127,16 +127,14 @@ func TestMetricStore(t *testing.T) { {mtype: counter, scope: "test", name: "A", value: 4, labels: labels{{"id", "123"}}}, {mtype: gauge, scope: "test", name: "B", value: 42, labels: labels{{"a", "1"}}}, {mtype: gauge, scope: "test", name: "B", value: 21, labels: labels{{"a", "1"}, {"b", "2"}}}, - // The 10 observation exceeds the highest registered boundary and is - // counted here, so C_bucket{le="+Inf"} matches C_count. - // - // +Inf sorts first because label values compare as raw strings and '+' - // precedes every digit in ASCII. Fixed in a follow-up commit. - {mtype: histogram, scope: "test", name: "C_bucket", value: 4, labels: labels{{"le", "+Inf"}}}, {mtype: histogram, scope: "test", name: "C_bucket", value: 2, labels: labels{{"le", "0.25"}}}, {mtype: histogram, scope: "test", name: "C_bucket", value: 3, labels: labels{{"le", "0.5"}}}, {mtype: histogram, scope: "test", name: "C_bucket", value: 3, labels: labels{{"le", "0.75"}}}, {mtype: histogram, scope: "test", name: "C_bucket", value: 3, labels: labels{{"le", "1"}}}, + // The 10 observation exceeds the highest registered boundary, so the + // +Inf bucket matches C_count. It sorts last because le compares + // numerically. + {mtype: histogram, scope: "test", name: "C_bucket", value: 4, labels: labels{{"le", "+Inf"}}}, {mtype: histogram, scope: "test", name: "C_count", value: 4, labels: labels{}}, {mtype: histogram, scope: "test", name: "C_sum", value: 10.7, labels: labels{}}, } From 1eed1d726ad1abef5be95619ce895557967a9751 Mon Sep 17 00:00:00 2001 From: Sathvik Date: Sat, 19 Sep 2026 03:56:09 +0530 Subject: [PATCH 3/8] prometheus: fall back to a default bucket set on a registry miss MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit stats.Buckets is empty by default, and the lookup in HandleMeasures returned a nil slice with no error. collect() then ranged over it zero times and wrote no _bucket series, while _sum and _count were emitted unconditionally — so a histogram with no registered boundaries looked healthy and had no percentiles. DefaultBuckets holds the boundaries used by the reference Prometheus client. stats converts Duration values to seconds before bucketing, so timing histograms land on this range without configuration. This is a floor, not a replacement for choosing boundaries: a histogram whose values sit outside the range lands entirely in +Inf. What changes is that the failure is now visible in the exposition rather than absent from it. Co-Authored-By: Claude Opus 5 (1M context) --- prometheus/handler.go | 41 ++++++++++++++++++++++++++++++++++++++ prometheus/handler_test.go | 37 ++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/prometheus/handler.go b/prometheus/handler.go index 720b2bc..e64a9b7 100644 --- a/prometheus/handler.go +++ b/prometheus/handler.go @@ -43,6 +43,8 @@ type Handler struct { // Buckets is the registry of histogram buckets used by the handler, // If nil, stats.Buckets is used instead. + // + // Histograms with no entry in the registry fall back to DefaultBuckets. Buckets stats.HistogramBuckets opcount atomic.Uint64 @@ -71,6 +73,15 @@ func (h *Handler) HandleMeasures(mtime time.Time, measures ...stats.Measure) { } else { buckets = stats.Buckets[k] } + + // A registry miss returns a nil slice with no error, which + // used to mean the histogram was published with _sum and + // _count but no _bucket series at all — nothing looked wrong, + // and no percentile could be computed. Fall back so that a + // histogram is never silently bucket-less. + if buckets == nil { + buckets = DefaultBuckets + } } h.metrics.update(metric{ @@ -201,6 +212,36 @@ var DefaultHandler = &Handler{ TrimPrefix: stats.DefaultEngine.Prefix, } +// DefaultBuckets is the bucket set used for histograms that have no boundaries +// registered in stats.Buckets or in Handler.Buckets. +// +// The boundaries are the ones used by the reference Prometheus client, chosen +// for request latencies measured in seconds. stats.Duration values are +// converted to seconds before bucketing, so timing histograms land on this +// range without configuration. +// +// They are a starting point, not a substitute for choosing boundaries: a +// bucketed percentile is only as accurate as the bucket it falls in, and a +// histogram whose values sit outside this range lands entirely in the +Inf +// bucket. Register real boundaries with Engine.SetBuckets wherever p99 +// accuracy matters. +// +// Programs may replace this during initialization, before any measure is +// handled. +var DefaultBuckets = []stats.Value{ + stats.ValueOf(0.005), + stats.ValueOf(0.01), + stats.ValueOf(0.025), + stats.ValueOf(0.05), + stats.ValueOf(0.1), + stats.ValueOf(0.25), + stats.ValueOf(0.5), + stats.ValueOf(1.0), + stats.ValueOf(2.5), + stats.ValueOf(5.0), + stats.ValueOf(10.0), +} + func typeOf(t stats.FieldType) metricType { switch t { case stats.Counter: diff --git a/prometheus/handler_test.go b/prometheus/handler_test.go index a026d98..25363f1 100644 --- a/prometheus/handler_test.go +++ b/prometheus/handler_test.go @@ -5,6 +5,7 @@ import ( "io" "net/http" "net/http/httptest" + "strings" "testing" "time" @@ -158,3 +159,39 @@ func BenchmarkHandleMetric(b *testing.B) { }) } } + +// TestHistogramWithoutRegisteredBuckets covers the fail-silent case: a +// histogram with no entry in the bucket registry used to publish _sum and +// _count with no _bucket series at all, so nothing looked wrong and no +// percentile could be computed. +func TestHistogramWithoutRegisteredBuckets(t *testing.T) { + now := time.Date(2017, 6, 4, 22, 12, 0, 0, time.UTC) + + handler := &Handler{} // no Buckets registry at all + + handler.HandleMeasures(now, + stats.Measure{Fields: []stats.Field{stats.MakeField("D", 0.003, stats.Histogram)}}, + stats.Measure{Fields: []stats.Field{stats.MakeField("D", 0.4, stats.Histogram)}}, + stats.Measure{Fields: []stats.Field{stats.MakeField("D", 900, stats.Histogram)}}, + ) + + var buf strings.Builder + handler.WriteStats(&buf) + out := buf.String() + + for _, want := range []string{ + `D_bucket{le="0.005"} 1`, + `D_bucket{le="0.5"} 2`, + `D_bucket{le="+Inf"} 3`, + `D_count 3`, + } { + if !strings.Contains(out, want) { + t.Errorf("missing %q in output:\n%s", want, out) + } + } + + // Every registered boundary plus +Inf must be present. + if n := strings.Count(out, "D_bucket{"); n != len(DefaultBuckets)+1 { + t.Errorf("found %d bucket series, expected %d", n, len(DefaultBuckets)+1) + } +} From 612b93c77f1727b3e145e1e4abd4251e882ba8ce Mon Sep 17 00:00:00 2001 From: Sathvik Date: Sat, 19 Sep 2026 03:58:41 +0530 Subject: [PATCH 4/8] prometheus: declare metric type once per scope, not per field name WriteStats deduplicated "# TYPE" lines on the bare field name with the scope discarded, so same-named fields arriving from different engine prefixes looked like repeats of each other and every one after the first was emitted untyped. Deriving sub-engines with WithPrefix is idiomatic across Segment services and exists precisely so subsystems can reuse short field names, so this fires readily: three sub-engines exposing hits and size shipped four of six metrics with no type. The dedup key becomes the scope and the root name together, and byNameAndLabels.Less orders by scope before name so that each family stays contiguous. Both halves are required. 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. Tests cover each half failing on its own. Less compares scope and name in turn rather than the joined string to avoid allocating per comparison in the sort. Co-Authored-By: Claude Opus 5 (1M context) --- prometheus/handler.go | 18 ++++++-- prometheus/handler_test.go | 89 ++++++++++++++++++++++++++++++++++++++ prometheus/metric.go | 18 +++++++- 3 files changed, 120 insertions(+), 5 deletions(-) diff --git a/prometheus/handler.go b/prometheus/handler.go index e64a9b7..b9dc3a3 100644 --- a/prometheus/handler.go +++ b/prometheus/handler.go @@ -152,15 +152,25 @@ func (h *Handler) ServeHTTP(res http.ResponseWriter, req *http.Request) { func (h *Handler) WriteStats(w io.Writer) { b := make([]byte, 1024) - var lastMetricName string + // A metric family is identified by its scope and root name together. The + // scope cannot be dropped here: two sub-engines derived with WithPrefix + // commonly expose the same field name, and comparing the bare name made + // the second family look like a repeat of the first, so its "# TYPE" line + // was suppressed and it ingested as untyped. + // + // byNameAndLabels.Less orders by scope before name for the same reason. + // Deduplicating on the scoped name without that ordering would turn + // missing type declarations into duplicate ones. + var lastScope, lastRootName string + metrics := h.metrics.collect(make([]metric, 0, 10000)) sort.Sort(byNameAndLabels(metrics)) for i, m := range metrics { b = b[:0] - name := m.rootName() + scope, name := m.scope, m.rootName() - if name == lastMetricName { + if scope == lastScope && name == lastRootName { // Silence the repeated output of type for values belonging to the // same metric. m.mtype, m.help = untyped, "" @@ -171,7 +181,7 @@ func (h *Handler) WriteStats(w io.Writer) { } _, _ = w.Write(appendMetric(b, m)) - lastMetricName = name + lastScope, lastRootName = scope, name } } diff --git a/prometheus/handler_test.go b/prometheus/handler_test.go index 25363f1..733732a 100644 --- a/prometheus/handler_test.go +++ b/prometheus/handler_test.go @@ -195,3 +195,92 @@ func TestHistogramWithoutRegisteredBuckets(t *testing.T) { t.Errorf("found %d bucket series, expected %d", n, len(DefaultBuckets)+1) } } + +// TestTypeDeclarationPerScope covers same-named fields arriving from +// different engine prefixes, which is what deriving sub-engines with +// WithPrefix produces. +// +// The dedup used to compare the bare field name with the scope discarded, so +// only the first scope to emit "hits" got a "# TYPE" line and every later one +// ingested as untyped. +func TestTypeDeclarationPerScope(t *testing.T) { + now := time.Date(2017, 6, 4, 22, 12, 0, 0, time.UTC) + + handler := &Handler{} + + scopes := []string{"alpha", "beta", "gamma"} + for _, scope := range scopes { + handler.HandleMeasures(now, stats.Measure{ + Name: scope, + Fields: []stats.Field{ + stats.MakeField("hits", 1, stats.Counter), + stats.MakeField("size", 2, stats.Gauge), + }, + }) + } + + var buf strings.Builder + handler.WriteStats(&buf) + out := buf.String() + + for _, scope := range scopes { + for _, want := range []string{ + "# TYPE " + scope + "_hits counter", + "# TYPE " + scope + "_size gauge", + } { + if n := strings.Count(out, want); n != 1 { + t.Errorf("found %q %d times, expected exactly 1:\n%s", want, n, out) + } + } + } + + // Six metrics, six type declarations, none repeated. + if n := strings.Count(out, "# TYPE "); n != 2*len(scopes) { + t.Errorf("found %d type declarations, expected %d", n, 2*len(scopes)) + } +} + +// TestTypeDeclarationAcrossAdjacentScopes is the tighter version of the case +// above: when each scope exposes the same single field name, the families are +// adjacent in the output and a dedup that compares the bare name suppresses +// every one after the first. +func TestTypeDeclarationAcrossAdjacentScopes(t *testing.T) { + now := time.Date(2017, 6, 4, 22, 12, 0, 0, time.UTC) + + handler := &Handler{} + + scopes := []string{"alpha", "beta", "gamma"} + for _, scope := range scopes { + handler.HandleMeasures(now, stats.Measure{ + Name: scope, + Fields: []stats.Field{ + stats.MakeField("hits", 1, stats.Counter), + // A histogram spans three series names, so its family only + // stays contiguous if the sort orders by scope before name. + // Sorting on the bare name groups every scope's _bucket + // together and pushes _count and _sum away from it, which + // makes the same family declare its type more than once. + stats.MakeField("latency", 0.1, stats.Histogram), + }, + }) + } + + var buf strings.Builder + handler.WriteStats(&buf) + out := buf.String() + + for _, scope := range scopes { + for _, want := range []string{ + "# TYPE " + scope + "_hits counter", + "# TYPE " + scope + "_latency histogram", + } { + if n := strings.Count(out, want); n != 1 { + t.Errorf("found %q %d times, expected exactly 1:\n%s", want, n, out) + } + } + } + + if n := strings.Count(out, "# TYPE "); n != 2*len(scopes) { + t.Errorf("found %d type declarations, expected %d:\n%s", n, 2*len(scopes), out) + } +} diff --git a/prometheus/metric.go b/prometheus/metric.go index df76b44..e941f6c 100644 --- a/prometheus/metric.go +++ b/prometheus/metric.go @@ -451,8 +451,24 @@ func (metrics byNameAndLabels) Swap(i, j int) { metrics[i], metrics[j] = metrics[j], metrics[i] } +// Less orders by scope before name, so that every metric sharing a scope and +// a root name stays contiguous in the output. +// +// Ordering on the bare name would interleave same-named fields coming from +// different engine prefixes — hits from two WithPrefix sub-engines, say — +// which breaks up the family a single "# TYPE" line is meant to cover. +// +// Comparing the two parts in turn rather than the joined "scope_name" avoids +// building a string for every comparison in the sort. func (metrics byNameAndLabels) Less(i, j int) bool { m1 := &metrics[i] m2 := &metrics[j] - return m1.name < m2.name || (m1.name == m2.name && m1.labels.less(m2.labels)) + + if m1.scope != m2.scope { + return m1.scope < m2.scope + } + if m1.name != m2.name { + return m1.name < m2.name + } + return m1.labels.less(m2.labels) } From 6af41f97242ecd2f1d7160d56eb98b7cf6b8f5b0 Mon Sep 17 00:00:00 2001 From: Sathvik Date: Sat, 19 Sep 2026 03:59:26 +0530 Subject: [PATCH 5/8] prometheus: stop exposing metric timestamps appendMetric wrote metric.time as an explicit timestamp on every sample. The field is optional in the exposition format, and a series that carries one opts out of Prometheus stale-marker handling: once the series stops being exported the scraper keeps serving its last value for five minutes rather than letting it go stale. An idle metric therefore looked live long after it stopped reporting. Dropping it lets the scraper assign scrape time, which is the behaviour every other exporter has. metric.time stays on the struct, where MetricTimeout and the store cleanup still depend on it. This changes staleness behaviour for anyone already scraping this package. Co-Authored-By: Claude Opus 5 (1M context) --- prometheus/append.go | 14 ++++++++------ prometheus/append_test.go | 7 +++++-- prometheus/handler_test.go | 22 +++++++++++----------- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/prometheus/append.go b/prometheus/append.go index 38b1e8c..94fb371 100644 --- a/prometheus/append.go +++ b/prometheus/append.go @@ -34,12 +34,14 @@ func appendMetric(b []byte, metric metric) []byte { b = append(b, ' ') b = strconv.AppendFloat(b, metric.value, 'g', -1, 64) - if !metric.time.IsZero() { - t := metric.time.Unix() * 1000 - t += int64(metric.time.Nanosecond() / 1e6) // millisecond - b = append(b, ' ') - b = strconv.AppendInt(b, t, 10) - } + // The timestamp is deliberately omitted. It is an optional field, and a + // series that carries one opts out of Prometheus stale-marker handling: + // when the series stops being exported the scraper keeps returning its + // last value for five minutes instead of letting it go stale. + // + // Leaving it off lets the scraper assign scrape time, which is what every + // other exporter does. metric.time is still tracked internally, where + // MetricTimeout and the store cleanup depend on it. return append(b, '\n') } diff --git a/prometheus/append_test.go b/prometheus/append_test.go index 35640f3..6a04e65 100644 --- a/prometheus/append_test.go +++ b/prometheus/append_test.go @@ -48,7 +48,10 @@ hello_world_bucket{le="0.5"} 42 }, { - scenario: "counter metric with help, floating point value, labels, and timestamp", + // metric.time is set here and must not reach the output: the + // timestamp is tracked for MetricTimeout and store cleanup, but + // exposing it opts the series out of stale-marker handling. + scenario: "counter metric with help, floating point value, and labels", metric: metric{ mtype: counter, scope: "global", @@ -60,7 +63,7 @@ hello_world_bucket{le="0.5"} 42 }, string: `# HELP global_hello_world This is a great metric!\n # TYPE global_hello_world counter -global_hello_world{question="\"???\"\n",answer="42"} 0.5 1496614320000 +global_hello_world{question="\"???\"\n",answer="42"} 0.5 `, }, } diff --git a/prometheus/handler_test.go b/prometheus/handler_test.go index 733732a..523cac2 100644 --- a/prometheus/handler_test.go +++ b/prometheus/handler_test.go @@ -96,21 +96,21 @@ func TestServeHTTP(t *testing.T) { b, _ := io.ReadAll(res.Body) const expects = `# TYPE A counter -A 3 1496614320000 -A{id="123"} 4 1496614320000 +A 3 +A{id="123"} 4 # TYPE B gauge -B{a="1"} 42 1496614320000 -B{a="1",b="2"} 21 1496614320000 +B{a="1"} 42 +B{a="1",b="2"} 21 # TYPE C histogram -C_bucket{le="0.25"} 2 1496614320000 -C_bucket{le="0.5"} 3 1496614320000 -C_bucket{le="0.75"} 3 1496614320000 -C_bucket{le="1"} 3 1496614320000 -C_bucket{le="+Inf"} 4 1496614320000 -C_count 4 1496614320000 -C_sum 10.7 1496614320000 +C_bucket{le="0.25"} 2 +C_bucket{le="0.5"} 3 +C_bucket{le="0.75"} 3 +C_bucket{le="1"} 3 +C_bucket{le="+Inf"} 4 +C_count 4 +C_sum 10.7 ` if s := string(b); s != expects { From 2ecadbbd3939a8aa7457906b0100be8a98119f95 Mon Sep 17 00:00:00 2001 From: Sathvik Date: Sat, 19 Sep 2026 04:00:18 +0530 Subject: [PATCH 6/8] prometheus: suffix counters with _total MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Incr("requests") produced app_requests. Prometheus names an accumulating count with a "total" suffix, and the convention is load bearing: the OpenMetrics encoder keys the type line on the suffix, so a counter without it is published as unknown rather than as a counter. The suffix is applied in newMetricEntry alongside the cached _bucket, _sum and _count names for histograms, so it covers every collection path at once. The store key keeps the raw field name, so nothing about lookup, state identity or cleanup changes — only what collect() emits. A name already ending in _total is left alone, so a program that has already adopted the convention does not produce requests_total_total. This renames every counter on this path for anyone already scraping the package. Co-Authored-By: Claude Opus 5 (1M context) --- prometheus/handler_test.go | 58 ++++++++++++++++++++++++++++++++++---- prometheus/metric.go | 20 +++++++++++-- prometheus/metric_test.go | 8 +++--- 3 files changed, 74 insertions(+), 12 deletions(-) diff --git a/prometheus/handler_test.go b/prometheus/handler_test.go index 523cac2..a0e2a23 100644 --- a/prometheus/handler_test.go +++ b/prometheus/handler_test.go @@ -95,9 +95,9 @@ func TestServeHTTP(t *testing.T) { b, _ := io.ReadAll(res.Body) - const expects = `# TYPE A counter -A 3 -A{id="123"} 4 + const expects = `# TYPE A_total counter +A_total 3 +A_total{id="123"} 4 # TYPE B gauge B{a="1"} 42 @@ -225,7 +225,7 @@ func TestTypeDeclarationPerScope(t *testing.T) { for _, scope := range scopes { for _, want := range []string{ - "# TYPE " + scope + "_hits counter", + "# TYPE " + scope + "_hits_total counter", "# TYPE " + scope + "_size gauge", } { if n := strings.Count(out, want); n != 1 { @@ -271,7 +271,7 @@ func TestTypeDeclarationAcrossAdjacentScopes(t *testing.T) { for _, scope := range scopes { for _, want := range []string{ - "# TYPE " + scope + "_hits counter", + "# TYPE " + scope + "_hits_total counter", "# TYPE " + scope + "_latency histogram", } { if n := strings.Count(out, want); n != 1 { @@ -284,3 +284,51 @@ func TestTypeDeclarationAcrossAdjacentScopes(t *testing.T) { t.Errorf("found %d type declarations, expected %d:\n%s", n, 2*len(scopes), out) } } + +// TestCounterTotalSuffix covers the _total naming rule. Only counters get the +// suffix, and a counter that already carries it is left alone. +func TestCounterTotalSuffix(t *testing.T) { + now := time.Date(2017, 6, 4, 22, 12, 0, 0, time.UTC) + + handler := &Handler{} + handler.HandleMeasures(now, stats.Measure{ + Name: "svc", + Fields: []stats.Field{ + stats.MakeField("requests", 1, stats.Counter), + stats.MakeField("errors_total", 2, stats.Counter), + stats.MakeField("queue_depth", 3, stats.Gauge), + stats.MakeField("latency", 0.1, stats.Histogram), + }, + }) + + var buf strings.Builder + handler.WriteStats(&buf) + out := buf.String() + + for _, want := range []string{ + "# TYPE svc_requests_total counter", + "svc_requests_total 1", + // Already suffixed: must not become errors_total_total. + "# TYPE svc_errors_total counter", + "svc_errors_total 2", + // Gauges and histograms are untouched. + "# TYPE svc_queue_depth gauge", + "svc_queue_depth 3", + "# TYPE svc_latency histogram", + "svc_latency_count 1", + } { + if !strings.Contains(out, want) { + t.Errorf("missing %q in output:\n%s", want, out) + } + } + + for _, unwanted := range []string{ + "svc_errors_total_total", + "svc_queue_depth_total", + "svc_latency_total", + } { + if strings.Contains(out, unwanted) { + t.Errorf("unexpected %q in output:\n%s", unwanted, out) + } + } +} diff --git a/prometheus/metric.go b/prometheus/metric.go index e941f6c..2665a6a 100644 --- a/prometheus/metric.go +++ b/prometheus/metric.go @@ -151,9 +151,23 @@ func newMetricEntry(mtype metricType, scope, name, help string) *metricEntry { states: make(metricStateMap), } - if mtype == histogram { - // Here we cache those metric names to avoid having to recompute them - // every time we collect the state of the metrics. + // Here we cache those metric names to avoid having to recompute them + // every time we collect the state of the metrics. + switch mtype { + case counter: + // Prometheus expects an accumulating count to carry a "total" suffix. + // It is more than convention: the OpenMetrics encoder keys the type + // line on the suffix, so a counter without it is published as + // unknown. + // + // A name that already ends in _total is left alone, so a program that + // has already adopted the convention does not end up with + // requests_total_total. + if !strings.HasSuffix(name, "_total") { + entry.name = name + "_total" + } + + case histogram: entry.bucket = name + "_bucket" entry.sum = name + "_sum" entry.count = name + "_count" diff --git a/prometheus/metric_test.go b/prometheus/metric_test.go index f3b6c33..7f5da5e 100644 --- a/prometheus/metric_test.go +++ b/prometheus/metric_test.go @@ -100,7 +100,7 @@ func TestMetricStore(t *testing.T) { {mtype: counter, scope: "test", name: "A", value: 2}, {mtype: histogram, scope: "test", name: "C", value: 0.1}, {mtype: gauge, scope: "test", name: "B", value: 1, labels: labels{{"a", "1"}, {"b", "2"}}}, - {mtype: counter, scope: "test", name: "A", value: 4, labels: labels{{"id", "123"}}}, + {mtype: counter, scope: "test", name: "A_total", value: 4, labels: labels{{"id", "123"}}}, {mtype: gauge, scope: "test", name: "B", value: 42, labels: labels{{"a", "1"}}}, {mtype: histogram, scope: "test", name: "C", value: 0.1}, {mtype: gauge, scope: "test", name: "B", value: 21, labels: labels{{"a", "1"}, {"b", "2"}}}, @@ -123,8 +123,8 @@ func TestMetricStore(t *testing.T) { sort.Sort(byNameAndLabels(metrics)) expects := []metric{ - {mtype: counter, scope: "test", name: "A", value: 3, labels: labels{}}, - {mtype: counter, scope: "test", name: "A", value: 4, labels: labels{{"id", "123"}}}, + {mtype: counter, scope: "test", name: "A_total", value: 3, labels: labels{}}, + {mtype: counter, scope: "test", name: "A_total", value: 4, labels: labels{{"id", "123"}}}, {mtype: gauge, scope: "test", name: "B", value: 42, labels: labels{{"a", "1"}}}, {mtype: gauge, scope: "test", name: "B", value: 21, labels: labels{{"a", "1"}, {"b", "2"}}}, {mtype: histogram, scope: "test", name: "C_bucket", value: 2, labels: labels{{"le", "0.25"}}}, @@ -253,7 +253,7 @@ func TestMetricStoreCleanup(t *testing.T) { sort.Sort(byNameAndLabels(metrics)) if !reflect.DeepEqual(metrics, []metric{ - {mtype: counter, name: "E", value: 1, time: now.Add(time.Second), labels: labels{}}, + {mtype: counter, name: "E_total", value: 1, time: now.Add(time.Second), labels: labels{}}, }) { t.Errorf("bad metrics: %#v", metrics) } From 63a71a2e36d830b51908b1cbaea1c03434ed24fd Mon Sep 17 00:00:00 2001 From: Sathvik Date: Sat, 19 Sep 2026 05:07:46 +0530 Subject: [PATCH 7/8] stats: add Engine.SetBuckets for prefix-aware bucket registration Observe and Buckets.Set name the same metric differently. Observe takes a name relative to the engine and has the prefix attached after the name is split; Set attaches no prefix and merely splits what it is handed, so it needs the fully-qualified name. Registering buckets therefore means restating the engine prefix, and getting it wrong is an ordinary map miss: a mistyped key and no key at all produce identical output, so the histogram silently loses its buckets with no error anywhere. Deriving sub-engines with WithPrefix makes this worse, since buckets then have to be registered once per derived prefix, and services derive a dozen. SetBuckets moves key construction to the engine, which is the only thing that knows its own prefix. Callers pass the same string they pass to Observe, so the two cannot drift, and a sub-engine computes its own key. Additive: Buckets.Set is unchanged and keeps working. The test reads the expected key back out of what Observe actually emitted rather than restating the derivation, so it fails if either side changes. Co-Authored-By: Claude Opus 5 (1M context) --- buckets_test.go | 150 ++++++++++++++++++++++++++++++++++++++++++++++++ engine.go | 28 +++++++++ 2 files changed, 178 insertions(+) create mode 100644 buckets_test.go diff --git a/buckets_test.go b/buckets_test.go new file mode 100644 index 0000000..f59c500 --- /dev/null +++ b/buckets_test.go @@ -0,0 +1,150 @@ +package stats_test + +import ( + "strings" + "testing" + + stats "github.com/segmentio/stats/v5" + "github.com/segmentio/stats/v5/prometheus" + "github.com/segmentio/stats/v5/statstest" +) + +// TestEngineSetBucketsKeyMatchesObserve pins the invariant SetBuckets exists +// for: the registry key it writes is exactly the one Observe produces for the +// same name, on the engine it was called on and on any sub-engine derived +// from it. +// +// The key is read back from what Observe actually emitted rather than being +// restated here, so the test fails if either side of the pair changes. +func TestEngineSetBucketsKeyMatchesObserve(t *testing.T) { + for _, test := range []struct { + scenario string + name string + engine func(stats.Handler) *stats.Engine + }{ + { + scenario: "engine with a prefix", + name: "latency", + engine: func(h stats.Handler) *stats.Engine { return stats.NewEngine("app", h) }, + }, + { + scenario: "engine with no prefix", + name: "latency", + engine: func(h stats.Handler) *stats.Engine { return stats.NewEngine("", h) }, + }, + { + scenario: "sub-engine derived with WithPrefix", + name: "latency", + engine: func(h stats.Handler) *stats.Engine { + return stats.NewEngine("app", h).WithPrefix("sub") + }, + }, + { + scenario: "sub-engine derived twice", + name: "latency", + engine: func(h stats.Handler) *stats.Engine { + return stats.NewEngine("app", h).WithPrefix("sub").WithPrefix("deeper") + }, + }, + { + // Observe splits the name on its last dot and prefixes only the + // measure half, so SetBuckets has to do the same. + scenario: "dotted name", + name: "db.latency", + engine: func(h stats.Handler) *stats.Engine { return stats.NewEngine("app", h) }, + }, + } { + t.Run(test.scenario, func(t *testing.T) { + name := test.name + + h := &statstest.Handler{} + e := test.engine(h) + + e.SetBuckets(name, 0.1, 0.2, 0.3) + e.Observe(name, 0.15) + + // Find the measure Observe produced, skipping the go_version + // gauge the engine reports once. + var key stats.Key + var found bool + for _, m := range h.Measures() { + for _, f := range m.Fields { + if strings.HasSuffix(name, f.Name) { + key = stats.Key{Measure: m.Name, Field: f.Name} + found = true + } + } + } + if !found { + t.Fatalf("Observe produced no measure for %q", name) + } + + buckets, ok := stats.Buckets[key] + if !ok { + t.Fatalf("SetBuckets did not register %#v; registry holds %#v", + key, keysOf(stats.Buckets)) + } + if len(buckets) != 3 { + t.Errorf("registered %d buckets, expected 3", len(buckets)) + } + + delete(stats.Buckets, key) + }) + } +} + +// TestEngineSetBucketsEndToEnd runs the whole chain: buckets registered on a +// sub-engine reach the exposition, rather than the metric silently falling +// back to the default set. +func TestEngineSetBucketsEndToEnd(t *testing.T) { + ph := &prometheus.Handler{} + e := stats.NewEngine("svc", ph).WithPrefix("sub") + + e.SetBuckets("latency", 0.1, 0.2, 0.3) + defer delete(stats.Buckets, stats.Key{Measure: "svc.sub", Field: "latency"}) + + e.Observe("latency", 0.15) + + var buf strings.Builder + ph.WriteStats(&buf) + out := buf.String() + + for _, want := range []string{ + `svc_sub_latency_bucket{le="0.1"} 0`, + `svc_sub_latency_bucket{le="0.2"} 1`, + `svc_sub_latency_bucket{le="0.3"} 1`, + `svc_sub_latency_bucket{le="+Inf"} 1`, + `svc_sub_latency_count 1`, + } { + if !strings.Contains(out, want) { + t.Errorf("missing %q in output:\n%s", want, out) + } + } + + // Three registered boundaries plus +Inf. More would mean the registration + // missed and DefaultBuckets was used instead. + if n := strings.Count(out, "svc_sub_latency_bucket{"); n != 4 { + t.Errorf("found %d bucket series, expected 4:\n%s", n, out) + } +} + +// TestBucketsSetStillWorks covers the pre-existing registration path, which +// SetBuckets is additive to. +func TestBucketsSetStillWorks(t *testing.T) { + key := stats.Key{Measure: "legacy.svc", Field: "latency"} + defer delete(stats.Buckets, key) + + stats.Buckets.Set("legacy.svc.latency", 0.1, 0.2) + + if _, ok := stats.Buckets[key]; !ok { + t.Errorf("Buckets.Set did not register %#v", key) + } +} + +func keysOf(b stats.HistogramBuckets) []stats.Key { + keys := make([]stats.Key, 0, len(b)) + for k := range b { + keys = append(keys, k) + } + return keys +} diff --git a/engine.go b/engine.go index c2d63d6..dc07fd9 100644 --- a/engine.go +++ b/engine.go @@ -130,6 +130,34 @@ func (e *Engine) ObserveAt(t time.Time, name string, value any, tags ...Tag) { e.measure(t, name, value, Histogram, tags...) } +// SetBuckets registers histogram buckets for the metric that Observe(name) +// reports, deriving the registry key from the engine's own prefix. +// +// Buckets is a single global registry shared by every engine, so it cannot +// infer which engine a name belongs to: HistogramBuckets.Set takes the +// fully-qualified name and merely splits what it is handed, while Observe +// takes a name relative to the engine and has the prefix attached afterwards. +// Registering buckets through Set therefore means restating the prefix, and 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. +// +// SetBuckets removes that by moving key construction to the engine, which does +// know its prefix. Callers pass the same string they pass to Observe: +// +// e := stats.NewEngine("app", h) +// e.SetBuckets("latency", 0.005, 0.01, 0.025, 0.05, 0.1) +// e.Observe("latency", d) +// +// A sub-engine derived with WithPrefix computes its own key, so buckets no +// longer have to be registered once per derived prefix. +// +// It writes to the global Buckets registry, so it must be called before the +// metrics it covers start being reported — an init function or program setup. +// The existing Buckets.Set keeps working unchanged. +func (e *Engine) SetBuckets(name string, buckets ...any) { + Buckets.Set(e.makeName(name), buckets...) +} + // Clock returns a new clock identified by name and tags. func (e *Engine) Clock(name string, tags ...Tag) *Clock { return e.ClockAt(name, time.Now(), tags...) From 7098e8a9811cd8023960070eaec6fd5f28689ecf Mon Sep 17 00:00:00 2001 From: Sathvik Date: Sun, 20 Sep 2026 01:39:45 +0530 Subject: [PATCH 8/8] docs: record the prometheus exposition changes HISTORY.md leads the v5.11.0 entry with the breaking change, since nothing fails to compile but every counter is renamed and staleness behaviour changes for anyone already scraping the package. The README gains the bucket registration the handler now needs, using Engine.SetBuckets. Snippet compile-checked. Co-Authored-By: Claude Opus 5 (1M context) --- HISTORY.md | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ README.md | 19 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/HISTORY.md b/HISTORY.md index 86f5b51..e6f7603 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -2,6 +2,57 @@ ### v5.11.0 (Unreleased) +**The `prometheus` handler's exposition has changed. Nothing fails to compile, +but the series it publishes are different, so anyone already scraping this +package will see renamed metrics and different staleness behaviour on upgrade.** +The `datadog`, `influxdb`, `otlp` and `veneur` handlers are untouched. + +What changed, and why: + +- **Counters are now suffixed with `_total`.** `Incr("requests")` published + `app_requests` and now publishes `app_requests_total`. This is not only + Prometheus naming convention: the OpenMetrics encoder keys the type line on + the suffix, so a counter without it was published as `unknown`. A name that + already ends in `_total` is left alone. **Queries and dashboards referring to + the old names need updating.** + +- **Histograms always emit a `+Inf` bucket.** The handler allocated exactly one + bucket per registered boundary and never appended an overflow bucket, so + observations above the highest boundary were counted in `_sum` and `_count` + but landed in no bucket at all. `histogram_quantile()` returns `NaN` unless + the highest bucket is `+Inf`, so no histogram published by this handler could + be evaluated. + +- **Histograms with no registered boundaries fall back to `prometheus.DefaultBuckets`.** + `stats.Buckets` is empty by default and a miss returned a nil slice with no + error, so such a histogram published `_sum` and `_count` with no `_bucket` + series and nothing looked wrong. The defaults are the reference Prometheus + client's, suited to latencies in seconds; they are a floor, not a substitute + for choosing boundaries. **This adds bucket series for histograms that + previously published none.** + +- **Bucket `le` labels sort numerically.** They compared as raw strings, which + put `+Inf` first and `10` ahead of `2`. + +- **`# TYPE` is declared once per scope, not once per field name.** The dedup + discarded the scope, so same-named fields coming from different engine + prefixes looked like repeats and every one after the first was published + untyped. Sub-engines derived with `WithPrefix` exist precisely so subsystems + can reuse short field names, so this fired readily. + +- **Timestamps are no longer exposed.** The field is optional, and a series + carrying one opts out of Prometheus stale-marker handling — the scraper kept + serving the last value for five minutes after a series stopped being + exported. The scraper now assigns scrape time. `MetricTimeout` is unaffected. + +- **New: `Engine.SetBuckets(name, buckets...)`.** `Observe` takes a name + relative to the engine, while `HistogramBuckets.Set` needs the + fully-qualified name, so registering buckets meant restating the engine + prefix — and a mismatch was an ordinary map miss, indistinguishable from no + registration at all. `SetBuckets` derives the key from the engine's own + prefix, so callers pass the same string they pass to `Observe` and a + `WithPrefix` sub-engine computes its own key. `Buckets.Set` is unchanged. + **The minimum supported Go version is now 1.26.** The `golang.org/x/*` modules (`net`, `sys`, `sync`, `text`) all declare `go 1.26.0` as of their latest releases, and `stats` depends on them both directly and transitively through diff --git a/README.md b/README.md index e41627f..3bed6b7 100644 --- a/README.md +++ b/README.md @@ -205,6 +205,25 @@ http.Handle("/metrics", prometheus.DefaultHandler) instance. Construct your own `&prometheus.Handler{}` literal instead if you need to set `TrimPrefix`, `MetricTimeout`, or `Buckets`. +Counters are published with a `_total` suffix, and the handler leaves timestamps +off so the scraper assigns scrape time. + +Histograms need bucket boundaries. Register them with `Engine.SetBuckets`, which +takes the same name you pass to `Observe` and derives the registry key from the +engine's own prefix: + +```go +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) +``` + +A sub-engine derived with `WithPrefix` computes its own key, so buckets do not +have to be registered once per derived prefix. Histograms with nothing +registered fall back to `prometheus.DefaultBuckets`, which suits latencies +measured in seconds — a floor that keeps percentiles computable, not a +substitute for picking boundaries where accuracy matters. + ### InfluxDB The [github.com/segmentio/stats/v5/influxdb](https://godoc.org/github.com/segmentio/stats/v5/influxdb) package sends metrics to InfluxDB using the line protocol over HTTP.