Enable logging via environment variable

[sabrehagen]

Hi,

I often find it desirable during debugging to turn on logging for aws-sdk so that I can see the requests and their parameters being made to AWS. To enable logging, I am required to alter my code with:

const AWS = require('aws-sdk');
AWS.config.update({ logger: console });

We use aws-sdk in approximately 50 repositories as part of our microservice architecture. This alteration becomes unwieldy when I need to turn on logging for multiple services. It would be highly convenient and efficient to be able to set an environment variable that would turn on logging in aws-sdk, for example AWS_DEBUG=1.

I have made a demonstration of how this might be done in this PR, albeit a bit raw. If you think it can be done in a better place, please alter the PR. I wanted to create a sample implementation to kick of discussion around what I see as a very necessary and useful feature.

[codecov-io]

Codecov Report

Merging #1739 into master will increase coverage by 18.61%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #1739       +/-   ##
==========================================
+ Coverage   77.09%   95.7%   +18.61%     
==========================================
  Files         189     189               
  Lines        6593    6594        +1     
  Branches     1343    1344        +1     
==========================================
+ Hits         5083    6311     +1228     
+ Misses       1510     283     -1227

Impacted Files	Coverage Δ
lib/config.js	87.35% <ø> (+24.56%)	⬆️
lib/s3/managed_upload.js	93.89% <0%> (+0.38%)	⬆️
lib/services/s3.js	98.13% <0%> (+0.7%)	⬆️
lib/model/api.js	100% <0%> (+2.27%)	⬆️
lib/http.js	100% <0%> (+4.83%)	⬆️
lib/node_loader.js	87.5% <0%> (+10.71%)	⬆️
lib/credentials/environment_credentials.js	91.3% <0%> (+13.04%)	⬆️
lib/credentials/credential_provider_chain.js	100% <0%> (+15.62%)	⬆️
lib/credentials.js	100% <0%> (+15.78%)	⬆️
lib/util.js	94.33% <0%> (+16.09%)	⬆️
... and 109 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 700cecd...0639c17. Read the comment docs.

[jeskew]

Hi @sabrehagen,

This seems like a useful addition to the SDK, but we tend to shy away from adding new environment variables (particularly ones prefixed with AWS_) unless there is a plan to add support across all SDKs. Let me shop this around the other SDK teams and see if they want to offer similar support for an AWS_DEBUG environment variable.

[sabrehagen]

Hi @jeskew, how did you go asking around? Can we get something merged? :)

[sabrehagen]

What lends further credence to this is that aws-sdk is globally available in the lambda execution environment which saves packaging aws-sdk with each lambda function.

If this functionality were added into aws-sdk this would benefit all lambda users, and would not require them to deploy a thinly-wrapped copy of aws-sdk with each lambda (which appears to be the only option currently).

[sabrehagen]

Hi @jeskew, do you have any updates on this? I am having trouble seeing any reason why this isn't a useful addition to the library. Thanks :)

[jeskew]

Hi @sabrehagen,

Would you be open to choosing a new mechanism for enabling debug mode? I would be much more open something that makes it clear that the mechanism is specific to this library or to Node applications in general.

I know many libraries will enable debug mode if process.env.NODE_ENV !== 'production', but I'm not sure if suddenly supporting that configuration setting and emitting a bunch of log messages would be reasonable behavior for a library frequently used as a dependency of other libraries.

[sabrehagen]

I know many libraries will enable debug mode if process.env.NODE_ENV !== 'production', but I'm not sure if suddenly supporting that configuration setting and emitting a bunch of log messages would be reasonable behaviour for a library frequently used as a dependency of other libraries.

I agree. Most developers will be operating in a process.env.NODE_ENV !== 'production' environment during their daily work, so enabling log output by default in this environment will not be desirable. It should be opt in only.

Would you be open to choosing a new mechanism for enabling debug mode?

What mechanism do you propose? I've proposed the use of an environment variable as this is de facto in the Node.js ecosystem (example 1, example 2, example 3, example 4, example 5).

I would be much more open something that makes it clear that the mechanism is specific to this library

By supplying the AWS prefix to the environment variable AWS_DEBUG I believe it is clear that this debug option is only specific to the AWS libraries.

[jeskew]

@sabrehagen My concern is that we cannot offer a simple on/off experience in all AWS SDKs, so using the AWS_ prefix implies a level of cross-language support that may not be possible. How about AWSJS_DEBUG?

[sabrehagen]

I see where you're coming from. AWSJS_DEBUG works for me.

[sabrehagen]

I've updated the environment variable name. What's the ETA for getting this merged?

[sabrehagen]

@jeskew Hello again, I wanted to ping you on this issue as I encounter the need for it daily, and want to see what I can do to help get it in the SDK :)

[sabrehagen]

Hi again @jeskew, it's been a bit over a week since I've heard from you. What would you like done to this PR to get it merged?

I have a severe need for this feature every day, often first thing when I start work. Debugging the AWS SDK operations by having to modify the codebase every time I want to see what the SDK is doing is exceptionally time consuming and fragile. This explains my persistence with following up :)

[jeskew]

I'll try to get this prepped for merge today. In the meantime, you can use an environment variable to set the debug level of your code by setting the logger configuration parameter programmatically:

const AWS = require('aws-sdk');
AWS.config.update({ logger: process.env.AWSJS_DEBUG ? console : undefined });

[jeskew]

This was merged in as part of #1783. Thanks for your contribution!

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.