add aws-sdk as parameter to xray helper

[cmjarreau]

this will give us the functionality to accommodate the conditional operators encountered in many cases by being able to return a captureAWS object from the aws-sdk

[coveralls]

Pull Request Test Coverage Report for Build 169

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 100.0%

---

Totals
Change from base Build 155:	0.0%
Covered Lines:	375
Relevant Lines:	375

---

💛 - Coveralls

[mdlavin]

Would it be possible to make awsSdk as optional, because I think most times default aws-sdk would be fine.

[jagoda]

I think that using the default is actually problematic unless we add a peerDependencies requirement to package.json. If the calling project and lambda-tools do not agree on the aws-sdk version then the caller's SDK instance will not get instrumented and there will not be any warning.

[mdlavin]

Good point. I was trying to keep the calling code simple but it might not be worth the effort to avoid the extra require. Maybe a comment could be added to the code to explain for future readers

…

On Thu, Aug 9, 2018, 9:43 AM Jeffrey Jagoda ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/xray.js <#28 (comment)>: > @@ -1,9 +1,7 @@ const xray = require('aws-xray-sdk-core'); -exports.captureWithXRay = () => { - if (process.env.LAMBDA_TASK_ROOT) { - xray.captureHTTPsGlobal(require('http')); - xray.captureAWS(require('aws-sdk')); - xray.capturePromise(); - } +exports.captureWithXRay = function (awsSdk) { I think that using the default is actually problematic unless we add a peerDependencies requirement to package.json. If the calling project and lambda-tools do not agree on the aws-sdk version then the caller's SDK instance will not get instrumented and there will not be any warning. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#28 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGhl-Ex2YE1vhuT3NSYdAYd1dhb43zQks5uPDxqgaJpZM4V0e1E> .