Un-deprecate NewWriter

[alexec]

Creating a kafka.Writer directly seems to be much more complex that calling NewWriter. For example, it puts into place sensible defaults, makes setting up the kafka.Dialer for TLS really easy. I propose either:

1. Un-deprecate it - it's really useful.
2. Something else?

[achille-roussel]

Hello @alexec.

The kafka.Writer type already uses default values for fields that aren't being set, which are documented, for example:

    // The balancer used to distribute messages across partitions.
    //
    // The default is to use a round-robin distribution.
    Balancer Balancer

Your program does not need to assign every single field of the kafka.Writer type to successfully use it.

Would you be able to provide examples where using the kafka.Writer increases complexity of the code over kafka.NewWriter? These would be useful in helping guide the design decisions.

[alexec]

The specific case I found was setting up TLS, kafka.Writer{} does not have Dialer field. Instead I need to create a Transport. Was not clear how to do so.

[alexec]

Aside:

I think I've mentioned I want both speed and reliability, but reliability trumps speed be default. I think I need to provide a knob so users can choose between them.

I believe that main way to do this would be to expose the BatchTimeout and Async flags to the user. Then they can choose their reliability/speed trade-off.

Does this sound correct?

[nikola-sever-syntio]

Hello,

This is also my concern. We also want to support TLS encryption but now in default Writer type there is, as mentioned before, no Dialer support which is used to implement TLS. And with NewWriter type will lose support in v1. Will there be TLS support implemented into Writer type?

[kishaningithub]

@nikola-sever-syntio / @alexec An example of configuring dialer with a second timeout and TLS

tlsConfig := // construct tls config

kafkaWriter.Transport = &kafka.Transport{
    Dial: (&net.Dialer{
        Timeout: 3 * time.Second,
    }).DialContext,
    TLS: tlsConfig,
}

[nikola-sever-syntio]

@kishaningithub thank you

[achille-roussel]

@alexec

The specific case I found was setting up TLS, kafka.Writer{} does not have Dialer field. Instead I need to create a Transport. Was not clear how to do so.

The writer uses kafka.DefaultTransport by default. You can configure the dialer function there, or create an alternative transport like @kishaningithub suggested. This designed is mirrored after the net/http package.

I believe that main way to do this would be to expose the BatchTimeout and Async flags to the user. Then they can choose their reliability/speed trade-off.

The kafka.Writer type already expose BatchTimeout and Async, so I would assume you are referring to something else. Could you be more specific about where you would like to see these configuration options added in?

[adrian-mcmichael]

To add to the sentiment above, I'd say the pattern you have now allows teams to wrap configuration and provide context local defaults more easily via your WriterConfig struct. To reference a comment on the Writer:

// Methods of Writer are safe to use concurrently from multiple goroutines, // however the writer configuration should not be modified after first use.

This becomes a non-issue if you keep the WriterConfig, as from an engineer's perspective there is less of an expectation that altering a field on a config after its passed to a constructor would have an effect whereas making changes to a field directly on a Writer is something that there would be a viable change to the way it worked.

As it stands at the moment, to apply local contextual config on behalf of teams I need to mirror each and everyone of your config variables if you were to remove the WriterConfig. Whereas if you keep it I can provide context local sensible defaults (cluster addresses etc) whilst still providing deep config for power users, without needing to create a mirrored setting for each Writer setting.

[achille-roussel]

Thanks for the context @adrian-mcmichael, these are interesting points that we'll take into account.

[calmera]

There also seems to be an inconsistency in the api since a reader config requires a Dialer while writers require a transport. Or am I missing something?

[achille-roussel]

You are correct, this is an inconsistency we are planning to resolve in upcoming versions of the package.