-
Notifications
You must be signed in to change notification settings - Fork 613
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
use structured logger #4046
base: feature/single-region-accounts
Are you sure you want to change the base?
use structured logger #4046
Conversation
should this go straight to dev branch instead of the feature branch? |
|
|
||
if utils.ZeroOrNil(credentials.ARN) && utils.ZeroOrNil(credentials.IAMRoleCredentials) { | ||
// This can happen when the agent is restarted and is reconciling its state. | ||
errText := errPrefix + "Credentials uninitialized for ID" | ||
seelog.Errorf("Error processing credential request credentialType=%s taskARN=%s credentialScope=%s: %s", | ||
credentials.IAMRoleCredentials.RoleType, credentials.ARN, credentials.IAMRoleCredentials.CredentialScope, errText) | ||
logger.Error(fmt.Sprintf("Error processing credential request credentialType=%s taskARN=%s credentialScope=%s: %s", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making these changes!
In addition to replacing seelog with logger in this PR, we should refactor the message with fields defined here to make it easier to read.
For example,
logger.Error("Error processing credential request", logger.Fields{
"credentialType": credentials.IAMRoleCredentials.RoleType,
field.TaskARN: credentials.ARN,
"credentialScope": credentials.IAMRoleCredentials.CredentialScope,
field.Error: errText,
})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, we should think of if any cx application has dependency on the exact log message as these changes are made in TMDEv1.
Summary
This PR use the structured logger for logging in this file to follow the convention and help make the logs more readable and traceable
Testing
New tests cover the changes: no
Description for the changelog
NA
Licensing
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.