Skip to content

Conversation

asubiotto
Copy link

Which issue does this PR close?

Given this is a small change, I did not think it was necessary to open an issue. I can do so if necessary.

Rationale for this change

These log messages are very noisy and it seems like they can be downgraded to debug.

What changes are included in this PR?

Just downgrading log messages from info to debug

Are there any user-facing changes?

Users that would previously see Using * credential provider log messages with info level will not see them any more.

@tustvold
Copy link
Contributor

I don't feel strongly about this, although I do think these logs are useful. I would say if you are seeing lots of these logs it might suggest you are not reusing stores, which will cause serious performance problems. You should not create a store per request for example.

@asubiotto
Copy link
Author

The thing that creates these object stores are cloud run/lambda functions so I don't have much control over that. I also don't feel strongly since I can just turn them off, but the key question is should these log messages be info or debug?

@@ -943,7 +943,7 @@ impl AmazonS3Builder {
} else if self.access_key_id.is_some() || self.secret_access_key.is_some() {
match (self.access_key_id, self.secret_access_key, self.token) {
(Some(key_id), Some(secret_key), token) => {
info!("Using Static credential provider");
debug!("Using Static credential provider");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given these messages are just static strings, I agree that debug is a more appropriate level of logging in my opinion

@asubiotto
Copy link
Author

Updated the PR to fix the build issue; forgot to build with the feature flag locally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants