Skip to content

feat(om2): add native histograms to OpenMetrics2.0 #2634

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

krajorama
Copy link
Member

@krajorama krajorama commented Apr 24, 2025

Background

Based on https://github.com/prometheus/proposals/blob/main/proposals/2024-01-29_native_histograms_text_format.md
And OpenMetrics 2.0 WG discussions.

Changes

  • Allow structured complex types marked by "{" and "}" in the specification.
  • Allow multiple exemplars per complex type value.
  • Require that exemplars for complex type values have the timestamp.
  • Be permissive about observing NaN , +Inf, -Inf. Discourage observing NaN.
  • Split histogram into ones with classic and native buckets.
  • For classic buckets, define behavior when observing NaN.
  • Define the native buckets and also how NaN is handled.
  • Define the text format of native histograms and also their exemplars.

Open questions / decisions

See OpenMetrics2.0 WG meeting notes tab

Signed-off-by: György Krajcsovits <[email protected]>
Signed-off-by: György Krajcsovits <[email protected]>
Signed-off-by: György Krajcsovits <[email protected]>

Numbers MUST be either floating points or integers. Note that ingestors of the format MAY only support float64. The non-real values NaN, +Inf and -Inf MUST be supported. NaN MUST NOT be considered a missing value, but it MAY be used to signal a division by zero.

Complex data types MUST contain all information necessary to recreate a Metric Type, with the exception of Created time and Exemplars.
Copy link
Member Author

Choose a reason for hiding this comment

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

This assume we'll have the created timestamp separate from the JSON like data. prometheus/OpenMetrics#285

Signed-off-by: György Krajcsovits <[email protected]>
@krajorama
Copy link
Member Author

Note self: add details how summaries and classic histograms one liners (so not NHCB spans/deltas) fit into it.

krajorama added 4 commits May 22, 2025 10:05
Signed-off-by: György Krajcsovits <[email protected]>
Signed-off-by: György Krajcsovits <[email protected]>
Add semantic conventions about where complex types may occure.
Allow empty spans and deltas.
Be more precise.

Signed-off-by: György Krajcsovits <[email protected]>
Signed-off-by: György Krajcsovits <[email protected]>
Copy link
Member

@ArthurSens ArthurSens left a comment

Choose a reason for hiding this comment

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

Had some minutes to read the PR. Couldn't read everything though, I'll do a more complete review another day!

Reviewed from mobile


##### Exponential buckets

Histogram MetricPoints with exponential buckets MUST have a Schema value. The Schema is an 8 bit signed integer between -4 and 127. Schema values between -4 and 127 are also called Standard Schemas.
Copy link
Member

Choose a reason for hiding this comment

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

Carrie's document mentions that schema goes from -4 up to 8. I'm a bit confused here, is it 8 or 127?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to keep this open ended, 8 is the current implementation maximum. This way we reserve higher resolutions.

@beorn7 beorn7 self-requested a review May 27, 2025 11:26
krajorama added 2 commits May 27, 2025 15:53
Signed-off-by: György Krajcsovits <[email protected]>
Signed-off-by: György Krajcsovits <[email protected]>
@krajorama krajorama marked this pull request as ready for review May 27, 2025 14:01
@krajorama

This comment was marked as outdated.

I did not want to clutter the examples with adding "# EOF" to all,
so I made a new marker to explicitly add it on request.

There was only one faulty example where we had an extra space, see
the Exemplars section.

Signed-off-by: György Krajcsovits <[email protected]>
Allow multiple exemplars for complex types, i.e. native histograms.
But require that the timestamp is present.

Signed-off-by: György Krajcsovits <[email protected]>
@krajorama krajorama force-pushed the krajo/om2.0-native-histograms branch from c16426b to 7965787 Compare June 11, 2025 07:27
Copy link
Member

@beorn7 beorn7 left a comment

Choose a reason for hiding this comment

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

Thanks for doing all of this.

