[sathvik09]

increase() never counts the first sample of a series, because that sample is taken as the series' baseline. The handler creates a series on its first update and deletes it after MetricTimeout, so sparse, high-cardinality counters lose their first value again every time a series is created. Prometheus fixes this with created timestamps: the target publishes when each series started, and a scraper with start-timestamp zero ingestion writes a 0 at that time. This PR has the handler publish them.

What changes:

• metricState.created is set on a series' first update. A first update with a zero time uses the wall clock instead, because a zero created time means none is set. A series re-created after MetricTimeout gets a new one. A histogram bucket rebuild leaves it unchanged, because _sum and _count keep counting.
• ServeHTTP serves the protobuf exposition (application/vnd.google.protobuf; proto=io.prometheus.client.MetricFamily; encoding=delimited) when the Accept header ranks it above every text type. Counters and histograms carry created_timestamp; gauges don't. Gzip works for both formats. Responses carry Vary: Accept, Accept-Encoding, since both headers choose the response. The messages are built after the store lock is released, one family at a time.
• Names and labels are rendered as WriteStats renders them. That covers the _total suffix, exposedName collisions and invalid-byte replacement, so a Prometheus 3.x scrape that switches format keeps the same series. On Prometheus 2.x, histogram buckets with integer bounds start new series (le="1" becomes le="1.0"), because the scraper formats a protobuf bound itself and only 3.x normalizes the text value to match. Two entries that render to the same name (a.b and a_b) are merged into one family and ordered as WriteStats orders them, so a scraper keeps the same duplicate from either format on every scrape. Histograms leave out the +Inf bucket, as client_golang does; the scraper derives it from sample_count.
• Text writes le="+Inf" from _count instead of the bucket total, so +Inf == _count holds in both formats. The two used to differ after a NaN observation, which lands in no bucket, or a bucket rebuild, which zeroes the buckets but not _count.
• Text 0.0.4 stays the default. Its body is unchanged apart from the +Inf fix above. A scraper receives protobuf only when it ranks it first: a scrape configured with PrometheusProto first, or a Prometheus with native histograms or created-timestamp zero ingestion on, which puts it first by default. Such a Prometheus switches to protobuf when the application upgrades, with no change to its configuration. Every other scraper keeps receiving text.

New dependency: github.com/prometheus/client_model v0.6.3, for the generated MetricFamily types. Its only dependency is google.golang.org/protobuf, which stats already uses.

Tests:

• Accept negotiation, including the headers Prometheus 3.x sends by default and with protobuf first, and OpenMetrics ranked above protobuf.
• A decoded protobuf scrape with and without gzip, including the Vary header.
• A new created time after MetricTimeout, and a zero-time first update.
• The protobuf and text outputs yielding the same series set: names, full label sets and values, for counters, gauges and histograms with integer and fractional bounds.
• Colliding names giving identical bytes over 100 scrapes, in text order.
• +Inf equal to _count in both formats after a NaN and after a bucket rebuild.
• A scrape racing measures and expiry.

go test -race ./... and golangci-lint pass.

Benchmarks: BenchmarkCollect, BenchmarkProtoEntries and BenchmarkWriteProtobuf in protobuf_test.go, at 100k series (90k counters, 10k histograms). BenchmarkWriteProtobuf spreads them over 2 families and over 100. The protobuf path copies series under the store lock, as collect() does, then builds and writes one family at a time after releasing it. Ten runs each, compared with benchstat on an Apple M4 Pro:

	Time	Memory	Allocations	Peak heap
Text, under the lock (collect())	26.8 ms ± 5%	126 MiB	15
Protobuf, under the lock (copy)	18.7 ms ± 3%	63 MiB	79
Protobuf, whole build and write, 2 families	288 ms ± 4%	165 MiB	2.59M	88 MiB
Protobuf, whole build and write, 100 families	208 ms ± 1%	145 MiB	2.59M	100 MiB

The first version built the messages under the lock, holding it for 169 ms with 2.59M allocations. Building and writing one family at a time instead of all of them first lowers peak heap from 112 MiB to 88 MiB (2 families) and 100 MiB (100 families), with time and allocations unchanged. Peak heap is HeapAlloc sampled at each write, above its level before the call. It includes garbage not yet collected, so compare it between rows rather than reading it as absolute.

🤖 Generated with Claude Code

[sathvik09]

Review Summary

This PR adds a protobuf output format to the Prometheus handler, which a scraper gets only by ranking it first in its Accept header. Counters and histograms in that format carry a created_timestamp, so increase() counts a new series' first value. Text stays the default. I found no blocking defects. The biggest risks are two claims that are too broad: which scrapers switch to protobuf, and that switching keeps every series the same. The new HISTORY.md entry repeats both claims. 5 findings: 0 critical, 0 high; 5 non-blocking.

Recommendation: APPROVE
Readiness: NOT READY: RR-02 (fail). This is report only and doesn't change the review.

✅ Approve Basis

• Prometheus's protobuf parser rebuilds the +Inf bucket from sample_count when the last bound isn't +Inf (prometheus@v0.314.0 model/textparse/protobufparse.go:690-698). Leaving it out is correct.
• Prometheus 3.x reads the same field numbers under the name start_timestamp, so the created timestamps reach it.
• The text path is unchanged. ServeHTTP still writes text/plain; version=0.0.4 by default (handler.go L156-L164).
• go test -race ./prometheus, go vet and golangci-lint all pass.

Non-blocking findings

[medium] Scrapers with some feature flags already put protobuf first — protobuf.go L24-L28

The comment, the PR body and HISTORY.md say only a scrape configured with PrometheusProto first gets protobuf. But Prometheus changes its default protocol order to protobuf-first when native histograms or created-timestamp zero ingestion are on (prometheus@v0.314.0 config/config.go:181, 598-608, 948-957). Those scrapers switch format as soon as they pick up this version of stats, with no config change.
Suggestion: Say this in the comment, the PR body and HISTORY.md, so operators know which scrapers will switch.
high confidence · found by two passes

[medium] Integer le values change on Prometheus 2.x scrapers — protobuf.go L199-L202

UpperBound: proto.Float64(bucket.limit) reaches the scraper as le="1.0", while the text path writes le="1". Prometheus 3.x normalizes both to the same value. Prometheus 2.x (v0.53.0 promparse) doesn't, so on 2.x every histogram bucket with an integer bound starts a new _bucket series after the switch. This contradicts "a scrape that switches format keeps the same series", which the PR body and HISTORY.md both say. Combined with the previous finding, this can happen without anyone opting in.
Suggestion: Limit the claim to Prometheus 3.x and Alloy builds based on it, or document the le change for 2.x.
medium confidence · found by one pass

[medium] OpenMetrics is ignored when ranking formats — protobuf.go L46-L54

Only text/plain, text/* and */* count as text. application/openmetrics-text falls through to default: continue. So openmetrics;q=0.5, protobuf;q=0.4, text/plain;q=0.3 serves protobuf, which was confirmed with a scratch test. That breaks the docstring's rule ("above every text type it accepts") and moves a scraper configured [OpenMetricsText1.0.0, PrometheusProto, …] to protobuf.
Suggestion: Add application/openmetrics-text to the text case, which keeps it answered with text 0.0.4 as today, or reword the docstring. Either way, add this header to the negotiation test.
high confidence · found by two passes

[medium] Protobuf scrape holds the store read lock about 7x longer — protobuf.go L101-L141

families() builds every dto.Metric, LabelPair and label-name string while holding store.mutex.RLock(). At 100k series it takes 85 ms and 2.2M allocations, against 12 ms and 11 allocations for the text path's collect(). While it runs, a new metric name's write lock waits, and that waiting writer blocks the read locks HandleMeasures needs.
Suggestion: Copy plain values under the lock, as collect() does, and build the protobuf messages after RUnlock.
medium confidence · found by one pass (scratch benchmark)

[medium] The name-parity test covers only counters with one label — protobuf_test.go L240-L264

TestProtobufMatchesTextNames reads only m.Label[0] and checks with strings.Contains. Histograms, le values, dotted scopes, invalid label bytes and series with several labels are never compared, and those are where the formats drift (see the le finding).
Suggestion: Parse both outputs into full label sets and compare them, including a histogram with integer bounds.
high confidence · found by one pass

Notes

• protobuf.go L116-L118: the comment says the text format declares a merged family once, but WriteStats prints # TYPE s_a_b_total counter twice for a.b and a_b. Only the wording is wrong; protobuf merges them correctly.
• The +Inf value can differ between the formats after a NaN observation or a bucket rebuild. Text reports the bucket sum and protobuf reports sample_count. Protobuf is the more correct of the two, and the choice is deliberate.
• The family key includes the metric type, so a gauge and a histogram that render to the same name produce two MetricFamily messages with that name. The text output already prints TYPE twice in that case, so this isn't a regression.
• The writeProtobuf error is discarded (handler.go L174), as text write errors already are. A failure part-way through would cut the scrape short without a log.
• If a caller passes a zero time, created is set by a later update, after a value has already been counted. stats.Engine always passes time.Now(), so no real input triggers this.
• Locks are taken in the same order as in collect() (store, then entry, then state), and the race detector is clean.
• client_model v0.6.2 is proto2, so invalid UTF-8 is rejected by the scraper, the same as with text. go mod tidy changes nothing.

📋 Coverage

Depth: full review, because the change alters the HTTP response format and adds a dependency.

• What breaks: one pass. It read the diff, prometheus/{protobuf,handler,metric,append,label}.go, buckets.go, and the client_model v0.6.2 metrics.proto, ran vet and the tests, and ran a scratch benchmark and an Accept header check.
• Callers, claims, runtime, security and tests: one shared pass. It read the same files plus handler_test.go, and Prometheus v0.314.0 and v0.53.0 (scrape config defaults, the Accept header Prometheus sends, and the text and protobuf parsers). It ran go test -race and golangci-lint.
• Scope: the code was reviewed at bbde2e8. The later commit 64da5c6 changes only HISTORY.md and was read on its own. Its claims are covered by the first two findings.
• Not read: Alloy's source. govulncheck and gosec aren't installed, so no dependency vulnerability scan ran.
• Skipped: the optional design pass and the readiness checklist for human reviewers.
• Recheck: not run, because no finding was high enough to block.
• Policy scan: clean (POL-001 to POL-003).

---

_{Reviewed by review-toolkit}

[sathvik09]

Inline findings from the review summary comment. All are non-blocking.

[sathvik09]

[medium] Scrapers with some feature flags already put protobuf first

Prometheus changes its default protocol order to protobuf-first when native histograms or created-timestamp zero ingestion are on (config/config.go:181, 598-608, 948-957 in v0.314.0). So "neither puts protobuf first unless configured to" is too broad: those scrapers switch format on upgrade with no config change. The PR body and HISTORY.md say the same thing.

Suggestion: Say this here, in the PR body and in HISTORY.md.

[sathvik09]

Fixed in 04d508b. The docstring, HISTORY.md and the PR body now say that Prometheus also puts protobuf first by default when native histograms or created-timestamp zero ingestion are on, and that such a scraper switches to protobuf when the application upgrades, with no change to its configuration. The behaviour stays as it is: ranking by q is the right negotiation, and those scrapers are asking for protobuf.

[sathvik09]

Prometheus changes its default protocol order to protobuf-first when either of these is on:

• native histograms
• created-timestamp zero ingestion

[sathvik09]

[medium] OpenMetrics is ignored when ranking formats

application/openmetrics-text falls through to default: continue, so openmetrics;q=0.5, protobuf;q=0.4, text/plain;q=0.3 serves protobuf. That breaks the docstring rule ("above every text type it accepts").

Suggestion: Add application/openmetrics-text to the text case, or reword the docstring. Either way, add this header to the negotiation test.

[sathvik09]

Keeping the behaviour and fixing the docstring.

stats serves two formats: text 0.0.4, as before, and protobuf, which this PR adds. We picked protobuf over OpenMetrics deliberately, because it is the format Prometheus recommends for created timestamps (see the feature flag docs). OpenMetrics carries them as _created lines, which are error-prone, expensive to parse, and add extra series to storage. Enabling zero ingestion puts PrometheusProto first for the same reason.

