Open jasonterando opened 1 year ago
Hi @jasonterando, thanks for contributing this instrumentation! The implementation looks good so far, I think it's on the right track and follows the structure of the current instrumentations in this package. The main point I wanted to call out was the possibility of supporting the new fetch API as well as node-fetch, which I see you were one step ahead on :)
Not sure if you had the time to skim through some of the automatic vs. manual mode documentation we have, so I'll leave a few references that would hopefully help clarify the difference. There's a simple explanation in the express package that says:
The AWS X-Ray SDK Core has two modes - manual and automatic.
Automatic mode uses the cls-hooked package and automatically tracks the current
segment and subsegment. This is the default mode.
Manual mode requires that you pass around the segment reference.
References:
AWSXRay.getSegment()
, since it is stored in the context created using the cls-hooked package. These docs have a few simple examples on how context works behind the scenes. The idea is, in manual mode (with no context), you would have to manually mimic the behavior of a context by setting the current segment yourself so that the SDK knows which segment is currently active. Hope this helps, please feel free to ask any follow-up questions on this issue. Looking forward to the PR!
Ok, after reading through the links you sent, I think I have something that's workable. I've submitted a PR, but your pipelines are very angry about Lerna on any NodeJS version past 14. If I need to adjust package.json (or anything else) let me know.
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.
Some particulars:
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