Skip to content

Drop negative-bucket histogram _sum and _count only in OpenMetrics 1.0 output - #1254

Merged
csmarchbanks merged 3 commits into
prometheus:masterfrom
haeganm:negative-bucket-histogram-count
Nov 13, 2026
Merged

Drop negative-bucket histogram _sum and _count only in OpenMetrics 1.0 output#1254
csmarchbanks merged 3 commits into
prometheus:masterfrom
haeganm:negative-bucket-histogram-count

Conversation

@haeganm

@haeganm haeganm commented Nov 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Histogram leaves out _sum when its lowest bucket is negative, since OpenMetrics 1.0 says a histogram with negative buckets MUST NOT contain a sum. MultiProcessCollector never did that check, so in multiprocess mode the same histogram still had a _sum, OM 1.0 output included:

# Histogram('temp_delta', 'help', buckets=[-1, 0, 1]).observe(-0.5), multiprocess
temp_delta_sum -0.5
temp_delta_bucket{le="-1.0"} 0.0
temp_delta_bucket{le="0.0"} 1.0
temp_delta_bucket{le="1.0"} 1.0
temp_delta_bucket{le="+Inf"} 1.0
temp_delta_count 1.0

OpenMetrics 2.0 goes the other way (every histogram has count and sum), so per review this keeps _sum in Histogram's samples (single and multiprocess) and filters it at exposition instead:

  • Histogram always emits _sum
  • openmetrics.exposition.generate_latest drops _sum and _count for histograms with a negative bucket when version is below 2.0.0, since 1.0 only allows _count alongside a sum (same as HistogramMetricFamily)
  • the histogram docs say which output leaves them out

behavior changes: OM 1.0 scrapes no longer have _count for these, and multiprocess ones lose _sum too (the +Inf bucket still has the count). the classic text format now shows _sum for these histograms in single process too.

test_histogram_negative_buckets in tests/test_multiprocess.py fails on master (b'h_sum' unexpectedly found in the OM 1.0 output) and passes here, and the OM exposition test now checks that 1.0 leaves out _sum and _count (and parses with the library's OM parser) while 2.0 keeps both. full suite passes on 3.12 (Windows), and flake8, isort and mypy are clean.

@csmarchbanks

@haeganm
Omit _sum for negative-bucket histograms in multiprocess mode
8426f96 
Histogram leaves out _sum when its lowest bucket is negative, since
OpenMetrics 1.0 doesn't allow a sum there. MultiProcessCollector builds
its own histogram samples and kept the _sum from the mmap files, so the
same histogram exposed an extra series in multiprocess mode.

Drop _sum on the accumulate path when the lowest bucket bound is
negative. merge(accumulate=False) still returns the raw value.

Signed-off-by: Haegan McGarry <[email protected]>
csmarchbanks
csmarchbanks reviewed Nov 12, 2026
View reviewed changes

@csmarchbanks csmarchbanks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since OM 2.0 is adding sum back in everywhere I would learn towards at lest keeping it in the samples. That said, we should probably filter it out in exposition for at least OM 1.0 users.

@haeganm
Keep histogram _sum in samples, drop it only in OpenMetrics 1.0 output
ca715c8 
OpenMetrics 2.0 wants a sum on every histogram, so Histogram now keeps
_sum even when the lowest bucket is negative, and multiprocess goes back
to keeping it too. OpenMetrics 1.0 doesn't allow a sum with negative
buckets, so generate_latest drops _sum for those histograms when the
version is below 2.0.0.

Signed-off-by: Haegan McGarry <[email protected]>
@haeganm haeganm changed the title Omit _sum for negative-bucket histograms in multiprocess mode Drop negative-bucket histogram _sum only in OpenMetrics 1.0 output Nov 13, 2026
@haeganm

haeganm commented Nov 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

makes sense, flipped it in ca715c8. Histogram now always keeps _sum (multiprocess already did, so that part of the diff is gone), and openmetrics.exposition.generate_latest drops _sum for histograms with a negative bucket when version is below 2.0.0. so 1.0 output has no sum for these and 2.0 gets it.

one side effect, the classic text format now shows _sum for these in single process too, since that format never had the rule. happy to filter it there as well if you'd rather.

one more thing on 1.0: its text format says _count is there if and only if there's a sum, so a fully valid 1.0 output would drop _count for these too. that's what the OM parser trips on (_sum/_gsum must be present if _count is present), and single process already fails that way on master. HistogramMetricFamily already drops both for negative buckets (c158b76). want me to do the same here? single process 1.0 scrapes would lose _count, but the +Inf bucket has the same value. the parser can't read 2.0 output for these either, it rejects any _sum on a histogram with negative buckets. left that one out of this PR.

@csmarchbanks

csmarchbanks commented Nov 13, 2026

Copy link
Copy Markdown
Member

I'm good with keeping _sum in classic for single process. It is useful IMO.

I would say let's drop _count as well for OM 1.0. This library is supposed to be a reference implementation of OM 1.0 so we should stay as aligned as possible to the spec. Thank you for all your work!

We need to make the parser 2.0 compliant still so no worries about that not working yet.

@haeganm
Drop _count too for negative-bucket histograms in OpenMetrics 1.0
a46b0aa 
OpenMetrics 1.0 only allows _count on a histogram when it has a _sum,
so dropping just _sum left output that this library's own OpenMetrics
parser rejects ("_sum/_gsum must be present if _count is present").
Drop both, like HistogramMetricFamily already does. The +Inf bucket
still has the count.

Also write a custom bucket with a malformed le out as is, instead of
failing the whole OpenMetrics 1.0 output.

Signed-off-by: Haegan McGarry <[email protected]>
@haeganm haeganm changed the title Drop negative-bucket histogram _sum only in OpenMetrics 1.0 output Drop negative-bucket histogram _sum and _count only in OpenMetrics 1.0 output Nov 13, 2026
@haeganm

haeganm commented Nov 13, 2026

Copy link
Copy Markdown
Contributor Author

sounds good! dropped _count too in a46b0aa, and the test now checks the 1.0 output against the library's own OM parser. kept _sum in classic and added a test for that.

also, since the 1.0 path parses le now, a custom bucket with a bad le just gets written out as is instead of breaking the whole 1.0 output.

updated the title and description to match. thanks for the quick reviews!

csmarchbanks
csmarchbanks approved these changes Nov 13, 2026
View reviewed changes

@csmarchbanks csmarchbanks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@csmarchbanks
csmarchbanks merged commit c0f1bdf into prometheus:master Nov 13, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@csmarchbanks csmarchbanks csmarchbanks approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@haeganm @csmarchbanks