Skip to content

refact(om2.0): reorder ABFN and enhance comments#2898

Merged
krajorama merged 2 commits intomainfrom
krajo/om2-reorder-abnf
Mar 20, 2026
Merged

refact(om2.0): reorder ABFN and enhance comments#2898
krajorama merged 2 commits intomainfrom
krajo/om2-reorder-abnf

Conversation

@krajorama
Copy link
Member

Improve ABNF readability.

Move timestamp and start-timestamp next to sample. Move numbers to one group (integers moved up) and improve comments. Move classic histograms before native histograms. Improve comments on histograms in general. Fix doble negative_spans error.

Follow up to #2894

Let the metric be name and labels AND sample.

Also fix a typo picked up by Claude.

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Improve ABNF readability.

Move timestamp and start-timestamp next to sample.
Move numbers to one group (integers moved up) and improve comments.
Move classic histograms before native histograms. Improve comments on
histograms in general. Fix doble negative_spans error.

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
@krajorama krajorama requested review from bwplotka, dashpole and ywwg March 18, 2026 09:29
@krajorama krajorama added the priority/p3 Where Doc WG adds issues for consideration as a priority. label Mar 18, 2026
dashpole
dashpole previously approved these changes Mar 18, 2026
Base automatically changed from krajo/om2-walkthrough-text to main March 18, 2026 15:13
@krajorama krajorama dismissed dashpole’s stale review March 18, 2026 15:13

The base branch was changed.

Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

LGTM, I have unrelated comments only, so let's merge!


A GaugeHistogram Sample MUST contain Gcount, Gsum values.

The GCount value MUST be equal to the number of measurements currently in the GaugeHistogram. The GCount is a gauge semantically. The GCount SHOULD be an integer. The GCount SHOULD NOT be -Inf, +Inf, NAN, or negative.
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps "Gcount" but it's unrelated nit for different PR

Copy link
Member Author

Choose a reason for hiding this comment

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

will fix


metric = metricname-and-labels SP sample

sample = value [SP timestamp] [SP start-timestamp] *exemplar LF
Copy link
Member

Choose a reason for hiding this comment

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

not multiple exemplars (unrelated question to this PR I guess)?

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 agreed to support multiple exemplars for all, if that's what you're asking? Let's quickly discuss at the meeting.

Copy link
Member

Choose a reason for hiding this comment

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

I meant this ABNF looks like only one exemplar is allowed....?

Copy link
Member Author

Choose a reason for hiding this comment

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

The star means zero or any number.

@krajorama krajorama merged commit 32aba1e into main Mar 20, 2026
7 checks passed
@krajorama krajorama deleted the krajo/om2-reorder-abnf branch March 20, 2026 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority/p3 Where Doc WG adds issues for consideration as a priority.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants