Log dispatch errors in stub bus#386
Conversation
knative-prow-robot
added
the
size/XS
greghaynes
force-pushed
the
fix/stub-dispatch-error-log
branch
from
f130cf6 to
4aee5e0
Compare
|
The following is the coverage report on pkg/.
|
|
could you please add |
|
/lgtm |
|
/lgtm This will conflict with #382, but I can manage the diff 😄 |
greghaynes
force-pushed
the
fix/stub-dispatch-error-log
branch
from
4aee5e0 to
e40dc37
Compare
| go func() { | ||
| err := stubSubscription.dispatchMessage(message) | ||
| if err != nil { | ||
| glog.Errorf("Failed to dispatch message: %v", err) |
There was a problem hiding this comment.
Error seems a bit extreme here. The dispatch could fail because of a misconfiguration in the subscription resource or the subscriber is down. It's not necessarily the fault of the bus.
There was a problem hiding this comment.
Good point - since this is also not expected behavior how do we feel about warning?
greghaynes
force-pushed
the
fix/stub-dispatch-error-log
branch
from
e40dc37 to
a6daae9
Compare
|
I was recently bit by the stub bus silently dropping errors and had no idea what was going on until @greghaynes pointed me in this direction and the coredns issues. So, +1 for logging here. |
greghaynes
force-pushed
the
fix/stub-dispatch-error-log
branch
from
a6daae9 to
00e52df
Compare
knative-prow-robot
added
size/S
| ReceiveMessageFunc: func(channelRef buses.ChannelReference, message *buses.Message) error { | ||
| bus.logger.Infof("Recieved message for %q channel", channelRef.String()) | ||
| bus.channel(channelRef).receiveMessage(message) | ||
| bus.channel(channelRef).receiveMessage(bus, message) |
There was a problem hiding this comment.
I'd be more inclined to capture the logger when creating the stubChannel rather then as a function param.
Sorry, this is a moving target. If you'd rather I'd be ok with taking this as is and following up later. (Of course someone with approval rights will need to agree)
There was a problem hiding this comment.
Not a problem - I can update with adding the logger the the stubChannel struct.
If we encounter an error when dispatching an event in the stub bus we silently ignore it.
greghaynes
force-pushed
the
fix/stub-dispatch-error-log
branch
from
00e52df to
b6cf009
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: greghaynes, vaikas-google The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow-robot
added
the
approved
The Channel and Subscription are interesting context to capture for messages logged within stub bus. Follow up to knative#386
The Channel and Subscription are interesting context to capture for messages logged within stub bus. Follow up to #386
Updating the serving/serverless yaml
7 participants
Proposed Changes
If we encounter an error when dispatching an event in the stub bus we
silently ignore it. Lets log the error instead.
Release Note