-
Notifications
You must be signed in to change notification settings - Fork 152
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
Replace import sirupsen/logrus by pkg/log #323
Conversation
d97c470
to
a38a13a
Compare
cool! |
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.
please prefer the structured fields over fmt.Sprintf
.
pkg/blockstorage/awsebs/awsebs.go
Outdated
@@ -522,7 +523,7 @@ func waitOnVolume(ctx context.Context, ec2Cli *EC2, vol *ec2.Volume) error { | |||
for { | |||
dvo, err := ec2Cli.DescribeVolumesWithContext(ctx, dvi) | |||
if err != nil { | |||
log.Errorf("Failed to describe volume %s Error: %+v", aws.StringValue(vol.VolumeId), err) | |||
log.WithError(err).Print(fmt.Sprintf("Failed to describe volume %s", aws.StringValue(vol.VolumeId))) |
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.
please avoid using fmt.Sprintf when logging
Co-Authored-By: Thomas Manville <tom@kasten.io>
Co-Authored-By: Thomas Manville <tom@kasten.io>
0cfdb6d
to
618a1d9
Compare
@@ -165,7 +167,8 @@ func (c *Controller) onUpdate(oldObj, newObj interface{}) { | |||
c.logAndErrorEvent(nil, "Callback onUpdateBlueprint() failed:", "Error", err, new) | |||
} | |||
default: | |||
log.Errorf("Unknown object type <%T>", oldObj) | |||
objType := fmt.Sprintf("%T", oldObj) |
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.
FYI we can just pass in the object directly. this is fine by me though
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.
Awesome!
Co-Authored-By: Thomas Manville <tom@kasten.io>
Change Overview
Replace import sirupsen/logrus by pkg/log
Pull request type
Please check the type of change your PR introduces:
Issues
Test Plan