Allow decorating ConsumerDelegate

[colega]

We want to instrument our NSQ clients and detect the E_FIN_FAILED errors received here:

go-nsq/conn.go

Lines 530 to 532 in 61f49c0

	case FrameTypeError:
	c.log(LogLevelError, "protocol error - %s", data)
	c.delegate.OnError(c, data)

It looks like the best way of doing this would be to implement a custom ConnDelegate listening to the OnError method:

go-nsq/delegates.go

Lines 66 to 68 in 61f49c0

	// OnError is called when the connection
	// receives a FrameTypeError from nsqd
	OnError(*Conn, []byte)

Instead of just calling consumerConnDelegate

go-nsq/delegates.go

Line 111 in 61f49c0

func (d *consumerConnDelegate) OnError(c *Conn, data []byte) { d.r.onConnError(c, data) }

Which actually does nothing

go-nsq/consumer.go

Line 680 in 61f49c0

func (r *Consumer) onConnError(c *Conn, data []byte) {}

Maybe instead of using just one delegate, we could have a slice of ConnDelegates 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.

[colega]

@ploxiln can I ask for you feedback on this as you're the most recently active member of NSQ?

[jvrplmlmn]

Ping @mreiferson @jehiah 🙏 We would be happy to contribute back this patch but would appreciate some early feedback 😃

[mreiferson]

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.