dazn-lambda-powertools icon indicating copy to clipboard operation
dazn-lambda-powertools copied to clipboard

Move HTTP client to built-in http/https modules

Open theburningmonk opened this issue 5 years ago • 3 comments

This is a Feature Proposal

Description

If you're using X-Ray, then the X-Ray SDK lets you instrument the http/https modules to support tracing through API Gateway. But since we're using superagent in our HTTP client, it means we're not able to leverage that capability.

It'll also help to make the HTTP client lighter as well.

theburningmonk avatar Jun 21 '19 09:06 theburningmonk

@simontabor @ThomasRogersDAZN @gsingh1 @justin-caldicott @rehangit @ChrisWestcottUK what do you guys think?

Is having a custom HTTP client even the right way to go? Everyone seems to have their preferred HTTP client already, and you can already grab the correlation IDs from the correlation-ids package and shove them into headers.

We can provide value-adds like metrics and possibly IAM authentication as well (not in there yet).

theburningmonk avatar Jun 21 '19 09:06 theburningmonk

@theburningmonk are you proposing to monkey patch http?

Personally I think this is a really good idea, although we'd have to be very careful in the implementation as the http module has changed quite a lot between node versions.

ChrisWestcottUK avatar Jun 27 '19 09:06 ChrisWestcottUK

Building our own client feels like reimplementing the wheel and we'd risk a lot of feature bloat. I'd vote for deprecating the HTTP client and building small plugins for common modules (axios, request, etc) which can automatically add correlation IDs and track metrics. This keeps things simple and allows devs to continue using the libraries which they're comfortable with.

You can use captureHTTPsGlobal to capture all http(s) requests by default (proper monkey patch)

simontabor avatar Jul 02 '19 15:07 simontabor