Mostly commented about the histogram aspect (but I couldn't resist and referred to OM specific things now and then).

Comment on lines +72 to +73
- Integer counter native histograms for the Metric Type Histogram.
- Integer gauge native histograms for the Metric Type GaugeHistogram.
Copy link
Member

Choose a reason for hiding this comment

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

Are float histograms planned for later? Or are they excluded for good? (Note that we need float histograms if we want to support federation via OM text format.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Haven't thought about it. There's two options: allow floats everywhere and use absolute numbers for buckets similar to how we do regular float counters. Which means we'd have a single exposition format for both.

Or introduce a new kind of exposition to be detected from the fields that occur (deltas vs buckets).

Copy link
Member Author

Choose a reason for hiding this comment

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

In general this is a somewhat separate question as OpenMetric1.0 didn't allow floats. But we need to prepare for sure.

Copy link
Member Author

@krajorama krajorama Jun 13, 2025

Choose a reason for hiding this comment

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

update: moved discussion to the PR doc


If the NaN value is not allowed, then the Count value MUST be equal to the value of the +Inf bucket.

If the NaN value is allowed, it SHOULD be counted in the +Inf bucket, and MUST not be counted in any other bucket. In case the NaN is counted in the +Inf bucket, then the Count MUST be equal to the value of the +Inf bucket, otherwise the Count MUST be greater. The ratonale is that NaN does not belong to any bucket mathematically, however instrumentation libraries traditionally put it into the +Inf bucket, which is why the wording "SHOULD" is used.
Copy link
Member

Choose a reason for hiding this comment

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

🤔 Since there are several code locations with the baked-in assumption that the +Inf bucket is identical to the count (e.g. only storing one value, feeding into both of these), I don't think we can relax the requirement here without causing trouble.

Since it doesn't really matter in practice how we treat NaNs (they will only happen because of bugs), I would propose to keep the current way, even if it is mathematically somewhat inconsistent.

Another point for that is that we don't get rid of the old way by allowing the new way. We now have to deal with two different ways.

Copy link
Member Author

Choose a reason for hiding this comment

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

Version 1.0 does not say anything, but I'll change SHOULD to MUST and rework. https://prometheus.io/docs/specs/om/open_metrics_spec/#histogram


The bucket and Gsum of a GaugeHistogram are conceptually gauges, however bucket values MUST NOT be negative or NaN. If negative threshold buckets are present, then sum MAY be negative. Gsum MUST NOT be NaN. Bucket values MUST be integers.
The bucket and Gsum of a GaugeHistogram are conceptually gauges, however bucket values MUST NOT be negative or NaN. If negative threshold buckets are present, then Gsum MAY be negative. Gsum MUST NOT be NaN. Bucket values MUST be integers.

A GaugeHistogram's Metric's LabelSet MUST NOT have a "le" label name.
Copy link
Member

Choose a reason for hiding this comment

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

Unless it's only native buckets.


A GaugeHistogram's Metric's LabelSet MUST NOT have a "le" label name.

Bucket values can have exemplars.
The buckets for a GaugeHistogram follow all the same rules as for a Histogram, with Gcount playing the same role as Count.
Copy link
Member

Choose a reason for hiding this comment

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

And Gsum as Sum?


Histograms with exponential buckets use the integer native histogram data type.

The integer native histogram data type is a JSON like structure with fields. There MUST NOT be any whitespace around fields.
Copy link
Member

Choose a reason for hiding this comment

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

It's not really JSON. Many language use vaguely this kind of structure. I wouldn't call out JSON because it creates the expectation that other JSON syntax rules also apply.

Copy link
Member

Choose a reason for hiding this comment

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

There MUST NOT be any whitespace around fields.

Has it already been decided that OMv2 will stay whitespace intolerant?

It's one of my big big red flags with OMv1 that it doesn't tolerate whitespace like the classic Prometheus text format. If OMv2 will see the light and allow whitespace again, then of course we should allow whitespace here.

Copy link
Member Author

Choose a reason for hiding this comment

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

We have not discussed this in OM2.0 explicitly. Comes from https://github.com/prometheus/proposals/blob/main/proposals/2024-01-29_native_histograms_text_format.md.

However we did say that since native histograms use a structure that's essentially internal, we kind of already given up on being very human friendly. I still keep the count and sum fields up front because that's easy to understand and relates to PromQL.

A consequence is that I think we'll have to give some tooling (like promtool) for people to pretty print the output and also instrumentations may allow pretty printing themselves.


Exponential bucket values MUST be ordered by their index, and their values MUST be placed in the `negative_deltas` (and/or `positive_deltas`) field using delta encoding, that is the first bucket value is written as is and the following values only as a delta relative to the previous value. For example bucket values 1, 5, 4, 4 will become 1, 4, -1, 0.

To map the `negative_deltas` (and/or `positive_deltas`) back to their indices, the `negative_spans` (and/or `positive_spans`) field MUST be constructed in the following way: each span consists of a pair of numbers, an integer called offset and an non-negative integer called length. Only the first span in each list can have a negative offset. It defines the index of the first bucket in its corresponding `negative_deltas` (and/or `positive_deltas`). The length defines the number of consecutive buckets the bucket list starts with. The offsets of the following spans define the number of excluded (and thus unpopulated buckets). The lengths define the number of consecutive buckets in the list following the excluded buckets.
Copy link
Member

Choose a reason for hiding this comment

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

Capitalize after :: each → Each

###### Exemplars

Exemplars without Labels MUST represent an empty LabelSet as {}.

An example of Exemplars showcasing several valid cases:
The native histogram version of the histogram has multiple Exemplars.
Copy link
Member

Choose a reason for hiding this comment

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

But I see only one?

Copy link
Member Author

Choose a reason for hiding this comment

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

There's two

foo {count:10,sum:1.0,schema:0,zero_threshold:1e-4,zero_count:0,positive_spans:[0:2],positive_deltas:[5,0]} # {trace_id="shaZ8oxi"} 0.67 1520879607.789 # {trace_id="ookahn0M"} 1.2 1520879608.589

Signed-off-by: György Krajcsovits <[email protected]>
even unpopulated ones.

Signed-off-by: György Krajcsovits <[email protected]>
Empty may be exposed for optimizing spans.

Signed-off-by: György Krajcsovits <[email protected]>
Comment on lines +309 to +310
The values of exemplars in a Histogram MetricPoint with native buckets MUST fall into one of the native buckets.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I'll remove this, since:

  1. if the exemplar's value is not NaN then definition some native bucket will cover it
  2. if the exemplar's value is NaN this is not true
  3. after reset the actual stored bucket may be missing, but the exemplar can be there.
Suggested change
The values of exemplars in a Histogram MetricPoint with native buckets MUST fall into one of the native buckets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants