aws / aws/aws-xray-sdk-node

Request for Review: Adding node-fetch support to XRay

Open
#585 2 comments 0 reactions 0 assignees View on GitHub
feature request
Dominant language
JavaScript
Stars
280
Forks
157
PR merge metrics
No merged PRs in 30d

Description

Hi, given that fetch is going to be in mainline NodeJS, not to mention it's easier to use than alternatives, I would like to see it supported in XRay. To that end, I was wondering if _somebody_ could take a look at my initial attempt and see if I'm on the right path. Most notably, I'm still a little fuzzy on what automatic versus manual mode entails. The code for my fetch capture routines in my forked repo is [here.](https://github.com/jasonterando/aws-xray-sdk-node/blob/fetch/packages/core/lib/patchers/fetch_p.js)

Some particulars:

1. The patch _should_ support both "built-in" Fetch in current NodeJS versions, as well as manually adding the node-fetch package.
2. Like fetch itself, the patched calls are set up for async/promises
3. I have not yet added Typescript defs (although will be easy to do)
4. Need to build unit testing around automatic/manual mode (once somebody confirms I'm taking an appropriate approach)

If somebody can either confirm it's looking okay, or give me guidance on what needs to be corrected, I can work on finishing the unit tests, TypeScript defs, etc. and submit a PR.

The full forked/branched repo is [here]
(https://github.com/jasonterando/aws-xray-sdk-node/tree/fetch)

Thanks in advance

Contributor guide

Open the contributing guide

Research direction

Start with the proposed fetch capture routines in packages/core/lib/patchers/fetch_p.js from the linked fork, then review how automatic and manual modes are handled. Add support for built-in Fetch and manually installed node-fetch, cover both modes with unit tests, and add the missing TypeScript definitions before submitting a pull request.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, node.js
Domain
backend
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.