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

Create new tools for http.Handler instrumentation #224

Closed
beorn7 opened this issue Aug 16, 2016 · 6 comments
Closed

Create new tools for http.Handler instrumentation #224

beorn7 opened this issue Aug 16, 2016 · 6 comments
Assignees
Milestone

Comments

@beorn7
Copy link
Member

beorn7 commented Aug 16, 2016

As mentioned in the doc comment, InstrumentHandler has issues beyond repair.
In the promhttp package, new utilities should be provided to instrument handlers and round-trippers in a more specific way.

See #200, #101, #57 for other issues to solve on the way.

@beorn7 beorn7 added the v0.9 label Aug 16, 2016
@beorn7 beorn7 added this to the v0.9 milestone Aug 19, 2016
@beorn7
Copy link
Member Author

beorn7 commented Oct 5, 2016

@orian
Copy link

orian commented Jan 13, 2017

Hi. I need this, is anyone else working on it? If not, I'll try to work on it.

@beorn7
Copy link
Member Author

beorn7 commented Jan 13, 2017

@stuartnelson3 is, kind of. And it already became apparent how hard this is to get right and how much context is needed. Not sure if it makes any sense at this time to involve somebody who misses that context.
If you are creating something like this anyway, no harm in sharing, and if it fits this library, we'll happily integrate.

@orian
Copy link

orian commented Jan 13, 2017

@beorn7 Sure, I understand.

@stuartnelson3 Is this context to read somewhere? Is the current work available somewhere?

Edit: I see you both work at SoundCloud so I guess it was chatted about?

@stuartnelson3
Copy link
Contributor

That is correct, context was shared with me face-to-face. I'll try to look at this soon, but as is recommended in the library, rolling your own custom handler, at this point, will probably be a much quicker solution.

@orian
Copy link

orian commented Jan 16, 2017

Just for others (following comment in: https://github.com/prometheus/client_golang/blob/master/prometheus/http.go#L152):

  • switching to seconds is easy;
  • switching to histogram is easy, the problem may be how to choose bucket sizes. I've decided to use {0.1, 0.2, ..., 1.0, 2.0, ... 10.0}; I guess most people expect their service to serve response strongly under 1 sec;
  • keeping the Summary for a given instance may make sense;
  • calculating the size of Request is not trivial as the Handler/HandleFunc gets parsed Request, the best would be to modify the go http.Server to accumulate the size (it's returned anyway during read phase);

The links provided by by @beorn7 are about the client tracing and required to modify the net/http.Transport

You can find my instrumented handler at: https://github.com/orian/go-http-instrument

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

No branches or pull requests

3 participants