Opentelemetry-go: Unify Configuration and Initialization

[MrAlias]

Being consistent in how we configure and create things in the project means users can easily anticipate what they need to do. Currently this is not the case.

We should standardize on the approach outlined in this comment:

type Option func(*Config)
type Config struct { ... }
func NewT(config Config) *T { ... }
func ConfigureT(opts ...Option) *T { 
  config := Config{}
  for _, opt := range opts {
    opt(&config)
  }
  t := NewT(config)
  // ...
}

[Aneurysm9]

There was discussion regarding whether Option should be a function or an interface. I think there may be situations where an Option might require more than a bare function, so I would propose the following types:

type Option interface {
  Apply(*Config)
}

type OptionFunc func(*Config)

func (o OptionFunc) Apply(c *Config) {
  o(c)
}

Then simple options can be implemented in functions and wrapped with OptionFunc:

func WithName(name string) Option {
  return OptionFunc(func(c *Config) {
    c.name = name
  })
}

[krnowak]

One place where bare functions weren't enough was in metrics options for instruments. We used to have three kinds of instruments (counters, gauges and measures). They had some common options (like WithUnit or WithDescription). They also had their own specific options (like measure had WithAbsolute). Some of them shared some option name (like gauge and counter had WithMonotonic option). This is all gone and I'm not sure if they'll come back, but anyway. To solve I had:

type Config struct {…}
// measure-specific options
type MeasureOption interface {
    ApplyMeasure(*Config)
}
// gauge-specific options
type GaugeOption interface {…}
// counter-specific options
type CounterOption interface {…}
// general option (with unit, with description)
// this can be passed as an instrument-specific option too
type Option interface {
    MeasureOption
    GaugeOption
    CounterOption
}

func WithDescription(n string) Option {…}
func WithAbsolute(a bool) MeasureOption {…}

func NewMeasure(n string, o ...MeasureOption) Measure {…}
func NewCounter(n string, o ...CounterOption) Counter {…}

so NewMeasure("foo", WithUnit("cm"), WithAbsolute(true)) worked fine, but NewCounter("bar", WithAbsolute(false)) was a compilation error as desired.

So I'd amend the proposal above to what @Aneurysm9 proposed:

type Option interface {
  // this can be named differently in complex cases
  Apply(*Config)
}
// this can be a struct in complex cases
type OptionFunc func(*Config)
func (o OptionFunc) Apply(c *Config) {
  o(c)
}
type Config struct { ... }
// Not sure to standardize on those, though
func Configure(opts ...Option) Config {
  config Config{}
  for _, opt := range opts {
    opt(&config)
  }
  return config
}
func NewT(config Config) *T { ... }
func ConfigureT(opts ...Option) *T { 
  t := NewT(Configure(opts…))
  // ...
}

[johananl]

Happy to take this one @MrAlias.

[krnowak]

Maybe one thing that needs to be looked at, is how to configure two same things within one object. So an example would be in otlp exporter, where we can configure (not in master yet) two connections separately (one for sending metrics, one for sending traces).

So I don't have an ideal solution yet, but that's what I have done for now:

type Config struct {
    // unexported fields
}

type Option func(*Config)

// makes a copy of config
func (Config) Apply(Option ...opts) Config { … }

// modifies config in place
func (*Config) ApplyInPlace(Option ...opts) { … }

type ConnectionManager interface { … }

func NewSingleConnectionManager(cfg Config)  ConnectionManager { … }

type MultiConnectionConfig struct {
    Tracing Config
    Metrics Config
}

func NewMultiConnectionManager(cfg MultiConnectionConfig) ConnectionManager { … }

so the use is:

cfg := Config{}.Apply(WithThis(…), WithThat(…))
cm := NewSingleConnectionManager(cfg)
// or
cfg := MultiConnectionConfig{}
cfg.Tracing.ApplyInPlace(WithThis(…), WithThat(…))
cfg.Metrics.ApplyInPlace(WithThis(…), WithSomethingElse(…))
cm := NewMultiConnectionManager(cfg)

For NewMultiConnectionManager, having func NewMultiConnectionManager(opts ...Option) ConnectionManager would not work, because how do we configure tracing connection separately from metrics connection without doing WithTracingThis and WithMetricsThis?

[johananl]

My WIP is here: https://github.com/johananl/opentelemetry-go/tree/johananl/refactor-config-init