Hi there. Thank you for your work on this project. X-Ray is great!
I have some concerns about some of the dependency decisions made in the project. I'd like to use the aws-xray-sdk-core module in a Lambda handler, however simply including this library adds around 900 KB of code. That's crazy!
There must be something we can do to reduce the code size. For example, does the package really need to depend on Moment.js? Also, is it necessary to inline polyfills? Is winston really necessary?
Let me know your thoughts. I'm willing to raise a PR with some improvements (like removing Moment, for example.)
Hi adieuadieu,
Winston and Moment are used for formatting and configuring all the SDK output. They are only used to a) capture and format timing information and b) use log levels appropriately. Ideally, we would replace these with our own baked in minimal solution, and create our own base logger object that continues to support log levels and timestamps without the additional large dependencies (or replaced if a lightweight option already exists). If you'd like to open a PR we would be happy to review your changes.
Thanks,
Sandra
@awssandra based on https://github.com/aws/aws-xray-sdk-node/blob/master/packages/core/lib/logger.js#L21, would you welcome a PR to just remove winston entirely? I'm not sure what value it is providing considering the size it adds to the Lambda's when included.
Hi jplock,
Yes, I've been hoping we could to move off Winston for a long while now. It's simply too large/complex for our simple use case. Did you have any suggestions in mind?
And yes, we are always happy to accept CRs!
I'll see if I can submit a PR to remove winston for review.
@jplock check out this PR https://github.com/aws/aws-xray-sdk-node/pull/51
It reduced winston library size from 16 MB to 132 KB for me after upgrade
Thank you for your deep dive and contribution @vladgolubev. I have merged your PR.
We will keep the action item of upgrading Winston3 or a better replacement if possible.
@haotianw465 is it possible to create a new aws-xray-sdk-node release after the winston change?
@jplock just in case there will be no new release, this is can be fixed if you are using yarn via resolutions in package.json
"resolutions": {
"winston": "2.4.4"
}
See https://yarnpkg.com/lang/en/docs/selective-version-resolutions/
Thanks @vladgolubev but we're using npm. I'll wait until the next published released as I don't want to reference GitHub.
Closing issue as all concerns have been addressed.
Thanks!
Most helpful comment
@jplock check out this PR https://github.com/aws/aws-xray-sdk-node/pull/51
It reduced winston library size from 16 MB to 132 KB for me after upgrade