From 2fe8b577f30b2e80455fd4b5d43e13654d21fcaa Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 2 Sep 2020 07:20:52 -0700 Subject: [PATCH] Update trace API TracerOption (#1109) * Update Tracer configuration. Conform to API option design outlined in #536. Add tests to validate new TracerConfigure function Drop the `instrumentation` prefix. * Stick with instrumentationVersion for now * Propagate changes * Add changes to Changelog --- CHANGELOG.md | 6 ++++ api/trace/api.go | 34 ++++++++++++++------ api/trace/traceprovider_test.go | 57 +++++++++++++++++++++++++++++++++ api/trace/tracetest/provider.go | 5 +-- sdk/trace/provider.go | 5 +-- 5 files changed, 90 insertions(+), 17 deletions(-) create mode 100644 api/trace/traceprovider_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 22a6a61bd943..142c9da45c87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,9 +10,15 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added +- In the `go.opentelemetry.io/otel/api/trace` package a new `TracerConfigure` function was added to configure a new `TracerConfig`. + This addition was made to conform with our project option conventions. (#1109) + +### Changed + - Add reconnecting udp connection type to Jaeger exporter. This change adds a new optional implementation of the udp conn interface used to detect changes to an agent's host dns record. It then adopts the new destination address to ensure the exporter doesn't get stuck. This change was ported from jaegertracing/jaeger-client-go#520. (#1063) +- The `go.opentelemetry.io/otel/api/trace` `TracerOption` was changed to an interface to conform to project option conventions. (#1109) ## [0.11.0] - 2020-08-24 diff --git a/api/trace/api.go b/api/trace/api.go index 19528d43bda5..c55fcb31d93e 100644 --- a/api/trace/api.go +++ b/api/trace/api.go @@ -32,22 +32,38 @@ type Provider interface { Tracer(instrumentationName string, opts ...TracerOption) Tracer } -// TODO (MrAlias): unify this API option design: -// https://github.com/open-telemetry/opentelemetry-go/issues/536 - -// TracerConfig contains options for a Tracer. +// TracerConfig is a group of options for a Tracer. type TracerConfig struct { + // InstrumentationVersion is the version of the instrumentation library. InstrumentationVersion string } -// TracerOption configures a TracerConfig option. -type TracerOption func(*TracerConfig) +// TracerConfigure applies all the options to a returned TracerConfig. +// The default value for all the fields of the returned TracerConfig are the +// default zero value of the type. Also, this does not perform any validation +// on the returned TracerConfig (e.g. no uniqueness checking or bounding of +// data), instead it is left to the implementations of the SDK to perform this +// action. +func TracerConfigure(options []TracerOption) *TracerConfig { + config := new(TracerConfig) + for _, option := range options { + option.Apply(config) + } + return config +} + +// TracerOption applies an options to a TracerConfig. +type TracerOption interface { + Apply(*TracerConfig) +} + +type instVersionTracerOption string + +func (o instVersionTracerOption) Apply(c *TracerConfig) { c.InstrumentationVersion = string(o) } // WithInstrumentationVersion sets the instrumentation version for a Tracer. func WithInstrumentationVersion(version string) TracerOption { - return func(c *TracerConfig) { - c.InstrumentationVersion = version - } + return instVersionTracerOption(version) } type Tracer interface { diff --git a/api/trace/traceprovider_test.go b/api/trace/traceprovider_test.go new file mode 100644 index 000000000000..9e28841b2937 --- /dev/null +++ b/api/trace/traceprovider_test.go @@ -0,0 +1,57 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package trace + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestTracerConfigure(t *testing.T) { + v1 := "semver:0.0.1" + v2 := "semver:1.0.0" + tests := []struct { + options []TracerOption + expected *TracerConfig + }{ + { + // No non-zero-values should be set. + []TracerOption{}, + new(TracerConfig), + }, + { + []TracerOption{ + WithInstrumentationVersion(v1), + }, + &TracerConfig{ + InstrumentationVersion: v1, + }, + }, + { + []TracerOption{ + // Multiple calls should overwrite. + WithInstrumentationVersion(v1), + WithInstrumentationVersion(v2), + }, + &TracerConfig{ + InstrumentationVersion: v2, + }, + }, + } + for _, test := range tests { + assert.Equal(t, test.expected, TracerConfigure(test.options)) + } +} diff --git a/api/trace/tracetest/provider.go b/api/trace/tracetest/provider.go index 8388fc87e751..3668c6119a92 100644 --- a/api/trace/tracetest/provider.go +++ b/api/trace/tracetest/provider.go @@ -41,10 +41,7 @@ type instrumentation struct { } func (p *Provider) Tracer(instName string, opts ...trace.TracerOption) trace.Tracer { - conf := new(trace.TracerConfig) - for _, o := range opts { - o(conf) - } + conf := trace.TracerConfigure(opts) inst := instrumentation{ Name: instName, Version: conf.InstrumentationVersion, diff --git a/sdk/trace/provider.go b/sdk/trace/provider.go index 82a67e2cc1bf..90dea4aa06cc 100644 --- a/sdk/trace/provider.go +++ b/sdk/trace/provider.go @@ -95,10 +95,7 @@ func NewProvider(opts ...ProviderOption) (*Provider, error) { // Tracer with the given name. If a tracer for the given name does not exist, // it is created first. If the name is empty, DefaultTracerName is used. func (p *Provider) Tracer(name string, opts ...apitrace.TracerOption) apitrace.Tracer { - c := new(apitrace.TracerConfig) - for _, o := range opts { - o(c) - } + c := apitrace.TracerConfigure(opts) p.mu.Lock() defer p.mu.Unlock() if name == "" {