-
Notifications
You must be signed in to change notification settings - Fork 71
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
Integrate airbrake with datastore #146
Conversation
datastore/zmq/zmq.go
Outdated
// ReportError to airbrake and logger | ||
func (p *ZMQProducer) ReportError(message string, err error, logInfo logrus.LogInfo) { | ||
p.airbrakeHandler.ReportLogMessage(logrus.ERROR, message, err, logInfo) | ||
p.ReportError(message, err, logInfo) |
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.
Infinite recursion. Did you mean p.logger.ErrorLog
?
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.
Same for each of the datastores.
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.
hmm.. my branch got messed on rebase. Fixed
@@ -208,6 +214,7 @@ var _ = Describe("BinarySerializer", func() { | |||
result, _ = bs.Deserialize(msgBytes, "Socket-42") | |||
bs.Dispatch(result) | |||
Expect(CallbackTester.counter).To(Equal(1)) | |||
Expect(CallbackTester.errors).To(Equal(0)) |
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.
Can we test when ReportError gets called?
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.
Not on this test - but can we test the case that does
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.
would be hard to simulate this scenario in tests. Will prefer keeping the testing as is.
6c9ca55
to
8240851
Compare
Report datastore errors to airbrake. Note that it will only dispatch errors to airbrake if its actually configured otherwise func (a *AirbrakeHandler) ReportLogMessage acts as a noop.
8240851
to
c5c61d8
Compare
Description
Report datastore errors to airbrake. Note that it will only dispatch errors to airbrake if its actually configured otherwise
func (a *AirbrakeHandler) ReportLogMessage
acts as a noop.Fixes # (issue)
Type of change
Please select all options that apply to this change:
Checklist:
Confirm you have completed the following steps: