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

Promtool should support openmetrics #8932

Open
roidelapluie opened this issue Jun 13, 2021 · 14 comments
Open

Promtool should support openmetrics #8932

roidelapluie opened this issue Jun 13, 2021 · 14 comments

Comments

@roidelapluie
Copy link
Member

Promtool should support openmetrics, next to the text format.

Maybe via --format=openmetrics

@ide-rea
Copy link
Contributor

ide-rea commented Jun 14, 2021

Hi, does this mean check openmetrics format like check prom text format, could you tell me?

@roidelapluie
Copy link
Member Author

This issue is to add support for promtool check metrics --format=openmetrics

@ide-rea
Copy link
Contributor

ide-rea commented Jun 14, 2021

Thanks, i would like to try if not anybody is working on this.

@ide-rea
Copy link
Contributor

ide-rea commented Jun 15, 2021

Hi, i'm a little confused about the check rule. Do we parse the text wire openmetrics format into prometheus MetricFamily, and then encoded the metrics into bytes stream, at last use promlint package to do the check? need your help, thanks :) @roidelapluie

@roidelapluie
Copy link
Member Author

This issue would require a openmetrics text parser in go, which does not exist at the moment. We only have a textformat parser in go.

@roidelapluie
Copy link
Member Author

Here is the textparser in go: https://github.com/prometheus/common/blob/main/expfmt/text_parse.go
We should have one openmetrics parser in go for this issue. It should be developed next to the text parser, in the prometheus/common repository.

@ide-rea
Copy link
Contributor

ide-rea commented Jun 15, 2021

Thanks, i'm understanding the issue better!

@jyz0309
Copy link
Contributor

jyz0309 commented Sep 6, 2023

Is this Issue still valid and can I try it? :)

@machine424
Copy link
Collaborator

machine424 commented Sep 14, 2023

I think so, especially that prometheus supports back filling from openmetrics #8084

@beorn7
Copy link
Member

beorn7 commented Jun 25, 2024

Hello from the bug scrub!

@jyz0309 are you still willing to work on this? We'll happily assign this issue to you (or to others that want to help with it).

Note that it requires an implementation of an OpenMetrics parsers in https://github.com/prometheus/common/tree/main/expfmt , so it's a bit of work and maybe not as easy as it looks.

@jyz0309
Copy link
Contributor

jyz0309 commented Jun 26, 2024

@beorn7 Yes, I am willing to try this issue, I think I need to implement an OpenMetrics parser and merge in expfmt firstly then use it to support openmetrics in promtool.

@beorn7 beorn7 assigned beorn7 and jyz0309 and unassigned beorn7 Jun 26, 2024
@jyz0309
Copy link
Contributor

jyz0309 commented Jul 9, 2024

@beorn7 Hello! I want to ask sth about the parser in expfmt, I am implementing the OpenMetrics parser in expfmt and I found that in client_model still have not support info, stateset types, should I add these to protobuf file first or just unsupported these two type firstly (I found that the encoder also unsupported these type yet)?

@beorn7
Copy link
Member

beorn7 commented Jul 9, 2024

Yeah, good question.

So the source of dissonance here is that the encoders and parsers in https://github.com/prometheus/common/tree/main/expfmt are all based on https://github.com/prometheus/client_model , which in turn represents the data model behind the classic Prometheus text format. (OpenMetrics in fact has its own protobuf spec here.)

As the encoder is also just encoding from the classic Prometheus protobuf, let's do what you said above, i.e. implement a parser that supports everything we can easily put into the classic Prometheus protobuf. We can later decide if we want to convert info and stateset into other metrics (which is essentially what Prometheus itself is doing right now upon ingestion) or add support for it into the classic protobuf or port the whole shebang to the OpenMetrics protobuf (which isn't that different after all).

@jyz0309
Copy link
Contributor

jyz0309 commented Jul 22, 2024

prometheus/common#669
@beorn7 I have implement OpenMetrics parser in expfmt in this PR, PTAL, thanks :)

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

No branches or pull requests

6 participants