Skip to content

[chore] prom translation rw2 add support for histogram #40494

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 1 commit into
base: main
Choose a base branch
from

Conversation

jmichalek132
Copy link
Contributor

@jmichalek132 jmichalek132 commented Jun 4, 2025

Description

Adds support for adding histograms in the RW2 translation path.

Link to tracking issue

Fixes

Partially implements #33661 (when merging PR please don't close the tracing issue)

Testing

  • Added unit test
  • e2e ran with prometheus

Histogram metrics in prometheus ui
image

@jmichalek132
Copy link
Contributor Author

Review please @dashpole @ArthurSens @ywwg @krajorama

// omitted
if pt.HasSum() {
// treat sum as a sample in an individual TimeSeries
sum := &writev2.Sample{
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought histograms were sent as writev2.Histogram so that they are atomic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah forgot about that let me fix it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to re-read RW2 spec...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So looking at https://github.com/prometheus/prometheus/blob/e7e3ab282431de827ee7d03d27d056551a134e6a/storage/remote/queue_manager.go#L1914 and doing some testing locally looks like, normal histograms with buckets are still treated as separate series in remote write, and I guess only the new native histograms are send as writev2.Histogram, but I think I remember there being some discussion about translating normal histograms into native histograms let me try and find it.

@ywwg
Copy link

ywwg commented Jun 6, 2025

My understanding of conventional commit style is that "chore" is for routine maintenance that does not affect production: "tool changes, configuration changes, and changes to things that do not actually go into production at all.". So this I would actually call "feat" because it is new functionality.

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

Successfully merging this pull request may close these issues.

4 participants