-
Notifications
You must be signed in to change notification settings - Fork 21
Prometheus Monitoring Middleware/Tripperware #12
base: master
Are you sure you want to change the base?
Conversation
@stuartnelson3 and I have been working on replacing the deprecated Happy to have a look here, too. |
@beorn7, I saw the new server-side code. However, there's two things that are missing:
In terms of round-tripper: Would love to have this code use exactly the same names and labels, if not the upstream code. |
Hi @mwitkow , Finally, I had a closer look. The selling point of this package is certainly the integration with the http_ctxtags, which are used in all the related package in this repo. In https://github.com/prometheus/client_golang/tree/master/prometheus/promhttp , we tried to solve a bunch of things more flexible by letting the user pass in their own About the missing points:
This is possible with https://github.com/prometheus/client_golang/tree/master/prometheus/promhttp , too, you just have to use multiple
That's a very good point. Which makes me think we should provide a middleware for that case, too, in https://github.com/prometheus/client_golang/tree/master/prometheus/promhttp . @stuartnelson3 , perhaps that's something you would be interested in adding? Now that you are the middleware expert for client_golang. :) |
Oh I wasn't aware that the promhttp was *that* flexible. I honestly wanted
this to be slightly more prescriptive, but align with any aggregation rules
that already exist for promhttp. Given this flexibility I see that it's
unlikely to be worth it.
In regards to service name and const labels, unfortunately this would mean
creating a CounterVec per request (with a lot of allocations) since the
service is not predefined on middleware initialization but comes from the
ctxtags.
In general, I wanted to make sure we at least keep similar naming
conventions. Httpwares is intended to be slightly more prescriptive, and be
a mirror library to go-grpc-middleware.
…On Thu, 11 May 2017, 18:11 Björn Rabenstein, ***@***.***> wrote:
Hi @mwitkow <https://github.com/mwitkow> ,
Finally, I had a closer look. The selling point of this package is
certainly the integration with the http_ctxtags, which are used in all the
related package in this repo.
In
https://github.com/prometheus/client_golang/tree/master/prometheus/promhttp
, we tried to solve a bunch of things more flexible by letting the user
pass in their own ObserverVec or CounterVec. That solves the problem of
custom naming. (The generic http_server_... naming is quite problematic.
Imagine you provide an instrumented handler in a library, but then the
handler is wrapped by another instrumented handler. You want different
metric names for those different layers in your stack.) It also gives the
user the option to use a Histogram or a Summary, and all the bucketing
and other configuration is done as usual. Partitioning by code and/or
method is done automatically, depending on the labels the passed in
vector has. In most cases, for example, you would not like to partition a
(relatively expensive) Histogram, but you could if needed. If you have a
Summary (because you don't need to aggregate the results across
instances), you can afford partitioning more easily, but with our approach,
you don't need a separate middleware for that. In this package, you always
partition the Histogram (and you cannot even opt to use a Summary). This
could easily lead to a time series explosion.
About the missing points:
we want to make sure that we can group handlers together, so instead of a
single handler we would like to have two labels.
This is possible with
https://github.com/prometheus/client_golang/tree/master/prometheus/promhttp
, too, you just have to use multiple ConstLabels. It's a bit more
tedious, but you gain the flexibility in turn.
we found that actually having a "time to respond with headers" is
important, as most code is actually done pre-header return, and this is a
good "operation time" latency measyre, and doesn't involve for example
streaming a large file.
That's a very good point. Which makes me think we should provide a
middleware for that case, too, in
https://github.com/prometheus/client_golang/tree/master/prometheus/promhttp
. @stuartnelson3 <https://github.com/stuartnelson3> , perhaps that's
something you would be interested in adding? Now that you are the
middleware expert for client_golang. :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#12 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AJNWo90e_EAVeYnoea2mhB4oy4n7HNN_ks5r40EvgaJpZM4NPnnA>
.
|
OK, makes sense. About my naming remark: You actually do let the user pass in the "namespace", so my example about layering will work fine by providing different namespaces. |
This adds server-side (Middleware) and client-side (Tripperware) monitoring of HTTP calls in Prometheus.
It uses
http_ctxtags
from this package to make sure that theHandler
are identified usinghandlerName
tag andhandlerGroupName
. Client calls are identified byserviceName
tag.This implements post-call metrics for: