- Notifications
You must be signed in to change notification settings - Fork 1.2k
Optionally print OM created lines #1408
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
Conversation
c26d312
to b7d00b2
Compare e930252
to 240345e
Compare There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! One minor nit, otherwise well done!
33ed3e6
to e9c6f77
Compare There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Epic, proposed some improvements, thanks!
e9c6f77
to 81909c5
Compare We need prometheus/common v0.49.0 for this feature, but this version of prometheus/common requires go 1.21. Therefore, the PR is blocked until we drop support for go 1.20 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
81909c5
to 65df2d7
Compare After quite some work to unblock this, this is finally ready :) Do you want to review again or should I just merge? |
Now that prometheus/prometheus#14356 was merged, should we revive this? :) |
eb0f4ef
to b51b530
Compare Since we have approvals from the past, I'll take the liberty to merge this PR next week :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's timebox this, but wanted to try last more time to consider an improvement to the config structure, otherwise LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we add summaries and counters in the example too?
This PR adds an extra option to print _created lines when OpenMetrics is the chosen exposition format. It's implemented following the conditional option added in prometheus/common#504.