feat(HttpClient): add timings to res

[hekike]

Measure timings:

• DNS Lookup
• TCP Connection
• TLS Handshake
• First Byte
• Content Transfer
• Total

Questions

• Should we put it behind a flag?
  • What's the performance impact of calling process.hrtime() multiple times, is there any known?

[coveralls]

Coverage increased (+0.4%) to 86.641% when pulling 9c0bec8e8be88ae2a1ee148b272fe96b2a2dc9dc on hekike:feat/timings into e0f36ef2e606e7ee544d8c95d966191ef0d18637 on restify:master.

[coveralls]

Coverage increased (+0.5%) to 85.267% when pulling f759c5868e96acdc16f3acb9172e868276ac2654 on hekike:feat/timings into bbdeb79442a0b79bf55a788b0a409a0b5dc9cc84 on restify:master.

[coveralls]

Coverage increased (+0.5%) to 85.267% when pulling f759c5868e96acdc16f3acb9172e868276ac2654 on hekike:feat/timings into bbdeb79442a0b79bf55a788b0a409a0b5dc9cc84 on restify:master.

[DonutEspresso]

Love this idea! Assuming no significant perf impacts I'm all for this. The only thing I wonder about is if there might be some value in having an API/getter for this vs just plopping it on the req? Otherwise 👍

[hekike]

@DonutEspresso I turned it into a getter! ;)

[DonutEspresso]

I was looking to see if there were other metrics that might be useful - retries may also be useful since the client by default attempts to retry up to 4 times to establish a connection (i.e., it would be good to know on which did this request succeed in establishing a connection?).

I know this metric isn't strictly timing, so we could punt this to another PR.

EDIT: added this in #149

[hekike]

@DonutEspresso awesome, I see you named it to req.attemtpts(), and I named it to res.getTimings(), I think we should have some alignment about the naming here. I don't feel strongly about the get prefix. What do you think?

PS: seems to be that Node.js prefers to add the get prefix: https://nodejs.org/dist/latest-v8.x/docs/api/http.html#http_response_getheader_name

[DonutEspresso]

get prefix works for me! I'll make that change in the other PR.

I also think we should align on these being on the same object (req or res?). I'm somewhat partial to req since that's how we generally speak about it, e.g., "how long did that request take?" It also makes sense for some things like attempts as well. What do you think?

[hekike]

@DonutEspresso I agree, I'm going to move this to req!

[hekike]

@DonutEspresso rebased and moved to req.

[DonutEspresso]

Left some minor comments, otherwise LGTM! ccing other folks for visibility @retrohacker @rajatkumar

[DonutEspresso]

Awesome, this looks great. Let's get this in, we can iterate on it if we need to.