-
Notifications
You must be signed in to change notification settings - Fork 320
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
Write created lines when negotiating OpenMetrics #504
Conversation
ee3b27b
to
8ffc995
Compare
cc @bwplotka @macxamin @TheSpiritXIII |
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.
Nice work! Just need that client_model changed upstream really :)
@TheSpiritXIII @bwplotka I opened this PR before I understood what needs to be done to have protobuf working in client_golang. Looking at this now, it seems unnecessary since it is focused on OM's text. Do we want to proceed anyway? (happy to address the comments if this is still useful) |
I think this might be useful, especially once we implement _Created TS handling for OM in Prometheus (and to test that). But it's low prio, so feel free to close for now OR finish it by adding second function / option for opt-in created timestamp series (: |
Once prometheus/prometheus#12733 gets merged I'll get back to this and start working towards ingesting _created from OM :) |
8ffc995
to
913b8f0
Compare
I'm finally continuing this PR! Rebased and made printing of created lines optional as requested |
I'm now looking at how client_golang can use this, maybe this PR will need to change strategies 🤔 Client golang calls NegotiateWithOpenMetrics to decide which content type to use, and then a new encoder is created according to the chosen format. If we add another option to NewEncoder, telling that there is yet another OM option we will need another option for content-type negotiation as well 🤔 For example: case FmtOpenMetrics_0_0_1, FmtOpenMetrics_1_0_0:
return encoderCloser{
encode: func(v *dto.MetricFamily) error {
_, err := MetricFamilyToOpenMetrics(w, v)
return err
},
close: func() error {
_, err := FinalizeOpenMetrics(w)
return err
},
}
case FmtOpenMetrics_with_CT:
encode: func(v *dto.MetricFamily) error {
_, err := MetricFamilyToOpenMetrics(w, v, WithCreatedLines())
return err
},
close: func() error {
_, err := FinalizeOpenMetrics(w)
return err
},
} And that doesn't sound like a good approach. |
d3383ad
to
bdac7de
Compare
Added an extra commit that adds the same |
bdac7de
to
efc83ad
Compare
efc83ad
to
f34371c
Compare
@bwplotka, @TheSpiritXIII @SuperQ It's been a while, but I'm finally continuing this PR and will work on supporting _created lines in OM Text in client_golang and Prometheus afterward. WIP PRs:
Do you mind giving reviewing this one again?
|
f34371c
to
4e141fb
Compare
6a6b93b
to
dd191f6
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.
Nice, well-done, only a small thing, looking good 👍🏽
36d5bfb
to
6330ddb
Compare
…and histograms Signed-off-by: Arthur Silva Sens <[email protected]>
Signed-off-by: Arthur Silva Sens <[email protected]>
Signed-off-by: Arthur Silva Sens <[email protected]>
6330ddb
to
fb88846
Compare
Conflicts resolved again 😅 |
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.
Minor nit about some memory allocation, otherwise LGTM.
Signed-off-by: Arthur Silva Sens <[email protected]>
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [github.com/prometheus/common](https://togithub.com/prometheus/common) | `v0.48.0` -> `v0.52.3` | [![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fprometheus%2fcommon/v0.52.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fprometheus%2fcommon/v0.52.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fprometheus%2fcommon/v0.48.0/v0.52.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fprometheus%2fcommon/v0.48.0/v0.52.3?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>prometheus/common (github.com/prometheus/common)</summary> ### [`v0.52.3`](https://togithub.com/prometheus/common/compare/v0.52.2...v0.52.3) [Compare Source](https://togithub.com/prometheus/common/compare/v0.52.2...v0.52.3) ### [`v0.52.2`](https://togithub.com/prometheus/common/releases/tag/v0.52.2) [Compare Source](https://togithub.com/prometheus/common/compare/v0.51.1...v0.52.2) #### What's Changed - Drop support for Go older than 1.18 by [@​SuperQ](https://togithub.com/SuperQ) in [https://github.com/prometheus/common/pull/612](https://togithub.com/prometheus/common/pull/612) - fix(protobuf): Correctly decode multi-messages streams by [@​srebhan](https://togithub.com/srebhan) in [https://github.com/prometheus/common/pull/616](https://togithub.com/prometheus/common/pull/616) - Bump github.com/aws/aws-sdk-go from 1.50.31 to 1.51.11 in /sigv4 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/prometheus/common/pull/615](https://togithub.com/prometheus/common/pull/615) #### New Contributors - [@​srebhan](https://togithub.com/srebhan) made their first contribution in [https://github.com/prometheus/common/pull/616](https://togithub.com/prometheus/common/pull/616) **Full Changelog**: prometheus/common@v0.51.1...v0.52.2 ### [`v0.51.1`](https://togithub.com/prometheus/common/releases/tag/v0.51.1) [Compare Source](https://togithub.com/prometheus/common/compare/v0.51.0...v0.51.1) #### What's Changed - Synchronize common files from prometheus/prometheus by [@​prombot](https://togithub.com/prombot) in [https://github.com/prometheus/common/pull/606](https://togithub.com/prometheus/common/pull/606) - Synchronize common files from prometheus/prometheus by [@​prombot](https://togithub.com/prombot) in [https://github.com/prometheus/common/pull/609](https://togithub.com/prometheus/common/pull/609) - Retract v0.50.0 by [@​SuperQ](https://togithub.com/SuperQ) in [https://github.com/prometheus/common/pull/610](https://togithub.com/prometheus/common/pull/610) **Full Changelog**: prometheus/common@v0.51.0...v0.51.1 ### [`v0.51.0`](https://togithub.com/prometheus/common/releases/tag/v0.51.0) [Compare Source](https://togithub.com/prometheus/common/compare/v0.50.0...v0.51.0) #### What's Changed - Synchronize common files from prometheus/prometheus by [@​prombot](https://togithub.com/prombot) in [https://github.com/prometheus/common/pull/604](https://togithub.com/prometheus/common/pull/604) - expfmt: Add a way to generate different OpenMetrics Formats by [@​ywwg](https://togithub.com/ywwg) in [https://github.com/prometheus/common/pull/596](https://togithub.com/prometheus/common/pull/596) - Fix string slice definition for FormatFlagOptions. by [@​gizmoguy](https://togithub.com/gizmoguy) in [https://github.com/prometheus/common/pull/607](https://togithub.com/prometheus/common/pull/607) - Correct logic in sample naming for counters, add new test by [@​vesari](https://togithub.com/vesari) in [https://github.com/prometheus/common/pull/608](https://togithub.com/prometheus/common/pull/608) #### New Contributors - [@​gizmoguy](https://togithub.com/gizmoguy) made their first contribution in [https://github.com/prometheus/common/pull/607](https://togithub.com/prometheus/common/pull/607) **Full Changelog**: prometheus/common@v0.50.0...v0.51.0 ### [`v0.50.0`](https://togithub.com/prometheus/common/releases/tag/v0.50.0) [Compare Source](https://togithub.com/prometheus/common/compare/v0.49.0...v0.50.0) #### What's Changed - Synchronize common files from prometheus/prometheus by [@​prombot](https://togithub.com/prombot) in [https://github.com/prometheus/common/pull/594](https://togithub.com/prometheus/common/pull/594) - Bump github.com/stretchr/testify from 1.8.4 to 1.9.0 in /sigv4 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/prometheus/common/pull/593](https://togithub.com/prometheus/common/pull/593) - Bump github.com/aws/aws-sdk-go from 1.50.27 to 1.50.29 in /sigv4 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/prometheus/common/pull/592](https://togithub.com/prometheus/common/pull/592) - Bump github.com/aws/aws-sdk-go from 1.50.29 to 1.50.31 in /sigv4 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/prometheus/common/pull/595](https://togithub.com/prometheus/common/pull/595) - Remove unused 'Host' member from HTTPClientConfig by [@​bboreham](https://togithub.com/bboreham) in [https://github.com/prometheus/common/pull/597](https://togithub.com/prometheus/common/pull/597) - Add OpenMetrics unit support by [@​vesari](https://togithub.com/vesari) in [https://github.com/prometheus/common/pull/544](https://togithub.com/prometheus/common/pull/544) - Remove deprecated version function by [@​SuperQ](https://togithub.com/SuperQ) in [https://github.com/prometheus/common/pull/591](https://togithub.com/prometheus/common/pull/591) - Synchronize common files from prometheus/prometheus by [@​prombot](https://togithub.com/prombot) in [https://github.com/prometheus/common/pull/599](https://togithub.com/prometheus/common/pull/599) - Bump golang.org/x/oauth2 from 0.17.0 to 0.18.0 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/prometheus/common/pull/600](https://togithub.com/prometheus/common/pull/600) - Bump google.golang.org/protobuf from 1.32.0 to 1.33.0 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/prometheus/common/pull/601](https://togithub.com/prometheus/common/pull/601) **Full Changelog**: prometheus/common@v0.49.0...v0.50.0 ### [`v0.49.0`](https://togithub.com/prometheus/common/releases/tag/v0.49.0) [Compare Source](https://togithub.com/prometheus/common/compare/v0.48.0...v0.49.0) #### What's Changed - Synchronize common files from prometheus/prometheus by [@​prombot](https://togithub.com/prombot) in [https://github.com/prometheus/common/pull/574](https://togithub.com/prometheus/common/pull/574) - Bump github.com/aws/aws-sdk-go from 1.49.13 to 1.50.8 in /sigv4 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/prometheus/common/pull/571](https://togithub.com/prometheus/common/pull/571) - Synchronize common files from prometheus/prometheus by [@​prombot](https://togithub.com/prombot) in [https://github.com/prometheus/common/pull/581](https://togithub.com/prometheus/common/pull/581) - Update Go by [@​SuperQ](https://togithub.com/SuperQ) in [https://github.com/prometheus/common/pull/588](https://togithub.com/prometheus/common/pull/588) - Deprecate version.NewCollector by [@​SuperQ](https://togithub.com/SuperQ) in [https://github.com/prometheus/common/pull/579](https://togithub.com/prometheus/common/pull/579) - Bump github.com/aws/aws-sdk-go from 1.50.8 to 1.50.27 in /sigv4 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/prometheus/common/pull/587](https://togithub.com/prometheus/common/pull/587) - Avoid off-spec openmetrics exposition when exemplars have empty labels by [@​orls](https://togithub.com/orls) in [https://github.com/prometheus/common/pull/569](https://togithub.com/prometheus/common/pull/569) - Bump golang.org/x/oauth2 from 0.16.0 to 0.17.0 by [@​dependabot](https://togithub.com/dependabot) in [https://github.com/prometheus/common/pull/585](https://togithub.com/prometheus/common/pull/585) - Write created lines when negotiating OpenMetrics by [@​ArthurSens](https://togithub.com/ArthurSens) in [https://github.com/prometheus/common/pull/504](https://togithub.com/prometheus/common/pull/504) - Upgrade client_model to v.0.6.0 by [@​vesari](https://togithub.com/vesari) in [https://github.com/prometheus/common/pull/589](https://togithub.com/prometheus/common/pull/589) - http_config: Add host by [@​jkroepke](https://togithub.com/jkroepke) in [https://github.com/prometheus/common/pull/549](https://togithub.com/prometheus/common/pull/549) - LabelSet: Fix alphabetical sorting for prometheus LabelSet by [@​wasim-nihal](https://togithub.com/wasim-nihal) in [https://github.com/prometheus/common/pull/575](https://togithub.com/prometheus/common/pull/575) - labelset: optimise String() function by [@​bboreham](https://togithub.com/bboreham) in [https://github.com/prometheus/common/pull/590](https://togithub.com/prometheus/common/pull/590) #### New Contributors - [@​orls](https://togithub.com/orls) made their first contribution in [https://github.com/prometheus/common/pull/569](https://togithub.com/prometheus/common/pull/569) - [@​vesari](https://togithub.com/vesari) made their first contribution in [https://github.com/prometheus/common/pull/589](https://togithub.com/prometheus/common/pull/589) **Full Changelog**: prometheus/common@v0.48.0...v0.49.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "on tuesday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/open-telemetry/opentelemetry-collector). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yMjAuMiIsInVwZGF0ZWRJblZlciI6IjM3LjI2OS4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: opentelemetrybot <[email protected]>
Following up on the Design doc, this PR aims to implement writing created timestamps when using OpenMetrics.