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

Support Prometheus native histograms #16333

Open
runiq opened this issue Feb 7, 2023 · 6 comments
Open

Support Prometheus native histograms #16333

runiq opened this issue Feb 7, 2023 · 6 comments
Labels
sink: prometheus_exporter Anything `prometheus_exporter` sink related sink: prometheus_remote_write Anything `prometheus_remote_write` sink related source: prometheus_remote_write Anything `prometheus_remote_write` source related source: prometheus_scrape Anything `prometheus_scrape` source related type: feature A value-adding code addition that introduce new functionality.

Comments

@runiq
Copy link

runiq commented Feb 7, 2023

A note for the community

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Use Cases

Compared to Prometheus' current histogram implementation, native histograms:

  • use a uniform bucket sizing scale
  • remove the requirement to know your bucket sizes beforehand
  • allow performing arithmetic on different bucket sizings (by using the less granular size as the "baseline")
  • save only one time series per histogram (not one per bucket), which can save a lot of space

Attempted Solutions

No response

Proposal

No response

References

https://www.youtube.com/watch?v=AcmABV6NCYk&list=PLoz-W_CUquUmVOjYTqWHX4CJ0jP61vif8&index=2

Version

vector 0.27.0 (x86_64-unknown-linux-gnu 5623d1e 2023-01-18)

@runiq runiq added the type: feature A value-adding code addition that introduce new functionality. label Feb 7, 2023
@jszwedko jszwedko added source: prometheus_scrape Anything `prometheus_scrape` source related source: prometheus_remote_write Anything `prometheus_remote_write` source related sink: prometheus_remote_write Anything `prometheus_remote_write` sink related sink: prometheus_exporter Anything `prometheus_exporter` sink related labels Feb 7, 2023
@jszwedko
Copy link
Member

jszwedko commented Apr 6, 2023

Note that we'll probably want to wait for this feature to stabilize in Prometheus before adding to Vector. It currently appears to be undocumented.

@jacobstr
Copy link

jacobstr commented Apr 23, 2024

To follow up @jszwedko's comment, indeed this un/under-documentation still appears to be the case in April 2024. It looks like the functionality is making it's way into mainline prom in fits and starts but:

Much (all?) of this development appears to mostly center around the golang prometheus community.

@Reimirno
Copy link

Reimirno commented Nov 4, 2024

Follow up: @jszwedko @jacobstr

Native histogram does still seem to be experimental and under-documented as of Nov 2024. There seems to be no ongoing work regarding this issue.

I wonder if makes sense to contribute now, because we do have a use case relying Vector being able to ingest and output Prom native histogram, at least through remote write route. My reasoning:

  • The project milestone still is 73%, but it seems most of the unfinished items (before Prometheus marks this feature stable) are P2/P3. They are mostly related to minor fixes/improvement of promql engine (unrelated to Vector) or NHCB.
  • Importantly it seems that the prometheus native histogram proto/data model is pretty much "stable" and won't have significant change (last change 1.5 years ago).
  • Prometheus native histogram is effectively OTLP exponential histogram which is a stable feature. Their proto are almost 1-1 identical.

While NHCB is a uncertainty, I want to make a case that we can go ahead and implement support for native histogram in Vector already without supporting NHCB. This is effectively the approach taken by most of the instrumentation library that already supports native histogram anyways (prom java sdk, prom golang sdk etc).

I scoped out some changes to be made:

  • Sync up prompb proto with the latest prometheus protos (lib/prometheus-parser/proto/prometheus-types.proto)
  • Figure out how to represent native histogram/exponential histogram in vector core (lib/vector-core/src/event/metric/value.rs) -- maybe MetricValue::Sketch? maybe a brand new type is needed? -- this is where major decision making is needed.
  • Update parser + source to handle native histogram intake (lib/prometheus-parser/src/lib.rs and lib/vector-core/src/event/metric/value.rs) and convert to the agreed-upon representation of native histogram in vector.
  • Update sink to handle this agreed-upon representation of native histogram in vector and be able to covert back to prom native histogram.
  • Bookkeeping: Update scrape to scrape with correct header (so prom native histogram are scraped); update sink to remote write with correct header (src/sinks/prometheus/remote_write/service.rs); feature flagging etc.

Happy to iterate/discuss this further.

@jszwedko
Copy link
Member

Thanks for following up on this @runiq . I agree with your assessment. This feature seems relatively stable, at this point, even if it not well-documented. The risk of it changing drastically seems minimal. I think we'd support a PR implementing this if you want to give it a shot.

If it is possible to map to the the existing sketch or histogram types in Vector without being lossy, that would definitely be preferable. Otherwise we'll need to add an additional type and do the mapping in the sinks where needed. However, if we do need to do that, the fact that OTLP has the same model will mean we'll be able to take advantage of that when we expand Vector to support receiving and sending metrics in OTLP.

@runiq
Copy link
Author

runiq commented Nov 16, 2024

Thanks for following up on this @runiq .

That wasn't me, it was @Reimirno .

I think we'd support a PR implementing this if you want to give it a shot.

I'm afraid I won't be able to implement this, it'd probably be way above my capabilities if I'm being honest with myself.

@Reimirno
Copy link

@jszwedko Yes, that was me following up. Thanks for your reply and thoughts.

I might take a stab at this once I have some bandwidth. I don't think existing histogram/sketches types can be lossless conversion for prometheus native histogram OR OTLP exponential histogram. Most likely we do need an additional type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sink: prometheus_exporter Anything `prometheus_exporter` sink related sink: prometheus_remote_write Anything `prometheus_remote_write` sink related source: prometheus_remote_write Anything `prometheus_remote_write` source related source: prometheus_scrape Anything `prometheus_scrape` source related type: feature A value-adding code addition that introduce new functionality.
Projects
None yet
Development

No branches or pull requests

4 participants