Skip to content

Commit

Permalink
Change Options to accept type not pointer (#2558)
Browse files Browse the repository at this point in the history
* Change trace options to accept type not pointer

Add benchmark to show allocation improvement.

* Update CONTRIBUTING.md guidelines

* Update all Option iface

* Fix grammar in CONTRIBUTING
  • Loading branch information
MrAlias committed Feb 1, 2022
1 parent e58070a commit bd817df
Show file tree
Hide file tree
Showing 31 changed files with 477 additions and 277 deletions.
64 changes: 45 additions & 19 deletions CONTRIBUTING.md
Expand Up @@ -228,11 +228,11 @@ all options to create a configured `config`.

```go
// newConfig returns an appropriately configured config.
func newConfig([]Option) config {
func newConfig(options ...Option) config {
// Set default values for config.
config := config{/* […] */}
for _, option := range options {
option.apply(&config)
config = option.apply(config)
}
// Preform any validation here.
return config
Expand All @@ -253,14 +253,17 @@ To set the value of the options a `config` contains, a corresponding

```go
type Option interface {
apply(*config)
apply(config) config
}
```

Having `apply` unexported makes sure that it will not be used externally.
Moreover, the interface becomes sealed so the user cannot easily implement
the interface on its own.

The `apply` method should return a modified version of the passed config.
This approach, instead of passing a pointer, is used to prevent the config from being allocated to the heap.

The name of the interface should be prefixed in the same way the
corresponding `config` is (if at all).

Expand All @@ -283,8 +286,9 @@ func With*(…) Option { … }
```go
type defaultFalseOption bool

func (o defaultFalseOption) apply(c *config) {
func (o defaultFalseOption) apply(c config) config {
c.Bool = bool(o)
return c
}

// WithOption sets a T to have an option included.
Expand All @@ -296,8 +300,9 @@ func WithOption() Option {
```go
type defaultTrueOption bool

func (o defaultTrueOption) apply(c *config) {
func (o defaultTrueOption) apply(c config) config {
c.Bool = bool(o)
return c
}

// WithoutOption sets a T to have Bool option excluded.
Expand All @@ -313,8 +318,9 @@ type myTypeOption struct {
MyType MyType
}

func (o myTypeOption) apply(c *config) {
func (o myTypeOption) apply(c config) config {
c.MyType = o.MyType
return c
}

// WithMyType sets T to have include MyType.
Expand All @@ -326,16 +332,17 @@ func WithMyType(t MyType) Option {
##### Functional Options

```go
type optionFunc func(*config)
type optionFunc func(config) config

func (fn optionFunc) apply(c *config) {
fn(c)
func (fn optionFunc) apply(c config) config {
return fn(c)
}

// WithMyType sets t as MyType.
func WithMyType(t MyType) Option {
return optionFunc(func(c *config) {
return optionFunc(func(c config) config {
c.MyType = t
return c
})
}
```
Expand Down Expand Up @@ -370,12 +377,12 @@ type config struct {

// DogOption apply Dog specific options.
type DogOption interface {
applyDog(*config)
applyDog(config) config
}

// BirdOption apply Bird specific options.
type BirdOption interface {
applyBird(*config)
applyBird(config) config
}

// Option apply options for all animals.
Expand All @@ -385,17 +392,36 @@ type Option interface {
}

type weightOption float64
func (o weightOption) applyDog(c *config) { c.Weight = float64(o) }
func (o weightOption) applyBird(c *config) { c.Weight = float64(o) }
func WithWeight(w float64) Option { return weightOption(w) }

func (o weightOption) applyDog(c config) config {
c.Weight = float64(o)
return c
}

func (o weightOption) applyBird(c config) config {
c.Weight = float64(o)
return c
}

func WithWeight(w float64) Option { return weightOption(w) }

type furColorOption string
func (o furColorOption) applyDog(c *config) { c.Color = string(o) }
func WithFurColor(c string) DogOption { return furColorOption(c) }

func (o furColorOption) applyDog(c config) config {
c.Color = string(o)
return c
}

func WithFurColor(c string) DogOption { return furColorOption(c) }

type maxAltitudeOption float64
func (o maxAltitudeOption) applyBird(c *config) { c.MaxAltitude = float64(o) }
func WithMaxAltitude(a float64) BirdOption { return maxAltitudeOption(a) }

func (o maxAltitudeOption) applyBird(c config) config {
c.MaxAltitude = float64(o)
return c
}

func WithMaxAltitude(a float64) BirdOption { return maxAltitudeOption(a) }

func NewDog(name string, o ...DogOption) Dog {…}
func NewBird(name string, o ...BirdOption) Bird {…}
Expand Down
54 changes: 32 additions & 22 deletions exporters/jaeger/uploader.go
Expand Up @@ -55,15 +55,15 @@ func (fn endpointOptionFunc) newBatchUploader() (batchUploader, error) {
// will be used if neither are provided.
func WithAgentEndpoint(options ...AgentEndpointOption) EndpointOption {
return endpointOptionFunc(func() (batchUploader, error) {
cfg := &agentEndpointConfig{
cfg := agentEndpointConfig{
agentClientUDPParams{
AttemptReconnecting: true,
Host: envOr(envAgentHost, "localhost"),
Port: envOr(envAgentPort, "6831"),
},
}
for _, opt := range options {
opt.apply(cfg)
cfg = opt.apply(cfg)
}

client, err := newAgentClientUDP(cfg.agentClientUDPParams)
Expand All @@ -76,26 +76,27 @@ func WithAgentEndpoint(options ...AgentEndpointOption) EndpointOption {
}

type AgentEndpointOption interface {
apply(*agentEndpointConfig)
apply(agentEndpointConfig) agentEndpointConfig
}

type agentEndpointConfig struct {
agentClientUDPParams
}

type agentEndpointOptionFunc func(*agentEndpointConfig)
type agentEndpointOptionFunc func(agentEndpointConfig) agentEndpointConfig

func (fn agentEndpointOptionFunc) apply(cfg *agentEndpointConfig) {
fn(cfg)
func (fn agentEndpointOptionFunc) apply(cfg agentEndpointConfig) agentEndpointConfig {
return fn(cfg)
}

// WithAgentHost sets a host to be used in the agent client endpoint.
// This option overrides any value set for the
// OTEL_EXPORTER_JAEGER_AGENT_HOST environment variable.
// If this option is not passed and the env var is not set, "localhost" will be used by default.
func WithAgentHost(host string) AgentEndpointOption {
return agentEndpointOptionFunc(func(o *agentEndpointConfig) {
return agentEndpointOptionFunc(func(o agentEndpointConfig) agentEndpointConfig {
o.Host = host
return o
})
}

Expand All @@ -104,36 +105,41 @@ func WithAgentHost(host string) AgentEndpointOption {
// OTEL_EXPORTER_JAEGER_AGENT_PORT environment variable.
// If this option is not passed and the env var is not set, "6831" will be used by default.
func WithAgentPort(port string) AgentEndpointOption {
return agentEndpointOptionFunc(func(o *agentEndpointConfig) {
return agentEndpointOptionFunc(func(o agentEndpointConfig) agentEndpointConfig {
o.Port = port
return o
})
}

// WithLogger sets a logger to be used by agent client.
func WithLogger(logger *log.Logger) AgentEndpointOption {
return agentEndpointOptionFunc(func(o *agentEndpointConfig) {
return agentEndpointOptionFunc(func(o agentEndpointConfig) agentEndpointConfig {
o.Logger = logger
return o
})
}

// WithDisableAttemptReconnecting sets option to disable reconnecting udp client.
func WithDisableAttemptReconnecting() AgentEndpointOption {
return agentEndpointOptionFunc(func(o *agentEndpointConfig) {
return agentEndpointOptionFunc(func(o agentEndpointConfig) agentEndpointConfig {
o.AttemptReconnecting = false
return o
})
}

// WithAttemptReconnectingInterval sets the interval between attempts to re resolve agent endpoint.
func WithAttemptReconnectingInterval(interval time.Duration) AgentEndpointOption {
return agentEndpointOptionFunc(func(o *agentEndpointConfig) {
return agentEndpointOptionFunc(func(o agentEndpointConfig) agentEndpointConfig {
o.AttemptReconnectInterval = interval
return o
})
}

// WithMaxPacketSize sets the maximum UDP packet size for transport to the Jaeger agent.
func WithMaxPacketSize(size int) AgentEndpointOption {
return agentEndpointOptionFunc(func(o *agentEndpointConfig) {
return agentEndpointOptionFunc(func(o agentEndpointConfig) agentEndpointConfig {
o.MaxPacketSize = size
return o
})
}

Expand All @@ -149,15 +155,15 @@ func WithMaxPacketSize(size int) AgentEndpointOption {
// If neither values are provided for the username or the password, they will not be set since there is no default.
func WithCollectorEndpoint(options ...CollectorEndpointOption) EndpointOption {
return endpointOptionFunc(func() (batchUploader, error) {
cfg := &collectorEndpointConfig{
cfg := collectorEndpointConfig{
endpoint: envOr(envEndpoint, "http://localhost:14268/api/traces"),
username: envOr(envUser, ""),
password: envOr(envPassword, ""),
httpClient: http.DefaultClient,
}

for _, opt := range options {
opt.apply(cfg)
cfg = opt.apply(cfg)
}

return &collectorUploader{
Expand All @@ -170,7 +176,7 @@ func WithCollectorEndpoint(options ...CollectorEndpointOption) EndpointOption {
}

type CollectorEndpointOption interface {
apply(*collectorEndpointConfig)
apply(collectorEndpointConfig) collectorEndpointConfig
}

type collectorEndpointConfig struct {
Expand All @@ -187,10 +193,10 @@ type collectorEndpointConfig struct {
httpClient *http.Client
}

type collectorEndpointOptionFunc func(*collectorEndpointConfig)
type collectorEndpointOptionFunc func(collectorEndpointConfig) collectorEndpointConfig

func (fn collectorEndpointOptionFunc) apply(cfg *collectorEndpointConfig) {
fn(cfg)
func (fn collectorEndpointOptionFunc) apply(cfg collectorEndpointConfig) collectorEndpointConfig {
return fn(cfg)
}

// WithEndpoint is the URL for the Jaeger collector that spans are sent to.
Expand All @@ -199,8 +205,9 @@ func (fn collectorEndpointOptionFunc) apply(cfg *collectorEndpointConfig) {
// If this option is not passed and the environment variable is not set,
// "http://localhost:14268/api/traces" will be used by default.
func WithEndpoint(endpoint string) CollectorEndpointOption {
return collectorEndpointOptionFunc(func(o *collectorEndpointConfig) {
return collectorEndpointOptionFunc(func(o collectorEndpointConfig) collectorEndpointConfig {
o.endpoint = endpoint
return o
})
}

Expand All @@ -209,8 +216,9 @@ func WithEndpoint(endpoint string) CollectorEndpointOption {
// OTEL_EXPORTER_JAEGER_USER environment variable.
// If this option is not passed and the environment variable is not set, no username will be set.
func WithUsername(username string) CollectorEndpointOption {
return collectorEndpointOptionFunc(func(o *collectorEndpointConfig) {
return collectorEndpointOptionFunc(func(o collectorEndpointConfig) collectorEndpointConfig {
o.username = username
return o
})
}

Expand All @@ -219,15 +227,17 @@ func WithUsername(username string) CollectorEndpointOption {
// OTEL_EXPORTER_JAEGER_PASSWORD environment variable.
// If this option is not passed and the environment variable is not set, no password will be set.
func WithPassword(password string) CollectorEndpointOption {
return collectorEndpointOptionFunc(func(o *collectorEndpointConfig) {
return collectorEndpointOptionFunc(func(o collectorEndpointConfig) collectorEndpointConfig {
o.password = password
return o
})
}

// WithHTTPClient sets the http client to be used to make request to the collector endpoint.
func WithHTTPClient(client *http.Client) CollectorEndpointOption {
return collectorEndpointOptionFunc(func(o *collectorEndpointConfig) {
return collectorEndpointOptionFunc(func(o collectorEndpointConfig) collectorEndpointConfig {
o.httpClient = client
return o
})
}

Expand Down
2 changes: 1 addition & 1 deletion exporters/otlp/otlpmetric/exporter.go
Expand Up @@ -120,7 +120,7 @@ func NewUnstarted(client Client, opts ...Option) *Exporter {
}

for _, opt := range opts {
opt.apply(&cfg)
cfg = opt.apply(cfg)
}

e := &Exporter{
Expand Down

0 comments on commit bd817df

Please sign in to comment.