Skip to content
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

Add support for native histograms #515

Closed
beorn7 opened this issue Nov 29, 2022 · 6 comments · Fixed by #596
Closed

Add support for native histograms #515

beorn7 opened this issue Nov 29, 2022 · 6 comments · Fixed by #596
Assignees

Comments

@beorn7
Copy link
Member

beorn7 commented Nov 29, 2022

It's still an experimental feature, but it should be relatively easy to add support to the PGW.

@beorn7 beorn7 self-assigned this Nov 29, 2022
@beorn7
Copy link
Member Author

beorn7 commented Dec 6, 2022

For protobuf, that is.

To support pushing in the text format, first the text format has to support native histograms. Which will happen in OpenMetrics, not in the old text format, if at all. So then this depends on #400 .

@beorn7
Copy link
Member Author

beorn7 commented Nov 9, 2023

@jan--f is working on this.

@jan--f
Copy link
Contributor

jan--f commented Nov 15, 2023

So far I think this will just work as is. So things to be done are:

  • add storage test for histograms
  • add api test for histograms
  • confirm that scraping in text format returns at least _sum and _count
  • improve UI to display histograms

The last item might benefit from aligning with improvements made in Prometheus itself?

@beorn7
Copy link
Member Author

beorn7 commented Nov 15, 2023

The Prometheus UI issue is prometheus/prometheus#11269 . Currently, the UI just displays the Histogram.String() output. PGW can do the same for now. Note that the PGW UI uses its own bespoke query API to render the UI (GET http://pushgateway.example.org:9091/api/v1/metrics etc., see README.md). For now, it's probably fine to simply replace the value string, which is currently always a float in string form, with the string representation of the Histogram. For fancy graphical rendering, we needed to return the histogram in some structured form.

@jan--f
Copy link
Contributor

jan--f commented Nov 22, 2023

Displaying native histograms via Histogram.String() is in #596.

Otherwise pushgateway just works with native histograms.

Push via protobuf works as expected

2023-11-22-150837_891x784

Scraping via protobuf works as expected

2023-11-22-150924_877x446

Scraping with text format, give us an old-style histogram with a single bucket and a _count and _sum metric.

2023-11-22-151429_852x485

@beorn7
Copy link
Member Author

beorn7 commented Nov 23, 2023

Thank you very much for all the work so far.

The representation in the PGW UI is the "machine level" one, not the human-readable one that you can see in the Prometheus UI (also see my comment on #596).

Another thought that crossed my mind: We do have an API that returns all the pushed metrics in JSON, see https://github.com/prometheus/pushgateway#url-2 . I wrongly assumed that the UI is using this API call, but it isn't (yet), it just uses Go HTML templating directly. However, that API endpoint also needs to support native histograms, because some users might use it for their own tooling. (And of course, a future UI might actually use the API endpoint as originally planned.) I assume some code is needed to create a makeshift JSON format.

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

Successfully merging a pull request may close this issue.

3 participants