diff --git a/CHANGELOG.md b/CHANGELOG.md index fb91daf7486..89df8a8c5a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Change the `otlpmetric.Client` interface's `UploadMetrics` method to accept a single `ResourceMetrics` instead of a slice of them. (#2491) - Specify explicit buckets in Prometheus example. (#2493) - W3C baggage will now decode urlescaped values. (#2529) +- Baggage members are now only validated once, when calling `NewMember` and not also when adding it to the baggage itself. (#2522) ### Removed diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f25fd9008b4..7dead7084d3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -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 @@ -253,7 +253,7 @@ To set the value of the options a `config` contains, a corresponding ```go type Option interface { - apply(*config) + apply(config) config } ``` @@ -261,6 +261,9 @@ 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). @@ -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. @@ -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. @@ -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. @@ -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 }) } ``` @@ -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. @@ -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 {…} diff --git a/baggage/baggage.go b/baggage/baggage.go index d52a298ed1b..824c67b27a3 100644 --- a/baggage/baggage.go +++ b/baggage/baggage.go @@ -61,45 +61,57 @@ type Property struct { // hasValue indicates if a zero-value value means the property does not // have a value or if it was the zero-value. hasValue bool + + // hasData indicates whether the created property contains data or not. + // Properties that do not contain data are invalid with no other check + // required. + hasData bool } func NewKeyProperty(key string) (Property, error) { - p := Property{} if !keyRe.MatchString(key) { - return p, fmt.Errorf("%w: %q", errInvalidKey, key) + return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidKey, key) } - p.key = key + + p := Property{key: key, hasData: true} return p, nil } func NewKeyValueProperty(key, value string) (Property, error) { - p := Property{} if !keyRe.MatchString(key) { - return p, fmt.Errorf("%w: %q", errInvalidKey, key) + return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidKey, key) } if !valueRe.MatchString(value) { - return p, fmt.Errorf("%w: %q", errInvalidValue, value) + return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidValue, value) + } + + p := Property{ + key: key, + value: value, + hasValue: true, + hasData: true, } - p.key = key - p.value = value - p.hasValue = true return p, nil } +func newInvalidProperty() Property { + return Property{} +} + // parseProperty attempts to decode a Property from the passed string. It // returns an error if the input is invalid according to the W3C Baggage // specification. func parseProperty(property string) (Property, error) { - p := Property{} if property == "" { - return p, nil + return newInvalidProperty(), nil } match := propertyRe.FindStringSubmatch(property) if len(match) != 4 { - return p, fmt.Errorf("%w: %q", errInvalidProperty, property) + return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidProperty, property) } + p := Property{hasData: true} if match[1] != "" { p.key = match[1] } else { @@ -107,6 +119,7 @@ func parseProperty(property string) (Property, error) { p.value = match[3] p.hasValue = true } + return p, nil } @@ -117,6 +130,10 @@ func (p Property) validate() error { return fmt.Errorf("invalid property: %w", err) } + if !p.hasData { + return errFunc(fmt.Errorf("%w: %q", errInvalidProperty, p)) + } + if !keyRe.MatchString(p.key) { return errFunc(fmt.Errorf("%w: %q", errInvalidKey, p.key)) } @@ -220,26 +237,40 @@ func (p properties) String() string { type Member struct { key, value string properties properties + + // hasData indicates whether the created property contains data or not. + // Properties that do not contain data are invalid with no other check + // required. + hasData bool } // NewMember returns a new Member from the passed arguments. An error is // returned if the created Member would be invalid according to the W3C // Baggage specification. func NewMember(key, value string, props ...Property) (Member, error) { - m := Member{key: key, value: value, properties: properties(props).Copy()} + m := Member{ + key: key, + value: value, + properties: properties(props).Copy(), + hasData: true, + } if err := m.validate(); err != nil { - return Member{}, err + return newInvalidMember(), err } return m, nil } +func newInvalidMember() Member { + return Member{} +} + // parseMember attempts to decode a Member from the passed string. It returns // an error if the input is invalid according to the W3C Baggage // specification. func parseMember(member string) (Member, error) { if n := len(member); n > maxBytesPerMembers { - return Member{}, fmt.Errorf("%w: %d", errMemberBytes, n) + return newInvalidMember(), fmt.Errorf("%w: %d", errMemberBytes, n) } var ( @@ -254,7 +285,7 @@ func parseMember(member string) (Member, error) { for _, pStr := range strings.Split(parts[1], propertyDelimiter) { p, err := parseProperty(pStr) if err != nil { - return Member{}, err + return newInvalidMember(), err } props = append(props, p) } @@ -265,7 +296,7 @@ func parseMember(member string) (Member, error) { // Take into account a value can contain equal signs (=). kv := strings.SplitN(parts[0], keyValueDelimiter, 2) if len(kv) != 2 { - return Member{}, fmt.Errorf("%w: %q", errInvalidMember, member) + return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidMember, member) } // "Leading and trailing whitespaces are allowed but MUST be trimmed // when converting the header into a data structure." @@ -273,13 +304,13 @@ func parseMember(member string) (Member, error) { var err error value, err = url.QueryUnescape(strings.TrimSpace(kv[1])) if err != nil { - return Member{}, fmt.Errorf("%w: %q", err, value) + return newInvalidMember(), fmt.Errorf("%w: %q", err, value) } if !keyRe.MatchString(key) { - return Member{}, fmt.Errorf("%w: %q", errInvalidKey, key) + return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidKey, key) } if !valueRe.MatchString(value) { - return Member{}, fmt.Errorf("%w: %q", errInvalidValue, value) + return newInvalidMember(), fmt.Errorf("%w: %q", errInvalidValue, value) } default: // This should never happen unless a developer has changed the string @@ -288,12 +319,16 @@ func parseMember(member string) (Member, error) { panic("failed to parse baggage member") } - return Member{key: key, value: value, properties: props}, nil + return Member{key: key, value: value, properties: props, hasData: true}, nil } // validate ensures m conforms to the W3C Baggage specification, returning an // error otherwise. func (m Member) validate() error { + if !m.hasData { + return fmt.Errorf("%w: %q", errInvalidMember, m) + } + if !keyRe.MatchString(m.key) { return fmt.Errorf("%w: %q", errInvalidKey, m.key) } @@ -329,9 +364,10 @@ type Baggage struct { //nolint:golint list baggage.List } -// New returns a new valid Baggage. It returns an error if the passed members -// are invalid according to the W3C Baggage specification or if it results in -// a Baggage exceeding limits set in that specification. +// New returns a new valid Baggage. It returns an error if it results in a +// Baggage exceeding limits set in that specification. +// +// It expects all the provided members to have already been validated. func New(members ...Member) (Baggage, error) { if len(members) == 0 { return Baggage{}, nil @@ -339,9 +375,10 @@ func New(members ...Member) (Baggage, error) { b := make(baggage.List) for _, m := range members { - if err := m.validate(); err != nil { - return Baggage{}, err + if !m.hasData { + return Baggage{}, errInvalidMember } + // OpenTelemetry resolves duplicates by last-one-wins. b[m.key] = baggage.Item{ Value: m.value, @@ -406,6 +443,8 @@ func Parse(bStr string) (Baggage, error) { // // If there is no list-member matching the passed key the returned Member will // be a zero-value Member. +// The returned member is not validated, as we assume the validation happened +// when it was added to the Baggage. func (b Baggage) Member(key string) Member { v, ok := b.list[key] if !ok { @@ -413,7 +452,7 @@ func (b Baggage) Member(key string) Member { // where a zero-valued Member is included in the Baggage because a // zero-valued Member is invalid according to the W3C Baggage // specification (it has an empty key). - return Member{} + return newInvalidMember() } return Member{ @@ -425,6 +464,9 @@ func (b Baggage) Member(key string) Member { // Members returns all the baggage list-members. // The order of the returned list-members does not have significance. +// +// The returned members are not validated, as we assume the validation happened +// when they were added to the Baggage. func (b Baggage) Members() []Member { if len(b.list) == 0 { return nil @@ -448,8 +490,8 @@ func (b Baggage) Members() []Member { // If member is invalid according to the W3C Baggage specification, an error // is returned with the original Baggage. func (b Baggage) SetMember(member Member) (Baggage, error) { - if err := member.validate(); err != nil { - return b, fmt.Errorf("%w: %s", errInvalidMember, err) + if !member.hasData { + return b, errInvalidMember } n := len(b.list) diff --git a/baggage/baggage_test.go b/baggage/baggage_test.go index 88c820319b5..8cc4832ad00 100644 --- a/baggage/baggage_test.go +++ b/baggage/baggage_test.go @@ -138,7 +138,7 @@ func TestNewKeyProperty(t *testing.T) { p, err = NewKeyProperty("key") assert.NoError(t, err) - assert.Equal(t, Property{key: "key"}, p) + assert.Equal(t, Property{key: "key", hasData: true}, p) } func TestNewKeyValueProperty(t *testing.T) { @@ -152,11 +152,14 @@ func TestNewKeyValueProperty(t *testing.T) { p, err = NewKeyValueProperty("key", "value") assert.NoError(t, err) - assert.Equal(t, Property{key: "key", value: "value", hasValue: true}, p) + assert.Equal(t, Property{key: "key", value: "value", hasValue: true, hasData: true}, p) } func TestPropertyValidate(t *testing.T) { p := Property{} + assert.ErrorIs(t, p.validate(), errInvalidProperty) + + p.hasData = true assert.ErrorIs(t, p.validate(), errInvalidKey) p.key = "k" @@ -179,7 +182,7 @@ func TestNewEmptyBaggage(t *testing.T) { } func TestNewBaggage(t *testing.T) { - b, err := New(Member{key: "k"}) + b, err := New(Member{key: "k", hasData: true}) assert.NoError(t, err) assert.Equal(t, Baggage{list: baggage.List{"k": {}}}, b) } @@ -192,8 +195,9 @@ func TestNewBaggageWithDuplicates(t *testing.T) { for i := range m { // Duplicates are collapsed. m[i] = Member{ - key: "a", - value: fmt.Sprintf("%d", i), + key: "a", + value: fmt.Sprintf("%d", i), + hasData: true, } } b, err := New(m...) @@ -205,9 +209,9 @@ func TestNewBaggageWithDuplicates(t *testing.T) { assert.Equal(t, want, b) } -func TestNewBaggageErrorInvalidMember(t *testing.T) { - _, err := New(Member{key: ""}) - assert.ErrorIs(t, err, errInvalidKey) +func TestNewBaggageErrorEmptyMember(t *testing.T) { + _, err := New(Member{}) + assert.ErrorIs(t, err, errInvalidMember) } func key(n int) string { @@ -223,7 +227,7 @@ func key(n int) string { func TestNewBaggageErrorTooManyBytes(t *testing.T) { m := make([]Member, (maxBytesPerBaggageString/maxBytesPerMembers)+1) for i := range m { - m[i] = Member{key: key(maxBytesPerMembers)} + m[i] = Member{key: key(maxBytesPerMembers), hasData: true} } _, err := New(m...) assert.ErrorIs(t, err, errBaggageBytes) @@ -232,7 +236,7 @@ func TestNewBaggageErrorTooManyBytes(t *testing.T) { func TestNewBaggageErrorTooManyMembers(t *testing.T) { m := make([]Member, maxMembers+1) for i := range m { - m[i] = Member{key: fmt.Sprintf("%d", i)} + m[i] = Member{key: fmt.Sprintf("%d", i), hasData: true} } _, err := New(m...) assert.ErrorIs(t, err, errMemberNumber) @@ -533,7 +537,7 @@ func TestBaggageDeleteMember(t *testing.T) { assert.NotContains(t, b1.list, key) } -func TestBaggageSetMemberError(t *testing.T) { +func TestBaggageSetMemberEmpty(t *testing.T) { _, err := Baggage{}.SetMember(Member{}) assert.ErrorIs(t, err, errInvalidMember) } @@ -542,7 +546,7 @@ func TestBaggageSetMember(t *testing.T) { b0 := Baggage{} key := "k" - m := Member{key: key} + m := Member{key: key, hasData: true} b1, err := b0.SetMember(m) assert.NoError(t, err) assert.NotContains(t, b0.list, key) @@ -558,7 +562,7 @@ func TestBaggageSetMember(t *testing.T) { assert.Equal(t, 1, len(b1.list)) assert.Equal(t, 1, len(b2.list)) - p := properties{{key: "p"}} + p := properties{{key: "p", hasData: true}} m.properties = p b3, err := b2.SetMember(m) assert.NoError(t, err) @@ -569,12 +573,12 @@ func TestBaggageSetMember(t *testing.T) { // The returned baggage needs to be immutable and should use a copy of the // properties slice. - p[0] = Property{key: "different"} + p[0] = Property{key: "different", hasData: true} assert.Equal(t, baggage.Item{Value: "v", Properties: []baggage.Property{{Key: "p"}}}, b3.list[key]) // Reset for below. - p[0] = Property{key: "p"} + p[0] = Property{key: "p", hasData: true} - m = Member{key: "another"} + m = Member{key: "another", hasData: true} b4, err := b3.SetMember(m) assert.NoError(t, err) assert.Equal(t, baggage.Item{Value: "v", Properties: []baggage.Property{{Key: "p"}}}, b3.list[key]) @@ -664,7 +668,10 @@ func TestMemberProperties(t *testing.T) { } func TestMemberValidation(t *testing.T) { - m := Member{} + m := Member{hasData: false} + assert.ErrorIs(t, m.validate(), errInvalidMember) + + m.hasData = true assert.ErrorIs(t, m.validate(), errInvalidKey) m.key, m.value = "k", "\\" @@ -677,13 +684,18 @@ func TestMemberValidation(t *testing.T) { func TestNewMember(t *testing.T) { m, err := NewMember("", "") assert.ErrorIs(t, err, errInvalidKey) - assert.Equal(t, Member{}, m) + assert.Equal(t, Member{hasData: false}, m) key, val := "k", "v" - p := Property{key: "foo"} + p := Property{key: "foo", hasData: true} m, err = NewMember(key, val, p) assert.NoError(t, err) - expected := Member{key: key, value: val, properties: properties{{key: "foo"}}} + expected := Member{ + key: key, + value: val, + properties: properties{{key: "foo", hasData: true}}, + hasData: true, + } assert.Equal(t, expected, m) // Ensure new member is immutable. @@ -692,12 +704,46 @@ func TestNewMember(t *testing.T) { } func TestPropertiesValidate(t *testing.T) { - p := properties{{}} + p := properties{{hasData: true}} assert.ErrorIs(t, p.validate(), errInvalidKey) p[0].key = "foo" assert.NoError(t, p.validate()) - p = append(p, Property{key: "bar"}) + p = append(p, Property{key: "bar", hasData: true}) assert.NoError(t, p.validate()) } + +var benchBaggage Baggage + +func BenchmarkNew(b *testing.B) { + mem1, _ := NewMember("key1", "val1") + mem2, _ := NewMember("key2", "val2") + mem3, _ := NewMember("key3", "val3") + mem4, _ := NewMember("key4", "val4") + + b.ReportAllocs() + b.ResetTimer() + + for i := 0; i < b.N; i++ { + benchBaggage, _ = New(mem1, mem2, mem3, mem4) + } +} + +var benchMember Member + +func BenchmarkNewMember(b *testing.B) { + b.ReportAllocs() + + for i := 0; i < b.N; i++ { + benchMember, _ = NewMember("key", "value") + } +} + +func BenchmarkParse(b *testing.B) { + b.ReportAllocs() + + for i := 0; i < b.N; i++ { + benchBaggage, _ = Parse(`userId=alice,serverNode = DF28 , isProduction = false,hasProp=stuff;propKey;propWValue=value`) + } +} diff --git a/example/otel-collector/go.mod b/example/otel-collector/go.mod index 7391dc1dd97..acbc10cd1bb 100644 --- a/example/otel-collector/go.mod +++ b/example/otel-collector/go.mod @@ -12,7 +12,7 @@ require ( go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.3.0 go.opentelemetry.io/otel/sdk v1.3.0 go.opentelemetry.io/otel/trace v1.3.0 - google.golang.org/grpc v1.43.0 + google.golang.org/grpc v1.44.0 ) replace go.opentelemetry.io/otel/bridge/opencensus => ../../bridge/opencensus diff --git a/example/otel-collector/go.sum b/example/otel-collector/go.sum index 1cc6cdbcf0e..ca5dbebdb8b 100644 --- a/example/otel-collector/go.sum +++ b/example/otel-collector/go.sum @@ -134,8 +134,9 @@ google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQ google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk= google.golang.org/grpc v1.33.1/go.mod h1:fr5YgcSWrqhRRxogOsw7RzIpsmvOZ6IcH4kBYTpR3n0= google.golang.org/grpc v1.36.0/go.mod h1:qjiiYl8FncCW8feJPdyg3v6XW24KsRHe+dy9BAGRRjU= -google.golang.org/grpc v1.43.0 h1:Eeu7bZtDZ2DpRCsLhUlcrLnvYaMK1Gz86a+hMVvELmM= google.golang.org/grpc v1.43.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU= +google.golang.org/grpc v1.44.0 h1:weqSxi/TMs1SqFRMHCtBgXRs8k3X39QIDEZ0pRcttUg= +google.golang.org/grpc v1.44.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM= diff --git a/exporters/jaeger/uploader.go b/exporters/jaeger/uploader.go index 84a517985a7..d907f74b9f3 100644 --- a/exporters/jaeger/uploader.go +++ b/exporters/jaeger/uploader.go @@ -55,7 +55,7 @@ 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"), @@ -63,7 +63,7 @@ func WithAgentEndpoint(options ...AgentEndpointOption) EndpointOption { }, } for _, opt := range options { - opt.apply(cfg) + cfg = opt.apply(cfg) } client, err := newAgentClientUDP(cfg.agentClientUDPParams) @@ -76,17 +76,17 @@ 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. @@ -94,8 +94,9 @@ func (fn agentEndpointOptionFunc) apply(cfg *agentEndpointConfig) { // 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 }) } @@ -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 }) } @@ -149,7 +155,7 @@ 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, ""), @@ -157,7 +163,7 @@ func WithCollectorEndpoint(options ...CollectorEndpointOption) EndpointOption { } for _, opt := range options { - opt.apply(cfg) + cfg = opt.apply(cfg) } return &collectorUploader{ @@ -170,7 +176,7 @@ func WithCollectorEndpoint(options ...CollectorEndpointOption) EndpointOption { } type CollectorEndpointOption interface { - apply(*collectorEndpointConfig) + apply(collectorEndpointConfig) collectorEndpointConfig } type collectorEndpointConfig struct { @@ -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. @@ -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 }) } @@ -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 }) } @@ -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 }) } diff --git a/exporters/otlp/otlpmetric/exporter.go b/exporters/otlp/otlpmetric/exporter.go index 2cd90ac435a..36c41cce762 100644 --- a/exporters/otlp/otlpmetric/exporter.go +++ b/exporters/otlp/otlpmetric/exporter.go @@ -120,7 +120,7 @@ func NewUnstarted(client Client, opts ...Option) *Exporter { } for _, opt := range opts { - opt.apply(&cfg) + cfg = opt.apply(cfg) } e := &Exporter{ diff --git a/exporters/otlp/otlpmetric/go.mod b/exporters/otlp/otlpmetric/go.mod index 8394ba3362b..ba19e06432d 100644 --- a/exporters/otlp/otlpmetric/go.mod +++ b/exporters/otlp/otlpmetric/go.mod @@ -11,7 +11,7 @@ require ( go.opentelemetry.io/otel/sdk v1.3.0 go.opentelemetry.io/otel/sdk/metric v0.26.0 go.opentelemetry.io/proto/otlp v0.12.0 - google.golang.org/grpc v1.43.0 + google.golang.org/grpc v1.44.0 google.golang.org/protobuf v1.27.1 ) diff --git a/exporters/otlp/otlpmetric/go.sum b/exporters/otlp/otlpmetric/go.sum index 983af28691b..c965d892816 100644 --- a/exporters/otlp/otlpmetric/go.sum +++ b/exporters/otlp/otlpmetric/go.sum @@ -114,8 +114,9 @@ google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQ google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk= google.golang.org/grpc v1.33.1/go.mod h1:fr5YgcSWrqhRRxogOsw7RzIpsmvOZ6IcH4kBYTpR3n0= google.golang.org/grpc v1.36.0/go.mod h1:qjiiYl8FncCW8feJPdyg3v6XW24KsRHe+dy9BAGRRjU= -google.golang.org/grpc v1.43.0 h1:Eeu7bZtDZ2DpRCsLhUlcrLnvYaMK1Gz86a+hMVvELmM= google.golang.org/grpc v1.43.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU= +google.golang.org/grpc v1.44.0 h1:weqSxi/TMs1SqFRMHCtBgXRs8k3X39QIDEZ0pRcttUg= +google.golang.org/grpc v1.44.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM= diff --git a/exporters/otlp/otlpmetric/internal/otlpconfig/envconfig.go b/exporters/otlp/otlpmetric/internal/otlpconfig/envconfig.go index a62b548496f..ecf2ecaf9c2 100644 --- a/exporters/otlp/otlpmetric/internal/otlpconfig/envconfig.go +++ b/exporters/otlp/otlpmetric/internal/otlpconfig/envconfig.go @@ -33,12 +33,12 @@ var DefaultEnvOptionsReader = EnvOptionsReader{ ReadFile: ioutil.ReadFile, } -func ApplyGRPCEnvConfigs(cfg *Config) { - DefaultEnvOptionsReader.ApplyGRPCEnvConfigs(cfg) +func ApplyGRPCEnvConfigs(cfg Config) Config { + return DefaultEnvOptionsReader.ApplyGRPCEnvConfigs(cfg) } -func ApplyHTTPEnvConfigs(cfg *Config) { - DefaultEnvOptionsReader.ApplyHTTPEnvConfigs(cfg) +func ApplyHTTPEnvConfigs(cfg Config) Config { + return DefaultEnvOptionsReader.ApplyHTTPEnvConfigs(cfg) } type EnvOptionsReader struct { @@ -46,18 +46,20 @@ type EnvOptionsReader struct { ReadFile func(filename string) ([]byte, error) } -func (e *EnvOptionsReader) ApplyHTTPEnvConfigs(cfg *Config) { +func (e *EnvOptionsReader) ApplyHTTPEnvConfigs(cfg Config) Config { opts := e.GetOptionsFromEnv() for _, opt := range opts { - opt.ApplyHTTPOption(cfg) + cfg = opt.ApplyHTTPOption(cfg) } + return cfg } -func (e *EnvOptionsReader) ApplyGRPCEnvConfigs(cfg *Config) { +func (e *EnvOptionsReader) ApplyGRPCEnvConfigs(cfg Config) Config { opts := e.GetOptionsFromEnv() for _, opt := range opts { - opt.ApplyGRPCOption(cfg) + cfg = opt.ApplyGRPCOption(cfg) } + return cfg } func (e *EnvOptionsReader) GetOptionsFromEnv() []GenericOption { @@ -74,7 +76,7 @@ func (e *EnvOptionsReader) GetOptionsFromEnv() []GenericOption { } else { opts = append(opts, WithSecure()) } - opts = append(opts, newSplitOption(func(cfg *Config) { + opts = append(opts, newSplitOption(func(cfg Config) Config { cfg.Metrics.Endpoint = u.Host // For endpoint URLs for OTLP/HTTP per-signal variables, the // URL MUST be used as-is without any modification. The only @@ -85,10 +87,12 @@ func (e *EnvOptionsReader) GetOptionsFromEnv() []GenericOption { path = "/" } cfg.Metrics.URLPath = path - }, func(cfg *Config) { + return cfg + }, func(cfg Config) Config { // For OTLP/gRPC endpoints, this is the target to which the // exporter is going to send telemetry. cfg.Metrics.Endpoint = path.Join(u.Host, u.Path) + return cfg })) } } else if v, ok = e.getEnvValue("ENDPOINT"); ok { @@ -101,16 +105,18 @@ func (e *EnvOptionsReader) GetOptionsFromEnv() []GenericOption { } else { opts = append(opts, WithSecure()) } - opts = append(opts, newSplitOption(func(cfg *Config) { + opts = append(opts, newSplitOption(func(cfg Config) Config { cfg.Metrics.Endpoint = u.Host // For OTLP/HTTP endpoint URLs without a per-signal // configuration, the passed endpoint is used as a base URL // and the signals are sent to these paths relative to that. cfg.Metrics.URLPath = path.Join(u.Path, DefaultMetricsPath) - }, func(cfg *Config) { + return cfg + }, func(cfg Config) Config { // For OTLP/gRPC endpoints, this is the target to which the // exporter is going to send telemetry. cfg.Metrics.Endpoint = path.Join(u.Host, u.Path) + return cfg })) } } diff --git a/exporters/otlp/otlpmetric/internal/otlpconfig/options.go b/exporters/otlp/otlpmetric/internal/otlpconfig/options.go index 8cfbc7dfcf8..fed8a82f1c7 100644 --- a/exporters/otlp/otlpmetric/internal/otlpconfig/options.go +++ b/exporters/otlp/otlpmetric/internal/otlpconfig/options.go @@ -90,9 +90,9 @@ func NewDefaultConfig() Config { // any unset setting using the default gRPC config values. func NewGRPCConfig(opts ...GRPCOption) Config { cfg := NewDefaultConfig() - ApplyGRPCEnvConfigs(&cfg) + cfg = ApplyGRPCEnvConfigs(cfg) for _, opt := range opts { - opt.ApplyGRPCOption(&cfg) + cfg = opt.ApplyGRPCOption(cfg) } if cfg.ServiceConfig != "" { @@ -129,8 +129,8 @@ func NewGRPCConfig(opts ...GRPCOption) Config { type ( // GenericOption applies an option to the HTTP or gRPC driver. GenericOption interface { - ApplyHTTPOption(*Config) - ApplyGRPCOption(*Config) + ApplyHTTPOption(Config) Config + ApplyGRPCOption(Config) Config // A private method to prevent users implementing the // interface and so future additions to it will not @@ -140,7 +140,7 @@ type ( // HTTPOption applies an option to the HTTP driver. HTTPOption interface { - ApplyHTTPOption(*Config) + ApplyHTTPOption(Config) Config // A private method to prevent users implementing the // interface and so future additions to it will not @@ -150,7 +150,7 @@ type ( // GRPCOption applies an option to the gRPC driver. GRPCOption interface { - ApplyGRPCOption(*Config) + ApplyGRPCOption(Config) Config // A private method to prevent users implementing the // interface and so future additions to it will not @@ -162,128 +162,138 @@ type ( // genericOption is an option that applies the same logic // for both gRPC and HTTP. type genericOption struct { - fn func(*Config) + fn func(Config) Config } -func (g *genericOption) ApplyGRPCOption(cfg *Config) { - g.fn(cfg) +func (g *genericOption) ApplyGRPCOption(cfg Config) Config { + return g.fn(cfg) } -func (g *genericOption) ApplyHTTPOption(cfg *Config) { - g.fn(cfg) +func (g *genericOption) ApplyHTTPOption(cfg Config) Config { + return g.fn(cfg) } func (genericOption) private() {} -func newGenericOption(fn func(cfg *Config)) GenericOption { +func newGenericOption(fn func(cfg Config) Config) GenericOption { return &genericOption{fn: fn} } // splitOption is an option that applies different logics // for gRPC and HTTP. type splitOption struct { - httpFn func(*Config) - grpcFn func(*Config) + httpFn func(Config) Config + grpcFn func(Config) Config } -func (g *splitOption) ApplyGRPCOption(cfg *Config) { - g.grpcFn(cfg) +func (g *splitOption) ApplyGRPCOption(cfg Config) Config { + return g.grpcFn(cfg) } -func (g *splitOption) ApplyHTTPOption(cfg *Config) { - g.httpFn(cfg) +func (g *splitOption) ApplyHTTPOption(cfg Config) Config { + return g.httpFn(cfg) } func (splitOption) private() {} -func newSplitOption(httpFn func(cfg *Config), grpcFn func(cfg *Config)) GenericOption { +func newSplitOption(httpFn func(cfg Config) Config, grpcFn func(cfg Config) Config) GenericOption { return &splitOption{httpFn: httpFn, grpcFn: grpcFn} } // httpOption is an option that is only applied to the HTTP driver. type httpOption struct { - fn func(*Config) + fn func(Config) Config } -func (h *httpOption) ApplyHTTPOption(cfg *Config) { - h.fn(cfg) +func (h *httpOption) ApplyHTTPOption(cfg Config) Config { + return h.fn(cfg) } func (httpOption) private() {} -func NewHTTPOption(fn func(cfg *Config)) HTTPOption { +func NewHTTPOption(fn func(cfg Config) Config) HTTPOption { return &httpOption{fn: fn} } // grpcOption is an option that is only applied to the gRPC driver. type grpcOption struct { - fn func(*Config) + fn func(Config) Config } -func (h *grpcOption) ApplyGRPCOption(cfg *Config) { - h.fn(cfg) +func (h *grpcOption) ApplyGRPCOption(cfg Config) Config { + return h.fn(cfg) } func (grpcOption) private() {} -func NewGRPCOption(fn func(cfg *Config)) GRPCOption { +func NewGRPCOption(fn func(cfg Config) Config) GRPCOption { return &grpcOption{fn: fn} } // Generic Options func WithEndpoint(endpoint string) GenericOption { - return newGenericOption(func(cfg *Config) { + return newGenericOption(func(cfg Config) Config { cfg.Metrics.Endpoint = endpoint + return cfg }) } func WithCompression(compression Compression) GenericOption { - return newGenericOption(func(cfg *Config) { + return newGenericOption(func(cfg Config) Config { cfg.Metrics.Compression = compression + return cfg }) } func WithURLPath(urlPath string) GenericOption { - return newGenericOption(func(cfg *Config) { + return newGenericOption(func(cfg Config) Config { cfg.Metrics.URLPath = urlPath + return cfg }) } func WithRetry(rc retry.Config) GenericOption { - return newGenericOption(func(cfg *Config) { + return newGenericOption(func(cfg Config) Config { cfg.RetryConfig = rc + return cfg }) } func WithTLSClientConfig(tlsCfg *tls.Config) GenericOption { - return newSplitOption(func(cfg *Config) { + return newSplitOption(func(cfg Config) Config { cfg.Metrics.TLSCfg = tlsCfg.Clone() - }, func(cfg *Config) { + return cfg + }, func(cfg Config) Config { cfg.Metrics.GRPCCredentials = credentials.NewTLS(tlsCfg) + return cfg }) } func WithInsecure() GenericOption { - return newGenericOption(func(cfg *Config) { + return newGenericOption(func(cfg Config) Config { cfg.Metrics.Insecure = true + return cfg }) } func WithSecure() GenericOption { - return newGenericOption(func(cfg *Config) { + return newGenericOption(func(cfg Config) Config { cfg.Metrics.Insecure = false + return cfg }) } func WithHeaders(headers map[string]string) GenericOption { - return newGenericOption(func(cfg *Config) { + return newGenericOption(func(cfg Config) Config { cfg.Metrics.Headers = headers + return cfg }) } func WithTimeout(duration time.Duration) GenericOption { - return newGenericOption(func(cfg *Config) { + return newGenericOption(func(cfg Config) Config { cfg.Metrics.Timeout = duration + return cfg }) } diff --git a/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go b/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go index ee267e4fd00..d82802603e3 100644 --- a/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go +++ b/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go @@ -387,9 +387,9 @@ func TestConfigs(t *testing.T) { // Tests Generic options as HTTP Options cfg := otlpconfig.NewDefaultConfig() - otlpconfig.ApplyHTTPEnvConfigs(&cfg) + cfg = otlpconfig.ApplyHTTPEnvConfigs(cfg) for _, opt := range tt.opts { - opt.ApplyHTTPOption(&cfg) + cfg = opt.ApplyHTTPOption(cfg) } tt.asserts(t, &cfg, false) diff --git a/exporters/otlp/otlpmetric/options.go b/exporters/otlp/otlpmetric/options.go index ce8a5f58c5a..bd8706a74d3 100644 --- a/exporters/otlp/otlpmetric/options.go +++ b/exporters/otlp/otlpmetric/options.go @@ -18,13 +18,13 @@ import "go.opentelemetry.io/otel/sdk/metric/export/aggregation" // Option are setting options passed to an Exporter on creation. type Option interface { - apply(*config) + apply(config) config } -type exporterOptionFunc func(*config) +type exporterOptionFunc func(config) config -func (fn exporterOptionFunc) apply(cfg *config) { - fn(cfg) +func (fn exporterOptionFunc) apply(cfg config) config { + return fn(cfg) } type config struct { @@ -36,7 +36,8 @@ type config struct { // aggregation). If not specified otherwise, exporter will use a // cumulative temporality selector. func WithMetricAggregationTemporalitySelector(selector aggregation.TemporalitySelector) Option { - return exporterOptionFunc(func(cfg *config) { + return exporterOptionFunc(func(cfg config) config { cfg.temporalitySelector = selector + return cfg }) } diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/go.mod b/exporters/otlp/otlpmetric/otlpmetricgrpc/go.mod index 8ca2da5ccf3..11fe57e8a7c 100644 --- a/exporters/otlp/otlpmetric/otlpmetricgrpc/go.mod +++ b/exporters/otlp/otlpmetric/otlpmetricgrpc/go.mod @@ -12,7 +12,7 @@ require ( go.opentelemetry.io/otel/sdk/metric v0.26.0 go.opentelemetry.io/proto/otlp v0.12.0 google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013 - google.golang.org/grpc v1.43.0 + google.golang.org/grpc v1.44.0 google.golang.org/protobuf v1.27.1 ) diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/go.sum b/exporters/otlp/otlpmetric/otlpmetricgrpc/go.sum index 983af28691b..c965d892816 100644 --- a/exporters/otlp/otlpmetric/otlpmetricgrpc/go.sum +++ b/exporters/otlp/otlpmetric/otlpmetricgrpc/go.sum @@ -114,8 +114,9 @@ google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQ google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk= google.golang.org/grpc v1.33.1/go.mod h1:fr5YgcSWrqhRRxogOsw7RzIpsmvOZ6IcH4kBYTpR3n0= google.golang.org/grpc v1.36.0/go.mod h1:qjiiYl8FncCW8feJPdyg3v6XW24KsRHe+dy9BAGRRjU= -google.golang.org/grpc v1.43.0 h1:Eeu7bZtDZ2DpRCsLhUlcrLnvYaMK1Gz86a+hMVvELmM= google.golang.org/grpc v1.43.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU= +google.golang.org/grpc v1.44.0 h1:weqSxi/TMs1SqFRMHCtBgXRs8k3X39QIDEZ0pRcttUg= +google.golang.org/grpc v1.44.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM= diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/options.go b/exporters/otlp/otlpmetric/otlpmetricgrpc/options.go index e6d3919eab1..27769ff6b7c 100644 --- a/exporters/otlp/otlpmetric/otlpmetricgrpc/options.go +++ b/exporters/otlp/otlpmetric/otlpmetricgrpc/options.go @@ -28,7 +28,7 @@ import ( // Option applies an option to the gRPC driver. type Option interface { - applyGRPCOption(*otlpconfig.Config) + applyGRPCOption(otlpconfig.Config) otlpconfig.Config } func asGRPCOptions(opts []Option) []otlpconfig.GRPCOption { @@ -50,8 +50,8 @@ type wrappedOption struct { otlpconfig.GRPCOption } -func (w wrappedOption) applyGRPCOption(cfg *otlpconfig.Config) { - w.ApplyGRPCOption(cfg) +func (w wrappedOption) applyGRPCOption(cfg otlpconfig.Config) otlpconfig.Config { + return w.ApplyGRPCOption(cfg) } // WithInsecure disables client transport security for the exporter's gRPC @@ -77,8 +77,9 @@ func WithEndpoint(endpoint string) Option { // // This option has no effect if WithGRPCConn is used. func WithReconnectionPeriod(rp time.Duration) Option { - return wrappedOption{otlpconfig.NewGRPCOption(func(cfg *otlpconfig.Config) { + return wrappedOption{otlpconfig.NewGRPCOption(func(cfg otlpconfig.Config) otlpconfig.Config { cfg.ReconnectionPeriod = rp + return cfg })} } @@ -117,8 +118,9 @@ func WithHeaders(headers map[string]string) Option { // // This option has no effect if WithGRPCConn is used. func WithTLSCredentials(creds credentials.TransportCredentials) Option { - return wrappedOption{otlpconfig.NewGRPCOption(func(cfg *otlpconfig.Config) { + return wrappedOption{otlpconfig.NewGRPCOption(func(cfg otlpconfig.Config) otlpconfig.Config { cfg.Metrics.GRPCCredentials = creds + return cfg })} } @@ -126,8 +128,9 @@ func WithTLSCredentials(creds credentials.TransportCredentials) Option { // // This option has no effect if WithGRPCConn is used. func WithServiceConfig(serviceConfig string) Option { - return wrappedOption{otlpconfig.NewGRPCOption(func(cfg *otlpconfig.Config) { + return wrappedOption{otlpconfig.NewGRPCOption(func(cfg otlpconfig.Config) otlpconfig.Config { cfg.ServiceConfig = serviceConfig + return cfg })} } @@ -138,8 +141,9 @@ func WithServiceConfig(serviceConfig string) Option { // // This option has no effect if WithGRPCConn is used. func WithDialOption(opts ...grpc.DialOption) Option { - return wrappedOption{otlpconfig.NewGRPCOption(func(cfg *otlpconfig.Config) { + return wrappedOption{otlpconfig.NewGRPCOption(func(cfg otlpconfig.Config) otlpconfig.Config { cfg.DialOptions = opts + return cfg })} } @@ -152,8 +156,9 @@ func WithDialOption(opts ...grpc.DialOption) Option { // It is the callers responsibility to close the passed conn. The client // Shutdown method will not close this connection. func WithGRPCConn(conn *grpc.ClientConn) Option { - return wrappedOption{otlpconfig.NewGRPCOption(func(cfg *otlpconfig.Config) { + return wrappedOption{otlpconfig.NewGRPCOption(func(cfg otlpconfig.Config) otlpconfig.Config { cfg.GRPCConn = conn + return cfg })} } diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go index b472611fb40..aa381f8e038 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go @@ -78,9 +78,9 @@ type client struct { // NewClient creates a new HTTP metric client. func NewClient(opts ...Option) otlpmetric.Client { cfg := otlpconfig.NewDefaultConfig() - otlpconfig.ApplyHTTPEnvConfigs(&cfg) + cfg = otlpconfig.ApplyHTTPEnvConfigs(cfg) for _, opt := range opts { - opt.applyHTTPOption(&cfg) + cfg = opt.applyHTTPOption(cfg) } for pathPtr, defaultPath := range map[*string]string{ diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/go.sum b/exporters/otlp/otlpmetric/otlpmetrichttp/go.sum index 983af28691b..c965d892816 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/go.sum +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/go.sum @@ -114,8 +114,9 @@ google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQ google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk= google.golang.org/grpc v1.33.1/go.mod h1:fr5YgcSWrqhRRxogOsw7RzIpsmvOZ6IcH4kBYTpR3n0= google.golang.org/grpc v1.36.0/go.mod h1:qjiiYl8FncCW8feJPdyg3v6XW24KsRHe+dy9BAGRRjU= -google.golang.org/grpc v1.43.0 h1:Eeu7bZtDZ2DpRCsLhUlcrLnvYaMK1Gz86a+hMVvELmM= google.golang.org/grpc v1.43.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU= +google.golang.org/grpc v1.44.0 h1:weqSxi/TMs1SqFRMHCtBgXRs8k3X39QIDEZ0pRcttUg= +google.golang.org/grpc v1.44.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM= diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/options.go b/exporters/otlp/otlpmetric/otlpmetrichttp/options.go index 61dbccc3078..bec26204c74 100644 --- a/exporters/otlp/otlpmetric/otlpmetrichttp/options.go +++ b/exporters/otlp/otlpmetric/otlpmetrichttp/options.go @@ -37,7 +37,7 @@ const ( // Option applies an option to the HTTP client. type Option interface { - applyHTTPOption(*otlpconfig.Config) + applyHTTPOption(otlpconfig.Config) otlpconfig.Config } // RetryConfig defines configuration for retrying batches in case of export @@ -48,8 +48,8 @@ type wrappedOption struct { otlpconfig.HTTPOption } -func (w wrappedOption) applyHTTPOption(cfg *otlpconfig.Config) { - w.ApplyHTTPOption(cfg) +func (w wrappedOption) applyHTTPOption(cfg otlpconfig.Config) otlpconfig.Config { + return w.ApplyHTTPOption(cfg) } // WithEndpoint allows one to set the address of the collector @@ -83,7 +83,7 @@ func WithMaxAttempts(maxAttempts int) Option { maxAttempts = 5 } return wrappedOption{ - otlpconfig.NewHTTPOption(func(cfg *otlpconfig.Config) { + otlpconfig.NewHTTPOption(func(cfg otlpconfig.Config) otlpconfig.Config { cfg.RetryConfig.Enabled = true var ( @@ -104,7 +104,7 @@ func WithMaxAttempts(maxAttempts int) Option { attempts := int64(maxE+init) / int64(maxI) if int64(maxAttempts) == attempts { - return + return cfg } maxE = time.Duration(int64(maxAttempts)*int64(maxI)) - init @@ -112,6 +112,8 @@ func WithMaxAttempts(maxAttempts int) Option { cfg.RetryConfig.InitialInterval = init cfg.RetryConfig.MaxInterval = maxI cfg.RetryConfig.MaxElapsedTime = maxE + + return cfg }), } } @@ -126,7 +128,7 @@ func WithBackoff(duration time.Duration) Option { duration = 300 * time.Millisecond } return wrappedOption{ - otlpconfig.NewHTTPOption(func(cfg *otlpconfig.Config) { + otlpconfig.NewHTTPOption(func(cfg otlpconfig.Config) otlpconfig.Config { cfg.RetryConfig.Enabled = true cfg.RetryConfig.MaxInterval = duration if cfg.RetryConfig.InitialInterval == 0 { @@ -135,6 +137,7 @@ func WithBackoff(duration time.Duration) Option { if cfg.RetryConfig.MaxElapsedTime == 0 { cfg.RetryConfig.MaxElapsedTime = retry.DefaultConfig.MaxElapsedTime } + return cfg }), } } diff --git a/exporters/otlp/otlptrace/go.mod b/exporters/otlp/otlptrace/go.mod index 91ebb6e0ce7..ddc8f8a5ed8 100644 --- a/exporters/otlp/otlptrace/go.mod +++ b/exporters/otlp/otlptrace/go.mod @@ -10,7 +10,7 @@ require ( go.opentelemetry.io/otel/sdk v1.3.0 go.opentelemetry.io/otel/trace v1.3.0 go.opentelemetry.io/proto/otlp v0.12.0 - google.golang.org/grpc v1.43.0 + google.golang.org/grpc v1.44.0 google.golang.org/protobuf v1.27.1 ) diff --git a/exporters/otlp/otlptrace/go.sum b/exporters/otlp/otlptrace/go.sum index 8bfa2adcab2..b41a0280fd8 100644 --- a/exporters/otlp/otlptrace/go.sum +++ b/exporters/otlp/otlptrace/go.sum @@ -112,8 +112,9 @@ google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQ google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk= google.golang.org/grpc v1.33.1/go.mod h1:fr5YgcSWrqhRRxogOsw7RzIpsmvOZ6IcH4kBYTpR3n0= google.golang.org/grpc v1.36.0/go.mod h1:qjiiYl8FncCW8feJPdyg3v6XW24KsRHe+dy9BAGRRjU= -google.golang.org/grpc v1.43.0 h1:Eeu7bZtDZ2DpRCsLhUlcrLnvYaMK1Gz86a+hMVvELmM= google.golang.org/grpc v1.43.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU= +google.golang.org/grpc v1.44.0 h1:weqSxi/TMs1SqFRMHCtBgXRs8k3X39QIDEZ0pRcttUg= +google.golang.org/grpc v1.44.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM= diff --git a/exporters/otlp/otlptrace/internal/otlpconfig/envconfig.go b/exporters/otlp/otlptrace/internal/otlpconfig/envconfig.go index 2d1937beda2..77f13a1937b 100644 --- a/exporters/otlp/otlptrace/internal/otlpconfig/envconfig.go +++ b/exporters/otlp/otlptrace/internal/otlpconfig/envconfig.go @@ -33,12 +33,12 @@ var DefaultEnvOptionsReader = EnvOptionsReader{ ReadFile: ioutil.ReadFile, } -func ApplyGRPCEnvConfigs(cfg *Config) { - DefaultEnvOptionsReader.ApplyGRPCEnvConfigs(cfg) +func ApplyGRPCEnvConfigs(cfg Config) Config { + return DefaultEnvOptionsReader.ApplyGRPCEnvConfigs(cfg) } -func ApplyHTTPEnvConfigs(cfg *Config) { - DefaultEnvOptionsReader.ApplyHTTPEnvConfigs(cfg) +func ApplyHTTPEnvConfigs(cfg Config) Config { + return DefaultEnvOptionsReader.ApplyHTTPEnvConfigs(cfg) } type EnvOptionsReader struct { @@ -46,18 +46,20 @@ type EnvOptionsReader struct { ReadFile func(filename string) ([]byte, error) } -func (e *EnvOptionsReader) ApplyHTTPEnvConfigs(cfg *Config) { +func (e *EnvOptionsReader) ApplyHTTPEnvConfigs(cfg Config) Config { opts := e.GetOptionsFromEnv() for _, opt := range opts { - opt.ApplyHTTPOption(cfg) + cfg = opt.ApplyHTTPOption(cfg) } + return cfg } -func (e *EnvOptionsReader) ApplyGRPCEnvConfigs(cfg *Config) { +func (e *EnvOptionsReader) ApplyGRPCEnvConfigs(cfg Config) Config { opts := e.GetOptionsFromEnv() for _, opt := range opts { - opt.ApplyGRPCOption(cfg) + cfg = opt.ApplyGRPCOption(cfg) } + return cfg } func (e *EnvOptionsReader) GetOptionsFromEnv() []GenericOption { @@ -74,7 +76,7 @@ func (e *EnvOptionsReader) GetOptionsFromEnv() []GenericOption { } else { opts = append(opts, WithSecure()) } - opts = append(opts, newSplitOption(func(cfg *Config) { + opts = append(opts, newSplitOption(func(cfg Config) Config { cfg.Traces.Endpoint = u.Host // For endpoint URLs for OTLP/HTTP per-signal variables, the // URL MUST be used as-is without any modification. The only @@ -85,10 +87,12 @@ func (e *EnvOptionsReader) GetOptionsFromEnv() []GenericOption { path = "/" } cfg.Traces.URLPath = path - }, func(cfg *Config) { + return cfg + }, func(cfg Config) Config { // For OTLP/gRPC endpoints, this is the target to which the // exporter is going to send telemetry. cfg.Traces.Endpoint = path.Join(u.Host, u.Path) + return cfg })) } } else if v, ok = e.getEnvValue("ENDPOINT"); ok { @@ -101,16 +105,18 @@ func (e *EnvOptionsReader) GetOptionsFromEnv() []GenericOption { } else { opts = append(opts, WithSecure()) } - opts = append(opts, newSplitOption(func(cfg *Config) { + opts = append(opts, newSplitOption(func(cfg Config) Config { cfg.Traces.Endpoint = u.Host // For OTLP/HTTP endpoint URLs without a per-signal // configuration, the passed endpoint is used as a base URL // and the signals are sent to these paths relative to that. cfg.Traces.URLPath = path.Join(u.Path, DefaultTracesPath) - }, func(cfg *Config) { + return cfg + }, func(cfg Config) Config { // For OTLP/gRPC endpoints, this is the target to which the // exporter is going to send telemetry. cfg.Traces.Endpoint = path.Join(u.Host, u.Path) + return cfg })) } } diff --git a/exporters/otlp/otlptrace/internal/otlpconfig/options.go b/exporters/otlp/otlptrace/internal/otlpconfig/options.go index 2af734a9de8..e6fb14e00e8 100644 --- a/exporters/otlp/otlptrace/internal/otlpconfig/options.go +++ b/exporters/otlp/otlptrace/internal/otlpconfig/options.go @@ -83,9 +83,9 @@ func NewDefaultConfig() Config { // any unset setting using the default gRPC config values. func NewGRPCConfig(opts ...GRPCOption) Config { cfg := NewDefaultConfig() - ApplyGRPCEnvConfigs(&cfg) + cfg = ApplyGRPCEnvConfigs(cfg) for _, opt := range opts { - opt.ApplyGRPCOption(&cfg) + cfg = opt.ApplyGRPCOption(cfg) } if cfg.ServiceConfig != "" { @@ -122,8 +122,8 @@ func NewGRPCConfig(opts ...GRPCOption) Config { type ( // GenericOption applies an option to the HTTP or gRPC driver. GenericOption interface { - ApplyHTTPOption(*Config) - ApplyGRPCOption(*Config) + ApplyHTTPOption(Config) Config + ApplyGRPCOption(Config) Config // A private method to prevent users implementing the // interface and so future additions to it will not @@ -133,7 +133,7 @@ type ( // HTTPOption applies an option to the HTTP driver. HTTPOption interface { - ApplyHTTPOption(*Config) + ApplyHTTPOption(Config) Config // A private method to prevent users implementing the // interface and so future additions to it will not @@ -143,7 +143,7 @@ type ( // GRPCOption applies an option to the gRPC driver. GRPCOption interface { - ApplyGRPCOption(*Config) + ApplyGRPCOption(Config) Config // A private method to prevent users implementing the // interface and so future additions to it will not @@ -155,128 +155,138 @@ type ( // genericOption is an option that applies the same logic // for both gRPC and HTTP. type genericOption struct { - fn func(*Config) + fn func(Config) Config } -func (g *genericOption) ApplyGRPCOption(cfg *Config) { - g.fn(cfg) +func (g *genericOption) ApplyGRPCOption(cfg Config) Config { + return g.fn(cfg) } -func (g *genericOption) ApplyHTTPOption(cfg *Config) { - g.fn(cfg) +func (g *genericOption) ApplyHTTPOption(cfg Config) Config { + return g.fn(cfg) } func (genericOption) private() {} -func newGenericOption(fn func(cfg *Config)) GenericOption { +func newGenericOption(fn func(cfg Config) Config) GenericOption { return &genericOption{fn: fn} } // splitOption is an option that applies different logics // for gRPC and HTTP. type splitOption struct { - httpFn func(*Config) - grpcFn func(*Config) + httpFn func(Config) Config + grpcFn func(Config) Config } -func (g *splitOption) ApplyGRPCOption(cfg *Config) { - g.grpcFn(cfg) +func (g *splitOption) ApplyGRPCOption(cfg Config) Config { + return g.grpcFn(cfg) } -func (g *splitOption) ApplyHTTPOption(cfg *Config) { - g.httpFn(cfg) +func (g *splitOption) ApplyHTTPOption(cfg Config) Config { + return g.httpFn(cfg) } func (splitOption) private() {} -func newSplitOption(httpFn func(cfg *Config), grpcFn func(cfg *Config)) GenericOption { +func newSplitOption(httpFn func(cfg Config) Config, grpcFn func(cfg Config) Config) GenericOption { return &splitOption{httpFn: httpFn, grpcFn: grpcFn} } // httpOption is an option that is only applied to the HTTP driver. type httpOption struct { - fn func(*Config) + fn func(Config) Config } -func (h *httpOption) ApplyHTTPOption(cfg *Config) { - h.fn(cfg) +func (h *httpOption) ApplyHTTPOption(cfg Config) Config { + return h.fn(cfg) } func (httpOption) private() {} -func NewHTTPOption(fn func(cfg *Config)) HTTPOption { +func NewHTTPOption(fn func(cfg Config) Config) HTTPOption { return &httpOption{fn: fn} } // grpcOption is an option that is only applied to the gRPC driver. type grpcOption struct { - fn func(*Config) + fn func(Config) Config } -func (h *grpcOption) ApplyGRPCOption(cfg *Config) { - h.fn(cfg) +func (h *grpcOption) ApplyGRPCOption(cfg Config) Config { + return h.fn(cfg) } func (grpcOption) private() {} -func NewGRPCOption(fn func(cfg *Config)) GRPCOption { +func NewGRPCOption(fn func(cfg Config) Config) GRPCOption { return &grpcOption{fn: fn} } // Generic Options func WithEndpoint(endpoint string) GenericOption { - return newGenericOption(func(cfg *Config) { + return newGenericOption(func(cfg Config) Config { cfg.Traces.Endpoint = endpoint + return cfg }) } func WithCompression(compression Compression) GenericOption { - return newGenericOption(func(cfg *Config) { + return newGenericOption(func(cfg Config) Config { cfg.Traces.Compression = compression + return cfg }) } func WithURLPath(urlPath string) GenericOption { - return newGenericOption(func(cfg *Config) { + return newGenericOption(func(cfg Config) Config { cfg.Traces.URLPath = urlPath + return cfg }) } func WithRetry(rc retry.Config) GenericOption { - return newGenericOption(func(cfg *Config) { + return newGenericOption(func(cfg Config) Config { cfg.RetryConfig = rc + return cfg }) } func WithTLSClientConfig(tlsCfg *tls.Config) GenericOption { - return newSplitOption(func(cfg *Config) { + return newSplitOption(func(cfg Config) Config { cfg.Traces.TLSCfg = tlsCfg.Clone() - }, func(cfg *Config) { + return cfg + }, func(cfg Config) Config { cfg.Traces.GRPCCredentials = credentials.NewTLS(tlsCfg) + return cfg }) } func WithInsecure() GenericOption { - return newGenericOption(func(cfg *Config) { + return newGenericOption(func(cfg Config) Config { cfg.Traces.Insecure = true + return cfg }) } func WithSecure() GenericOption { - return newGenericOption(func(cfg *Config) { + return newGenericOption(func(cfg Config) Config { cfg.Traces.Insecure = false + return cfg }) } func WithHeaders(headers map[string]string) GenericOption { - return newGenericOption(func(cfg *Config) { + return newGenericOption(func(cfg Config) Config { cfg.Traces.Headers = headers + return cfg }) } func WithTimeout(duration time.Duration) GenericOption { - return newGenericOption(func(cfg *Config) { + return newGenericOption(func(cfg Config) Config { cfg.Traces.Timeout = duration + return cfg }) } diff --git a/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go b/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go index 20decbc37bc..b12e0bd9a5c 100644 --- a/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go +++ b/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go @@ -385,9 +385,9 @@ func TestConfigs(t *testing.T) { // Tests Generic options as HTTP Options cfg := otlpconfig.NewDefaultConfig() - otlpconfig.ApplyHTTPEnvConfigs(&cfg) + cfg = otlpconfig.ApplyHTTPEnvConfigs(cfg) for _, opt := range tt.opts { - opt.ApplyHTTPOption(&cfg) + cfg = opt.ApplyHTTPOption(cfg) } tt.asserts(t, &cfg, false) diff --git a/exporters/otlp/otlptrace/otlptracegrpc/go.mod b/exporters/otlp/otlptrace/otlptracegrpc/go.mod index 55b90f84f98..7324309e2d9 100644 --- a/exporters/otlp/otlptrace/otlptracegrpc/go.mod +++ b/exporters/otlp/otlptrace/otlptracegrpc/go.mod @@ -11,7 +11,7 @@ require ( go.opentelemetry.io/proto/otlp v0.12.0 go.uber.org/goleak v1.1.12 google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013 - google.golang.org/grpc v1.43.0 + google.golang.org/grpc v1.44.0 google.golang.org/protobuf v1.27.1 ) diff --git a/exporters/otlp/otlptrace/otlptracegrpc/go.sum b/exporters/otlp/otlptrace/otlptracegrpc/go.sum index 963b22efe7a..d2dc43a4872 100644 --- a/exporters/otlp/otlptrace/otlptracegrpc/go.sum +++ b/exporters/otlp/otlptrace/otlptracegrpc/go.sum @@ -138,8 +138,9 @@ google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQ google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk= google.golang.org/grpc v1.33.1/go.mod h1:fr5YgcSWrqhRRxogOsw7RzIpsmvOZ6IcH4kBYTpR3n0= google.golang.org/grpc v1.36.0/go.mod h1:qjiiYl8FncCW8feJPdyg3v6XW24KsRHe+dy9BAGRRjU= -google.golang.org/grpc v1.43.0 h1:Eeu7bZtDZ2DpRCsLhUlcrLnvYaMK1Gz86a+hMVvELmM= google.golang.org/grpc v1.43.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU= +google.golang.org/grpc v1.44.0 h1:weqSxi/TMs1SqFRMHCtBgXRs8k3X39QIDEZ0pRcttUg= +google.golang.org/grpc v1.44.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM= diff --git a/exporters/otlp/otlptrace/otlptracegrpc/options.go b/exporters/otlp/otlptrace/otlptracegrpc/options.go index 0b27a35a8e1..e2e5bd696f6 100644 --- a/exporters/otlp/otlptrace/otlptracegrpc/options.go +++ b/exporters/otlp/otlptrace/otlptracegrpc/options.go @@ -28,7 +28,7 @@ import ( // Option applies an option to the gRPC driver. type Option interface { - applyGRPCOption(*otlpconfig.Config) + applyGRPCOption(otlpconfig.Config) otlpconfig.Config } func asGRPCOptions(opts []Option) []otlpconfig.GRPCOption { @@ -50,8 +50,8 @@ type wrappedOption struct { otlpconfig.GRPCOption } -func (w wrappedOption) applyGRPCOption(cfg *otlpconfig.Config) { - w.ApplyGRPCOption(cfg) +func (w wrappedOption) applyGRPCOption(cfg otlpconfig.Config) otlpconfig.Config { + return w.ApplyGRPCOption(cfg) } // WithInsecure disables client transport security for the exporter's gRPC @@ -77,8 +77,9 @@ func WithEndpoint(endpoint string) Option { // // This option has no effect if WithGRPCConn is used. func WithReconnectionPeriod(rp time.Duration) Option { - return wrappedOption{otlpconfig.NewGRPCOption(func(cfg *otlpconfig.Config) { + return wrappedOption{otlpconfig.NewGRPCOption(func(cfg otlpconfig.Config) otlpconfig.Config { cfg.ReconnectionPeriod = rp + return cfg })} } @@ -117,8 +118,9 @@ func WithHeaders(headers map[string]string) Option { // // This option has no effect if WithGRPCConn is used. func WithTLSCredentials(creds credentials.TransportCredentials) Option { - return wrappedOption{otlpconfig.NewGRPCOption(func(cfg *otlpconfig.Config) { + return wrappedOption{otlpconfig.NewGRPCOption(func(cfg otlpconfig.Config) otlpconfig.Config { cfg.Traces.GRPCCredentials = creds + return cfg })} } @@ -126,8 +128,9 @@ func WithTLSCredentials(creds credentials.TransportCredentials) Option { // // This option has no effect if WithGRPCConn is used. func WithServiceConfig(serviceConfig string) Option { - return wrappedOption{otlpconfig.NewGRPCOption(func(cfg *otlpconfig.Config) { + return wrappedOption{otlpconfig.NewGRPCOption(func(cfg otlpconfig.Config) otlpconfig.Config { cfg.ServiceConfig = serviceConfig + return cfg })} } @@ -138,8 +141,9 @@ func WithServiceConfig(serviceConfig string) Option { // // This option has no effect if WithGRPCConn is used. func WithDialOption(opts ...grpc.DialOption) Option { - return wrappedOption{otlpconfig.NewGRPCOption(func(cfg *otlpconfig.Config) { + return wrappedOption{otlpconfig.NewGRPCOption(func(cfg otlpconfig.Config) otlpconfig.Config { cfg.DialOptions = opts + return cfg })} } @@ -152,8 +156,9 @@ func WithDialOption(opts ...grpc.DialOption) Option { // It is the callers responsibility to close the passed conn. The client // Shutdown method will not close this connection. func WithGRPCConn(conn *grpc.ClientConn) Option { - return wrappedOption{otlpconfig.NewGRPCOption(func(cfg *otlpconfig.Config) { + return wrappedOption{otlpconfig.NewGRPCOption(func(cfg otlpconfig.Config) otlpconfig.Config { cfg.GRPCConn = conn + return cfg })} } diff --git a/exporters/otlp/otlptrace/otlptracehttp/client.go b/exporters/otlp/otlptrace/otlptracehttp/client.go index dbc6c5ba785..81487f9b6f9 100644 --- a/exporters/otlp/otlptrace/otlptracehttp/client.go +++ b/exporters/otlp/otlptrace/otlptracehttp/client.go @@ -80,9 +80,9 @@ var _ otlptrace.Client = (*client)(nil) // NewClient creates a new HTTP trace client. func NewClient(opts ...Option) otlptrace.Client { cfg := otlpconfig.NewDefaultConfig() - otlpconfig.ApplyHTTPEnvConfigs(&cfg) + cfg = otlpconfig.ApplyHTTPEnvConfigs(cfg) for _, opt := range opts { - opt.applyHTTPOption(&cfg) + cfg = opt.applyHTTPOption(cfg) } for pathPtr, defaultPath := range map[*string]string{ diff --git a/exporters/otlp/otlptrace/otlptracehttp/go.sum b/exporters/otlp/otlptrace/otlptracehttp/go.sum index 8bfa2adcab2..b41a0280fd8 100644 --- a/exporters/otlp/otlptrace/otlptracehttp/go.sum +++ b/exporters/otlp/otlptrace/otlptracehttp/go.sum @@ -112,8 +112,9 @@ google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQ google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk= google.golang.org/grpc v1.33.1/go.mod h1:fr5YgcSWrqhRRxogOsw7RzIpsmvOZ6IcH4kBYTpR3n0= google.golang.org/grpc v1.36.0/go.mod h1:qjiiYl8FncCW8feJPdyg3v6XW24KsRHe+dy9BAGRRjU= -google.golang.org/grpc v1.43.0 h1:Eeu7bZtDZ2DpRCsLhUlcrLnvYaMK1Gz86a+hMVvELmM= google.golang.org/grpc v1.43.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU= +google.golang.org/grpc v1.44.0 h1:weqSxi/TMs1SqFRMHCtBgXRs8k3X39QIDEZ0pRcttUg= +google.golang.org/grpc v1.44.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0= google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM= diff --git a/exporters/otlp/otlptrace/otlptracehttp/options.go b/exporters/otlp/otlptrace/otlptracehttp/options.go index 5b52f8fc65c..e550cfb5d51 100644 --- a/exporters/otlp/otlptrace/otlptracehttp/options.go +++ b/exporters/otlp/otlptrace/otlptracehttp/options.go @@ -37,7 +37,7 @@ const ( // Option applies an option to the HTTP client. type Option interface { - applyHTTPOption(*otlpconfig.Config) + applyHTTPOption(otlpconfig.Config) otlpconfig.Config } // RetryConfig defines configuration for retrying batches in case of export @@ -48,8 +48,8 @@ type wrappedOption struct { otlpconfig.HTTPOption } -func (w wrappedOption) applyHTTPOption(cfg *otlpconfig.Config) { - w.ApplyHTTPOption(cfg) +func (w wrappedOption) applyHTTPOption(cfg otlpconfig.Config) otlpconfig.Config { + return w.ApplyHTTPOption(cfg) } // WithEndpoint allows one to set the address of the collector diff --git a/exporters/stdout/stdoutmetric/config.go b/exporters/stdout/stdoutmetric/config.go index 097afda83ce..305800ddbde 100644 --- a/exporters/stdout/stdoutmetric/config.go +++ b/exporters/stdout/stdoutmetric/config.go @@ -54,7 +54,7 @@ func newConfig(options ...Option) (config, error) { LabelEncoder: defaultLabelEncoder, } for _, opt := range options { - opt.apply(&cfg) + cfg = opt.apply(cfg) } return cfg, nil @@ -62,7 +62,7 @@ func newConfig(options ...Option) (config, error) { // Option sets the value of an option for a Config. type Option interface { - apply(*config) + apply(config) config } // WithWriter sets the export stream destination. @@ -74,8 +74,9 @@ type writerOption struct { W io.Writer } -func (o writerOption) apply(cfg *config) { +func (o writerOption) apply(cfg config) config { cfg.Writer = o.W + return cfg } // WithPrettyPrint sets the export stream format to use JSON. @@ -85,8 +86,9 @@ func WithPrettyPrint() Option { type prettyPrintOption bool -func (o prettyPrintOption) apply(cfg *config) { +func (o prettyPrintOption) apply(cfg config) config { cfg.PrettyPrint = bool(o) + return cfg } // WithoutTimestamps sets the export stream to not include timestamps. @@ -96,8 +98,9 @@ func WithoutTimestamps() Option { type timestampsOption bool -func (o timestampsOption) apply(cfg *config) { +func (o timestampsOption) apply(cfg config) config { cfg.Timestamps = bool(o) + return cfg } // WithLabelEncoder sets the label encoder used in export. @@ -109,6 +112,7 @@ type labelEncoderOption struct { LabelEncoder attribute.Encoder } -func (o labelEncoderOption) apply(cfg *config) { +func (o labelEncoderOption) apply(cfg config) config { cfg.LabelEncoder = o.LabelEncoder + return cfg } diff --git a/exporters/stdout/stdouttrace/config.go b/exporters/stdout/stdouttrace/config.go index c645ec86455..6b5a97b04cf 100644 --- a/exporters/stdout/stdouttrace/config.go +++ b/exporters/stdout/stdouttrace/config.go @@ -47,7 +47,7 @@ func newConfig(options ...Option) (config, error) { Timestamps: defaultTimestamps, } for _, opt := range options { - opt.apply(&cfg) + cfg = opt.apply(cfg) } return cfg, nil @@ -55,7 +55,7 @@ func newConfig(options ...Option) (config, error) { // Option sets the value of an option for a Config. type Option interface { - apply(*config) + apply(config) config } // WithWriter sets the export stream destination. @@ -67,8 +67,9 @@ type writerOption struct { W io.Writer } -func (o writerOption) apply(cfg *config) { +func (o writerOption) apply(cfg config) config { cfg.Writer = o.W + return cfg } // WithPrettyPrint sets the export stream format to use JSON. @@ -78,8 +79,9 @@ func WithPrettyPrint() Option { type prettyPrintOption bool -func (o prettyPrintOption) apply(cfg *config) { +func (o prettyPrintOption) apply(cfg config) config { cfg.PrettyPrint = bool(o) + return cfg } // WithoutTimestamps sets the export stream to not include timestamps. @@ -89,6 +91,7 @@ func WithoutTimestamps() Option { type timestampsOption bool -func (o timestampsOption) apply(cfg *config) { +func (o timestampsOption) apply(cfg config) config { cfg.Timestamps = bool(o) + return cfg } diff --git a/exporters/zipkin/zipkin.go b/exporters/zipkin/zipkin.go index 23a44a1ba97..5c7c20049d9 100644 --- a/exporters/zipkin/zipkin.go +++ b/exporters/zipkin/zipkin.go @@ -55,26 +55,28 @@ type config struct { // Option defines a function that configures the exporter. type Option interface { - apply(*config) + apply(config) config } -type optionFunc func(*config) +type optionFunc func(config) config -func (fn optionFunc) apply(cfg *config) { - fn(cfg) +func (fn optionFunc) apply(cfg config) config { + return fn(cfg) } // WithLogger configures the exporter to use the passed logger. func WithLogger(logger *log.Logger) Option { - return optionFunc(func(cfg *config) { + return optionFunc(func(cfg config) config { cfg.logger = logger + return cfg }) } // WithClient configures the exporter to use the passed HTTP client. func WithClient(client *http.Client) Option { - return optionFunc(func(cfg *config) { + return optionFunc(func(cfg config) config { cfg.client = client + return cfg }) } @@ -94,7 +96,7 @@ func New(collectorURL string, opts ...Option) (*Exporter, error) { cfg := config{} for _, opt := range opts { - opt.apply(&cfg) + cfg = opt.apply(cfg) } if cfg.client == nil { diff --git a/metric/config.go b/metric/config.go index 686ca7c1acb..3f722344fa7 100644 --- a/metric/config.go +++ b/metric/config.go @@ -38,7 +38,7 @@ func (cfg *InstrumentConfig) Unit() unit.Unit { type InstrumentOption interface { // ApplyMeter is used to set a InstrumentOption value of a // InstrumentConfig. - applyInstrument(*InstrumentConfig) + applyInstrument(InstrumentConfig) InstrumentConfig } // NewInstrumentConfig creates a new InstrumentConfig @@ -46,28 +46,30 @@ type InstrumentOption interface { func NewInstrumentConfig(opts ...InstrumentOption) InstrumentConfig { var config InstrumentConfig for _, o := range opts { - o.applyInstrument(&config) + config = o.applyInstrument(config) } return config } -type instrumentOptionFunc func(*InstrumentConfig) +type instrumentOptionFunc func(InstrumentConfig) InstrumentConfig -func (fn instrumentOptionFunc) applyInstrument(cfg *InstrumentConfig) { - fn(cfg) +func (fn instrumentOptionFunc) applyInstrument(cfg InstrumentConfig) InstrumentConfig { + return fn(cfg) } // WithDescription applies provided description. func WithDescription(desc string) InstrumentOption { - return instrumentOptionFunc(func(cfg *InstrumentConfig) { + return instrumentOptionFunc(func(cfg InstrumentConfig) InstrumentConfig { cfg.description = desc + return cfg }) } // WithUnit applies provided unit. func WithUnit(unit unit.Unit) InstrumentOption { - return instrumentOptionFunc(func(cfg *InstrumentConfig) { + return instrumentOptionFunc(func(cfg InstrumentConfig) InstrumentConfig { cfg.unit = unit + return cfg }) } @@ -90,7 +92,7 @@ func (cfg *MeterConfig) SchemaURL() string { // MeterOption is an interface for applying Meter options. type MeterOption interface { // ApplyMeter is used to set a MeterOption value of a MeterConfig. - applyMeter(*MeterConfig) + applyMeter(MeterConfig) MeterConfig } // NewMeterConfig creates a new MeterConfig and applies @@ -98,27 +100,29 @@ type MeterOption interface { func NewMeterConfig(opts ...MeterOption) MeterConfig { var config MeterConfig for _, o := range opts { - o.applyMeter(&config) + config = o.applyMeter(config) } return config } -type meterOptionFunc func(*MeterConfig) +type meterOptionFunc func(MeterConfig) MeterConfig -func (fn meterOptionFunc) applyMeter(cfg *MeterConfig) { - fn(cfg) +func (fn meterOptionFunc) applyMeter(cfg MeterConfig) MeterConfig { + return fn(cfg) } // WithInstrumentationVersion sets the instrumentation version. func WithInstrumentationVersion(version string) MeterOption { - return meterOptionFunc(func(config *MeterConfig) { + return meterOptionFunc(func(config MeterConfig) MeterConfig { config.instrumentationVersion = version + return config }) } // WithSchemaURL sets the schema URL. func WithSchemaURL(schemaURL string) MeterOption { - return meterOptionFunc(func(config *MeterConfig) { + return meterOptionFunc(func(config MeterConfig) MeterConfig { config.schemaURL = schemaURL + return config }) } diff --git a/sdk/metric/controller/basic/config.go b/sdk/metric/controller/basic/config.go index 01691ecadd1..f3a9830c6af 100644 --- a/sdk/metric/controller/basic/config.go +++ b/sdk/metric/controller/basic/config.go @@ -62,7 +62,7 @@ type config struct { // Option is the interface that applies the value to a configuration option. type Option interface { // apply sets the Option value of a Config. - apply(*config) + apply(config) config } // WithResource sets the Resource configuration option of a Config by merging it @@ -73,12 +73,13 @@ func WithResource(r *resource.Resource) Option { type resourceOption struct{ *resource.Resource } -func (o resourceOption) apply(cfg *config) { +func (o resourceOption) apply(cfg config) config { res, err := resource.Merge(cfg.Resource, o.Resource) if err != nil { otel.Handle(err) } cfg.Resource = res + return cfg } // WithCollectPeriod sets the CollectPeriod configuration option of a Config. @@ -88,8 +89,9 @@ func WithCollectPeriod(period time.Duration) Option { type collectPeriodOption time.Duration -func (o collectPeriodOption) apply(cfg *config) { +func (o collectPeriodOption) apply(cfg config) config { cfg.CollectPeriod = time.Duration(o) + return cfg } // WithCollectTimeout sets the CollectTimeout configuration option of a Config. @@ -99,8 +101,9 @@ func WithCollectTimeout(timeout time.Duration) Option { type collectTimeoutOption time.Duration -func (o collectTimeoutOption) apply(cfg *config) { +func (o collectTimeoutOption) apply(cfg config) config { cfg.CollectTimeout = time.Duration(o) + return cfg } // WithExporter sets the exporter configuration option of a Config. @@ -110,8 +113,9 @@ func WithExporter(exporter export.Exporter) Option { type exporterOption struct{ exporter export.Exporter } -func (o exporterOption) apply(cfg *config) { +func (o exporterOption) apply(cfg config) config { cfg.Exporter = o.exporter + return cfg } // WithPushTimeout sets the PushTimeout configuration option of a Config. @@ -121,6 +125,7 @@ func WithPushTimeout(timeout time.Duration) Option { type pushTimeoutOption time.Duration -func (o pushTimeoutOption) apply(cfg *config) { +func (o pushTimeoutOption) apply(cfg config) config { cfg.PushTimeout = time.Duration(o) + return cfg } diff --git a/sdk/metric/controller/basic/config_test.go b/sdk/metric/controller/basic/config_test.go index d93cc53a677..32757b8a966 100644 --- a/sdk/metric/controller/basic/config_test.go +++ b/sdk/metric/controller/basic/config_test.go @@ -26,12 +26,12 @@ import ( func TestWithResource(t *testing.T) { r := resource.NewSchemaless(attribute.String("A", "a")) - c := &config{} - WithResource(r).apply(c) + c := config{} + c = WithResource(r).apply(c) assert.Equal(t, r.Equivalent(), c.Resource.Equivalent()) // Ensure overwriting works. - c = &config{Resource: &resource.Resource{}} - WithResource(r).apply(c) + c = config{Resource: &resource.Resource{}} + c = WithResource(r).apply(c) assert.Equal(t, r.Equivalent(), c.Resource.Equivalent()) } diff --git a/sdk/metric/controller/basic/controller.go b/sdk/metric/controller/basic/controller.go index a684f65678e..ee694056228 100644 --- a/sdk/metric/controller/basic/controller.go +++ b/sdk/metric/controller/basic/controller.go @@ -112,13 +112,13 @@ type accumulatorCheckpointer struct { // and options (including optional exporter) to configure a metric // export pipeline. func New(checkpointerFactory export.CheckpointerFactory, opts ...Option) *Controller { - c := &config{ + c := config{ CollectPeriod: DefaultPeriod, CollectTimeout: DefaultPeriod, PushTimeout: DefaultPeriod, } for _, opt := range opts { - opt.apply(c) + c = opt.apply(c) } if c.Resource == nil { c.Resource = resource.Default() diff --git a/sdk/metric/processor/basic/basic.go b/sdk/metric/processor/basic/basic.go index 08912da0308..71101bc4bce 100644 --- a/sdk/metric/processor/basic/basic.go +++ b/sdk/metric/processor/basic/basic.go @@ -132,7 +132,7 @@ type factory struct { func NewFactory(aselector export.AggregatorSelector, tselector aggregation.TemporalitySelector, opts ...Option) export.CheckpointerFactory { var config config for _, opt := range opts { - opt.applyProcessor(&config) + config = opt.applyProcessor(config) } return factory{ aselector: aselector, diff --git a/sdk/metric/processor/basic/config.go b/sdk/metric/processor/basic/config.go index f4cf440cf4c..5170864d7e9 100644 --- a/sdk/metric/processor/basic/config.go +++ b/sdk/metric/processor/basic/config.go @@ -24,7 +24,7 @@ type config struct { } type Option interface { - applyProcessor(*config) + applyProcessor(config) config } // WithMemory sets the memory behavior of a Processor. If this is @@ -37,6 +37,7 @@ func WithMemory(memory bool) Option { type memoryOption bool -func (m memoryOption) applyProcessor(cfg *config) { +func (m memoryOption) applyProcessor(cfg config) config { cfg.Memory = bool(m) + return cfg } diff --git a/sdk/resource/config.go b/sdk/resource/config.go index 5fa45859b53..d80b5ae6214 100644 --- a/sdk/resource/config.go +++ b/sdk/resource/config.go @@ -31,7 +31,7 @@ type config struct { // Option is the interface that applies a configuration option. type Option interface { // apply sets the Option value of a config. - apply(*config) + apply(config) config } // WithAttributes adds attributes to the configured Resource. @@ -56,8 +56,9 @@ type detectorsOption struct { detectors []Detector } -func (o detectorsOption) apply(cfg *config) { +func (o detectorsOption) apply(cfg config) config { cfg.detectors = append(cfg.detectors, o.detectors...) + return cfg } // WithFromEnv adds attributes from environment variables to the configured resource. @@ -82,8 +83,9 @@ func WithSchemaURL(schemaURL string) Option { type schemaURLOption string -func (o schemaURLOption) apply(cfg *config) { +func (o schemaURLOption) apply(cfg config) config { cfg.schemaURL = string(o) + return cfg } // WithOS adds all the OS attributes to the configured Resource. diff --git a/sdk/resource/resource.go b/sdk/resource/resource.go index 4ef49b314fc..e842744ae91 100644 --- a/sdk/resource/resource.go +++ b/sdk/resource/resource.go @@ -48,7 +48,7 @@ var errMergeConflictSchemaURL = errors.New("cannot merge resource due to conflic func New(ctx context.Context, opts ...Option) (*Resource, error) { cfg := config{} for _, opt := range opts { - opt.apply(&cfg) + cfg = opt.apply(cfg) } resource, err := Detect(ctx, cfg.detectors...) diff --git a/sdk/trace/provider.go b/sdk/trace/provider.go index 68bdefe0c94..c6b311f9cdc 100644 --- a/sdk/trace/provider.go +++ b/sdk/trace/provider.go @@ -74,10 +74,13 @@ type TracerProvider struct { mu sync.Mutex namedTracer map[instrumentation.Library]*tracer spanProcessors atomic.Value - sampler Sampler - idGenerator IDGenerator - spanLimits SpanLimits - resource *resource.Resource + + // These fields are not protected by the lock mu. They are assumed to be + // immutable after creation of the TracerProvider. + sampler Sampler + idGenerator IDGenerator + spanLimits SpanLimits + resource *resource.Resource } var _ trace.TracerProvider = &TracerProvider{} @@ -93,13 +96,13 @@ var _ trace.TracerProvider = &TracerProvider{} // The passed opts are used to override these default values and configure the // returned TracerProvider appropriately. func NewTracerProvider(opts ...TracerProviderOption) *TracerProvider { - o := &tracerProviderConfig{} + o := tracerProviderConfig{} for _, opt := range opts { - opt.apply(o) + o = opt.apply(o) } - ensureValidTracerProviderConfig(o) + o = ensureValidTracerProviderConfig(o) tp := &TracerProvider{ namedTracer: make(map[instrumentation.Library]*tracer), @@ -256,13 +259,13 @@ func (p *TracerProvider) Shutdown(ctx context.Context) error { } type TracerProviderOption interface { - apply(*tracerProviderConfig) + apply(tracerProviderConfig) tracerProviderConfig } -type traceProviderOptionFunc func(*tracerProviderConfig) +type traceProviderOptionFunc func(tracerProviderConfig) tracerProviderConfig -func (fn traceProviderOptionFunc) apply(cfg *tracerProviderConfig) { - fn(cfg) +func (fn traceProviderOptionFunc) apply(cfg tracerProviderConfig) tracerProviderConfig { + return fn(cfg) } // WithSyncer registers the exporter with the TracerProvider using a @@ -285,8 +288,9 @@ func WithBatcher(e SpanExporter, opts ...BatchSpanProcessorOption) TracerProvide // WithSpanProcessor registers the SpanProcessor with a TracerProvider. func WithSpanProcessor(sp SpanProcessor) TracerProviderOption { - return traceProviderOptionFunc(func(cfg *tracerProviderConfig) { + return traceProviderOptionFunc(func(cfg tracerProviderConfig) tracerProviderConfig { cfg.processors = append(cfg.processors, sp) + return cfg }) } @@ -298,12 +302,13 @@ func WithSpanProcessor(sp SpanProcessor) TracerProviderOption { // If this option is not used, the TracerProvider will use the // resource.Default() Resource by default. func WithResource(r *resource.Resource) TracerProviderOption { - return traceProviderOptionFunc(func(cfg *tracerProviderConfig) { + return traceProviderOptionFunc(func(cfg tracerProviderConfig) tracerProviderConfig { var err error cfg.resource, err = resource.Merge(resource.Environment(), r) if err != nil { otel.Handle(err) } + return cfg }) } @@ -315,10 +320,11 @@ func WithResource(r *resource.Resource) TracerProviderOption { // If this option is not used, the TracerProvider will use a random number // IDGenerator by default. func WithIDGenerator(g IDGenerator) TracerProviderOption { - return traceProviderOptionFunc(func(cfg *tracerProviderConfig) { + return traceProviderOptionFunc(func(cfg tracerProviderConfig) tracerProviderConfig { if g != nil { cfg.idGenerator = g } + return cfg }) } @@ -330,10 +336,11 @@ func WithIDGenerator(g IDGenerator) TracerProviderOption { // If this option is not used, the TracerProvider will use a // ParentBased(AlwaysSample) Sampler by default. func WithSampler(s Sampler) TracerProviderOption { - return traceProviderOptionFunc(func(cfg *tracerProviderConfig) { + return traceProviderOptionFunc(func(cfg tracerProviderConfig) tracerProviderConfig { if s != nil { cfg.sampler = s } + return cfg }) } @@ -345,13 +352,14 @@ func WithSampler(s Sampler) TracerProviderOption { // If this option is not used, the TracerProvider will use the default // SpanLimits. func WithSpanLimits(sl SpanLimits) TracerProviderOption { - return traceProviderOptionFunc(func(cfg *tracerProviderConfig) { + return traceProviderOptionFunc(func(cfg tracerProviderConfig) tracerProviderConfig { cfg.spanLimits = sl + return cfg }) } // ensureValidTracerProviderConfig ensures that given TracerProviderConfig is valid. -func ensureValidTracerProviderConfig(cfg *tracerProviderConfig) { +func ensureValidTracerProviderConfig(cfg tracerProviderConfig) tracerProviderConfig { if cfg.sampler == nil { cfg.sampler = ParentBased(AlwaysSample()) } @@ -362,4 +370,5 @@ func ensureValidTracerProviderConfig(cfg *tracerProviderConfig) { if cfg.resource == nil { cfg.resource = resource.Default() } + return cfg } diff --git a/sdk/trace/sampling.go b/sdk/trace/sampling.go index 849248638c4..a4ac588f666 100644 --- a/sdk/trace/sampling.go +++ b/sdk/trace/sampling.go @@ -187,7 +187,7 @@ func configureSamplersForParentBased(samplers []ParentBasedSamplerOption) sample } for _, so := range samplers { - so.apply(&c) + c = so.apply(c) } return c @@ -201,7 +201,7 @@ type samplerConfig struct { // ParentBasedSamplerOption configures the sampler for a particular sampling case. type ParentBasedSamplerOption interface { - apply(*samplerConfig) + apply(samplerConfig) samplerConfig } // WithRemoteParentSampled sets the sampler for the case of sampled remote parent. @@ -213,8 +213,9 @@ type remoteParentSampledOption struct { s Sampler } -func (o remoteParentSampledOption) apply(config *samplerConfig) { +func (o remoteParentSampledOption) apply(config samplerConfig) samplerConfig { config.remoteParentSampled = o.s + return config } // WithRemoteParentNotSampled sets the sampler for the case of remote parent @@ -227,8 +228,9 @@ type remoteParentNotSampledOption struct { s Sampler } -func (o remoteParentNotSampledOption) apply(config *samplerConfig) { +func (o remoteParentNotSampledOption) apply(config samplerConfig) samplerConfig { config.remoteParentNotSampled = o.s + return config } // WithLocalParentSampled sets the sampler for the case of sampled local parent. @@ -240,8 +242,9 @@ type localParentSampledOption struct { s Sampler } -func (o localParentSampledOption) apply(config *samplerConfig) { +func (o localParentSampledOption) apply(config samplerConfig) samplerConfig { config.localParentSampled = o.s + return config } // WithLocalParentNotSampled sets the sampler for the case of local parent @@ -254,8 +257,9 @@ type localParentNotSampledOption struct { s Sampler } -func (o localParentNotSampledOption) apply(config *samplerConfig) { +func (o localParentNotSampledOption) apply(config samplerConfig) samplerConfig { config.localParentNotSampled = o.s + return config } func (pb parentBased) ShouldSample(p SamplingParameters) SamplingResult { diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 0111c475895..bf0c41c1112 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -126,14 +126,6 @@ type recordingSpan struct { // childSpanCount holds the number of child spans created for this span. childSpanCount int - // resource contains attributes representing an entity that produced this - // span. - resource *resource.Resource - - // instrumentationLibrary defines the instrumentation library used to - // provide instrumentation. - instrumentationLibrary instrumentation.Library - // spanContext holds the SpanContext of this span. spanContext trace.SpanContext @@ -152,9 +144,6 @@ type recordingSpan struct { // tracer is the SDK tracer that created this span. tracer *tracer - - // spanLimits holds the limits to this span. - spanLimits SpanLimits } var _ ReadWriteSpan = (*recordingSpan)(nil) @@ -336,9 +325,9 @@ func (s *recordingSpan) addEvent(name string, o ...trace.EventOption) { // Discard over limited attributes attributes := c.Attributes() var discarded int - if len(attributes) > s.spanLimits.AttributePerEventCountLimit { - discarded = len(attributes) - s.spanLimits.AttributePerEventCountLimit - attributes = attributes[:s.spanLimits.AttributePerEventCountLimit] + if len(attributes) > s.tracer.provider.spanLimits.AttributePerEventCountLimit { + discarded = len(attributes) - s.tracer.provider.spanLimits.AttributePerEventCountLimit + attributes = attributes[:s.tracer.provider.spanLimits.AttributePerEventCountLimit] } s.mu.Lock() defer s.mu.Unlock() @@ -440,7 +429,7 @@ func (s *recordingSpan) Status() Status { func (s *recordingSpan) InstrumentationLibrary() instrumentation.Library { s.mu.Lock() defer s.mu.Unlock() - return s.instrumentationLibrary + return s.tracer.instrumentationLibrary } // Resource returns the Resource associated with the Tracer that created this @@ -448,7 +437,7 @@ func (s *recordingSpan) InstrumentationLibrary() instrumentation.Library { func (s *recordingSpan) Resource() *resource.Resource { s.mu.Lock() defer s.mu.Unlock() - return s.resource + return s.tracer.provider.resource } func (s *recordingSpan) addLink(link trace.Link) { @@ -461,9 +450,9 @@ func (s *recordingSpan) addLink(link trace.Link) { var droppedAttributeCount int // Discard over limited attributes - if len(link.Attributes) > s.spanLimits.AttributePerLinkCountLimit { - droppedAttributeCount = len(link.Attributes) - s.spanLimits.AttributePerLinkCountLimit - link.Attributes = link.Attributes[:s.spanLimits.AttributePerLinkCountLimit] + if len(link.Attributes) > s.tracer.provider.spanLimits.AttributePerLinkCountLimit { + droppedAttributeCount = len(link.Attributes) - s.tracer.provider.spanLimits.AttributePerLinkCountLimit + link.Attributes = link.Attributes[:s.tracer.provider.spanLimits.AttributePerLinkCountLimit] } s.links.add(Link{link.SpanContext, link.Attributes, droppedAttributeCount}) @@ -514,10 +503,10 @@ func (s *recordingSpan) snapshot() ReadOnlySpan { defer s.mu.Unlock() sd.endTime = s.endTime - sd.instrumentationLibrary = s.instrumentationLibrary + sd.instrumentationLibrary = s.tracer.instrumentationLibrary sd.name = s.name sd.parent = s.parent - sd.resource = s.resource + sd.resource = s.tracer.provider.resource sd.spanContext = s.spanContext sd.spanKind = s.spanKind sd.startTime = s.startTime diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go index 1177c729a8f..b63b4196516 100644 --- a/sdk/trace/tracer.go +++ b/sdk/trace/tracer.go @@ -122,18 +122,15 @@ func (tr *tracer) newRecordingSpan(psc, sc trace.SpanContext, name string, sr Sa } s := &recordingSpan{ - parent: psc, - spanContext: sc, - spanKind: trace.ValidateSpanKind(config.SpanKind()), - name: name, - startTime: startTime, - attributes: newAttributesMap(tr.provider.spanLimits.AttributeCountLimit), - events: newEvictedQueue(tr.provider.spanLimits.EventCountLimit), - links: newEvictedQueue(tr.provider.spanLimits.LinkCountLimit), - tracer: tr, - spanLimits: tr.provider.spanLimits, - resource: tr.provider.resource, - instrumentationLibrary: tr.instrumentationLibrary, + parent: psc, + spanContext: sc, + spanKind: trace.ValidateSpanKind(config.SpanKind()), + name: name, + startTime: startTime, + attributes: newAttributesMap(tr.provider.spanLimits.AttributeCountLimit), + events: newEvictedQueue(tr.provider.spanLimits.EventCountLimit), + links: newEvictedQueue(tr.provider.spanLimits.LinkCountLimit), + tracer: tr, } for _, l := range config.Links() { diff --git a/trace/config.go b/trace/config.go index 8461a15ccd4..bcc333e04e2 100644 --- a/trace/config.go +++ b/trace/config.go @@ -41,20 +41,20 @@ func (t *TracerConfig) SchemaURL() string { func NewTracerConfig(options ...TracerOption) TracerConfig { var config TracerConfig for _, option := range options { - option.apply(&config) + config = option.apply(config) } return config } // TracerOption applies an option to a TracerConfig. type TracerOption interface { - apply(*TracerConfig) + apply(TracerConfig) TracerConfig } -type tracerOptionFunc func(*TracerConfig) +type tracerOptionFunc func(TracerConfig) TracerConfig -func (fn tracerOptionFunc) apply(cfg *TracerConfig) { - fn(cfg) +func (fn tracerOptionFunc) apply(cfg TracerConfig) TracerConfig { + return fn(cfg) } // SpanConfig is a group of options for a Span. @@ -106,7 +106,7 @@ func (cfg *SpanConfig) SpanKind() SpanKind { func NewSpanStartConfig(options ...SpanStartOption) SpanConfig { var c SpanConfig for _, option := range options { - option.applySpanStart(&c) + c = option.applySpanStart(c) } return c } @@ -118,7 +118,7 @@ func NewSpanStartConfig(options ...SpanStartOption) SpanConfig { func NewSpanEndConfig(options ...SpanEndOption) SpanConfig { var c SpanConfig for _, option := range options { - option.applySpanEnd(&c) + c = option.applySpanEnd(c) } return c } @@ -126,19 +126,19 @@ func NewSpanEndConfig(options ...SpanEndOption) SpanConfig { // SpanStartOption applies an option to a SpanConfig. These options are applicable // only when the span is created type SpanStartOption interface { - applySpanStart(*SpanConfig) + applySpanStart(SpanConfig) SpanConfig } -type spanOptionFunc func(*SpanConfig) +type spanOptionFunc func(SpanConfig) SpanConfig -func (fn spanOptionFunc) applySpanStart(cfg *SpanConfig) { - fn(cfg) +func (fn spanOptionFunc) applySpanStart(cfg SpanConfig) SpanConfig { + return fn(cfg) } // SpanEndOption applies an option to a SpanConfig. These options are // applicable only when the span is ended. type SpanEndOption interface { - applySpanEnd(*SpanConfig) + applySpanEnd(SpanConfig) SpanConfig } // EventConfig is a group of options for an Event. @@ -170,7 +170,7 @@ func (cfg *EventConfig) StackTrace() bool { func NewEventConfig(options ...EventOption) EventConfig { var c EventConfig for _, option := range options { - option.applyEvent(&c) + c = option.applyEvent(c) } if c.timestamp.IsZero() { c.timestamp = time.Now() @@ -180,7 +180,7 @@ func NewEventConfig(options ...EventOption) EventConfig { // EventOption applies span event options to an EventConfig. type EventOption interface { - applyEvent(*EventConfig) + applyEvent(EventConfig) EventConfig } // SpanOption are options that can be used at both the beginning and end of a span. @@ -203,12 +203,14 @@ type SpanEndEventOption interface { type attributeOption []attribute.KeyValue -func (o attributeOption) applySpan(c *SpanConfig) { +func (o attributeOption) applySpan(c SpanConfig) SpanConfig { c.attributes = append(c.attributes, []attribute.KeyValue(o)...) + return c } -func (o attributeOption) applySpanStart(c *SpanConfig) { o.applySpan(c) } -func (o attributeOption) applyEvent(c *EventConfig) { +func (o attributeOption) applySpanStart(c SpanConfig) SpanConfig { return o.applySpan(c) } +func (o attributeOption) applyEvent(c EventConfig) EventConfig { c.attributes = append(c.attributes, []attribute.KeyValue(o)...) + return c } var _ SpanStartEventOption = attributeOption{} @@ -234,10 +236,16 @@ type SpanEventOption interface { type timestampOption time.Time -func (o timestampOption) applySpan(c *SpanConfig) { c.timestamp = time.Time(o) } -func (o timestampOption) applySpanStart(c *SpanConfig) { o.applySpan(c) } -func (o timestampOption) applySpanEnd(c *SpanConfig) { o.applySpan(c) } -func (o timestampOption) applyEvent(c *EventConfig) { c.timestamp = time.Time(o) } +func (o timestampOption) applySpan(c SpanConfig) SpanConfig { + c.timestamp = time.Time(o) + return c +} +func (o timestampOption) applySpanStart(c SpanConfig) SpanConfig { return o.applySpan(c) } +func (o timestampOption) applySpanEnd(c SpanConfig) SpanConfig { return o.applySpan(c) } +func (o timestampOption) applyEvent(c EventConfig) EventConfig { + c.timestamp = time.Time(o) + return c +} var _ SpanEventOption = timestampOption{} @@ -249,9 +257,15 @@ func WithTimestamp(t time.Time) SpanEventOption { type stackTraceOption bool -func (o stackTraceOption) applyEvent(c *EventConfig) { c.stackTrace = bool(o) } -func (o stackTraceOption) applySpan(c *SpanConfig) { c.stackTrace = bool(o) } -func (o stackTraceOption) applySpanEnd(c *SpanConfig) { o.applySpan(c) } +func (o stackTraceOption) applyEvent(c EventConfig) EventConfig { + c.stackTrace = bool(o) + return c +} +func (o stackTraceOption) applySpan(c SpanConfig) SpanConfig { + c.stackTrace = bool(o) + return c +} +func (o stackTraceOption) applySpanEnd(c SpanConfig) SpanConfig { return o.applySpan(c) } // WithStackTrace sets the flag to capture the error with stack trace (e.g. true, false). func WithStackTrace(b bool) SpanEndEventOption { @@ -261,8 +275,9 @@ func WithStackTrace(b bool) SpanEndEventOption { // WithLinks adds links to a Span. The links are added to the existing Span // links, i.e. this does not overwrite. Links with invalid span context are ignored. func WithLinks(links ...Link) SpanStartOption { - return spanOptionFunc(func(cfg *SpanConfig) { + return spanOptionFunc(func(cfg SpanConfig) SpanConfig { cfg.links = append(cfg.links, links...) + return cfg }) } @@ -270,28 +285,32 @@ func WithLinks(links ...Link) SpanStartOption { // existing parent span context will be ignored when defining the Span's trace // identifiers. func WithNewRoot() SpanStartOption { - return spanOptionFunc(func(cfg *SpanConfig) { + return spanOptionFunc(func(cfg SpanConfig) SpanConfig { cfg.newRoot = true + return cfg }) } // WithSpanKind sets the SpanKind of a Span. func WithSpanKind(kind SpanKind) SpanStartOption { - return spanOptionFunc(func(cfg *SpanConfig) { + return spanOptionFunc(func(cfg SpanConfig) SpanConfig { cfg.spanKind = kind + return cfg }) } // WithInstrumentationVersion sets the instrumentation version. func WithInstrumentationVersion(version string) TracerOption { - return tracerOptionFunc(func(cfg *TracerConfig) { + return tracerOptionFunc(func(cfg TracerConfig) TracerConfig { cfg.instrumentationVersion = version + return cfg }) } // WithSchemaURL sets the schema URL for the Tracer. func WithSchemaURL(schemaURL string) TracerOption { - return tracerOptionFunc(func(cfg *TracerConfig) { + return tracerOptionFunc(func(cfg TracerConfig) TracerConfig { cfg.schemaURL = schemaURL + return cfg }) } diff --git a/trace/config_test.go b/trace/config_test.go index 877bd024eeb..a4cafcbcd09 100644 --- a/trace/config_test.go +++ b/trace/config_test.go @@ -252,3 +252,71 @@ func TestTracerConfig(t *testing.T) { assert.Equal(t, test.expected, config) } } + +// Save benchmark results to a file level var to avoid the compiler optimizing +// away the actual work. +var ( + tracerConfig TracerConfig + spanConfig SpanConfig + eventConfig EventConfig +) + +func BenchmarkNewTracerConfig(b *testing.B) { + opts := []TracerOption{ + WithInstrumentationVersion("testing verion"), + WithSchemaURL("testing URL"), + } + + b.ReportAllocs() + b.ResetTimer() + + for i := 0; i < b.N; i++ { + tracerConfig = NewTracerConfig(opts...) + } +} + +func BenchmarkNewSpanStartConfig(b *testing.B) { + opts := []SpanStartOption{ + WithAttributes(attribute.Bool("key", true)), + WithTimestamp(time.Now()), + WithLinks(Link{}), + WithNewRoot(), + WithSpanKind(SpanKindClient), + } + + b.ReportAllocs() + b.ResetTimer() + + for i := 0; i < b.N; i++ { + spanConfig = NewSpanStartConfig(opts...) + } +} + +func BenchmarkNewSpanEndConfig(b *testing.B) { + opts := []SpanEndOption{ + WithTimestamp(time.Now()), + WithStackTrace(true), + } + + b.ReportAllocs() + b.ResetTimer() + + for i := 0; i < b.N; i++ { + spanConfig = NewSpanEndConfig(opts...) + } +} + +func BenchmarkNewEventConfig(b *testing.B) { + opts := []EventOption{ + WithAttributes(attribute.Bool("key", true)), + WithTimestamp(time.Now()), + WithStackTrace(true), + } + + b.ReportAllocs() + b.ResetTimer() + + for i := 0; i < b.N; i++ { + eventConfig = NewEventConfig(opts...) + } +} diff --git a/website_docs/_index.md b/website_docs/_index.md index 818e0c26eab..a21cc50329d 100644 --- a/website_docs/_index.md +++ b/website_docs/_index.md @@ -7,7 +7,7 @@ aliases: [/golang, /golang/metrics, /golang/tracing] cascade: github_repo: &repo https://github.com/open-telemetry/opentelemetry-go github_subdir: website_docs - path_base_for_github_subdir: content/en/docs/go/ + path_base_for_github_subdir: content/en/docs/instrumentation/go/ github_project_repo: *repo spelling: cSpell:ignore godoc weight: 16