prometheus
Add openmetrics format parser
Ref: prometheus/issue/8932 Add openmetrics protocol decoder, for promtool to support check openmetric protocol
This in my review queue (but the queue is long, so if somebody beats me to it, even better).
I have reached the beginning of my (unplugged) vacations and didn't get to this. I'm very sorry for that. I'll try to find another reviewer (or will come back to this in about two weeks time).
I have updated the code according to the code review comments. PTAL If you have time @ArthurSens
@ArthurSens I assume you'll take care of the further review. Let me know if you need any help.
Yes, I plan to continue the review! But if you find time to also give a review, please do :)
Onboarding at the new job has been very time-consuming. Too many videos to watch, and too many trainings to attend 😅
We are clearly both lacking time. Which in turn means we should try even harder to avoid redundant review work. Instead of a parallel review, one of us should do the main review, and only loop in the other where needed (open questions of any kind, tough calls to make, …).
@ArthurSens is this still on your queue?
Yes, it is! To be completely honest with you, @jyz0309, I've opened this PR to review quite a few times in the past weeks, but I've been feeling a bit overwhelmed with its size, so I ended up procrastinating the review.
I wonder if we could split this PR into smaller ones, to make the review process smoother and we're able to advance here. What do you think of the idea? Would that be ok?
If we split, we can start with a super simple parser implementing each OpenMetrics feature in parts. For example:
- First PR: a parser that can parse HELP, TYPE, and UNIT, plus tests that assert this works.
- Second PR: extend the parser to also parse counters and gauges, not including fancy features like timestamps, exemplars, etc. Tests are also extended and previous tests need to keep passing.
- Third PR: Let's parse summaries/histograms
etc etc, until we cover all OM features.
What do you think? 🙂
@ArthurSens is this still on your queue?
Yes, it is! To be completely honest with you, @jyz0309, I've opened this PR to review quite a few times in the past weeks, but I've been feeling a bit overwhelmed with its size, so I ended up procrastinating the review.
I wonder if we could split this PR into smaller ones, to make the review process smoother and we're able to advance here. What do you think of the idea? Would that be ok?
If we split, we can start with a super simple parser implementing each OpenMetrics feature in parts. For example:
- First PR: a parser that can parse HELP, TYPE, and UNIT, plus tests that assert this works.
- Second PR: extend the parser to also parse counters and gauges, not including fancy features like timestamps, exemplars, etc. Tests are also extended and previous tests need to keep passing.
- Third PR: Let's parse summaries/histograms
etc etc, until we cover all OM features.
What do you think? 🙂
Okay, I will finish this work next week(maybe by next Friday)
