Skip to content
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

OM-42925: Fix logging errors #60

Merged
merged 4 commits into from
Feb 20, 2019

Conversation

ading1977
Copy link
Collaborator

Problem

Original problem reported as follows:
I0211 15:08:18.775160 1 remote_mediation_client.go:415] [ActionMessageHandler] Received: action {%!s(*proto.MediationServerMessage_ActionRequest=&{0xc00072c000}) %!s(*int32=0xc001f58b0c) } request
from the following line of code:
glog.V(4).Infof("[ActionMessageHandler] Received: action %s request", serverRequest)

Cause

We are trying to print out a protobuf message for debugging purpose. The protobuf message type implements the String() method that satisfies the Stringer interface, however, the method has a pointer receiver instead of a value receiver:
func (m *MediationServerMessage) String() string { return proto.CompactTextString(m) }
Because we are only passing in a value of proto.MediationServerMessage, the implemented String() method is never called.

Fix

  • The simple fix is to pass in a pointer to the protobuf message to the Infof() function.
  • There are a few other places in the code with similar issues, or with incorrect print format. Fix them together.

@ading1977
Copy link
Collaborator Author

Test

After the fix, the human readable pb message is printed out:
I0219 15:34:44.990976 1 remote_mediation_client.go:412] [ActionMessageHandler] Received: action request actionRequest:<probeType:"Kubernetes-3408882790" accountValue:<key:"targetIdentifier" stringValue:"Kubernetes-spc-smalltest" > accountValue:<key:"password" stringValue:"defaultPassword" > accountValue:<key:"username" stringValue:"defaultUser" > actionExecutionDTO:<actionType:SUSPEND actionItem:<actionType:SUSPEND uuid:"_ebVhgjRXEem9upHbj4OBUw" targetSE:<entityType:CONTAINER_POD id:"e4359cab-3456-11e9-a464-0e8e22e09e66" displayName:"default/twitter-cass-api-6d9d98c875-xc7tt" commoditiesSold:<commodityType:VCPU used:15.328129768371582 reservation:650 capacity:5000 limit:0 vcpu_data:<hotAddSupported:false hotRemoveSupported:false > > commoditiesSold:<commodityType:VMEM used:85916 reservation:65536 capacity:7.872444e+06 limit:0 vmem_data: ... ...

Copy link
Contributor

@maxwangvmt maxwangvmt left a comment

Choose a reason for hiding this comment

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

LGTM. Please fix the go fmt issue before merging.

@coveralls
Copy link

Coverage Status

Coverage increased (+24.4%) to 75.353% when pulling 0051f1d on ading1977:fix-logging-errors into aed6fd0 on turbonomic:master.

@ading1977 ading1977 merged commit abf93a0 into turbonomic:master Feb 20, 2019
@ading1977 ading1977 deleted the fix-logging-errors branch February 20, 2019 02:22
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