clients
clients copied to clipboard
feat: add support for before/after hooks to support tracing outbound requests
Carries #95 over the finish line - would like to get input from the peanut gallery. This should dove tail nicely with our metrics needs as well.
Coverage increased (+0.3%) to 87.5% when pulling 5ce8ce8a1eacb7d94882a3744a150d6a4fc77dc4 on req-tracing into 471ac194f791fea0e1acd9442a9005ae6948eadf on master.
Hey @joshwilsdon, let's continue the convo here. I rebased against master so this should be in a good place now.
For context, we're looking at building a custom REST client for internal use and we need to do some pretty low level metrics and error logging. That's not currently possible without doing something intrusive like extending directly from JSONClient and overriding existing class methods. These life cycle hooks are a good alternative.
The one thing I would want to add in this PR is the ability for after
to mutate the return values (specifically, err
, but open to having after's next
be a pass through to the final callback). For example, if using Cueball, this after hook would allow one to create an error chain or MultiError combining ConnectionPool errors with the likely last cause of failure via pool.getLastError()
.
Since before
has the ability to mutate the outgoing request, it seems to make sense to make after
symmetrical. If not, after
might be more suitable as an event emitted on the client. Thoughts?
Would be nice to create an OpenTracing
example one day, with the new timings object we could have the full Request and TCP connection lifecycle like here: https://github.com/RisingStack/opentracing-auto/blob/master/src/instrumentation/httpClient.js#L36 just without monkey patching.