Hi Eyal, here’s a code review.
Perhaps there’s a simpler name? Something like “timing.Timer” would look
better than “clienttiming.Timer”.
return fmt.Sprintf("%s %s", req.Method, req.URL.Path)
could just be
return req.Method + “ “ + req.URL.Path
which removes the package fmt dependency.
The godoc summary could be improved with a bullet list or by rewriting “It
provides:\n\n…”.
Some of the godoc identifier descriptions are missing the period.
I’m not sure about the Timer factory for http.Client and http.RoundTripper.
I can see that two sets of options need to be provided, but the package API
could be reduced by doing something like:
func NewClient(ctx context.Context, timingOpts []Option, httpOpts []Option)
*http.Client
There is a tradeoff in readability though. Maybe using the options pattern
is adding unnecessary complexity here?
Thanks for the MIT licensing.
Matt
On Monday, February 26, 2018 at 4:16:04 AM UTC-6, Eyal Posener wrote:
>
> Hi,
> Recently mitchellh wrote a really awesome library
> <https://github.com/mitchellh/go-server-timing> that provide HTTP
> middleware for server-timing headers.
> I saw that and thought it would be really nice to automate those headers
> for HTTP calls between servers.
>
> So I created this library: https://github.com/posener/client-timing.
>
> - An HTTP Client or RoundTripper, fully compatible with Go's standard
> library.
> - Automatically time HTTP requests sent from an HTTP handler.
> - Collects all timing headers from upstream servers. So if you called
> server A, A called B and B called C, you'll get all the information in the
> response, assuming all the servers used the middleware and the timing
> client.
> - Customize timing headers according to the request, response and
> error of the HTTP round trip.
>
> Would love to hear your feedback about it.
> Cheers,
> Eyal
>
--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