Because stats does not serve OpenMetrics, acceptsProtobuf only ranks the two formats it does serve. For openmetrics;q=0.5, protobuf;q=0.4, text/plain;q=0.3, the scraper prefers protobuf over plain text, so it gets protobuf. Counting OpenMetrics as text would give that scraper plain text even though it ranked protobuf higher. This header only comes from an explicit scrape_protocols that lists OpenMetrics above PrometheusProto; Prometheus's default lists either leave protobuf out or put it first.

The docstring now says protobuf is served when the scraper ranks it above the text format. TestAcceptsProtobuf covers this header and expects protobuf. Fixed in 9ae25aa.

[sathvik09]

[medium] Protobuf scrape holds the store read lock about 7x longer

All dto messages are built under store.mutex.RLock(). At 100k series that is 85 ms and 2.2M allocations, against 12 ms for collect(). A waiting writer (a new metric name) then blocks the read locks HandleMeasures needs.

Suggestion: Copy plain values under the lock, as collect() does, and build the messages after RUnlock.

[sathvik09]

Fixed in 1ccab19. families() now copies each series into a plain struct under the locks (protoEntries), as collect() does, and builds the dto messages after releasing them. Bucket counts are copied into one shared backing array, so a histogram series costs no allocation under the lock.

Benchmarks at 100k series (90k counters and 10k histograms, Apple M4 Pro), now in protobuf_test.go:

	Time under the store lock	Allocs under the lock
collect() (text)	28.8 ms	15
families() before	170 ms (the whole build)	2.59M
protoEntries() after	19.6 ms	79

The whole protobuf build still takes about the same time (~187 ms) and allocates more (162 MB against 108 MB, for the extra copy), but none of it blocks writers any more.

[sathvik09]

[medium] Integer le values change on Prometheus 2.x scrapers

The scraper renders UpperBound 1 as le="1.0", while the text path writes le="1". Prometheus 3.x normalizes both to the same value; 2.x (v0.53.0 promparse) does not. So on 2.x, switching format starts new _bucket series, which contradicts "a scrape that switches format keeps the same series".

Suggestion: Limit the claim to Prometheus 3.x, or document the le change for 2.x.

[sathvik09]

Fixed in c8f1cb3 by narrowing the claim. stats cannot fix this side: with protobuf the scraper formats UpperBound into the le label itself, and stats only sends a float.

The writeProtobuf docstring, HISTORY.md and the PR body now say series names are unchanged on Prometheus 3.x. They add that on 2.x, histogram buckets with integer bounds start new series (le="1" becomes le="1.0") when a scrape switches format, because only 3.x normalizes the text value to match. Grafana Alloy embeds Prometheus 3.x (v0.312.0 in Alloy v1.18.1, v0.314.0 in v1.21.3), so Alloy scrapers keep the same series.

[sathvik09]

[medium] The name-parity test covers only counters with one label

It reads only m.Label[0] and checks with strings.Contains. Histograms, le values, dotted scopes and series with several labels are never compared.

Suggestion: Parse both outputs into full label sets and compare them, including a histogram with integer bounds.

[sathvik09]

Fixed in ac502e6. TestProtobufMatchesText replaces the old test. It expands both outputs into the series a scraper stores, compares them in both directions and requires the values to match:

• protobuf: a series per counter and gauge; per histogram, _bucket for each bound plus +Inf from sample_count, then _sum and _count
• text: each line parsed into its name, full label set and value

le is compared as a number, matching Prometheus 3.x normalization (see the comment on le above).

The inputs cover a histogram with integer and fractional bounds (0.25, 1, 5), a dotted scope, two labels including one that needs sanitizing (status.code), a gauge, a second label set on the same counter, and the hits / hits_total collision.

I checked that the test fails on purpose-made bugs: dropping all but the first label, off-by-one bucket counts, and a wrong _sum. It caught all three.

[tobias-eyl-twilio]

Review of correctness and production readiness for created timestamps over protobuf.

Verified OK: -race passes, and the lock order (store → entry → state) matches collect(). Accept negotiation is correct against the real Prometheus 3.x default headers, including the proto-first ones. With the real Prometheus 3.x parser, text and protobuf yield the same series (le normalization, _total, derived +Inf). Gauges carry no created time, a series re-created after MetricTimeout gets a new created time, go vet is clean, and v5.13.0 is the right next version.

Needs changes before merge: non-deterministic ordering of colliding series/families, and a late created after a zero-time first update (inline).

Not run: golangci-lint, Prometheus 2.x parsing, and a live scrape → Alloy → Mimir test.

[tobias-eyl-twilio]

Major: colliding series and families are emitted in random order.

byKey is a map, and both slices.SortFunc calls are unstable with no tie-break. When a.b and a_b render to the same name with identical labels (or a counter and a gauge share a name), the order changes from scrape to scrape. Prometheus keeps the first duplicate and drops the other, so the stored value flips between two different counters, which increase() reads as resets.

A local probe over 200 calls put value 1 first 169 times and value 2 first 31 times; family order flipped too. The text path is deterministic.

Suggested fix: slices.SortStableFunc plus a tie-break on scope, raw name and type for both series and families, and a test that scrapes the collision case repeatedly and asserts identical bytes.

[sathvik09]

Fixed in 11e0021. Entries are now sorted by scope, raw name and type before merging, which is the order WriteStats uses. Series sort stably on labels, and when a counter and a gauge share a name, their families are ordered by their first entry. So protobuf keeps the same duplicate as text (a.b before a_b). TestProtobufCollisionOrder scrapes the collision case 100 times and asserts identical bytes, plus the text-matching order. It fails without the fix.

[tobias-eyl-twilio]

Nit: this comment does not match the text path. In the collision case WriteStats declares # TYPE twice, because declared is keyed by {scope, rootName} (handler.go:183-220), not by the rendered name. Suggest rewording to say what protobuf does without claiming text parity.

[sathvik09]

Fixed in 20b23c4. The comment now says only what protobuf does: colliding entries are merged into one family, so the name is declared once. The text-parity claim is gone.

[tobias-eyl-twilio]

Major: a zero-time first update leaves created too late.

If the first update has time.IsZero() (ReportAt with a zero time, or a direct HandleMeasures), created stays zero, and the next update sets it to that later time. Example: Add 5 at zero time, then Add 1 at now, publishes value 6, created=now. The scraper writes a 0 at now after it already holds a sample of 5, which reads as a reset and double-counts.

Today the engine's only zero-time measure is the go_version gauge, so user ReportAt callers are the exposure. Suggested fix: track first-seen with a bool, and fall back to time.Now() when the measure time is zero. Worth a test.

[sathvik09]

Fixed in 8d11553. When the series has no created time yet and the measure time is zero, created falls back to time.Now(). That is never zero, so created can't fall back into the unset state, and no extra bool is needed. TestCreatedTimestampZeroTime adds 5 at zero time, then 1 a minute later, and asserts value 6 with created at the first update. It fails without the fix (created lands on the second update).

[tobias-eyl-twilio]

Minor: +Inf can differ between formats. Text writes le="+Inf" as the sum of bucket counts, while protobuf lets the scraper derive it from sample_count. After a NaN observation or a bucket rebuild (which zeroes buckets but not _count), the two disagree (2 vs 3 in a probe). The text-side drift predates this PR, but switching a target from text to protobuf will step +Inf. Options: emit an explicit +Inf bucket, or count NaN consistently. A NaN/rebuild test would pin whichever is chosen.

[sathvik09]

Fixed in e1b2131 by making text follow the protobuf behaviour rather than the other way round. Text now writes le="+Inf" from _count, so +Inf == _count holds in both formats, including after a NaN or a rebuild. Sending an explicit +Inf in protobuf would have kept the formats in step, but both would then break that rule. Text output only changes in those two cases, where it was already inconsistent. Routing NaN into the +Inf bucket is no longer needed, because +Inf no longer depends on the bucket total. TestHistogramInfBucket covers a NaN observation and a bucket rebuild and checks +Inf in both formats. Without the fix, text gives 2 and 1 against _count=3.

[tobias-eyl-twilio]

Minor:

• The error from writeProtobuf (and gzip Close via defer) is dropped. A partial body is silently served; at least worth a comment, or logging if the handler has a logger.
• Responses vary by Accept now; consider Vary: Accept alongside the content type.
• Memory: at 100k series, protobuf built ~155 MiB / 2.59M allocations per scrape vs ~126 MiB / 17 allocations for text in a local benchmark. Building and marshalling one family at a time would lower the peak.

[sathvik09]

Addressed in 470da00:

• Dropped errors: added a comment instead of logging. The only way the write (or the gzip Close) can fail is the connection failing. By then the status and part of the body have been sent, so there is nothing left to report the error with, and the scraper sees a response that ends early and records a failed scrape. The text path already behaves this way, and Handler has no logger. Adding one would be new public API for little gain.
• Vary: the handler now sends Vary: Accept, Accept-Encoding. The gzip choice depends on a request header too and never had one. scrapeProtobuf asserts it.
• Memory: families() now returns the sorted families with their copied series, and writeProtobuf builds each family's messages just before writing it, so only one family is held at a time. I did not reuse messages between families, so the allocation count stays at about 2.59M. New BenchmarkWriteProtobuf (build and marshal, 100k series, 10 runs, benchstat):

	before	after
peak heap, 2 families	111.7 MiB	88.3 MiB (−21%)
peak heap, 100 families	111.7 MiB	98.6 MiB (−12%)
time	287 / 210 ms	284 / 205 ms
allocs	2.59M	2.59M

Peak heap is HeapAlloc sampled at each write, above the level before the call. It includes garbage the GC has not collected yet, so read it as relative, not absolute.

[tobias-eyl-twilio]

Wording suggestions:

• Alloy's start_timestamp_zero_ingestion does not put protobuf first; scrape_protocols must be set explicitly. Readers may assume the option alone is enough.
• In Prometheus 3.x the native-histograms feature flag is a no-op; the trigger is the scrape_native_histograms config option.
• Prometheus is renaming created timestamps to start timestamps (wire field numbers unchanged); mentioning both names would help searchability.

[sathvik09]

Thanks, all three applied in 0f58552:

• Alloy: the entry now says Alloy puts protobuf first by default only with scrape_native_histograms, and that start_timestamp_zero_ingestion alone does not change the order, so scrape_protocols has to list PrometheusProto first. This matches the prometheus.scrape docs.
• Native histograms: it names the scrape_native_histograms option instead of the feature flag.
• Naming: it says Prometheus is renaming created timestamps to start timestamps on the same wire field, and uses "created (start) timestamp" for the zero-ingestion option.

The entry also now covers the text le="+Inf" change and the Vary header from this round.

[tobias-eyl-twilio]

Test gaps:

• Comparing via maps can't catch duplicates or ordering (see the collision comment in protobuf.go).
• Nothing covers NaN observations, a bucket rebuild, a zero-time first update, colliding names, or a concurrent scrape under -race.
• The Accept header used here isn't the real Prometheus 3.x one (its protobuf entry starts at q=0.7 by default); adding the real headers as cases would harden acceptsProtobuf.
• Nit: client_model v0.6.3 is out.

[sathvik09]

Addressed. Each gap and its test:

• Duplicates and ordering: TestProtobufCollisionOrder (11e0021) scrapes the collision case 100 times, asserts identical bytes, and checks the order directly rather than through a map.
• NaN and bucket rebuild: TestHistogramInfBucket (e1b2131).
• Zero-time first update: TestCreatedTimestampZeroTime (8d11553).
• Colliding names: TestProtobufCollisionOrder (11e0021).
• Concurrent scrape under -race: TestProtobufConcurrentScrape (1ca7a60) scrapes while measures arrive and series expire.
• Real Accept headers: added Prometheus' acceptHeader output as cases (1ca7a60): the defaults, protobuf first at q=0.8 (main, 7 protocols), and q=0.7 (3.0). The weights start at one above the number of protocols Prometheus knows, so they shift between releases.
• client_model: bumped to v0.6.3 (1ca7a60).

[tobias-eyl-twilio]

Re-review at 164519f. Every earlier finding is fixed, and I verified each fix by reverting it locally: the new tests (TestProtobufCollisionOrder, TestCreatedTimestampZeroTime, TestHistogramInfBucket, the concurrent scrape) each fail without it.

Verified:

• go build, go vet, go test -race ./prometheus/... (-count=10 on the new tests), go mod verify, go mod tidy -diff and gofmt are clean.
• 500 protobuf scrapes gave byte-identical output.
• +Inf from _count can never fall below a finite bucket, and NaN handling matches client_golang.
• Lock order is unchanged (store → entry → state), and no lock is held while writing.
• Vary is set on both formats.
• HISTORY matches the Prometheus 3.15.1 and Alloy 1.21.3 sources.

Memory at 100k series: peak heap dropped about 20%, to ~94 MB (text is ~67 MB). Allocations are still 2.59M per scrape.

The remaining comments are minor or nits and don't block merging. Leaving write errors unlogged, with the comment explaining why, is fine.

[tobias-eyl-twilio]

Minor: one collision case still keeps a different series in protobuf than in text. Histogram series sort under the histogram's root name, but text sorts _count/_sum/_bucket under their full names.

• A gauge x.count next to histogram x: text keeps the gauge's x_count (70), and protobuf keeps the histogram's x_count (1).
• A gauge y_count next to histogram y: the text output itself changed in 132 of 500 scrapes in a probe. sort.Sort(byNameAndLabels) (handler.go:216, metric.go:552) is unstable and ties on scope/name/labels after map iteration. That part predates this PR.

Suggested fix: add a type-then-raw-name tie-break to byNameAndLabels.Less, and either order protobuf histograms by their sub-series names or document this edge case.

[sathvik09]

Fixed in 5fcbb1a. byNameAndLabels.Less now breaks ties on the root name and then the type, so the text format always lists the histogram's y_count first, as protobuf does. TestHistogramCollisionOrder covers it and failed on the first run without the change. For x.count next to histogram x I documented the difference at the protobuf sort instead: matching it would mean reordering the text format for every metric, and each format already keeps the same series on every scrape.

[tobias-eyl-twilio]

Nit: the fallback overwrites t, so a zero-time first update also sets state.time to now, while later zero-time updates set it back to zero. Using a separate local for the created time would keep state.time consistent:

if state.created.IsZero() {
	created := t
	if created.IsZero() {
		created = time.Now()
	}
	state.created = created
}
state.time = t

[sathvik09]

Fixed in 516b501 with a separate created local, so state.time stays zero and expiry treats the first zero-time update like the rest. TestZeroTimeUpdateKeepsZeroTime covers it and fails without the change.

[tobias-eyl-twilio]

Nit: the version labels look swapped. Prometheus v3.15.1's ScrapeProtocolsHeaders has 6 entries (it added OpenMetrics 2.0.0), so its weights start at q=0.7. The q=0.7 string is therefore current main/3.15, not 3.0, and 3.0 would start lower (I couldn't confirm 3.0's exact header). The behaviour under test is fine; only the comments and case names need fixing.

[sathvik09]

Right, thanks. acceptHeader starts at one above the number of known protocols: 5 on 3.0–3.14 (q=0.6), 6 on 3.15 (q=0.7), and 7 on main (q=0.8). Fixed in 4b983a7: the cases are now named main and 3.15, and I added 3.0's exact header from the v3.0.0 source, which starts at q=0.6 and appends escaping=allow-utf-8 to every protocol, protobuf included. It still picks protobuf.

[tobias-eyl-twilio]

Nit: the bump from v0.6.2 to v0.6.3 came in with test commit 1ca7a60 and no stated reason. The only change in v0.6.3 is a newer go/protobuf requirement, and nothing here uses it. Either drop the bump or say why in the commit.

[sathvik09]

Dropped in f522143. There was no reason for it: v0.6.3's generated Go code is the same as v0.6.2's, and nothing here needs it.