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

Don't recommend converting to prometheus base units, but do convert to full words #3019

Closed
wants to merge 1 commit into from

Conversation

dashpole
Copy link
Contributor

@dashpole dashpole commented Dec 7, 2022

As described in #2497 (comment) (and the following discussion), it isn't possible to convert from units to base units (e.g. convert milliseconds to seconds) for all metric types. This PR withdraws that recommendation.

I opened a few issues for adding units to metric names:

During that process, I didn't want to recommend converting to base units for the reasons above, but I do still think implementations should convert to full words where possible.

cc @gouthamve @jmacd

@dashpole dashpole requested review from a team December 7, 2022 01:03
@gouthamve
Copy link
Member

I don't understand why we can't convert from milliseconds to seconds on fixed bucket histograms. Its easy.

Its not possible in exponential histograms, and we have a pending issue here: #2977

I would wait for #2977 to resolve before we decide this tbh.

And even if we keep milliseconds in exponential histograms, I might be tempted to keep seconds for fixed bucket histograms. I need to think more about this.

@dashpole
Copy link
Contributor Author

dashpole commented Dec 7, 2022

Discussed this as the prometheus wg. We are going to hold off on this while we discuss #2977

@dashpole dashpole closed this Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants