go-nsq
go-nsq copied to clipboard
Allow decorating ConsumerDelegate
We want to instrument our NSQ clients and detect the E_FIN_FAILED
errors received here:
https://github.com/nsqio/go-nsq/blob/61f49c096d0d767be61e4de06f724e1cb5fd85d7/conn.go#L530-L532
It looks like the best way of doing this would be to implement a custom ConnDelegate
listening to the OnError
method:
https://github.com/nsqio/go-nsq/blob/61f49c096d0d767be61e4de06f724e1cb5fd85d7/delegates.go#L66-L68
Instead of just calling consumerConnDelegate
https://github.com/nsqio/go-nsq/blob/61f49c096d0d767be61e4de06f724e1cb5fd85d7/delegates.go#L111
Which actually does nothing
https://github.com/nsqio/go-nsq/blob/61f49c096d0d767be61e4de06f724e1cb5fd85d7/consumer.go#L680
Maybe instead of using just one delegate, we could have a slice of ConnDelegate
s in the consumer? Since those are not exported, that would not change the API and will not affect the existing users.
In order to keep backwards compatibility, we'd have to create a
func NewConsumerDelegates(topic string, channel string, config *Config, delegates ...ConnDelegate) (*Consumer, error) {
Constructor, or even better, looking into the future where someone else may need configuring something else, the constructor may accept functional options with something like:
type ConsumerOption func(*Consumer)
func WithConsumerConnDelegate(delegate ConnDelegate) ConsumerOption {
return func(r *Consumer) {
r.delegates = append(r.delegates, delegate)
}
}
func NewConsumerWithOptions(topic, channel string, config *Config, options ...ConsumerOption) (*Consumer, error) {
// ...
r := &Consumer{ /* ... */}
for _, applyOption := range options {
applyOption(r)
}
// ...
return r, nil
}
Of course, another option would be adding a ConnDelegate
slices into the existing Config
, but I'm not sure how open is that for accepting behaviours instead of just values.
@ploxiln can I ask for you feedback on this as you're the most recently active member of NSQ?
Ping @mreiferson @jehiah 🙏 We would be happy to contribute back this patch but would appreciate some early feedback 😃
Of course, another option would be adding a ConnDelegate slices into the existing Config, but I'm not sure how open is that for accepting behaviours instead of just values.
I'm curious at what this implementation would look like, as I'd prefer to not have the "how should we set options" debate and instead more specifically think through the tradeoffs of making it possible for users to chain delegates.