Change http_request metric names to match go #15
Conversation
|
If we're going to change this we should probably switch to seconds while we're at it, and standardise this similar to the standard exports. |
|
http_request_durations_microseconds < http_request_duration_microseconds < http_request_duration_seconds :) In the upcoming revamp of client_golang, I'll press for seconds everywhere, too. |
|
It's @grobie. |
|
I had assumed this would be an easy change. It would appear that keeping the metrics standardized is tricky. If there is anything I can do to help, let me know. |
|
Thinking on it, I'm not sure it's going to be possible to completely standardise as most languages will have multiple HTTP server libraries or middleware each of which we should probably encourage to export similarly (metric type, labels, which if any buckets/quantiles to expose) - but we also need to avoid clashing on names. Considering this is rack instrumentation, What do others think? |
|
I think in general we need the semi-automated HTTP instrumentation to be configurable. Still, we need sane defaults. That will need a more general discussion. What about this change? Should we not change anything until we have settled on something, or should we go for Having |
Having different metric names doesn't prevent that, and that seems like a very rare use case. |
|
I challenge that it is a rare use case. The naming would also be misleading as I don't care on a high level service view what middleware served the HTTP requests I want to monitor. |
|
The problem is though that a given binary may have multiple of such middlewares, so it's not safe for them to share metric names. I care a lot about which piece of code is reporting a given metric, as otherwise I don't know exactly what it's measuring. |
|
Put it in a label, then? At least the architecture of |
|
That'd violate the rule that metrics should be meaningful if summed or averaged. With very few exceptions, a given metric should exist only within a single file. |
|
It does not violate the rule. It's perfectly reasonable to sum up all HTTP requests within a binary. |
|
It's reasonable to count all HTTP requests going through a given bit of code. If you're summing it across several bits of code in different parts of the stack you'll end up counting some requests multiple times, so it's not valid. Each bit of middleware or other http processing code needs it's own metric name. The question here is do we try and want to standardise the general structure of those metrics. |
|
I'm not referring to that case. I'm referring to the case where some requests go through one middleware and some through another. I'm not arguing that there might be the case where you want to encode the middleware in the metric name. I'm just arguing it's not a good default to do so. |
|
You'd have to be very careful with that case - you'd want both middlewares to have exactly the semantics for that to be correct, which is very unlikely (I've seen exactly one use case in this area, where there was an odd interplay between plugins and the framework on top of them). If you wanted that in the same metric the right way would be to do it in the common code that called both middlewares. Otherwise such a use should be strongly discouraged, and not something we should be offering any library-level support for. |
|
OK, now we have put the whole discussion upside-down, implying that a generic name for the HTTP request duration metric is equivalent to library-level support for collecting metrics at various places in the code. After going down that rabbit hole, I still think we should not by default expose details of the used middleware in something as fundamental as the default metric name for HTTP request duration. Please note that it would make utterly no sense in Go as all the middlewares I know work with the same interface, so the function to instrument HTTP calls we have right now could be used on any of them, which makes a reasonable default name that would include the name of the middlename pointless. While convenience HTTP instrumentation should definitely be configurable, it should use a very generic metric name by default. |
I think we should, as that's critical debugging information. Knowing exactly which level of the call stack the latency is reported from helps you figure out which level is introducing the latency. Only where there's a standard http server for a given language/runtime should it get the name
It sounds like we should expose a namespace option there. A given metric name should only be used by a single file/class/module, with very very few exceptions. Something having the same interface doesn't imply it should have the same metric name. In any case, here we're talking about rack metrics. Rack is a middleware that has a number of functions, so it should have |
|
We can do all that, just not by default. In a microservice architecture (which is the one Prometheus was built for), I will have many languages and in each many different ways of serving HTTP requests. Sure, in many situations, I want very specific metric names, and that has to be possible in an easy way. But we need a sane default that doesn't depend on the language I have implemented my microservice in. |
|
If that's the default, then it's going to cause breakage as two distinct bits of code will be exporting the same timeseries as they both will think they know the latency of say We can standardise that the metrics be called |
|
I have to object again: The current behavior of |
|
That's still unsafe in Go, the Go client gives you an error so you find out sooner that it's broken. If a user pulls in two middlewares that both happen to define the same metric, then it'll break. This is not a good outcome for the user, and will not give them a good impression of Prometheus. |
|
I can't follow your argumentation. The ruby client doesn't let you register the same metric twice either. Rack is de facto the standard for ruby HTTP libraries. It's not a web server or a HTTP library though, it's simply an interface description saying a middleware has to respond to The ruby client does not automatically use this middleware. It's provided as additional option for users to make their lives easier. It has to be explicitly embedded in a chain. |
Change http_request metric names to match go
|
@grobie Thanks for ending the stalemate. 😄 I think we should still go from microseconds -> seconds eventually. That seemed to be the consensus by now. @brian-brazil We seem to have very different definitions of "safe". in In completely different news, my current work on |
|
If rack is the defacto standard, then it getting the unprefixed name makes sense. We'll still need some standardisation around that (e.g. it's the metric for the first http layer typically hit).
I think here's the confusion, I'm not worried about "you" doing this as that's obviously your error and one you can easily fix. I'm worried about multiple independent library authors doing it which then makes it impossible to use those libraries together - instrumentation's lite version of dependency hell. Catching it at compile or startup time is better than at scrape time, but it's still the same fundamental problem. If the "you" can't change those libraries, then it's a very bad outcome. I'd also like to avoid this all ending up being configurable (as that's not scalable in different ways), and aim for library metrics to be globally unique across an entire language. My vision is that library authors provide useful prometheus instrumentation by default, without any work required by the user other than choosing which monitoring system they want to hook it into.
You should try Python, at least Go has a common interface :) |
|
On Fri, Sep 11, 2015 at 10:51 AM, Brian Brazil notifications@github.com wrote:
That's all true, but you are talking about a library doing However, this PR is not about how to instrument a library, this is Björn Rabenstein, Engineer SoundCloud Ltd. | Rheinsberger Str. 76/77, 10115 Berlin, Germany |
I recently used prometheus to monitor web servers in go and ruby/rake that are using the official prometheus client libraries. I noticed that the go and ruby/rake client libs were using different names for the http metrics:
I assumed that the prometheus clients should be using the same metric names.