Skip to content

Histogram issue with time unit duration_seconds #106

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

Open
prashanth057 opened this issue May 5, 2020 · 5 comments
Open

Histogram issue with time unit duration_seconds #106

prashanth057 opened this issue May 5, 2020 · 5 comments

Comments

@prashanth057
Copy link

When I declare a histogram metric with a name that ends in duration_seconds, for example request_time_duration_seconds, and then use promethues_histogram:observe(request_time_duration_seconds, 30), I would expect this to be recorded as 30 seconds. But when I see the data recorded in the buckets, it is totally off. It seems to record this as 30 nano seconds. Is this a bug or am I missing something ?

@dadarakt
Copy link

dadarakt commented Dec 6, 2020

I can confirm this behavior, had this problem when using a Histogram with a _duration_milliseconds suffix in an Elixir application. This also occurred when using _duration suffix name and setting the duration_unit manually when calling Histogram.new.

I think the intention of the authors is that clients use the macro Histogram.observe_duration/2, as documented in the example section for the histograms?

But I also ran into the same issue by accident, which was somewhat hard to debug. From my point of view this is relevant when developers would like to have a little more control over the computation of the duration and don't want to use the provided macro. Intuitively, when using Histogram.observe/1, I would expect the time unit to be respected as well, so bumping this issue.

If this is an issue with prometheus.ex I'm happy to move the discussion there.

@essen
Copy link
Contributor

essen commented Feb 3, 2021

It expects native time and then converts to seconds automatically.

@dylan-chong
Copy link

Why was this implemented this way? This design decision to change behaviour based on a metric name cost me many hours.
PLEASE DOCUMENT THIS CLEARLY

@mtaylorsamsung
Copy link

This design decision and lack of clear documentation also cost me several hours. A more prominent call-out of this behavior or (better yet, at least in my opinion) a reconsideration of this design decision would maybe help usability a bit for new users.

@essen
Copy link
Contributor

essen commented Apr 21, 2022

I don't know why it was made like this, but I'd be happy to merge a PR updating the documentation. Just ping me on the PR.

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

No branches or pull requests

5 participants