Skip to content
This repository has been archived by the owner on Sep 15, 2022. It is now read-only.

Prometheus Monitoring Middleware/Tripperware #12

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mwitkow
Copy link

@mwitkow mwitkow commented May 3, 2017

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 the Handler are identified using handlerName tag and handlerGroupName. Client calls are identified by serviceName tag.

This implements post-call metrics for:

  • handled requests with HTTP methods and HTTP status codes
  • latency until HTTP headers - indicating time to respond without the delay of body handlig
  • latency until complete handling (server side) - indicating all time resources are used.

@mwitkow
Copy link
Author

mwitkow commented May 4, 2017

@beorn7 can you take a peak here if you have time, as you wrote the original prometheus.InstrumentHandler?

This is prometheus handler and client-side monitoring for HTTP that fits into this whole ecosystem of client-side and server side HTTP wares.

@yifanzz as well :)

@beorn7
Copy link

beorn7 commented May 4, 2017

@stuartnelson3 and I have been working on replacing the deprecated prometheus.InstrumentHandler for a while now. Server middleware is already in head (package promhttp), while roundtripper-ware is being reviewed in prometheus/client_golang#295 .

Happy to have a look here, too.

@mwitkow
Copy link
Author

mwitkow commented May 4, 2017

@beorn7, I saw the new server-side code. However, there's two things that are missing:

  • we want to make sure that we can group handlers together, so instead of a single handler we would like to have two labels.
  • 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.

In terms of round-tripper:
Yeah, I thought about using the httptrace, but most of it seems massive overkill. Instead the things we found is that having a class of "client-side connection errors" is very important (to differentiate between resolution and timeouts and connectivity errors).

Would love to have this code use exactly the same names and labels, if not the upstream code.

@beorn7
Copy link

beorn7 commented May 11, 2017

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 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 , perhaps that's something you would be interested in adding? Now that you are the middleware expert for client_golang. :)

@mwitkow
Copy link
Author

mwitkow commented May 11, 2017 via email

@beorn7
Copy link

beorn7 commented May 12, 2017

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants